A step towards handling `new Object()` in compile_from_dill_test

A step towards handling `new Object()` in compile_from_dill_test

R=sigmund@google.com

Committed: https://github.com/dart-lang/sdk/commit/0f7ee4a6ed43fbe12bcae84010ceea0155c5914d

[Johnni Winther, 2017-06-08 12:30:33]

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

[Siggi Cherem (dart-lang), 2017-06-08 18:01:46]

lgtm

https://codereview.chromium.org/2925263002/diff/1/pkg/compiler/lib/src/elemen...
File pkg/compiler/lib/src/elements/common.dart (right):

https://codereview.chromium.org/2925263002/diff/1/pkg/compiler/lib/src/elemen...
pkg/compiler/lib/src/elements/common.dart:476: Link<ConstructorBodyElement>
backendMembers =
TODO: rename?

https://codereview.chromium.org/2925263002/diff/1/pkg/compiler/lib/src/js_emi...
File pkg/compiler/lib/src/js_emitter/program_builder/program_builder.dart
(right):

https://codereview.chromium.org/2925263002/diff/1/pkg/compiler/lib/src/js_emi...
pkg/compiler/lib/src/js_emitter/program_builder/program_builder.dart:240:
c.setMixinClass(_classes[computeMixinClass(cls)]);
should we do here the same fix you did in the other file? Or change
computeMixinClass to take the _elementEnvironment and continue to use it in all
3 places?

https://codereview.chromium.org/2925263002/diff/1/pkg/compiler/lib/src/ssa/bu...
File pkg/compiler/lib/src/ssa/builder_kernel.dart (left):

https://codereview.chromium.org/2925263002/diff/1/pkg/compiler/lib/src/ssa/bu...
pkg/compiler/lib/src/ssa/builder_kernel.dart:306:
_elementMap.getClass(constructedClass), closedWorld),
wow... 6 times!

https://codereview.chromium.org/2925263002/diff/1/pkg/compiler/lib/src/ssa/bu...
File pkg/compiler/lib/src/ssa/builder_kernel.dart (right):

https://codereview.chromium.org/2925263002/diff/1/pkg/compiler/lib/src/ssa/bu...
pkg/compiler/lib/src/ssa/builder_kernel.dart:154: throw 'No case implemented to
handle target: $target for $targetElement';
nit: reword to fit under 80?

[Johnni Winther, 2017-06-09 09:28:49]

https://codereview.chromium.org/2925263002/diff/1/pkg/compiler/lib/src/elemen...
File pkg/compiler/lib/src/elements/common.dart (right):

https://codereview.chromium.org/2925263002/diff/1/pkg/compiler/lib/src/elemen...
pkg/compiler/lib/src/elements/common.dart:476: Link<ConstructorBodyElement>
backendMembers =
On 2017/06/08 18:01:46, Siggi Cherem (dart-lang) wrote:
> TODO: rename?

Renamed all methods - and removed the reverseBackendMembers by using a List
instead of a Link.

https://codereview.chromium.org/2925263002/diff/1/pkg/compiler/lib/src/js_emi...
File pkg/compiler/lib/src/js_emitter/program_builder/program_builder.dart
(right):

https://codereview.chromium.org/2925263002/diff/1/pkg/compiler/lib/src/js_emi...
pkg/compiler/lib/src/js_emitter/program_builder/program_builder.dart:240:
c.setMixinClass(_classes[computeMixinClass(cls)]);
On 2017/06/08 18:01:46, Siggi Cherem (dart-lang) wrote:
> should we do here the same fix you did in the other file? Or change
> computeMixinClass to take the _elementEnvironment and continue to use it in
all
> 3 places?

ElementEnvironment.forEachMixin returns all mixins, here we need just the one...

https://codereview.chromium.org/2925263002/diff/1/pkg/compiler/lib/src/ssa/bu...
File pkg/compiler/lib/src/ssa/builder_kernel.dart (right):

https://codereview.chromium.org/2925263002/diff/1/pkg/compiler/lib/src/ssa/bu...
pkg/compiler/lib/src/ssa/builder_kernel.dart:154: throw 'No case implemented to
handle target: $target for $targetElement';
On 2017/06/08 18:01:46, Siggi Cherem (dart-lang) wrote:
> nit: reword to fit under 80?

Split the string.

[Johnni Winther, 2017-06-09 09:58:38]

Description was changed from

==========
A step towards handling `new Object()` in compile_from_dill_test
==========

to

==========
A step towards handling `new Object()` in compile_from_dill_test

R=sigmund@google.com

Committed:
https://github.com/dart-lang/sdk/commit/0f7ee4a6ed43fbe12bcae84010ceea0155c5914d
==========

[Johnni Winther, 2017-06-09 09:58:39]

Committed patchset #2 (id:20001) manually as
0f7ee4a6ed43fbe12bcae84010ceea0155c5914d (presubmit successful).

[Siggi Cherem (dart-lang), 2017-06-14 18:13:24]

lgtm

https://codereview.chromium.org/2925263002/diff/1/pkg/compiler/lib/src/js_emi...
File pkg/compiler/lib/src/js_emitter/program_builder/program_builder.dart
(right):

https://codereview.chromium.org/2925263002/diff/1/pkg/compiler/lib/src/js_emi...
pkg/compiler/lib/src/js_emitter/program_builder/program_builder.dart:240:
c.setMixinClass(_classes[computeMixinClass(cls)]);
On 2017/06/09 09:28:49, Johnni Winther wrote:
> On 2017/06/08 18:01:46, Siggi Cherem (dart-lang) wrote:
> > should we do here the same fix you did in the other file? Or change
> > computeMixinClass to take the _elementEnvironment and continue to use it in
> all
> > 3 places?
> 
> ElementEnvironment.forEachMixin returns all mixins, here we need just the
one...

are you saying that the code in program_builder/collector.dart was actually
broken?

[Johnni Winther, 2017-06-15 07:44:12]

https://codereview.chromium.org/2925263002/diff/1/pkg/compiler/lib/src/js_emi...
File pkg/compiler/lib/src/js_emitter/program_builder/program_builder.dart
(right):

https://codereview.chromium.org/2925263002/diff/1/pkg/compiler/lib/src/js_emi...
pkg/compiler/lib/src/js_emitter/program_builder/program_builder.dart:240:
c.setMixinClass(_classes[computeMixinClass(cls)]);
On 2017/06/14 18:13:24, Siggi Cherem (dart-lang) wrote:
> On 2017/06/09 09:28:49, Johnni Winther wrote:
> > On 2017/06/08 18:01:46, Siggi Cherem (dart-lang) wrote:
> > > should we do here the same fix you did in the other file? Or change
> > > computeMixinClass to take the _elementEnvironment and continue to use it
in
> > all
> > > 3 places?
> > 
> > ElementEnvironment.forEachMixin returns all mixins, here we need just the
> one...
> 
> are you saying that the code in program_builder/collector.dart was actually
> broken?

It wasn't - just weird: computeMixinClass didn't make sense when it was written
(a mixed in class could not itself be a mixin application). Turned out that the
refactoring was inconsistent. Fixed in
https://codereview.chromium.org/2934783002/