Add support for converted closures with explicit contexts to VM

runtime/vm/kernel_binary_flowgraph.cc

 // Copyright (c) 2016, 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.
 
 #include "vm/kernel_binary_flowgraph.h"
 
 #include "vm/compiler.h"
 #include "vm/longjump.h"
 #include "vm/object_store.h"
 
@@ skipping 103 matching lines @@
   scope_->AddVariable(context_var);
 
   parsed_function->SetNodeSequence(
       new SequenceNode(TokenPosition::kNoSource, scope_));
 
   intptr_t parent_offset = -1;
   builder_->SetOffset(kernel_offset_);
 
   switch (function.kind()) {
     case RawFunction::kClosureFunction:
+    case RawFunction::kConvertedClosureFunction:
     case RawFunction::kRegularFunction:
     case RawFunction::kGetterFunction:
     case RawFunction::kSetterFunction:
     case RawFunction::kConstructor: {
       const Tag tag = builder_->PeekTag();
       parent_offset = builder_->ReadUntilFunctionNode();
       word async_marker_word;
       builder_->ReadFunctionNodeUntilTypeParameters(
           &unused_tokenposition, &unused_tokenposition, &async_marker_word,
           &unused_word);  // read first part of function node.
@@ skipping 218 matching lines @@
   Tag function_node = builder_->ReadProcedureUntilFunctionNode(
       &unused_word, &unused_intptr);  // read first part of procedure.
   if (function_node == kSomething) {
     VisitFunctionNode();
   }
 }
 
 void StreamingScopeBuilder::VisitField() {
   builder_->ReadFieldUntilAnnotation(
       &unused_nameindex, &unused_tokenposition, &unused_tokenposition,
-      &unused_intptr, &unused_word);       // read first part of field.
-  builder_->SkipListOfExpressions();       // read annotations.
-  VisitDartType();                         // read type.
-  Tag tag = builder_->ReadTag();           // read initializer (part 1).
+      &unused_intptr, &unused_word);  // read first part of field.
+  builder_->SkipListOfExpressions();  // read annotations.
+  VisitDartType();                    // read type.
+  Tag tag = builder_->ReadTag();      // read initializer (part 1).
   if (tag == kSomething) {
     VisitExpression();  // read initializer (part 2).
   }
 }
 
 void StreamingScopeBuilder::VisitFunctionNode() {
   word async_marker_word;
   word dart_async_marker_word;
   builder_->ReadFunctionNodeUntilTypeParameters(
       &unused_tokenposition, &unused_tokenposition, &async_marker_word,
@@ skipping 310 matching lines @@
       return;
     case kDoubleLiteral:
       builder_->SkipStringReference();  // read index into string table.
       return;
     case kTrueLiteral:
       return;
     case kFalseLiteral:
       return;
     case kNullLiteral:
       return;
+    case kVectorCreation:
+      builder_->ReadUInt();  // read size.
+      return;
+    case kVectorGet:
+      VisitExpression();     // read expression.
+      builder_->ReadUInt();  // read index.
+      return;
+    case kVectorSet:
+      VisitExpression();     // read vector expression.
+      builder_->ReadUInt();  // read index.
+      VisitExpression();     // read value.
+      return;
+    case kVectorCopy:
+      VisitExpression();  // read vector expression.
+      return;
+    case kClosureCreation:
+      builder_->SkipCanonicalNameReference();  // read function reference.
+      VisitExpression();                       // read context vector.
+      VisitDartType();  // read function type of the closure.
+      return;
     default:
       UNREACHABLE();
   }
 }
 
 void StreamingScopeBuilder::VisitStatement() {
   Tag tag = builder_->ReadTag();  // read tag.
   switch (tag) {
     case kInvalidStatement:
       return;
@@ skipping 319 matching lines @@
   result_->locals.Insert(kernel_offset_no_tag, variable);
 }
 
 void StreamingScopeBuilder::VisitDartType() {
   Tag tag = builder_->ReadTag();
   switch (tag) {
     case kInvalidType:
     case kDynamicType:
     case kVoidType:
     case kBottomType:
+    case kVectorType:
       // those contain nothing.
       return;
     case kInterfaceType:
       VisitInterfaceType(false);
       return;
     case kSimpleInterfaceType:
       VisitInterfaceType(true);
       return;
     case kFunctionType:
       VisitFunctionType(false);
@@ skipping 391 matching lines @@
           Error::Handle(Z),  // No previous error.
           dart::Script::Handle(Z, dart::Script::null()),
           TokenPosition::kNoSource, "[InvalidType] in Kernel IR.");
       break;
     case kDynamicType:
       result_ = Object::dynamic_type().raw();
       break;
     case kVoidType:
       result_ = Object::void_type().raw();
       break;
+    case kVectorType:
+      result_ = Object::vector_type().raw();
+      break;
     case kBottomType:
       result_ = dart::Class::Handle(Z, I->object_store()->null_class())
                     .CanonicalType();
       break;
     case kInterfaceType:
       BuildInterfaceType(false);
       break;
     case kSimpleInterfaceType:
       BuildInterfaceType(true);
       break;
@@ skipping 1678 matching lines @@
                      argument_names);
 
   // Return the result.
   body += Return(end_position);
 
   return new (Z)
       FlowGraph(*parsed_function(), flow_graph_builder_->graph_entry_,
                 flow_graph_builder_->next_block_id_ - 1);
 }
 
+
+// This method follows the logic similar to that of

[kustermann, 2017/06/20 12:13:14]

s/follows the logic .... of/follows the logic of/

[Dmitry Stefantsov, 2017/06/22 14:12:52]

Agreed.  It reads better.  Thanks!

+// StreamingFlowGraphBuilder::BuildGraphOfImplicitClosureFunction.  For
+// additional details on converted closure functions, please, see the comment on
+// the method Function::ConvertedClosureFunction.
+FlowGraph* StreamingFlowGraphBuilder::BuildGraphOfConvertedClosureFunction(
+    const Function& function) {

[kustermann, 2017/06/20 12:13:13]

Given the significant duplication of code, maybe consider just parametrizing the
old code?

[Dmitry Stefantsov, 2017/06/22 14:12:52]

I agree, we may benefit from the parametrization.  But I would rather do it in a
separate CL.  What are your thoughts, Martin?

[kustermann, 2017/06/26 10:32:36]

Just keep it then.

+  const Function& target = Function::ZoneHandle(Z, function.parent_function());
+
+  TargetEntryInstr* normal_entry = flow_graph_builder_->BuildTargetEntry();
+  flow_graph_builder_->graph_entry_ = new (Z) GraphEntryInstr(
+      *parsed_function(), normal_entry, Compiler::kNoOSRDeoptId);
+  SetupDefaultParameterValues();
+
+  Fragment body(normal_entry);
+  body += flow_graph_builder_->CheckStackOverflowInPrologue();
+
+  // Load all the arguments.
+  ASSERT(target.is_static());
+
+  TokenPosition end_position;
+  ReadFunctionNodeUntilTypeParameters(
+      &unused_tokenposition, &end_position, &unused_word,
+      &unused_word);         // read first part of function node.
+  SkipTypeParametersList();  // read type parameter list.
+  ReadUInt();                // read total parameter count.
+  ReadUInt();                // read required_parameter_count.
+
+  // Positional.
+  intptr_t positional_argument_count = ReadListLength();

[kustermann, 2017/06/20 12:13:14]

Usually we try to use "const" where possible. (also in other places in the CL)

[Dmitry Stefantsov, 2017/06/22 14:12:52]

Do you mean something like `const intptr_t positional_argument_count ...`?  I
can make this change, but this variable declaration is the same like the similar
one from `BuildGraphOfImplicitClosureConversion` that I took as the basis. 
Should I then also make the changes there?

[kustermann, 2017/06/26 10:32:36]

It's fine to just change your code. The benefit of const (or 'final' in dart) is
that one knows there's no other assignment to the variable.

+
+  // The first argument is the instance of the closure class.  For converted
+  // 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()));  // 0th variable offset.
+  body += flow_graph_builder_->LoadField(Closure::context_offset());
+  body += PushArgument();
+  SkipVariableDeclaration();  // read 0th variable.
+
+  // 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()));  // ith variable offset.
+    body += PushArgument();
+    SkipVariableDeclaration();  // read ith variable.
+  }
+
+  // Named.
+  intptr_t named_argument_count = ReadListLength();
+  Array& argument_names = Array::ZoneHandle(Z);
+  if (named_argument_count > 0) {
+    argument_names = Array::New(named_argument_count);
+    for (intptr_t i = 0; i < named_argument_count; i++) {
+      body +=
+          LoadLocal(LookupVariable(ReaderOffset()));  // ith variable offset.
+      body += PushArgument();
+      argument_names.SetAt(
+          i, H.DartSymbol(GetNameFromVariableDeclaration(ReaderOffset())));
+      SkipVariableDeclaration();  // read ith variable.
+    }
+  }
+
+  // Forward them to the target.
+  intptr_t argument_count = positional_argument_count + named_argument_count;
+  body += StaticCall(TokenPosition::kNoSource, target, argument_count,
+                     argument_names);
+
+  // Return the result.
+  body += Return(end_position);
+
+  return new (Z)
+      FlowGraph(*parsed_function(), flow_graph_builder_->graph_entry_,
+                flow_graph_builder_->next_block_id_ - 1);
+}
+
+
 static bool IsGetMainClosure(const String& name) {
   if (name.Length() < 16) return false;
   const char* cstr = "_getMainClosure@";
   for (intptr_t i = 0; i < 16; ++i) {
     if (name.CharAt(i) != cstr[i]) return false;
   }
   return true;
 }
 
 FlowGraph* StreamingFlowGraphBuilder::BuildGraphOfFunction(
@@ skipping 403 matching lines @@
   // will not need an AST.  The code generator will assume that there is a
   // local variable stack slot allocated for the current context and (I
   // think) that the runtime will expect it to be at a fixed offset which
   // requires allocating an unused expression temporary variable.
   set_scopes(parsed_function()->EnsureKernelScopes());
 
   SetOffset(kernel_offset);
 
   switch (function.kind()) {
     case RawFunction::kClosureFunction:
+    case RawFunction::kConvertedClosureFunction:
     case RawFunction::kRegularFunction:
     case RawFunction::kGetterFunction:
     case RawFunction::kSetterFunction: {
       ReadUntilFunctionNode();  // read until function node.
-      return function.IsImplicitClosureFunction()
-                 ? BuildGraphOfImplicitClosureFunction(function)
-                 : BuildGraphOfFunction(is_in_builtin_library_toplevel);
+      if (function.IsImplicitClosureFunction()) {
+        return BuildGraphOfImplicitClosureFunction(function);
+      } else if (function.IsConvertedClosureFunction()) {
+        return BuildGraphOfConvertedClosureFunction(function);
+      }
+      return BuildGraphOfFunction(is_in_builtin_library_toplevel);
     }
     case RawFunction::kConstructor: {
       bool is_factory = function.IsFactory();
       if (is_factory) {
         ReadUntilFunctionNode();  // read until function node.
         return BuildGraphOfFunction(is_in_builtin_library_toplevel);
       } else {
         // Constructor: Pass offset to parent class.
         return BuildGraphOfFunction(
             is_in_builtin_library_toplevel,
@@ skipping 110 matching lines @@
     case kPositiveIntLiteral:
       return BuildIntLiteral(false, position);
     case kDoubleLiteral:
       return BuildDoubleLiteral(position);
     case kTrueLiteral:
       return BuildBoolLiteral(true, position);
     case kFalseLiteral:
       return BuildBoolLiteral(false, position);
     case kNullLiteral:
       return BuildNullLiteral(position);
+    case kVectorCreation:
+      return BuildVectorCreation(position);
+    case kVectorGet:
+      return BuildVectorGet(position);
+    case kVectorSet:
+      return BuildVectorSet(position);
+    case kVectorCopy:
+      return BuildVectorCopy(position);
+    case kClosureCreation:
+      return BuildClosureCreation(position);
     default:
       UNREACHABLE();
   }
 
   return Fragment();
 }
 
 Fragment StreamingFlowGraphBuilder::BuildStatement() {
   Tag tag = ReadTag();  // read tag.
   switch (tag) {
@@ skipping 135 matching lines @@
   ReadUInt();
 }
 
 void StreamingFlowGraphBuilder::SkipDartType() {
   Tag tag = ReadTag();
   switch (tag) {
     case kInvalidType:
     case kDynamicType:
     case kVoidType:
     case kBottomType:
+    case kVectorType:
       // those contain nothing.
       return;
     case kInterfaceType:
       SkipInterfaceType(false);
       return;
     case kSimpleInterfaceType:
       SkipInterfaceType(true);
       return;
     case kFunctionType:
       SkipFunctionType(false);
@@ skipping 24 matching lines @@
 void StreamingFlowGraphBuilder::SkipInterfaceType(bool simple) {
   ReadUInt();  // read klass_name.
   if (!simple) {
     SkipListOfDartTypes();  // read list of types.
   }
 }
 
 void StreamingFlowGraphBuilder::SkipFunctionType(bool simple) {
   if (!simple) {
     SkipTypeParametersList();  // read type_parameters.
-    ReadUInt();  // read required parameter count.
-    ReadUInt();  // read total parameter count.
+    ReadUInt();                // read required parameter count.
+    ReadUInt();                // read total parameter count.
   }
 
   SkipListOfDartTypes();  // read positional_parameters types.
 
   if (!simple) {
     const intptr_t named_count =
         ReadListLength();  // read named_parameters list length.
     for (intptr_t i = 0; i < named_count; ++i) {
       // read string reference (i.e. named_parameters[i].name).
       SkipStringReference();
@@ skipping 128 matching lines @@
       SkipBytes(1);      // read operator.
       SkipExpression();  // read right.
       return;
     case kConditionalExpression:
       SkipExpression();        // read condition.
       SkipExpression();        // read then.
       SkipExpression();        // read otherwise.
       SkipOptionalDartType();  // read unused static type.
       return;
     case kStringConcatenation:
-      ReadPosition();                           // read position.
-      SkipListOfExpressions();                  // read list of expressions.
+      ReadPosition();           // read position.
+      SkipListOfExpressions();  // read list of expressions.
       return;
     case kIsExpression:
       ReadPosition();    // read position.
       SkipExpression();  // read operand.
       SkipDartType();    // read type.
       return;
     case kAsExpression:
       ReadPosition();    // read position.
       SkipExpression();  // read operand.
       SkipDartType();    // read type.
       return;
     case kSymbolLiteral:
       SkipStringReference();  // read index into string table.
       return;
     case kTypeLiteral:
       SkipDartType();  // read type.
       return;
     case kThisExpression:
       return;
     case kRethrow:
       ReadPosition();  // read position.
       return;
     case kThrow:
       ReadPosition();    // read position.
       SkipExpression();  // read expression.
       return;
     case kListLiteral:
     case kConstListLiteral:
-      ReadPosition();                           // read position.
-      SkipDartType();                           // read type.
-      SkipListOfExpressions();                  // read list of expressions.
+      ReadPosition();           // read position.
+      SkipDartType();           // read type.
+      SkipListOfExpressions();  // read list of expressions.
       return;
     case kMapLiteral:
     case kConstMapLiteral: {
       ReadPosition();                           // read position.
       SkipDartType();                           // read key type.
       SkipDartType();                           // read value type.
       intptr_t list_length = ReadListLength();  // read list length.
       for (intptr_t i = 0; i < list_length; ++i) {
         SkipExpression();  // read ith key.
         SkipExpression();  // read ith value.
       }
       return;
     }
     case kFunctionExpression:
       SkipFunctionNode();  // read function node.
       return;
     case kLet:
       SkipVariableDeclaration();  // read variable declaration.
       SkipExpression();           // read expression.
       return;
+    case kVectorCreation:
+      ReadUInt();  // read value.
+      return;
+    case kVectorGet:
+      SkipExpression();  // read vector expression.
+      ReadUInt();        // read index.
+      return;
+    case kVectorSet:
+      SkipExpression();  // read vector expression.
+      ReadUInt();        // read index.
+      SkipExpression();  // read value.
+      return;
+    case kVectorCopy:
+      SkipExpression();  // read vector expression.
+      return;
+    case kClosureCreation:
+      SkipCanonicalNameReference();  // read top-level function reference.
+      SkipExpression();              // read context vector.
+      SkipDartType();                // read function type.
+      return;
     case kBigIntLiteral:
       SkipStringReference();  // read string reference.
       return;
     case kStringLiteral:
       SkipStringReference();  // read string reference.
       return;
     case kSpecialIntLiteral:
       return;
     case kNegativeIntLiteral:
       ReadUInt();  // read value.
@@ skipping 52 matching lines @@
     case kWhileStatement:
       SkipExpression();  // read condition.
       SkipStatement();   // read body.
       return;
     case kDoStatement:
       SkipStatement();   // read body.
       SkipExpression();  // read condition.
       return;
     case kForStatement: {
       SkipListOfVariableDeclarations();  // read variables.
-      Tag tag = ReadTag();  // Read first part of condition.
+      Tag tag = ReadTag();               // Read first part of condition.
       if (tag == kSomething) {
         SkipExpression();  // read rest of condition.
       }
       SkipListOfExpressions();  // read updates.
-      SkipStatement();  // read body.
+      SkipStatement();          // read body.
       return;
     }
     case kForInStatement:
     case kAsyncForInStatement:
       ReadPosition();             // read position.
       SkipVariableDeclaration();  // read variable.
       SkipExpression();           // read iterable.
       SkipStatement();            // read body.
       return;
     case kSwitchStatement: {
@@ skipping 222 matching lines @@
 
 GrowableArray<YieldContinuation>&
 StreamingFlowGraphBuilder::yield_continuations() {
   return flow_graph_builder_->yield_continuations_;
 }
 
 Value* StreamingFlowGraphBuilder::stack() {
   return flow_graph_builder_->stack_;
 }
 
+void StreamingFlowGraphBuilder::Push(Definition* definition) {
+  flow_graph_builder_->Push(definition);
+}
+
 Value* StreamingFlowGraphBuilder::Pop() {
   return flow_graph_builder_->Pop();
 }
 
 Tag StreamingFlowGraphBuilder::PeekArgumentsFirstPositionalTag() {
   // read parts of arguments, then go back to before doing so.
   AlternativeReadingScope alt(reader_);
   ReadUInt();  // read number of arguments.
 
   SkipListOfDartTypes();  // Read list of types.
@@ skipping 16 matching lines @@
   intptr_t list_length = ReadListLength();  // read types list length.
   return T.BuildInstantiatedTypeArguments(klass, list_length);  // read types.
 }
 
 intptr_t StreamingFlowGraphBuilder::PeekArgumentsCount() {
   return PeekUInt();
 }
 
 intptr_t StreamingFlowGraphBuilder::PeekArgumentsTypeCount() {
   AlternativeReadingScope alt(reader_);
-  ReadUInt();                               // read arguments count.
-  return ReadListLength();                  // read length of types list.
+  ReadUInt();               // read arguments count.
+  return ReadListLength();  // read length of types list.
 }
 
 void StreamingFlowGraphBuilder::SkipArgumentsBeforeActualArguments() {
-  ReadUInt();  // read arguments count.
+  ReadUInt();             // read arguments count.
   SkipListOfDartTypes();  // read list of types.
 }
 
 LocalVariable* StreamingFlowGraphBuilder::LookupVariable(
     intptr_t kernel_offset) {
   return flow_graph_builder_->LookupVariable(kernel_offset);
 }
 
 LocalVariable* StreamingFlowGraphBuilder::MakeTemporary() {
   return flow_graph_builder_->MakeTemporary();
@@ skipping 104 matching lines @@
 Fragment StreamingFlowGraphBuilder::StrictCompare(Token::Kind kind,
                                                   bool number_check) {
   return flow_graph_builder_->StrictCompare(kind, number_check);
 }
 
 Fragment StreamingFlowGraphBuilder::AllocateObject(const dart::Class& klass,
                                                    intptr_t argument_count) {
   return flow_graph_builder_->AllocateObject(klass, argument_count);
 }
 
+Fragment StreamingFlowGraphBuilder::AllocateObject(
+    const dart::Class& klass,
+    const Function& closure_function) {
+  return flow_graph_builder_->AllocateObject(klass, closure_function);
+}
+
+Fragment StreamingFlowGraphBuilder::AllocateContext(int size) {
+  return flow_graph_builder_->AllocateContext(size);
+}
+
+Fragment StreamingFlowGraphBuilder::LoadField(intptr_t offset) {
+  return flow_graph_builder_->LoadField(offset);
+}
+
 Fragment StreamingFlowGraphBuilder::InstanceCall(TokenPosition position,
                                                  const dart::String& name,
                                                  Token::Kind kind,
                                                  intptr_t argument_count,
                                                  const Array& argument_names,
                                                  intptr_t num_args_checked) {
   return flow_graph_builder_->InstanceCall(position, name, kind, argument_count,
                                            argument_names, num_args_checked);
 }
 
 Fragment StreamingFlowGraphBuilder::StoreLocal(TokenPosition position,
                                                LocalVariable* variable) {
   return flow_graph_builder_->StoreLocal(position, variable);
 }
 
 Fragment StreamingFlowGraphBuilder::StoreStaticField(TokenPosition position,
                                                      const dart::Field& field) {
   return flow_graph_builder_->StoreStaticField(position, field);
 }
 
+Fragment StreamingFlowGraphBuilder::StoreInstanceField(TokenPosition position,
+                                                       intptr_t offset) {
+  return flow_graph_builder_->StoreInstanceField(position, offset);
+}
+
 Fragment StreamingFlowGraphBuilder::StringInterpolate(TokenPosition position) {
   return flow_graph_builder_->StringInterpolate(position);
 }
 
 Fragment StreamingFlowGraphBuilder::StringInterpolateSingle(
     TokenPosition position) {
   return flow_graph_builder_->StringInterpolateSingle(position);
 }
 
 Fragment StreamingFlowGraphBuilder::ThrowTypeError() {
@@ skipping 890 matching lines @@
 
     // Let condition be always true.
     instructions += Constant(Bool::True());
   } else {
     instructions += PushArgument();
 
     // See if simple instanceOf is applicable.
     if (dart::FlowGraphBuilder::SimpleInstanceOfType(type)) {
       instructions += Constant(type);
       instructions += PushArgument();  // Type.
-      instructions += InstanceCall(position, dart::Library::PrivateCoreLibName(
-                                                 Symbols::_simpleInstanceOf()),
-                                   Token::kIS, 2, 2);  // 2 checked arguments.
+      instructions += InstanceCall(
+          position,
+          dart::Library::PrivateCoreLibName(Symbols::_simpleInstanceOf()),
+          Token::kIS, 2, 2);  // 2 checked arguments.
       return instructions;
     }
 
     if (!type.IsInstantiated(kCurrentClass)) {
       instructions += LoadInstantiatorTypeArguments();
     } else {
       instructions += NullConstant();
     }
     instructions += PushArgument();  // Instantiator type arguments.
 
@@ skipping 247 matching lines @@
 }
 
 Fragment StreamingFlowGraphBuilder::BuildFunctionExpression() {
   intptr_t offset = ReaderOffset() - 1;  // -1 to include tag byte.
   return BuildFunctionNode(offset, TokenPosition::kNoSource, false, -1);
 }
 
 Fragment StreamingFlowGraphBuilder::BuildLet(TokenPosition* position) {
   if (position != NULL) *position = TokenPosition::kNoSource;
 
-  Fragment instructions = BuildVariableDeclaration();       // read variable.
-  instructions += BuildExpression();                        // read body.
+  Fragment instructions = BuildVariableDeclaration();  // read variable.
+  instructions += BuildExpression();                   // read body.
   return instructions;
 }
 
 
 Fragment StreamingFlowGraphBuilder::BuildBigIntLiteral(
     TokenPosition* position) {
   if (position != NULL) *position = TokenPosition::kNoSource;
 
   const dart::String& value =
       H.DartString(ReadStringReference());  // read index into string table.
@@ skipping 40 matching lines @@
 
   return Constant(Bool::Get(value));
 }
 
 Fragment StreamingFlowGraphBuilder::BuildNullLiteral(TokenPosition* position) {
   if (position != NULL) *position = TokenPosition::kNoSource;
 
   return Constant(Instance::ZoneHandle(Z, Instance::null()));
 }
 
+Fragment StreamingFlowGraphBuilder::BuildVectorCreation(
+    TokenPosition* position) {
+  if (position != NULL) *position = TokenPosition::kNoSource;
+
+  intptr_t size = ReadUInt();  // read size.
+  return AllocateContext(size);
+}
+
+Fragment StreamingFlowGraphBuilder::BuildVectorGet(TokenPosition* position) {
+  if (position != NULL) *position = TokenPosition::kNoSource;
+
+  Fragment instructions = BuildExpression();  // read expression.
+  intptr_t index = ReadUInt();                // read index.
+  instructions += LoadField(Context::variable_offset(index));
+  return instructions;
+}
+
+Fragment StreamingFlowGraphBuilder::BuildVectorSet(TokenPosition* position) {
+  if (position != NULL) *position = TokenPosition::kNoSource;
+
+  Fragment instructions = BuildExpression();  // read vector expression.
+  intptr_t index = ReadUInt();                // read index.
+  instructions += BuildExpression();          // read value expression.
+
+  // The assigned value is the result of the expression.
+  LocalVariable* result = MakeTemporary();

[kustermann, 2017/06/20 12:13:14]

I suspect this is unsafe:

The MakeTemporary() makes the top-of-stack value addressable relative to the
frame pointer. You can use [result] (e.g. via LoadLocal(result)) as long as it's
on the stack.

But the StoreInstanceFieldInstr below will consume 2 elements from the stack
(value/context: both are Pop()ed) and will push the result. So the [result] got
Pop()ed from the stack and you can't use it anymore.

This means that [result] will point to one item past top-of-stack.

Take a look at kernel_to_il.cc:StoreLocal() to see how you can fix it.

[Dmitry Stefantsov, 2017/06/22 14:12:52]

Thank you so much for pointing me to that!  Indeed this code should be
rewritten.  I'll upload a Patch Set in this CL fixing this.

+
+  Value* value = Pop();
+  StoreInstanceFieldInstr* store = new (Z)
+      StoreInstanceFieldInstr(Context::variable_offset(index), Pop(), value,
+                              kNoStoreBarrier, TokenPosition::kNoSource);
+  instructions <<= store;
+
+  // Load the result that is stored in the temporary variable.
+  instructions += LoadLocal(result);
+
+  return instructions;
+}
+
+Fragment StreamingFlowGraphBuilder::BuildVectorCopy(TokenPosition* position) {
+  if (position != NULL) *position = TokenPosition::kNoSource;
+
+  Fragment instructions = BuildExpression();  // read vector expression.
+  CloneContextInstr* clone_instruction = new (Z) CloneContextInstr(
+      TokenPosition::kNoSource, Pop(), Thread::Current()->GetNextDeoptId());

[kustermann, 2017/06/20 12:13:13]

Maybe introduce local variables, so one knows what gets Pop()ed.

Value* context_to_copy = Pop();

[Dmitry Stefantsov, 2017/06/22 14:12:52]

Thanks!  That would read better indeed.  Done.

+  instructions <<= clone_instruction;
+  Push(clone_instruction);
+
+  return instructions;
+}
+
+Fragment StreamingFlowGraphBuilder::BuildClosureCreation(
+    TokenPosition* position) {
+  if (position != NULL) *position = TokenPosition::kNoSource;
+
+  NameIndex function_reference =
+      ReadCanonicalNameReference();  // read function reference.
+  Function& function = Function::ZoneHandle(
+      Z, H.LookupStaticMethodByKernelProcedure(function_reference));
+  function = function.ConvertedClosureFunction();
+  ASSERT(!function.IsNull());
+
+  const dart::Class& closure_class =
+      dart::Class::ZoneHandle(Z, I->object_store()->closure_class());
+  Fragment instructions = AllocateObject(closure_class, function);
+  LocalVariable* closure = MakeTemporary();

[kustermann, 2017/06/20 12:13:14]

Here for example (as opposed to above), the [closure] will stay on the stack
until the very end, so it's safe to make it addressable via [MakeTemporary] and
do several LoadLocal(closure)s.

[Dmitry Stefantsov, 2017/06/22 14:12:52]

Yep.  Now I see it.  Thanks!

+
+  instructions += BuildExpression();  // read context vector.
+  LocalVariable* context = MakeTemporary();
+
+  instructions += LoadLocal(closure);
+  instructions += Constant(function);
+  instructions +=
+      StoreInstanceField(TokenPosition::kNoSource, Closure::function_offset());
+
+  instructions += LoadLocal(closure);
+  instructions += LoadLocal(context);
+  instructions +=
+      StoreInstanceField(TokenPosition::kNoSource, Closure::context_offset());
+
+  instructions += Drop();
+
+  SkipDartType();  // skip function type of the closure.
+
+  return instructions;
+}
+
 Fragment StreamingFlowGraphBuilder::BuildInvalidStatement() {
   H.ReportError("Invalid statements not implemented yet!");
   return Fragment();
 }
 
 Fragment StreamingFlowGraphBuilder::BuildExpressionStatement() {
   Fragment instructions = BuildExpression();  // read expression.
   instructions += Drop();
   return instructions;
 }
@@ skipping 1272 matching lines @@
     metadata_values.SetAt(i, value);
   }
 
   return metadata_values.raw();
 }
 
 }  // namespace kernel
 }  // namespace dart
 
 #endif  // !defined(DART_PRECOMPILED_RUNTIME)