Made dart2js constructor lookup logic "private"-aware, fixed 4740 bug.

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.
 
 abstract class TreeElements {
   Element operator[](Node node);
   Selector getSelector(Send send);
   DartType getType(TypeAnnotation annotation);
   bool isParameterChecked(Element element);
 }
@@ skipping 87 matching lines @@
     // A synthetic constructor does not have a node.
     if (node === null) return null;
     if (node.initializers === null) return null;
     Link<Node> initializers = node.initializers.nodes;
     if (!initializers.isEmpty() &&
         Initializers.isConstructorRedirect(initializers.head)) {
       final ClassElement classElement = constructor.getEnclosingClass();
       final SourceString constructorName =
           getConstructorName(initializers.head);
       final SourceString className = classElement.name;
-      return classElement.lookupConstructor(className, constructorName);
+
+      return classElement.lookupConstructor(
+          new Selector.callConstructor(className,
+                                       constructorName,
+                                       constructor.getLibrary()));

[kasperl, 2012/09/24 05:52:38]

I'm a bit worried about getting the library from various different elements. It
is so easy to get the wrong library which will end up messing with our privacy
mechanism. Would it make sense to have a helper function in the resolver for
creating new call-constructor (and other) selectors that automatically get the
library from:

   resolver.enclosingElement.getLibrary()

WDYT?

[aam-me, 2012/09/25 04:15:18]

Having helper function in the resolver (ResolverVisitor I assume, right?) would
only work when you create selector from ResolverVisitor or when ResolveVisitor
is available. But about half of the constructors selectors are created without
ResolverVistor at hand - inside ResolverTask, for example.

     }
     return null;
   }
 
   void resolveRedirectingConstructor(InitializerResolver resolver,
                                      Node node,
                                      FunctionElement constructor,
                                      FunctionElement redirection) {
     Set<FunctionElement> seen = new Set<FunctionElement>();
     seen.add(constructor);
@@ skipping 76 matching lines @@
     // of the interface.
     SourceString name;
     if (defaultClass.implementsInterface(intrface)) {
       // TODO(ahe): Don't use string replacement here.
       name = new SourceString(constructor.name.slowToString().replaceFirst(
                  intrface.name.slowToString(),
                  defaultClass.name.slowToString()));
     } else {
       name = constructor.name;
     }
-    constructor.defaultImplementation = defaultClass.lookupConstructor(name);
+    constructor.defaultImplementation = defaultClass.lookupConstructor(
+        new Selector.callDefaultConstructor(name,
+                                            defaultClass.getLibrary()));
 
     if (constructor.defaultImplementation === null) {
       // We failed to find a constructor named either
       // "MyInterface.name" or "MyClass.name".
       error(node,
             MessageKind.CANNOT_FIND_CONSTRUCTOR2,
             [name, defaultClass.name]);
     }
   }
 
@@ skipping 386 matching lines @@
       if (lookupTarget === visitor.compiler.objectClass) {
         error(diagnosticNode, MessageKind.SUPER_INITIALIZER_IN_OBJECT);
       } else {
         lookupTarget = lookupTarget.supertype.element;
       }
     }
 
     // Lookup constructor and try to match it to the selector.
     ResolverTask resolver = visitor.compiler.resolver;
     final SourceString className = lookupTarget.name;
-    result = lookupTarget.lookupConstructor(className, constructorName);
+    Selector lookupSelector =
+      new Selector.callConstructor(className,

[kasperl, 2012/09/24 05:52:38]

4 space indent.

+                                   constructorName,
+                                   constructor.getLibrary());
+    result = lookupTarget.lookupConstructor(lookupSelector);
     if (result === null || !result.isGenerativeConstructor()) {
-      String classNameString = className.slowToString();
-      String constructorNameString = constructorName.slowToString();
-      String name = (constructorName === const SourceString(''))
-                        ? classNameString
-                        : "$classNameString.$constructorNameString";
       MessageKind kind = isImplicitSuperCall
                          ? MessageKind.CANNOT_RESOLVE_CONSTRUCTOR_FOR_IMPLICIT
                          : MessageKind.CANNOT_RESOLVE_CONSTRUCTOR;
-      error(diagnosticNode, kind, [name]);
+      error(diagnosticNode, kind, [lookupSelector.name]);
     } else {
       if (!selector.applies(result, visitor.compiler)) {
         MessageKind kind = isImplicitSuperCall
                            ? MessageKind.NO_MATCHING_CONSTRUCTOR_FOR_IMPLICIT
                            : MessageKind.NO_MATCHING_CONSTRUCTOR;
         error(diagnosticNode, kind);
       }
     }
     return result;
   }
@@ skipping 2007 matching lines @@
       return new ErroneousFunctionElement(kind, arguments, targetName,
                                           enclosing);
     }
   }
 
   // TODO(ngeoffray): method named lookup should not report errors.
   FunctionElement lookupConstructor(ClassElement cls,
                                     Node diagnosticNode,
                                     SourceString constructorName) {
     cls.ensureResolved(compiler);
-    Element result = cls.lookupConstructor(cls.name, constructorName);
+    Selector selector =
+        new Selector.callConstructor(cls.name,
+                                     constructorName,
+                                     resolver.enclosingElement.getLibrary());
+    Element result = cls.lookupConstructor(selector);
     if (result === null) {
-      String fullConstructorName = cls.name.slowToString();
-      if (constructorName !== const SourceString('')) {
-        fullConstructorName = '$fullConstructorName'
-                              '.${constructorName.slowToString()}';
-      }
       return failOrReturnErroneousElement(cls, diagnosticNode,
-                                          new SourceString(fullConstructorName),
+                                          selector.name,

[kasperl, 2012/09/24 05:52:38]

So there are two places where you need the full name of a constructor including
the class prefix (here and in line 627). In both places you know what the class
is. Maybe just create a helper function in the resolver that takes a class (or
maybe a className) and a call-constructor selector and constructs the human
readable full constructor name?

At least this shouldn't be the reason why you store "full" names in the
selectors (which is a bit complicated in the selector implementation and when
"normalizing" it for lookup).

                                           MessageKind.CANNOT_FIND_CONSTRUCTOR,
-                                          [fullConstructorName]);
+                                          [selector.name.slowToString()]);
     } else if (inConstContext &&
                (result.modifiers == null || !result.modifiers.isConst())) {
       error(diagnosticNode, MessageKind.CONSTRUCTOR_IS_NOT_CONST);
     }
     return result;
   }
 
   visitNewExpression(NewExpression node) {
     Node selector = node.send.selector;
     Element e = visit(selector);
@@ skipping 200 matching lines @@
 
   Element localLookup(SourceString name) => library.find(name);
   Element lookup(SourceString name) => localLookup(name);
   Element lexicalLookup(SourceString name) => localLookup(name);
 
   Element add(Element newElement) {
     throw "Cannot add an element in the top scope";
   }
   String toString() => '$element';
 }