Malformed generic method comment freezes analyzer

[munificent]

Analyze this (incorrect) program with --strong:

void method(dynamic/*=T extends Uri*/ t) {}

Observe that the analyzer freezes until it eventually runs out of memory and crashes. Be sad.

I looked into it a little bit. When the generic method comment is inserted into the token stream, the original comment remains. So the first time the comment is handled, you end up with something like:

void method(dynamic T extends Uri /*=T extends Uri*/ t) {}

Then it fails to parse the extends.... At some point, it ends up retrying and hits the comment again:

void method(dynamic T extends Uri T extends Uri /*=T extends Uri*/ t) {}

Or something along those lines. I'm not sure why it keeps hitting the same code path again and again, but the result is an infinite expansion in the token stream.

cc @vsmenon @jmesserly @leafpetersen @bwilkerson

[bwilkerson]

Cool! (In a sarcastic sense.)

void method(dynamic T extends Uri /*=T extends Uri*/ t) {}

At the very least I would have expected the type being replaced to be removed from the token stream, producing

void method(T extends Uri /*=T extends Uri*/ t) {}

@jmesserly Is this working as expected?

Then it fails to parse the extends....

What do you mean by "fails"? Does it throw an exception?

My guess is that we're not correctly recording the failure, and as a result we attempt to re-parse, but of course the token stream has been modified by then. Removing the comments as they are processed would certainly take care of the recursion problem.

[jmesserly]

This happens in _injectTokenList in src/generated/parser.dart

sdk/pkg/analyzer/lib/src/generated/parser.dart

Line 3994 in c5f1818

void _injectTokenList(Token firstToken) {

We don't remove the tokens from comment token stream. I could swear there was a TODO there about that. Not sure what happened to it, but definitely remember being worried about the fact that we still have those comments in there. :)

Most of the time it's fine, because the injected tokens don't themselves have preceding comments. But if we failed to advance at all, not even getting to those tokens (maybe we just peek'd?) then we'd hit a problem. Probably an easy fix. As long as we consume the generic method comment.

Also, FWIW, these comments were not supposed to last long. :)
There's some pretty egregious issues in the current implementation: #25637

[jmesserly]

Also BTW: you don't need to write "dynamic" there. It's implicit. :)

[munificent]

What do you mean by "fails"? Does it throw an exception?

That's what I'm not sure about. I stepped through the parser for a while but eventually got lost and figured it was better for me to just file the bug and get back to what I was working on.

All I know is that it seems to repeatedly walk over that token stream, adding the parsed comment tokens in again and again each time.

[jmesserly]

https://codereview.chromium.org/1686173002/