Rewrite mixin application handling in Fasta.

Rewrite mixin application handling in Fasta.

R=johnniwinther@google.com

Committed: https://github.com/dart-lang/sdk/commit/4880a2876e02f0764a8b4faada9f44dc1fcc80f7

[ahe, 2017-05-05 13:19:12]

ahe@google.com changed reviewers:
+ johnniwinther@google.com

[ahe, 2017-05-05 13:19:12]

This CL is based on my development branch. I plan to rebase it on master.

Johnni, it's up to you: you can either start reviewing this now, or wait until
I've rebased it on master.

[ahe, 2017-05-05 16:53:09]

I've merged with master.

[Johnni Winther, 2017-05-08 10:05:34]

lgtm

https://codereview.chromium.org/2862223002/diff/1/pkg/front_end/lib/src/fasta...
File pkg/front_end/lib/src/fasta/builder/named_type_builder.dart (right):

https://codereview.chromium.org/2862223002/diff/1/pkg/front_end/lib/src/fasta...
pkg/front_end/lib/src/fasta/builder/named_type_builder.dart:59: if
(arguments?.isEmpty ?? true) return buffer;
This is weird to read. I'd prefer `arguments == null || arguments.isEmpty`

https://codereview.chromium.org/2862223002/diff/1/pkg/front_end/lib/src/fasta...
File pkg/front_end/lib/src/fasta/kernel/kernel_library_builder.dart (right):

https://codereview.chromium.org/2862223002/diff/1/pkg/front_end/lib/src/fasta...
pkg/front_end/lib/src/fasta/kernel/kernel_library_builder.dart:177: if (owner is
ClassBuilder) {
Why is the only for classes? Aren't `typedef F<F>()` and `m<m>() {}` also errors

https://codereview.chromium.org/2862223002/diff/1/pkg/front_end/lib/src/fasta...
pkg/front_end/lib/src/fasta/kernel/kernel_library_builder.dart:371: var t =
addNamedType(n, null, -1)..bind(variables[n]);
var t = ...
return t;

->

return ...

https://codereview.chromium.org/2862223002/diff/1/pkg/front_end/lib/src/fasta...
pkg/front_end/lib/src/fasta/kernel/kernel_library_builder.dart:402: return t;
Ditto

[Johnni Winther, 2017-05-08 10:44:59]

2nd patch lgtm

[ahe, 2017-05-08 11:13:16]

Thank you, Johnni!

https://codereview.chromium.org/2862223002/diff/1/pkg/front_end/lib/src/fasta...
File pkg/front_end/lib/src/fasta/builder/named_type_builder.dart (right):

https://codereview.chromium.org/2862223002/diff/1/pkg/front_end/lib/src/fasta...
pkg/front_end/lib/src/fasta/builder/named_type_builder.dart:59: if
(arguments?.isEmpty ?? true) return buffer;
On 2017/05/08 10:05:33, Johnni Winther wrote:
> This is weird to read. I'd prefer `arguments == null || arguments.isEmpty`

Other reviewers have said the opposite, so I'm going to stick with what I have
:-)

https://codereview.chromium.org/2862223002/diff/1/pkg/front_end/lib/src/fasta...
File pkg/front_end/lib/src/fasta/kernel/kernel_library_builder.dart (right):

https://codereview.chromium.org/2862223002/diff/1/pkg/front_end/lib/src/fasta...
pkg/front_end/lib/src/fasta/kernel/kernel_library_builder.dart:177: if (owner is
ClassBuilder) {
On 2017/05/08 10:05:34, Johnni Winther wrote:
> Why is the only for classes? Aren't `typedef F<F>()` and `m<m>() {}` also
errors

Not according to our tests, and it also seems like the language team is leaning
in the same direction:

https://github.com/dart-lang/sdk/issues/29555

I'll add  a comment about the bug.

https://codereview.chromium.org/2862223002/diff/1/pkg/front_end/lib/src/fasta...
pkg/front_end/lib/src/fasta/kernel/kernel_library_builder.dart:371: var t =
addNamedType(n, null, -1)..bind(variables[n]);
On 2017/05/08 10:05:34, Johnni Winther wrote:
> var t = ...
> return t;
> 
> ->
> 
> return ...

Done.

https://codereview.chromium.org/2862223002/diff/1/pkg/front_end/lib/src/fasta...
pkg/front_end/lib/src/fasta/kernel/kernel_library_builder.dart:402: return t;
On 2017/05/08 10:05:34, Johnni Winther wrote:
> Ditto

Done.

[ahe, 2017-05-09 07:23:12]

Description was changed from

==========
Rewrite mixin application handling in Fasta.
==========

to

==========
Rewrite mixin application handling in Fasta.

R=johnniwinther@google.com

Committed:
https://github.com/dart-lang/sdk/commit/4880a2876e02f0764a8b4faada9f44dc1fcc80f7
==========

[ahe, 2017-05-09 07:23:13]

Committed patchset #3 (id:40001) manually as
4880a2876e02f0764a8b4faada9f44dc1fcc80f7 (presubmit successful).