Work around for private names in mixin applications.

Work around for private names in mixin applications.

R=kasperl@google.com

Committed: https://github.com/dart-lang/rasta/commit/a9f39d2b2d589814f6a4f822afe4fd41d8bf13af

[ahe, 2016-07-01 14:28:36]

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

[ahe, 2016-08-01 10:16:23]

ping

[kustermann, 2016-08-01 10:37:39]

kustermann@google.com changed reviewers:
+ kustermann@google.com

[kustermann, 2016-08-01 10:37:39]

https://codereview.chromium.org/2116793002/diff/20001/lib/kernel.dart
File lib/kernel.dart (right):

https://codereview.chromium.org/2116793002/diff/20001/lib/kernel.dart#newcode146
lib/kernel.dart:146: name = "$name@${element.library.libraryOrScriptName}";
I landed a modified version of this patch on origin/temporary. The modification
included not doing any name mangling (since this caused some issues IIRC).

[ahe, 2016-08-01 10:44:42]

https://codereview.chromium.org/2116793002/diff/20001/lib/kernel.dart
File lib/kernel.dart (right):

https://codereview.chromium.org/2116793002/diff/20001/lib/kernel.dart#newcode146
lib/kernel.dart:146: name = "$name@${element.library.libraryOrScriptName}";
On 2016/08/01 10:37:39, kustermann wrote:
> I landed a modified version of this patch on origin/temporary. The
modification
> included not doing any name mangling (since this caused some issues IIRC).

What kind of issues? In the Dart VM?

[kustermann, 2016-08-01 11:23:39]

https://codereview.chromium.org/2116793002/diff/20001/lib/kernel.dart
File lib/kernel.dart (right):

https://codereview.chromium.org/2116793002/diff/20001/lib/kernel.dart#newcode146
lib/kernel.dart:146: name = "$name@${element.library.libraryOrScriptName}";
On 2016/08/01 10:44:41, ahe wrote:
> On 2016/08/01 10:37:39, kustermann wrote:
> > I landed a modified version of this patch on origin/temporary. The
> modification
> > included not doing any name mangling (since this caused some issues IIRC).
> 
> What kind of issues? In the Dart VM?

I don't recall the details, since it's almost 4 weeks ago, but I suppose it
should be easy to try running the result on the VM and see  what the difference
in behavior is.

[kasperl, 2016-08-02 06:17:44]

LGTM with a (hopefully) helpful comment:

https://codereview.chromium.org/2116793002/diff/20001/lib/kernel.dart
File lib/kernel.dart (right):

https://codereview.chromium.org/2116793002/diff/20001/lib/kernel.dart#newcode146
lib/kernel.dart:146: name = "$name@${element.library.libraryOrScriptName}";
On 2016/08/01 11:23:39, kustermann wrote:
> On 2016/08/01 10:44:41, ahe wrote:
> > On 2016/08/01 10:37:39, kustermann wrote:
> > > I landed a modified version of this patch on origin/temporary. The
> > modification
> > > included not doing any name mangling (since this caused some issues IIRC).
> > 
> > What kind of issues? In the Dart VM?
> 
> I don't recall the details, since it's almost 4 weeks ago, but I suppose it
> should be easy to try running the result on the VM and see  what the
difference
> in behavior is.

I think the VM may have special treatment of symbols with @ in them (private
symbols). Maybe just pick a different separator character?

https://codereview.chromium.org/2116793002/diff/20001/lib/kernel_visitor.dart
File lib/kernel_visitor.dart (right):

https://codereview.chromium.org/2116793002/diff/20001/lib/kernel_visitor.dart...
lib/kernel_visitor.dart:313: assert(
Maybe 

  assert(!name.isPrivate || ...)

?

[ahe, 2016-08-02 11:05:50]

Patchset #3 (id:40001) has been deleted

[ahe, 2016-08-02 11:09:10]

Thank you. With @ flutter-aot-jit crashes, with :: it doesn't (when
bootstrapping dart2js).

https://codereview.chromium.org/2116793002/diff/20001/lib/kernel.dart
File lib/kernel.dart (right):

https://codereview.chromium.org/2116793002/diff/20001/lib/kernel.dart#newcode146
lib/kernel.dart:146: name = "$name@${element.library.libraryOrScriptName}";
On 2016/08/02 06:17:44, kasperl wrote:
> On 2016/08/01 11:23:39, kustermann wrote:
> > On 2016/08/01 10:44:41, ahe wrote:
> > > On 2016/08/01 10:37:39, kustermann wrote:
> > > > I landed a modified version of this patch on origin/temporary. The
> > > modification
> > > > included not doing any name mangling (since this caused some issues
IIRC).
> > > 
> > > What kind of issues? In the Dart VM?
> > 
> > I don't recall the details, since it's almost 4 weeks ago, but I suppose it
> > should be easy to try running the result on the VM and see  what the
> difference
> > in behavior is.
> 
> I think the VM may have special treatment of symbols with @ in them (private
> symbols). Maybe just pick a different separator character?

This seems to work:

"${element.library.libraryOrScriptName}::$name"

https://codereview.chromium.org/2116793002/diff/20001/lib/kernel_visitor.dart
File lib/kernel_visitor.dart (right):

https://codereview.chromium.org/2116793002/diff/20001/lib/kernel_visitor.dart...
lib/kernel_visitor.dart:313: assert(
On 2016/08/02 06:17:44, kasperl wrote:
> Maybe 
> 
>   assert(!name.isPrivate || ...)
> 
> ?

Done.

[ahe, 2016-08-02 11:17:05]

Description was changed from

==========
Work around for private names in mixin applications.
==========

to

==========
Work around for private names in mixin applications.

R=kasperl@google.com

Committed:
https://github.com/dart-lang/rasta/commit/a9f39d2b2d589814f6a4f822afe4fd41d8b...
==========

[ahe, 2016-08-02 11:17:06]

Committed patchset #4 (id:80001) manually as
a9f39d2b2d589814f6a4f822afe4fd41d8bf13af (presubmit successful).

[kustermann, 2016-08-02 14:26:33]

The name mangling cause an issue in the following case (and the mangling
shouldn't be necessary):


// mixin lib

class Mixin {
  foo() => print('foo');
  _bar() => foo();
}

doit(Mixin a) => a._bar();

// usage

class X extends Object with Mixin {}

main() {
  doit(new X());
}

[ahe, 2016-08-03 11:17:17]

On 2016/08/02 14:26:33, kustermann wrote:
> The name mangling cause an issue in the following case (and the mangling
> shouldn't be necessary):

Thank you, Martin. I fixed that in https://codereview.chromium.org/2205943003.