dart2js: native classes go through the normal code path.

dart2js: native classes go through the normal code path.

R=herhut@google.com

Committed: https://code.google.com/p/dart/source/detail?r=43178

[floitsch, 2015-01-26 13:34:11]

floitsch@google.com changed reviewers:
+ herhut@google.com

[floitsch, 2015-01-26 13:34:12]

https://codereview.chromium.org/878553002/diff/20001/pkg/compiler/lib/src/js_...
File pkg/compiler/lib/src/js_emitter/old_emitter/reflection_data_parser.dart
(left):

https://codereview.chromium.org/878553002/diff/20001/pkg/compiler/lib/src/js_...
pkg/compiler/lib/src/js_emitter/old_emitter/reflection_data_parser.dart:26:
String metadataField = '"${namer.metadataField}"';
was unused before.

[herhut, 2015-01-26 14:58:08]

lgtm

https://codereview.chromium.org/878553002/diff/20001/pkg/compiler/lib/src/js_...
File pkg/compiler/lib/src/js_emitter/code_emitter_task.dart (right):

https://codereview.chromium.org/878553002/diff/20001/pkg/compiler/lib/src/js_...
pkg/compiler/lib/src/js_emitter/code_emitter_task.dart:40: final
List<ClassElement> nativeOrExtendsNativeClasses = <ClassElement>[];
How about nativeClassesAndSubclasses ?

https://codereview.chromium.org/878553002/diff/20001/pkg/compiler/lib/src/js_...
File pkg/compiler/lib/src/js_emitter/native_emitter.dart (right):

https://codereview.chromium.org/878553002/diff/20001/pkg/compiler/lib/src/js_...
pkg/compiler/lib/src/js_emitter/native_emitter.dart:46: * Prepares native
classes for emission. Returns the unneeded classes.
Why is this better? Do you expect that this set is smaller?

https://codereview.chromium.org/878553002/diff/20001/pkg/compiler/lib/src/js_...
pkg/compiler/lib/src/js_emitter/native_emitter.dart:71:
assert(classes.every((Class cls) => cls != null));
Why is this needed?

[floitsch, 2015-01-26 17:57:50]

https://codereview.chromium.org/878553002/diff/20001/pkg/compiler/lib/src/js_...
File pkg/compiler/lib/src/js_emitter/code_emitter_task.dart (right):

https://codereview.chromium.org/878553002/diff/20001/pkg/compiler/lib/src/js_...
pkg/compiler/lib/src/js_emitter/code_emitter_task.dart:40: final
List<ClassElement> nativeOrExtendsNativeClasses = <ClassElement>[];
On 2015/01/26 14:58:08, herhut wrote:
> How about nativeClassesAndSubclasses ?

Done.

https://codereview.chromium.org/878553002/diff/20001/pkg/compiler/lib/src/js_...
File pkg/compiler/lib/src/js_emitter/native_emitter.dart (right):

https://codereview.chromium.org/878553002/diff/20001/pkg/compiler/lib/src/js_...
pkg/compiler/lib/src/js_emitter/native_emitter.dart:46: * Prepares native
classes for emission. Returns the unneeded classes.
On 2015/01/26 14:58:08, herhut wrote:
> Why is this better? Do you expect that this set is smaller?

I think it's smaller, but more importantly it's a clear set.
If I return the "needed" classes, it would be a needed *subset* of all needed
classes. Basically I would need to do something like `if (!cls.isNative ||
needed.contains(cls))`. And even then it would not be clear that I caught all
classes: does it include mixins?...

https://codereview.chromium.org/878553002/diff/20001/pkg/compiler/lib/src/js_...
pkg/compiler/lib/src/js_emitter/native_emitter.dart:71:
assert(classes.every((Class cls) => cls != null));
On 2015/01/26 14:58:08, herhut wrote:
> Why is this needed?

If the mapping from classElement to Class fails in the program builder it could
fail here.
I can remove it, but I don't think it's expensive.

[herhut, 2015-01-27 09:50:29]

https://codereview.chromium.org/878553002/diff/20001/pkg/compiler/lib/src/js_...
File pkg/compiler/lib/src/js_emitter/native_emitter.dart (right):

https://codereview.chromium.org/878553002/diff/20001/pkg/compiler/lib/src/js_...
pkg/compiler/lib/src/js_emitter/native_emitter.dart:71:
assert(classes.every((Class cls) => cls != null));
On 2015/01/26 17:57:50, floitsch wrote:
> On 2015/01/26 14:58:08, herhut wrote:
> > Why is this needed?
> 
> If the mapping from classElement to Class fails in the program builder it
could
> fail here.
> I can remove it, but I don't think it's expensive.

I was just curious how this could happen.

[floitsch, 2015-01-27 12:15:04]

Committed patchset #4 (id:60001) manually as 43178 (presubmit successful).