-
Notifications
You must be signed in to change notification settings - Fork 99
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
Allow multiple annotations, but check for duplicate targets. #3808
base: main
Are you sure you want to change the base?
Allow multiple annotations, but check for duplicate targets. #3808
Conversation
Is this a duplicate of #3805? |
if seen.contains(&name) { | ||
dcx.struct_span_err( | ||
span, | ||
format!("Multiple occurrences of `stub_verified({})`.", name), | ||
) | ||
.with_span_note( | ||
self.tcx.def_span(def_id), | ||
format!("Use a single `stub_verified({})` annotation.", name), | ||
) | ||
.emit(); | ||
} else { |
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.
Can this be a warning instead? They will be redundant, but they can be safely ignored.
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.
Changed it to a warning, and moved the checks earlier in the process.
@@ -371,7 +371,7 @@ impl<'tcx> KaniAttributes<'tcx> { | |||
attrs.iter().for_each(|attr| self.check_proof_attribute(kind, attr)) | |||
} | |||
KaniAttributeKind::StubVerified => { | |||
expect_single(self.tcx, kind, &attrs); | |||
// Actual validation happens when the annotation are handled |
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.
Ideally, this should be similar to parse_stubs
so we can emit as many errors as possible, and sooner rather than later. Can you please add a test to see if that is the case today. Something like:
#[kani::proof]
#[kani::stub_verified(dummy)]
fn stub_does_not_exist() { }
#[kani::proof]
#[kani::stub_verified(dummy, dummy2)]
fn stub_invalid_args() { }
That said, for this PR, I think a TODO comment should suffice, since you are not introducing this limitation. Thanks!
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.
We already have tests for these cases in missing_contracts_for_check.rs
and missing_contracts_for_replace.rs
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.
Thanks @remi-delmas-3000
Resolves #3804.
Enables multiple
stub_verified(TARGET)
annotations on a harness, but still detect and report duplicate targets.Adds positive and negative tests.
By submitting this pull request, I confirm that my contribution is made under the terms of the Apache 2.0 and MIT licenses.