Skip to content

Parsing in tools and spec disagree #26602

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

Closed
eernstg opened this issue Jun 2, 2016 · 7 comments
Closed

Parsing in tools and spec disagree #26602

eernstg opened this issue Jun 2, 2016 · 7 comments
Labels
area-specification (deprecated) Deprecated: use area-language and a language- label.

Comments

@eernstg
Copy link
Member

eernstg commented Jun 2, 2016

According to the grammar fragments in the language specification (v. 1.11), we can derive as follows (where A stands for a construct that we need not worry about because it may be omitted or it is one or more unchosen alternatives):

  fieldInitializer ::= A? identifier '=' conditionalExpression A*
  conditionalExpression ::= ifNullExpression A?
  ifNullExpression ::= logicalOrExpression A*
  logicalOrExpression ::= logicalAndExpression A*
  logicalAndExpression ::= equalityExpression A*
  equalityExpression ::= relationalExpression A? | A
  relationalExpression ::= bitwiseOrExpression A? | A
  bitwiseOrExpression ::= bitwiseXorExpression A* | A
  bitwiseXorExpression ::= bitwiseAndExpression A* | A
  bitwiseAndExpression ::= shiftExpression A* | A
  shiftExpression ::= additiveExpression A* | A
  additiveExpression ::= multiplicativeExpression A* | A
  multiplicativeExpression ::= unaryExpression A* | A
  unaryExpression ::= postFixExpression | A
  postFixExpression ::= .. | primary (A* | A)
  primary ::= functionExpression | A
  functionExpression ::= formalParameterList functionBody

This shows that a fieldInitializer can be x = () { y = 3; }.

Yet, the following program is rejected during parsing:

var y;

class A {
  var x;
  A() : x = () { y = 3; };
}

main() => new A();

This applies to dart, dart2js, and dartanalyzer, all in version 1.17.0-dev.6.0.

@eernstg eernstg added the area-language Dart language related items (some items might be better tracked at github.com/dart-lang/language). label Jun 2, 2016
@floitschG
Copy link
Contributor

Since all tools agree, this should maybe be addressed by the spec, disallowing it.

@bwilkerson
Copy link
Member

I couldn't find any text in the spec explaining why, but analyzer has an explicit test in order to disallow exactly this case, with the following un-helpful comment:

    // Function expressions aren't allowed in initializer lists.

@lrhn
Copy link
Member

lrhn commented Jun 2, 2016

I believe the problem is that the grammar is harder to parse if you allow unparenthesized function literals.

   C(y) : x = (y) { print(y); };

You can't see whether x is assigned a the value of variable or a function literal until you see the final ;.
I don't think the grammar is actually ambiguous. A constructor needs a final body or a semicolon, and the initializer list continues on a comma, so looking after the potential function body always tells you whether it is a function body or a constructor body. You just can't do it with one-token look-ahead.

We should either specify the restriction: initializer list expressions must not contain something that looks like a constructor body - however we say that, fx anything is allowed if it's parenthesized because then we know it's not the constructor body, so it's not just "no functions expressions in initializers".

Alternatively we just accept the current grammar and make the parsers match it. I think the VM parser may be the biggest challenge because they try hard to be look-ahead-1. Dart2JS should not have a problem since it matches braces while tokenizing so it can quickly check the token after the closing brace before determining the next step.
If we switch to a single front-end, then we only need to fix it in one place.

@bwilkerson
Copy link
Member

@lrhn Thanks for reminding me why this was disallowed.

Analyzer also does brace matching during tokenization, and could use the same technique. However, allowing this would also have implications for recovery in the face of certain errors (such as a missing semicolon). I don't believe that the problem is insurmountable, but we should take it into account when deciding what to do.

@munificent munificent added area-specification (deprecated) Deprecated: use area-language and a language- label. and removed area-language Dart language related items (some items might be better tracked at github.com/dart-lang/language). labels Dec 13, 2016
@munificent
Copy link
Member

Speculatively moving this to area-specification on the hunch that what we already implemented is what we want and that we should tweak the spec to follow it. If we end up deciding otherwise, we can retarget this.

@eernstg
Copy link
Member Author

eernstg commented Feb 6, 2018

No milestone now: This correction is part of a larger update of grammar related parts of the specification, based on Dart.g.

@eernstg
Copy link
Member Author

eernstg commented Mar 3, 2020

Closing, the language specification is being updated to include the restriction in this PR: dart-lang/language#866.

@eernstg eernstg closed this as completed Mar 3, 2020
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
area-specification (deprecated) Deprecated: use area-language and a language- label.
Projects
None yet
Development

No branches or pull requests

5 participants