[fe-analyzer-shared] Glitch in `initializerExpression` parse

[eernstg]

Thanks to @modulovalue who brought up this behavior here. Consider the following program:

var x = throw 0..isEven; // OK.

class A {
  var x;
  A() : x = throw 0..isEven; // Syntax error.
}

void main() {}

This program causes the analyzer and the CFE to report a syntax error for the <initializerExpression> in the constructor, whereas the initializing expression in the top-level variable declaration is accepted.

The non-terminal <initializerExpression> has for a long time been somewhat vaguely defined because we have a rule that it cannot be a function literal. That rule was never defined precisely. (However, a step in that direction is being taken here.)

It is possible that the error above is caused by the use of <throwExpressionWithoutCascade> rather than <throwExpression>, but unless there is a specific reason to make that choice, I'd recommend that the throw expression is parsed using the "normal" non-terminal for that purpose, that is, <throwExpression>.

@jensjoha, perhaps you could say something about some of the questions related to this topic? For example:

• How is the parsing of an <initializerExpression> and/or an <initializerList> performed, and how is it enforced that an <initializerExpression> cannot be a function literal?
• Does the parser actually use <throwExpressionWithoutCascade> rather than <throwExpression> in an initializer list?
• If it is the former, would it create any particular difficulties to switch over and use <throwExpression>?

[modulovalue]

One observation that is relevant to this issue:

Consider:

class Foo {
  Foo() : a = () {}, b = 0;
}

While that program is rejected by analyzer, but not by DSP, (and it seems like analyzer exhibits intended behavior), the following is accepted by both, analyzer and DSP:

class Foo {
  Foo() : a = <T>() {}, b = 0;
}

Also, consider:

class Foo {
  Foo() : a = <T>() => 0;
}

Without <T>, analyzer and DSP both agree. With <T>, DSP rejects that program and analyzer accepts it.

[jensjoha]

@jensjoha, perhaps you could say something about some of the questions related to this topic? For example:

• How is the parsing of an <initializerExpression> and/or an <initializerList> performed, and how is it enforced that an <initializerExpression> cannot be a function literal?
• Does the parser actually use <throwExpressionWithoutCascade> rather than <throwExpression> in an initializer list?
• If it is the former, would it create any particular difficulties to switch over and use <throwExpression>?

When parsing initializers (parseInitializersOpt -> parseInitializers -> parseInitializer), if it finds identifier '=' it parses the whole thing as an expression (parseInitializerExpressionRest -> parseExpression) meaning that it goes parsePrecedenceExpression ( -> parseUnaryExpression) -> _parsePrecedenceExpressionLoop with ASSIGNMENT_PRECEDENCE where it calls parseThrowExpression(next, /* allowCascades = */ false).

Said more directly, perhaps, it (tried to) parse x = throw 0..isEven as an expression. As such we get the same behavior if we for instance do

void foo() {
  var x;
  print(x = throw 0..isEven);
}

(In both cases parenthesizing the throw is a workaround).

Whereas for the field initializer it calls parseExpression on part after the equal sign, i.e. tries to parse throw 0..isEven as an expression (which directly calls parseThrowExpression(token, /* allowCascades = */ true)).

As for disallowing function literals there's a variable mayParseFunctionExpressions which is set to false in parseInitializers. (although again parenthesizing the thing will work around that).

Wrt allowing it --- it seems a little weird that parseThrowExpression(next, /* allowCascades = */ false) is just called with false when there's actually a variable called allowCascades --- shouldn't we use that? It was seemingly introduced in https://dart-review.googlesource.com/c/sdk/+/60600

I'll try to make that change and run the bots to see what happens.