Make selector registration in the resolver and code generator more explicit.

lib/compiler/implementation/resolver.dart

 // Copyright (c) 2012, the Dart project authors.  Please see the AUTHORS file
 // for details. All rights reserved. Use of this source code is governed by a
 // BSD-style license that can be found in the LICENSE file.
 
 interface TreeElements {
   Element operator[](Node node);
   Selector getSelector(Send send);
   Type getType(TypeAnnotation annotation);
 }
 
@@ skipping 431 matching lines @@
         error(selector, MessageKind.CANNOT_RESOLVE, [name]);
       } else if (target.kind != ElementKind.FIELD) {
         error(selector, MessageKind.NOT_A_FIELD, [name]);
       } else if (!target.isInstanceMember()) {
         error(selector, MessageKind.INIT_STATIC_FIELD, [name]);
       }
     } else {
       error(init, MessageKind.INVALID_RECEIVER_IN_INITIALIZER);
     }
     visitor.useElement(init, target);
+    visitor.world.registerStaticUse(target);
     // Check for duplicate initializers.
     if (initialized.containsKey(name)) {
       error(init, MessageKind.DUPLICATE_INITIALIZER, [name]);
       warning(initialized[name], MessageKind.ALREADY_INITIALIZED, [name]);
     }
     initialized[name] = init;
     // Resolve initializing value.
     visitor.visitInStaticContext(init.arguments.head);
   }
 
   Element resolveSuperOrThisForSend(FunctionElement constructor,
                                     FunctionExpression functionNode,
                                     Send call) {
     // Resolve the selector and the arguments.
     ResolverTask resolver = visitor.compiler.resolver;
     visitor.inStaticContext(() {
       visitor.resolveSelector(call);
-      visitor.visitArguments(call.argumentsNode);
+      visitor.resolveArguments(call.argumentsNode);
     });
     Selector selector = visitor.mapping.getSelector(call);
     bool isSuperCall = Initializers.isSuperConstructorCall(call);
     SourceString constructorName = resolver.getConstructorName(call);
     Element result = resolveSuperOrThis(
         constructor, isSuperCall, false, constructorName, selector, call);
     visitor.useElement(call, result);
+    visitor.world.registerStaticUse(result);
     return result;
   }
 
   void resolveImplicitSuperConstructorSend(FunctionElement constructor,
                                            FunctionExpression functionNode) {
     // If the class has a super resolve the implicit super call.
     ClassElement classElement = constructor.getEnclosingClass();
     ClassElement superClass = classElement.superclass;
     if (classElement != visitor.compiler.objectClass) {
       assert(superClass !== null);
@@ skipping 468 matching lines @@
       Element existing = scope.add(element);
       if (existing != element) {
         error(node, MessageKind.DUPLICATE_DEFINITION, [node]);
       }
     }
     return element;
   }
 
   Element useElement(Node node, Element element) {
     if (element === null) return null;
-    mapping[node] = element;
-    if (element.isTopLevel() || element.isMember()) {
-      if (element.isInstanceMember()) {
-        var send = node.asSend();
-        if (send !== null) {
-          Selector selector = mapping.getSelector(send);
-          if (selector !== null) {
-            world.registerDynamicInvocation(element.name, selector);
-          }
-        }
-      } else {
-        if (!element.isPrefix() &&
-            !element.isClass() &&
-            !element.isTypedef() &&
-            !element.isTypeVariable()) {
-          world.registerStaticUse(element);
-        }
-      }
-    }
-    return element;
+    return mapping[node] = element;

[floitsch, 2012/08/16 11:49:53]

I would prefer the assignment in a separate line.

   }
 
   Type useType(TypeAnnotation annotation, Type type) {
     if (type !== null) {
       mapping.setType(annotation, type);
       useElement(annotation, type.element);
     }
     return type;
   }
 
@@ skipping 183 matching lines @@
     if (node === null) {
       // node is of the form !Type.
       node = argument.asSend().receiver.asTypeAnnotation();
       if (node === null) compiler.cancel("malformed send");
     }
     return resolveTypeRequired(node);
   }
 
   static Selector computeSendSelector(Send node, LibraryElement library) {
     // First determine if this is part of an assignment.
-    bool isSet = node is SendSet;
+    bool isSet = node.asSendSet() !== null;
 
     if (node.isIndex) {
       return isSet ? new Selector.indexSet() : new Selector.index();
     }
 
     if (node.isOperator) {
       SourceString source = node.selector.asOperator().source;
-      switch (source.stringValue) {
-        case '!'  : case '&&' : case '||':
-        case 'is' : case 'as' :
-        case '===': case '!==':
-        case '>>>':
-          return null;
+      String string = source.stringValue;
+      if (string === '!'   || string === '&&'  || string == '||' ||
+          string === 'is'  || string === 'as'  ||
+          string === '===' || string === '!==' ||
+          string === '>>>') {
+        return null;
       }
       return node.arguments.isEmpty()
           ? new Selector.unaryOperator(source)
           : new Selector.binaryOperator(source);
     }
 
     Identifier identifier = node.selector.asIdentifier();
     if (node.isPropertyAccess) {
       assert(!isSet);
       return new Selector.getter(identifier.source, library);
     } else if (isSet) {
       return new Selector.setter(identifier.source, library);
     }
 
     // Compute the arity and the list of named arguments.
     int arity = 0;
     List<SourceString> named = <SourceString>[];
     for (Link<Node> link = node.argumentsNode.nodes;
         !link.isEmpty();
         link = link.tail) {
       Expression argument = link.head;
-      if (argument.asNamedArgument() != null) {
-        named.add((argument as NamedArgument).name.source);
+      NamedArgument namedArgument = argument.asNamedArgument();
+      if (namedArgument !== null) {
+        named.add(namedArgument.name.source);
       }
       arity++;
     }
 
-    // If we're invoking a closure, we do not have an identifier. In
-    // that case, we create a wildcard selector that can call any
-    // method that may have been closurized.
-    return (identifier == null)
-        ? new Selector.callAny(arity, named)
+    // If we're invoking a closure, we do not have an identifier.
+    return (identifier === null)
+        ? new Selector.callClosure(arity, named)
         : new Selector.call(identifier.source, library, arity, named);
   }
 
   Selector resolveSelector(Send node) {
     LibraryElement library = enclosingElement.getLibrary();
     Selector selector = computeSendSelector(node, library);
     if (selector != null) mapping.setSelector(node, selector);
     return selector;
   }
 
-  void visitArguments(NodeList list) {
+  void resolveArguments(NodeList list) {
+    if (list === null) return;
     bool seenNamedArgument = false;
     for (Link<Node> link = list.nodes; !link.isEmpty(); link = link.tail) {
       Expression argument = link.head;
       visit(argument);
       if (argument.asNamedArgument() != null) {
         seenNamedArgument = true;
       } else if (seenNamedArgument) {
         error(argument, MessageKind.INVALID_ARGUMENT_AFTER_NAMED);
       }
     }
   }
 
   visitSend(Send node) {
     Element target = resolveSend(node);
-    if (node.isOperator) {
-      Operator op = node.selector.asOperator();
-      if (op.source.stringValue === 'is' || op.source.stringValue === 'as') {
-        resolveTypeTest(node.arguments.head);
-        assert(node.arguments.tail.isEmpty());
-      } else if (node.arguments.isEmpty()) {
-        assert(op.token.kind !== PLUS_TOKEN);
-      } else {
-        visit(node.argumentsNode);
-      }
-    } else if (node.isIndex) {
-      visit(node.argumentsNode);
-      assert(node.arguments.tail.isEmpty());
-    } else if (node.isPropertyAccess) {
-      // Nothing to do here.
-    } else {
-      visitArguments(node.argumentsNode);
-    }
-
     if (target != null && target.kind == ElementKind.ABSTRACT_FIELD) {
       AbstractFieldElement field = target;
       target = field.getter;
     }
+
+    bool resolvedArguments = false;
+    if (node.isOperator) {

[ahe, 2012/08/16 12:09:54]

You could write something like this:

if (node.isOperatorWithTypeExpression) {
  resolveTypeTest(node.arguments.head);
} else {
  resolveArguments(node.argumentsNode);
}

You could also change resolveTypeTest to resolveTypeExpression and have it take
node.argumentsNode.

+      String operatorString = node.selector.asOperator().source.stringValue;
+      if (operatorString === 'is' || operatorString === 'as') {
+        assert(node.arguments.tail.isEmpty());
+        resolveTypeTest(node.arguments.head);
+        resolvedArguments = true;
+      }
+    }
+
+    if (!resolvedArguments) {
+      resolveArguments(node.argumentsNode);
+    }
+
+    // If the selector is null, it means that we will not be generating
+    // code for this as a send.
+    Selector selector = mapping.getSelector(node);
+    if (selector === null) return;
+
+    // If we don't know what we're calling or if we are calling a getter,
+    // we need to register that fact that we may be calling a closure
+    // with the same arguments.
     if (node.isCall &&
         (target === null ||
          target.isGetter() ||
          Elements.isClosureSend(node, target))) {
-      Selector selector = mapping.getSelector(node);
-      Selector call = new Selector.call(
-          compiler.namer.CLOSURE_INVOCATION_NAME,
-          selector.library,
-          selector.argumentCount,
-          selector.namedArguments);
-      world.registerDynamicInvocation(compiler.namer.CLOSURE_INVOCATION_NAME,
-                                      call);
+      Selector call = new Selector.callClosureFrom(selector);
+      world.registerDynamicInvocation(call.name, call);
     }
+
     // TODO(ngeoffray): We should do the check in
     // visitExpressionStatement instead.
     if (target === compiler.assertMethod && !node.isCall) {
       // We can only use assert by calling it.
       if (!inInstanceContext) {
         error(node, MessageKind.MISSING_ARGUMENTS_TO_ASSERT, [node]);
       }
       target = null;
     }
+
     // TODO(ngeoffray): Warn if target is null and the send is
     // unqualified.
     useElement(node, target);
-    if (target === null) registerDynamicSend(node);
-    if (node.isPropertyAccess) return target;
+    registerSend(selector, target);
+    return node.isPropertyAccess ? target : null;
   }
 
   visitSendSet(SendSet node) {
     Element target = resolveSend(node);
-    Element setter = null;
-    Element getter = null;
+    Element setter = target;
+    Element getter = target;
     if (target != null && target.kind == ElementKind.ABSTRACT_FIELD) {
       AbstractFieldElement field = target;
       setter = field.setter;
       getter = field.getter;
-    } else {
-      setter = target;
-      getter = target;
     }
+
+    visit(node.argumentsNode);
+
     // TODO(ngeoffray): Check if the target can be assigned.
-    Identifier identifier = node.assignmentOperator;
-    bool compoundAssignment = identifier.source.stringValue !== '=';
-    if (compoundAssignment) {
-      useElement(node.selector, getter);
-    }
-    visit(node.argumentsNode);
     // TODO(ngeoffray): Warn if target is null and the send is
     // unqualified.
-    registerDynamicSend(node);
+
+    Selector selector = mapping.getSelector(node);
+    Identifier identifier = node.assignmentOperator;
+    bool isCompound = identifier.source.stringValue !== '=';

[ahe, 2012/08/16 12:09:54]

isCompound is not entirely correct as this also is true for ++ and --. Perhaps
the word "complex" is better?

+    if (isCompound) {
+      if (selector.isSetter()) {
+        useElement(node.selector, getter);
+        registerSend(new Selector.getterFrom(selector), getter);
+      } else {
+        // TODO(kasperl): If [getter] is resolved, it will actually
+        // refer to the []= operator which isn't the one we want to
+        // register here. We should consider using some notation of

[ahe, 2012/08/16 12:09:54]

Great idea.

+        // abstract indexable element that we can resolve to so we can
+        // distinguish the two.
+        assert(selector.isIndexSet());
+        registerSend(new Selector.index(), null);
+      }
+    }
+
+    registerSend(selector, setter);
     return useElement(node, setter);
   }
 
-  registerDynamicSend(Send node) {
-    Identifier id = node.selector.asIdentifier();
-    if (id === null) return;
-    SourceString name = node.selector.asIdentifier().source;
-    if (node.isIndex && node is SendSet) {
-      name = Elements.constructOperatorName(
-          const SourceString('operator'),
-          const SourceString('[]='));
-    } else if (node.selector.asOperator() != null) {
-      switch (name.stringValue) {
-        case '===':
-        case '!==':
-        case '!':
-        case '&&':
-        case '||':
-        case 'is':
-        case 'as':
-        case '>>>':
-          return null;
+  void registerSend(Selector selector, Element target) {
+    if (target === null || target.isInstanceMember()) {
+      if (selector.isGetter()) {
+        world.registerDynamicGetter(selector.name, selector);
+      } else if (selector.isSetter()) {
+        world.registerDynamicSetter(selector.name, selector);
+      } else {
+        world.registerDynamicInvocation(selector.name, selector);
       }
-      name = Elements.constructOperatorName(
-          const SourceString('operator'),
-          name,
-          node.argumentsNode is Prefix);
+    } else if (Elements.isStaticOrTopLevel(target)) {
+      // TODO(kasperl): It seems like we're not supposed to register
+      // the use of classes. Wouldn't it be simpler if we just did?

[ahe, 2012/08/16 12:09:54]

I'm not so sure about that. There is a difference between using a class for
instantiation, or using a class for accessing a static.

+      if (!target.isClass()) world.registerStaticUse(target);
     }
-    Selector selector = mapping.getSelector(node);
-    if (selector.isGetter()) {
-      world.registerDynamicGetter(name, selector);
-    } else if (selector.isSetter()) {
-      world.registerDynamicSetter(name, selector);
-      // Also register the getter for compound assignments.
-      LibraryElement library = enclosingElement.getLibrary();
-      Selector getter = new Selector.getter(name, library);
-      world.registerDynamicGetter(name, getter);
-    } else {
-      if (selector.isIndexSet()) {
-        // Also register the index selector for compound assignments.
-        Selector index = new Selector.index();
-        world.registerDynamicInvocation(index.name, index);
-      }
-      world.registerDynamicInvocation(name, selector);
-    }
+
+    // TODO(kasperl): Pass the selector directly.
     var interceptor = new Interceptors(compiler).getStaticInterceptor(
-        name,
-        node.argumentCount());
+        selector.name,
+        selector.argumentCount);
     if (interceptor !== null) {
       world.registerStaticUse(interceptor);
     }
   }
 
   visitLiteralInt(LiteralInt node) {
   }
 
   visitLiteralDouble(LiteralDouble node) {
   }
@@ skipping 44 matching lines @@
 
   visitParenthesizedExpression(ParenthesizedExpression node) {
     visit(node.expression);
   }
 
   visitNewExpression(NewExpression node) {
     Node selector = node.send.selector;
 
     FunctionElement constructor = resolveConstructor(node);
     resolveSelector(node.send);
-    visitArguments(node.send.argumentsNode);
+    resolveArguments(node.send.argumentsNode);
     if (constructor === null) return null;
     // TODO(karlklose): handle optional arguments.
     if (node.send.argumentCount() != constructor.parameterCount(compiler)) {
       // TODO(ngeoffray): resolution error with wrong number of
       // parameters. We cannot do this rigth now because of the
       // List constructor.
     }
     useElement(node.send, constructor);
+    world.registerStaticUse(constructor);
     compiler.withCurrentElement(constructor, () {
       FunctionExpression tree = constructor.parseNode(compiler);
       compiler.resolver.resolveConstructorImplementation(constructor, tree);
     });
     world.registerStaticUse(constructor.defaultImplementation);
     ClassElement cls = constructor.defaultImplementation.getEnclosingClass();
     world.registerInstantiatedClass(cls);
     cls.forEachInstanceField(
         includeBackendMembers: false,
         includeSuperMembers: true,
@@ skipping 995 matching lines @@
   TopScope(LibraryElement library) : super(null, library);
   Element lookup(SourceString name) {
     return library.find(name);
   }
 
   Element add(Element newElement) {
     throw "Cannot add an element in the top scope";
   }
   String toString() => '$element';
 }