Don't export unnamed mixins.

Don't export unnamed mixins.

Fixes https://github.com/dart-lang/sdk/issues/30553

R=sigmund@google.com

Committed: https://github.com/dart-lang/sdk/commit/48503c8720d758a61cc4ceb1281bdd81fbd92e13

[ahe, 2017-08-28 12:21:44]

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

[ahe, 2017-08-28 12:22:39]

Description was changed from

==========
Don't export unnamed mixins.
==========

to

==========
Don't export unnamed mixins.

Fixes https://github.com/dart-lang/sdk/issues/30553
==========

[Siggi Cherem (dart-lang), 2017-08-28 15:57:55]

lgtm

https://codereview.chromium.org/3004703002/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/3004703002/diff/1/pkg/front_end/lib/src/fasta...
pkg/front_end/lib/src/fasta/kernel/kernel_library_builder.dart:245: name =
"_$name&${mixin.name}$signature";
minor style question for the generated code: do we care if we have "__" or
should we check whether `name` is already private?

https://codereview.chromium.org/3004703002/diff/1/pkg/front_end/testcases/sha...
File pkg/front_end/testcases/shaker.status (right):

https://codereview.chromium.org/3004703002/diff/1/pkg/front_end/testcases/sha...
pkg/front_end/testcases/shaker.status:9: transitive_mixins: Fail # Don't know
how to update expectations.
not sure I understand the reason in this comment

[ahe, 2017-08-28 18:19:24]

Thank you, Siggi!

https://codereview.chromium.org/3004703002/diff/1/pkg/front_end/testcases/sha...
File pkg/front_end/testcases/shaker.status (right):

https://codereview.chromium.org/3004703002/diff/1/pkg/front_end/testcases/sha...
pkg/front_end/testcases/shaker.status:9: transitive_mixins: Fail # Don't know
how to update expectations.
On 2017/08/28 15:57:54, Siggi Cherem (dart-lang) wrote:
> not sure I understand the reason in this comment

The expectation file
(pkg/front_end/testcases/shaker/transitive_mixins.dart.shaker) says:

DO NOT EDIT -- this file is autogenerated ---

What am I supposed to edit instead?

[ahe, 2017-08-28 18:22:45]

https://codereview.chromium.org/3004703002/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/3004703002/diff/1/pkg/front_end/lib/src/fasta...
pkg/front_end/lib/src/fasta/kernel/kernel_library_builder.dart:245: name =
"_$name&${mixin.name}$signature";
On 2017/08/28 15:57:54, Siggi Cherem (dart-lang) wrote:
> minor style question for the generated code: do we care if we have "__" or
> should we check whether `name` is already private?

Actually, I started with a check that I only added "_" if the name wasn't
private already. However, then I realized that we would have problems in this
case:

class Foo {}
class _Foo {}
class M {}

class Bar extends Foo with M {}
class _Bar extends _Foo with M {}

In this case, we'd get two *different* mixin applications with the same name.

[Siggi Cherem (dart-lang), 2017-08-28 19:43:29]

https://codereview.chromium.org/3004703002/diff/1/pkg/front_end/testcases/sha...
File pkg/front_end/testcases/shaker.status (right):

https://codereview.chromium.org/3004703002/diff/1/pkg/front_end/testcases/sha...
pkg/front_end/testcases/shaker.status:9: transitive_mixins: Fail # Don't know
how to update expectations.
On 2017/08/28 18:19:24, ahe wrote:
> On 2017/08/28 15:57:54, Siggi Cherem (dart-lang) wrote:
> > not sure I understand the reason in this comment
> 
> The expectation file
> (pkg/front_end/testcases/shaker/transitive_mixins.dart.shaker) says:
> 
> DO NOT EDIT -- this file is autogenerated ---
> 
> What am I supposed to edit instead?

Thank you, we can make the .shaker file self document how to regenerate it.

You can do it in a similar way as with the other tests via:

  ./pkg/front_end/tool/fasta testing -DupdateExpectations=true
shaker/transitive_mixins

[ahe, 2017-08-29 09:15:24]

https://codereview.chromium.org/3004703002/diff/1/pkg/front_end/testcases/sha...
File pkg/front_end/testcases/shaker.status (right):

https://codereview.chromium.org/3004703002/diff/1/pkg/front_end/testcases/sha...
pkg/front_end/testcases/shaker.status:9: transitive_mixins: Fail # Don't know
how to update expectations.
On 2017/08/28 19:43:29, Siggi Cherem (dart-lang) wrote:
> On 2017/08/28 18:19:24, ahe wrote:
> > On 2017/08/28 15:57:54, Siggi Cherem (dart-lang) wrote:
> > > not sure I understand the reason in this comment
> > 
> > The expectation file
> > (pkg/front_end/testcases/shaker/transitive_mixins.dart.shaker) says:
> > 
> > DO NOT EDIT -- this file is autogenerated ---
> > 
> > What am I supposed to edit instead?
> 
> Thank you, we can make the .shaker file self document how to regenerate it.
> 
> You can do it in a similar way as with the other tests via:
> 
>   ./pkg/front_end/tool/fasta testing -DupdateExpectations=true
> shaker/transitive_mixins

So in other words, I'm actually supposed to edit
pkg/front_end/testcases/shaker/transitive_mixins.dart.shaker if the expectations
changes?

[ahe, 2017-08-29 14:33:39]

Description was changed from

==========
Don't export unnamed mixins.

Fixes https://github.com/dart-lang/sdk/issues/30553
==========

to

==========
Don't export unnamed mixins.

Fixes https://github.com/dart-lang/sdk/issues/30553

R=sigmund@google.com

Committed:
https://github.com/dart-lang/sdk/commit/48503c8720d758a61cc4ceb1281bdd81fbd92e13
==========

[ahe, 2017-08-29 14:33:40]

Committed patchset #2 (id:20001) manually as
48503c8720d758a61cc4ceb1281bdd81fbd92e13 (presubmit successful).

[Siggi Cherem (dart-lang), 2017-08-29 15:43:10]

https://codereview.chromium.org/3004703002/diff/1/pkg/front_end/testcases/sha...
File pkg/front_end/testcases/shaker.status (right):

https://codereview.chromium.org/3004703002/diff/1/pkg/front_end/testcases/sha...
pkg/front_end/testcases/shaker.status:9: transitive_mixins: Fail # Don't know
how to update expectations.
On 2017/08/29 09:15:24, ahe wrote:
> On 2017/08/28 19:43:29, Siggi Cherem (dart-lang) wrote:
> > On 2017/08/28 18:19:24, ahe wrote:
> > > On 2017/08/28 15:57:54, Siggi Cherem (dart-lang) wrote:
> > > > not sure I understand the reason in this comment
> > > 
> > > The expectation file
> > > (pkg/front_end/testcases/shaker/transitive_mixins.dart.shaker) says:
> > > 
> > > DO NOT EDIT -- this file is autogenerated ---
> > > 
> > > What am I supposed to edit instead?
> > 
> > Thank you, we can make the .shaker file self document how to regenerate it.
> > 
> > You can do it in a similar way as with the other tests via:
> > 
> >   ./pkg/front_end/tool/fasta testing -DupdateExpectations=true
> > shaker/transitive_mixins
> 
> So in other words, I'm actually supposed to edit
> pkg/front_end/testcases/shaker/transitive_mixins.dart.shaker if the
expectations
> changes?

correct - would it help if we change the extension from .shaker to
.shaker.expect ?

It's just like the expected golden files you have for kernel code, except that
the contents are tailored to the shaker

[ahe, 2017-08-29 16:24:57]

https://codereview.chromium.org/3004703002/diff/1/pkg/front_end/testcases/sha...
File pkg/front_end/testcases/shaker.status (right):

https://codereview.chromium.org/3004703002/diff/1/pkg/front_end/testcases/sha...
pkg/front_end/testcases/shaker.status:9: transitive_mixins: Fail # Don't know
how to update expectations.
On 2017/08/29 15:43:10, Siggi Cherem (dart-lang) wrote:
> On 2017/08/29 09:15:24, ahe wrote:
> > On 2017/08/28 19:43:29, Siggi Cherem (dart-lang) wrote:
> > > On 2017/08/28 18:19:24, ahe wrote:
> > > > On 2017/08/28 15:57:54, Siggi Cherem (dart-lang) wrote:
> > > > > not sure I understand the reason in this comment
> > > > 
> > > > The expectation file
> > > > (pkg/front_end/testcases/shaker/transitive_mixins.dart.shaker) says:
> > > > 
> > > > DO NOT EDIT -- this file is autogenerated ---
> > > > 
> > > > What am I supposed to edit instead?
> > > 
> > > Thank you, we can make the .shaker file self document how to regenerate
it.
> > > 
> > > You can do it in a similar way as with the other tests via:
> > > 
> > >   ./pkg/front_end/tool/fasta testing -DupdateExpectations=true
> > > shaker/transitive_mixins
> > 
> > So in other words, I'm actually supposed to edit
> > pkg/front_end/testcases/shaker/transitive_mixins.dart.shaker if the
> expectations
> > changes?
> 
> correct - would it help if we change the extension from .shaker to
> .shaker.expect ?
> 
> It's just like the expected golden files you have for kernel code, except that
> the contents are tailored to the shaker

That would help. And also if the file didn't say "do not edit" :-)