dart2js cps: Use 'this' instead of receiver-arg when possible.

This fixes a bug in how intercepted super calls were handled,
and enables the redundant receiver optimization.

BUG=
R=kmillikin@google.com

Review URL: https://codereview.chromium.org//1312393002 .

pkg/compiler/lib/src/cps_ir/type_propagation.dart

@@ -74,6 +74,13 @@ class TypeMaskSystem {
                            classWorld);
   }
 
+  bool methodUsesReceiverArgument(FunctionElement function) {
+    assert(backend.isInterceptedMethod(function));
+    ClassElement clazz = function.enclosingClass.declaration;
+    return clazz.isSubclassOf(backend.jsInterceptorClass) || 
+           classWorld.isUsedAsMixin(clazz);
+  }
+
   Element locateSingleElement(TypeMask mask, Selector selector) {
     return mask.locateSingleElement(selector, mask, classWorld.compiler);
   }
@@ -88,7 +95,14 @@ class TypeMaskSystem {
 
   TypeMask getReceiverType(MethodElement method) {
     assert(method.isInstanceMember);
-    return nonNullSubclass(method.enclosingClass);
+    if (classWorld.isUsedAsMixin(method.enclosingClass.declaration)) {
+      // If used as a mixin, the receiver could be any of the classes that mix
+      // in the class, and these are not considered subclasses.
+      // TODO(asgerf): Exclude the subtypes that only `implement` the class.
+      return nonNullSubtype(method.enclosingClass);
+    } else {
+      return nonNullSubclass(method.enclosingClass);
+    }
   }
 
   TypeMask getParameterType(ParameterElement parameter) {
@@ -128,6 +142,11 @@ class TypeMaskSystem {
     return new TypeMask.nonNullSubclass(element.declaration, classWorld);
   }
 
+  TypeMask nonNullSubtype(ClassElement element) {
+    if (element.isClosure) return functionType;
+    return new TypeMask.nonNullSubtype(element.declaration, classWorld);
+  }
+
   bool isDefinitelyBool(TypeMask t, {bool allowNull: false}) {
     if (!allowNull && t.isNullable) return false;
     return t.nonNullable().containsOnlyBool(classWorld);
@@ -1581,6 +1600,30 @@ class TransformingVisitor extends LeafVisitor {
 
     AbstractValue receiver = getValue(node.receiver.definition);
     node.receiverIsNotNull = receiver.isDefinitelyNotNull;
+
+    if (isInterceptedSelector(node.selector) &&
+        node.receiver.definition == node.arguments[0].definition) {
+      // The receiver and first argument are the same; that means we already
+      // determined in visitInterceptor that we are targeting a non-interceptor.
+
+      // Check if any of the possible targets depend on the extra receiver
+      // argument. Mixins do this, and tear-offs always needs the extra receiver
+      // argument because BoundClosure uses it for equality and hash code.
+      bool needsReceiver(Element target) {
+        if (target is! FunctionElement) return false;
+        FunctionElement function = target;
+        return typeSystem.methodUsesReceiverArgument(function) ||
+               node.selector.isGetter && !function.isGetter;
+      }
+      if (!getAllTargets(receiver.type, node.selector).any(needsReceiver)) {
+        // Replace the extra receiver argument with a dummy value if the
+        // target definitely does not use it.
+        Constant dummy = makeConstantPrimitive(new IntConstantValue(0));
+        insertLetPrim(node, dummy);
+        node.arguments[0].unlink();
+        node.arguments[0] = new Reference<Primitive>(dummy);
+      }
+    }
   }
 
   void visitTypeCast(TypeCast node) {
@@ -2069,9 +2112,15 @@ class TypePropagationVisitor implements Visitor {
   void visitFunctionDefinition(FunctionDefinition node) {
     int firstActualParameter = 0;
     if (backend.isInterceptedMethod(node.element)) {
-      setValue(node.thisParameter, nonConstant(typeSystem.nonNullType));
-      setValue(node.parameters[0],
-               nonConstant(typeSystem.getReceiverType(node.element)));
+      if (typeSystem.methodUsesReceiverArgument(node.element)) {
+        setValue(node.thisParameter, nonConstant(typeSystem.nonNullType));
+        setValue(node.parameters[0],
+                 nonConstant(typeSystem.getReceiverType(node.element)));
+      } else {
+        setValue(node.thisParameter, 
+              nonConstant(typeSystem.getReceiverType(node.element)));
+        setValue(node.parameters[0], nonConstant());
+      }
       firstActualParameter = 1;
     } else if (node.thisParameter != null) {
       setValue(node.thisParameter,

pkg/compiler/lib/src/js_backend/codegen/glue.dart

@@ -54,6 +54,10 @@ class Glue {
     _compiler.internalError(_compiler.currentElement, message);
   }
 
+  bool isUsedAsMixin(ClassElement clazz) {
+    return classWorld.isUsedAsMixin(clazz);
+  }
+
   ConstantValue getConstantValueForVariable(VariableElement variable) {
     return _backend.constants.getConstantValueForVariable(variable);
   }
@@ -114,6 +118,10 @@ class Glue {
     return _backend.isInterceptedMethod(element);
   }
 
+  bool isInterceptorClass(ClassElement element) {
+    return element.isSubclassOf(_backend.jsInterceptorClass);
+  }
+
   Set<ClassElement> getInterceptedClassesOn(Selector selector) {
     return _backend.getInterceptedClassesOn(selector.name);
   }

pkg/compiler/lib/src/js_backend/codegen/unsugar.dart

@@ -52,7 +52,15 @@ class UnsugarVisitor extends RecursiveVisitor {
 
   UnsugarVisitor(this._glue);
 
+  bool methodUsesReceiverArgument(FunctionElement function) {
+    assert(_glue.isInterceptedMethod(function));
+    ClassElement clazz = function.enclosingClass.declaration;
+    return _glue.isInterceptorClass(clazz) || 
+           _glue.isUsedAsMixin(clazz);
+  }
+
   void rewrite(FunctionDefinition function) {
+    thisParameter = function.thisParameter;
     bool inInterceptedMethod = _glue.isInterceptedMethod(function.element);
 
     if (function.element.name == '==' &&
@@ -64,7 +72,6 @@ class UnsugarVisitor extends RecursiveVisitor {
     }
 
     if (inInterceptedMethod) {
-      thisParameter = function.thisParameter;
       ThisParameterLocal holder = thisParameter.hint;
       explicitReceiverParameter = new Parameter(
           new ExplicitReceiverParameterEntity(
@@ -75,7 +82,7 @@ class UnsugarVisitor extends RecursiveVisitor {
     // Set all parent pointers.
     _parentVisitor.visit(function);
 
-    if (inInterceptedMethod) {
+    if (inInterceptedMethod && methodUsesReceiverArgument(function.element)) {
       explicitReceiverParameter.substituteFor(thisParameter);
     }
 
@@ -235,8 +242,13 @@ class UnsugarVisitor extends RecursiveVisitor {
   /// The type propagation pass will later narrow the set of interceptors
   /// based on the input type, and the let sinking pass will propagate the
   /// getInterceptor call closer to its use when this is profitable.
-  Interceptor getInterceptorFor(Primitive prim, Selector selector,
-                                SourceInformation sourceInformation) {
+  Primitive getInterceptorFor(Primitive prim, Selector selector,
+                              SourceInformation sourceInformation) {
+    if (prim == explicitReceiverParameter) {
+      // If the receiver is the explicit receiver, we are calling a method in
+      // the same interceptor.
+      return thisParameter;
+    }
     assert(prim is! Interceptor);
     Interceptor interceptor = interceptors[prim];
     if (interceptor == null) {
@@ -255,37 +267,24 @@ class UnsugarVisitor extends RecursiveVisitor {
   }
 
   processInvokeMethod(InvokeMethod node) {
-    Selector selector = node.selector;
-    if (!_glue.isInterceptedSelector(selector)) return;
-
-    Primitive receiver = node.receiver.definition;
-    Primitive newReceiver;
-
-    if (receiver == explicitReceiverParameter) {
-      // If the receiver is the explicit receiver, we are calling a method in
-      // the same interceptor:
-      //  Change 'receiver.foo()'  to  'this.foo(receiver)'.
-      newReceiver = thisParameter;
-    } else {
-      newReceiver = getInterceptorFor(
-          receiver, node.selector, node.sourceInformation);
+    if (_glue.isInterceptedSelector(node.selector)) {
+      // Rewrite `x.foo()` => `INTERCEPTOR.foo(x, ..)`.
+      Primitive receiver = node.receiver.definition;
+      Primitive newReceiver = 
+          getInterceptorFor(receiver, node.selector, node.sourceInformation);
+      node.arguments.insert(0, node.receiver);
+      node.receiver = new Reference<Primitive>(newReceiver);
     }
-
-    node.arguments.insert(0, node.receiver);
-    node.receiver = new Reference<Primitive>(newReceiver);
   }
 
   processInvokeMethodDirectly(InvokeMethodDirectly node) {
     if (_glue.isInterceptedMethod(node.target)) {
-      Primitive nullPrim = nullConstant;
-      insertLetPrim(nullPrim, node);
+      // Rewrite `x.foo()` => `INTERCEPTOR.foo(x, ..)`.
+      Primitive receiver = node.receiver.definition;
+      Primitive newReceiver = 
+          getInterceptorFor(receiver, node.selector, node.sourceInformation);
       node.arguments.insert(0, node.receiver);
-      // TODO(sra): `null` is not adequate.  Interceptors project the class
-      // hierarchy onto an interceptor hierarchy.  A super call that does a
-      // method call will use the javascript 'this' parameter to avoid calling
-      // getInterceptor again, so the receiver must be the interceptor (likely
-      // `this`), not `null`.
-      node.receiver = new Reference<Primitive>(nullPrim);
+      node.receiver = new Reference<Primitive>(newReceiver);
     }
   }
 

pkg/pkg.status

@@ -194,4 +194,3 @@ analyzer/test/src/task/html_work_manager_test: Crash # (Iterable<JavaFile> ...
 analyzer/test/src/task/inputs_test: Crash # (Iterable<JavaFile> ...  cannot handle sync*/async* functions
 analyzer/test/src/task/manager_test: Crash # (Iterable<JavaFile> ...  cannot handle sync*/async* functions
 analyzer/test/src/task/model_test: Crash # (Iterable<JavaFile> ...  cannot handle sync*/async* functions
-typed_data/test/typed_buffers_test/none: RuntimeError # Please triage this failure.

tests/co19/co19-dart2js.status

@@ -9592,30 +9592,3 @@ LibTest/core/Symbol/Symbol_A01_t03: RuntimeError # Please triage this failure.
 LibTest/core/Symbol/Symbol_A01_t05: RuntimeError # Please triage this failure.
 LibTest/core/double/INFINITY_A01_t04: Pass # Please triage this failure.
 LibTest/core/double/NEGATIVE_INFINITY_A01_t04: Pass # Please triage this failure.
-LibTest/typed_data/Float32List/setAll_A01_t01: RuntimeError # this.get$length is not a function
-LibTest/typed_data/Float32List/setRange_A01_t01: RuntimeError # this.get$length is not a function
-LibTest/typed_data/Float32List/setRange_A02_t01: RuntimeError # this.get$length is not a function
-LibTest/typed_data/Float64List/setAll_A01_t01: RuntimeError # this.get$length is not a function
-LibTest/typed_data/Float64List/setRange_A01_t01: RuntimeError # this.get$length is not a function
-LibTest/typed_data/Float64List/setRange_A02_t01: RuntimeError # this.get$length is not a function
-LibTest/typed_data/Int16List/setAll_A01_t01: RuntimeError # this.get$length is not a function
-LibTest/typed_data/Int16List/setRange_A01_t01: RuntimeError # this.get$length is not a function
-LibTest/typed_data/Int16List/setRange_A02_t01: RuntimeError # this.get$length is not a function
-LibTest/typed_data/Int32List/setAll_A01_t01: RuntimeError # this.get$length is not a function
-LibTest/typed_data/Int32List/setRange_A01_t01: RuntimeError # this.get$length is not a function
-LibTest/typed_data/Int32List/setRange_A02_t01: RuntimeError # this.get$length is not a function
-LibTest/typed_data/Int8List/setAll_A01_t01: RuntimeError # this.get$length is not a function
-LibTest/typed_data/Int8List/setRange_A01_t01: RuntimeError # this.get$length is not a function
-LibTest/typed_data/Int8List/setRange_A02_t01: RuntimeError # this.get$length is not a function
-LibTest/typed_data/Uint16List/setAll_A01_t01: RuntimeError # this.get$length is not a function
-LibTest/typed_data/Uint16List/setRange_A01_t01: RuntimeError # this.get$length is not a function
-LibTest/typed_data/Uint16List/setRange_A02_t01: RuntimeError # this.get$length is not a function
-LibTest/typed_data/Uint32List/setAll_A01_t01: RuntimeError # this.get$length is not a function
-LibTest/typed_data/Uint32List/setRange_A01_t01: RuntimeError # this.get$length is not a function
-LibTest/typed_data/Uint32List/setRange_A02_t01: RuntimeError # this.get$length is not a function
-LibTest/typed_data/Uint8ClampedList/setAll_A01_t01: RuntimeError # this.get$length is not a function
-LibTest/typed_data/Uint8ClampedList/setRange_A01_t01: RuntimeError # this.get$length is not a function
-LibTest/typed_data/Uint8ClampedList/setRange_A02_t01: RuntimeError # this.get$length is not a function
-LibTest/typed_data/Uint8List/setAll_A01_t01: RuntimeError # this.get$length is not a function
-LibTest/typed_data/Uint8List/setRange_A01_t01: RuntimeError # this.get$length is not a function
-LibTest/typed_data/Uint8List/setRange_A02_t01: RuntimeError # this.get$length is not a function

tests/compiler/dart2js_native/dart2js_native.status

@@ -29,7 +29,5 @@ native_no_such_method_exception3_frog_test: RuntimeError # Please triage this fa
 optimization_hints_test: RuntimeError # Please triage this failure.
 subclassing_constructor_1_test: RuntimeError # Please triage this failure.
 subclassing_constructor_2_test: RuntimeError # Please triage this failure.
-subclassing_super_call_test: RuntimeError # this.get$afield is not a function
 subclassing_super_field_1_test: RuntimeError # Please triage this failure.
 subclassing_super_field_2_test: RuntimeError # Please triage this failure.
-super_call_test: RuntimeError # this.bar$0 is not a function

tests/corelib/corelib.status

@@ -226,16 +226,12 @@ iterable_return_type_test/none: RuntimeError # Please triage this failure.
 iterable_to_list_test: RuntimeError # Please triage this failure.
 iterable_to_set_test: RuntimeError # Please triage this failure.
 list_filled_type_argument_test: RuntimeError # Please triage this failure.
-list_test/01: RuntimeError # this.get$length is not a function
-list_test/none: RuntimeError # this.get$length is not a function
 list_unmodifiable_test: RuntimeError # Please triage this failure.
 map_values2_test: RuntimeError # Please triage this failure.
 map_values3_test: RuntimeError # Please triage this failure.
 map_values4_test: RuntimeError # Please triage this failure.
 package_resource_test: Crash # (await for(var byteSlice in resource.openRead()){streamBytes.addAll(byteSlice);}): await for
 regexp/pcre_test: Crash # Stack Overflow
-shuffle_test: RuntimeError # this.get$length is not a function
-string_fromcharcodes_test: RuntimeError # this.get$length is not a function
 symbol_operator_test/03: RuntimeError # Please triage this failure.
 symbol_reserved_word_test/03: Pass # Please triage this failure.
 symbol_reserved_word_test/06: RuntimeError # Please triage this failure.

tests/lib/lib.status

@@ -336,9 +336,7 @@ convert/streamed_conversion_json_utf8_decode_test: Skip  # Timeout.
 async/async_await_zones_test: Crash # (await for(var x in bar().take(100)){sum+= x;}): await for
 async/stream_empty_test: RuntimeError # $async$temp1.runTest_unreachable is not a function
 async/stream_iterator_test: Crash # (Stream createCancel...  cannot handle sync*/async* functions
-convert/ascii_test: RuntimeError # this.get$length is not a function
 convert/json_pretty_test: Crash # (static Iterable<Str...  cannot handle sync*/async* functions
-convert/latin1_test: RuntimeError # this.get$length is not a function
 convert/line_splitter_test: Crash # (static Iterable<Str...  cannot handle sync*/async* functions
 mirrors/abstract_class_test/00: Crash # (static Iterable<Str...  cannot handle sync*/async* functions
 mirrors/abstract_class_test/none: Crash # (static Iterable<Str...  cannot handle sync*/async* functions
@@ -564,4 +562,3 @@ mirrors/typedef_reflected_type_test/01: Crash # (static Iterable<Str...  cannot
 mirrors/typedef_reflected_type_test/none: Crash # (static Iterable<Str...  cannot handle sync*/async* functions
 mirrors/unnamed_library_test: Crash # (static Iterable<Str...  cannot handle sync*/async* functions
 mirrors/variable_is_const_test/none: Crash # (static Iterable<Str...  cannot handle sync*/async* functions
-typed_data/typed_data_list_test: RuntimeError # this.get$length is not a function