Revert of Don't use different function scopes when parsing with temp zones...

Revert of Don't use different function scopes when parsing with temp zones (patchset #11 id:200001 of https://codereview.chromium.org/2368313002/ )

Reason for revert:
Revert due to asm.js slowdown

Original issue's description:
> Don't use different function scopes when parsing with temp zones
>
> Previously we'd have a scope in the main zone, and another in the temp zone. Then we carefully copied back data to the main zone. This CL changes it so that the scope is just fixed up to only contain data from the main zone. That avoids additional copies and additional allocations; while not increasing the care that needs to be taken. This will also make it easier to abort preparsing while parsing using a temp zone.
>
> BUG=
>
> Committed: https://crrev.com/f41e7ebd62b32e861b6aa14ad8bfce3018d03c3c
> Cr-Commit-Position: refs/heads/master@{#39800}

TBR=marja@chromium.org,adamk@chromium.org
# Skipping CQ checks because original CL landed less than 1 days ago.
NOPRESUBMIT=true
NOTREECHECKS=true
NOTRY=true
BUG=

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

src/ast/scopes.cc

@@ -115,6 +115,7 @@ Scope::Scope(Zone* zone, Scope* outer_scope, ScopeType scope_type)
   force_context_allocation_ =
       !is_function_scope() && outer_scope->has_forced_context_allocation();
   outer_scope_->AddInnerScope(this);
+  if (outer_scope_->is_lazily_parsed_) is_lazily_parsed_ = true;
 }
 
 Scope::Snapshot::Snapshot(Scope* scope)
@@ -277,7 +278,6 @@ void Scope::SetDefaults() {
 #ifdef DEBUG
   scope_name_ = nullptr;
   already_resolved_ = false;
-  needs_migration_ = false;
 #endif
   inner_scope_ = nullptr;
   sibling_ = nullptr;
@@ -953,7 +953,7 @@ VariableProxy* Scope::NewUnresolved(AstNodeFactory* factory,
   // the same name because they may be removed selectively via
   // RemoveUnresolved().
   DCHECK(!already_resolved_);
-  DCHECK_EQ(!needs_migration_, factory->zone() == zone());
+  DCHECK_EQ(factory->zone(), zone());
   VariableProxy* proxy =
       factory->NewVariableProxy(name, kind, start_position, end_position);
   proxy->set_next_unresolved(unresolved_);
@@ -1212,36 +1212,42 @@ void DeclarationScope::ResetAfterPreparsing(bool aborted) {
     }
   } else {
     params_.Clear();
+    // Make sure we won't try to allocate the rest parameter. {params_} was
+    // cleared above.
+    has_rest_ = false;
   }
-
-#ifdef DEBUG
-  needs_migration_ = false;
-#endif
-
-  is_lazily_parsed_ = !aborted;
 }
 
-void DeclarationScope::AnalyzePartially(AstNodeFactory* ast_node_factory) {
-  DCHECK(!force_eager_compilation_);
-  VariableProxy* unresolved = nullptr;
-
-  if (!outer_scope_->is_script_scope()) {
-    // Try to resolve unresolved variables for this Scope and migrate those
-    // which cannot be resolved inside. It doesn't make sense to try to resolve
-    // them in the outer Scopes here, because they are incomplete.
+void DeclarationScope::AnalyzePartially(DeclarationScope* migrate_to,
+                                        AstNodeFactory* ast_node_factory) {
+  // Try to resolve unresolved variables for this Scope and migrate those which
+  // cannot be resolved inside. It doesn't make sense to try to resolve them in
+  // the outer Scopes here, because they are incomplete.
   for (VariableProxy* proxy =
            FetchFreeVariables(this, !FLAG_lazy_inner_functions);
        proxy != nullptr; proxy = proxy->next_unresolved()) {
     DCHECK(!proxy->is_resolved());
     VariableProxy* copy = ast_node_factory->CopyVariableProxy(proxy);
-      copy->set_next_unresolved(unresolved);
-      unresolved = copy;
-    }
+    migrate_to->AddUnresolved(copy);
   }
 
-  ResetAfterPreparsing(false);
-
-  unresolved_ = unresolved;
+  // Push scope data up to migrate_to. Note that migrate_to and this Scope
+  // describe the same Scope, just in different Zones.
+  PropagateUsageFlagsToScope(migrate_to);
+  if (scope_uses_super_property_) migrate_to->scope_uses_super_property_ = true;
+  if (inner_scope_calls_eval_) migrate_to->inner_scope_calls_eval_ = true;
+  if (is_lazily_parsed_) migrate_to->is_lazily_parsed_ = true;
+  DCHECK(!force_eager_compilation_);
+  migrate_to->set_start_position(start_position_);
+  migrate_to->set_end_position(end_position_);
+  migrate_to->set_language_mode(language_mode());
+  migrate_to->arity_ = arity_;
+  migrate_to->force_context_allocation_ = force_context_allocation_;
+  outer_scope_->RemoveInnerScope(this);
+  DCHECK_EQ(outer_scope_, migrate_to->outer_scope_);
+  DCHECK_EQ(outer_scope_->zone(), migrate_to->zone());
+  DCHECK_EQ(NeedsHomeObject(), migrate_to->NeedsHomeObject());
+  DCHECK_EQ(asm_function_, migrate_to->asm_function_);
 }
 
 #ifdef DEBUG
@@ -1440,7 +1446,6 @@ void Scope::CheckScopePositions() {
 }
 
 void Scope::CheckZones() {
-  DCHECK(!needs_migration_);
   for (Scope* scope = inner_scope_; scope != nullptr; scope = scope->sibling_) {
     CHECK_EQ(scope->zone(), zone());
   }
@@ -1812,8 +1817,6 @@ void ModuleScope::AllocateModuleVariables() {
 void Scope::AllocateVariablesRecursively() {
   DCHECK(!already_resolved_);
   DCHECK_EQ(0, num_stack_slots_);
-  // Don't allocate variables of preparsed scopes.
-  if (is_lazily_parsed_) return;
 
   // Allocate variables for inner scopes.
   for (Scope* scope = inner_scope_; scope != nullptr; scope = scope->sibling_) {

src/ast/scopes.h

@@ -74,7 +74,6 @@ class Scope: public ZoneObject {
   void SetScopeName(const AstRawString* scope_name) {
     scope_name_ = scope_name;
   }
-  void set_needs_migration() { needs_migration_ = true; }
 #endif
 
   // TODO(verwaest): Is this needed on Scope?
@@ -416,7 +415,9 @@ class Scope: public ZoneObject {
   void set_is_debug_evaluate_scope() { is_debug_evaluate_scope_ = true; }
   bool is_debug_evaluate_scope() const { return is_debug_evaluate_scope_; }
 
-  bool is_lazily_parsed() const { return is_lazily_parsed_; }
+  void set_is_lazily_parsed(bool is_lazily_parsed) {
+    is_lazily_parsed_ = is_lazily_parsed;
+  }
 
  protected:
   explicit Scope(Zone* zone);
@@ -484,10 +485,7 @@ class Scope: public ZoneObject {
 
   // True if it doesn't need scope resolution (e.g., if the scope was
   // constructed based on a serialized scope info or a catch context).
-  bool already_resolved_;
-  // True if this scope may contain objects from a temp zone that needs to be
-  // fixed up.
-  bool needs_migration_;
+  bool already_resolved_ : 1;
 #endif
 
   // Source positions.
@@ -568,7 +566,6 @@ class Scope: public ZoneObject {
         Handle<ScopeInfo> scope_info);
 
   void AddInnerScope(Scope* inner_scope) {
-    DCHECK_EQ(!needs_migration_, inner_scope->zone() == zone());
     inner_scope->sibling_ = inner_scope_;
     inner_scope_ = inner_scope;
     inner_scope->outer_scope_ = this;
@@ -782,7 +779,8 @@ class DeclarationScope : public Scope {
   // records variables which cannot be resolved inside the Scope (we don't yet
   // know what they will resolve to since the outer Scopes are incomplete) and
   // migrates them into migrate_to.
-  void AnalyzePartially(AstNodeFactory* ast_node_factory);
+  void AnalyzePartially(DeclarationScope* migrate_to,
+                        AstNodeFactory* ast_node_factory);
 
   Handle<StringSet> CollectNonLocals(ParseInfo* info,
                                      Handle<StringSet> non_locals);

src/parsing/parser-base.h

@@ -294,8 +294,8 @@ class ParserBase {
     // allocation.
     // TODO(verwaest): Move to LazyBlockState class that only allocates the
     // scope when needed.
-    explicit BlockState(Zone* zone, ScopeState** scope_stack)
-        : ScopeState(scope_stack, NewScope(zone, *scope_stack)) {}
+    explicit BlockState(ScopeState** scope_stack)
+        : ScopeState(scope_stack, NewScope(*scope_stack)) {}
 
     void SetNonlinear() { this->scope()->SetNonlinear(); }
     void set_start_position(int pos) { this->scope()->set_start_position(pos); }
@@ -309,8 +309,9 @@ class ParserBase {
     }
 
    private:
-    Scope* NewScope(Zone* zone, ScopeState* outer_state) {
+    Scope* NewScope(ScopeState* outer_state) {
       Scope* parent = outer_state->scope();
+      Zone* zone = outer_state->zone();
       return new (zone) Scope(zone, parent, BLOCK_SCOPE);
     }
   };
@@ -3840,8 +3841,6 @@ ParserBase<Impl>::ParseArrowFunctionLiteral(
         LazyParsingResult result = impl()->SkipLazyFunctionBody(
             &materialized_literal_count, &expected_property_count, false, true,
             CHECK_OK);
-        formal_parameters.scope->ResetAfterPreparsing(result ==
-                                                      kLazyParsingAborted);
 
         if (formal_parameters.materialized_literals_count > 0) {
           materialized_literal_count +=
@@ -4431,7 +4430,7 @@ typename ParserBase<Impl>::BlockT ParserBase<Impl>::ParseBlock(
   // Parse the statements and collect escaping labels.
   Expect(Token::LBRACE, CHECK_OK_CUSTOM(NullBlock));
   {
-    BlockState block_state(zone(), &scope_state_);
+    BlockState block_state(&scope_state_);
     block_state.set_start_position(scanner()->location().beg_pos);
     typename Types::Target target(this, body);
 
@@ -4461,7 +4460,7 @@ typename ParserBase<Impl>::StatementT ParserBase<Impl>::ParseScopedStatement(
     }
     // Make a block around the statement for a lexical binding
     // is introduced by a FunctionDeclaration.
-    BlockState block_state(zone(), &scope_state_);
+    BlockState block_state(&scope_state_);
     block_state.set_start_position(scanner()->location().beg_pos);
     BlockT block = factory()->NewBlock(NULL, 1, false, kNoSourcePosition);
     StatementT body = impl()->ParseFunctionDeclaration(CHECK_OK);
@@ -4832,7 +4831,7 @@ typename ParserBase<Impl>::StatementT ParserBase<Impl>::ParseSwitchStatement(
   auto switch_statement = factory()->NewSwitchStatement(labels, switch_pos);
 
   {
-    BlockState cases_block_state(zone(), &scope_state_);
+    BlockState cases_block_state(&scope_state_);
     cases_block_state.set_start_position(scanner()->location().beg_pos);
     cases_block_state.SetNonlinear();
     typename Types::Target target(this, switch_statement);
@@ -4923,7 +4922,7 @@ typename ParserBase<Impl>::StatementT ParserBase<Impl>::ParseTryStatement(
       // Create a block scope to hold any lexical declarations created
       // as part of destructuring the catch parameter.
       {
-        BlockState catch_variable_block_state(zone(), &scope_state_);
+        BlockState catch_variable_block_state(&scope_state_);
         catch_variable_block_state.set_start_position(
             scanner()->location().beg_pos);
         typename Types::Target target(this, catch_block);
@@ -4978,7 +4977,7 @@ typename ParserBase<Impl>::StatementT ParserBase<Impl>::ParseForStatement(
   bool bound_names_are_lexical = false;
 
   // Create an in-between scope for let-bound iteration variables.
-  BlockState for_state(zone(), &scope_state_);
+  BlockState for_state(&scope_state_);
   Expect(Token::FOR, CHECK_OK);
   Expect(Token::LPAREN, CHECK_OK);
   for_state.set_start_position(scanner()->location().beg_pos);
@@ -5049,7 +5048,7 @@ typename ParserBase<Impl>::StatementT ParserBase<Impl>::ParseForStatement(
         {
           ReturnExprScope no_tail_calls(function_state_,
                                         ReturnExprContext::kInsideForInOfBody);
-          BlockState block_state(zone(), &scope_state_);
+          BlockState block_state(&scope_state_);
           block_state.set_start_position(scanner()->location().beg_pos);
 
           StatementT body = ParseScopedStatement(nullptr, true, CHECK_OK);
@@ -5132,7 +5131,7 @@ typename ParserBase<Impl>::StatementT ParserBase<Impl>::ParseForStatement(
         {
           ReturnExprScope no_tail_calls(function_state_,
                                         ReturnExprContext::kInsideForInOfBody);
-          BlockState block_state(zone(), &scope_state_);
+          BlockState block_state(&scope_state_);
           block_state.set_start_position(scanner()->location().beg_pos);
 
           // For legacy compat reasons, give for loops similar treatment to

src/parsing/parser.cc

@@ -2686,9 +2686,11 @@ FunctionLiteral* Parser::ParseFunctionLiteral(
       !(FLAG_validate_asm && scope()->IsAsmModule());
   bool is_lazy_inner_function = use_temp_zone && FLAG_lazy_inner_functions;
 
-  // This Scope lives in the main zone. We'll migrate data into that zone later.
-  DeclarationScope* scope = NewFunctionScope(kind);
-  SetLanguageMode(scope, language_mode);
+  DeclarationScope* main_scope = nullptr;
+  if (use_temp_zone) {
+    // This Scope lives in the main Zone; we'll migrate data into it later.
+    main_scope = NewFunctionScope(kind);
+  }
 
   ZoneList<Statement*>* body = nullptr;
   int arity = -1;
@@ -2708,14 +2710,21 @@ FunctionLiteral* Parser::ParseFunctionLiteral(
     // new temporary zone if the preconditions are satisfied, and ensures that
     // the previous zone is always restored after parsing the body. To be able
     // to do scope analysis correctly after full parsing, we migrate needed
-    // information when the function is parsed.
+    // information from scope into main_scope when the function has been parsed.
     Zone temp_zone(zone()->allocator());
     DiscardableZoneScope zone_scope(this, &temp_zone, use_temp_zone);
 
+    DeclarationScope* scope = NewFunctionScope(kind);
+    SetLanguageMode(scope, language_mode);
+    if (!use_temp_zone) {
+      main_scope = scope;
+    } else {
+      DCHECK(main_scope->zone() != scope->zone());
+    }
+
     FunctionState function_state(&function_state_, &scope_state_, scope);
 #ifdef DEBUG
     scope->SetScopeName(function_name);
-    if (use_temp_zone) scope->set_needs_migration();
 #endif
     ExpressionClassifier formals_classifier(this, &duplicate_finder);
 
@@ -2777,23 +2786,24 @@ FunctionLiteral* Parser::ParseFunctionLiteral(
         // used once.
         eager_compile_hint = FunctionLiteral::kShouldEagerCompile;
         should_be_used_once_hint = true;
-        scope->ResetAfterPreparsing(true);
+      } else if (is_lazy_inner_function) {
+        DCHECK(main_scope != scope);
+        scope->AnalyzePartially(main_scope, &previous_zone_ast_node_factory);
       }
     }
-
     if (!is_lazy_top_level_function && !is_lazy_inner_function) {
       body = ParseEagerFunctionBody(function_name, pos, formals, kind,
                                     function_type, CHECK_OK);
 
       materialized_literal_count = function_state.materialized_literal_count();
       expected_property_count = function_state.expected_property_count();
-    }
-
-    if (use_temp_zone || is_lazy_top_level_function) {
+      if (use_temp_zone) {
         // If the preconditions are correct the function body should never be
         // accessed, but do this anyway for better behaviour if they're wrong.
         body = nullptr;
-      scope->AnalyzePartially(&previous_zone_ast_node_factory);
+        DCHECK(main_scope != scope);
+        scope->AnalyzePartially(main_scope, &previous_zone_ast_node_factory);
+      }
     }
 
     // Parsing the body may change the language mode in our scope.
@@ -2830,7 +2840,7 @@ FunctionLiteral* Parser::ParseFunctionLiteral(
 
   // Note that the FunctionLiteral needs to be created in the main Zone again.
   FunctionLiteral* function_literal = factory()->NewFunctionLiteral(
-      function_name, scope, body, materialized_literal_count,
+      function_name, main_scope, body, materialized_literal_count,
       expected_property_count, arity, duplicate_parameters, function_type,
       eager_compile_hint, pos);
   function_literal->set_function_token_position(function_token_pos);
@@ -2852,6 +2862,7 @@ Parser::LazyParsingResult Parser::SkipLazyFunctionBody(
   int function_block_pos = position();
   DeclarationScope* scope = function_state_->scope();
   DCHECK(scope->is_function_scope());
+  scope->set_is_lazily_parsed(true);
   // Inner functions are not part of the cached data.
   if (!is_inner_function && consume_cached_parse_data() &&
       !cached_parse_data_->rejected()) {
@@ -2884,7 +2895,10 @@ Parser::LazyParsingResult Parser::SkipLazyFunctionBody(
       ParseLazyFunctionBodyWithPreParser(&logger, is_inner_function, may_abort);
 
   // Return immediately if pre-parser decided to abort parsing.
-  if (result == PreParser::kPreParseAbort) return kLazyParsingAborted;
+  if (result == PreParser::kPreParseAbort) {
+    scope->set_is_lazily_parsed(false);
+    return kLazyParsingAborted;
+  }
   if (result == PreParser::kPreParseStackOverflow) {
     // Propagate stack overflow.
     set_stack_overflow();
@@ -3384,6 +3398,11 @@ PreParser::PreParseResult Parser::ParseLazyFunctionBodyWithPreParser(
   PreParser::PreParseResult result = reusable_preparser_->PreParseLazyFunction(
       function_scope, parsing_module_, logger, is_inner_function, may_abort,
       use_counts_);
+  // Detaching the scopes created by PreParser from the Scope chain must be done
+  // above (see ParseFunctionLiteral & AnalyzePartially).
+  if (!is_inner_function) {
+    function_scope->ResetAfterPreparsing(result == PreParser::kPreParseAbort);
+  }
   if (pre_parse_timer_ != NULL) {
     pre_parse_timer_->Stop();
   }
@@ -3529,7 +3548,7 @@ Expression* Parser::ParseClassLiteral(const AstRawString* name,
     return nullptr;
   }
 
-  BlockState block_state(zone(), &scope_state_);
+  BlockState block_state(&scope_state_);
   RaiseLanguageMode(STRICT);
 #ifdef DEBUG
   scope()->SetScopeName(name);

src/parsing/preparser.cc

@@ -268,7 +268,7 @@ PreParserExpression PreParser::ParseClassLiteral(
   }
 
   LanguageMode class_language_mode = language_mode();
-  BlockState block_state(zone(), &scope_state_);
+  BlockState block_state(&scope_state_);
   scope()->SetLanguageMode(
       static_cast<LanguageMode>(class_language_mode | STRICT));
   // TODO(marja): Make PreParser use scope names too.