Implement super initializers.

frog/leg/ssa/builder.dart

Index: frog/leg/ssa/builder.dart
diff --git a/frog/leg/ssa/builder.dart b/frog/leg/ssa/builder.dart
index 2b775977c74ef1969cb3798c52f6743c6ccb4fc6..e2503881d77568058d32add1a9efc2df44208d4a 100644
--- a/frog/leg/ssa/builder.dart
+++ b/frog/leg/ssa/builder.dart
@@ -129,32 +129,20 @@ class SsaBuilderTask extends CompilerTask {
 
   HGraph compileConstructor(SsaBuilder builder, WorkItem work) {
     // The body of the constructor will be generated in a separate function.
-    ClassElement classElement = work.element.enclosingElement;
-    ConstructorBodyElement bodyElement;
-    // In case of a bailout version, the constructor body has already
-    // been created.
-    if (work.isBailoutVersion()) {
-      for (Link<Element> backendMembers = classElement.backendMembers;
-           !backendMembers.isEmpty();
-           backendMembers = backendMembers.tail) {
-        Element current = backendMembers.head;
-        if (current.kind == ElementKind.GENERATIVE_CONSTRUCTOR_BODY) {
-          ConstructorBodyElement temp = current;
-          if (temp.constructor == work.element) {
-            bodyElement = temp;
-            break;
-          }
-        }
-      }
-    } else {
-      bodyElement = new ConstructorBodyElement(work.element);
-      compiler.enqueue(
-          new WorkItem.toCodegen(bodyElement, work.resolutionTree));
-      classElement.backendMembers =
-          classElement.backendMembers.prepend(bodyElement);
-    }
-    // TODO(floitsch): pass initializer-list to builder.
-    return builder.buildFactory(classElement, bodyElement, work.element);
+    final ClassElement classElement = work.element.enclosingElement;
+    return builder.buildFactory(classElement, work.element);
+  }
+
+  HGraph compileConstructorBody(SsaBuilder builder,

[floitsch, 2012/02/07 17:19:37]

remove this method.

[karlklose, 2012/02/08 14:12:00]

Done.

+                                FunctionElement element,
+                                TreeElements elements) {
+    return builder.buildMethod(element);
+  }
+
+  HGraph compileMethod(SsaBuilder builder,

[floitsch, 2012/02/07 17:19:38]

ditto.

[karlklose, 2012/02/08 14:12:00]

Done.

+                       FunctionElement element,
+                       TreeElements elements) {
+    return builder.buildMethod(element);
   }
 }
 
@@ -411,7 +399,7 @@ class LocalsHandler {
 
 class SsaBuilder implements Visitor {
   final Compiler compiler;
-  final TreeElements elements;
+  TreeElements elements;
   final Interceptors interceptors;
   bool methodInterceptionEnabled;
   HGraph graph;
@@ -450,24 +438,133 @@ class SsaBuilder implements Visitor {
     return closeFunction();
   }
 
+  /**
+   * Returns the constructor body associated with the given constructor or
+   * creates a new constructor body, if none can be found.
+   */
+  ConstructorBodyElement getConstructorBody(ClassElement classElement,
+                                            FunctionElement constructor) {
+    assert(constructor.kind === ElementKind.GENERATIVE_CONSTRUCTOR);
+    ConstructorBodyElement bodyElement;
+    for (Link<Element> backendMembers = classElement.backendMembers;
+         !backendMembers.isEmpty();
+         backendMembers = backendMembers.tail) {
+      Element backendMember = backendMembers.head;
+      if (backendMember.kind == ElementKind.GENERATIVE_CONSTRUCTOR_BODY) {
+        ConstructorBodyElement body = backendMember;
+        if (body.constructor== constructor) {

[ngeoffray, 2012/02/07 14:25:59]

space missing before '=='

[karlklose, 2012/02/08 14:12:00]

Done.

+          bodyElement = backendMember;
+          break;
+        }
+      }
+    }
+    if (bodyElement === null) {
+      bodyElement = new ConstructorBodyElement(constructor);
+      compiler.enqueue(

[floitsch, 2012/02/07 17:19:38]

one line?

[karlklose, 2012/02/08 14:12:00]

Done.

+          new WorkItem.toCompile(bodyElement));
+      classElement.backendMembers =
+          classElement.backendMembers.prepend(bodyElement);
+    }
+    assert(bodyElement !== null &&

[floitsch, 2012/02/07 17:19:38]

bodyElement is trivially non null.
I would remove the first part of the assert. But it's not hurting anybody.

[karlklose, 2012/02/08 14:12:00]

Done.

+           bodyElement.kind === ElementKind.GENERATIVE_CONSTRUCTOR_BODY);
+    return bodyElement;
+  }
+
+  /**
+   * Call [forEach] for every argument and parameter and element of [target]

[ngeoffray, 2012/02/07 14:25:59]

Shouldn't it just be 'argument'? If there are remaining parameters you're not
doing anything with them.

Also, there is an addStaticSendArgumentsToList already in that file. Maybe you
could use this method instead, because it looks like it's the same functionality
that is needed (matching a statically resolved call to the method definition).

[karlklose, 2012/02/08 14:12:00]

Done. I do not use environments anymore, so addStatucSendArgumentsToList is not
usable directly.

+   * that is used in the invocation [send].
+   */
+  forEachArgument(Send send, FunctionElement target,
+                  forEach(VariableElement parameter, Node argument)) {

[floitsch, 2012/02/07 17:19:38]

please name to something else. Even 'f' would be better imho.

[karlklose, 2012/02/08 14:12:00]

Done.

+    final FunctionParameters parameters = target.computeParameters(compiler);
+    Link<Element> parameterElements = parameters.requiredParameters;
+    for (Link<Node> arguments = send.arguments;
+         !arguments.isEmpty();
+         arguments = arguments.tail) {
+      if (parameterElements.isEmpty()) {
+        parameterElements = parameters.optionalParameters;
+      }
+      forEach(parameterElements.head, arguments.head);
+      parameterElements = parameterElements.tail;
+    };
+  }
+
+  /**
+   * Build the factory function corresponding to the constructor [bodyElement]:

[floitsch, 2012/02/07 17:19:38]

s/bodyElement/functionElement.

[karlklose, 2012/02/08 14:12:00]

Done.

+   *  - Initialize fields with the values of the field initializers of the
+   *    current constructor and super constructors or constructors redirected
+   *    to, starting from the current constructor.
+   *  - Call the the constructor bodies, starting from the constructor(s) in the
+   *    super class(es).
+   */
   HGraph buildFactory(ClassElement classElement,
-                      ConstructorBodyElement bodyElement,
                       FunctionElement functionElement) {
     FunctionExpression function = functionElement.parseNode(compiler);
     // The initializer list could contain closures.
     openFunction(functionElement, function);
 
-    NodeList initializers = function.initializers;
-
-    // Run through the initializers.
-    if (initializers !== null) {
+    final Map<FunctionElement, TreeElements> constructorElements =

[ngeoffray, 2012/02/07 14:25:59]

Why is this one final and not the others? For consistency, I would not put final
on local variables.

Also, this variable is unused.

[karlklose, 2012/02/08 14:12:00]

Done, I removed unused variables and the environments.

+        compiler.resolver.constructorElements;
+    List<FunctionElement> constructors = new List<FunctionElement>();
+    Map<FunctionElement, List<HInstruction>> environments =
+        new Map<FunctionElement, List<HInstruction>>();

[ngeoffray, 2012/02/07 14:25:59]

If this method was a LinkedHashMap, would you need the constructors list?

[karlklose, 2012/02/08 14:12:00]

see above.

+
+    // Prepare initial environment used to call body of this constructor.
+    FunctionElement next = functionElement;

[ngeoffray, 2012/02/07 14:25:59]

next -> nextSuperCall ?

[karlklose, 2012/02/08 14:12:00]

Changed to nextConstructor.

+    List<HInstruction> initialEnvironment = new List<HInstruction>();
+    functionElement.functionParameters.forEachParameter((parameter) {
+        initialEnvironment.addLast(localsHandler.readLocal(parameter));
+      });

[ngeoffray, 2012/02/07 14:25:59]

strange indentation.

[karlklose, 2012/02/08 14:12:00]

Removed.

+    environments[functionElement] = initialEnvironment;
+    constructors.addLast(functionElement);
+
+    // Analyze the constructor and all referenced constructors and collect
+    // initializers and constructor bodies.
+    while (next != null) {
+      final FunctionElement constructor = next;
+      next = null;
+      elements = compiler.resolver.resolveMethodElement(constructor);
+      compiler.enqueue(new WorkItem.toCompile(constructor));

[floitsch, 2012/02/07 17:19:38]

remove enqueuing here.

[karlklose, 2012/02/08 14:12:00]

Done.

+      final FunctionExpression functionNode = constructor.parseNode(compiler);
+      final NodeList initializers = functionNode.initializers;
+
+      if (initializers === null) continue;

[ngeoffray, 2012/02/07 14:25:59]

break?

[floitsch, 2012/02/07 17:19:38]

We have to find the implicit super-constructor instead.

[karlklose, 2012/02/08 14:12:00]

Done.

[karlklose, 2012/02/08 14:12:00]

Changed to set initializers to an empty list.

+
+      // Run through the initializers. Collect all field initializers and
+      // analyze referenced superinitializers and constructor redirections.

[ngeoffray, 2012/02/07 14:25:59]

Maybe add a function for the code running through the initializers, that return
the nextSuperCall ?

[karlklose, 2012/02/08 14:12:00]

Done.

       for (Link<Node> link = initializers.nodes;

[floitsch, 2012/02/07 17:19:38]

once you evaluate the initializers with the current localsHandler you could move
this code into a function.

[karlklose, 2012/02/08 14:12:00]

Done.

            !link.isEmpty();
            link = link.tail) {
         assert(link.head is Send);
+
         if (link.head is !SendSet) {
-          compiler.unimplemented('SsaBuilder.buildFactory super-init');
+          Send call = link.head;
+          // A super initializer or constructor redirection.

[floitsch, 2012/02/07 17:19:38]

move comment one line up.

[karlklose, 2012/02/08 14:12:00]

Done.

+          if (Initializers.isSuperConstructorCall(link.head)) {

[floitsch, 2012/02/07 17:19:38]

s/link.head/call

[karlklose, 2012/02/08 14:12:00]

Done.

+            FunctionElement superconstructor = elements[link.head];

[floitsch, 2012/02/07 17:19:38]

superConstructor and s/link.head/call.

[karlklose, 2012/02/08 14:12:00]

Done.

+            FunctionExpression superNode = superconstructor.parseNode(compiler);

[ngeoffray, 2012/02/07 14:25:59]

Unused variable.

[karlklose, 2012/02/08 14:12:00]

Done.

+            List<HInstruction> environment = new List<HInstruction>();
+            Link<Element> argumentElements =
+              superconstructor.functionParameters.requiredParameters;

[ngeoffray, 2012/02/07 14:25:59]

This variable seems unnecessary.

[karlklose, 2012/02/08 14:12:00]

Done.

+            // Visit arguments and map the corresponding parameter value to
+            // the resulting HInstruction value.
+            forEachArgument(call, superconstructor, (parameter, node) {
+              visit(node);
+              HInstruction value = pop();
+              updateElementDefinition(parameter, value);
+              // Store environment for invocation of constructor body.
+              environment.addLast(value);
+              argumentElements = argumentElements.tail;
+            });
+            // Setup constructor and environment for later and process it next.
+            constructors.addLast(superconstructor);
+            environments[superconstructor] = environment;
+            next = superconstructor;
+          } else {
+            compiler.unimplemented('SsaBuilder.buildFactory redirect');
+          }
         } else {
+          // A field initializer.
           SendSet init = link.head;
           Link<Node> arguments = init.arguments;
           assert(!arguments.isEmpty() && arguments.tail.isEmpty());
@@ -479,37 +576,44 @@ class SsaBuilder implements Visitor {
         }
       }
     }
-
     // Call the JavaScript constructor with the fields as argument.
-    // TODO(floitsch): allow super calls.
-    // TODO(floitsch): allow inits at field declarations.
     List<HInstruction> constructorArguments = <HInstruction>[];
-    for (Element member in classElement.members) {
-      if (member.isInstanceMember() && member.kind == ElementKind.FIELD) {
+    ClassElement element = classElement;
+    while (element != null) {

[floitsch, 2012/02/07 17:19:38]

add TODO to share this code with the emitter. There should be a function on the
ClassElement.

[karlklose, 2012/02/08 14:12:00]

Done.

+      for (Element member in element.members) {
+        if (member.isInstanceMember() && member.kind == ElementKind.FIELD) {
         HInstruction value;
         if (localsHandler.hasValueForDirectLocal(member)) {
           value = localsHandler.readLocal(member);
         } else {
-          value = new HLiteral(null, HType.UNKNOWN);
-          add(value);
-        }
+            value = new HLiteral(null, HType.UNKNOWN);

[floitsch, 2012/02/07 17:19:38]

Add TODO that we need to get the default-values if there are some.

[karlklose, 2012/02/08 14:12:00]

Done.

+            add(value);
+          }
         constructorArguments.add(value);

[ngeoffray, 2012/02/07 14:25:59]

strange indentation.

[karlklose, 2012/02/08 14:12:00]

Done.

+        }
       }
+      element = element.superclass;
     }
     HForeignNew newObject = new HForeignNew(classElement, constructorArguments);
     add(newObject);
-
-    // Call the method body.
-    SourceString methodName = bodyElement.name;
-
-    List bodyCallInputs = <HInstruction>[];
-    bodyCallInputs.add(newObject);
-    FunctionParameters parameters = functionElement.computeParameters(compiler);
-    parameters.forEachParameter((Element parameterElement) {
-      HInstruction currentValue = localsHandler.readLocal(parameterElement);
-      bodyCallInputs.add(currentValue);
-    });
-    add(new HInvokeDynamicMethod(null, methodName, bodyCallInputs));
+    // Generate calls to the constructor bodies.
+    for (int index = constructors.length - 1; index >= 0; index--) {
+      FunctionElement constructor = constructors[index];
+      List bodyCallInputs = <HInstruction>[];
+      bodyCallInputs.add(newObject);
+      int argumentCount = 0;
+      List<HInstruction> environment = environments[constructor];
+      for (HInstruction value in environment) {

[ngeoffray, 2012/02/07 14:25:59]

Instead of creating a new list and adding the instructions, you could reuse the
same list, and make sure you have an empty slot at the beginning.

[karlklose, 2012/02/08 14:12:00]

I removed the environments.

+        bodyCallInputs.add(value);
+        argumentCount++;
+      }
+      Selector selector = new Invocation(argumentCount);
+      ConstructorBodyElement body = this.getConstructorBody(classElement,
+                                                            constructor);
+      // Call the method body.
+      SourceString methodName = body.name;
+      add(new HInvokeDynamicMethod(selector, methodName, bodyCallInputs));

[ngeoffray, 2012/02/07 14:25:59]

I believe you don't need a selector for this HInvokeDynamic, why did you add
one?

[karlklose, 2012/02/08 14:12:00]

Removed.

+    }
     close(new HReturn(newObject)).addSuccessor(graph.exit);
     return closeFunction();
   }
@@ -1368,6 +1472,11 @@ class SsaBuilder implements Visitor {
 
   visitNewExpression(NewExpression node) => visitSend(node.send);
 
+  HInstruction updateElementDefinition(Element element, HInstruction value) {

[ngeoffray, 2012/02/07 14:25:59]

Since this method is only used once, I would inline it.

[karlklose, 2012/02/08 14:12:00]

Done.

+    localsHandler.updateLocal(element, value);
+    return value;
+  }
+
   visitSendSet(SendSet node) {
     Operator op = node.assignmentOperator;
     if (node.isIndex) {