Allocate instance closures similarly to regular closures, i.e. without a

runtime/vm/flow_graph_builder.cc

 // 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.
 
 #include "vm/flow_graph_builder.h"
 
 #include "lib/invocation_mirror.h"
 #include "vm/ast_printer.h"
 #include "vm/bit_vector.h"
 #include "vm/class_finalizer.h"
@@ skipping 2303 matching lines @@
       // Store current context.
       tmp_val = Bind(new LoadLocalInstr(*tmp_var));
       Value* context = Bind(new CurrentContextInstr());
       Do(new StoreInstanceFieldInstr(context_field,
                                      tmp_val,
                                      context,
                                      kEmitStoreBarrier));
       ReturnDefinition(ExitTempLocalScope(tmp_var));
     }
   } else {
+    // TODO(regis): Merge this branch in the above one.
     ASSERT(function.IsImplicitInstanceClosureFunction());
-    ValueGraphVisitor for_receiver(owner());
-    node->receiver()->Visit(&for_receiver);
-    Append(for_receiver);
-    Value* receiver = for_receiver.value();
 
-    PushArgumentInstr* push_receiver = PushArgument(receiver);
     ZoneGrowableArray<PushArgumentInstr*>* arguments =
-        new ZoneGrowableArray<PushArgumentInstr*>(2);
-    arguments->Add(push_receiver);
+        new ZoneGrowableArray<PushArgumentInstr*>(2);  // 1? here and above too?
     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. Otherwise, pass null object.
-    const Class& cls = Class::Handle(function.signature_class());
+    const Class& cls = Class::ZoneHandle(function.signature_class());
     ASSERT(!cls.IsNull());
     const bool requires_type_arguments = cls.NumTypeArguments() > 0;
     Value* type_arguments = NULL;
     if (requires_type_arguments) {
       const Class& instantiator_class = Class::Handle(
           owner()->parsed_function()->function().Owner());
       type_arguments = BuildInstantiatorTypeArguments(node->token_pos(),
                                                       instantiator_class,
                                                       NULL);
-    } else {
-      type_arguments = BuildNullValue();
+      arguments->Add(PushArgument(type_arguments));
     }
-    PushArgumentInstr* push_type_arguments = PushArgument(type_arguments);
-    arguments->Add(push_type_arguments);
-    ReturnDefinition(
-        new CreateClosureInstr(node->function(), arguments, node->token_pos()));
+    AllocateObjectInstr* alloc = new AllocateObjectInstr(node->token_pos(),
+                                                         cls,
+                                                         arguments);
+    alloc->set_closure_function(function);
+
+    // Create fake fields for function and context. Only the context field is
+    // stored at the allocation to be used later when inlining a closure call.
+    const Field& function_field =
+        Field::ZoneHandle(
+            Field::New(Symbols::ClosureFunctionField(),
+            false,  // !static
+            false,  // !final
+            false,  // !const
+            alloc->cls(),
+            0));  // No token position.
+    function_field.SetOffset(Closure::function_offset());
+    const Field& context_field =
+        Field::ZoneHandle(Field::New(
+            Symbols::ClosureContextField(),
+            false,  // !static
+            false,  // !final
+            false,  // !const
+            alloc->cls(),
+            0));  // No token position.
+    context_field.SetOffset(Closure::context_offset());
+    alloc->set_context_field(context_field);
+
+    Value* closure_val = Bind(alloc);
+    { LocalVariable* closure_tmp_var = EnterTempLocalScope(closure_val);
+      // Store function.
+      Value* closure_tmp_val = Bind(new LoadLocalInstr(*closure_tmp_var));
+      Value* func_val =
+          Bind(new ConstantInstr(Function::ZoneHandle(function.raw())));
+      Do(new StoreInstanceFieldInstr(function_field,
+                                     closure_tmp_val,
+                                     func_val,
+                                     kEmitStoreBarrier));
+      // Create new context containing the receiver.
+      const intptr_t kNumContextVariables = 1;  // The receiver.
+      Value* allocated_context =
+          Bind(new AllocateContextInstr(node->token_pos(),
+                                        kNumContextVariables));
+      { LocalVariable* context_tmp_var = EnterTempLocalScope(allocated_context);
+        // Store receiver in new context.
+        Value* context_tmp_val = Bind(new LoadLocalInstr(*context_tmp_var));

[Florian Schneider, 2014/02/25 09:21:15]

In StoreVMFieldInstr the order of arguments is reversed relative to
StoreInstanceField. It is an artifact of how we build stores to captured locals.
(and should probably be changed, since it is annoying to get right)

I think the bug is fixed by reordering evaluation of the input operands here:

// First operand (receiver).
ValueGraphVisitor for_receiver(owner());
node->receiver()->Visit(&for_receiver);
Append(for_receiver);
Value* receiver = for_receiver.value();

// Second operand (context)
Value* context_tmp_val = Bind(new LoadLocalInstr(*context_tmp_var));

+        ValueGraphVisitor for_receiver(owner());
+        node->receiver()->Visit(&for_receiver);
+        Append(for_receiver);
+        Value* receiver = for_receiver.value();
+        Do(new StoreVMFieldInstr(context_tmp_val,
+                                 Context::variable_offset(0),
+                                 receiver,
+                                 Type::ZoneHandle()));
+        // Store new context.
+        closure_tmp_val = Bind(new LoadLocalInstr(*closure_tmp_var));
+        context_tmp_val = Bind(new LoadLocalInstr(*context_tmp_var));
+        Do(new StoreInstanceFieldInstr(context_field,
+                                       closure_tmp_val,
+                                       context_tmp_val,
+                                       kEmitStoreBarrier));
+        Do(ExitTempLocalScope(context_tmp_var));
+      }
+      ReturnDefinition(ExitTempLocalScope(closure_tmp_var));
+    }
   }
 }
 
 
 void EffectGraphVisitor::BuildPushArguments(
     const ArgumentListNode& node,
     ZoneGrowableArray<PushArgumentInstr*>* values) {
   for (intptr_t i = 0; i < node.length(); ++i) {
     ValueGraphVisitor for_argument(owner());
     node.NodeAt(i)->Visit(&for_argument);
@@ skipping 1576 matching lines @@
                                   LanguageError::kError,
                                   Heap::kNew,
                                   "FlowGraphBuilder Bailout: %s %s",
                                   String::Handle(function.name()).ToCString(),
                                   reason));
   Isolate::Current()->long_jump_base()->Jump(1, error);
 }
 
 
 }  // namespace dart