Throw NSM on unresolved factory redirection.

pkg/front_end/lib/src/fasta/kernel/redirecting_factory_body.dart

Index: pkg/front_end/lib/src/fasta/kernel/redirecting_factory_body.dart
diff --git a/pkg/front_end/lib/src/fasta/kernel/redirecting_factory_body.dart b/pkg/front_end/lib/src/fasta/kernel/redirecting_factory_body.dart
index ee0c60543366bc7f36bd9e31e03b5b68c68dffbb..1fa7dd80fbc534085ebc48ef3453523d81fadbc0 100644
--- a/pkg/front_end/lib/src/fasta/kernel/redirecting_factory_body.dart
+++ b/pkg/front_end/lib/src/fasta/kernel/redirecting_factory_body.dart
@@ -6,38 +6,67 @@ library fasta.redirecting_factory_body;
 
 import 'package:kernel/ast.dart'
     show
+        Expression,
         ExpressionStatement,
+        FunctionNode,
         InvalidExpression,
         Let,
         Member,
         Procedure,
         StaticGet,
+        StringLiteral,
         VariableDeclaration;
 
+const String letName = "#redirecting_factory";
+
 class RedirectingFactoryBody extends ExpressionStatement {
-  RedirectingFactoryBody(Member target)
-      : super(new Let(new VariableDeclaration.forValue(new StaticGet(target)),
+  RedirectingFactoryBody.internal(Expression value)

[Paul Berry, 2017/06/12 20:55:41]

Nit: I prefer to name internal constructors like this one with a leading "_". 
It avoids bugs by reducing the risk that someone will call it accidentally.  It
also reduces cognitive load because it lets a reviewer know that there are no
callers outside this file.

[ahe, 2017/06/13 07:11:41]

Acknowledged.

+      : super(new Let(new VariableDeclaration(letName, initializer: value),
             new InvalidExpression()));
 
+  RedirectingFactoryBody(Member target) : this.internal(new StaticGet(target));
+
+  RedirectingFactoryBody.unresolved(String name)
+      : this.internal(new StringLiteral(name));
+
   Member get target {
-    Let let = expression;
-    StaticGet staticGet = let.variable.initializer;
-    return staticGet.target;
+    var value = getValue(expression);
+    return value is StaticGet ? value.target : null;
   }
-}
 
-bool isRedirectingFactory(Member member) {
-  return member is Procedure && member.function.body is RedirectingFactoryBody;
-}
+  String get unresolvedName {
+    var value = getValue(expression);
+    return value is StringLiteral ? value.value : null;
+  }
 
-Member getImmediateRedirectionTarget(Member member) {
-  if (isRedirectingFactory(member)) {
-    Procedure procedure = member;
-    RedirectingFactoryBody body = procedure.function.body;
-    return body.target;
-  } else {
+  bool get isUnresolved => unresolvedName != null;
+
+  static getValue(Expression expression) {
+    if (expression is Let) {
+      VariableDeclaration variable = expression.variable;
+      if (variable.name == letName) {
+        return variable.initializer;
+      }
+    }
     return null;
   }
+
+  static void restoreFromDill(Procedure factory) {
+    // This is a hack / work around for storing redirecting constructors in
+    // dill files. See `KernelClassBuilder.addRedirectingConstructor` in
+    // [kernel_class_builder.dart](kernel_class_builder.dart).
+    FunctionNode function = factory.function;
+    ExpressionStatement statement = function.body;
+    function.body =
+        new RedirectingFactoryBody.internal(getValue(statement.expression))

[Paul Berry, 2017/06/12 20:55:41]

Nit: I think we should report an internalError if the value returned by
getValue() is `null`, because that indicates that we've read corrupted data from
the Dill file.

[ahe, 2017/06/13 07:11:41]

I think I want to iterate more on this. I'll follow up with another CL.

+          ..parent = function;
+  }
+}
+
+RedirectingFactoryBody getRedirectingFactoryBody(Member member) {
+  return member is Procedure && member.function.body is RedirectingFactoryBody
+      ? member.function.body
+      : null;
 }
 
 Member getRedirectionTarget(Procedure member) {
@@ -45,11 +74,15 @@ Member getRedirectionTarget(Procedure member) {
   // (https://en.wikipedia.org/wiki/Cycle_detection#Tortoise_and_hare) to
   // handle cycles.
   Member tortoise = member;
-  Member hare = getImmediateRedirectionTarget(member);
+  RedirectingFactoryBody tortoiseBody = getRedirectingFactoryBody(tortoise);
+  Member hare = tortoiseBody?.target;
+  RedirectingFactoryBody hareBody = getRedirectingFactoryBody(hare);
   while (tortoise != hare) {
-    if (!isRedirectingFactory(tortoise)) return tortoise;
-    tortoise = getImmediateRedirectionTarget(tortoise);
-    hare = getImmediateRedirectionTarget(getImmediateRedirectionTarget(hare));
+    if (tortoiseBody?.isUnresolved ?? true) return tortoise;
+    tortoise = tortoiseBody.target;
+    tortoiseBody = getRedirectingFactoryBody(tortoise);
+    hare = getRedirectingFactoryBody(hareBody?.target)?.target;
+    hareBody = getRedirectingFactoryBody(hare);
   }
   return null;
 }