Add a getter interceptor to the new interceptor scheme. Also cleanup the "this" that turns into a r…

sdk/lib/_internal/compiler/implementation/ssa/builder.dart

Index: sdk/lib/_internal/compiler/implementation/ssa/builder.dart
===================================================================
--- sdk/lib/_internal/compiler/implementation/ssa/builder.dart	(revision 14850)
+++ sdk/lib/_internal/compiler/implementation/ssa/builder.dart	(working copy)
@@ -50,7 +50,6 @@
     // interceptor classes.
     JavaScriptBackend backend = compiler.backend;
     if (backend.shouldInterceptSelector(selector)) {
-      backend.addInterceptedSelector(selector);
       return backend.getInterceptorMethod;
     }
 
@@ -409,9 +408,9 @@
     });
     if (closureData.isClosure()) {
       // Inside closure redirect references to itself to [:this:].
-      HInstruction thisInstruction = new HThis(closureData.thisElement);
-      builder.graph.entry.addAtEntry(thisInstruction);
-      updateLocal(closureData.closureElement, thisInstruction);
+      builder.thisInstruction = new HThis(closureData.thisElement);
+      builder.graph.entry.addAtEntry(builder.thisInstruction);
+      updateLocal(closureData.closureElement, builder.thisInstruction);
     } else if (element.isInstanceMember()
                || element.isGenerativeConstructor()) {
       // Once closures have been mapped to classes their instance members might
@@ -419,12 +418,20 @@
       // context.
       ClassElement cls = element.getEnclosingClass();
       DartType type = cls.computeType(builder.compiler);
-      HInstruction thisInstruction = new HThis(closureData.thisElement,
-                                               new HBoundedType.nonNull(type));
-      thisInstruction.sourceElement = closureData.thisElement;
-      builder.graph.entry.addAtEntry(thisInstruction);
-      directLocals[closureData.thisElement] = thisInstruction;
+      builder.thisInstruction = new HThis(closureData.thisElement,
+                                          new HBoundedType.nonNull(type));
+      builder.graph.entry.addAtEntry(builder.thisInstruction);
+      directLocals[closureData.thisElement] = builder.thisInstruction;
     }
+
+    if (builder.backend.isInterceptorClass(element.getEnclosingClass())) {
+      Element parameter = new Element(
+          const SourceString('receiver'), ElementKind.VARIABLE, element);
+      HParameterValue value = new HParameterValue(parameter);
+      builder.graph.entry.addAfter(
+          directLocals[closureData.thisElement], value);
+      directLocals[closureData.thisElement] = value;
+    }
   }
 
   bool hasValueForDirectLocal(Element element) {
@@ -892,6 +899,7 @@
   LocalsHandler localsHandler;
   HInstruction rethrowableException;
   Map<Element, HInstruction> parameters;
+  HInstruction thisInstruction;
   final RuntimeTypeInformation rti;
   HParameterValue lastAddedParameter;
 
@@ -2282,34 +2290,36 @@
     assert(selector.isGetter());
     SourceString getterName = selector.name;
     Element interceptor = null;
-    if (elements[send] == null && methodInterceptionEnabled) {
+    if (methodInterceptionEnabled) {
       interceptor = interceptors.getStaticInterceptor(selector);
     }
     bool hasGetter = compiler.world.hasAnyUserDefinedGetter(selector);
-    if (interceptor != null) {
-      if (interceptor == backend.getInterceptorMethod) {
-        // If we're using an interceptor class, emit a call to the
-        // interceptor method and then the actual dynamic call on the
-        // interceptor object.
+    if (interceptor == backend.getInterceptorMethod && interceptor != null) {

[floitsch, 2012/11/13 18:12:24]

remove "&& interceptor != null".

[ngeoffray, 2012/11/14 08:44:58]

No, I want to make sure we don't have to have getInterceptor (for eg unit
tests). Long term, I think I'll just change getStaticInterceptor into
"isIntercepted".

+      // If we're using an interceptor class, emit a call to the
+      // interceptor method and then the actual dynamic call on the
+      // interceptor object.
+      HInstruction instruction;
+      if (backend.isInterceptorClass(currentElement.getEnclosingClass())) {

[floitsch, 2012/11/13 18:12:24]

don't you need to verify that the receiver is of the same class as 'this' ?

[ngeoffray, 2012/11/14 08:44:58]

Yes, very good catch. I need to check that the receiver is the extra parameter.

+        instruction = thisInstruction;
+      } else {
         HStatic target = new HStatic(interceptor);
         add(target);
-        HInstruction instruction =
-            new HInvokeStatic(<HInstruction>[target, receiver]);
+        instruction = new HInvokeStatic(<HInstruction>[target, receiver]);
         add(instruction);
-        instruction = new HInvokeDynamicGetter(
-            selector, null, instruction, !hasGetter);
-        // Add the receiver as an argument to the getter call on the
-        // interceptor.
-        instruction.inputs.add(receiver);
-        pushWithPosition(instruction, send);
-      } else {
-        // Use the old, deprecated interceptor mechanism.
-        HStatic target = new HStatic(interceptor);
-        add(target);
-        List<HInstruction> inputs = <HInstruction>[target, receiver];
-        pushWithPosition(new HInvokeInterceptor(selector, inputs, !hasGetter),
-                         send);
       }
+      instruction = new HInvokeDynamicGetter(
+          selector, null, instruction, !hasGetter);
+      // Add the receiver as an argument to the getter call on the
+      // interceptor.
+      instruction.inputs.add(receiver);
+      pushWithPosition(instruction, send);
+    } else if (elements[send] == null && interceptor != null) {
+      // Use the old, deprecated interceptor mechanism.
+      HStatic target = new HStatic(interceptor);
+      add(target);
+      List<HInstruction> inputs = <HInstruction>[target, receiver];
+      pushWithPosition(new HInvokeInterceptor(selector, inputs, !hasGetter),
+                       send);
     } else {

[floitsch, 2012/11/13 18:12:24]

Eventually (once there is no old deprecated interceptor anymore), I would prefer
to inverse the order of the test:
if (interceptor == null) {
  ...
} else {
  assert(interceptor == backend.getInterceptorMethod);
  ...
}

       pushWithPosition(
           new HInvokeDynamicGetter(selector, null, receiver, !hasGetter), send);
@@ -2362,13 +2372,16 @@
     Selector selector = elements.getSelector(send);
     assert(selector.isSetter());
     SourceString setterName = selector.name;
-    Element staticInterceptor = null;
-    if (elements[send] == null && methodInterceptionEnabled) {
-      staticInterceptor = interceptors.getStaticInterceptor(selector);
+    Element interceptor = null;
+    if (methodInterceptionEnabled) {
+      interceptor = interceptors.getStaticInterceptor(selector);
     }
     bool hasSetter = compiler.world.hasAnyUserDefinedSetter(selector);
-    if (staticInterceptor != null) {
-      HStatic target = new HStatic(staticInterceptor);
+    if (interceptor != null && interceptor == backend.getInterceptorMethod) {

[floitsch, 2012/11/13 18:12:24]

ditto. remove "interceptor != null".

+      compiler.internalError(
+          'Unimplemented intercepted setter call with interceptor classes');
+    } else if (interceptor != null && elements[send] == null) {
+      HStatic target = new HStatic(interceptor);
       add(target);
       List<HInstruction> inputs = <HInstruction>[target, receiver, value];
       addWithPosition(new HInvokeInterceptor(selector, inputs), send);
@@ -2633,30 +2646,33 @@
     }
 
     Element interceptor = null;
-    if (methodInterceptionEnabled && elements[node] == null) {
+    if (methodInterceptionEnabled) {
       interceptor = interceptors.getStaticInterceptor(selector);
     }
+
     if (interceptor != null) {
-      // TODO(ngeoffray): The receiver never being null is currently the
-      // case, but should be updated once we start implementing more of
-      // the interceptors class.
-      assert(node.receiver != null);
       if (interceptor == backend.getInterceptorMethod) {
-        HStatic target = new HStatic(interceptor);
-        add(target);
-        visit(node.receiver);
-        HInstruction receiver = pop();
-        HInstruction instruction =
-            new HInvokeStatic(<HInstruction>[target, receiver]);
-        add(instruction);
-        inputs.add(instruction);
-        inputs.add(receiver);
+        if (backend.isInterceptorClass(currentElement.getEnclosingClass())) {
+          inputs.add(thisInstruction);

[floitsch, 2012/11/13 18:12:24]

ditto:
- don't you need to verify that the node's receiver is the same type as this.
Also visiting the receiver might have a side-effect.

[ngeoffray, 2012/11/14 08:44:58]

In this situation there is no receiver.

+          inputs.add(localsHandler.readThis());
+        } else {
+          HStatic target = new HStatic(interceptor);
+          add(target);
+          visit(node.receiver);
+          HInstruction receiver = pop();
+          HInstruction instruction =
+              new HInvokeStatic(<HInstruction>[target, receiver]);
+          add(instruction);
+          inputs.add(instruction);
+          inputs.add(receiver);
+        }
         addDynamicSendArgumentsToList(node, inputs);
         // The first entry in the inputs list is the interceptor. The
         // second is the receiver, and the others are the arguments.
-        instruction = new HInvokeDynamicMethod(selector, inputs);
+        HInstruction instruction = new HInvokeDynamicMethod(selector, inputs);
         pushWithPosition(instruction, node);
-      } else {
+        return;
+      } else if (elements[node] == null) {
         HStatic target = new HStatic(interceptor);
         add(target);
         inputs.add(target);
@@ -2664,8 +2680,8 @@
         inputs.add(pop());
         addGenericSendArgumentsToList(node.arguments, inputs);
         pushWithPosition(new HInvokeInterceptor(selector, inputs), node);
+        return;
       }
-      return;
     }
 
     Element element = elements[node];