Implement named constructors and resolving of redirecting constructors and super-initializers.

frog/leg/resolver.dart

Index: frog/leg/resolver.dart
diff --git a/frog/leg/resolver.dart b/frog/leg/resolver.dart
index 2fcfbbf6864b04680c7182a8ee5f131d67ab4003..54c3b21acd6855ca052e6bc5361a37cb320c5395 100644
--- a/frog/leg/resolver.dart
+++ b/frog/leg/resolver.dart
@@ -42,7 +42,7 @@ class ResolverTask extends CompilerTask {
 
     visitor = new FullResolverVisitor.from(visitor);
     if (tree.initializers != null) {
-      resolveInitializers(element, tree, visitor);
+      new InitializerResolver(visitor, element).resolveInitializers(tree);
     }
     visitor.visit(tree.body);
 
@@ -63,77 +63,173 @@ class ResolverTask extends CompilerTask {
     return visitor.mapping;
   }
 
-  bool isInitializer(SendSet node) {
+  void resolveType(ClassElement element) {
+    measure(() {
+      ClassNode tree = element.parseNode(compiler, compiler);
+      ClassResolverVisitor visitor = new ClassResolverVisitor(compiler);
+      visitor.visit(tree);
+    });
+  }
+
+  void resolveSignature(FunctionElement element) {
+    measure(() {
+      FunctionExpression node = element.parseNode(compiler, compiler);
+      SignatureResolverVisitor visitor =
+          new SignatureResolverVisitor(compiler, element);
+      visitor.visitFunctionExpression(node);
+    });
+  }
+}
+
+
+class InitializerResolver {
+  final ResolverVisitor visitor;
+  final FunctionElement constructor;
+  Map<SourceString, Node> initialized;

[ngeoffray, 2012/01/19 08:56:12]

Make initialized final?

[karlklose, 2012/01/19 13:51:24]

Done.

+  Node initializerOrSuper;

[ngeoffray, 2012/01/19 08:56:12]

Instead of having initializerOrSuper, I suggest keeping the initializers node
around. Then, if you see a redirector consturctor, make sure the length of its
list of nodes is 1.

[karlklose, 2012/01/19 13:51:24]

Done.

+  bool hasSuper;
+
+  InitializerResolver(this.visitor, this.constructor)
+    : initialized = new Map<SourceString, Node>(), hasSuper = false;
+
+  Universe get universe() => visitor.compiler.universe;

[ngeoffray, 2012/01/19 08:56:12]

Unused?

[karlklose, 2012/01/19 13:51:24]

Done, removed.

+
+  error(Node node, MessageKind kind, [arguments = const []]) {
+    visitor.error(node, kind, arguments);
+  }
+
+  warning(Node node, MessageKind kind, [arguments = const []]) {
+    visitor.warning(node, kind, arguments);
+  }
+
+  bool isFieldInitializer(SendSet node) {
     if (node.selector.asIdentifier() == null) return false;
     if (node.receiver == null) return true;
     if (node.receiver.asIdentifier() == null) return false;
     return node.receiver.asIdentifier().isThis();
   }
 
-  SourceString getInitializerFieldName(SendSet node, onError(node)) {
-    if (!isInitializer(node)) onError(node);
-    return node.selector.asIdentifier().source;
+  void resolveFieldInitializer(SendSet init) {
+    // init is of the form [this.]field = value.
+    final Node selector = init.selector;
+    final SourceString name = selector.asIdentifier().source;
+    // Lookup target field.
+    Element target;
+    if (isFieldInitializer(init)) {
+      final ClassElement classElement = constructor.enclosingElement;
+      target = classElement.lookupLocalMember(name);
+      if (target === null) {
+        error(selector, MessageKind.CANNOT_RESOLVE, [name]);
+      } else if (target.kind != ElementKind.FIELD) {
+        error(selector, MessageKind.NOT_A_FIELD, [name]);
+      } else if (!target.isInstanceMember()) {
+        error(selector, MessageKind.INIT_STATIC_FIELD, [name]);
+      }
+    } else {
+      error(init, MessageKind.INVALID_RECEIVER_IN_INITIALIZER);
+    }
+    visitor.useElement(init, target);
+    // Check for duplicate initializers.
+    if (initialized.containsKey(name)) {
+      error(init, MessageKind.DUPLICATE_INITIALIZER, [name]);

[ngeoffray, 2012/01/19 08:56:12]

Why error + warning? Shouldn't it be just one of those? (don't have the spec
available, so I don't know which one)

[karlklose, 2012/01/19 13:51:24]

The warning gives additional feedback.

[ngeoffray, 2012/01/19 14:36:33]

So why not putting all the feedback in a single error?

+      warning(initialized[name], MessageKind.ALREADY_INITIALIZED, [name]);
+    }
+    initialized[name] = init;
+    // Resolve initializing value.
+    visitor.visitInStaticContext(init.arguments.head);
+    if (initializerOrSuper == null) initializerOrSuper = init;
   }
 
-  void resolveInitializers(Element element, FunctionExpression node,
-                           ResolverVisitor visitor) {
-    void onError(node) {
-      visitor.error(node, MessageKind.INVALID_RECEIVER_IN_INITIALIZER);
+  SourceString getConstructorName(ClassElement cls, Send node) {
+    SourceString constructor = node.selector.asIdentifier().source;
+    if (node.receiver !== null) {
+      return new SourceString('${cls.name}.$constructor');
+    } else {
+      return cls.name;
+    }
+  }
+
+  void resolveSuperOrThis(Send call, Node next) {
+    noConstructor(e) {
+      if (e !== null) error(call, MessageKind.NO_CONSTRUCTOR, [e.name, e.kind]);
+    }
+
+    ClassElement lookupTarget = constructor.enclosingElement;
+    if (call.isSuperConstructorCall) {
+      // Check for invalid initializers.
+      if (hasSuper) {
+        error(call, MessageKind.DUPLICATE_SUPER_INITIALIZER);
+      }
+      if (initializerOrSuper == null) initializerOrSuper = call;
+      hasSuper = true;
+      // Calculate correct lookup target and constructor name.
+      if (constructor.name === Types.OBJECT) {

[ngeoffray, 2012/01/19 08:56:12]

Shouldn't that just be lookupTarget === Types.OBJECT ?

[karlklose, 2012/01/19 13:51:24]

Done.

+        error(call, MessageKind.SUPER_INITIALIZER_IN_OBJECT);

[ngeoffray, 2012/01/19 08:56:12]

Should you return here? Otherwise you would do a lookup of something in Object,
which is maybe not what you want, as you will get another similar error message.

[karlklose, 2012/01/19 13:51:24]

I skipped the lookup in this case but still resolve the arguments.

[ngeoffray, 2012/01/19 14:36:33]

Good point.

+      } else {
+        lookupTarget = lookupTarget.supertype.element;
+      }
+    } else if (call.isConstructorRedirect) {
+      // Check that there are no other initializers.
+      if (initializerOrSuper !== null || next !== null) {
+        Node diagnosticNode =
+            initializerOrSuper !== null ? initializerOrSuper
+                                        : next;
+        error(diagnosticNode,
+              MessageKind.REDIRECTING_CTOR_HAS_INITIALIZER);
+      }
+    } else {
+      visitor.error(call, MessageKind.CONSTRUCTOR_CALL_EXPECTED);
+    }
+
+    final SourceString name = getConstructorName(lookupTarget, call);
+    FunctionElement target =
+        lookupTarget.lookupConstructor(name, noConstructor);
+    if (target === null && call.arguments.isEmpty()) {
+      target = lookupTarget.getSynthesizedConstructor();
     }
-    Map<SourceString, Node> initialized = new Map<SourceString, Node>();
+    if (target === null) {
+      error(call, MessageKind.CANNOT_RESOLVE, ["constructor $name"]);

[ngeoffray, 2012/01/19 08:56:12]

Maybe add a CANNOT_RESOLVE_CONSTRUCTOR, to avoid having the 'constructor' in the
argument?

[karlklose, 2012/01/19 13:51:24]

Done.

+    } else {

[ngeoffray, 2012/01/19 08:56:12]

The parameters may already there, so I don't think you should call parseNode
here. Instead, I would add a parameterCount method on a FunctionElement and let
it resolve its signature:

int parameterCount(Compiler compiler) {
  if (parameters == null) compiler.resolveSignature(element);
  return parameters.length;
}

[karlklose, 2012/01/19 13:51:24]

Done.

+      final Compiler compiler = visitor.compiler;
+      final FunctionExpression targetNode =
+          target.parseNode(compiler, compiler);
+      // TODO(karlklose): support optional arguments.
+      if (targetNode.parameterCount() != call.argumentCount()) {
+        error(call, MessageKind.NO_MATCHING_CONSTRUCTOR);
+      }
+    }
+    visitor.compiler.enqueue(new WorkItem.toCompile(target));

[ngeoffray, 2012/01/19 08:56:12]

You should leave the codegen deciding if it should be enqueued or not.

[karlklose, 2012/01/19 13:51:24]

Done.

+    visitor.useElement(call, target);
+    // Resolve the arguments of the call.
+    for (Link<Node> arguments = call.arguments;
+         !arguments.isEmpty();
+         arguments = arguments.tail) {
+      visitor.visitInStaticContext(arguments.head);
+    }
+  }
+
+  void resolveInitializers(FunctionExpression node) {
+    if (node.initializers === null) return;
+    Compiler compiler = visitor.compiler;
+    // TODO(karlklose): implement initializer parameters.

[ngeoffray, 2012/01/19 08:56:12]

Please add an unimplemented where this is not handled. It looks to me that
resolveFieldInitializer will just report a random error.

[karlklose, 2012/01/19 13:51:24]

This is not unimplemented here, it was only a note that we have to take care of
the names of initializer parameters when checking that all finals are
initialized. I removed the TODO.

[ngeoffray, 2012/01/19 14:36:33]

I see. Thanks for the explanation.

     for (Link<Node> link = node.initializers.nodes;
          !link.isEmpty();
          link = link.tail) {
       if (link.head.asSendSet() != null) {
-        SendSet init = link.head;
-        SourceString name = getInitializerFieldName(init, onError);
-        ClassElement classElement = element.enclosingElement;
-        Element target = classElement.lookupLocalMember(name);
-        Node selector = init.selector;
-        if (target == null) {
-          visitor.error(selector, MessageKind.CANNOT_RESOLVE, [name]);
-        } else if (target.kind != ElementKind.FIELD) {
-          visitor.error(selector, MessageKind.NOT_A_FIELD, [name]);
-        } else if (!target.isInstanceMember()) {
-          visitor.error(selector, MessageKind.INIT_STATIC_FIELD, [name]);
-        }
-        visitor.useElement(init, target);
-        if (initialized.containsKey(name)) {
-          visitor.error(init, MessageKind.DUPLICATE_INITIALIZER, [name]);
-          visitor.warning(initialized[name], MessageKind.ALREADY_INITIALIZED,
-                          [name]);
-        }
-        initialized[name] = init;
-        Node value = init.arguments.head;
-        visitor.visitInStaticContext(value);
+        final SendSet init = link.head.asSendSet();
+        resolveFieldInitializer(init);
       } else if (link.head.asSend() !== null) {
-        // TODO(karlklose): super(...), this(...).
-        compiler.cancel('uniplemented', node:link.head);
+        final Send call = link.head.asSend();
+        resolveSuperOrThis(call, link.tail.isEmpty() ? null : link.tail.head);
       } else {
-        compiler.cancel('internal error: invalid initializer',
-                        node: link.head);
+        visitor.compiler.cancel('internal error: invalid initializer',
+                                node: link.head);
       }
     }
   }
-
-  void resolveType(ClassElement element) {
-    measure(() {
-      ClassNode tree = element.parseNode(compiler, compiler);
-      ClassResolverVisitor visitor = new ClassResolverVisitor(compiler);
-      visitor.visit(tree);
-    });
-  }
-
-  void resolveSignature(FunctionElement element) {
-    measure(() {
-      FunctionExpression node = element.parseNode(compiler, compiler);
-      SignatureResolverVisitor visitor =
-          new SignatureResolverVisitor(compiler, element);
-      visitor.visitFunctionExpression(node);
-    });
-  }
 }
 
+
 // TODO(ahe): Frog cannot handle generic types.
 class ResolverVisitor extends AbstractVisitor/*<Element>*/ {
   final Compiler compiler;
@@ -180,7 +276,7 @@ class ResolverVisitor extends AbstractVisitor/*<Element>*/ {
   Element lookup(Node node, SourceString name) {
     Element result = context.lookup(name);
     if (!inInstanceContext && result != null && result.isInstanceMember()) {
-      error(node, MessageKind.NOT_STATIC, [node]);
+      error(node, MessageKind.NO_INSTANCE_AVAILABLE, [node]);
     }
     return result;
   }
@@ -198,8 +294,10 @@ class ResolverVisitor extends AbstractVisitor/*<Element>*/ {
   }
 
   visitIdentifier(Identifier node) {
-    if (node.isThis()) {
-      if (!inInstanceContext) error(node, MessageKind.NO_THIS_IN_STATIC);
+    if (node.isThis() || node.isSuper()) {

[ngeoffray, 2012/01/19 08:56:12]

This is already handled line 302.

[karlklose, 2012/01/19 13:51:24]

Done.

+      if (!inInstanceContext) {
+        error(node, MessageKind.NO_INSTANCE_AVAILABLE, [node]);
+      }
       return null;
     } else if (node.isSuper()) {
       if (!inInstanceContext) error(node, MessageKind.NO_SUPER_IN_STATIC);
@@ -214,25 +312,29 @@ class ResolverVisitor extends AbstractVisitor/*<Element>*/ {
   }
 
   visitTypeAnnotation(TypeAnnotation node) {
-    Identifier name = node.typeName.asIdentifier();
-    if (name === null) {
-      // TODO(karlklose): In progress.
-      cancel(node.typeName, "not implemented");
+    SourceString className;
+    if (node.typeName.asSend() !== null) {
+      // In new and const expressions, the type name can be a Send to
+      // denote named parameters or library prefixes.

[ngeoffray, 2012/01/19 08:56:12]

named parameters -> named constructor?

[karlklose, 2012/01/19 13:51:24]

Done.

+      Send send = node.typeName.asSend();
+      className = send.receiver.asIdentifier().source;
+    } else {
+      className = node.typeName.asIdentifier().source;
     }
-    if (name.source == const SourceString('var')) return null;
-    if (name.source == const SourceString('void')) return null;
-    Element element = context.lookup(name.source);
+    if (className == const SourceString('var')) return null;
+    if (className == const SourceString('void')) return null;
+    Element element = context.lookup(className);
     if (element === null) {
       if (typeRequired) {
-        error(node, MessageKind.CANNOT_RESOLVE_TYPE, [name]);
+        error(node, MessageKind.CANNOT_RESOLVE_TYPE, [className]);
       } else {
-        warning(node, MessageKind.CANNOT_RESOLVE_TYPE, [name]);
+        warning(node, MessageKind.CANNOT_RESOLVE_TYPE, [className]);
       }
     } else if (element.kind !== ElementKind.CLASS) {
       if (typeRequired) {
-        error(node, MessageKind.NOT_A_TYPE, [name]);
+        error(node, MessageKind.NOT_A_TYPE, [className]);
       } else {
-        warning(node, MessageKind.NOT_A_TYPE, [name]);
+        warning(node, MessageKind.NOT_A_TYPE, [className]);
       }
     } else {
       ClassElement cls = element;
@@ -333,14 +435,18 @@ class FullResolverVisitor extends ResolverVisitor {
 
   visitFunctionExpression(FunctionExpression node) {

[ngeoffray, 2012/01/19 08:56:12]

I don't think you can be here for a constructor. This method should just be for
closures, as you're defining a new element. The parser takes care of defining an
element for constructors.

[karlklose, 2012/01/19 13:51:24]

Done.

     visit(node.returnType);
+    SourceString name;
     if (node.name === null) {
       cancel(node, "anonymous functions are not implemented");
-    }
-    if (node.name.asIdentifier() === null) {
-      cancel(node.name, "named constructors are not implemented");
+    } else if (node.name.asSend() != null) {
+      Identifier cls = node.asSend().receiver.asIdentifier();
+      Identifier constructor = node.asSend().selector.asIdentifier();
+      name = new SourceString('${cls.source}.${constructor.source}');
+    } else {
+      name = node.name.asIdentifier().source;
     }
     FunctionElement enclosingElement = new FunctionElement.node(
-        node, ElementKind.FUNCTION, null, context.element);
+        name, node, ElementKind.FUNCTION, null, context.element);
     defineElement(node, enclosingElement);
     context = new MethodScope(context, enclosingElement);
 
@@ -517,22 +623,39 @@ class FullResolverVisitor extends ResolverVisitor {
 
     visit(node.send.argumentsNode);
 
+    SourceString constructorName;
+    Node typeName = node.send.selector.asTypeAnnotation().typeName;
+    if (typeName.asSend() !== null) {
+      Identifier receiver = typeName.asSend().receiver.asIdentifier();
+      Identifier selector = typeName.asSend().selector.asIdentifier();
+      SourceString className = receiver.source;
+      SourceString name = selector.source;
+      constructorName = new SourceString('$className.$name');
+    } else {
+      constructorName = typeName.asIdentifier().source;
+    }
     ClassElement cls = resolveTypeRequired(node.send.selector);
     Element constructor = null;
     if (cls !== null) {
-      // TODO(ngeoffray): set constructor-name correctly.
-      SourceString name = cls.name;
-      constructor = cls.resolve(compiler).lookupConstructor(name);
-      if (name == cls.name
+      constructor = cls.resolve(compiler).lookupConstructor(constructorName);
+      if (constructorName == cls.name
           && constructor === null
           && node.send.argumentsNode.isEmpty()) {
         constructor = cls.getSynthesizedConstructor();
       }
       if (constructor === null) {
-        error(node, MessageKind.CANNOT_FIND_CONSTRUCTOR, [node]);
+        error(node.send, MessageKind.CANNOT_FIND_CONSTRUCTOR, [node.send]);
+      } else {
+        FunctionExpression fun = constructor.parseNode(compiler, compiler);

[ngeoffray, 2012/01/19 08:56:12]

Same comment for resolveSignature.

[karlklose, 2012/01/19 13:51:24]

Done.

+        // TODO(karlklose): handle optional arguments.
+        if (node.send.argumentCount() != fun.parameterCount()) {
+          error(node.send, MessageKind.CANNOT_FIND_CONSTRUCTOR, [node.send]);
+        }
       }
+    } else {
+      Node selector = node.send.selector;
+      error(selector, MessageKind.CANNOT_RESOLVE_TYPE, [selector]);
     }
-
     useElement(node.send, constructor);
     return null;
   }