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

lib/compiler/implementation/elements/elements.dart

Index: lib/compiler/implementation/elements/elements.dart
diff --git a/lib/compiler/implementation/elements/elements.dart b/lib/compiler/implementation/elements/elements.dart
index 71a7d0007dc859acdfd21e5800441658cf7d6755..a9c0c060255602ae93a31eb319a9900c05a64fda 100644
--- a/lib/compiler/implementation/elements/elements.dart
+++ b/lib/compiler/implementation/elements/elements.dart
@@ -1443,24 +1443,43 @@ abstract class ClassElement extends ScopeContainerElement
     }
   }
 
-  Element lookupConstructor(SourceString className,
-                            [SourceString constructorName =
-                                 const SourceString(''),
-                            Element noMatch(Element)]) {
+  Element validateConstructorLookupResults(Selector selector,
+                                           Element result,
+                                           Element noMatch(Element)) {
+    if (result === null
+        || !result.isConstructor()
+        || (selector.name.isPrivate()
+            && result.getLibrary() != selector.library)) {
+      result = noMatch !== null ? noMatch(result) : null;
+    }
+    return result;
+  }
+
+  Element lookupConstructor(Selector selector, [Element noMatch(Element)]) {

[ahe, 2012/10/11 04:55:40]

Perhaps in a future CL, it would be great to make noMatch a required argument.
My suspicion is that most callers of this method would benefit from using the
noMatch method.

[aam-me, 2012/10/11 06:14:00]

Added as TODO.

     // TODO(karlklose): have a map from class names to a map of constructors

[ahe, 2012/10/11 04:55:40]

I think this comment will soon be obsolete.

[aam-me, 2012/10/11 06:14:00]

Removed the comment.

     //                  instead of creating the name here?
     SourceString normalizedName;
-    if (constructorName !== const SourceString('')) {
+    SourceString className = this.name;
+    SourceString constructorName = selector.name;
+    if (constructorName !== const SourceString('') &&
+        ((className === null) ||
+         (constructorName.slowToString() != className.slowToString()))) {
       normalizedName = Elements.constructConstructorName(className,
                                                          constructorName);
     } else {
       normalizedName = className;
     }
     Element result = localLookup(normalizedName);
-    if (result === null || !result.isConstructor()) {
-      result = noMatch !== null ? noMatch(result) : null;
-    }
-    return result;
+    return validateConstructorLookupResults(selector, result, noMatch);
+  }
+
+  Element lookupFactoryConstructor(Selector selector,
+                                   [Element noMatch(Element)]) {

[ahe, 2012/10/11 04:55:40]

I don't understand why noMatch is optional here.

[aam-me, 2012/10/11 06:14:00]

Some users of lookupFactoryConstructor just check return result for null to
determine whether there was a match.
Would you recommend always use noMatch function instead?

[ahe, 2012/10/11 07:14:46]

I would probably prefer to not have the nomatch function at all. But that
depends on the performance impact.

In general, I think we have a tendency to overuse optional arguments in the
compiler. When there are less than a handful of callers of a method, I don't
think it helps making stuff optional.

+    // TODO(karlklose): have a map from class names to a map of constructors

[ahe, 2012/10/11 04:55:40]

Ditto.

[aam-me, 2012/10/11 06:14:00]

Removed the comment.

+    //                  instead of creating the name here?
+    SourceString constructorName = selector.name;
+    Element result = localLookup(constructorName);
+    return validateConstructorLookupResults(selector, result, noMatch);
   }
 
   bool get hasConstructor {