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

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 71 matching lines @@
         return resolveParameter(element);
       }
 
       compiler.unimplemented("resolve($element)",
                              node: element.parseNode(compiler));
     });
   }
 
   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,
                                    SourceString constructorName) {

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

Weird indentation.

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

Done.

     String classNameString = className.slowToString();
     String constructorNameString = constructorName.slowToString();
     return (constructorName === const SourceString(''))
         ? classNameString
         : "$classNameString.$constructorNameString";
    }
 
   FunctionElement resolveConstructorRedirection(InitializerResolver resolver,
                                                 FunctionElement constructor) {
     if (constructor.isPatched) {
       checkMatchingPatchSignatures(constructor, constructor.patch);
       constructor = constructor.patch;
     }
     FunctionExpression node = constructor.parseNode(compiler);
 
     // 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();
-      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;
   }
 
   void resolveRedirectingConstructor(InitializerResolver resolver,
                                      Node node,
                                      FunctionElement constructor,
                                      FunctionElement redirection) {
     Set<FunctionElement> seen = new Set<FunctionElement>();
@@ skipping 598 matching lines @@
       visitor.resolveArguments(call.argumentsNode);
     });
     Selector selector = visitor.mapping.getSelector(call);
     bool isSuperCall = Initializers.isSuperConstructorCall(call);
 
     ClassElement lookupTarget = getSuperOrThisLookupTarget(constructor,
                                                            isSuperCall,
                                                            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,
                                            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);
       assert(superClass.resolutionState == STATE_DONE);
       SourceString constructorName = const SourceString('');
       Selector callToMatch = new Selector.call(
           constructorName,
           classElement.getLibrary(),
           0);
 
       final bool isSuperCall = true;
       ClassElement lookupTarget = getSuperOrThisLookupTarget(constructor,
                                                              isSuperCall,
                                                              functionNode);
+      Selector constructorSelector = new Selector.callDefaultConstructor(
+          visitor.enclosingElement.getLibrary());
+      Element calledConstructor = lookupTarget.lookupConstructor(
+          constructorSelector);
+
       final SourceString className = lookupTarget.name;
-      Element calledConstructor = lookupTarget.lookupConstructor(
-          new Selector.callDefaultConstructor(
-              visitor.enclosingElement.getLibrary()));
-
       final bool isImplicitSuperCall = true;
       verifyThatConstructorMatchesCall(calledConstructor,
                                        callToMatch,
                                        isImplicitSuperCall,
                                        functionNode,
                                        className,
-                                       const SourceString(''));
+                                       constructorSelector);
 
       visitor.world.registerStaticUse(calledConstructor);
     }
   }
 
   void verifyThatConstructorMatchesCall(
       FunctionElement lookedupConstructor,
       Selector call,
       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;
       error(diagnosticNode, kind, [fullConstructorName]);
     } else {
       if (!call.applies(lookedupConstructor, visitor.compiler)) {
         MessageKind kind = isImplicitSuperCall
                            ? MessageKind.NO_MATCHING_CONSTRUCTOR_FOR_IMPLICIT
                            : MessageKind.NO_MATCHING_CONSTRUCTOR;
         error(diagnosticNode, kind);
@@ skipping 2393 matching lines @@
     return result;
   }
   Element lookup(SourceString name) => localLookup(name);
   Element lexicalLookup(SourceString name) => localLookup(name);
 
   Element add(Element newElement) {
     throw "Cannot add an element in a patch library scope";
   }
   String toString() => 'PatchLibraryScope($origin,$patch)';
 }