Fix several bugs in closure conversion.

runtime/vm/kernel_binary_flowgraph.cc

Index: runtime/vm/kernel_binary_flowgraph.cc
diff --git a/runtime/vm/kernel_binary_flowgraph.cc b/runtime/vm/kernel_binary_flowgraph.cc
index eb976d501057907c862ded382cf5c70f9c8cdba2..5f07ae774fa80ba237fb8a765b1fd8d2d0a9f63d 100644
--- a/runtime/vm/kernel_binary_flowgraph.cc
+++ b/runtime/vm/kernel_binary_flowgraph.cc
@@ -1197,15 +1197,15 @@ void StreamingScopeBuilder::VisitStatement() {
     case kWhileStatement:
       ++depth_.loop_;
       builder_->ReadPosition();  // read position.
-      VisitExpression();  // read condition.
-      VisitStatement();   // read body.
+      VisitExpression();         // read condition.
+      VisitStatement();          // read body.
       --depth_.loop_;
       return;
     case kDoStatement:
       ++depth_.loop_;
       builder_->ReadPosition();  // read position.
-      VisitStatement();   // read body.
-      VisitExpression();  // read condition.
+      VisitStatement();          // read body.
+      VisitExpression();         // read condition.
       --depth_.loop_;
       return;
     case kForStatement: {
@@ -3424,6 +3424,17 @@ FlowGraph* StreamingFlowGraphBuilder::BuildGraphOfImplicitClosureFunction(
                 flow_graph_builder_->next_block_id_ - 1);
 }
 
+LocalVariable* StreamingFlowGraphBuilder::LookupParameterDirect(
+    intptr_t kernel_offset,
+    intptr_t parameter_index) {
+  LocalVariable* var = LookupVariable(kernel_offset);
+  LocalVariable* parameter =
+      new (Z) LocalVariable(TokenPosition::kNoSource, TokenPosition::kNoSource,
+                            Symbols::TempParam(), var->type());
+  parameter->set_index(parameter_index);
+  if (var->is_captured()) parameter->set_is_captured_parameter(true);
+  return parameter;
+}
 // This method follows the logic of
 // StreamingFlowGraphBuilder::BuildGraphOfImplicitClosureFunction.  For
 // additional details on converted closure functions, please, see the comment on
@@ -3464,8 +3475,10 @@ FlowGraph* StreamingFlowGraphBuilder::BuildGraphOfConvertedClosureFunction(
   // closures its context field contains the context vector that is used by the
   // converted top-level function (target) explicitly and that should be passed
   // to that function as the first parameter.
-  body += LoadLocal(LookupVariable(
-      ReaderOffset() + relative_kernel_offset_));  // 0th variable offset.
+  intptr_t parameter_index = parsed_function()->first_parameter_index();
+  LocalVariable* parameter = LookupParameterDirect(
+      ReaderOffset() + relative_kernel_offset_, parameter_index--);
+  body += LoadLocal(parameter);  // 0th variable offset.
   body += flow_graph_builder_->LoadField(Closure::context_offset());
   LocalVariable* context = MakeTemporary();
 
@@ -3482,8 +3495,9 @@ FlowGraph* StreamingFlowGraphBuilder::BuildGraphOfConvertedClosureFunction(
   // The rest of the parameters are the same for the method of the Closure class
   // being invoked and the top-level function (target).
   for (intptr_t i = 1; i < positional_argument_count; i++) {
-    body += LoadLocal(LookupVariable(
-        ReaderOffset() + relative_kernel_offset_));  // ith variable offset.
+    LocalVariable* parameter = LookupParameterDirect(

[Dmitry Stefantsov, 2017/08/24 15:07:29]

Good catch!

+        ReaderOffset() + relative_kernel_offset_, parameter_index--);
+    body += LoadLocal(parameter);  // ith variable offset.
     body += PushArgument();
     SkipVariableDeclaration();  // read ith variable.
   }
@@ -3545,6 +3559,7 @@ FlowGraph* StreamingFlowGraphBuilder::BuildGraphOfFunction(bool constructor) {
     LocalScope* scope = parsed_function()->node_sequence()->scope();
     intptr_t parameter_count = dart_function.NumParameters();
     intptr_t parameter_index = parsed_function()->first_parameter_index();
+
     for (intptr_t i = 0; i < parameter_count; ++i, --parameter_index) {
       LocalVariable* variable = scope->VariableAt(i);
       if (variable->is_captured()) {
@@ -4310,16 +4325,16 @@ void StreamingFlowGraphBuilder::SkipExpression() {
       SkipExpression();  // read expression.
       return;
     case kPropertyGet:
-      ReadPosition();    // read position.
-      SkipExpression();  // read receiver.
-      SkipName();        // read name.
+      ReadPosition();                // read position.
+      SkipExpression();              // read receiver.
+      SkipName();                    // read name.
       SkipCanonicalNameReference();  // read interface_target_reference.
       return;
     case kPropertySet:
-      ReadPosition();    // read position.
-      SkipExpression();  // read receiver.
-      SkipName();        // read name.
-      SkipExpression();  // read value.
+      ReadPosition();                // read position.
+      SkipExpression();              // read receiver.
+      SkipName();                    // read name.
+      SkipExpression();              // read value.
       SkipCanonicalNameReference();  // read interface_target_reference.
       return;
     case kDirectPropertyGet:
@@ -4343,10 +4358,10 @@ void StreamingFlowGraphBuilder::SkipExpression() {
       SkipExpression();              // read expression.
       return;
     case kMethodInvocation:
-      ReadPosition();    // read position.
-      SkipExpression();  // read receiver.
-      SkipName();        // read name.
-      SkipArguments();   // read arguments.
+      ReadPosition();                // read position.
+      SkipExpression();              // read receiver.
+      SkipName();                    // read name.
+      SkipArguments();               // read arguments.
       SkipCanonicalNameReference();  // read interface_target_reference.
       return;
     case kDirectMethodInvocation:
@@ -5180,9 +5195,9 @@ Fragment StreamingFlowGraphBuilder::TranslateCondition(bool* negate) {
 }
 
 const TypeArguments& StreamingFlowGraphBuilder::BuildTypeArguments() {
-  ReadUInt();                                // read arguments count.
-  intptr_t type_count = ReadListLength();    // read type count.
-  return T.BuildTypeArguments(type_count);   // read types.
+  ReadUInt();                               // read arguments count.
+  intptr_t type_count = ReadListLength();   // read type count.
+  return T.BuildTypeArguments(type_count);  // read types.
 }
 
 Fragment StreamingFlowGraphBuilder::BuildArguments(Array* argument_names,
@@ -5221,7 +5236,7 @@ Fragment StreamingFlowGraphBuilder::BuildArgumentsFromActualArguments(
   }
   for (intptr_t i = 0; i < list_length; ++i) {
     String& name = H.DartSymbol(ReadStringReference());  // read ith name index.
-    instructions += BuildExpression();        // read ith expression.
+    instructions += BuildExpression();                   // read ith expression.
     if (!skip_push_arguments) instructions += PushArgument();
     if (do_drop) instructions += Drop();
     if (argument_names != NULL) {