-
Notifications
You must be signed in to change notification settings - Fork 274
Parser support for assigns contracts on loops #6196
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
ec43ea3
to
83a39db
Compare
Codecov Report
@@ Coverage Diff @@
## develop #6196 +/- ##
===========================================
+ Coverage 75.30% 75.47% +0.16%
===========================================
Files 1458 1458
Lines 161436 161437 +1
===========================================
+ Hits 121575 121838 +263
+ Misses 39861 39599 -262
Continue to review full report at Codecov.
|
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 rebase against current develop
and resolve the merge conflict -- it's due to the decreases contract that Long's PR introduced. That should be added to cprover_loop_contract
and should be removed from individual iteration statements.
990341f
to
c50fb3a
Compare
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.
LGTM.
36419cc
to
c07518e
Compare
Marking this as draft. There are several issues with commits, including merge conflicts. |
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 rebase as there seems to be at least one unrelated commit in this PR.
c07518e
to
c5e09d0
Compare
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.
LGTM, but one minor issue.
c5e09d0
to
26a3b3c
Compare
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.
The patch looks OK but I really don't think we should be adding syntax extensions without test cases that at least demonstrate how to use them.
e166702
to
64b5182
Compare
64b5182
to
df7696e
Compare
dbe9e64
to
5717865
Compare
This test shows that the current implementation only supports | ||
parsing and still uses alias analysis. Fixing this is a part |
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.
What do you mean by "current implementation only supports parsing and still uses alias analysis"? Could you elaborate?
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 PR only adds parsing to it, and assertion statements are yet not generated. Therefore, this proof does not go through.
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.
What Aalok is saying is that, he added this test just to check the parsing support (and address Martin's comment regarding usage of assigns clause). But it's not considered during instrumentation, so this test which he obtained from my bug report, still fails as before.
6d7db3d
to
1868741
Compare
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.
LGTM.
1. Renamed cprover_contract_assigns_opt to cprover_contract_assigns. 2. Created cprover_contract_assigns_opt which has a unit production and cprover_contract_assigns. 3. Renamed cprover_decreases_opt to cprover_contract_decreases_opt. 4. Renamed loop_invariant to cprover_contract_loop_invariant. 5. Renamed loop_invariant_list to cprover_contract_loop_invariant_list. 6. Renamed loop_invariant_list_opt to cprover_contract_loop_invariant_list_opt. 7. Added cprover_contract_assigns_opt to iteration statement. 8. Created the regression test invar_assigns_alias_analysis (KNOWNBUG). 9. Created the regression test invar_assigns_opt (CORE). 10. Created the regression test invar_assigns_empty (CORE). This adds parser support for assigns contracts on loops.
1868741
to
e190b7f
Compare
@martin-cs could you give another review on this PR? |
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 @feliperodri . There are now tests so no longer blocking.
In this commit we implement the parsing actions, type checking and conversion routines for assigns clauses on loops. The syntax rules along with regressions tests were introduced in an earlier PR: diffblue#6196.
In this commit we implement the parsing actions, type checking and conversion routines for assigns clauses on loops. The syntax rules along with regressions tests were introduced in an earlier PR: diffblue#6196.
In this commit we implement the parsing actions, type checking and conversion routines for assigns clauses on loops. The syntax rules along with regressions tests were introduced in an earlier PR: diffblue#6196.
In this commit we implement the parsing actions, type checking and conversion routines for assigns clauses on loops. The syntax rules along with regressions tests were introduced in an earlier PR: diffblue#6196.
This adds support for parsing assigns clauses for loops.