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

match: nonlinear pat refs in list-no-order #4304

Open
wants to merge 13 commits into
base: master
Choose a base branch
from

Conversation

AlexKnauth
Copy link
Member

@AlexKnauth AlexKnauth commented Jun 16, 2022

Allows patterns inside list-no-order to refer to nonlinear patterns declared prior, such as in (cons a (list-no-order a rst ...)).

Includes nonlinear patterns declared under ellipses, such as in (list (cons a (list-no-order a rst ...)) ...).

This also makes nonlinear pattern declarations syntax errors if the first use is in the scope of a ... pattern that doesn't encompass all uses.

(if (identifier? id)
id
(begin
(log-error "non-linear pattern used in `match` with ... at ~a and ~a"
Copy link
Sponsor Member

Choose a reason for hiding this comment

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

If this is supposed to work now this log should be removed.

Copy link
Member Author

Choose a reason for hiding this comment

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

I didn't want things like this to change for now:

(match '(1 2 3 4)
  [(list a ... a) a])

Order matters. When the a without ellipses comes first, that's when it works as a nonlinear pattern:

(match '(1 1 1 1)
  [(list a a ...) a])

Copy link
Sponsor Member

Choose a reason for hiding this comment

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

We should try to make the set of things that works clearer and simpler, not adding more weird exceptions.

Copy link
Member Author

Choose a reason for hiding this comment

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

I've updated the PR, it now makes nonlinear pattern declarations syntax errors if the first use is in the scope of a ... pattern that doesn't encompass all uses.

@samth
Copy link
Sponsor Member

samth commented Jun 16, 2022

  1. I think this needs a lot more testing if it's going to work.
  2. In particular, the problems that led to the warnings should be definitely checked.

@AlexKnauth
Copy link
Member Author

I have an idea for checking the problems that lead to warnings:

Track ellipsis depth, and when merging pattern-variables together into non-linear patterns, only accept them when the ellipsis-depth of the first one, the declaration, is less than or equal to the ellipsis-depth of the later ones, the references.

So (list a a ...) would be allowed because the declaration ellipsis-depth 0 is less than the reference ellipsis-depth 1, but (list a ... a) would be an error because the declaration ellipsis-depth 1 is greater than the reference ellipsis-depth 0.

nonlinear pattern declarations on the left, references on the right
doesn't really catch it yet because of the renaming affects "seen" and not just final binding
for detecting nonlinear patterns, use the original vars in the seen list, not their renamed versions
fix find-seen to check that nonlinear pattern declarations have zero ellipsis-depth with respect to their references
@AlexKnauth AlexKnauth marked this pull request as ready for review June 29, 2022 21:54
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.

None yet

2 participants