[CP] [parser] Ensure that list and map pattern parsing always makes progress.

[stereotype441]

Commit(s) to merge

8d2b2a1

Target

Stable

Prepared changelist for beta/stable

https://dart-review.googlesource.com/c/sdk/+/303081

Issue Description

In some circumstances involving error recovery, the parser can fail to make progress, resulting in an infinite loop, which causes the analysis server to become unresponsive and consume progressively more memory. For example, the following code triggers the infinite loop:

void f(Object? x) {
  switch (x) {
    case [if]:
  };
}

Note that due to code completion this situation is not as rare as it might seem at first. If the user types case [ and then requests code completions, one of the possible completions is if. Accepting this completion, even for only a split second, triggers the error condition.

What is the fix

The parser is modified so that when handling a list or map pattern, if an attempt to parse a sub-pattern fails to consume any tokens, the next token is dropped. This ensures that progress is made, and the parser can finish parsing the input file.

Why cherry-pick

Without the cherry-pick, the analysis server may become unresponsive and rapidly consume memory while the user is typing.

The fix only takes effect when a list or map pattern is being parsed, and the parser fails to make progress when trying to parse a sub-pattern; this in turn can only happen when there is a parse error. Since well-formed patterns always contain at least noe token, parsing of correct code is unaffected, so the fix should be low-risk.

Risk

low

Issue link(s)

#52352

Extra Info

No response

[stereotype441]

Note: there are currently some failures on the analyzer-win-release bot that may or may not bisect to this CL (see https://ci.chromium.org/ui/p/dart/builders/ci.sandbox/analyzer-win-release/23007/overview). I'm currently coordinating with the analyzer team to investigate. I won't merge this change until I have more definitive information about the source of the failures.

[stereotype441]

Just got confirmation from @bwilkerson that the test failures on analyzer-win-release are unrelated to the CL I want to cherry-pick, so this cherry-pick is ready for review now.

[srawlins]

I approve 👍

[sigmundch]

I'm in favor of including this too