dart-lang / sdk

The Dart SDK, including the VM, JS and Wasm compilers, analysis, core libraries, and more.
https://dart.dev
BSD 3-Clause "New" or "Revised" License
10.09k stars 1.56k forks source link

[patterns] Const record inconsistency between spec and analyzer. #54229

Open modulovalue opened 9 months ago

modulovalue commented 9 months ago

Consider the following program:

void main() {
  if ((0,) case const (0,)) print("foo");
}

It compiles without errors, prints out foo and terminates successfully on DartPad.

However, a parser compiled from the ANTLR-grammar seems to reject this program.

These are the parse trees that the analyzer and ANTLR report:

 === Analyzer ===
Scan errors: 0
Parse errors: 0
<CompilationUnitImpl> [0-58]
┗━ <FunctionDeclarationImpl> [0-57]
  ┣━ <NamedTypeImpl> [0-4]
  ┃  ┗━ 'void' [0-4]
  ┣━ 'main' [5-9]
  ┗━ <FunctionExpressionImpl> [9-57]
    ┣━ <FormalParameterListImpl> [9-11]
    ┃  ┣━ '(' [9-10]
    ┃  ┗━ ')' [10-11]
    ┗━ <BlockFunctionBodyImpl> [12-57]
      ┗━ <BlockImpl> [12-57]
        ┣━ '{' [12-13]
        ┣━ <IfStatementImpl> [16-55]
        ┃  ┣━ 'if' [16-18]
        ┃  ┣━ '(' [19-20]
        ┃  ┣━ <RecordLiteralImpl> [20-24]
        ┃  ┃  ┣━ '(' [20-21]
        ┃  ┃  ┣━ <IntegerLiteralImpl> [21-22]
        ┃  ┃  ┃  ┗━ '0' [21-22]
        ┃  ┃  ┗━ ')' [23-24]
        ┃  ┣━ <CaseClauseImpl> [25-40]
        ┃  ┃  ┣━ 'case' [25-29]
        ┃  ┃  ┗━ <GuardedPatternImpl> [30-40]
        ┃  ┃    ┗━ <ConstantPatternImpl> [30-40]
        ┃  ┃      ┣━ 'const' [30-35]
        ┃  ┃      ┗━ <RecordLiteralImpl> [36-40]
        ┃  ┃        ┣━ '(' [36-37]
        ┃  ┃        ┣━ <IntegerLiteralImpl> [37-38]
        ┃  ┃        ┃  ┗━ '0' [37-38]
        ┃  ┃        ┗━ ')' [39-40]
        ┃  ┣━ ')' [40-41]
        ┃  ┗━ <ExpressionStatementImpl> [42-55]
        ┃    ┣━ <MethodInvocationImpl> [42-54]
        ┃    ┃  ┣━ <SimpleIdentifierImpl> [42-47]
        ┃    ┃  ┃  ┗━ 'print' [42-47]
        ┃    ┃  ┗━ <ArgumentListImpl> [47-54]
        ┃    ┃    ┣━ '(' [47-48]
        ┃    ┃    ┣━ <SimpleStringLiteralImpl> [48-53]
        ┃    ┃    ┃  ┗━ '"foo"' [48-53]
        ┃    ┃    ┗━ ')' [53-54]
        ┃    ┗━ ';' [54-55]
        ┗━ '}' [56-57]
--------------------------------------------------------------------------------
 === ANTLR ===
line 2:16 missing ')' at 'const'
Errors of type 1: [[@15,38:38=',',<3>,2:24], [@17,40:40=')',<7>,2:26]]
Errors of type 2: [<missing ')'>]
line 2:16 missing ')' at 'const'
<startSymbol>
┗━ <libraryDefinition>
  ┣━ <metadata>
  ┣━ <topLevelDefinition>
  ┃  ┣━ <functionSignature>
  ┃  ┃  ┣━ <type>
  ┃  ┃  ┃  ┗━ <typeNotFunction>
  ┃  ┃  ┃    ┗━ 'void'
  ┃  ┃  ┣━ <identifier>
  ┃  ┃  ┃  ┗━ 'main'
  ┃  ┃  ┗━ <formalParameterPart>
  ┃  ┃    ┗━ <formalParameterList>
  ┃  ┃      ┣━ '('
  ┃  ┃      ┗━ ')'
  ┃  ┗━ <functionBody>
  ┃    ┗━ <block>
  ┃      ┣━ '{'
  ┃      ┣━ <statements>
  ┃      ┃  ┣━ <statement>
  ┃      ┃  ┃  ┗━ <nonLabelledStatement>
  ┃      ┃  ┃    ┗━ <ifStatement>
  ┃      ┃  ┃      ┣━ <ifCondition>
  ┃      ┃  ┃      ┃  ┣━ 'if'
  ┃      ┃  ┃      ┃  ┣━ '('
  ┃      ┃  ┃      ┃  ┣━ <expression>
  ┃      ┃  ┃      ┃  ┃  ┗━ <conditionalExpression>
  ┃      ┃  ┃      ┃  ┃    ┗━ <ifNullExpression>
  ┃      ┃  ┃      ┃  ┃      ┗━ <logicalOrExpression>
  ┃      ┃  ┃      ┃  ┃        ┗━ <logicalAndExpression>
  ┃      ┃  ┃      ┃  ┃          ┗━ <equalityExpression>
  ┃      ┃  ┃      ┃  ┃            ┗━ <relationalExpression>
  ┃      ┃  ┃      ┃  ┃              ┗━ <bitwiseOrExpression>
  ┃      ┃  ┃      ┃  ┃                ┗━ <bitwiseXorExpression>
  ┃      ┃  ┃      ┃  ┃                  ┗━ <bitwiseAndExpression>
  ┃      ┃  ┃      ┃  ┃                    ┗━ <shiftExpression>
  ┃      ┃  ┃      ┃  ┃                      ┗━ <additiveExpression>
  ┃      ┃  ┃      ┃  ┃                        ┗━ <multiplicativeExpression>
  ┃      ┃  ┃      ┃  ┃                          ┗━ <unaryExpression>
  ┃      ┃  ┃      ┃  ┃                            ┗━ <postfixExpression>
  ┃      ┃  ┃      ┃  ┃                              ┗━ <primary>
  ┃      ┃  ┃      ┃  ┃                                ┗━ <literal>
  ┃      ┃  ┃      ┃  ┃                                  ┗━ <recordLiteral>
  ┃      ┃  ┃      ┃  ┃                                    ┗━ <recordLiteralNoConst>
  ┃      ┃  ┃      ┃  ┃                                      ┣━ '('
  ┃      ┃  ┃      ┃  ┃                                      ┣━ <expression>
  ┃      ┃  ┃      ┃  ┃                                      ┃  ┗━ <conditionalExpression>
  ┃      ┃  ┃      ┃  ┃                                      ┃    ┗━ <ifNullExpression>
  ┃      ┃  ┃      ┃  ┃                                      ┃      ┗━ <logicalOrExpression>
  ┃      ┃  ┃      ┃  ┃                                      ┃        ┗━ <logicalAndExpression>
  ┃      ┃  ┃      ┃  ┃                                      ┃          ┗━ <equalityExpression>
  ┃      ┃  ┃      ┃  ┃                                      ┃            ┗━ <relationalExpression>
  ┃      ┃  ┃      ┃  ┃                                      ┃              ┗━ <bitwiseOrExpression>
  ┃      ┃  ┃      ┃  ┃                                      ┃                ┗━ <bitwiseXorExpression>
  ┃      ┃  ┃      ┃  ┃                                      ┃                  ┗━ <bitwiseAndExpression>
  ┃      ┃  ┃      ┃  ┃                                      ┃                    ┗━ <shiftExpression>
  ┃      ┃  ┃      ┃  ┃                                      ┃                      ┗━ <additiveExpression>
  ┃      ┃  ┃      ┃  ┃                                      ┃                        ┗━ <multiplicativeExpression>
  ┃      ┃  ┃      ┃  ┃                                      ┃                          ┗━ <unaryExpression>
  ┃      ┃  ┃      ┃  ┃                                      ┃                            ┗━ <postfixExpression>
  ┃      ┃  ┃      ┃  ┃                                      ┃                              ┗━ <primary>
  ┃      ┃  ┃      ┃  ┃                                      ┃                                ┗━ <literal>
  ┃      ┃  ┃      ┃  ┃                                      ┃                                  ┗━ <numericLiteral>
  ┃      ┃  ┃      ┃  ┃                                      ┃                                    ┗━ '0'
  ┃      ┃  ┃      ┃  ┃                                      ┣━ ','
  ┃      ┃  ┃      ┃  ┃                                      ┗━ ')'
  ┃      ┃  ┃      ┃  ┣━ 'case'
  ┃      ┃  ┃      ┃  ┣━ <guardedPattern>
  ┃      ┃  ┃      ┃  ┃  ┗━ <pattern>
  ┃      ┃  ┃      ┃  ┃    ┗━ <logicalOrPattern>
  ┃      ┃  ┃      ┃  ┃      ┗━ <logicalAndPattern>
  ┃      ┃  ┃      ┃  ┃        ┗━ <relationalPattern>
  ┃      ┃  ┃      ┃  ┃          ┗━ <unaryPattern>
  ┃      ┃  ┃      ┃  ┗━ '<missing ')'>'
  ┃      ┃  ┃      ┗━ <statement>
  ┃      ┃  ┃        ┗━ <nonLabelledStatement>
  ┃      ┃  ┃          ┗━ <expressionStatement>
  ┃      ┃  ┃            ┣━ <expression>
  ┃      ┃  ┃            ┃  ┗━ <conditionalExpression>
  ┃      ┃  ┃            ┃    ┗━ <ifNullExpression>
  ┃      ┃  ┃            ┃      ┗━ <logicalOrExpression>
  ┃      ┃  ┃            ┃        ┗━ <logicalAndExpression>
  ┃      ┃  ┃            ┃          ┗━ <equalityExpression>
  ┃      ┃  ┃            ┃            ┗━ <relationalExpression>
  ┃      ┃  ┃            ┃              ┗━ <bitwiseOrExpression>
  ┃      ┃  ┃            ┃                ┗━ <bitwiseXorExpression>
  ┃      ┃  ┃            ┃                  ┗━ <bitwiseAndExpression>
  ┃      ┃  ┃            ┃                    ┗━ <shiftExpression>
  ┃      ┃  ┃            ┃                      ┗━ <additiveExpression>
  ┃      ┃  ┃            ┃                        ┗━ <multiplicativeExpression>
  ┃      ┃  ┃            ┃                          ┗━ <unaryExpression>
  ┃      ┃  ┃            ┃                            ┗━ <postfixExpression>
  ┃      ┃  ┃            ┃                              ┗━ <primary>
  ┃      ┃  ┃            ┃                                ┗━ <literal>
  ┃      ┃  ┃            ┃                                  ┗━ <recordLiteral>
  ┃      ┃  ┃            ┃                                    ┣━ 'const'
  ┃      ┃  ┃            ┃                                    ┗━ <recordLiteralNoConst>
  ┃      ┃  ┃            ┃                                      ┣━ '('
  ┃      ┃  ┃            ┃                                      ┣━ <expression>
  ┃      ┃  ┃            ┃                                      ┃  ┗━ <conditionalExpression>
  ┃      ┃  ┃            ┃                                      ┃    ┗━ <ifNullExpression>
  ┃      ┃  ┃            ┃                                      ┃      ┗━ <logicalOrExpression>
  ┃      ┃  ┃            ┃                                      ┃        ┗━ <logicalAndExpression>
  ┃      ┃  ┃            ┃                                      ┃          ┗━ <equalityExpression>
  ┃      ┃  ┃            ┃                                      ┃            ┗━ <relationalExpression>
  ┃      ┃  ┃            ┃                                      ┃              ┗━ <bitwiseOrExpression>
  ┃      ┃  ┃            ┃                                      ┃                ┗━ <bitwiseXorExpression>
  ┃      ┃  ┃            ┃                                      ┃                  ┗━ <bitwiseAndExpression>
  ┃      ┃  ┃            ┃                                      ┃                    ┗━ <shiftExpression>
  ┃      ┃  ┃            ┃                                      ┃                      ┗━ <additiveExpression>
  ┃      ┃  ┃            ┃                                      ┃                        ┗━ <multiplicativeExpression>
  ┃      ┃  ┃            ┃                                      ┃                          ┗━ <unaryExpression>
  ┃      ┃  ┃            ┃                                      ┃                            ┗━ <postfixExpression>
  ┃      ┃  ┃            ┃                                      ┃                              ┗━ <primary>
  ┃      ┃  ┃            ┃                                      ┃                                ┗━ <literal>
  ┃      ┃  ┃            ┃                                      ┃                                  ┗━ <numericLiteral>
  ┃      ┃  ┃            ┃                                      ┃                                    ┗━ '0'
  ┃      ┃  ┃            ┃                                      ┣━ ','
  ┃      ┃  ┃            ┃                                      ┗━ ')'
  ┃      ┃  ┃            ┗━ ')'
  ┃      ┃  ┗━ <statement>
  ┃      ┃    ┗━ <nonLabelledStatement>
  ┃      ┃      ┗━ <expressionStatement>
  ┃      ┃        ┣━ <expression>
  ┃      ┃        ┃  ┗━ <conditionalExpression>
  ┃      ┃        ┃    ┗━ <ifNullExpression>
  ┃      ┃        ┃      ┗━ <logicalOrExpression>
  ┃      ┃        ┃        ┗━ <logicalAndExpression>
  ┃      ┃        ┃          ┗━ <equalityExpression>
  ┃      ┃        ┃            ┗━ <relationalExpression>
  ┃      ┃        ┃              ┗━ <bitwiseOrExpression>
  ┃      ┃        ┃                ┗━ <bitwiseXorExpression>
  ┃      ┃        ┃                  ┗━ <bitwiseAndExpression>
  ┃      ┃        ┃                    ┗━ <shiftExpression>
  ┃      ┃        ┃                      ┗━ <additiveExpression>
  ┃      ┃        ┃                        ┗━ <multiplicativeExpression>
  ┃      ┃        ┃                          ┗━ <unaryExpression>
  ┃      ┃        ┃                            ┗━ <postfixExpression>
  ┃      ┃        ┃                              ┣━ <primary>
  ┃      ┃        ┃                              ┃  ┗━ <identifier>
  ┃      ┃        ┃                              ┃    ┗━ 'print'
  ┃      ┃        ┃                              ┗━ <selector>
  ┃      ┃        ┃                                ┗━ <argumentPart>
  ┃      ┃        ┃                                  ┗━ <arguments>
  ┃      ┃        ┃                                    ┣━ '('
  ┃      ┃        ┃                                    ┣━ <argumentList>
  ┃      ┃        ┃                                    ┃  ┗━ <argument>
  ┃      ┃        ┃                                    ┃    ┗━ <expression>
  ┃      ┃        ┃                                    ┃      ┗━ <conditionalExpression>
  ┃      ┃        ┃                                    ┃        ┗━ <ifNullExpression>
  ┃      ┃        ┃                                    ┃          ┗━ <logicalOrExpression>
  ┃      ┃        ┃                                    ┃            ┗━ <logicalAndExpression>
  ┃      ┃        ┃                                    ┃              ┗━ <equalityExpression>
  ┃      ┃        ┃                                    ┃                ┗━ <relationalExpression>
  ┃      ┃        ┃                                    ┃                  ┗━ <bitwiseOrExpression>
  ┃      ┃        ┃                                    ┃                    ┗━ <bitwiseXorExpression>
  ┃      ┃        ┃                                    ┃                      ┗━ <bitwiseAndExpression>
  ┃      ┃        ┃                                    ┃                        ┗━ <shiftExpression>
  ┃      ┃        ┃                                    ┃                          ┗━ <additiveExpression>
  ┃      ┃        ┃                                    ┃                            ┗━ <multiplicativeExpression>
  ┃      ┃        ┃                                    ┃                              ┗━ <unaryExpression>
  ┃      ┃        ┃                                    ┃                                ┗━ <postfixExpression>
  ┃      ┃        ┃                                    ┃                                  ┗━ <primary>
  ┃      ┃        ┃                                    ┃                                    ┗━ <literal>
  ┃      ┃        ┃                                    ┃                                      ┗━ <stringLiteral>
  ┃      ┃        ┃                                    ┃                                        ┗━ <singleLineString>
  ┃      ┃        ┃                                    ┃                                          ┗━ '"foo"'
  ┃      ┃        ┃                                    ┗━ ')'
  ┃      ┃        ┗━ ';'
  ┃      ┗━ '}'
  ┗━ '<EOF>'

It appears that either the implementation is not meant to support records as constantPatterns, or that the spec is missing, e.g., recordPatterns with a leading const as a production in the constantPattern production rule.

lrhn commented 9 months ago

The spec-parser follows the specification, the analyzer is ... better. We should probably fix the spec. So, technically a bug, but let's consider whether we should fix the spec instead.

The grammar for constant patterns is:

constantPattern ::= booleanLiteral
                  | nullLiteral
                  | '-'? numericLiteral
                  | stringLiteral
                  | symbolLiteral
                  | qualifiedName
                  | constObjectExpression
                  | 'const' typeArguments? '[' elements? ']'
                  | 'const' typeArguments? '{' elements? '}'
                  | 'const' '(' expression ')'

That grammar does not allow const (0,), the parentheses around '(' expression ')' are syntactically part of the constant pattern syntax. We could add a | 'const' recordLiteralNoConst as well, since it's syntactically unambiguous, and it works.

Arguably, you can remove the const and have a record pattern instead, but if that contains a lot of consts, it might be shorter to write:

  case const (Token("watermelon"), Token("cataloupe")):

than

  case (const Token("watermelon"), const Token("cataloupe")):
eernstg commented 9 months ago

Here's a variant of the original example that is derivable in the specified grammar:

void main() {
  if ((0,) case const ((0,))) print("foo");
}

Both dart and dart analyze and DartPad accept this program. As @lrhn mentions,

We could add a | 'const' recordLiteralNoConst

in order to extend the language such that the original example can be derived. This seems to be the behavior which is already implemented, and it is also consistent with the existing constant patterns for collection literals and <constObjectExpression>: They are just special cases that allow us to avoid those extra () in the general const (<expression>) form.

Interestingly, the specification grammar in Dart.g still succeeds in parsing all language tests after changing | 'const' '(' <expression> ')' to | 'const' <expression>. So we might very well be able to eliminate those parentheses entirely. The cases involving collection literals would still be parsed by the preceeding alternatives (same as today), because of the ordered nature of the Dart grammar.

lrhn commented 9 months ago

The problem with omitting parentheses is that it makes const a && b ambiguous.

eernstg commented 9 months ago

Right, that would presumably parse as const (a && b) if b parses as an expression, and as (const a) && b if b only parses as a pattern, and that would probably not be very readable even if it does work (especially if b parses as an expression, but was intended as a pattern ;-).

eernstg commented 9 months ago

With that, I think adding | 'const' recordLiteralNoConst looks good! (.. and it is presumably just a spec change, because the tools do this already).