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
Infer that the right-hand side of claims is defined #2136
Infer that the right-hand side of claims is defined #2136
Conversation
kore/src/Kore/Step/Rule/Simplify.hs
Outdated
lhs' = simplified { Pattern.substitution = mempty } | ||
rhs = ClaimPattern.right claimPattern | ||
simplifiedLhs <- simplify lhs | ||
simplifiedRhs <- OrPattern.filterOr <$> traverse simplify rhs |
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.
@ana-pantilie Aren't we already simplify the right-hand side of the claim somewhere else?
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.
Yes, in the simplify
transition of the strategy: https://github.com/kframework/kore/blob/c28200738e0a5e8c2b9aaf6a6046b996f68082e3/kore/src/Kore/Strategies/Goal.hs#L838
kore/src/Kore/Step/Rule/Simplify.hs
Outdated
simplifiedRhs <- OrPattern.filterOr <$> traverse simplify rhs | ||
let substitution = | ||
Pattern.substitution simplifiedLhs | ||
<> foldMap Pattern.substitution (MultiOr.getMultiOr simplifiedRhs) |
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.
We cannot add the substitution from the right-hand side to the global substitution. We can't apply it to the left-hand side, and it isn't right to drop it from the right-hand side (below) either.
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.
Initially I didn't do that, but I tried it out of other ideas
\ceil{SortGeneratedTopCell{}, SortGeneratedTopCell{}}( | ||
/* Fn Spa */ | ||
Lbl'-LT-'generatedTop'-GT-'{}( |
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.
I know this hasn't been simplified because Lbl'-LT-'generatedTop'-GT-'{}
is a functional constructor.
kore/src/Kore/Step/Rule/Simplify.hs
Outdated
let simplifiedRhs = OrPattern.filterOr (OrPattern.map Pattern.requireDefined rhs) | ||
let substitution = Pattern.substitution simplifiedLhs | ||
lhs' = simplifiedLhs { Pattern.substitution = mempty } | ||
claimPattern | ||
{ ClaimPattern.left = lhs' | ||
, ClaimPattern.right = simplifiedRhs |
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 code will only be called once, at start-up. Please move it to Kore.Reachability.Claim.simplify'
, where we infer that the left-hand side of the claim is defined, and where we simplify both sides of the claim. The \ceil
patterns must be simplified under the same conditions as the rest of the right-hand side.
Fixes #2110
Reviewer checklist
stack test --coverage
stack haddock