-
Notifications
You must be signed in to change notification settings - Fork 92
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Reject proof harnesses with arguments #2132
Conversation
- Change `#[kani::proof]` expansion so it doesn't include `#[no_mangle]` but includes `[allow(dead_code)]`. Fixes model-checking#661 and fixes model-checking#689. - Add a check for harnesses with arguments and merge the checks into one function. Fixes model-checking#1919
One other thing: Can we change the PR title? Want to ensure that whoever write the release notes will notice that this is a possible "breaking" change. Maybe:
|
let all_attributes = self.tcx.get_attrs_unchecked(def_id); | ||
let (proof_attributes, _) = partition_kanitool_attributes(all_attributes); | ||
/// Check that if an item is tagged with a proof_attribute, it is a valid harness. | ||
fn check_proof_attribute(&self, def_id: DefId, proof_attributes: Vec<&Attribute>) { | ||
if !proof_attributes.is_empty() { |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This condition is also checked by the callers. Should we remove it and turn it into an assert?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Sure
Co-authored-by: Zyad Hassan <88045115+zhassan-aws@users.noreply.github.com>
I had forgotten to add the tests before. Sorry!
#[kani::proof]
handling
Description of changes:
Change
#[kani::proof]
expansion so it doesn't include#[no_mangle]
but includes[allow(dead_code)]
. (RMC should use fully qualified name for harness selection and it should not rely onno_mangle
attribute #661 and Introduce rmctool attribute validation #689).Add a check for harnesses with arguments and merge the checks into one function (Inconsistent behavior when functions with parameters are used as harnesses #1919).
Resolved issues:
Fixes #661
Fixes #689
Fixes #1919
Related RFC:
Call-outs:
Testing:
How is this change tested? New test
Is this a refactor change?
Checklist
By submitting this pull request, I confirm that my contribution is made under the terms of the Apache 2.0 and MIT licenses.