From 5d68529be240a1fc9fcc67c8950ead4baf7604e4 Mon Sep 17 00:00:00 2001 From: marja Date: Tue, 10 Feb 2015 06:39:00 -0800 Subject: [PATCH] Parsing: Make Scope not know about Isolate. 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} --- src/arm/full-codegen-arm.cc | 2 +- src/arm64/full-codegen-arm64.cc | 2 +- src/compiler/ast-graph-builder.cc | 2 +- src/full-codegen.cc | 4 +- src/hydrogen.cc | 2 +- src/ia32/full-codegen-ia32.cc | 2 +- src/preparser.h | 4 +- src/runtime/runtime-debug.cc | 3 +- src/scopes.cc | 97 +++++++++++++++++++-------------------- src/scopes.h | 24 ++++------ src/x64/full-codegen-x64.cc | 2 +- test/cctest/test-parsing.cc | 6 +-- 12 files changed, 71 insertions(+), 79 deletions(-) diff --git a/src/arm/full-codegen-arm.cc b/src/arm/full-codegen-arm.cc index a4a17ef..7508c01 100644 --- a/src/arm/full-codegen-arm.cc +++ b/src/arm/full-codegen-arm.cc @@ -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); diff --git a/src/arm64/full-codegen-arm64.cc b/src/arm64/full-codegen-arm64.cc index c6d3267..1d24b91 100644 --- a/src/arm64/full-codegen-arm64.cc +++ b/src/arm64/full-codegen-arm64.cc @@ -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) { diff --git a/src/compiler/ast-graph-builder.cc b/src/compiler/ast-graph-builder.cc index 5a87e32..50216a7 100644 --- a/src/compiler/ast-graph-builder.cc +++ b/src/compiler/ast-graph-builder.cc @@ -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; diff --git a/src/full-codegen.cc b/src/full-codegen.cc index c702526..0896e42 100644 --- a/src/full-codegen.cc +++ b/src/full-codegen.cc @@ -622,7 +622,7 @@ void FullCodeGenerator::AllocateModules(ZoneList* 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); diff --git a/src/hydrogen.cc b/src/hydrogen.cc index 47cddd0..d7619e4 100644 --- a/src/hydrogen.cc +++ b/src/hydrogen.cc @@ -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( - outer_context, function, scope->GetScopeInfo()); + outer_context, function, scope->GetScopeInfo(isolate())); HInstruction* instr = Add(inner_context); set_scope(scope); environment()->BindContext(inner_context); diff --git a/src/ia32/full-codegen-ia32.cc b/src/ia32/full-codegen-ia32.cc index 8ab3763..950a129 100644 --- a/src/ia32/full-codegen-ia32.cc +++ b/src/ia32/full-codegen-ia32.cc @@ -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); diff --git a/src/preparser.h b/src/preparser.h index 3d33dd0..bb7b40d 100644 --- a/src/preparser.h +++ b/src/preparser.h @@ -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); diff --git a/src/runtime/runtime-debug.cc b/src/runtime/runtime-debug.cc index ea5b68a..a1a5ee1 100644 --- a/src/runtime/runtime-debug.cc +++ b/src/runtime/runtime-debug.cc @@ -1491,7 +1491,8 @@ class ScopeIterator { Handle 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 diff --git a/src/scopes.cc b/src/scopes.cc index 4680a1c..fa95bde 100644 --- a/src/scopes.cc +++ b/src/scopes.cc @@ -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 scope_info, - AstValueFactory* value_factory) - : isolate_(isolate), - inner_scopes_(4, zone), +Scope::Scope(Zone* zone, Scope* inner_scope, ScopeType scope_type, + Handle 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::null(), script_scope->ast_value_factory_); + Scope(zone, current_scope, WITH_SCOPE, Handle::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(scope_info), script_scope->ast_value_factory_); + current_scope = new (zone) Scope(zone, current_scope, SCRIPT_SCOPE, + Handle(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(scope_info), script_scope->ast_value_factory_); + current_scope = new (zone) Scope(zone, current_scope, MODULE_SCOPE, + Handle(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(scope_info), script_scope->ast_value_factory_); + current_scope = new (zone) Scope(zone, current_scope, FUNCTION_SCOPE, + Handle(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(scope_info), script_scope->ast_value_factory_); + current_scope = new (zone) + Scope(zone, current_scope, BLOCK_SCOPE, Handle(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(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 Scope::GetScopeInfo() { +Handle 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 >* chain, - int position) { - if (!is_eval_scope()) chain->Add(Handle(GetScopeInfo())); +void Scope::GetNestedScopeChain(Isolate* isolate, + List >* chain, int position) { + if (!is_eval_scope()) chain->Add(Handle(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 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, diff --git a/src/scopes.h b/src/scopes.h index 412ad85..cd4658d 100644 --- a/src/scopes.h +++ b/src/scopes.h @@ -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 GetScopeInfo(); + Handle 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 >* chain, + void GetNestedScopeChain(Isolate* isolate, List >* 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 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 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) { diff --git a/src/x64/full-codegen-x64.cc b/src/x64/full-codegen-x64.cc index b248dd1..65e60fb 100644 --- a/src/x64/full-codegen-x64.cc +++ b/src/x64/full-codegen-x64.cc @@ -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); diff --git a/test/cctest/test-parsing.cc b/test/cctest/test-parsing.cc index e047918..ba2fa97 100644 --- a/test/cctest/test-parsing.cc +++ b/test/cctest/test-parsing.cc @@ -3176,7 +3176,7 @@ TEST(SerializationOfMaybeAssignmentFlag) { i::Handle 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); -- 2.7.4