Check cycles in redirecting factories.

sdk/lib/_internal/compiler/implementation/resolution/members.dart

Index: sdk/lib/_internal/compiler/implementation/resolution/members.dart
diff --git a/sdk/lib/_internal/compiler/implementation/resolution/members.dart b/sdk/lib/_internal/compiler/implementation/resolution/members.dart
index 130943c3f752e220ae367a76ff733065f9c28743..f3810b300faced2fc4c97b23bc51bc401bb00968 100644
--- a/sdk/lib/_internal/compiler/implementation/resolution/members.dart
+++ b/sdk/lib/_internal/compiler/implementation/resolution/members.dart
@@ -2666,19 +2666,33 @@ class ResolverVisitor extends MappingVisitor<Element> {
       compiler.backend.registerThrowNoSuchMethod(mapping);
     }
 
-    // TODO(ahe): Check that this doesn't lead to a cycle.  For now,
-    // just make sure that the redirection target isn't itself a
-    // redirecting factory.
-    { // This entire block is temporary code per the above TODO.
-      FunctionElement targetImplementation = redirectionTarget.implementation;
-      FunctionExpression function = targetImplementation.parseNode(compiler);
-      if (function != null
-          && function.body != null
-          && function.body.asReturn() != null
-          && function.body.asReturn().isRedirectingFactoryBody) {
-        unimplemented(node.expression, 'redirecting to redirecting factory');
+    // Register a post process to check for cycles in the redirection chain and
+    // set the actual generative constructor at the end of the chain.
+    compiler.enqueuer.resolution.addPostProcessAction(constructor, () {
+      FunctionElementX current = constructor;
+      if (current.internalRedirectionTarget != null) {
+        return;
       }
-    }
+      List<Element> seen = new List<Element>();
+      // Follow the chain of redirections and check for cycles.
+      while (current != current.defaultImplementation &&
+             current.internalRedirectionTarget == null) {
+        Element target = current.defaultImplementation;
+        if (seen.contains(target)) {
+          error(node, MessageKind.CYCLIC_REDIRECTING_FACTORY);
+          return;
+        }
+        seen.add(current);
+        current = target;
+      }
+      // [current] is now the actual target of the redirections.  Run through
+      // the constructors again and set their [redirectionTarget].

[ngeoffray, 2013/09/18 11:19:23]

Why do you need to do this for all seen targets? Won't they be able to do that
for just themselves when their post process action is being executed?

[karlklose, 2013/09/18 12:30:39]

It is an optimization to not do the lookup again if two chains of redirections
share a path.

+      while (!seen.isEmpty) {
+        FunctionElementX factory = seen.removeLast();
+        factory.redirectionTarget = current;
+      }
+    });
+
     world.registerStaticUse(redirectionTarget);
     world.registerInstantiatedClass(
         redirectionTarget.enclosingElement.declaration, mapping);