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

[patterns] Syntactic ambiguity #2714

Closed
eernstg opened this issue Dec 11, 2022 · 3 comments · Fixed by #2785
Closed

[patterns] Syntactic ambiguity #2714

eernstg opened this issue Dec 11, 2022 · 3 comments · Fixed by #2785
Labels
patterns Issues related to pattern matching.

Comments

@eernstg
Copy link
Member

eernstg commented Dec 11, 2022

The grammar rules about patterns include an ambiguity:

flowchart TD;
    A[primaryPattern] --> B[constantPattern];
    A --> C[variablePattern];
    B --> D[identifier];
    C --> D;
Loading

I would recommend that the grammar is changed such that neither <constantPattern> nor <variablePattern> can derive <identifier>, but <primaryPattern> gets a new case <identifierPattern>:

<primaryPattern> ::= <constantPattern>
                   | <variablePattern>
                   | <identifierPattern> // New.
                   | <parenthesizedPattern>
                   | <listPattern>
                   | <mapPattern>
                   | <recordPattern>
                   | <objectPattern>

<identifierPattern> ::= <identifier>

<variablePattern> ::= ( 'var' | 'final' | 'final'? type ) <identifier> // Remove `?`.

<constantPattern> ::= <booleanLiteral>
                    | <nullLiteral>
                    | '-'? <numericLiteral>
                    | <stringLiteral> // Remove `<identifier>` alternative.
                    | <symbolLiteral>
                    | <qualifiedName>
                    | <constObjectExpression>
                    | 'const' <typeArguments>? '[' <elements>? ']'
                    | 'const' <typeArguments>? '{' <elements>? '}'
                    | 'const' '(' <expression> ')'

An identifier pattern would then be disambiguated based on the context:

  • In a declaration context, an identifier pattern introduces a fresh variable whose type is inferred.
  • In an assignment context, an identifier pattern is a reference to a local variable.
  • In a matching context, an identifier pattern is a reference to a constant variable.
@eernstg eernstg added the patterns Issues related to pattern matching. label Dec 11, 2022
@eernstg eernstg mentioned this issue Dec 12, 2022
@munificent
Copy link
Member

We can do this if you really think it's worth it, but I chose the current approach because I believe it leads to a simpler specification overall. If we have both <variablePattern> (for annotated variables) and <identifierPattern> unannotated variables in irrefutable contexts, then in later sections on semantics we either need to duplicate prose or reorganize it so that the same semantics covers multiple grammar productions.

I felt it was simpler to have the grammar productions have a 1-1 correspondence with semantics and then just disambiguate the syntax early on. That way, all variable declarations in patterns are <variablePattern>, and all constants are <constantPattern>.

@eernstg
Copy link
Member Author

eernstg commented Dec 14, 2022

The point is that the current grammar is ambiguous, and the parser can't decide on which derivation to use in each case.

If we have both <variablePattern> (for annotated variables) and <identifierPattern> unannotated variables in irrefutable contexts, then in later sections on semantics we either need to duplicate prose or reorganize it

The patterns spec refers to 'variablePattern' two times, total. Everywhere else it says 'variable pattern'.

So you just need to say, once, that a variable pattern is a <variablePattern> or an <identifierPattern> that occurs in a declaration context.

Similarly, a constant pattern is a <constantPattern> or an <identifierPattern> that occurs in a matching context.

There is no separate pattern for bare identifiers in an assignment context, but we could have an assignable expression pattern which would be an <identifierPattern> that occurs in an assignment context.

@leafpetersen
Copy link
Member

I think I agree with @eernstg here that if we're going to go to the trouble to specify the grammar, we shouldn't specify a grammar that is unnecessarily ambiguous, even if it should be obvious to the implementation teams how to programmatically disambiguate it.

munificent added a commit that referenced this issue Jan 20, 2023
There are no user-visible syntax or semantic changes here. It only
changes how the grammar is organized.

Fix #2714.
munificent added a commit that referenced this issue Jan 25, 2023
* [patterns] Disambiguate identifiers in the pattern grammar.

There are no user-visible syntax or semantic changes here. It only
changes how the grammar is organized.

Fix #2714.

* Apply review feedback.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
patterns Issues related to pattern matching.
Projects
None yet
Development

Successfully merging a pull request may close this issue.

3 participants