-
Notifications
You must be signed in to change notification settings - Fork 44
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
Loop invariant based extraction of redundancy sets #698
Conversation
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.
Looks good! I left some comments for (small) fixes and possible simplifications.
...AtoBoogie/src/de/uni_freiburg/informatik/ultimate/pea2boogie/InvariantResultTransformer.java
Outdated
Show resolved
Hide resolved
...AtoBoogie/src/de/uni_freiburg/informatik/ultimate/pea2boogie/InvariantResultTransformer.java
Outdated
Show resolved
Hide resolved
...gie/src/de/uni_freiburg/informatik/ultimate/pea2boogie/results/ReqCheckRedundancyResult.java
Show resolved
Hide resolved
...gie/src/de/uni_freiburg/informatik/ultimate/pea2boogie/results/ReqCheckRedundancyResult.java
Outdated
Show resolved
Hide resolved
...AtoBoogie/src/de/uni_freiburg/informatik/ultimate/pea2boogie/InvariantResultTransformer.java
Outdated
Show resolved
Hide resolved
...AtoBoogie/src/de/uni_freiburg/informatik/ultimate/pea2boogie/InvariantResultTransformer.java
Outdated
Show resolved
Hide resolved
...Boogie/src/de/uni_freiburg/informatik/ultimate/pea2boogie/VerificationResultTransformer.java
Outdated
Show resolved
Hide resolved
...Boogie/src/de/uni_freiburg/informatik/ultimate/pea2boogie/VerificationResultTransformer.java
Outdated
Show resolved
Hide resolved
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 looks good, everything is properly capsuled inside PEAToBoogie
without any consequences on other plugins.
...AtoBoogie/src/de/uni_freiburg/informatik/ultimate/pea2boogie/InvariantResultTransformer.java
Outdated
Show resolved
Hide resolved
trunk/examples/programs/FloatingPoint/regression/c/noMathSAT/ctrans-float-union.c
Outdated
Show resolved
Hide resolved
8ec82ab
to
fd7a66f
Compare
… new AST format of invariants. Traversing the ast extracts the needed identifiers to form the set.
fd7a66f
to
1df49a1
Compare
I hope I have adequately replied to most review feedback. Is there something I forgot? Since I am new, how does a merge generally happen, is it okay if I do it or will it be done by someone else? |
The (informal) rule is that there should be two approvals, and of course no unresolved issues from any of the reviews. Then the person who opened the PR (i.e. you) merges. |
…rministic printing
This pull request is referring to #681
Since a significant portion of the codebase changed, and a lot of time has passed since I have last looked at the PR, I decided to create a new branch. It was easier to just create things anew than modify all the previous work. I will also close the very outdated branch to avoid keeping around too many of them, since it is not relevant anymore.
As a short summary what was done:
I would be happy for a (short) review. I think most of what I added should not conflict with anything else, in the hopes that this can be merged soon, so the ideas of my thesis can help people by analyzing requirements for redundancies :)
There's still two improvements that are missing, but I couldn't implement them spontaneously like this new version of the simple loop invariant based approximation. The other ideas will be handled through new PRs once the details on this first simple method are decided and once it is added, as it will take some time to look at all the new things in the meantime, and also at the new way to get invariants from arbitrary locations.
Things I am working on in the future:
Many thanks in advance!
(PS I should have used the closing keyword for the other PR here, sorry, it's late)