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

src/parsing/parser.cc

 // Copyright 2012 the V8 project authors. All rights reserved.
 // Use of this source code is governed by a BSD-style license that can be
 // found in the LICENSE file.
 
 #include "src/parsing/parser.h"
 
 #include <memory>
 
 #include "src/api.h"
 #include "src/ast/ast-expression-rewriter.h"
@@ skipping 2723 matching lines @@
   // Inner functions will be parsed using a temporary Zone. After parsing, we
   // will migrate unresolved variable into a Scope in the main Zone.
   // TODO(marja): Refactor parsing modes: simplify this.
   bool use_temp_zone =
       !is_lazy_top_level_function && allow_lazy() &&
       function_type == FunctionLiteral::kDeclaration &&
       eager_compile_hint != FunctionLiteral::kShouldEagerCompile &&
       !(FLAG_validate_asm && scope()->IsAsmModule());
   bool is_lazy_inner_function = use_temp_zone && FLAG_lazy_inner_functions;
 
-  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);
-  }
+  // This Scope lives in the main zone. We'll migrate data into that zone later.
+  DeclarationScope* scope = NewFunctionScope(kind);
+  SetLanguageMode(scope, language_mode);
 
   ZoneList<Statement*>* body = nullptr;
   int arity = -1;
   int materialized_literal_count = -1;
   int expected_property_count = -1;
   DuplicateFinder duplicate_finder(scanner()->unicode_cache());
   bool should_be_used_once_hint = false;
   bool has_duplicate_parameters;
 
   {
     // Temporary zones can nest. When we migrate free variables (see below), we
     // need to recreate them in the previous Zone.
     AstNodeFactory previous_zone_ast_node_factory(ast_value_factory());
     previous_zone_ast_node_factory.set_zone(zone());
 
     // Open a new zone scope, which sets our AstNodeFactory to allocate in the
     // 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 from scope into main_scope when the function has been parsed.
+    // information when the function is 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);
 #endif
     ExpressionClassifier formals_classifier(this, &duplicate_finder);
 
     if (is_generator) {
       // For generators, allocating variables in contexts is currently a win
       // because it minimizes the work needed to suspend and resume an
       // activation.  The machine code produced for generators (by full-codegen)
@@ skipping 44 matching lines @@
         DCHECK(is_lazy_top_level_function);
         bookmark.Apply();
         // Trigger eager (re-)parsing, just below this block.
         is_lazy_top_level_function = false;
 
         // This is probably an initialization function. Inform the compiler it
         // should also eager-compile this function, and that we expect it to be
         // used once.
         eager_compile_hint = FunctionLiteral::kShouldEagerCompile;
         should_be_used_once_hint = true;
-      } else if (is_lazy_inner_function) {
-        DCHECK(main_scope != scope);
-        scope->AnalyzePartially(main_scope, &previous_zone_ast_node_factory);
+        scope->ResetAfterPreparsing();
       }
     }
+
     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) {
-        // 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;
-        DCHECK(main_scope != scope);
-        scope->AnalyzePartially(main_scope, &previous_zone_ast_node_factory);
-      }
+    }
+
+    if (use_temp_zone || is_lazy_top_level_function) {

[marja, 2016/09/27 07:14:03]

This sounds wrong... is_lazy_inner_function maybe?

[marja, 2016/09/27 11:17:34]

.... and I was confused; use_temp_zone contains that.

+      // 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);
     }
 
     // Parsing the body may change the language mode in our scope.
     language_mode = scope->language_mode();
 
     // Validate name and parameter names. We can do this only after parsing the
     // function, since the function can declare itself strict.
     CheckFunctionName(language_mode, function_name, function_name_validity,
                       function_name_location, CHECK_OK);
     const bool allow_duplicate_parameters =
@@ skipping 16 matching lines @@
     has_duplicate_parameters =
         !classifier()->is_valid_formal_parameter_list_without_duplicates();
   }  // DiscardableZoneScope goes out of scope.
 
   FunctionLiteral::ParameterFlag duplicate_parameters =
       has_duplicate_parameters ? FunctionLiteral::kHasDuplicateParameters
                                : FunctionLiteral::kNoDuplicateParameters;
 
   // Note that the FunctionLiteral needs to be created in the main Zone again.
   FunctionLiteral* function_literal = factory()->NewFunctionLiteral(
-      function_name, main_scope, body, materialized_literal_count,
+      function_name, scope, body, materialized_literal_count,
       expected_property_count, arity, duplicate_parameters, function_type,
       eager_compile_hint, kind, pos);
   function_literal->set_function_token_position(function_token_pos);
   if (should_be_used_once_hint)
     function_literal->set_should_be_used_once_hint();
 
   if (should_infer_name) {
     DCHECK_NOT_NULL(fni_);
     fni_->AddFunction(function_literal);
   }
@@ skipping 564 matching lines @@
 #undef SET_ALLOW
   }
   // Aborting inner function preparsing would leave scopes in an inconsistent
   // state; we don't parse inner functions in the abortable mode anyway.
   DCHECK(!is_inner_function || !may_abort);
 
   DeclarationScope* function_scope = function_state_->scope();
   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();
   if (pre_parse_timer_ != NULL) {
     pre_parse_timer_->Stop();
   }
   return result;
 }
 
 Expression* Parser::InstallHomeObject(Expression* function_literal,
                                       Expression* home_object) {
   Block* do_block = factory()->NewBlock(nullptr, 1, false, kNoSourcePosition);
   Variable* result_var =
@@ skipping 2103 matching lines @@
 
   return final_loop;
 }
 
 #undef CHECK_OK
 #undef CHECK_OK_VOID
 #undef CHECK_FAILED
 
 }  // namespace internal
 }  // namespace v8