Separate Scope into DeclarationScope and Scope

src/parsing/parser.cc

Index: src/parsing/parser.cc
diff --git a/src/parsing/parser.cc b/src/parsing/parser.cc
index bfa5e1c27bf89800af62604c7d3a59993f6e4693..b40e4b00db4000836a1ce103df2ce21ac63e11a5 100644
--- a/src/parsing/parser.cc
+++ b/src/parsing/parser.cc
@@ -220,7 +220,7 @@ FunctionLiteral* Parser::DefaultConstructor(const AstRawString* name,
 
   FunctionKind kind = call_super ? FunctionKind::kDefaultSubclassConstructor
                                  : FunctionKind::kDefaultBaseConstructor;
-  Scope* function_scope = NewFunctionScope(kind);
+  DeclarationScope* function_scope = NewFunctionScope(kind);
   SetLanguageMode(function_scope,
                   static_cast<LanguageMode>(language_mode | STRICT));
   // Set start and end position to the same value
@@ -952,10 +952,13 @@ FunctionLiteral* Parser::DoParseProgram(ParseInfo* info) {
   {
     // TODO(wingo): Add an outer SCRIPT_SCOPE corresponding to the native
     // context, which will have the "this" binding for script scopes.
-    Scope* scope = NewScriptScope();
+    DeclarationScope* scope = NewScriptScope();
     info->set_script_scope(scope);
+    Scope* inner = scope;
     if (!info->context().is_null() && !info->context()->IsNativeContext()) {
-      scope = Scope::DeserializeScopeChain(info->isolate(), zone(),
+      // If we're compiling for eval, this isn't necessarily a declaration
+      // scope.
+      inner = Scope::DeserializeScopeChain(info->isolate(), zone(),
                                            *info->context(), scope,
                                            ast_value_factory());
       // The Scope is backed up by ScopeInfo (which is in the V8 heap); this
@@ -965,16 +968,18 @@ FunctionLiteral* Parser::DoParseProgram(ParseInfo* info) {
       DCHECK(parsing_on_main_thread_);
       ast_value_factory()->Internalize(info->isolate());
     }
-    original_scope_ = scope;
+    original_scope_ = inner;
     if (info->is_eval()) {
-      if (!scope->is_script_scope() || is_strict(info->language_mode())) {
+      if (!inner->is_script_scope() || is_strict(info->language_mode())) {
         parsing_mode = PARSE_EAGERLY;
       }
-      scope = NewScopeWithParent(scope, EVAL_SCOPE);
+      inner = NewEvalScope(inner);
     } else if (info->is_module()) {
-      scope = NewScopeWithParent(scope, MODULE_SCOPE);
+      inner = NewModuleScope(inner);
     }
 
+    scope = inner->AsDeclarationScope();
+
     scope->set_start_position(0);
 
     // Enter 'scope' with the given parsing mode.
@@ -989,7 +994,8 @@ FunctionLiteral* Parser::DoParseProgram(ParseInfo* info) {
     if (parsing_module_) {
       ParseModuleItemList(body, &ok);
       ok = ok &&
-           module()->Validate(this->scope(), &pending_error_handler_, zone());
+           module()->Validate(this->scope()->AsDeclarationScope(),
+                              &pending_error_handler_, zone());
     } else {
       // Don't count the mode in the use counters--give the program a chance
       // to enable script-wide strict mode below.
@@ -1014,7 +1020,7 @@ FunctionLiteral* Parser::DoParseProgram(ParseInfo* info) {
       InsertSloppyBlockFunctionVarBindings(scope, nullptr, &ok);
     }
     if (ok) {
-      CheckConflictingVarDeclarations(this->scope(), &ok);
+      CheckConflictingVarDeclarations(scope, &ok);
     }
 
     if (ok && info->parse_restriction() == ONLY_SINGLE_FUNCTION_LITERAL) {
@@ -1030,7 +1036,7 @@ FunctionLiteral* Parser::DoParseProgram(ParseInfo* info) {
     if (ok) {
       ParserTraits::RewriteDestructuringAssignments();
       result = factory()->NewScriptOrEvalFunctionLiteral(
-          this->scope(), body, function_state.materialized_literal_count(),
+          scope, body, function_state.materialized_literal_count(),
           function_state.expected_property_count());
     }
   }
@@ -1118,8 +1124,9 @@ FunctionLiteral* Parser::ParseLazy(Isolate* isolate, ParseInfo* info,
 
   {
     // Parse the function literal.
-    Scope* scope = NewScriptScope();
-    info->set_script_scope(scope);
+    DeclarationScope* script_scope = NewScriptScope();
+    Scope* scope = script_scope;
+    info->set_script_scope(script_scope);
     if (!info->context().is_null()) {
       // Ok to use Isolate here, since lazy function parsing is only done in the
       // main thread.
@@ -1152,7 +1159,7 @@ FunctionLiteral* Parser::ParseLazy(Isolate* isolate, ParseInfo* info,
       }
 
       // TODO(adamk): We should construct this scope from the ScopeInfo.
-      Scope* scope = NewFunctionScope(FunctionKind::kArrowFunction);
+      DeclarationScope* scope = NewFunctionScope(FunctionKind::kArrowFunction);
 
       // These two bits only need to be explicitly set because we're
       // not passing the ScopeInfo to the Scope constructor.
@@ -1934,7 +1941,7 @@ VariableProxy* Parser::NewUnresolved(const AstRawString* name,
   // Let/const variables are always added to the immediately enclosing scope.
   Scope* scope = IsLexicalVariableMode(mode)
                      ? this->scope()
-                     : this->scope()->DeclarationScope();
+                     : this->scope()->GetDeclarationScope();
   return scope->NewUnresolved(factory(), name, Variable::NORMAL,
                               scanner()->location().beg_pos,
                               scanner()->location().end_pos);
@@ -1961,7 +1968,7 @@ Variable* Parser::Declare(Declaration* declaration,
   bool is_function_declaration = declaration->IsFunctionDeclaration();
   if (scope == nullptr) scope = this->scope();
   Scope* declaration_scope =
-      IsLexicalVariableMode(mode) ? scope : scope->DeclarationScope();
+      IsLexicalVariableMode(mode) ? scope : scope->GetDeclarationScope();
   Variable* var = NULL;
 
   // If a suitable scope exists, then we can statically declare this
@@ -2003,7 +2010,7 @@ Variable* Parser::Declare(Declaration* declaration,
         FunctionKind function_kind =
             declaration->AsFunctionDeclaration()->fun()->kind();
         duplicate_allowed =
-            scope->DeclarationScope()->sloppy_block_function_map()->Lookup(
+            scope->GetDeclarationScope()->sloppy_block_function_map()->Lookup(
                 const_cast<AstRawString*>(name), name->hash()) != nullptr &&
             !IsAsyncFunction(function_kind) &&
             !(allow_harmony_restrictive_generators() &&
@@ -2130,8 +2137,8 @@ Statement* Parser::ParseNativeDeclaration(bool* ok) {
   // isn't lazily compiled. The extension structures are only
   // accessible while parsing the first time not when reparsing
   // because of lazy compilation.
-  // TODO(adamk): Should this be ClosureScope()?
-  scope()->DeclarationScope()->ForceEagerCompilation();
+  // TODO(adamk): Should this be GetClosureScope()?
+  scope()->GetDeclarationScope()->ForceEagerCompilation();
 
   // TODO(1240846): It's weird that native function declarations are
   // introduced dynamically when we meet their declarations, whereas
@@ -2241,7 +2248,7 @@ Statement* Parser::ParseHoistableDeclaration(
       !is_async && !(allow_harmony_restrictive_generators() && is_generator)) {
     SloppyBlockFunctionStatement* delegate =
         factory()->NewSloppyBlockFunctionStatement(empty, scope());
-    scope()->DeclarationScope()->sloppy_block_function_map()->Declare(
+    scope()->GetDeclarationScope()->sloppy_block_function_map()->Declare(
         variable_name, delegate);
     return delegate;
   }
@@ -2808,7 +2815,7 @@ Statement* Parser::ParseReturnStatement(bool* ok) {
 
   result = factory()->NewReturnStatement(return_value, loc.beg_pos);
 
-  Scope* decl_scope = scope()->DeclarationScope();
+  DeclarationScope* decl_scope = scope()->GetDeclarationScope();
   if (decl_scope->is_script_scope() || decl_scope->is_eval_scope()) {
     ReportMessageAt(loc, MessageTemplate::kIllegalReturn);
     *ok = false;
@@ -4208,7 +4215,7 @@ DoExpression* Parser::ParseDoExpression(bool* ok) {
       scope()->NewTemporary(ast_value_factory()->dot_result_string());
   Block* block = ParseBlock(nullptr, CHECK_OK);
   DoExpression* expr = factory()->NewDoExpression(block, result, pos);
-  if (!Rewriter::Rewrite(this, scope()->ClosureScope(), expr,
+  if (!Rewriter::Rewrite(this, scope()->GetClosureScope(), expr,
                          ast_value_factory())) {
     *ok = false;
     return nullptr;
@@ -4357,7 +4364,7 @@ FunctionLiteral* Parser::ParseFunctionLiteral(
       eager_compile_hint != FunctionLiteral::kShouldEagerCompile &&
       !(FLAG_validate_asm && scope()->asm_module());
 
-  Scope* main_scope = nullptr;
+  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);
@@ -4385,7 +4392,7 @@ FunctionLiteral* Parser::ParseFunctionLiteral(
     Zone temp_zone(zone()->allocator());
     DiscardableZoneScope zone_scope(this, &temp_zone, use_temp_zone);
 
-    Scope* scope = NewFunctionScope(kind);
+    DeclarationScope* scope = NewFunctionScope(kind);
     SetLanguageMode(scope, language_mode);
     if (!use_temp_zone) {
       main_scope = scope;
@@ -4395,7 +4402,7 @@ FunctionLiteral* Parser::ParseFunctionLiteral(
 
     FunctionState function_state(&function_state_, &scope_state_, scope, kind);
 #ifdef DEBUG
-    this->scope()->SetScopeName(function_name);
+    scope->SetScopeName(function_name);
 #endif
     ExpressionClassifier formals_classifier(this, &duplicate_finder);
 
@@ -4731,8 +4738,7 @@ Block* Parser::BuildParameterInitializationBlock(
     Scope* param_scope = scope();
     Block* param_block = init_block;
     if (!parameter.is_simple() && scope()->calls_sloppy_eval()) {
-      param_scope = NewScope(BLOCK_SCOPE);
-      param_scope->set_is_declaration_scope();
+      param_scope = NewVarblockScope();
       param_scope->set_start_position(descriptor.initialization_pos);
       param_scope->set_end_position(parameter.initializer_end_position);
       param_scope->RecordEvalCall();
@@ -4838,11 +4844,11 @@ ZoneList<Statement*>* Parser::ParseEagerFunctionBody(
   }
 
   ZoneList<Statement*>* body = result;
-  Scope* inner_scope = scope();
+  DeclarationScope* function_scope = scope()->AsDeclarationScope();
+  DeclarationScope* inner_scope = function_scope;
   Block* inner_block = nullptr;
   if (!parameters.is_simple) {
-    inner_scope = NewScope(BLOCK_SCOPE);
-    inner_scope->set_is_declaration_scope();
+    inner_scope = NewVarblockScope();
     inner_scope->set_start_position(scanner()->location().beg_pos);
     inner_block = factory()->NewBlock(NULL, 8, true, kNoSourcePosition);
     inner_block->set_scope(inner_scope);
@@ -4928,13 +4934,15 @@ ZoneList<Statement*>* Parser::ParseEagerFunctionBody(
 
   if (!parameters.is_simple) {
     DCHECK_NOT_NULL(inner_scope);
+    DCHECK_EQ(function_scope, scope());
+    DCHECK_EQ(function_scope, inner_scope->outer_scope());
     DCHECK_EQ(body, inner_block->statements());
-    SetLanguageMode(scope(), inner_scope->language_mode());
+    SetLanguageMode(function_scope, inner_scope->language_mode());
     Block* init_block = BuildParameterInitializationBlock(parameters, CHECK_OK);
 
     if (is_sloppy(inner_scope->language_mode())) {
-      InsertSloppyBlockFunctionVarBindings(
-          inner_scope, inner_scope->outer_scope(), CHECK_OK);
+      InsertSloppyBlockFunctionVarBindings(inner_scope, function_scope,
+                                           CHECK_OK);
     }
 
     if (IsAsyncFunction(kind)) {
@@ -4944,8 +4952,7 @@ ZoneList<Statement*>* Parser::ParseEagerFunctionBody(
     DCHECK_NOT_NULL(init_block);
 
     inner_scope->set_end_position(scanner()->location().end_pos);
-    inner_scope = inner_scope->FinalizeBlockScope();

[adamk, 2016/08/04 00:18:51]

I don't think it's worth changing this, there's some advantage to any further
attempts to touch a finalized scope crashing instead of continuing silently (it
also seems unrelated to this patch).

[Toon Verwaest, 2016/08/04 12:01:17]

inner_scope is a DeclarationScope, and FinalizeBlockScope returns a Scope*. I
now explicitly null out inner_scope to guarantee we don't use it at all
afterwards.

-    if (inner_scope != nullptr) {
+    if (inner_scope->FinalizeBlockScope() != nullptr) {
       CheckConflictingVarDeclarations(inner_scope, CHECK_OK);
       InsertShadowingVarBindingInitializers(inner_block);
     }
@@ -4953,8 +4960,9 @@ ZoneList<Statement*>* Parser::ParseEagerFunctionBody(
     result->Add(init_block, zone());
     result->Add(inner_block, zone());
   } else {
-    if (is_sloppy(inner_scope->language_mode())) {
-      InsertSloppyBlockFunctionVarBindings(inner_scope, nullptr, CHECK_OK);
+    DCHECK_EQ(inner_scope, function_scope);
+    if (is_sloppy(function_scope->language_mode())) {
+      InsertSloppyBlockFunctionVarBindings(function_scope, nullptr, CHECK_OK);
     }
   }
 
@@ -4964,6 +4972,7 @@ ZoneList<Statement*>* Parser::ParseEagerFunctionBody(
     // NOTE: We create a proxy and resolve it here so that in the
     // future we can change the AST to only refer to VariableProxies
     // instead of Variables and Proxies as is the case now.
+    DCHECK_EQ(function_scope, scope());
     VariableMode fvar_mode = is_strict(language_mode()) ? CONST : CONST_LEGACY;
     Variable* fvar = new (zone())
         Variable(scope(), function_name, fvar_mode, Variable::NORMAL,
@@ -4971,7 +4980,7 @@ ZoneList<Statement*>* Parser::ParseEagerFunctionBody(
     VariableProxy* proxy = factory()->NewVariableProxy(fvar);
     VariableDeclaration* fvar_declaration = factory()->NewVariableDeclaration(
         proxy, fvar_mode, scope(), kNoSourcePosition);
-    scope()->DeclareFunctionVar(fvar_declaration);
+    function_scope->DeclareFunctionVar(fvar_declaration);
 
     VariableProxy* fproxy = factory()->NewVariableProxy(fvar);
     result->Set(kFunctionNameAssignmentIndex,
@@ -5015,8 +5024,8 @@ PreParser::PreParseResult Parser::ParseLazyFunctionBodyWithPreParser(
   }
   PreParser::PreParseResult result = reusable_preparser_->PreParseLazyFunction(
       language_mode(), function_state_->kind(),
-      scope()->has_simple_parameters(), parsing_module_, logger, bookmark,
-      use_counts_);
+      scope()->AsDeclarationScope()->has_simple_parameters(), parsing_module_,
+      logger, bookmark, use_counts_);
   if (pre_parse_timer_ != NULL) {
     pre_parse_timer_->Stop();
   }
@@ -5142,7 +5151,7 @@ Expression* Parser::ParseClassLiteral(ExpressionClassifier* classifier,
   do_block->statements()->Add(
       factory()->NewExpressionStatement(class_literal, pos), zone());
   do_expr->set_represented_function(constructor);
-  Rewriter::Rewrite(this, scope()->ClosureScope(), do_expr,
+  Rewriter::Rewrite(this, scope()->GetClosureScope(), do_expr,
                     ast_value_factory());
 
   return do_expr;
@@ -5168,7 +5177,7 @@ Expression* Parser::ParseV8Intrinsic(bool* ok) {
   if (extension_ != NULL) {
     // The extension structures are only accessible while parsing the
     // very first time not when reparsing because of lazy compilation.
-    scope()->DeclarationScope()->ForceEagerCompilation();
+    scope()->GetDeclarationScope()->ForceEagerCompilation();
   }
 
   const Runtime::Function* function = Runtime::FunctionForName(name->string());
@@ -5261,12 +5270,11 @@ void Parser::InsertShadowingVarBindingInitializers(Block* inner_block) {
   }
 }
 
-void Parser::InsertSloppyBlockFunctionVarBindings(Scope* scope,
+void Parser::InsertSloppyBlockFunctionVarBindings(DeclarationScope* scope,
                                                   Scope* complex_params_scope,
                                                   bool* ok) {
   // For each variable which is used as a function declaration in a sloppy
   // block,
-  DCHECK(scope->is_declaration_scope());
   SloppyBlockFunctionMap* map = scope->sloppy_block_function_map();
   for (ZoneHashMap::Entry* p = map->Start(); p != nullptr; p = map->Next(p)) {
     AstRawString* name = static_cast<AstRawString*>(p->key);
@@ -5298,9 +5306,9 @@ void Parser::InsertSloppyBlockFunctionVarBindings(Scope* scope,
     auto delegates = static_cast<SloppyBlockFunctionMap::Vector*>(p->value);
     for (SloppyBlockFunctionStatement* delegate : *delegates) {
       // Check if there's a conflict with a lexical declaration
-      Scope* decl_scope = scope;
+      DeclarationScope* decl_scope = scope;
       while (decl_scope->is_eval_scope()) {
-        decl_scope = decl_scope->outer_scope()->DeclarationScope();
+        decl_scope = decl_scope->outer_scope()->GetDeclarationScope();
       }
       Scope* outer_scope = decl_scope->outer_scope();
       Scope* query_scope = delegate->scope()->outer_scope();
@@ -6652,7 +6660,7 @@ Expression* ParserTraits::RewriteYieldStar(
 
     Variable* dot_result = scope->NewTemporary(avfactory->dot_result_string());
     yield_star = factory->NewDoExpression(do_block, dot_result, nopos);
-    Rewriter::Rewrite(parser_, scope->ClosureScope(), yield_star, avfactory);
+    Rewriter::Rewrite(parser_, scope->GetClosureScope(), yield_star, avfactory);
   }
 
   return yield_star;