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

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

 // Copyright (c) 2012, the Dart project authors.  Please see the AUTHORS file
 // for details. All rights reserved. Use of this source code is governed by a
 // BSD-style license that can be found in the LICENSE file.
 
 part of js_backend;
 
 class NativeEmitter {
 
   final Map<Element, ClassBuilder> cachedBuilders;
 
@@ skipping 219 matching lines @@
 
     // Emit the native class interceptors that were actually used.
     for (ClassElement classElement in classes) {
       if (!classElement.isNative) continue;
       if (neededClasses.contains(classElement)) {
         // Define interceptor class for [classElement].
         emitterTask.oldEmitter.classEmitter.emitClassBuilderWithReflectionData(
             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;
       }
     }
   }
 
   /**
    * Computes the native classes that are extended (subclassed) by non-native
    * classes and the set non-mative classes that extend them.  (A List is used
    * instead of a Set for out stability).
    */
@@ skipping 44 matching lines @@
     if (compiler.hasIncrementalSupport) {
       builder = cachedBuilders[classElement];
       if (builder != null) return builder;
       builder = new ClassBuilder(classElement, backend.namer);
       cachedBuilders[classElement] = builder;
     } else {
       builder = new ClassBuilder(classElement, backend.namer);
     }
     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);
 
     if (!hasFields &&
         builder.properties.length == propertyCount &&
         superclass is! MixinApplicationElement) {
       builder.isTrivial = true;
     }
 
@@ skipping 149 matching lines @@
         targetOutput.add(';');
       }
       targetOutput.addBuffer(jsAst.prettyPrint(
           new jsAst.ExpressionStatement(init), compiler));
       targetOutput.add('\n');
     }
 
     targetOutput.add('\n');
   }
 }