dart2js: Support mixins in the new emitter.

pkg/compiler/lib/src/js_emitter/new_emitter/model_emitter.dart

Index: pkg/compiler/lib/src/js_emitter/new_emitter/model_emitter.dart
diff --git a/pkg/compiler/lib/src/js_emitter/new_emitter/model_emitter.dart b/pkg/compiler/lib/src/js_emitter/new_emitter/model_emitter.dart
index 1df89ae5f815765aab7f02782a67071917199734..9ad9eca4b8e1d8687c7a3dee1b6a0862a5079211 100644
--- a/pkg/compiler/lib/src/js_emitter/new_emitter/model_emitter.dart
+++ b/pkg/compiler/lib/src/js_emitter/new_emitter/model_emitter.dart
@@ -256,11 +256,26 @@ class ModelEmitter {
 
   js.Expression _generateConstructor(Class cls) {
     List<String> allFieldNames = <String>[];
-    Class currentClass = cls;
-    while (currentClass != null) {
-      allFieldNames.addAll(
-          currentClass.fields.map((InstanceField field) => field.name));
-      currentClass = currentClass.superclass;
+
+    // If the class is not directly instantiated we only need it for inheritance
+    // or RTI. In either case we don't need its fields.
+    if (cls.isDirectlyInstantiated) {
+      Class currentClass = cls;
+      while (currentClass != null) {
+        assert(() {

[kasperl, 2014/11/27 09:05:14]

Add a comment for the assert. What are you verifying? It might be easier to
diagnose assertion failures if you split these out as separate asserts, but I
understand that you prefer not to have the mixinApplication variable introduced
outside the asserts. Maybe using an 'as' cast in an assert would be okay?

[floitsch, 2014/11/27 12:35:16]

Reduced the assert.
It is now just testing that mixins don't introduce new fields.
The fields and methods are empty by construction (in the constructor), so it is
redundant to check it here. And I'm not too afraid that refactorings would
change that.

done.

+          if (currentClass.isMixinApplication) {
+            if (currentClass.fields.isNotEmpty) return false;
+            if (currentClass.methods.isNotEmpty) return false;
+            MixinApplication mixinApplication = currentClass;
+            if (mixinApplication.mixinClass.fields.isNotEmpty) return false;
+          }
+          return true;
+        });
+
+        allFieldNames.addAll(
+            currentClass.fields.map((InstanceField field) => field.name));
+        currentClass = currentClass.superclass;
+      }
     }
     String name = cls.name;
     String parameters = allFieldNames.join(', ');
@@ -316,6 +331,8 @@ class ModelEmitter {
   }
 
   js.Expression emitClass(Class cls) {
+    if (cls.isMixinApplication) return emitMixinApplication(cls);
+
     List elements = [ js.string(cls.superclassName),
                       js.number(cls.superclassHolderIndex),
                       _generateConstructor(cls) ];
@@ -326,6 +343,16 @@ class ModelEmitter {
     return unparse(compiler, new js.ArrayInitializer.from(elements));
   }
 
+  static final String mixinFormatComment =
+      "Mixins have no constructor, but a reference to their mixin class.";

[sigurdm, 2014/11/27 09:25:54]

Empty line between members.

[floitsch, 2014/11/27 12:35:16]

Done.

+  js.Expression emitMixinApplication(MixinApplication cls) {
+    List elements = [ js.string(cls.superclassName),

[sigurdm, 2014/11/27 09:25:54]

Space inside the '['

[floitsch, 2014/11/27 12:35:16]

Done.

+                      js.number(cls.superclassHolderIndex),
+                      js.string(cls.mixinClass.name),
+                      js.number(cls.mixinClass.holder.index) ];
+    return unparse(compiler, new js.ArrayInitializer.from(elements));
+  }
+
   js.Expression emitLazyInitializer(StaticField field) {
     assert(field.isLazy);
     return unparse(compiler, field.code);
@@ -334,9 +361,8 @@ class ModelEmitter {
   js.Expression emitStaticMethod(StaticMethod method) {
     return unparse(compiler, method.code);
   }
-}
 
-final String boilerplate = r"""
+  static final String boilerplate = """
 {
 // Declare deferred-initializer global.
 #;
@@ -434,6 +460,10 @@ final String boilerplate = r"""
   function compileConstructor(name, descriptor) {
     descriptor = compile(name, descriptor);
     var prototype = determinePrototype(descriptor);
+    // $mixinFormatComment.

[floitsch, 2014/11/26 17:32:09]

I'm using this "marker" as a way to easily find all references of the
mixin-format, and to find the format-specification from its uses.

[sigurdm, 2014/11/27 09:25:54]

Perhaps also explain that in a comment in the code.

[floitsch, 2014/11/27 12:35:16]

Done.

+    if (typeof descriptor[2] !== 'function') {

[sigurdm, 2014/11/27 09:25:54]

Could there be a less fragile way of distinguishing mixins from other classes?

[floitsch, 2014/11/27 12:35:16]

I don't see any, that wouldn't require bigger changes, or that would waste
space.

+      return compileMixinConstructor(name, prototype, descriptor);
+    }
     var constructor = descriptor[2];
     for (var i = 3; i < descriptor.length; i += 2) {
       prototype[descriptor[i]] = descriptor[i + 1];
@@ -442,6 +472,26 @@ final String boilerplate = r"""
     return constructor;
   }
 
+  function compileMixinConstructor(name, prototype, descriptor) {
+    var mixinName = descriptor[2];
+    var mixinHolderIndex = descriptor[3];
+    var mixin = holders[mixinHolderIndex][mixinName];
+    if (mixin.resolve) mixin = mixin.resolve();
+    var mixinPrototype = mixin.prototype;
+
+    // Fill the prototype with the mixin's properties.
+    var mixinProperties = Object.keys(mixinPrototype);
+    for (var i = 0; i < mixinProperties.length; i++) {
+      var p = mixinProperties[i];
+      prototype[p] = mixinPrototype[p];
+    }
+    // Since this is a mixin application the constructor will actually never
+    // be invoked. We only use its prototype for the application's subclasses. 
+    var constructor = function() {};
+    constructor.prototype = prototype;
+    return constructor;
+  }
+
   function determinePrototype(descriptor) {
     var superclassName = descriptor[0];
     if (!superclassName) return { };
@@ -461,7 +511,7 @@ final String boilerplate = r"""
     'use strict';
     // TODO(floitsch): evaluate the performance impact of the string
     // concatenations.
-    return eval(__s__ + "\n//# sourceURL=" + __name__ + ".js");
+    return eval(__s__ + "\\n//# sourceURL=" + __name__ + ".js");
   }
 
   if (#) { // outputContainsConstantList
@@ -469,8 +519,8 @@ final String boilerplate = r"""
       // By assigning a function to the properties they become part of the
       // hidden class. The actual values of the fields don't matter, since we
       // only check if they exist.
-      list.immutable$list = Array;
-      list.fixed$length = Array;
+      list.immutable\$list = Array;
+      list.fixed\$length = Array;
       return list;
     }
   }
@@ -496,3 +546,5 @@ final String boilerplate = r"""
 
 }(Date.now(), #)
 }""";
+
+}