-
Notifications
You must be signed in to change notification settings - Fork 44
Remove Defined markers in simplifyConjunctionByAssumption #2372
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
, testDefined "contradiction: f(g(x)) = a ∧ f(g(x)) = b" (Right fg) (Right a) (Right b) | ||
, testDefined "contradiction: f(g(x)) = a ∧ f(g(x)) = b" (Left fg) (Right a) (Right b) |
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 is the difference between Right fg
and Left fg
?
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.
Right fg
will apply mkDefined
to the f(g(x)
in the second equals, and Left fg
to the one in the first. This makes sure we test that the Defined
markers are removed when both assuming and looking up terms.
equals2 = | ||
mkLocalDefn (applyDefined2 eitherFunc) eitherC2 | ||
& Condition.fromPredicate | ||
condition = definedFg <> equals1 <> definedFg <> equals2 |
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.
Using definedFg
here actually requires that eitherFunc
is Right fg
or Left fg
, so I would rather we not allow passing in any value there.
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 mean we should remove definedFg
from condition
? Edit: I think this is what you meant by this comment, so I pushed a commit with this change. If you think something else should change, let me know.
Needs #2366Until #2351 is done, this will fix those cases where a conjunction isn't simplified correctly because the terms differ by
Defined
markers.Review checklist
The author performs the actions on the checklist. The reviewer evaluates the work and checks the boxes as they are completed.