-
Notifications
You must be signed in to change notification settings - Fork 263
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
Solution to issue 2265 related to erroneous substitution under bindin… #2745
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.
Great fix! A few suggestions:
- There are still a few whitespace-only changes in unrelated parts of the code that would be nice to undo
- I think this PR is missing a fix for the
if-case
pattern that @RustanLeino sent an example of in the original issue discussion (if x :| P(x) => …
) - The split-quantifiers case isn't quite right, I think — but the original code was broken too (!)
- I've left other small comments in the code
With respect to the additional issue raised by Rustan involving a case statement, I decided to create a new issue because even if the fix might be small and simple, I would first like to make sure I understand the semantics, and I will add a new corresponding test. |
001219d
to
8c84c60
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.
Thanks! Can you revert the whitespace-only changes? Then we can merge! :)
…g guards
Fixes #2265
By submitting this pull request, I confirm that my contribution is made under the terms of the MIT license.