Check cycles in redirecting factories.

sdk/lib/_internal/compiler/implementation/resolution/members.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.
 
 part of resolution;
 
 abstract class TreeElements {
   Element get currentElement;
   Set<Node> get superUses;
 
@@ skipping 2648 matching lines @@
     }
 
     FunctionSignature targetSignature =
         redirectionTarget.computeSignature(compiler);
     FunctionSignature constructorSignature =
         constructor.computeSignature(compiler);
     if (!targetSignature.isCompatibleWith(constructorSignature)) {
       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);
     if (isSymbolConstructor) {
       compiler.backend.registerSymbolConstructor(mapping);
     }
   }
 
   visitThrow(Throw node) {
     compiler.backend.registerThrowExpression(mapping);
@@ skipping 1531 matching lines @@
     return e;
   }
 
   /// Assumed to be called by [resolveRedirectingFactory].
   Element visitReturn(Return node) {
     Node expression = node.expression;
     return finishConstructorReference(visit(expression),
                                       expression, expression);
   }
 }