[dart2dart] Support cosntructor redirects.

lib/compiler/implementation/dart_backend/placeholder_collector.dart

Index: lib/compiler/implementation/dart_backend/placeholder_collector.dart
diff --git a/lib/compiler/implementation/dart_backend/placeholder_collector.dart b/lib/compiler/implementation/dart_backend/placeholder_collector.dart
index a8e45f8795b4f8da67a6083a6c35e423d7967fa2..cf591d9a280ea3342fb856121e11dcbd46ae438b 100644
--- a/lib/compiler/implementation/dart_backend/placeholder_collector.dart
+++ b/lib/compiler/implementation/dart_backend/placeholder_collector.dart
@@ -27,7 +27,10 @@ class FunctionScope {
 class ConstructorPlaceholder {
   final Node node;
   final DartType type;
-  ConstructorPlaceholder(this.node, this.type);
+  final bool isRedirectingCall;
+  ConstructorPlaceholder(this.node, this.type, this.isRedirectingCall) {

[Anton Muhin, 2012/10/30 15:23:44]

it looks like as of now in all the call sites you pass a constant as
isRedirectingCall.

I would suggest to add named constructor ConstructorPlace.redirecting and
corresponding helper method, that would make code easier to read imho

[Roman, 2012/10/30 15:52:09]

Done.

+    assert(isRedirectingCall != (type != null));
+  }
 }
 
 class DeclarationTypePlaceholder {
@@ -47,7 +50,12 @@ class SendVisitor extends ResolvedVisitor {
   visitForeignSend(Send node) {}
 
   visitSuperSend(Send node) {
-    collector.tryMakeMemberPlaceholder(node.selector);
+    Element element = elements[node];
+    if (element != null && element.isConstructor()) {
+      collector.makeConstructorPlaceholder(node.selector, element, null, true);

[Anton Muhin, 2012/10/30 15:23:44]

my original understanding of constructor placeholders was a collection of
constructor declarations to rename.

Now it includes invocations too.

Should they go into the same place?

Cannot we add redirecting constructor invocations into member renames where they
apparently should belong?

[Roman, 2012/10/30 15:52:09]

1) We cannot add declarations to one placeholder type and invocations to
another, that way it would be quite difficult to rename them consistently.
2) Originally both constructor invocations and declarations made their way into
constructor placeholders, invocations - from visitNewExpression. This CL makes
constructor placeholders for redirecting invocations.

+    } else {
+      collector.tryMakeMemberPlaceholder(node.selector);
+    }
   }
 
   visitDynamicSend(Send node) {
@@ -98,15 +106,14 @@ class SendVisitor extends ResolvedVisitor {
         || identical(element, compiler.assertMethod)) {
       return;
     }
-    // TODO(smok): We should never go inside this IF, check?
     if (element.isConstructor() || element.isFactoryConstructor()) {
       // Rename named constructor in redirection position:
       // class C { C.named(); C.redirecting() : this.named(); }
-      // TODO(smok): Fix redirecting constructors.
       if (node.receiver is Identifier
           && node.receiver.asIdentifier().isThis()) {
         assert(node.selector is Identifier);
-        collector.tryMakeMemberPlaceholder(node.selector);
+        collector.makeConstructorPlaceholder(
+            node.selector, element, null, true);
       }
       return;
     }
@@ -159,12 +166,6 @@ class PlaceholderCollector extends Visitor {
       constructorPlaceholders =
           new Map<Element, List<ConstructorPlaceholder>>();
 
-  void tryMakeConstructorPlaceholder(
-      FunctionExpression constructor, FunctionElement constructorElement) {
-    DartType type = constructorElement.getEnclosingClass().type.asRaw();
-    makeConstructorPlaceholder(constructor.name, constructorElement, type);
-  }
-
   void collectFunctionDeclarationPlaceholders(
       FunctionElement element, FunctionExpression node) {
     if (element.isGenerativeConstructor() || element.isFactoryConstructor()) {
@@ -179,7 +180,8 @@ class PlaceholderCollector extends Visitor {
       //      0.dart: class C { C(); }
       //      1.dart: interface C default p0.C { C(); }
       //    the second case is just a bug now.
-      tryMakeConstructorPlaceholder(node, element);
+      DartType type = element.getEnclosingClass().type.asRaw();
+      makeConstructorPlaceholder(node.name, element, type, false);
     } else if (Elements.isStaticOrTopLevel(element)) {
       // Note: this code should only rename private identifiers for class'
       // fields/getters/setters/methods.  Top-level identifiers are renamed
@@ -331,10 +333,11 @@ class PlaceholderCollector extends Visitor {
     getLocalPlaceholder().nodes.add(identifier);
   }
 
-  void makeConstructorPlaceholder(Node node, Element element, DartType type) {
+  void makeConstructorPlaceholder(Node node, Element element, DartType type,
+      bool isRedirecting) {
     constructorPlaceholders
         .putIfAbsent(element, () => <ConstructorPlaceholder>[])
-            .add(new ConstructorPlaceholder(node, type));
+            .add(new ConstructorPlaceholder(node, type, isRedirecting));
   }
 
   void internalError(String reason, {Node node}) {
@@ -355,7 +358,7 @@ class PlaceholderCollector extends Visitor {
     assert(constructor != null);
     assert(send.receiver == null);
     if (constructor is !ErroneousElement) {
-      makeConstructorPlaceholder(node.send.selector, constructor, type);
+      makeConstructorPlaceholder(node.send.selector, constructor, type, false);
       // TODO(smok): Should this be in visitNamedArgument?
       // Field names can be exposed as names of optional arguments, e.g.
       // class C {