Support allocating and calling closures in the new non-optimizing compiler.

runtime/vm/flow_graph_builder.cc

Index: runtime/vm/flow_graph_builder.cc
diff --git a/runtime/vm/flow_graph_builder.cc b/runtime/vm/flow_graph_builder.cc
index 1aaf8bfce5675f29284ecc16b6ca004cb90267d6..3fd14138744e739117e5a005b2362b7dffccd670 100644
--- a/runtime/vm/flow_graph_builder.cc
+++ b/runtime/vm/flow_graph_builder.cc
@@ -807,7 +807,38 @@ void EffectGraphVisitor::VisitArrayNode(ArrayNode* node) {
 
 
 void EffectGraphVisitor::VisitClosureNode(ClosureNode* node) {
-  Bailout("EffectGraphVisitor::VisitClosureNode");
+  const Function& function = node->function();
+
+  int next_index = temp_index();
+  if (function.IsNonImplicitClosureFunction()) {

[srdjan, 2012/03/12 18:35:23]

This condition does not match what is in the (changed) code_generator_xxx.cc

+    const int context_level = 0;  // Only because we don't handle nesting yet.

[srdjan, 2012/03/12 18:35:23]

Dont you need to check that state()->context_level() == 0?

[Kevin Millikin (Google), 2012/03/13 08:59:50]

Eventually, or handle non-zero.  There is no context level tracking in this
compiler, yet.

+    const ContextScope& context_scope = ContextScope::ZoneHandle(
+        node->scope()->PreserveOuterScope(context_level));
+    ASSERT(!function.HasCode());
+    ASSERT(function.context_scope() == ContextScope::null());
+    function.set_context_scope(context_scope);
+  } else if (function.IsImplicitInstanceClosureFunction()) {
+    ValueGraphVisitor for_receiver(owner(), temp_index());
+    node->receiver()->Visit(&for_receiver);
+    Append(for_receiver);
+    if (!for_receiver.value()->IsTemp()) {
+      AddInstruction(new BindInstr(temp_index(), for_receiver.value()));
+    }
+    ++next_index;
+  }
+  ASSERT(function.context_scope() != ContextScope::null());
+
+  // The function type of a closure may have type arguments. In that case, pass
+  // the type arguments of the instantiator.
+  const Class& cls = Class::Handle(function.signature_class());
+  ASSERT(!cls.IsNull());
+  const bool requires_type_arguments = cls.HasTypeArguments();
+  if (requires_type_arguments) {
+    Bailout("Closure creation requiring type arguments");
+  }
+
+  CreateClosureComp* create = new CreateClosureComp(node);
+  ReturnComputation(create);
 }
 
 
@@ -859,7 +890,22 @@ void EffectGraphVisitor::VisitStaticCallNode(StaticCallNode* node) {
 
 
 void EffectGraphVisitor::VisitClosureCallNode(ClosureCallNode* node) {
-  Bailout("EffectGraphVisitor::VisitClosureCallNode");
+  // Context is saved around the call, it's treated as an extra operand
+  // consumed by the call (but not an argument).
+  AddInstruction(new BindInstr(temp_index(), new CurrentContextComp()));
+
+  ArgumentGraphVisitor for_closure(owner(), temp_index() + 1);
+  node->closure()->Visit(&for_closure);
+  Append(for_closure);
+
+  ZoneGrowableArray<Value*>* arguments =
+      new ZoneGrowableArray<Value*>(node->arguments()->length());
+  arguments->Add(for_closure.value());
+  TranslateArgumentList(*node->arguments(), temp_index() + 2, arguments);
+  // First operand is the saved context, consumed by the call.
+  ClosureCallComp* call =
+      new ClosureCallComp(node, new TempVal(temp_index()), arguments);
+  ReturnComputation(call);
 }
 
 
@@ -1235,9 +1281,25 @@ void FlowGraphPrinter::VisitAssertAssignable(AssertAssignableComp* comp) {
 }
 
 
+void FlowGraphPrinter::VisitCurrentContext(CurrentContextComp* comp) {
+  OS::Print("CurrentContext");
+}
+
+
+void FlowGraphPrinter::VisitClosureCall(ClosureCallComp* comp) {
+  OS::Print("ClosureCall(");
+  comp->context()->Accept(this);
+  for (intptr_t i = 0; i < comp->ArgumentCount(); ++i) {
+    OS::Print(", ");
+    comp->ArgumentAt(i)->Accept(this);
+  }
+  OS::Print(")");
+}
+
+
 void FlowGraphPrinter::VisitInstanceCall(InstanceCallComp* comp) {
   OS::Print("InstanceCall(%s", comp->function_name().ToCString());
-  for (int i = 0; i < comp->ArgumentCount(); ++i) {
+  for (intptr_t i = 0; i < comp->ArgumentCount(); ++i) {
     OS::Print(", ");
     comp->ArgumentAt(i)->Accept(this);
   }
@@ -1258,7 +1320,7 @@ void FlowGraphPrinter::VisitStrictCompare(StrictCompareComp* comp) {
 void FlowGraphPrinter::VisitStaticCall(StaticCallComp* comp) {
   OS::Print("StaticCall(%s",
             String::Handle(comp->function().name()).ToCString());
-  for (int i = 0; i < comp->ArgumentCount(); ++i) {
+  for (intptr_t i = 0; i < comp->ArgumentCount(); ++i) {
     OS::Print(", ");
     comp->ArgumentAt(i)->Accept(this);
   }
@@ -1370,6 +1432,11 @@ void FlowGraphPrinter::VisitCreateArray(CreateArrayComp* comp) {
 }
 
 
+void FlowGraphPrinter::VisitCreateClosure(CreateClosureComp* comp) {
+  OS::Print("CreateClosure(%s)", comp->function().ToCString());
+}
+
+
 void FlowGraphPrinter::VisitJoinEntry(JoinEntryInstr* instr) {
   OS::Print("%2d: [join]", instr->block_number());
 }
@@ -1419,7 +1486,12 @@ void FlowGraphPrinter::VisitBranch(BranchInstr* instr) {
 void FlowGraphBuilder::BuildGraph() {
   if (FLAG_print_ast) {
     // Print the function ast before IL generation.
-    AstPrinter::PrintFunctionNodes(parsed_function_);
+    AstPrinter::PrintFunctionNodes(parsed_function());
+  }
+  // Allocate variables before constructing the graph.

[Kevin Millikin (Google), 2012/03/12 14:06:36]

This comment is out of date, I'm deleting it.

+  const Function& function = parsed_function().function();
+  if ((function.num_optional_parameters() != 0)) {
+    Bailout("function has optional parameters");
   }

[srdjan, 2012/03/12 19:56:17]

Why do we bailout here and not in the FlowGraphCompiler.

[Kevin Millikin (Google), 2012/03/13 08:59:50]

I moved the bailout because I initially moved the variable allocation here
(needed for graph translation of closures and context variables correctly, in
the very back end is too late); and because the graph translation assumes (right
now) that we have already bailed out so it doesn't set an invalid context scope
on a function, that we could potentially observe if we bail out of this
compiler. 

When I realized that there was no good way to safely repeat the variable
allocation (it mutates variables, which are in the AST shared by the other
compiler, and so a pain in the butt to clear), I moved it out of the compilers.

It still seems like a good idea to bail out earlier for global properties like
this, rather than translate the graph and bail out much later.  It is safer and
easier to prototype this way---the graph translation can assume that we have
bailed out already in many cases.

   EffectGraphVisitor for_effect(this, 0);
   for_effect.AddInstruction(new TargetEntryInstr());