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.

Sign up for GitHub

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

Jump to bottom

[Patterns] Parser implementation #50035

Closed
stereotype441 opened this issue Sep 22, 2022 · 8 comments
Closed

[Patterns] Parser implementation #50035

stereotype441 opened this issue Sep 22, 2022 · 8 comments
Assignees
@stereotype441
Labels
area-analyzer Use area-analyzer for Dart analyzer issues, including the analysis server and code completion. area-front-end Use area-front-end for front end / CFE / kernel format related issues. P1 A high priority bug; for example, a single project is unusable or has many test failures type-enhancement A request for a change that isn't a bug
Milestone
Dart 3 beta 3

Comments

@stereotype441
Copy link
Member

No description provided.

@stereotype441 stereotype441 added area-analyzer Use area-analyzer for Dart analyzer issues, including the analysis server and code completion. area-front-end Use area-front-end for front end / CFE / kernel format related issues. labels Sep 22, 2022
@stereotype441 stereotype441 self-assigned this Sep 22, 2022
@srawlins srawlins added the P1 A high priority bug; for example, a single project is unusable or has many test failures label Sep 26, 2022
@itsjustkevin
Copy link
Contributor

Initial work merged at https://dart-review.googlesource.com/c/sdk/+/261020.

Thanks @stereotype441

copybara-service bot pushed a commit that referenced this issue Sep 28, 2022
@stereotype441
Parser: split some pattern/expression parsing logic.
9eb5544 
This change restores `parseParenthesizedExpressionOrRecordLiteral`,
`parseArguments`, and `parseArgumentsRest` to the way they looked
prior to adding parser support, and instead adds new methods
`parseParenthesizedPatternOrRecordPattern` and
`parseExtractorPatternRest` that are specialized for patterns parsing.

Bug: #50035
Change-Id: I104e031a796ef7f217cd427e1a9ae94a04dafa7b
Reviewed-on: https://dart-review.googlesource.com/c/sdk/+/261620
Commit-Queue: Paul Berry <[email protected]>
Reviewed-by: Jens Johansen <[email protected]>
copybara-service bot pushed a commit that referenced this issue Sep 29, 2022
@stereotype441
Remove bogus comment from parseLiteralSetOrMapSuffix
cba19c0 
This comment was relevant in a previous revision of the patterns
parsing logic and I neglected to revert it.

Bug: #50035
Change-Id: Ia4034d66c3cc354e2ea6f2e1485380bccf7333dd
Reviewed-on: https://dart-review.googlesource.com/c/sdk/+/261700
Reviewed-by: Konstantin Shcheglov <[email protected]>
Reviewed-by: Jens Johansen <[email protected]>
Commit-Queue: Paul Berry <[email protected]>
copybara-service bot pushed a commit that referenced this issue Oct 3, 2022
@stereotype441
Patterns: add support for parsing constant patterns
118f628 
Bug: #50035
Change-Id: I601248bc206d9b19521b020fadb2eccd048e0ec7
Reviewed-on: https://dart-review.googlesource.com/c/sdk/+/261701
Reviewed-by: Jens Johansen <[email protected]>
Commit-Queue: Paul Berry <[email protected]>
Reviewed-by: Konstantin Shcheglov <[email protected]>
copybara-service bot pushed a commit that referenced this issue Oct 14, 2022
@stereotype441
Parser: add support for wildcard patterns.
5923b88 
The bare identifier `_` is treated as a variable pattern wherever it
appears, even though other bare identifiers are sometimes treated as
constant patterns.

Bug: #50035
Change-Id: I475ef627b6d0bf519a1972aa5ec683e2d032f02b
Reviewed-on: https://dart-review.googlesource.com/c/sdk/+/263280
Reviewed-by: Jens Johansen <[email protected]>
Commit-Queue: Paul Berry <[email protected]>
copybara-service bot pushed a commit that referenced this issue Oct 14, 2022
@stereotype441
Parser and listener API changes for supporting pattern guards.
4cf63fb 
These are in their own CL so that it's easy to verify that no
functionalty has changed, even though a lot of parser expectations
files are affected.

In a follow-up CL, I'll add code that actually parses pattern guards.

Bug: #50035
Change-Id: I79f5a02df45e29b69aa4d8348cfdf27042ada9d3
Reviewed-on: https://dart-review.googlesource.com/c/sdk/+/264020
Reviewed-by: Jens Johansen <[email protected]>
Commit-Queue: Paul Berry <[email protected]>
copybara-service bot pushed a commit that referenced this issue Oct 14, 2022
@stereotype441
Patterns parsing: add support for guards.
9aab3c7 
Bug: #50035
Change-Id: I21db09c9d8c1d359e1b125eea2bae2749cdb72fd
Reviewed-on: https://dart-review.googlesource.com/c/sdk/+/264101
Reviewed-by: Jens Johansen <[email protected]>
Reviewed-by: Konstantin Shcheglov <[email protected]>
Commit-Queue: Paul Berry <[email protected]>
@srawlins
Copy link
Member

@stereotype441 is there more known parsing work, or is everything "known" done?

@stereotype441
Copy link
Member Author

@stereotype441 is there more known parsing work, or is everything "known" done?

No, there's still more parsing work left to do, including:

  • pattern variable declaration statement
  • pattern variable declaration inside for-loop parts
  • if-case inside collections
  • pattern assignments
  • detection of all relevant parse errors

as well as addressing all the TODO comments from the CLs above.

I took a break from patterns parsing to write up a specification for flow analysis of patterns (and then went on vacation), but I'm expecting to return to it soon. If the analyzer team becomes blocked on any of the above items before I can get to them, let me know and I'd be happy to hand over some parsing tasks to someone else.

copybara-service bot pushed a commit that referenced this issue Nov 14, 2022
@stereotype441
Patterns parsing: fix handling of builtin identifiers and pseudo-keyw…
1af3f76 
…ords.

Bug: #50035
Change-Id: I95f2b7d59a54d0d5e1975bd8cd1dce88db03fa51
Reviewed-on: https://dart-review.googlesource.com/c/sdk/+/269580
Reviewed-by: Konstantin Shcheglov <[email protected]>
Commit-Queue: Paul Berry <[email protected]>
Reviewed-by: Jens Johansen <[email protected]>
copybara-service bot pushed a commit that referenced this issue Nov 21, 2022
@stereotype441
Parser API changes in preparation for supporting patternVariableDecla…
a975bf0 
…ration.

Bug: #50035
Change-Id: I2661b7cbe60815b6232a165d56a478d8975aa6c2
Reviewed-on: https://dart-review.googlesource.com/c/sdk/+/270228
Reviewed-by: Jens Johansen <[email protected]>
Reviewed-by: Konstantin Shcheglov <[email protected]>
Commit-Queue: Paul Berry <[email protected]>
copybara-service bot pushed a commit that referenced this issue Nov 21, 2022
@stereotype441
Add parser support for patternVariableDeclaration.
ef4f5a3 
Bug: #50035
Change-Id: I13158547d9200f37eb4434d1792b4d4667a46071
Reviewed-on: https://dart-review.googlesource.com/c/sdk/+/270920
Reviewed-by: Jens Johansen <[email protected]>
Commit-Queue: Paul Berry <[email protected]>
copybara-service bot pushed a commit that referenced this issue Nov 22, 2022
@stereotype441
Rename extractorPattern to objectPattern in parser and listeners.
0037c6c 
This reflects a spec change that was done in
dart-lang/language@4c83abe.

Bug: #50035
Change-Id: I282ec073d723294b7c1cd84928d8404a50b0b195
Reviewed-on: https://dart-review.googlesource.com/c/sdk/+/271162
Reviewed-by: Konstantin Shcheglov <[email protected]>
Reviewed-by: Johnni Winther <[email protected]>
Commit-Queue: Paul Berry <[email protected]>
copybara-service bot pushed a commit that referenced this issue Nov 23, 2022
@stereotype441
Parser: Add assertions to check skipOuterPattern behaviour
4f37821 
The behaviour `skipOuterPattern` needs to be kept in sync with that of
`parsePrimaryPattern`.  To reduce the risk of these to going out of
sync, I've added logic to `parsePrimaryPattern` so that after parsing
an `outerPattern`, it asserts that `skipOuterPattern` would have
behaved appropriately.

This required adding some logic to `parser_test_parser.dart` so that
these calls to `skipOuterPattern` don't show up in front_end parser
expectations files.  (If they did show up, they would lead to
failures, since the bots run the front_end parser tests with
assertions both enabled and disabled).

Bug: #50035
Change-Id: Ida3a37532bb9a86837e70b5ec1aa2e557e3cb769
Reviewed-on: https://dart-review.googlesource.com/c/sdk/+/271163
Reviewed-by: Konstantin Shcheglov <[email protected]>
Commit-Queue: Paul Berry <[email protected]>
Reviewed-by: Johnni Winther <[email protected]>
copybara-service bot pushed a commit that referenced this issue Nov 23, 2022
@stereotype441
Add parser support for ... in list patterns.
ff18485 
Bug: #50035
Change-Id: I31c7003fd40a6e074f2561561d2fba49955cc098
Reviewed-on: https://dart-review.googlesource.com/c/sdk/+/271565
Reviewed-by: Johnni Winther <[email protected]>
Commit-Queue: Paul Berry <[email protected]>
copybara-service bot pushed a commit that referenced this issue Nov 23, 2022
@stereotype441
Fix parsing of single identifier patterns before when.
15c8f28 
There's already logic for addressing similar issues before `as`; I
just had to generalize it.

Bug: #50035
Change-Id: I09d9c69cd291fe8bc3ebea8181f632ddca0c49e8
Reviewed-on: https://dart-review.googlesource.com/c/sdk/+/271580
Reviewed-by: Konstantin Shcheglov <[email protected]>
Commit-Queue: Paul Berry <[email protected]>
copybara-service bot pushed a commit that referenced this issue Nov 23, 2022
@stereotype441
Add parser support for switch expressions.
fe087f2 
Bug: #50035
Change-Id: I0c58664d44312f5e9d61d24e34e7cd26dbee3a9c
Reviewed-on: https://dart-review.googlesource.com/c/sdk/+/271740
Reviewed-by: Konstantin Shcheglov <[email protected]>
Commit-Queue: Paul Berry <[email protected]>
copybara-service bot pushed a commit that referenced this issue Nov 28, 2022
@stereotype441
Add support for ... inside map patterns
a3befd3 
Bug: #50035
Change-Id: I350eb526e67e4cc42c94b4776e5d38315932ddc5
Reviewed-on: https://dart-review.googlesource.com/c/sdk/+/272382
Reviewed-by: Konstantin Shcheglov <[email protected]>
Commit-Queue: Paul Berry <[email protected]>
copybara-service bot pushed a commit that referenced this issue Nov 28, 2022
@stereotype441
Patterns parsing: Add logEvent call to Listener.handlePatternVariable…
e69a503 
…DeclarationStatement.

Normally, listener methods call `LogEvent`.  This helps find bugs
where a listener that extends `Listener` doesn't implement a necessary
callback.

Bug: #50035
Change-Id: Ideac739d8d63cda2244572f7d1deb83dceeced37
Reviewed-on: https://dart-review.googlesource.com/c/sdk/+/272346
Commit-Queue: Paul Berry <[email protected]>
Reviewed-by: Konstantin Shcheglov <[email protected]>
copybara-service bot pushed a commit that referenced this issue Nov 28, 2022
@stereotype441
Rename looksLikePatternVariableDeclaration.
2a9491b 
The new name, `looksLikeOuterPatternEquals`, is a more precise
description of what it does.

This helps prepare for adding parser support for pattern assignments,
which require the same functionality.

Bug: #50035
Change-Id: Ibfcc4c37faac1770c98a427facdffda5c40e6d08
Reviewed-on: https://dart-review.googlesource.com/c/sdk/+/272344
Reviewed-by: Konstantin Shcheglov <[email protected]>
Commit-Queue: Paul Berry <[email protected]>
copybara-service bot pushed a commit that referenced this issue Nov 29, 2022
@stereotype441
Add parser support for patternAssignment.
7b41b62 
Bug: #50035, #50502, #50575
Change-Id: Idd3bdcae7cbb95c6f2016bb5d3efc638867f52af
Reviewed-on: https://dart-review.googlesource.com/c/sdk/+/272383
Reviewed-by: Konstantin Shcheglov <[email protected]>
Commit-Queue: Paul Berry <[email protected]>
copybara-service bot pushed a commit that referenced this issue Nov 30, 2022
@stereotype441
Add support for expression statements beginning with a map pattern.
701a0d2 
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]>
@srawlins srawlins added this to the Dart 3 stable milestone Nov 30, 2022
@stereotype441 stereotype441 mentioned this issue Nov 30, 2022
Open
@srawlins srawlins modified the milestones: Dart 3 stable, Dart 3 beta 2 Nov 30, 2022
copybara-service bot pushed a commit that referenced this issue Dec 1, 2022
@stereotype441
Add parser support for for (var pattern = expression; ...; ...)
599911d 
Bug: #50035
Change-Id: I5f07b848ba06be403e56538a4cc64f6de51bd668
Reviewed-on: https://dart-review.googlesource.com/c/sdk/+/273005
Reviewed-by: Konstantin Shcheglov <[email protected]>
Commit-Queue: Paul Berry <[email protected]>
copybara-service bot pushed a commit that referenced this issue Dec 2, 2022
@stereotype441
Add parser support for patterns in for-in statements and collection e…
e9a1248 
…lements.

Bug: #50035
Change-Id: I9eda03c1501cf3164cfb09cf9084576a5d1141cc
Reviewed-on: https://dart-review.googlesource.com/c/sdk/+/273122
Reviewed-by: Konstantin Shcheglov <[email protected]>
Reviewed-by: Jens Johansen <[email protected]>
Commit-Queue: Paul Berry <[email protected]>
copybara-service bot pushed a commit that referenced this issue Dec 5, 2022
@stereotype441
CFE parser test cases for metadata in for-in loops with patterns.
7a3aa5c 
Bug: #50035
Change-Id: I336b391c9102e4e6dd3eabce1051763e3bc797d4
Reviewed-on: https://dart-review.googlesource.com/c/sdk/+/273621
Reviewed-by: Jens Johansen <[email protected]>
Commit-Queue: Paul Berry <[email protected]>
copybara-service bot pushed a commit that referenced this issue Dec 7, 2022
@stereotype441
Patterns parsing: track when variable patterns are in an assignment c…
38cab10 
…ontext.

Variable patterns behave so differently inside a patternAssignment
that we may want to represent them using different AST nodes inside
the analyzer/CFE.  This change adds a boolean flag allowing the
implementation to know what kind of variable pattern it's looking at
when parsing occurs.

Bug: #50035
Change-Id: I60adf2865bbe24f85b72a79b1360833bf823bd67
Reviewed-on: https://dart-review.googlesource.com/c/sdk/+/273829
Reviewed-by: Konstantin Shcheglov <[email protected]>
Commit-Queue: Paul Berry <[email protected]>
Reviewed-by: Jens Johansen <[email protected]>
@srawlins srawlins added the type-enhancement A request for a change that isn't a bug label Feb 7, 2023
@srawlins
Copy link
Member

srawlins commented Feb 8, 2023

Hi @stereotype441 @johnniwinther , the last CLs tagged with this issue landed in December. Is there more parser work ongoing?

@johnniwinther
Copy link
Member

Yes, I'm currently working on report errors for unsupported constant patterns.

@srawlins
Copy link
Member

srawlins commented Feb 8, 2023

Ah excellent, thanks!

copybara-service bot pushed a commit that referenced this issue Mar 30, 2023
@stereotype441
Patterns parsing: make it an error to use var before a type in a va…
134007a 
…riable pattern.

Bug: #50035
Change-Id: I46b23ef0d1856401eac4b0a05c6bc6008711d35a
Reviewed-on: https://dart-review.googlesource.com/c/sdk/+/291780
Commit-Queue: Paul Berry <[email protected]>
Reviewed-by: Jens Johansen <[email protected]>
copybara-service bot pushed a commit that referenced this issue Mar 31, 2023
@stereotype441
Fix parsing of PATTERN as T? when
452dcdf 
The logic for parsing types has special disambiguation rules for
deciding whether a trailing `?` should be included in the type, based
on what token(s) follow the `?`.  In the case where the token that
follows the `?` is `when`, we need to look further ahead to
disambiguate, to distinguish `PATTERN as T? when guard` from something
like `EXPRESSION is T ? when : otherwise`.

(Note: an alternative implementation would be to disambiguate based on
whether we're parsing a pattern or an expression.  But in the future I
want to move toward an architecture where expression parsing and
pattern parsing are combined, so that if the parser makes the wrong
decision about whether it's looking at a pattern or an expression,
error recovery will do a better job.  So I'm disambiguating based
solely on what follows the `?`.)

Bug: #50035
Change-Id: Idbc780b7b54fecc7fd01cae868c34771564dd804
Reviewed-on: https://dart-review.googlesource.com/c/sdk/+/292282
Commit-Queue: Paul Berry <[email protected]>
Reviewed-by: Jens Johansen <[email protected]>
copybara-service bot pushed a commit that referenced this issue Mar 31, 2023
@stereotype441
Patterns parsing: disallow unaryPattern or relationalPattern in unary…
d2184a9 
…Pattern.

The precedence-based pattern parser can understand a unary pattern
inside another unary pattern (e.g. `_ as int as num`) or a relational
pattern inside a unary pattern (e.g. `> 1?`), but the specification
prohibits these constructions because they're difficult to read and
not very useful.

This change updates the implementation to match the spec, by producing
the appropriate error.  The offset and length of the error cover the
inner pattern, so it should be easy to construct an analysis server
quick fix that inserts the necessary parentheses.

Bug: #50035
Change-Id: I33e74d6d1f863e7162851d26fefbacd4fd17277c
Reviewed-on: https://dart-review.googlesource.com/c/sdk/+/292120
Reviewed-by: Konstantin Shcheglov <[email protected]>
Commit-Queue: Paul Berry <[email protected]>
copybara-service bot pushed a commit that referenced this issue Apr 1, 2023
@stereotype441
Fix precedence of expression inside relational patterns.
406bdf3 
According to the spec, the expression inside a relational pattern is
`bitwiseOrExpression`.  We were only allowing `shiftExpression`.

Bug: #50035
Change-Id: Ie1a5746f1060b84e6e1b856a622e89db698b4684
Reviewed-on: https://dart-review.googlesource.com/c/sdk/+/292285
Commit-Queue: Paul Berry <[email protected]>
Reviewed-by: Konstantin Shcheglov <[email protected]>
copybara-service bot pushed a commit that referenced this issue Apr 1, 2023
@stereotype441
Parser: test that objectPattern accepts OTHER_IDENTIFIER.
7a51360 
The type name in an object pattern is a `typeIdentifier`; in the spec
grammar, `typeIdentifier` includes `OTHER_IDENTIFIER`, which is defined as:

    OTHER_IDENTIFIER ::=
        `async` | `hide` | `of` | `on` | `show` | `sync` | `await` | `yield`

This adds tests to verify that all these identifiers are accepted as
type names in an object pattern.

Bug: #50035
Change-Id: I6b9d752e1b34f7bc93dedc4304f695e7cb42df48
Reviewed-on: https://dart-review.googlesource.com/c/sdk/+/292542
Reviewed-by: Konstantin Shcheglov <[email protected]>
Commit-Queue: Paul Berry <[email protected]>
copybara-service bot pushed a commit that referenced this issue Apr 2, 2023
@stereotype441
Patterns parser: test that dynamic can be used in an object pattern.
22a8877 
This resolves a TODO in the patterns parser implementation.

(Note: the TODO says to file an issue, but there is no need; the spec
clearly states that `dynamic` is allowed).

Change-Id: I2ebbf5401052ce5b96679ac3cf8a1aba3cd384d4
Bug: #50035
Reviewed-on: https://dart-review.googlesource.com/c/sdk/+/292541
Commit-Queue: Paul Berry <[email protected]>
Reviewed-by: Konstantin Shcheglov <[email protected]>
copybara-service bot pushed a commit that referenced this issue Apr 2, 2023
@stereotype441
Patterns parser: remove bogus error recovery logic.
45429b3 
The "recovery" condition `|| /* recovery */ optional(':', next)` had
no effect, because that case was already covered by the previous `if`
statement.

Bug: #50035
Change-Id: I4eeeeb2ff6b3558888f07c65d85e9f79e334d9eb
Reviewed-on: https://dart-review.googlesource.com/c/sdk/+/292661
Reviewed-by: Konstantin Shcheglov <[email protected]>
copybara-service bot pushed a commit that referenced this issue Apr 2, 2023
@stereotype441
Patterns parsing: add error recovery tests.
489aefb 
Bug: #50035
Change-Id: If63ee17234f0b63ebbdfda8a046350def8851472
Reviewed-on: https://dart-review.googlesource.com/c/sdk/+/292662
Reviewed-by: Konstantin Shcheglov <[email protected]>
copybara-service bot pushed a commit that referenced this issue Apr 3, 2023
@stereotype441
Patterns parsing: improve error recovery for late in patternVariabl…
8d16c1d 
…eDeclaration.

Bug: #50035
Change-Id: Id706ff0afab3f14d817fbf99112f772d1f1f7817
Reviewed-on: https://dart-review.googlesource.com/c/sdk/+/292800
Commit-Queue: Paul Berry <[email protected]>
Reviewed-by: Johnni Winther <[email protected]>
copybara-service bot pushed a commit that referenced this issue Apr 3, 2023
@stereotype441
Patterns parsing: eliminate an unnecessary TODO.
f537137 
The grammar was updated in
dart-lang/language@f06d416
so that it now matches the implementation.

Bug: #50035
Change-Id: I2aa7d395360c96e39b77336c46f415f1b2280439
Reviewed-on: https://dart-review.googlesource.com/c/sdk/+/292801
Reviewed-by: Konstantin Shcheglov <[email protected]>
Commit-Queue: Paul Berry <[email protected]>
@vsmenon
Copy link
Member

vsmenon commented Apr 3, 2023

@stereotype441 - is there still open work here or can we close?

@stereotype441
Copy link
Member Author

@stereotype441 - is there still open work here or can we close?

I still have a couple of minor issues I'm trying to investigate before the beta cut, but they're issues with error recovery so they're not blocking. I'll close this issue when I'm done with them.

@johnniwinther johnniwinther removed the Epic label May 4, 2023
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Assignees

@stereotype441 stereotype441

Labels
area-analyzer Use area-analyzer for Dart analyzer issues, including the analysis server and code completion. area-front-end Use area-front-end for front end / CFE / kernel format related issues. P1 A high priority bug; for example, a single project is unusable or has many test failures type-enhancement A request for a change that isn't a bug
Projects
None yet
Milestone
Dart 3 beta 3
Development

No branches or pull requests
5 participants
@srawlins @stereotype441 @vsmenon @itsjustkevin @johnniwinther