Parsing: Make Scope not know about Isolate.
authormarja <marja@chromium.org>
Tue, 10 Feb 2015 14:39:00 +0000 (06:39 -0800)
committerCommit bot <commit-bot@chromium.org>
Tue, 10 Feb 2015 14:39:21 +0000 (14:39 +0000)
Scope, like Parser, must be able to operate independent of Isolate and the V8
heap (for background parsing). After the heap-independent phase, there is a heap
dependent phase, during which we do operations such as scope anaylysis.

This CL makes the phases explicit by not telling Scope about the Isolate too
early (during the heap-independent phase, Scope should know nothing about
Isolate). This decreases the probability of accidental code changes which would
add heap-dependent operations into the heap-independent phase.

R=rossberg@chromium.org
BUG=

Review URL: https://codereview.chromium.org/909093003

Cr-Commit-Position: refs/heads/master@{#26546}

12 files changed:
src/arm/full-codegen-arm.cc
src/arm64/full-codegen-arm64.cc
src/compiler/ast-graph-builder.cc
src/full-codegen.cc
src/hydrogen.cc
src/ia32/full-codegen-ia32.cc
src/preparser.h
src/runtime/runtime-debug.cc
src/scopes.cc
src/scopes.h
src/x64/full-codegen-x64.cc
test/cctest/test-parsing.cc

index a4a17ef..7508c01 100644 (file)
@@ -197,7 +197,7 @@ void FullCodeGenerator::Generate() {
     bool need_write_barrier = true;
     if (FLAG_harmony_scoping && info->scope()->is_script_scope()) {
       __ push(r1);
-      __ Push(info->scope()->GetScopeInfo());
+      __ Push(info->scope()->GetScopeInfo(info->isolate()));
       __ CallRuntime(Runtime::kNewScriptContext, 2);
     } else if (heap_slots <= FastNewContextStub::kMaximumSlots) {
       FastNewContextStub stub(isolate(), heap_slots);
index c6d3267..1d24b91 100644 (file)
@@ -197,7 +197,7 @@ void FullCodeGenerator::Generate() {
     Comment cmnt(masm_, "[ Allocate context");
     bool need_write_barrier = true;
     if (FLAG_harmony_scoping && info->scope()->is_script_scope()) {
-      __ Mov(x10, Operand(info->scope()->GetScopeInfo()));
+      __ Mov(x10, Operand(info->scope()->GetScopeInfo(info->isolate())));
       __ Push(x1, x10);
       __ CallRuntime(Runtime::kNewScriptContext, 2);
     } else if (heap_slots <= FastNewContextStub::kMaximumSlots) {
index 5a87e32..50216a7 100644 (file)
@@ -2444,7 +2444,7 @@ Node* AstGraphBuilder::BuildLocalBlockContext(Scope* scope) {
 
   // Allocate a new local context.
   const Operator* op = javascript()->CreateBlockContext();
-  Node* scope_info = jsgraph()->Constant(scope->GetScopeInfo());
+  Node* scope_info = jsgraph()->Constant(scope->GetScopeInfo(info_->isolate()));
   Node* local_context = NewNode(op, scope_info, closure);
 
   return local_context;
index c702526..0896e42 100644 (file)
@@ -622,7 +622,7 @@ void FullCodeGenerator::AllocateModules(ZoneList<Declaration*>* declarations) {
         // Set up module context.
         DCHECK(scope->interface()->Index() >= 0);
         __ Push(Smi::FromInt(scope->interface()->Index()));
-        __ Push(scope->GetScopeInfo());
+        __ Push(scope->GetScopeInfo(isolate()));
         __ CallRuntime(Runtime::kPushModuleContext, 2);
         StoreToFrameField(StandardFrameConstants::kContextOffset,
                           context_register());
@@ -1801,7 +1801,7 @@ FullCodeGenerator::EnterBlockScopeIfNeeded::EnterBlockScopeIfNeeded(
     codegen_->scope_ = scope;
     {
       Comment cmnt(masm(), "[ Extend block context");
-      __ Push(scope->GetScopeInfo());
+      __ Push(scope->GetScopeInfo(codegen->isolate()));
       codegen_->PushFunctionArgumentForContextAllocation();
       __ CallRuntime(Runtime::kPushBlockContext, 2);
 
index 47cddd0..d7619e4 100644 (file)
@@ -4562,7 +4562,7 @@ void HOptimizedGraphBuilder::VisitBlock(Block* stmt) {
       AddInstruction(function);
       // Allocate a block context and store it to the stack frame.
       HInstruction* inner_context = Add<HAllocateBlockContext>(
-          outer_context, function, scope->GetScopeInfo());
+          outer_context, function, scope->GetScopeInfo(isolate()));
       HInstruction* instr = Add<HStoreFrameContext>(inner_context);
       set_scope(scope);
       environment()->BindContext(inner_context);
index 8ab3763..950a129 100644 (file)
@@ -190,7 +190,7 @@ void FullCodeGenerator::Generate() {
     // Argument to NewContext is the function, which is still in edi.
     if (FLAG_harmony_scoping && info->scope()->is_script_scope()) {
       __ push(edi);
-      __ Push(info->scope()->GetScopeInfo());
+      __ Push(info->scope()->GetScopeInfo(info->isolate()));
       __ CallRuntime(Runtime::kNewScriptContext, 2);
     } else if (heap_slots <= FastNewContextStub::kMaximumSlots) {
       FastNewContextStub stub(isolate(), heap_slots);
index 3d33dd0..bb7b40d 100644 (file)
@@ -310,8 +310,8 @@ class ParserBase : public Traits {
     DCHECK(scope_type != MODULE_SCOPE || allow_harmony_modules());
     DCHECK((scope_type == FUNCTION_SCOPE && IsValidFunctionKind(kind)) ||
            kind == kNormalFunction);
-    Scope* result = new (zone())
-        Scope(isolate(), zone(), parent, scope_type, ast_value_factory());
+    Scope* result =
+        new (zone()) Scope(zone(), parent, scope_type, ast_value_factory());
     bool uninitialized_this =
         FLAG_experimental_classes && IsSubclassConstructor(kind);
     result->Initialize(uninitialized_this);
index ea5b68a..a1a5ee1 100644 (file)
@@ -1491,7 +1491,8 @@ class ScopeIterator {
                           Handle<SharedFunctionInfo> shared_info) {
     if (scope != NULL) {
       int source_position = shared_info->code()->SourcePosition(frame_->pc());
-      scope->GetNestedScopeChain(&nested_scope_chain_, source_position);
+      scope->GetNestedScopeChain(isolate_, &nested_scope_chain_,
+                                 source_position);
     } else {
       // A failed reparse indicates that the preparser has diverged from the
       // parser or that the preparse data given to the initial parse has been
index 4680a1c..fa95bde 100644 (file)
@@ -67,10 +67,9 @@ Variable* VariableMap::Lookup(const AstRawString* name) {
 // ----------------------------------------------------------------------------
 // Implementation of Scope
 
-Scope::Scope(Isolate* isolate, Zone* zone, Scope* outer_scope,
-             ScopeType scope_type, AstValueFactory* ast_value_factory)
-    : isolate_(isolate),
-      inner_scopes_(4, zone),
+Scope::Scope(Zone* zone, Scope* outer_scope, ScopeType scope_type,
+             AstValueFactory* ast_value_factory)
+    : inner_scopes_(4, zone),
       variables_(zone),
       internals_(4, zone),
       temps_(4, zone),
@@ -89,11 +88,9 @@ Scope::Scope(Isolate* isolate, Zone* zone, Scope* outer_scope,
 }
 
 
-Scope::Scope(Isolate* isolate, Zone* zone, Scope* inner_scope,
-             ScopeType scope_type, Handle<ScopeInfo> scope_info,
-             AstValueFactory* value_factory)
-    : isolate_(isolate),
-      inner_scopes_(4, zone),
+Scope::Scope(Zone* zone, Scope* inner_scope, ScopeType scope_type,
+             Handle<ScopeInfo> scope_info, AstValueFactory* value_factory)
+    : inner_scopes_(4, zone),
       variables_(zone),
       internals_(4, zone),
       temps_(4, zone),
@@ -115,11 +112,10 @@ Scope::Scope(Isolate* isolate, Zone* zone, Scope* inner_scope,
 }
 
 
-Scope::Scope(Isolate* isolate, Zone* zone, Scope* inner_scope,
+Scope::Scope(Zone* zone, Scope* inner_scope,
              const AstRawString* catch_variable_name,
              AstValueFactory* value_factory)
-    : isolate_(isolate),
-      inner_scopes_(1, zone),
+    : inner_scopes_(1, zone),
       variables_(zone),
       internals_(0, zone),
       temps_(0, zone),
@@ -201,8 +197,8 @@ Scope* Scope::DeserializeScopeChain(Isolate* isolate, Zone* zone,
   while (!context->IsNativeContext()) {
     if (context->IsWithContext()) {
       Scope* with_scope = new (zone)
-          Scope(isolate, zone, current_scope, WITH_SCOPE,
-                Handle<ScopeInfo>::null(), script_scope->ast_value_factory_);
+          Scope(zone, current_scope, WITH_SCOPE, Handle<ScopeInfo>::null(),
+                script_scope->ast_value_factory_);
       current_scope = with_scope;
       // All the inner scopes are inside a with.
       contains_with = true;
@@ -211,31 +207,31 @@ Scope* Scope::DeserializeScopeChain(Isolate* isolate, Zone* zone,
       }
     } else if (context->IsScriptContext()) {
       ScopeInfo* scope_info = ScopeInfo::cast(context->extension());
-      current_scope = new (zone) Scope(
-          isolate, zone, current_scope, SCRIPT_SCOPE,
-          Handle<ScopeInfo>(scope_info), script_scope->ast_value_factory_);
+      current_scope = new (zone) Scope(zone, current_scope, SCRIPT_SCOPE,
+                                       Handle<ScopeInfo>(scope_info),
+                                       script_scope->ast_value_factory_);
     } else if (context->IsModuleContext()) {
       ScopeInfo* scope_info = ScopeInfo::cast(context->module()->scope_info());
-      current_scope = new (zone) Scope(
-          isolate, zone, current_scope, MODULE_SCOPE,
-          Handle<ScopeInfo>(scope_info), script_scope->ast_value_factory_);
+      current_scope = new (zone) Scope(zone, current_scope, MODULE_SCOPE,
+                                       Handle<ScopeInfo>(scope_info),
+                                       script_scope->ast_value_factory_);
     } else if (context->IsFunctionContext()) {
       ScopeInfo* scope_info = context->closure()->shared()->scope_info();
-      current_scope = new (zone) Scope(
-          isolate, zone, current_scope, FUNCTION_SCOPE,
-          Handle<ScopeInfo>(scope_info), script_scope->ast_value_factory_);
+      current_scope = new (zone) Scope(zone, current_scope, FUNCTION_SCOPE,
+                                       Handle<ScopeInfo>(scope_info),
+                                       script_scope->ast_value_factory_);
       if (scope_info->IsAsmFunction()) current_scope->asm_function_ = true;
       if (scope_info->IsAsmModule()) current_scope->asm_module_ = true;
     } else if (context->IsBlockContext()) {
       ScopeInfo* scope_info = ScopeInfo::cast(context->extension());
-      current_scope = new (zone) Scope(
-          isolate, zone, current_scope, BLOCK_SCOPE,
-          Handle<ScopeInfo>(scope_info), script_scope->ast_value_factory_);
+      current_scope = new (zone)
+          Scope(zone, current_scope, BLOCK_SCOPE, Handle<ScopeInfo>(scope_info),
+                script_scope->ast_value_factory_);
     } else {
       DCHECK(context->IsCatchContext());
       String* name = String::cast(context->extension());
       current_scope = new (zone) Scope(
-          isolate, zone, current_scope,
+          zone, current_scope,
           script_scope->ast_value_factory_->GetString(Handle<String>(name)),
           script_scope->ast_value_factory_);
     }
@@ -662,7 +658,7 @@ bool Scope::AllocateVariables(CompilationInfo* info, AstNodeFactory* factory) {
   if (!ResolveVariablesRecursively(info, factory)) return false;
 
   // 4) Allocate variables.
-  AllocateVariablesRecursively();
+  AllocateVariablesRecursively(info->isolate());
 
   return true;
 }
@@ -751,18 +747,17 @@ Scope* Scope::DeclarationScope() {
 }
 
 
-Handle<ScopeInfo> Scope::GetScopeInfo() {
+Handle<ScopeInfo> Scope::GetScopeInfo(Isolate* isolate) {
   if (scope_info_.is_null()) {
-    scope_info_ = ScopeInfo::Create(isolate(), zone(), this);
+    scope_info_ = ScopeInfo::Create(isolate, zone(), this);
   }
   return scope_info_;
 }
 
 
-void Scope::GetNestedScopeChain(
-    List<Handle<ScopeInfo> >* chain,
-    int position) {
-  if (!is_eval_scope()) chain->Add(Handle<ScopeInfo>(GetScopeInfo()));
+void Scope::GetNestedScopeChain(Isolate* isolate,
+                                List<Handle<ScopeInfo> >* chain, int position) {
+  if (!is_eval_scope()) chain->Add(Handle<ScopeInfo>(GetScopeInfo(isolate)));
 
   for (int i = 0; i < inner_scopes_.length(); i++) {
     Scope* scope = inner_scopes_[i];
@@ -770,7 +765,7 @@ void Scope::GetNestedScopeChain(
     int end_pos = scope->end_position();
     DCHECK(beg_pos >= 0 && end_pos >= 0);
     if (beg_pos <= position && position < end_pos) {
-      scope->GetNestedScopeChain(chain, position);
+      scope->GetNestedScopeChain(isolate, chain, position);
       return;
     }
   }
@@ -1245,10 +1240,10 @@ bool Scope::MustAllocateInContext(Variable* var) {
 }
 
 
-bool Scope::HasArgumentsParameter() {
+bool Scope::HasArgumentsParameter(Isolate* isolate) {
   for (int i = 0; i < params_.length(); i++) {
     if (params_[i]->name().is_identical_to(
-            isolate_->factory()->arguments_string())) {
+            isolate->factory()->arguments_string())) {
       return true;
     }
   }
@@ -1266,14 +1261,14 @@ void Scope::AllocateHeapSlot(Variable* var) {
 }
 
 
-void Scope::AllocateParameterLocals() {
+void Scope::AllocateParameterLocals(Isolate* isolate) {
   DCHECK(is_function_scope());
   Variable* arguments = LookupLocal(ast_value_factory_->arguments_string());
   DCHECK(arguments != NULL);  // functions have 'arguments' declared implicitly
 
   bool uses_sloppy_arguments = false;
 
-  if (MustAllocate(arguments) && !HasArgumentsParameter()) {
+  if (MustAllocate(arguments) && !HasArgumentsParameter(isolate)) {
     // 'arguments' is used. Unless there is also a parameter called
     // 'arguments', we must be conservative and allocate all parameters to
     // the context assuming they will be captured by the arguments object.
@@ -1328,9 +1323,9 @@ void Scope::AllocateParameterLocals() {
 }
 
 
-void Scope::AllocateNonParameterLocal(Variable* var) {
+void Scope::AllocateNonParameterLocal(Isolate* isolate, Variable* var) {
   DCHECK(var->scope() == this);
-  DCHECK(!var->IsVariable(isolate_->factory()->dot_result_string()) ||
+  DCHECK(!var->IsVariable(isolate->factory()->dot_result_string()) ||
          !var->IsStackLocal());
   if (var->IsUnallocated() && MustAllocate(var)) {
     if (MustAllocateInContext(var)) {
@@ -1342,14 +1337,14 @@ void Scope::AllocateNonParameterLocal(Variable* var) {
 }
 
 
-void Scope::AllocateNonParameterLocals() {
+void Scope::AllocateNonParameterLocals(Isolate* isolate) {
   // All variables that have no rewrite yet are non-parameter locals.
   for (int i = 0; i < temps_.length(); i++) {
-    AllocateNonParameterLocal(temps_[i]);
+    AllocateNonParameterLocal(isolate, temps_[i]);
   }
 
   for (int i = 0; i < internals_.length(); i++) {
-    AllocateNonParameterLocal(internals_[i]);
+    AllocateNonParameterLocal(isolate, internals_[i]);
   }
 
   ZoneList<VarAndOrder> vars(variables_.occupancy(), zone());
@@ -1362,7 +1357,7 @@ void Scope::AllocateNonParameterLocals() {
   vars.Sort(VarAndOrder::Compare);
   int var_count = vars.length();
   for (int i = 0; i < var_count; i++) {
-    AllocateNonParameterLocal(vars[i].var());
+    AllocateNonParameterLocal(isolate, vars[i].var());
   }
 
   // For now, function_ must be allocated at the very end.  If it gets
@@ -1370,19 +1365,19 @@ void Scope::AllocateNonParameterLocals() {
   // because of the current ScopeInfo implementation (see
   // ScopeInfo::ScopeInfo(FunctionScope* scope) constructor).
   if (function_ != NULL) {
-    AllocateNonParameterLocal(function_->proxy()->var());
+    AllocateNonParameterLocal(isolate, function_->proxy()->var());
   }
 
   if (rest_parameter_) {
-    AllocateNonParameterLocal(rest_parameter_);
+    AllocateNonParameterLocal(isolate, rest_parameter_);
   }
 }
 
 
-void Scope::AllocateVariablesRecursively() {
+void Scope::AllocateVariablesRecursively(Isolate* isolate) {
   // Allocate variables for inner scopes.
   for (int i = 0; i < inner_scopes_.length(); i++) {
-    inner_scopes_[i]->AllocateVariablesRecursively();
+    inner_scopes_[i]->AllocateVariablesRecursively(isolate);
   }
 
   // If scope is already resolved, we still need to allocate
@@ -1394,8 +1389,8 @@ void Scope::AllocateVariablesRecursively() {
 
   // Allocate variables for this scope.
   // Parameters must be allocated first, if any.
-  if (is_function_scope()) AllocateParameterLocals();
-  AllocateNonParameterLocals();
+  if (is_function_scope()) AllocateParameterLocals(isolate);
+  AllocateNonParameterLocals(isolate);
 
   // Force allocation of a context for this scope if necessary. For a 'with'
   // scope and for a function scope that makes an 'eval' call we need a context,
index 412ad85..cd4658d 100644 (file)
@@ -72,7 +72,7 @@ class Scope: public ZoneObject {
   // ---------------------------------------------------------------------------
   // Construction
 
-  Scope(Isolate* isolate, Zone* zone, Scope* outer_scope, ScopeType scope_type,
+  Scope(Zone* zone, Scope* outer_scope, ScopeType scope_type,
         AstValueFactory* value_factory);
 
   // Compute top scope and allocate variables. For lazy compilation the top
@@ -95,7 +95,6 @@ class Scope: public ZoneObject {
   // tree and its children are reparented.
   Scope* FinalizeBlockScope();
 
-  Isolate* isolate() const { return isolate_; }
   Zone* zone() const { return zone_; }
 
   // ---------------------------------------------------------------------------
@@ -454,13 +453,13 @@ class Scope: public ZoneObject {
   // where var declarations will be hoisted to in the implementation.
   Scope* DeclarationScope();
 
-  Handle<ScopeInfo> GetScopeInfo();
+  Handle<ScopeInfo> GetScopeInfo(Isolate* isolate);
 
   // Get the chain of nested scopes within this scope for the source statement
   // position. The scopes will be added to the list from the outermost scope to
   // the innermost scope. Only nested block, catch or with scopes are tracked
   // and will be returned, but no inner function scopes.
-  void GetNestedScopeChain(List<Handle<ScopeInfo> >* chain,
+  void GetNestedScopeChain(Isolate* isolate, List<Handle<ScopeInfo> >* chain,
                            int statement_position);
 
   // ---------------------------------------------------------------------------
@@ -486,8 +485,6 @@ class Scope: public ZoneObject {
  protected:
   friend class ParserFactory;
 
-  Isolate* const isolate_;
-
   // Scope tree.
   Scope* outer_scope_;  // the immediately enclosing outer scope, or NULL
   ZoneList<Scope*> inner_scopes_;  // the immediately enclosed inner scopes
@@ -659,15 +656,15 @@ class Scope: public ZoneObject {
   // Predicates.
   bool MustAllocate(Variable* var);
   bool MustAllocateInContext(Variable* var);
-  bool HasArgumentsParameter();
+  bool HasArgumentsParameter(Isolate* isolate);
 
   // Variable allocation.
   void AllocateStackSlot(Variable* var);
   void AllocateHeapSlot(Variable* var);
-  void AllocateParameterLocals();
-  void AllocateNonParameterLocal(Variable* var);
-  void AllocateNonParameterLocals();
-  void AllocateVariablesRecursively();
+  void AllocateParameterLocals(Isolate* isolate);
+  void AllocateNonParameterLocal(Isolate* isolate, Variable* var);
+  void AllocateNonParameterLocals(Isolate* isolate);
+  void AllocateVariablesRecursively(Isolate* isolate);
   void AllocateModulesRecursively(Scope* host_scope);
 
   // Resolve and fill in the allocation information for all variables
@@ -683,12 +680,11 @@ class Scope: public ZoneObject {
 
  private:
   // Construct a scope based on the scope info.
-  Scope(Isolate* isolate, Zone* zone, Scope* inner_scope, ScopeType type,
+  Scope(Zone* zone, Scope* inner_scope, ScopeType type,
         Handle<ScopeInfo> scope_info, AstValueFactory* value_factory);
 
   // Construct a catch scope with a binding for the name.
-  Scope(Isolate* isolate, Zone* zone, Scope* inner_scope,
-        const AstRawString* catch_variable_name,
+  Scope(Zone* zone, Scope* inner_scope, const AstRawString* catch_variable_name,
         AstValueFactory* value_factory);
 
   void AddInnerScope(Scope* inner_scope) {
index b248dd1..65e60fb 100644 (file)
@@ -189,7 +189,7 @@ void FullCodeGenerator::Generate() {
     // Argument to NewContext is the function, which is still in rdi.
     if (FLAG_harmony_scoping && info->scope()->is_script_scope()) {
       __ Push(rdi);
-      __ Push(info->scope()->GetScopeInfo());
+      __ Push(info->scope()->GetScopeInfo(info->isolate()));
       __ CallRuntime(Runtime::kNewScriptContext, 2);
     } else if (heap_slots <= FastNewContextStub::kMaximumSlots) {
       FastNewContextStub stub(isolate(), heap_slots);
index e047918..ba2fa97 100644 (file)
@@ -3176,7 +3176,7 @@ TEST(SerializationOfMaybeAssignmentFlag) {
   i::Handle<i::String> str = name->string();
   CHECK(str->IsInternalizedString());
   i::Scope* script_scope =
-      new (&zone) i::Scope(isolate, &zone, NULL, i::SCRIPT_SCOPE, &avf);
+      new (&zone) i::Scope(&zone, NULL, i::SCRIPT_SCOPE, &avf);
   script_scope->Initialize();
   i::Scope* s =
       i::Scope::DeserializeScopeChain(isolate, &zone, context, script_scope);
@@ -3224,7 +3224,7 @@ TEST(IfArgumentsArrayAccessedThenParametersMaybeAssigned) {
   avf.Internalize(isolate);
 
   i::Scope* script_scope =
-      new (&zone) i::Scope(isolate, &zone, NULL, i::SCRIPT_SCOPE, &avf);
+      new (&zone) i::Scope(&zone, NULL, i::SCRIPT_SCOPE, &avf);
   script_scope->Initialize();
   i::Scope* s =
       i::Scope::DeserializeScopeChain(isolate, &zone, context, script_scope);
@@ -3272,7 +3272,7 @@ TEST(ExportsMaybeAssigned) {
   avf.Internalize(isolate);
 
   i::Scope* script_scope =
-      new (&zone) i::Scope(isolate, &zone, NULL, i::SCRIPT_SCOPE, &avf);
+      new (&zone) i::Scope(&zone, NULL, i::SCRIPT_SCOPE, &avf);
   script_scope->Initialize();
   i::Scope* s =
       i::Scope::DeserializeScopeChain(isolate, &zone, context, script_scope);