[cfe] Use MixinSuperStub

This CL adds stub for each concrete mixed in member into mixin
applications. The body of the stub calls the original member via
a super call.

This addition means that resolution of super access now use the
stub and not the original declaration as the target, which means
that it will be correct even when mixed in members are cloned. For
this reason, dart2js no longer needs to perform its own super
member resolution.

When a super call targets a mixin super stub (after cloning) it
can be optimized away by redirecting the call to the `stubTarget`
of the call. This optimization is performed in dart2js.

Since dart2js now uses the correct super target, its runtime
mixin application needs to avoid overriding members already
declared in the mixin application. These members are the
forwarding super stubs which ensure that correct runtime types
when members with covariant parameters are implemented.


Change-Id: Iab71ffcc400aa6a683987bc20b9553a263ebc8e1
Reviewed-on: https://dart-review.googlesource.com/c/sdk/+/176526
Commit-Queue: Johnni Winther <[email protected]>
Reviewed-by: Sigmund Cherem <[email protected]>
Reviewed-by: Dmitry Stefantsov <[email protected]>

pkg/compiler/lib/src/common_elements.dart

@@ -2405,9 +2405,11 @@ abstract class JElementEnvironment extends ElementEnvironment {
   void forEachNestedClosure(
       MemberEntity member, void f(FunctionEntity closure));
 
-  /// Returns `true` if [cls] is a mixin application that mixes in methods with
-  /// super calls.
-  bool isSuperMixinApplication(ClassEntity cls);
+  /// Returns `true` if [cls] is a mixin application with its own members.
+  ///
+  /// This occurs when a mixin contains methods with super calls or when
+  /// the mixin application contains forwarding super stubs.
+  bool isMixinApplicationWithMembers(ClassEntity cls);
 
   /// The default type of the [typeVariable].
   ///

pkg/compiler/lib/src/inferrer/builder_kernel.dart

@@ -1906,10 +1906,15 @@ class KernelTypeGraphBuilder extends ir.Visitor<TypeInformation> {
     // are calling does not expose this.
     _state.markThisAsExposed();
 
-    MemberEntity member =
-        _elementMap.getSuperMember(_analyzedMember, node.name);
-    assert(member != null, "No member found for super property get: $node");
+    ir.Member target = getEffectiveSuperTarget(node.interfaceTarget);
     Selector selector = new Selector.getter(_elementMap.getName(node.name));
+    if (target == null) {
+      // TODO(johnniwinther): Remove this when the CFE checks for missing
+      //  concrete super targets.
+      return handleSuperNoSuchMethod(node, selector, null);
+    }
+    MemberEntity member = _elementMap.getMember(target);
+    assert(member != null, "No member found for super property get: $node");
     TypeInformation type = handleStaticInvoke(node, selector, member, null);
     if (member.isGetter) {
       FunctionType functionType =
@@ -1937,11 +1942,16 @@ class KernelTypeGraphBuilder extends ir.Visitor<TypeInformation> {
     _state.markThisAsExposed();
 
     TypeInformation rhsType = visit(node.value);
-    MemberEntity member =
-        _elementMap.getSuperMember(_analyzedMember, node.name, setter: true);
-    assert(member != null, "No member found for super property set: $node");
+    ir.Member target = getEffectiveSuperTarget(node.interfaceTarget);
     Selector selector = new Selector.setter(_elementMap.getName(node.name));
     ArgumentsTypes arguments = new ArgumentsTypes([rhsType], null);
+    if (target == null) {
+      // TODO(johnniwinther): Remove this when the CFE checks for missing
+      //  concrete super targets.
+      return handleSuperNoSuchMethod(node, selector, arguments);
+    }
+    MemberEntity member = _elementMap.getMember(target);
+    assert(member != null, "No member found for super property set: $node");
     handleStaticInvoke(node, selector, member, arguments);
     return rhsType;
   }
@@ -1952,29 +1962,29 @@ class KernelTypeGraphBuilder extends ir.Visitor<TypeInformation> {
     // are calling does not expose this.
     _state.markThisAsExposed();
 
-    MemberEntity member =
-        _elementMap.getSuperMember(_analyzedMember, node.name);
+    ir.Member target = getEffectiveSuperTarget(node.interfaceTarget);
     ArgumentsTypes arguments = analyzeArguments(node.arguments);
     Selector selector = _elementMap.getSelector(node);
-    if (member == null) {
-      // TODO(johnniwinther): This shouldn't be necessary.
+    if (target == null) {
+      // TODO(johnniwinther): Remove this when the CFE checks for missing
+      //  concrete super targets.
+      return handleSuperNoSuchMethod(node, selector, arguments);
+    }
+    MemberEntity member = _elementMap.getMember(target);
+    assert(member.isFunction, "Unexpected super invocation target: $member");
+    if (isIncompatibleInvoke(member, arguments)) {
       return handleSuperNoSuchMethod(node, selector, arguments);
     } else {
-      assert(member.isFunction, "Unexpected super invocation target: $member");
-      if (isIncompatibleInvoke(member, arguments)) {
-        return handleSuperNoSuchMethod(node, selector, arguments);
-      } else {
-        TypeInformation type =
-            handleStaticInvoke(node, selector, member, arguments);
-        FunctionType functionType =
-            _elementMap.elementEnvironment.getFunctionType(member);
-        if (functionType.returnType.containsFreeTypeVariables) {
-          // The return type varies with the call site so we narrow the static
-          // return type.
-          type = _types.narrowType(type, _getStaticType(node));
-        }
-        return type;
+      TypeInformation type =
+          handleStaticInvoke(node, selector, member, arguments);
+      FunctionType functionType =
+          _elementMap.elementEnvironment.getFunctionType(member);
+      if (functionType.returnType.containsFreeTypeVariables) {
+        // The return type varies with the call site so we narrow the static
+        // return type.
+        type = _types.narrowType(type, _getStaticType(node));
       }
+      return type;
     }
   }
 

pkg/compiler/lib/src/ir/impact.dart

@@ -167,12 +167,12 @@ abstract class ImpactRegistry {
   void registerInstanceSet(
       ir.DartType receiverType, ClassRelation relation, ir.Member target);
 
-  void registerSuperInvocation(ir.Name name, int positionalArguments,
+  void registerSuperInvocation(ir.Member target, int positionalArguments,
       List<String> namedArguments, List<ir.DartType> typeArguments);
 
-  void registerSuperGet(ir.Name name);
+  void registerSuperGet(ir.Member target);
 
-  void registerSuperSet(ir.Name name);
+  void registerSuperSet(ir.Member target);
 
   void registerSuperInitializer(
       ir.Constructor source,
@@ -590,19 +590,22 @@ abstract class ImpactBuilderBase extends StaticTypeVisitor
   @override
   void handleSuperMethodInvocation(ir.SuperMethodInvocation node,
       ArgumentTypes argumentTypes, ir.DartType returnType) {
-    registerSuperInvocation(node.name, node.arguments.positional.length,
-        _getNamedArguments(node.arguments), node.arguments.types);
+    registerSuperInvocation(
+        getEffectiveSuperTarget(node.interfaceTarget),
+        node.arguments.positional.length,
+        _getNamedArguments(node.arguments),
+        node.arguments.types);
   }
 
   @override
   void handleSuperPropertyGet(
       ir.SuperPropertyGet node, ir.DartType resultType) {
-    registerSuperGet(node.name);
+    registerSuperGet(getEffectiveSuperTarget(node.interfaceTarget));
   }
 
   @override
   void handleSuperPropertySet(ir.SuperPropertySet node, ir.DartType valueType) {
-    registerSuperSet(node.name);
+    registerSuperSet(getEffectiveSuperTarget(node.interfaceTarget));
   }
 
   @override

pkg/compiler/lib/src/ir/impact_data.dart

@@ -43,23 +43,23 @@ abstract class ImpactRegistryMixin implements ImpactRegistry {
   }
 
   @override
-  void registerSuperSet(ir.Name name) {
+  void registerSuperSet(ir.Member target) {
     _data._superSets ??= [];
-    _data._superSets.add(name);
+    _data._superSets.add(target);
   }
 
   @override
-  void registerSuperGet(ir.Name name) {
+  void registerSuperGet(ir.Member target) {
     _data._superGets ??= [];
-    _data._superGets.add(name);
+    _data._superGets.add(target);
   }
 
   @override
-  void registerSuperInvocation(ir.Name name, int positionalArguments,
+  void registerSuperInvocation(ir.Member target, int positionalArguments,
       List<String> namedArguments, List<ir.DartType> typeArguments) {
     _data._superInvocations ??= [];
     _data._superInvocations.add(new _SuperInvocation(
-        name,
+        target,
         new _CallStructure(
             positionalArguments, namedArguments, typeArguments)));
   }
@@ -478,8 +478,8 @@ class ImpactDataImpl implements ImpactData {
   static const String tag = 'ImpactData';
 
   List<_SuperInitializer> _superInitializers;
-  List<ir.Name> _superSets;
-  List<ir.Name> _superGets;
+  List<ir.Member> _superSets;
+  List<ir.Member> _superGets;
   List<_SuperInvocation> _superInvocations;
   List<_InstanceAccess> _instanceSets;
   List<_DynamicAccess> _dynamicSets;
@@ -529,8 +529,10 @@ class ImpactDataImpl implements ImpactData {
     _superInitializers = source.readList(
         () => new _SuperInitializer.fromDataSource(source),
         emptyAsNull: true);
-    _superSets = source.readList(() => source.readName(), emptyAsNull: true);
-    _superGets = source.readList(() => source.readName(), emptyAsNull: true);
+    _superSets =
+        source.readList(() => source.readMemberNode(), emptyAsNull: true);
+    _superGets =
+        source.readList(() => source.readMemberNode(), emptyAsNull: true);
     _superInvocations = source.readList(
         () => new _SuperInvocation.fromDataSource(source),
         emptyAsNull: true);
@@ -632,8 +634,8 @@ class ImpactDataImpl implements ImpactData {
     sink.writeList(
         _superInitializers, (_SuperInitializer o) => o.toDataSink(sink),
         allowNull: true);
-    sink.writeList(_superSets, sink.writeName, allowNull: true);
-    sink.writeList(_superGets, sink.writeName, allowNull: true);
+    sink.writeList(_superSets, sink.writeMemberNode, allowNull: true);
+    sink.writeList(_superGets, sink.writeMemberNode, allowNull: true);
     sink.writeList(
         _superInvocations, (_SuperInvocation o) => o.toDataSink(sink),
         allowNull: true);
@@ -722,19 +724,19 @@ class ImpactDataImpl implements ImpactData {
       }
     }
     if (_superSets != null) {
-      for (ir.Name data in _superSets) {
+      for (ir.Member data in _superSets) {
         registry.registerSuperSet(data);
       }
     }
     if (_superGets != null) {
-      for (ir.Name data in _superGets) {
+      for (ir.Member data in _superGets) {
         registry.registerSuperGet(data);
       }
     }
     if (_superInvocations != null) {
       for (_SuperInvocation data in _superInvocations) {
         registry.registerSuperInvocation(
-            data.name,
+            data.target,
             data.callStructure.positionalArguments,
             data.callStructure.namedArguments,
             data.callStructure.typeArguments);
@@ -1113,22 +1115,22 @@ class _SuperInitializer {
 class _SuperInvocation {
   static const String tag = '_SuperInvocation';
 
-  final ir.Name name;
+  final ir.Member target;
   final _CallStructure callStructure;
 
-  _SuperInvocation(this.name, this.callStructure);
+  _SuperInvocation(this.target, this.callStructure);
 
   factory _SuperInvocation.fromDataSource(DataSource source) {
     source.begin(tag);
-    ir.Name name = source.readName();
+    ir.Member member = source.readMemberNode();
     _CallStructure callStructure = new _CallStructure.fromDataSource(source);
     source.end(tag);
-    return new _SuperInvocation(name, callStructure);
+    return new _SuperInvocation(member, callStructure);
   }
 
   void toDataSink(DataSink sink) {
     sink.begin(tag);
-    sink.writeName(name);
+    sink.writeMemberNode(target);
     callStructure.toDataSink(sink);
     sink.end(tag);
   }

pkg/compiler/lib/src/ir/util.dart

@@ -268,3 +268,20 @@ bool memberEntityIsInWebLibrary(MemberEntity entity) {
   if (importUri == null) return false;
   return _isWebLibrary(importUri);
 }
+
+/// Returns the effective target of a super access of [target].
+///
+/// If [target] is a mixin super stub then the stub target is returned instead
+/// of the mixin super stub. This is done to avoid unnecessary indirections
+/// in super accesses.
+///
+/// See [ir.ProcedureStubKind.MixinSuperStub] for why mixin super stubs are
+/// inserted in the first place.
+ir.Member getEffectiveSuperTarget(ir.Member target) {
+  if (target is ir.Procedure) {
+    if (target.stubKind == ir.ProcedureStubKind.MixinSuperStub) {
+      return getEffectiveSuperTarget(target.stubTarget);
+    }
+  }
+  return target;
+}

pkg/compiler/lib/src/js_emitter/model.dart

@@ -294,7 +294,7 @@ class Class implements FieldContainer {
   /// A soft-deferred class is only fully initialized at first instantiation.
   final bool isSoftDeferred;
 
-  final bool isSuperMixinApplication;
+  final bool isMixinApplicationWithMembers;
 
   // If the class implements a function type, and the type is encoded in the
   // metatada table, then this field contains the index into that field.
@@ -332,7 +332,7 @@ class Class implements FieldContainer {
       this.isNative,
       this.isClosureBaseClass,
       this.isSoftDeferred = false,
-      this.isSuperMixinApplication}) {
+      this.isMixinApplicationWithMembers}) {
     assert(onlyForRti != null);
     assert(onlyForConstructor != null);
     assert(isDirectlyInstantiated != null);
@@ -398,7 +398,7 @@ class MixinApplication extends Class {
             isDirectlyInstantiated: isDirectlyInstantiated,
             isNative: false,
             isClosureBaseClass: false,
-            isSuperMixinApplication: false);
+            isMixinApplicationWithMembers: false);
 
   @override
   bool get isSimpleMixinApplication => true;

pkg/compiler/lib/src/js_emitter/program_builder/program_builder.dart

@@ -221,7 +221,7 @@ class ProgramBuilder {
                 "No Class for has been created for superclass "
                 "${superclass} of $c."));
       }
-      if (c.isSimpleMixinApplication || c.isSuperMixinApplication) {
+      if (c.isSimpleMixinApplication || c.isMixinApplicationWithMembers) {
         ClassEntity effectiveMixinClass =
             _elementEnvironment.getEffectiveMixinClass(cls);
         c.setMixinClass(_classes[effectiveMixinClass]);
@@ -725,14 +725,14 @@ class ProgramBuilder {
 
     // MixinApplications run through the members of their mixin. Here, we are
     // only interested in direct members.
-    bool isSuperMixinApplication = false;
+    bool isMixinApplicationWithMembers = false;
     if (!onlyForConstructorOrRti) {
-      if (_elementEnvironment.isSuperMixinApplication(cls)) {
+      if (_elementEnvironment.isMixinApplicationWithMembers(cls)) {
         List<MemberEntity> members = <MemberEntity>[];
         void add(MemberEntity member) {
           if (member.enclosingClass == cls) {
             members.add(member);
-            isSuperMixinApplication = true;
+            isMixinApplicationWithMembers = true;
           }
         }
 
@@ -813,7 +813,7 @@ class ProgramBuilder {
     Class result;
     if (_elementEnvironment.isMixinApplication(cls) &&
         !onlyForConstructorOrRti &&
-        !isSuperMixinApplication) {
+        !isMixinApplicationWithMembers) {
       assert(!_nativeData.isNativeClass(cls));
       assert(methods.isEmpty);
       assert(!isClosureBaseClass);
@@ -854,7 +854,7 @@ class ProgramBuilder {
           isNative: _nativeData.isNativeClass(cls),
           isClosureBaseClass: isClosureBaseClass,
           isSoftDeferred: _isSoftDeferred(cls),
-          isSuperMixinApplication: isSuperMixinApplication);
+          isMixinApplicationWithMembers: isMixinApplicationWithMembers);
     }
     _classes[cls] = result;
     return result;

pkg/compiler/lib/src/js_emitter/startup_emitter/fragment_emitter.dart

@@ -54,6 +54,16 @@ function copyProperties(from, to) {
     to[key] = from[key];
   }
 }
+// Copies the own properties from [from] to [to] if not already present in [to].
+function mixinProperties(from, to) {
+  var keys = Object.keys(from);
+  for (var i = 0; i < keys.length; i++) {
+    var key = keys[i];
+    if (!to.hasOwnProperty(key)) {
+      to[key] = from[key];
+    }
+  }
+}
 
 // Only use direct proto access to construct the prototype chain (instead of
 // copying properties) on platforms where we know it works well (Chrome / d8).
@@ -111,7 +121,7 @@ function inheritMany(sup, classes) {
 
 // Mixes in the properties of [mixin] into [cls].
 function mixin(cls, mixin) {
-  copyProperties(mixin.prototype, cls.prototype);
+  mixinProperties(mixin.prototype, cls.prototype);
   cls.prototype.constructor = cls;
 }
 

pkg/compiler/lib/src/js_model/element_map.dart

@@ -74,11 +74,6 @@ abstract class JsToElementMap {
   /// Returns the [ClassEntity] corresponding to the class [node].
   ClassEntity getClass(ir.Class node);
 
-  /// Returns the super [MemberEntity] for a super invocation, get or set of
-  /// [name] from the member [context].
-  MemberEntity getSuperMember(MemberEntity context, ir.Name name,
-      {bool setter: false});
-
   /// Returns the `noSuchMethod` [FunctionEntity] call from a
   /// `super.noSuchMethod` invocation within [cls].
   FunctionEntity getSuperNoSuchMethod(ClassEntity cls);