Skip to content
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

MinimizeTermConstruction's behavior intersecting GuardOrPattern's functionality #4113

Closed
gtrepta opened this issue Mar 19, 2024 · 1 comment · Fixed by #4124
Closed

MinimizeTermConstruction's behavior intersecting GuardOrPattern's functionality #4113

gtrepta opened this issue Mar 19, 2024 · 1 comment · Fixed by #4124
Assignees

Comments

@gtrepta
Copy link
Contributor

gtrepta commented Mar 19, 2024

Following up on discussions in #3973

GuardOrPatterns, introduced in #340, wraps #Or terms on the RHS of rules in #as patterns to remove ambiguity. A regression test, or-llvm was updated to exercise the case that this pass is meant to cover.

Today, if the pass is removed from the compiler pipeline, those tests still pass, and the #Or terms in the RHS of rules in the generated kore are still wrapped up in #as patterns. It happens here, in MinimizeTermConstruction, sometime after the KoreBackend.steps transformation:

if (isLHS() && !isRHS() && !inBad) {
if (usedOnRhs.contains(k)) {
return KAs(
super.apply(k),
cache.get(k),
Att.empty()
.add(Att.SORT(), Sort.class, cache.get(k).att().get(Att.SORT(), Sort.class)));
}

I believe this isn't intended. It looks as if #Or patterns are explicitly meant to not be operated on:

if (k.klabel().head().equals(KLabels.ML_OR)) {
inBad = true;
}

So somehow the bookkeeping done by this Transformer as it traverses the terms gets messed up and the value of inBad is wrong when visiting the #Or term.

@gtrepta gtrepta changed the title GuardOrPatterns functionality intersecting MinimizeTermConstruction MinimizeTermConstruction's behavior intersecting GuardOrPattern's functionality Mar 19, 2024
@gtrepta
Copy link
Contributor Author

gtrepta commented Mar 21, 2024

After discussing with Dwight, I've learned that MinimizeTermConstruction is meant as an optimization to reduce the number of constructors in a rewrite, and is likely doing what it's supposed to. GuardOrPatterns is to ensure correctness. It's possible that there's a case that GuardOrPatterns handles that MinimizeTermConstruction doesn't handle, but it would probably be pretty difficult to find.

I've created a PR to add a little documentation to MinimizeTermConstruction, and we can close this.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging a pull request may close this issue.

2 participants