-
Notifications
You must be signed in to change notification settings - Fork 104
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
RFC: Attribute to distinguish safety preconditions from panic freedom #3893
base: main
Are you sure you want to change the base?
Conversation
This is to discuss the proposed solution for model-checking#3567 (Separate safety contract from correctness / panic freedom).
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 for writing this up!
|
||
**We recommend that you leave the Software Design section empty for the first version of your RFC**. | ||
|
||
Initial implementation suggestion: we will run Kani twice for any such harness |
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 seems sound to me, which is obviously our first priority -- I would love if we could come up with something that didn't require running twice, but I don't currently have any ideas.
Co-authored-by: Carolyn Zech <[email protected]>
Co-authored-by: Carolyn Zech <[email protected]>
|
||
The linked issue contains suggestions for alternative means to describe panic | ||
conditions, most notably `panic_if`. This was ruled out as it may at times not | ||
be possible to exactly describe the conditions under which a panic _must_ occur |
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.
Do you have an example?
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.
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 sounds to me like a corner case. The more common case is that a function's documentation includes a section of the panic condition (e.g. https://doc.rust-lang.org/std/string/struct.String.html#panics), and thus is clearly defined in terms of the function's input parameters.
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.
Okay, so your argument is that it's better to introduce #[panics_if(cond)]
that provides a stronger guarantee that it must panic, with the tradeoff that it's not applicable to cases like the linked issue?
Also, there isn't fundamentally a reason we can't just have both #[panics_if(cond)]
and #[may_panic_if(cond)]
, as long as we document the differences well. Although at that point it may make more sense to just have #[panics_if(cond)]
and #[may_panic]
, where #[may_panic]
just says "if it panics, succeed; otherwise, succeed iff the postcondition holds.
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.
Also, per #3567 (comment):
And it's worth noting it's always possible to implement panic_if in terms of may_panic and ensures (#[panics_if(cond)] is equivalent to #[may_panic(cond)] #[ensures(!cond)]) so if there's a strong desire for both it's not significantly more work than simply having the permissive variant.
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.
#[may_panic], where #[may_panic] just says "if it panics, succeed; otherwise, succeed iff the postcondition holds.
Which would be the same as the #[should_panic]
that harnesses can be annotated with?
(#[panics_if(cond)] is equivalent to #[may_panic(cond)] #[ensures(!cond)])
I don't see how the two are equivalent since cond
is essentially a pre-condition, so its not clear to me how adding #[ensures(!cond)]
would provide the stronger guarantee that #[panics_if]
provides.
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.
Which would be the same as the #[should_panic] that harnesses can be annotated with?
Not quite, e.g.:
#[kani::proof_for_contract(]
#[kani::should_panic]
fn prove_foo() {
foo();
}
fails with:
VERIFICATION:- FAILED (encountered no panics, but at least one was expected)
whereas with #[may_panic]
, it would succeed. (If I could go back and bikeshed, I would have named #[should_panic]
#[must_panic]
instead 😄 )
I don't see how the two are equivalent since cond is essentially a pre-condition, so its not clear to me how adding #[ensures(!cond)] would provide the stronger guarantee that #[panics_if] provides.
We only check the postcondition if the function does not panic, so if we reach the point where we are enforcing the postcondition, cond
must not hold. (If it does hold, then we should have panicked).
Taking your example from #3567 (comment),
#[kani::requires(true)] // the function is safe
#[kani::panics_if(self.is_none())]
#[kani::ensures(|result| if let Some(x) = self { result == x })]
would become
#[kani::requires(true)] // the function is safe
#[kani::may_panic_if(self.is_none())]
// `panic_if(cond)` semantics imply that we check the postcondition iff !cond,
// so if are checking the `#[ensures]`, `cond = self.is_none()` cannot hold
#[kani::ensures(|| !self.is_none())]
#[kani::ensures(|result| if let Some(x) = self { result == x })]
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.
Perhaps it needs to be #[ensures(old(!cond))]
instead in case the function mutates self
?
This is to discuss the proposed solution for #3567 (Separate safety contract from correctness / panic freedom).
By submitting this pull request, I confirm that my contribution is made under the terms of the Apache 2.0 and MIT licenses.