Implement super initializers.

frog/leg/ssa/builder.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.
 
 class Interceptors {
   Compiler compiler;
   Interceptors(Compiler this.compiler);
 
   SourceString mapOperatorToMethodName(Operator op) {
     String name = op.source.stringValue;
@@ skipping 111 matching lines @@
         }
         new HTracer.singleton().traceCompilation(name);
         new HTracer.singleton().traceGraph('builder', graph);
       }
       return graph;
     });
   }
 
   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);
   }
 }
 
 /**
  * Keeps track of locals (including parameters and phis) when building. The
  * 'this' reference is treated as parameter and hence handled by this class,
  * too.
  */
 class LocalsHandler {
   // The values of locals that can be directly accessed (without redirections
@@ skipping 236 matching lines @@
         joinBlock.addPhi(phi);
         joinedLocals[element] = phi;
       }
     });
     directLocals = joinedLocals;
   }
 }
 
 class SsaBuilder implements Visitor {
   final Compiler compiler;
-  final TreeElements elements;
+  TreeElements elements;
   final Interceptors interceptors;
   bool methodInterceptionEnabled;
   HGraph graph;
   LocalsHandler localsHandler;
 
   // We build the Ssa graph by simulating a stack machine.
   List<HInstruction> stack;
 
   // The current block to add instructions to. Might be null, if we are
   // visiting dead code.
@@ skipping 18 matching lines @@
     methodInterceptionEnabled = true;
   }
 
   HGraph buildMethod(FunctionElement functionElement) {
     FunctionExpression function = functionElement.parseNode(compiler);
     openFunction(functionElement, function);
     function.body.accept(this);
     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;
+    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.

 
-    // Run through the initializers.
-    if (initializers !== null) {
+    // 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());
           visit(arguments.head);
           // We treat the init field-elements like locals. In the context of
           // the factory this is correct, and simplifies dealing with
           // parameter-initializers (like A(this.x)).
           localsHandler.updateLocal(elements[init], pop());
         }
       }
     }
-
     // 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.

         }
-        constructorArguments.add(value);
       }
+      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();
   }
 
   void openFunction(FunctionElement functionElement,
                     FunctionExpression node) {
     HBasicBlock block = graph.addNewBlock();
     open(graph.entry);
 
     localsHandler.startFunction(functionElement, node);
@@ skipping 838 matching lines @@
         // static call to a factory.
         visitStaticSend(node);
       } else {
         compiler.internalError("Cannot generate code for send", node: node);
       }
     }
   }
 
   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) {
       if (!methodInterceptionEnabled) {
         assert(op.source.stringValue === '=');
         visitDynamicSend(node);
       } else {
         HStatic target = new HStatic(
             interceptors.getIndexAssignmentInterceptor());
         add(target);
@@ skipping 330 matching lines @@
   }
 
   visitCatchBlock(CatchBlock node) {
     compiler.unimplemented('SsaBuilder.visitCatchBlock', node: node);
   }
 
   visitTypedef(Typedef node) {
     compiler.unimplemented('SsaBuilder.visitTypedef', node: node);
   }
 }