dart2js: Only emit constructors of the native class interceptors that are used.

pkg/compiler/lib/src/js_backend/native_emitter.dart

Index: pkg/compiler/lib/src/js_backend/native_emitter.dart
diff --git a/pkg/compiler/lib/src/js_backend/native_emitter.dart b/pkg/compiler/lib/src/js_backend/native_emitter.dart
index 60f11553430e71639cbfa93dd9780a1d1020a973..f400a99851f34c8d28a5646f23316992f02aa831 100644
--- a/pkg/compiler/lib/src/js_backend/native_emitter.dart
+++ b/pkg/compiler/lib/src/js_backend/native_emitter.dart
@@ -237,6 +237,10 @@ class NativeEmitter {
             backend.namer.getNameOfClass(classElement),
             classElement, builders[classElement],
             emitterTask.oldEmitter.getElementDescriptor(classElement));
+        emitterTask.oldEmitter.classEmitter.emitClassConstructor(

[floitsch, 2015/01/13 18:40:57]

Requires lots of comments, and verification.

As far as I can see (please confirm and document):
- we call generateNativeClass first, so that we have a builder that can tell us,
if the class `isTrivial`. (Sidenote: it's interesting that we have sets/maps
everywhere, but that we store the `isTrivial` in the class-builder, even though
the class builder is most often used for non-native classes.)
The `isTrivial` flag is dependent on the number of members, and it doesn't look
like it's easy to get the flag without running through all members and
generating them... (Hopefully we can fix that when building the model).

- Apparently emitting the class constructor or the getters/setters adds code
outside the builder. From what I can see this is only happening in CSP mode. (If
it happens only in CSP mode you have to add a comment in the CL description,
otherwise reviewers are in the dark).

This is a problem: if we figure out that the class is not needed we still have
those entries.


It looks like we don't need `emitClassGettersSetters` to identify if a native
class is needed or not, because getters/setters are only emitted for fields, and
in that case the `isTrivial` is already false. At least I was not able to write
a counterexample where moving the emitClassGettersSetters would have changed the
outcome of `neededClasses`.

The `emitClassConstructor` seems to be safe to move, too. As far as I can tell
its only purpose is to generate the constructors in CSP mode. As such it
shouldn't have any influence on the `neededClasses` either.

[zarah, 2015/01/15 08:10:01]

I agree with your observations. Updated the CL description and added comments. 

+            classElement, builders[classElement]);
+        emitterTask.oldEmitter.classEmitter.emitClassGettersSetters(
+            classElement, builders[classElement]);
         emitterTask.oldEmitter.needsClassSupport = true;
       }
     }
@@ -301,13 +305,10 @@ class NativeEmitter {
     }
     builder.superName = superName;
 
-    emitterTask.oldEmitter.classEmitter.emitClassConstructor(
-        classElement, builder);
     bool hasFields = emitterTask.oldEmitter.classEmitter.emitFields(
         classElement, builder, classIsNative: true);
     int propertyCount = builder.properties.length;
-    emitterTask.oldEmitter.classEmitter.emitClassGettersSetters(
-        classElement, builder);
+
     emitterTask.oldEmitter.classEmitter.emitInstanceMembers(
         classElement, builder);
     emitterTask.oldEmitter.typeTestEmitter.emitIsTests(classElement, builder);