Ensure that ClassElement.lookupConstructor fails when looking up default constructor using Selector…

lib/compiler/implementation/resolver.dart

Index: lib/compiler/implementation/resolver.dart
diff --git a/lib/compiler/implementation/resolver.dart b/lib/compiler/implementation/resolver.dart
index 7409bc630c7e7b8040e27f8555357b4fa7239f42..576032e11c546e5f41b6816a2290227d45c2f6a7 100644
--- a/lib/compiler/implementation/resolver.dart
+++ b/lib/compiler/implementation/resolver.dart
@@ -89,8 +89,26 @@ class ResolverTask extends CompilerTask {
 
   bool isNamedConstructor(Send node) => node.receiver !== null;
 
-  SourceString getConstructorName(Send node) {
-    return node.selector.asIdentifier().source;
+  Selector getConstructorSelector(Send node, SourceString className, ResolverVisitor visitor) {

[kasperl, 2012/10/15 12:14:12]

Long line.

[aam-me, 2012/10/16 04:09:32]

Done.

+    SourceString constructorName;
+    if (!isNamedConstructor(node)) {
+      constructorName = const SourceString("");

[kasperl, 2012/10/15 12:14:12]

Why don't you just return the selector here? It feels weird to have another
switch on the constructor name later.

[aam-me, 2012/10/16 04:09:32]

Done.

+    } else {
+      constructorName = node.selector.asIdentifier().source;
+      if (constructorName.slowToString() == className.slowToString()) {

[kasperl, 2012/10/15 12:14:12]

Add a comment that explains when this happens? This is a named constructor so
why do you need to deal with the case where the constructor name is the same as
the class name?

What happens for code like this? 

  class A {
    A.A();
  }

[aam-me, 2012/10/16 04:09:32]

You are right, Kasper. The code above is wrong. It doesn't properly handle
"this.C()" in redirecting constructors: when constructor name matches class
name, it gets resolved to default constructor instead of named constructor.

I added tests\compiler\dart2js_extra\this_redirecting_constructor_test.dart to
cover that.

I'm afraid I introduced this error as part of recent
https://chromiumcodereview.appspot.com/10947024 it seems.

+        constructorName = const SourceString("");
+      }
+    }
+    Selector selector;

[kasperl, 2012/10/15 12:14:12]

I would get rid of this part and just put the returns in the places where you
compute the constructor name. That way you can also get rid of the
constructorName local variable.

[aam-me, 2012/10/16 04:09:32]

Done.

+    if (constructorName != const SourceString("")) {
+      selector = new Selector.callConstructor(
+        constructorName,
+        visitor.enclosingElement.getLibrary());
+    } else {
+      selector = new Selector.callDefaultConstructor(
+        visitor.enclosingElement.getLibrary());
+    }
+    return selector;
   }
 
   String constructorNameForDiagnostics(SourceString className,
@@ -117,16 +135,9 @@ class ResolverTask extends CompilerTask {
     if (!initializers.isEmpty() &&
         Initializers.isConstructorRedirect(initializers.head)) {
       final ClassElement classElement = constructor.getEnclosingClass();
-      Selector selector;
-      if (isNamedConstructor(initializers.head)) {
-        SourceString constructorName = getConstructorName(initializers.head);
-        selector = new Selector.callConstructor(
-            constructorName,
-            resolver.visitor.enclosingElement.getLibrary());
-      } else {
-        selector = new Selector.callDefaultConstructor(
-            resolver.visitor.enclosingElement.getLibrary());
-      }
+      Selector selector = getConstructorSelector(initializers.head,
+                                                 classElement.name,
+                                                 resolver.visitor);
       return classElement.lookupConstructor(selector);
     }
     return null;
@@ -745,33 +756,22 @@ class InitializerResolver {
                                                            call);
     final SourceString className = lookupTarget.name;
 
-    SourceString constructorName;
-    Selector lookupSelector;
-    if (resolver.isNamedConstructor(call)) {
-      constructorName = resolver.getConstructorName(call);
-      lookupSelector = new Selector.callConstructor(
-          constructorName,
-          visitor.enclosingElement.getLibrary());
-    } else {
-      constructorName = const SourceString('');
-      lookupSelector = new Selector.callDefaultConstructor(
-          visitor.enclosingElement.getLibrary());
-    }
-
-    FunctionElement lookedupConstructor =
-        lookupTarget.lookupConstructor(lookupSelector);
+    Selector constructorSelector =
+        resolver.getConstructorSelector(call, className, visitor);
+    FunctionElement calledConstructor =
+        lookupTarget.lookupConstructor(constructorSelector);
 
     final bool isImplicitSuperCall = false;
-    verifyThatConstructorMatchesCall(lookedupConstructor,
+    verifyThatConstructorMatchesCall(calledConstructor,
                                      selector,
                                      isImplicitSuperCall,
                                      call,
-                                     constructorName,
-                                     className);
+                                     className,
+                                     constructorSelector);
 
-    visitor.useElement(call, lookedupConstructor);
-    visitor.world.registerStaticUse(lookedupConstructor);
-    return lookedupConstructor;
+    visitor.useElement(call, calledConstructor);
+    visitor.world.registerStaticUse(calledConstructor);
+    return calledConstructor;
   }
 
   void resolveImplicitSuperConstructorSend(FunctionElement constructor,
@@ -792,18 +792,19 @@ class InitializerResolver {
       ClassElement lookupTarget = getSuperOrThisLookupTarget(constructor,
                                                              isSuperCall,
                                                              functionNode);
-      final SourceString className = lookupTarget.name;
+      Selector constructorSelector = new Selector.callDefaultConstructor(
+          visitor.enclosingElement.getLibrary());
       Element calledConstructor = lookupTarget.lookupConstructor(
-          new Selector.callDefaultConstructor(
-              visitor.enclosingElement.getLibrary()));
+          constructorSelector);
 
+      final SourceString className = lookupTarget.name;
       final bool isImplicitSuperCall = true;
       verifyThatConstructorMatchesCall(calledConstructor,
                                        callToMatch,
                                        isImplicitSuperCall,
                                        functionNode,
                                        className,
-                                       const SourceString(''));
+                                       constructorSelector);
 
       visitor.world.registerStaticUse(calledConstructor);
     }
@@ -815,12 +816,13 @@ class InitializerResolver {
       bool isImplicitSuperCall,
       Node diagnosticNode,
       SourceString className,
-      SourceString constructorName) {
+      Selector constructorSelector) {
     if (lookedupConstructor === null
         || !lookedupConstructor.isGenerativeConstructor()) {
       var fullConstructorName =
-          visitor.compiler.resolver.constructorNameForDiagnostics(className,
-                                                              constructorName);
+          visitor.compiler.resolver.constructorNameForDiagnostics(
+              className,
+              constructorSelector.name);
       MessageKind kind = isImplicitSuperCall
           ? MessageKind.CANNOT_RESOLVE_CONSTRUCTOR_FOR_IMPLICIT
           : MessageKind.CANNOT_RESOLVE_CONSTRUCTOR;