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

fix: last check: multiple decls in one quantifier, minus operator RHS #198

Closed
wants to merge 2 commits into from

Conversation

tnelson
Copy link
Owner

@tnelson tnelson commented Feb 21, 2023

(1) Quantifiers can contain multiple declarations. E.g.,
some x: A, y: B | ...
However, since A and B are arbitrary expressions, B can technically use the variable x. Our last-checker module does not currently add (x A) to the environment used for checking the expression B, resulting in a confusing error. This PR rolls the expanded environment forward across all decl domains, from left to right.

(2) Our last-checker module wasn't ever checking the right-hand side of a minus expression. Now if that exists, it's subjected to last-checker scrutiny.

@tnelson tnelson requested a review from bennn February 21, 2023 17:38
(define one-more-decl (cons (assocify decl) sofar))
(checkExpression run-or-state domain one-more-decl checker-hash)
one-more-decl))
quantvars
decls)
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Use the foldl output to define new-quantvars (below) instead of throwing it away.

; CHECK: recur into domain(s)
(checkExpression run-or-state domain quantvars checker-hash)))
; CHECK: recur into domain(s), carrying decls from prior quantifiers
(define one-more-decl (cons (assocify decl) sofar))
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Is to okay to put the decl at the front?
This is reverse order from the syntax & the new-quantvars below

@tnelson
Copy link
Owner Author

tnelson commented Mar 16, 2024

This will be fixed in #247 as part of the AST testing improvements; closing this PR.

@tnelson tnelson closed this Mar 16, 2024
@tnelson tnelson deleted the fix_multi_quantifier_last_check branch March 16, 2024 16:55
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 this pull request may close these issues.

2 participants