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

Should we relax the rule against expression statements beginning with { for pattern assignments? #2662

Closed
stereotype441 opened this issue Nov 28, 2022 · 4 comments
Labels
patterns Issues related to pattern matching.

Comments

@stereotype441
Copy link
Member

Currently we have a disambiguation rule that says an expression statement can't begin with {, to prevent ambiguities between expression statements and blocks. This hasn't been a problem because the only way for an expression statement to start with { was if it was a map literal followed by zero or more selectors, and in normal usage, that's always silly. For example:

{a: b}.length; // no effect
{a: b}[c] = d; // no effect (the map gets thrown away)
{a: b}[a].foo(); // equivalent to `b.foo()`

However, with the addition of pattern support, an expression statement starting with { could now be a pattern assignment where the pattern is a map pattern, e.g.:

var foo, bar;
Map<String, Object> x = makeJsonRpcCall(...);
{'foo': foo, 'bar': bar} = x;

(Of course this is still a bit contrived; this would be better written as a pattern variable declaration, e.g. var {'foo': foo, 'bar': bar} = x. But I think the point still stands that a pattern assignment statement starting with { is not so unreasonable.)

I believe we could allow this code by changing the disambiguation rule from "an expression statement can't begin with {" to "an expression statement can only begin with { if it's a patternAssignment". This won't introduce any crucial ambiguity, because a pattern assignment beginning with { can always be told apart from a block by looking to see if the token to the right of the matching } is =.

Should we?

@stereotype441 stereotype441 added the patterns Issues related to pattern matching. label Nov 28, 2022
@stereotype441
Copy link
Member Author

@eernstg
Copy link
Member

eernstg commented Nov 28, 2022

I think we could do that, and it's probably a source of confusion and irritation if we don't. Of course, we would probably rely on having a lexer that knows about matching up braces and other parenthesis-like tokens (in order to avoid an actual no-limits lookahead), so there might be a cost in terms of parsing with non-standard tools (Cider, Github, ...).

@munificent
Copy link
Member

Ah, this is a good catch. Yes, I think we should relax that restriction. There's no principled reason why map patterns shouldn't work in pattern assignments, and users will likely expect them to.

@lrhn
Copy link
Member

lrhn commented Nov 29, 2022

Maybe a little contrived, because I'd be more likely to use a declaration pattern than an assignment pattern, but I'm sure it will happen, and it seems easy enough to support.

It comes back to assuming that our parser can cheaply find the matching close-brace, which we do elsewhere too.
So LGTM.

Phrasing could be something like:

The expression of a statement expression cannot start with a { token which starts a set or map literal. It may start with a { only if that starts a map pattern of a pattern assignment expression, in which case the corresponding closing } must be immediately followed by a =.

(We don't allow any other tokens surrounding a map pattern assignment, right? E.g., no {k: v} m = e - this one I know we don't have. Also, a { cannot occur in any other role than as starting a block, map literal, set literal or map pattern. Or named parameters or named record fields, but that definitely can't happen at the start of a statement, it needs a ( before it.)

copybara-service bot pushed a commit to dart-lang/sdk that referenced this issue Nov 30, 2022
Bug: #50035, dart-lang/language#2662
Change-Id: Ifb141cf563848fc0035423a625e27585cce2ea57
Reviewed-on: https://dart-review.googlesource.com/c/sdk/+/272523
Commit-Queue: Paul Berry <[email protected]>
Reviewed-by: Jens Johansen <[email protected]>
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

No branches or pull requests

4 participants