-
Notifications
You must be signed in to change notification settings - Fork 1
fix: Wrap preconditions in a closure to prevent early return #69
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
Conversation
This stack of pull requests is managed by Graphite. Learn more about stacking. |
4480c59 to
a4cf0b8
Compare
820fa74 to
15af57f
Compare
15af57f to
dd590ad
Compare
a4cf0b8 to
478df50
Compare
dd590ad to
97a6037
Compare
97a6037 to
3a9b605
Compare
mkovaxx
left a comment
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 is looking good, thanks!
Please add/extend an integration test in anodized to demonstrate that return statements in conditions are now handled correctly.
The easiest way is likely to change the following test case to be "maximally evil":
anodized/crates/anodized/tests/execution_order.rs
Lines 3 to 24 in 478df50
| #[spec( | |
| requires: [ | |
| log.push("requires1") == (), | |
| log.push("requires2") == (), | |
| ], | |
| maintains: [ | |
| log.push("maintains1") == (), | |
| log.push("maintains2") == (), | |
| ], | |
| captures: [ | |
| log.push("captures1") as _alias1, | |
| log.push("captures2") as _alias2, | |
| ], | |
| ensures: [ | |
| log.push("ensures1") == (), | |
| log.push("ensures2") == (), | |
| ], | |
| )] | |
| fn func(log: &mut Vec<&'static str>) { | |
| log.push("body"); | |
| return; | |
| } |
|
This change should do it |
9a2c5e4 to
2552d58
Compare
Ok latest push has all the comments addressed, have at it |
mkovaxx
left a comment
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.
Good stuff! This is likely the last round before we can land it. :)
2552d58 to
dd4c087
Compare
|
done 😏 |
mkovaxx
left a comment
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.
Getting super close. Just a few doc and test things.
mkovaxx
left a comment
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.
Please also update the README in anodized-core.
2368a2c to
143326c
Compare
mkovaxx
left a comment
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.
Last one!
8baee45 to
7093e7b
Compare
7093e7b to
f5fc4f0
Compare
|
Ok I think we sorted it out |
f5fc4f0 to
5cc4fae
Compare
mkovaxx
left a comment
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.
@brandonpollack23 Looks good, thanks!
Please add some detail to the PR description, check that cargo test passes locally, then squash and merge.
Merge activity
|

In the same vein as #57 , preconditions also must be wrapped in a closure to correctly handle any included return statements and prevent them from returning from the instrumented function.
This PR also modifies the tests for execution order to include the return keyword, removes the hacky Parse implementations used in those tests, and instead constructs the appropriate *Condition structs directly.
Fixes #66