Add support for metadata on type variables to Fasta parser.

Add support for metadata on type variables to Fasta parser.

R=ahe@google.com, sigmund@google.com

Committed: https://github.com/dart-lang/sdk/commit/5d8d01cdf737a662bf83ce72938ea3750da57acb

[Paul Berry, 2017-03-08 21:18:23]

paulberry@google.com changed reviewers:
+ ahe@google.com, sigmund@google.com

[Paul Berry, 2017-03-08 21:18:23]

This CL is not ready to land yet.  It causes the following tests to crash:

python tools/test.py -mrelease -cdart2js -rd8 -aia32 -t60 --dart2js-batch
co19/Language/Metadata/before_type_param_t01

python tools/test.py -mrelease -cdart2js -rd8 -aia32 -t60 --dart2js-batch
co19/Language/Generics/syntax_t17

python tools/test.py -mrelease -cdart2js -rd8 -aia32 -t60 --dart2js-batch
language/metadata_test

Siggi, Peter: I'm not familiar enough with dart2js internals to debug these
failures.  Would one of you mind taking a look?

[Siggi Cherem (dart-lang), 2017-03-08 23:39:34]

On 2017/03/08 21:18:23, Paul Berry wrote:
> This CL is not ready to land yet.  It causes the following tests to crash:
> 
> python tools/test.py -mrelease -cdart2js -rd8 -aia32 -t60 --dart2js-batch
> co19/Language/Metadata/before_type_param_t01
> 
> python tools/test.py -mrelease -cdart2js -rd8 -aia32 -t60 --dart2js-batch
> co19/Language/Generics/syntax_t17
> 
> python tools/test.py -mrelease -cdart2js -rd8 -aia32 -t60 --dart2js-batch
> language/metadata_test
> 
> Siggi, Peter: I'm not familiar enough with dart2js internals to debug these
> failures.  Would one of you mind taking a look?

I took a brief look - the code in question from dart2js is doing a custom
parsing to skip over type-parameters in a class declaration. Is doing so based
on the endGroup of the `<` token (the code uses directly the token stream
without using the actual parser, so this seems like a scanner bug).

[Paul Berry, 2017-03-09 01:24:54]

On 2017/03/08 23:39:34, Siggi Cherem (dart-lang) wrote:
> On 2017/03/08 21:18:23, Paul Berry wrote:
> > This CL is not ready to land yet.  It causes the following tests to crash:
> > 
> > python tools/test.py -mrelease -cdart2js -rd8 -aia32 -t60 --dart2js-batch
> > co19/Language/Metadata/before_type_param_t01
> > 
> > python tools/test.py -mrelease -cdart2js -rd8 -aia32 -t60 --dart2js-batch
> > co19/Language/Generics/syntax_t17
> > 
> > python tools/test.py -mrelease -cdart2js -rd8 -aia32 -t60 --dart2js-batch
> > language/metadata_test
> > 
> > Siggi, Peter: I'm not familiar enough with dart2js internals to debug these
> > failures.  Would one of you mind taking a look?
> 
> I took a brief look - the code in question from dart2js is doing a custom
> parsing to skip over type-parameters in a class declaration. Is doing so based
> on the endGroup of the `<` token (the code uses directly the token stream
> without using the actual parser, so this seems like a scanner bug).

(Repeating what I said on chat in case anyone is curious)

Aha! I think I know what the problem is now.  The scanner has a heuristic that
assumes parentheses can never appear between matching <>.  Annotated type
parameters violate that assumption.

Thanks, Siggi! I'll look into fixing the scanner tomorrow.

[Paul Berry, 2017-03-10 21:24:49]

PTAL.  I updated the scanner heuristics, and I fixed a latent bug in
element_listener.dart that was exposed by this change.

[Siggi Cherem (dart-lang), 2017-03-10 21:58:14]

lgtm!

[ahe, 2017-03-13 15:36:47]

lgtm

I'd like to take a look and see if we can eliminate the need for the boolean in
element_listener. But I can do that after you've submitted this CL.

https://codereview.chromium.org/2738013002/diff/20001/pkg/compiler/lib/src/pa...
File pkg/compiler/lib/src/parser/member_listener.dart (right):

https://codereview.chromium.org/2738013002/diff/20001/pkg/compiler/lib/src/pa...
pkg/compiler/lib/src/parser/member_listener.dart:167:
super.endTypeVariable(token, extendsOrSuper);
Is this necessary?

[Paul Berry, 2017-03-13 17:01:08]

https://codereview.chromium.org/2738013002/diff/20001/pkg/compiler/lib/src/pa...
File pkg/compiler/lib/src/parser/member_listener.dart (right):

https://codereview.chromium.org/2738013002/diff/20001/pkg/compiler/lib/src/pa...
pkg/compiler/lib/src/parser/member_listener.dart:167:
super.endTypeVariable(token, extendsOrSuper);
On 2017/03/13 15:36:47, ahe wrote:
> Is this necessary?

Whoops, clearly no.  I'm not sure what I was thinking.  I'll remove this before
committing.

[Paul Berry, 2017-03-13 17:19:27]

Description was changed from

==========
Add support for metadata on type variables to Fasta parser.
==========

to

==========
Add support for metadata on type variables to Fasta parser.

R=ahe@google.com, sigmund@google.com

Committed:
https://github.com/dart-lang/sdk/commit/5d8d01cdf737a662bf83ce72938ea3750da57acb
==========

[Paul Berry, 2017-03-13 17:19:28]

Committed patchset #2 (id:20001) manually as
5d8d01cdf737a662bf83ce72938ea3750da57acb (presubmit successful).

[Paul Berry, 2017-03-13 19:33:08]

On 2017/03/13 17:19:28, Paul Berry wrote:
> Committed patchset #2 (id:20001) manually as
> 5d8d01cdf737a662bf83ce72938ea3750da57acb (presubmit successful).

This caused test failures so I reverted it:
https://github.com/dart-lang/sdk/commit/71304714d4616538391d1902b11388adeac045b9

I'll track down the failures and send out a corrected patch for review.