Fix #29003

pkg/dev_compiler/lib/src/compiler/code_generator.dart

Index: pkg/dev_compiler/lib/src/compiler/code_generator.dart
diff --git a/pkg/dev_compiler/lib/src/compiler/code_generator.dart b/pkg/dev_compiler/lib/src/compiler/code_generator.dart
index 50d6c19e5535a112484041abfd2cd437bdcaf34b..f00fd619c7160d7b5d31c05d57cc6873156aa5a4 100644
--- a/pkg/dev_compiler/lib/src/compiler/code_generator.dart
+++ b/pkg/dev_compiler/lib/src/compiler/code_generator.dart
@@ -1274,6 +1274,31 @@ class CodeGenerator extends GeneralizingAstVisitor
         typeParams: typeParams, fields: jsFields);
   }
 
+  /// In some corner cases, a mixin may have its own mixins.
+  /// E.g., M2 is a valid mixin:
+  ///
+  /// class M1 { int foo() => 42; }
+  /// class M2 = Object with M1;
+  /// class M3 = Object with M2;
+  ///
+  /// We expand out the list of mixins to treat M1 and M2 as separate
+  /// applications.
+  List<InterfaceType> _flattenMixins(List<InterfaceType> mixins) {

[Jennifer Messerly, 2017/03/27 22:15:15]

I don't think this is correct in general?

it will expand out mixins arbitrarily, yes it happens to work with "class M2 =
Object with M1;" but it won't work with:

    class M2 extends Object with M1 { foo() => 123 }

... the method "foo" above would just get discarded?

For the moment, that may be disallowed by another check, but in general
flattening out seems really bad. For example:

library 1:
    // Private!
    class _M1 { int foo() => 42; }
    class M2 = Object with _M1;

library 2:

    class M3 extends M2 {} // does this expand to _M1 ??


... with this fix, wouldn't we try to generate a private reference to M1 from
library2  ... that might happen to work but it's very unsafe.

Finally ... this would support invalid things like:
    class M1 { int foo() => 42; }
    class M2 { int foo() => 42; }
    class M3 = Object with M1, M2;
    class M4 extends Object with M3 {} // not actually valid??

I described this in more detail here:
https://github.com/dart-lang/sdk/issues/29003#issuecomment-285471442

... what's going on here is that "class M2 = Object with M1;" is like a special
form of typedef.

I think the way to support this is handling the special case "type A = Object
with B" in visitClassTypeAlias. That has a lot less risk of accidentally causing
other breakage.

+    if (mixins.any((m) => m.mixins.isNotEmpty)) {
+      // Clone, so we do not overwrite.
+      mixins = new List<InterfaceType>.from(mixins);
+      for (int i = 0; i < mixins.length; ++i) {
+        var mixin = mixins[i];
+        if (mixin.mixins.isNotEmpty) {
+          var indirectMixins = _flattenMixins(mixin.mixins);
+          mixins.insertAll(i, indirectMixins);
+          i += indirectMixins.length;
+        }
+      }
+    }
+    return mixins;
+  }
+
   JS.Expression _emitClassHeritage(ClassElement element) {
     var type = element.type;
     if (type.isObject) return null;
@@ -1281,7 +1306,7 @@ class CodeGenerator extends GeneralizingAstVisitor
     _loader.startTopLevel(element);
 
     // List of "direct" supertypes (supertype + mixins)
-    var basetypes = [type.superclass]..addAll(type.mixins);
+    var basetypes = [type.superclass]..addAll(_flattenMixins(type.mixins));
 
     // If any of these are recursive (via type parameter), defer setting
     // the real superclass.
@@ -1721,7 +1746,7 @@ class CodeGenerator extends GeneralizingAstVisitor
       var newBaseClass = _emitType(classElem.type.superclass,
           nameType: false, subClass: classElem, className: className);
       if (classElem.type.mixins.isNotEmpty) {
-        var mixins = classElem.type.mixins
+        var mixins = _flattenMixins(classElem.type.mixins)
             .map((t) => _emitType(t, nameType: false))
             .toList();
         mixins.insert(0, newBaseClass);