Make instance methods whose names collide with intercepted methods have the interceptor calling con…

sdk/lib/_internal/compiler/implementation/js_backend/emitter.dart

Index: sdk/lib/_internal/compiler/implementation/js_backend/emitter.dart
===================================================================
--- sdk/lib/_internal/compiler/implementation/js_backend/emitter.dart	(revision 19046)
+++ sdk/lib/_internal/compiler/implementation/js_backend/emitter.dart	(working copy)
@@ -637,12 +637,11 @@
     if (alreadyGenerated.contains(invocationName)) return;
     alreadyGenerated.add(invocationName);
 
-    bool isInterceptorClass =
-        backend.isInterceptorClass(member.getEnclosingClass());
+    bool isInterceptedMethod = backend.isInterceptedMethod(member);
 
-    // If the method is in an interceptor class, we need to also pass
+    // If the method is intercepted, we need to also pass
     // the actual receiver.
-    int extraArgumentCount = isInterceptorClass ? 1 : 0;
+    int extraArgumentCount = isInterceptedMethod ? 1 : 0;
     // Use '$receiver' to avoid clashes with other parameter names. Using
     // '$receiver' works because [:namer.safeName:] used for getting parameter
     // names never returns a name beginning with a single '$'.
@@ -658,7 +657,7 @@
             parameters.parameterCount + extraArgumentCount);
 
     int count = 0;
-    if (isInterceptorClass) {
+    if (isInterceptedMethod) {
       count++;
       parametersBuffer[0] = new jsAst.Parameter(receiverArgumentName);
       argumentsBuffer[0] = js[receiverArgumentName];
@@ -898,11 +897,6 @@
   void emitInstanceMembers(ClassElement classElement,
                            ClassBuilder builder) {
     assert(invariant(classElement, classElement.isDeclaration));
-    if (classElement == backend.objectInterceptorClass) {
-      emitInterceptorMethods(builder);
-      // The ObjectInterceptor does not have any instance methods.
-      return;
-    }
 
     void visitMember(ClassElement enclosing, Element member) {
       assert(invariant(classElement, member.isDeclaration));
@@ -977,13 +971,10 @@
       }
     }
 
-    if (backend.isInterceptorClass(classElement)) {
-      // The operator== method in [:Object:] does not take the same
-      // number of arguments as an intercepted method, therefore we
-      // explicitely add one to all interceptor classes. Note that we
-      // would not have do do that if all intercepted methods had
-      // a calling convention where the receiver is the first
-      // parameter.
+    if (backend.isInterceptorClass(classElement)
+        && classElement != compiler.objectClass) {
+      // We optimize the operator== on interceptor classes to
+      // just do a JavaScript double or triple equals.
       String name = backend.namer.publicInstanceMethodNameByArity(
           const SourceString('=='), 1);
       Function kind = (classElement == backend.jsNullClass)
@@ -1108,10 +1099,15 @@
             ? member.fixedBackendName()
             : (isMixinNativeField ? member.name.slowToString() : accessorName);
         bool needsCheckedSetter = false;
-        if (needsSetter && compiler.enableTypeAssertions
-            && canGenerateCheckedSetter(member)) {
-          needsCheckedSetter = true;
-          needsSetter = false;
+        if (needsSetter) {
+          if (compiler.enableTypeAssertions
+              && canGenerateCheckedSetter(member)) {
+            needsCheckedSetter = true;
+            needsSetter = false;
+          } else if (backend.isInterceptedMethod(member)) {
+            // The [addField] will take care of generating the setter.
+            needsSetter = false;
+          }
         }
         // Getters and setters with suffixes will be generated dynamically.

[sra1, 2013/02/27 05:13:28]

How does the dynamic generator know whether to add a receiver.
e.g.  many Dart classes have a 'length' field.
Calling it via x.get$length(x) when it takes zero arguments will cause and arity
mismatch.

[ngeoffray, 2013/02/28 10:39:42]

Yes, nice catch. The dynamic generator does not deal with that. We could make it
know it (but that requires to emit the set of intercepted names), or explicitly
generate the getter, or expect the argument mismatch will not have big
performance impact. I chose the last one.

         addField(member,
@@ -1157,8 +1153,11 @@
   void generateSetter(Element member, String fieldName, String accessorName,
                       ClassBuilder builder) {
     String setterName = namer.setterNameFromAccessorName(accessorName);
+    List<String> args = backend.isInterceptedMethod(member)
+        ? ['receiver', 'v']
+        : ['v'];
     builder.addProperty(setterName,
-        js.fun(['v'], js['this'][fieldName].assign('v')));
+        js.fun(args, js['this'][fieldName].assign('v')));

[sra1, 2013/02/27 05:13:28]

Don't you need to choose between 
   this.field = v
and
   receiver.field = v
?

If setter is added to an interceptor class, you need receiver.field.

Ditto for generateGetter.

[ngeoffray, 2013/02/28 10:39:42]

Good catch again. We never emit implicit getters and setters for interceptor
classes (they don't have fields). I added an assertion.

   }
 
   bool canGenerateCheckedSetter(Element member) {
@@ -1189,8 +1188,11 @@
     }
 
     String setterName = namer.setterNameFromAccessorName(accessorName);
+    List<String> args = backend.isInterceptedMethod(member)
+        ? ['receiver', 'v']
+        : ['v'];
     builder.addProperty(setterName,
-        js.fun(['v'], js['this'][fieldName].assign(js[helperName](arguments))));
+        js.fun(args, js['this'][fieldName].assign(js[helperName](arguments))));
   }
 
   void emitClassConstructor(ClassElement classElement, ClassBuilder builder) {
@@ -1282,6 +1284,9 @@
           if (needsSetter) {
             generateSetter(member, name, accessorName, builder);
           }
+        } else if (backend.isInterceptedMethod(member)
+                   && instanceFieldNeedsSetter(member)) {
+          generateSetter(member, name, accessorName, builder);
         }
       });
     });
@@ -1344,38 +1349,6 @@
     return _selectorRank(selector1) - _selectorRank(selector2);
   }
 
-  void emitInterceptorMethods(ClassBuilder builder) {
-    // Emit forwarders for the ObjectInterceptor class. We need to
-    // emit all possible sends on intercepted methods. Because of
-    // typed selectors we have to avoid generating the same forwarder
-    // multiple times.
-    Set<String> alreadyGenerated = new Set<String>();
-    for (Selector selector in
-         backend.usedInterceptors.toList()..sort(_compareSelectorNames)) {
-      String name = backend.namer.invocationName(selector);
-      if (alreadyGenerated.contains(name)) continue;
-      alreadyGenerated.add(name);
-
-      List<jsAst.Parameter> parameters = <jsAst.Parameter>[];
-      List<jsAst.Expression> arguments = <jsAst.Expression>[];
-      parameters.add(new jsAst.Parameter('receiver'));
-
-      if (selector.isSetter()) {
-        parameters.add(new jsAst.Parameter('value'));
-        arguments.add(js['value']);
-      } else {
-        for (int i = 0; i < selector.argumentCount; i++) {
-          String argName = 'a$i';
-          parameters.add(new jsAst.Parameter(argName));
-          arguments.add(js[argName]);
-        }
-      }
-      jsAst.Fun function =
-          js.fun(parameters, js.return_(js['receiver'][name](arguments)));
-      builder.addProperty(name, function);
-    }
-  }
-
   Iterable<Element> getTypedefChecksOn(DartType type) {
     bool isSubtype(TypedefElement typedef) {
       FunctionType typedefType =
@@ -1537,8 +1510,9 @@
     }
 
     // Add unneeded interceptors to the [unneededClasses] set.
-    for (ClassElement interceptor in backend.interceptedClasses.keys) {
-      if (!needed.contains(interceptor)) {
+    for (ClassElement interceptor in backend.interceptedClasses) {
+      if (!needed.contains(interceptor)
+          && interceptor != compiler.objectClass) {
         unneededClasses.add(interceptor);
       }
     }
@@ -1721,9 +1695,9 @@
 
     Map<int, String> cache;
     String extraArg = null;
-    // Methods on interceptor classes take an extra parameter, which is the
-    // actual receiver of the call.
-    bool inInterceptor = backend.isInterceptorClass(member.getEnclosingClass());
+    // Intercepted methods take an extra parameter, which is the
+    // receiver of the call.
+    bool inInterceptor = backend.isInterceptedMethod(member);
     if (inInterceptor) {
       cache = interceptorClosureCache;
       extraArg = 'receiver';
@@ -1846,10 +1820,9 @@
                              DefineStubFunction defineStub) {
     assert(invariant(member, member.isDeclaration));
     LibraryElement memberLibrary = member.getLibrary();
-    // If the class is an interceptor class, the stub gets the
+    // If the method is intercepted, the stub gets the
     // receiver explicitely and we need to pass it to the getter call.
-    bool isInterceptorClass =
-        backend.isInterceptorClass(member.getEnclosingClass());
+    bool isInterceptedMethod = backend.isInterceptedMethod(member);
 
     const String receiverArgumentName = r'$receiver';
 
@@ -1857,7 +1830,7 @@
       if (member.isGetter()) {
         String getterName = namer.getterName(member);
         return js['this'][getterName](
-            isInterceptorClass
+            isInterceptedMethod
                 ? <jsAst.Expression>[js[receiverArgumentName]]
                 : <jsAst.Expression>[]);
       } else {
@@ -1885,7 +1858,7 @@
 
         List<jsAst.Parameter> parameters = <jsAst.Parameter>[];
         List<jsAst.Expression> arguments = <jsAst.Expression>[];
-        if (isInterceptorClass) {
+        if (isInterceptedMethod) {
           parameters.add(new jsAst.Parameter(receiverArgumentName));
         }
 
@@ -1909,6 +1882,8 @@
     Iterable<VariableElement> staticNonFinalFields =
         handler.getStaticNonFinalFieldsForEmission();
     for (Element element in Elements.sortedByPosition(staticNonFinalFields)) {
+      // [:interceptedNames:] is handled in [emitInterceptedNames].
+      if (element == backend.interceptedNames) continue;
       compiler.withCurrentElement(element, () {
         Constant initialValue = handler.getInitialValueFor(element);
         jsAst.Expression init =
@@ -2047,14 +2022,19 @@
       String createInvocationMirror = namer.getName(
           compiler.createInvocationMirrorElement);
 
+      assert(backend.isInterceptedName(Compiler.NO_SUCH_METHOD));
       jsAst.Expression expression = js['this.$noSuchMethodName'](
-          js[namer.CURRENT_ISOLATE][createInvocationMirror]([
-              js.string(methodName),
-              js.string(internalName),
-              type,
-              new jsAst.ArrayInitializer.from(
-                  parameters.map((param) => js[param.name]).toList()),
-              new jsAst.ArrayInitializer.from(argNames)]));
+          [js['this'],
+           js[namer.CURRENT_ISOLATE][createInvocationMirror]([
+               js.string(methodName),
+               js.string(internalName),
+               type,
+               new jsAst.ArrayInitializer.from(
+                   parameters.map((param) => js[param.name]).toList()),
+               new jsAst.ArrayInitializer.from(argNames)])]);
+      parameters = backend.isInterceptedName(selector.name)
+          ? ([new jsAst.Parameter('\$receiver')]..addAll(parameters))
+          : parameters;
       return js.fun(parameters, js.return_(expression));
     }
 
@@ -2211,7 +2191,6 @@
   }
 
   void emitGetInterceptorMethod(CodeBuffer buffer,
-                                String objectName,
                                 String key,
                                 Collection<ClassElement> classes) {
     jsAst.Statement buildReturnInterceptor(ClassElement cls) {
@@ -2263,10 +2242,11 @@
       else if (cls == backend.jsNullClass) hasNull = true;
       else if (cls == backend.jsNumberClass) hasNumber = true;
       else if (cls == backend.jsStringClass) hasString = true;
-      else throw 'Internal error: $cls';
+      else {
+        assert(cls == compiler.objectClass);
+      }
     }
     if (hasDouble) {
-      assert(!hasNumber);
       hasNumber = true;
     }
     if (hasInt) hasNumber = true;
@@ -2319,7 +2299,7 @@
     if (hasArray) {
       block.statements.add(buildInterceptorCheck(backend.jsArrayClass));
     }
-    block.statements.add(js.return_(js[objectName]['prototype']));
+    block.statements.add(js.return_(receiver));
 
     buffer.add(jsAst.prettyPrint(
         js[isolateProperties][key].assign(js.fun(['receiver'], block)),
@@ -2331,13 +2311,10 @@
    * Emit all versions of the [:getInterceptor:] method.
    */
   void emitGetInterceptorMethods(CodeBuffer buffer) {
-    // If no class needs to be intercepted, just return.
-    if (backend.objectInterceptorClass == null) return;
-    String objectName = namer.isolateAccess(backend.objectInterceptorClass);
     var specializedGetInterceptors = backend.specializedGetInterceptors;
     for (String name in specializedGetInterceptors.keys.toList()..sort()) {
       Collection<ClassElement> classes = specializedGetInterceptors[name];
-      emitGetInterceptorMethod(buffer, objectName, name, classes);
+      emitGetInterceptorMethod(buffer, name, classes);
     }
   }
 
@@ -2518,7 +2495,7 @@
     for (String name in names) {
       Selector selector = backend.oneShotInterceptors[name];
       Set<ClassElement> classes =
-          backend.getInterceptedClassesOn(selector);
+          backend.getInterceptedClassesOn(selector.name);
       String getInterceptorName =
           namer.getInterceptorName(backend.getInterceptorMethod, classes);
 
@@ -2560,6 +2537,32 @@
     }
   }
 
+  /**
+   * If [:invokeOn:] has been compiled, emit all the possible selector names
+   * that are intercepted into the [:interceptedNames:] top-level
+   * variable. The implementation of [:invokeOn:] will use it to
+   * determine whether it should call the method with an extra
+   * parameter.
+   */
+  void emitInterceptedNames(CodeBuffer buffer) {
+    if (!compiler.enabledInvokeOn) return;
+    String name = backend.namer.getName(backend.interceptedNames);
+    jsAst.PropertyAccess property = js[isolateProperties][name];
+
+    int index = 0;
+    List<jsAst.ArrayElement> elements = backend.usedInterceptors.map(
+      (Selector selector) {
+        jsAst.Literal str = js.string(namer.invocationName(selector));
+        return new jsAst.ArrayElement(index++, str);
+      }).toList();
+    jsAst.ArrayInitializer array = new jsAst.ArrayInitializer(
+        backend.usedInterceptors.length,
+        elements);
+
+    buffer.add(jsAst.prettyPrint(property.assign(array), compiler));
+    buffer.add(N);
+  }
+
   void emitInitFunction(CodeBuffer buffer) {
     jsAst.Fun fun = js.fun([], [
       js['$isolateProperties = {}'],
@@ -2603,6 +2606,7 @@
       // constants to be set up.
       emitStaticNonFinalFieldInitializations(mainBuffer);
       emitOneShotInterceptors(mainBuffer);
+      emitInterceptedNames(mainBuffer);
       emitGetInterceptorMethods(mainBuffer);
       emitLazilyInitializedStaticFields(mainBuffer);