Fully deserialize the scope chain after parsing, not before

To avoid a dependency on the heap during parsing, we only create a scope chain
without linking to the associated ScopeInfo objects before parsing. This is
enough to avoid special cases during parsing of arrow functions / eval.

Looking at the outer scope's variables during parsing was only needed for hosting
sloppy block functions inside eval. To be able to do this now, we hoist for the
outer-most eval scope after parsing, in DeclarationScope::Analyze.

DeclarationScope::Analyze is also where we replace the outer scope chain with the
fully deserialized version, so variables can be resolved.

Also, this unifies background and foreground thread parsing, as we don't have to
worry about ScopeInfos getting accessed before we're back on the main thread.

BUG=v8:5215
R=verwaest@chromium.org,marja@chromium.org,adamk@chromium.org

Review-Url: https://codereview.chromium.org/2306413002
Cr-Commit-Position: refs/heads/master@{#39452}

src/ast/ast.h

@@ -1176,10 +1176,6 @@ class SloppyBlockFunctionStatement final : public Statement {
  public:
   Statement* statement() const { return statement_; }
   void set_statement(Statement* statement) { statement_ = statement; }
-  VariableProxy* from() const { return from_; }
-  void set_from(VariableProxy* from) { from_ = from; }
-  VariableProxy* to() const { return to_; }
-  void set_to(VariableProxy* to) { to_ = to; }
   Scope* scope() const { return scope_; }
   SloppyBlockFunctionStatement* next() { return next_; }
   void set_next(SloppyBlockFunctionStatement* next) { next_ = next; }
@@ -1190,14 +1186,10 @@ class SloppyBlockFunctionStatement final : public Statement {
   SloppyBlockFunctionStatement(Statement* statement, Scope* scope)
       : Statement(kNoSourcePosition, kSloppyBlockFunctionStatement),
         statement_(statement),
-        from_(nullptr),
-        to_(nullptr),
         scope_(scope),
         next_(nullptr) {}
 
   Statement* statement_;
-  VariableProxy* from_;
-  VariableProxy* to_;
   Scope* const scope_;
   SloppyBlockFunctionStatement* next_;
 };

src/ast/scopes.cc

@@ -333,7 +333,9 @@ Scope* Scope::DeserializeScopeChain(Isolate* isolate, Zone* zone,
       // info of this script context onto the existing script scope to avoid
       // nesting script scopes.
       Handle<ScopeInfo> scope_info(context->scope_info(), isolate);
-      script_scope->SetScriptScopeInfo(scope_info);
+      if (deserialization_mode == DeserializationMode::kIncludingVariables) {
+        script_scope->SetScriptScopeInfo(scope_info);
+      }
       DCHECK(context->previous()->IsNativeContext());
       break;
     } else if (context->IsFunctionContext()) {
@@ -370,20 +372,20 @@ Scope* Scope::DeserializeScopeChain(Isolate* isolate, Zone* zone,
           Scope(zone, ast_value_factory->GetString(handle(name, isolate)),
                 Handle<ScopeInfo>(context->scope_info()));
     }
+    if (deserialization_mode == DeserializationMode::kScopesOnly) {
+      outer_scope->scope_info_ = Handle<ScopeInfo>::null();
+    }
     if (current_scope != nullptr) {
       outer_scope->AddInnerScope(current_scope);
       DCHECK_IMPLIES(
-          deserialization_mode == DeserializationMode::kKeepScopeInfo,
+          deserialization_mode == DeserializationMode::kIncludingVariables,
           current_scope->scope_info_->HasOuterScopeInfo());
       DCHECK_IMPLIES(
-          deserialization_mode == DeserializationMode::kKeepScopeInfo,
+          deserialization_mode == DeserializationMode::kIncludingVariables,
           outer_scope->scope_info_->Equals(
               current_scope->scope_info_->OuterScopeInfo()));
     }
     current_scope = outer_scope;
-    if (deserialization_mode == DeserializationMode::kDeserializeOffHeap) {
-      current_scope->DeserializeScopeInfo(isolate, ast_value_factory);
-    }
     if (innermost_scope == nullptr) innermost_scope = current_scope;
     context = context->previous();
   }
@@ -391,7 +393,7 @@ Scope* Scope::DeserializeScopeChain(Isolate* isolate, Zone* zone,
   if (innermost_scope == nullptr) return script_scope;
   script_scope->AddInnerScope(current_scope);
 #if DEBUG
-  if (deserialization_mode == DeserializationMode::kKeepScopeInfo) {
+  if (deserialization_mode == DeserializationMode::kIncludingVariables) {
     if (script_scope->scope_info_.is_null()) {
       DCHECK(!current_scope->scope_info_->HasOuterScopeInfo());
     } else {
@@ -404,46 +406,6 @@ Scope* Scope::DeserializeScopeChain(Isolate* isolate, Zone* zone,
   return innermost_scope;
 }
 
-void Scope::DeserializeScopeInfo(Isolate* isolate,
-                                 AstValueFactory* ast_value_factory) {
-  if (scope_info_.is_null()) return;
-
-  DCHECK(ThreadId::Current().Equals(isolate->thread_id()));
-
-  // Internalize context local variables.
-  for (int var = 0; var < scope_info_->ContextLocalCount(); ++var) {
-    Handle<String> name_handle(scope_info_->ContextLocalName(var), isolate);
-    const AstRawString* name = ast_value_factory->GetString(name_handle);
-    int index = Context::MIN_CONTEXT_SLOTS + var;
-    VariableMode mode = scope_info_->ContextLocalMode(var);
-    InitializationFlag init_flag = scope_info_->ContextLocalInitFlag(var);
-    MaybeAssignedFlag maybe_assigned_flag =
-        scope_info_->ContextLocalMaybeAssignedFlag(var);
-    VariableLocation location = VariableLocation::CONTEXT;
-    VariableKind kind = NORMAL_VARIABLE;
-    if (index == scope_info_->ReceiverContextSlotIndex()) {
-      kind = THIS_VARIABLE;
-    }
-
-    Variable* result = variables_.Declare(zone(), this, name, mode, kind,
-                                          init_flag, maybe_assigned_flag);
-    result->AllocateTo(location, index);
-  }
-
-  // Internalize function proxy for this scope.
-  if (scope_info_->HasFunctionName()) {
-    Handle<String> name_handle(scope_info_->FunctionName(), isolate);
-    const AstRawString* name = ast_value_factory->GetString(name_handle);
-    int index = scope_info_->FunctionContextSlotIndex(*name_handle);
-    if (index >= 0) {
-      Variable* result = AsDeclarationScope()->DeclareFunctionVar(name);
-      result->AllocateTo(VariableLocation::CONTEXT, index);
-    }
-  }
-
-  scope_info_ = Handle<ScopeInfo>::null();
-}
-
 DeclarationScope* Scope::AsDeclarationScope() {
   DCHECK(is_declaration_scope());
   return static_cast<DeclarationScope*>(this);
@@ -468,8 +430,7 @@ int Scope::num_parameters() const {
   return is_declaration_scope() ? AsDeclarationScope()->num_parameters() : 0;
 }
 
-void DeclarationScope::HoistSloppyBlockFunctions(AstNodeFactory* factory,
+void DeclarationScope::HoistSloppyBlockFunctions(AstNodeFactory* factory) {
-                                                 bool* ok) {
   DCHECK(is_sloppy(language_mode()));
   DCHECK(is_function_scope() || is_eval_scope() || is_script_scope() ||
          (is_block_scope() && outer_scope()->is_function_scope()));
@@ -545,18 +506,19 @@ void DeclarationScope::HoistSloppyBlockFunctions(AstNodeFactory* factory,
         // Based on the preceding check, it doesn't matter what we pass as
         // allow_harmony_restrictive_generators and
         // sloppy_mode_block_scope_function_redefinition.
+        bool ok = true;
         DeclareVariable(declaration, VAR,
                         Variable::DefaultInitializationFlag(VAR), false,
-                        nullptr, ok);
+                        nullptr, &ok);
-        DCHECK(*ok);  // Based on the preceding check, this should not fail
+        CHECK(ok);  // Based on the preceding check, this should not fail
-        if (!*ok) return;
       }
 
-      // Create VariableProxies for creating an assignment statement
+      Expression* assignment = factory->NewAssignment(
-      // (later). Read from the local lexical scope and write to the function
+          Token::ASSIGN, NewUnresolved(factory, name),
-      // scope.
+          delegate->scope()->NewUnresolved(factory, name), kNoSourcePosition);
-      delegate->set_to(NewUnresolved(factory, name));
+      Statement* statement =
-      delegate->set_from(delegate->scope()->NewUnresolved(factory, name));
+          factory->NewExpressionStatement(assignment, kNoSourcePosition);
+      delegate->set_statement(statement);
     }
   }
 }
@@ -565,6 +527,28 @@ void DeclarationScope::Analyze(ParseInfo* info, AnalyzeMode mode) {
   DCHECK(info->literal() != NULL);
   DeclarationScope* scope = info->literal()->scope();
 
+  if (!info->context().is_null() && !info->context()->IsNativeContext()) {
+    if (scope->outer_scope()) {
+      DeclarationScope* script_scope = new (info->zone())
+          DeclarationScope(info->zone(), info->ast_value_factory());
+      info->set_script_scope(script_scope);
+      scope->ReplaceOuterScope(Scope::DeserializeScopeChain(
+          info->isolate(), info->zone(), *info->context(), script_scope,
+          info->ast_value_factory(),
+          Scope::DeserializationMode::kIncludingVariables));
+    } else {
+      DCHECK(info->context()->IsScriptContext());
+      Handle<ScopeInfo> scope_info(info->context()->scope_info(),
+                                   info->isolate());
+      scope->SetScriptScopeInfo(scope_info);
+    }
+  }
+
+  if (scope->is_eval_scope() && is_sloppy(scope->language_mode())) {
+    AstNodeFactory factory(info->ast_value_factory());
+    scope->HoistSloppyBlockFunctions(&factory);
+  }
+
   // We are compiling one of three cases:
   // 1) top-level code,
   // 2) a function/eval/module on the top-level
@@ -747,8 +731,6 @@ void Scope::ReplaceOuterScope(Scope* outer) {
   DCHECK_NOT_NULL(outer);
   DCHECK_NOT_NULL(outer_scope_);
   DCHECK(!already_resolved_);
-  DCHECK(!outer->already_resolved_);
-  DCHECK(!outer_scope_->already_resolved_);
   outer_scope_->RemoveInnerScope(this);
   outer->AddInnerScope(this);
   outer_scope_ = outer;

src/ast/scopes.h

@@ -98,7 +98,7 @@ class Scope: public ZoneObject {
     int top_decl_;
   };
 
-  enum class DeserializationMode { kDeserializeOffHeap, kKeepScopeInfo };
+  enum class DeserializationMode { kIncludingVariables, kScopesOnly };
 
   static Scope* DeserializeScopeChain(Isolate* isolate, Zone* zone,
                                       Context* context,
@@ -576,9 +576,6 @@ class Scope: public ZoneObject {
 
   void SetDefaults();
 
-  void DeserializeScopeInfo(Isolate* isolate,
-                            AstValueFactory* ast_value_factory);
-
   friend class DeclarationScope;
 };
 
@@ -748,7 +745,7 @@ class DeclarationScope : public Scope {
 
   // Go through sloppy_block_function_map_ and hoist those (into this scope)
   // which should be hoisted.
-  void HoistSloppyBlockFunctions(AstNodeFactory* factory, bool* ok);
+  void HoistSloppyBlockFunctions(AstNodeFactory* factory);
 
   SloppyBlockFunctionMap* sloppy_block_function_map() {
     return &sloppy_block_function_map_;

src/background-parsing-task.cc

@@ -50,9 +50,8 @@ BackgroundParsingTask::BackgroundParsingTask(
   // Parser needs to stay alive for finalizing the parsing on the main
   // thread.
   source_->parser.reset(new Parser(source_->info.get()));
-  source_->parser->DeserializeScopeChain(
+  source_->parser->DeserializeScopeChain(source_->info.get(),
-      source_->info.get(), Handle<Context>::null(),
+                                         MaybeHandle<Context>());
-      Scope::DeserializationMode::kDeserializeOffHeap);
 }
 
 

src/compiler-dispatcher/compiler-dispatcher-job.cc

@@ -83,9 +83,8 @@ void CompilerDispatcherJob::PrepareToParseOnMainThread() {
   parse_info_->set_language_mode(shared->language_mode());
 
   parser_.reset(new Parser(parse_info_.get()));
-  parser_->DeserializeScopeChain(
+  parser_->DeserializeScopeChain(parse_info_.get(),
-      parse_info_.get(), handle(function_->context(), isolate_),
+                                 handle(function_->context(), isolate_));
-      Scope::DeserializationMode::kDeserializeOffHeap);
 
   Handle<String> name(String::cast(shared->name()));
   parse_info_->set_function_name(

src/parsing/parser.cc

@@ -603,31 +603,20 @@ Parser::Parser(ParseInfo* info)
   }
 }
 
-void Parser::DeserializeScopeChain(
+void Parser::DeserializeScopeChain(ParseInfo* info,
-    ParseInfo* info, Handle<Context> context,
+                                   MaybeHandle<Context> maybe_context) {
-    Scope::DeserializationMode deserialization_mode) {
   DCHECK(ThreadId::Current().Equals(info->isolate()->thread_id()));
   // TODO(wingo): Add an outer SCRIPT_SCOPE corresponding to the native
   // context, which will have the "this" binding for script scopes.
   DeclarationScope* script_scope = NewScriptScope();
   info->set_script_scope(script_scope);
   Scope* scope = script_scope;
-  if (!context.is_null() && !context->IsNativeContext()) {
+  Handle<Context> context;
-    scope = Scope::DeserializeScopeChain(info->isolate(), zone(), *context,
+  if (maybe_context.ToHandle(&context) && !context->IsNativeContext()) {
-                                         script_scope, ast_value_factory(),
+    scope = Scope::DeserializeScopeChain(
-                                         deserialization_mode);
+        info->isolate(), zone(), *context, script_scope, ast_value_factory(),
+        Scope::DeserializationMode::kScopesOnly);
     DCHECK(!info->is_module() || scope->is_module_scope());
-    if (info->context().is_null()) {
-      DCHECK(deserialization_mode ==
-             Scope::DeserializationMode::kDeserializeOffHeap);
-    } else {
-      // The Scope is backed up by ScopeInfo (which is in the V8 heap); this
-      // means the Parser cannot operate independent of the V8 heap. Tell the
-      // string table to internalize strings and values right after they're
-      // created. This kind of parsing can only be done in the main thread.
-      DCHECK(parsing_on_main_thread_);
-      ast_value_factory()->Internalize(info->isolate());
-    }
   }
   original_scope_ = scope;
 }
@@ -660,8 +649,7 @@ FunctionLiteral* Parser::ParseProgram(Isolate* isolate, ParseInfo* info) {
     cached_parse_data_->Initialize();
   }
 
-  DeserializeScopeChain(info, info->context(),
+  DeserializeScopeChain(info, info->context());
-                        Scope::DeserializationMode::kKeepScopeInfo);
 
   source = String::Flatten(source);
   FunctionLiteral* result;
@@ -780,7 +768,7 @@ FunctionLiteral* Parser::DoParseProgram(ParseInfo* info) {
       // pre-existing bindings should be made writable, enumerable and
       // nonconfigurable if possible, whereas this code will leave attributes
       // unchanged if the property already exists.
-      InsertSloppyBlockFunctionVarBindings(scope, &ok);
+      InsertSloppyBlockFunctionVarBindings(scope);
     }
     if (ok) {
       CheckConflictingVarDeclarations(scope, &ok);
@@ -826,8 +814,7 @@ FunctionLiteral* Parser::ParseLazy(Isolate* isolate, ParseInfo* info) {
     timer.Start();
   }
   Handle<SharedFunctionInfo> shared_info = info->shared_info();
-  DeserializeScopeChain(info, info->context(),
+  DeserializeScopeChain(info, info->context());
-                        Scope::DeserializationMode::kKeepScopeInfo);
 
   // Initialize parser state.
   source = String::Flatten(source);
@@ -3668,7 +3655,7 @@ ZoneList<Statement*>* Parser::ParseEagerFunctionBody(
     Block* init_block = BuildParameterInitializationBlock(parameters, CHECK_OK);
 
     if (is_sloppy(inner_scope->language_mode())) {
-      InsertSloppyBlockFunctionVarBindings(inner_scope, CHECK_OK);
+      InsertSloppyBlockFunctionVarBindings(inner_scope);
     }
 
     // TODO(littledan): Merge the two rejection blocks into one
@@ -3690,7 +3677,7 @@ ZoneList<Statement*>* Parser::ParseEagerFunctionBody(
   } else {
     DCHECK_EQ(inner_scope, function_scope);
     if (is_sloppy(function_scope->language_mode())) {
-      InsertSloppyBlockFunctionVarBindings(function_scope, CHECK_OK);
+      InsertSloppyBlockFunctionVarBindings(function_scope);
     }
   }
 
@@ -3908,29 +3895,17 @@ void Parser::InsertShadowingVarBindingInitializers(Block* inner_block) {
   }
 }
 
-void Parser::InsertSloppyBlockFunctionVarBindings(DeclarationScope* scope,
+void Parser::InsertSloppyBlockFunctionVarBindings(DeclarationScope* scope) {
-                                                  bool* ok) {
+  // For the outermost eval scope, we cannot hoist during parsing: let
-  scope->HoistSloppyBlockFunctions(factory(), CHECK_OK_VOID);
+  // declarations in the surrounding scope may prevent hoisting, but the
-
+  // information is unaccessible during parsing. In this case, we hoist later in
-  SloppyBlockFunctionMap* map = scope->sloppy_block_function_map();
+  // DeclarationScope::Analyze.
-  for (ZoneHashMap::Entry* p = map->Start(); p != nullptr; p = map->Next(p)) {
+  if (scope->is_eval_scope() && scope->outer_scope() == original_scope_) {
-    // Write in assignments to var for each block-scoped function declaration
+    return;
-    auto delegates = static_cast<SloppyBlockFunctionStatement*>(p->value);
-    for (SloppyBlockFunctionStatement* delegate = delegates;
-         delegate != nullptr; delegate = delegate->next()) {
-      if (delegate->to() == nullptr) {
-        continue;
-      }
-      Expression* assignment = factory()->NewAssignment(
-          Token::ASSIGN, delegate->to(), delegate->from(), kNoSourcePosition);
-      Statement* statement =
-          factory()->NewExpressionStatement(assignment, kNoSourcePosition);
-      delegate->set_statement(statement);
-    }
   }
+  scope->HoistSloppyBlockFunctions(factory());
 }
 
-
 // ----------------------------------------------------------------------------
 // Parser support
 

src/parsing/parser.h

@@ -184,8 +184,16 @@ class Parser : public ParserBase<Parser> {
   bool Parse(ParseInfo* info);
   void ParseOnBackground(ParseInfo* info);
 
-  void DeserializeScopeChain(ParseInfo* info, Handle<Context> context,
+  // Deserialize the scope chain prior to parsing in which the script is going
-                             Scope::DeserializationMode deserialization_mode);
+  // to be executed. If the script is a top-level script, or the scope chain
+  // consists of only a native context, maybe_context should be an empty
+  // handle.
+  //
+  // This only deserializes the scope chain, but doesn't connect the scopes to
+  // their corresponding scope infos. Therefore, looking up variables in the
+  // deserialized scopes is not possible.
+  void DeserializeScopeChain(ParseInfo* info,
+                             MaybeHandle<Context> maybe_context);
 
   // Handle errors detected during parsing, move statistics to Isolate,
   // internalize strings (move them to the heap).
@@ -433,8 +441,7 @@ class Parser : public ParserBase<Parser> {
   void InsertShadowingVarBindingInitializers(Block* block);
 
   // Implement sloppy block-scoped functions, ES2015 Annex B 3.3
-  void InsertSloppyBlockFunctionVarBindings(DeclarationScope* scope,
+  void InsertSloppyBlockFunctionVarBindings(DeclarationScope* scope);
-                                            bool* ok);
 
   VariableProxy* NewUnresolved(const AstRawString* name, int begin_pos,
                                int end_pos = kNoSourcePosition,

test/cctest/test-parsing.cc

@@ -3405,7 +3405,7 @@ TEST(SerializationOfMaybeAssignmentFlag) {
       new (&zone) i::DeclarationScope(&zone, &avf);
   i::Scope* s = i::Scope::DeserializeScopeChain(
       isolate, &zone, context, script_scope, &avf,
-      i::Scope::DeserializationMode::kKeepScopeInfo);
+      i::Scope::DeserializationMode::kIncludingVariables);
   CHECK(s != script_scope);
   CHECK(name != NULL);
 
@@ -3453,7 +3453,7 @@ TEST(IfArgumentsArrayAccessedThenParametersMaybeAssigned) {
       new (&zone) i::DeclarationScope(&zone, &avf);
   i::Scope* s = i::Scope::DeserializeScopeChain(
       isolate, &zone, context, script_scope, &avf,
-      i::Scope::DeserializationMode::kKeepScopeInfo);
+      i::Scope::DeserializationMode::kIncludingVariables);
   CHECK(s != script_scope);
   const i::AstRawString* name_x = avf.GetOneByteString("x");
 

test/unittests/compiler-dispatcher/compiler-dispatcher-job-unittest.cc

@@ -170,13 +170,14 @@ TEST_F(CompilerDispatcherJobTest, ScopeChain) {
   job->PrepareToParseOnMainThread();
   job->Parse();
   ASSERT_TRUE(job->FinalizeParsingOnMainThread());
-  ASSERT_TRUE(job->status() == CompileJobStatus::kReadyToAnalyse);
+  ASSERT_TRUE(job->PrepareToCompileOnMainThread());
+  ASSERT_TRUE(job->status() == CompileJobStatus::kReadyToCompile);
 
   const AstRawString* var_x =
       job->parse_info_->ast_value_factory()->GetOneByteString("x");
   Variable* var = job->parse_info_->literal()->scope()->Lookup(var_x);
   ASSERT_TRUE(var);
-  ASSERT_TRUE(var->IsUnallocated());
+  ASSERT_TRUE(var->IsParameter());
 
   const AstRawString* var_g =
       job->parse_info_->ast_value_factory()->GetOneByteString("g");