Force context allocation for variables in generator scopes.

src/scopes.cc

 // Copyright 2012 the V8 project authors. All rights reserved.
 // Redistribution and use in source and binary forms, with or without
 // modification, are permitted provided that the following conditions are
 // met:
 //
 //     * Redistributions of source code must retain the above copyright
 //       notice, this list of conditions and the following disclaimer.
 //     * Redistributions in binary form must reproduce the above
 //       copyright notice, this list of conditions and the following
 //       disclaimer in the documentation and/or other materials provided
@@ skipping 101 matching lines @@
       temps_(4, zone),
       params_(4, zone),
       unresolved_(16, zone),
       decls_(4, zone),
       interface_(FLAG_harmony_modules &&
                  (type == MODULE_SCOPE || type == GLOBAL_SCOPE)
                      ? Interface::NewModule(zone) : NULL),
       already_resolved_(false),
       zone_(zone) {
   SetDefaults(type, outer_scope, Handle<ScopeInfo>::null());
+  if (!is_function_scope() &&

[Michael Starzinger, 2013/04/05 11:38:42]

I would feel more comfortable if we could move this into SetDefaults(), even
though it is already correct here AFAICT. Something like the following there ...

force_context_allocation _ = (outer_scope != NULL && is_function_scope())
    ? outer_scope->has_forced_context_allocation() : false;

[wingo, 2013/04/05 12:25:06]

Done.  I assume you meant !is_function_scope() here.

[Michael Starzinger, 2013/04/05 13:06:56]

Yep, that was a typo on my end.

+      outer_scope &&
+      outer_scope->has_forced_context_allocation())
+    ForceContextAllocation();
   // The outermost scope must be a global scope.
   ASSERT(type == GLOBAL_SCOPE || outer_scope != NULL);
   ASSERT(!HasIllegalRedeclaration());
 }
 
 
 Scope::Scope(Scope* inner_scope,
              ScopeType type,
              Handle<ScopeInfo> scope_info,
              Zone* zone)
@@ skipping 58 matching lines @@
   illegal_redecl_ = NULL;
   scope_inside_with_ = false;
   scope_contains_with_ = false;
   scope_calls_eval_ = false;
   // Inherit the strict mode from the parent scope.
   language_mode_ = (outer_scope != NULL)
       ? outer_scope->language_mode_ : CLASSIC_MODE;
   outer_scope_calls_non_strict_eval_ = false;
   inner_scope_calls_eval_ = false;
   force_eager_compilation_ = false;
+  force_context_allocation_ = false;
   num_var_or_const_ = 0;
   num_stack_slots_ = 0;
   num_heap_slots_ = 0;
   num_modules_ = 0;
   module_var_ = NULL,
   scope_info_ = scope_info;
   start_position_ = RelocInfo::kNoPosition;
   end_position_ = RelocInfo::kNoPosition;
   if (!scope_info.is_null()) {
     scope_calls_eval_ = scope_info->CallsEval();
@@ skipping 386 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()) {
       ASSERT(var->IsContextSlot());
       context_locals->Add(var, zone());
     }
   }
 
-  // Collect temporaries which are always allocated on the stack.
+  // 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()) {
-      ASSERT(var->IsStackLocal());
-      stack_locals->Add(var, zone());
+      if (var->IsContextSlot()) {
+        ASSERT(has_forced_context_allocation());
+        context_locals->Add(var, zone());
+      } else {
+        ASSERT(var->IsStackLocal());
+        stack_locals->Add(var, zone());
+      }
     }
   }
 
   // Collect declared local variables.
   ZoneList<VarAndOrder> vars(variables_.occupancy(), zone());
   for (VariableMap::Entry* p = variables_.Start();
        p != NULL;
        p = variables_.Next(p)) {
     Variable* var = reinterpret_cast<Variable*>(p->value);
     if (var->is_used()) {
@@ skipping 553 matching lines @@
   return !var->IsGlobalObjectProperty() && var->is_used();
 }
 
 
 bool Scope::MustAllocateInContext(Variable* var) {
   // If var is accessed from an inner scope, or if there is a possibility
   // that it might be accessed from the current or an inner scope (through
   // an eval() call or a runtime with lookup), it must be allocated in the
   // context.
   //
-  // Exceptions: temporary variables are never allocated in a context;
-  // catch-bound variables are always allocated in a context.
+  // Exceptions: If the scope as a whole has forced context allocation, all
+  // variables will have context allocation, even temporaries.  Otherwise
+  // temporary variables are always stack-allocated.  Catch-bound variables are
+  // always context-allocated.
+  if (has_forced_context_allocation()) return true;
   if (var->mode() == TEMPORARY) return false;
   if (var->mode() == INTERNAL) return true;
   if (is_catch_scope() || is_block_scope() || is_module_scope()) return true;
   if (is_global_scope() && IsLexicalVariableMode(var->mode())) return true;
   return var->has_forced_context_allocation() ||
       scope_calls_eval_ ||
       inner_scope_calls_eval_ ||
       scope_contains_with_;
 }
 
@@ skipping 182 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