WIP context-allocation for “this” in arrow functions

src/scopes.cc

 // Copyright 2012 the V8 project authors. All rights reserved.
 // Use of this source code is governed by a BSD-style license that can be
 // found in the LICENSE file.
 
 #include "src/v8.h"
 
 #include "src/scopes.h"
 
 #include "src/accessors.h"
 #include "src/bootstrapper.h"
@@ skipping 591 matching lines @@
 
   // Collect internals which are always allocated on the heap.
   for (int i = 0; i < internals_.length(); i++) {
     Variable* var = internals_[i];
     if (var->is_used()) {
       DCHECK(var->IsContextSlot());
       context_locals->Add(var, zone());
     }
   }
 
+  // Arrow functions can cause the receiver to be collected in the context.

[wingo, 2015/01/15 10:09:06]

"allocated".  And really it's "copied to" the context -- the receiver is still
in the stack slot from the outer function's perspective.

[aperez, 2015/01/15 16:58:03]

Done.

+  if (receiver_ && receiver_->has_forced_context_allocation()) {
+    DCHECK(receiver_->IsContextSlot());
+    context_locals->Add(receiver_, zone());
+  }
+
   // Collect temporaries which are always allocated on the stack, unless the
   // context as a whole has forced context allocation.
   for (int i = 0; i < temps_.length(); i++) {
     Variable* var = temps_[i];
     if (var->is_used()) {
       if (var->IsContextSlot()) {
         DCHECK(has_forced_context_allocation());
         context_locals->Add(var, zone());
       } else {
         DCHECK(var->IsStackLocal());
@@ skipping 552 matching lines @@
       this->calls_sloppy_eval() || outer_scope_calls_sloppy_eval_;
   for (int i = 0; i < inner_scopes_.length(); i++) {
     Scope* inner = inner_scopes_[i];
     inner->PropagateScopeInfo(calls_sloppy_eval);
     if (inner->scope_calls_eval_ || inner->inner_scope_calls_eval_) {
       inner_scope_calls_eval_ = true;
     }
     // If the inner scope is an arrow function, propagate the flags tracking
     // usage of arguments/super/this, but do not propagate them out from normal
     // functions.
-    if (!inner->is_function_scope() || inner->is_arrow_scope()) {
+    if (inner->is_arrow_scope()) {
       if (inner->scope_uses_arguments_ || inner->inner_scope_uses_arguments_) {
         inner_scope_uses_arguments_ = true;
       }
       if (inner->scope_uses_super_property_ ||
           inner->inner_scope_uses_super_property_) {
         inner_scope_uses_super_property_ = true;
       }
       if (inner->uses_super_constructor_call() ||
           inner->inner_scope_uses_super_constructor_call_) {
         inner_scope_uses_super_constructor_call_ = true;
       }
       if (inner->scope_uses_this_ || inner->inner_scope_uses_this_) {
         inner_scope_uses_this_ = true;
       }
-    }
-    if (inner->force_eager_compilation_) {
-      force_eager_compilation_ = true;
-    }
-    if (asm_module_ && inner->scope_type() == FUNCTION_SCOPE) {
-      inner->asm_function_ = true;
+    } else if (inner->is_block_scope()) {
+      if (inner->scope_uses_this_) {

[wingo, 2015/01/15 10:09:06]

Why do we need the whole list of these?  At some point this should be refactored
to be a bitfield, if this propagation is really needed.

[aperez, 2015/01/15 16:58:03]

I have been trying to factor out the flag propagations, or move some of them
into Scope::FinalizeBlockScope(), but either the result ended up being far from
obvious (and quite difficult to understand when reading the code), or the chunk
of code being ever bigger than it is). So, I think the best is to leave mostly
as-is, and plan to change those flags to a bitfield later — and that will make
it possible to simplify the code.

+        scope_uses_this_ = true;
+      }
+      if (inner->scope_uses_arguments_) {
+        scope_uses_arguments_ = true;
+      }
+      if (inner->scope_uses_super_property_) {
+        scope_uses_super_property_ = true;
+      }
+      if (inner->scope_uses_super_constructor_call_) {
+        scope_uses_super_constructor_call_ = true;
+      }
+      if (inner->inner_scope_uses_arguments_) {
+        inner_scope_uses_arguments_ = true;
+      }
+      if (inner->inner_scope_uses_super_property_) {
+        inner_scope_uses_super_property_ = true;
+      }
+      if (inner->inner_scope_uses_super_constructor_call_) {
+        inner_scope_uses_super_constructor_call_ = true;
+      }
+      if (inner->inner_scope_uses_this_) {
+        inner_scope_uses_this_ = true;
+      }
+    } else if (!inner->is_function_scope() || inner->is_arrow_scope()) {
+      if (inner->force_eager_compilation_) {
+        force_eager_compilation_ = true;
+      }

[wingo, 2015/01/15 10:09:05]

These propagations should run for all scopes

[aperez, 2015/01/15 16:58:03]

Done.

+      if (asm_module_ && inner->scope_type() == FUNCTION_SCOPE) {
+        inner->asm_function_ = true;
+      }
     }
   }
 }
 
 
 bool Scope::MustAllocate(Variable* var) {
   // Give var a read/write use if there is a chance it might be accessed
   // via an eval() call.  This is only possible if the variable has a
   // visible name.
   if ((var->is_this() || !var->raw_name()->IsEmpty()) &&
@@ skipping 102 matching lines @@
           AllocateHeapSlot(var);
         }
       } else {
         DCHECK(var->IsUnallocated() || var->IsParameter());
         if (var->IsUnallocated()) {
           var->AllocateTo(Variable::PARAMETER, i);
         }
       }
     }
   }
+
+  // If the scope contains at least one inner arrow function which uses
+  // "this", it must be copied in the context, from where it will be looked

[wingo, 2015/01/15 10:09:06]

s/it must be copied in/the receiver must be copied to/

[aperez, 2015/01/15 16:58:03]

Done.

+  // up by code emitted in the prologues of arrow functions.
+  if (inner_scope_uses_this_) {
+    DCHECK(receiver_);
+    if (receiver_->IsUnallocated() || !receiver_->IsContextSlot()) {
+      receiver_->ForceContextAllocation();
+      AllocateHeapSlot(receiver_);
+    }
+    DCHECK(MustAllocateInContext(receiver_));
+  }
 }
 
 
 void Scope::AllocateNonParameterLocal(Variable* var) {
   DCHECK(var->scope() == this);
   DCHECK(!var->IsVariable(isolate_->factory()->dot_result_string()) ||
          !var->IsStackLocal());
   if (var->IsUnallocated() && MustAllocate(var)) {
     if (MustAllocateInContext(var)) {
       AllocateHeapSlot(var);
@@ skipping 96 matching lines @@
 }
 
 
 int Scope::ContextLocalCount() const {
   if (num_heap_slots() == 0) return 0;
   return num_heap_slots() - Context::MIN_CONTEXT_SLOTS -
       (function_ != NULL && function_->proxy()->var()->IsContextSlot() ? 1 : 0);
 }
 
 } }  // namespace v8::internal