Allocate block scopes in block states when possible

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 "src/api.h"
 #include "src/ast/ast.h"
 #include "src/ast/ast-expression-rewriter.h"
 #include "src/ast/ast-expression-visitor.h"
@@ skipping 2272 matching lines @@
 }
 
 Block* Parser::ParseBlock(ZoneList<const AstRawString*>* labels, bool* ok) {
   // The harmony mode uses block elements instead of statements.
   //
   // Block ::
   //   '{' StatementList '}'
 
   // Construct block expecting 16 statements.
   Block* body = factory()->NewBlock(labels, 16, false, kNoSourcePosition);
-  Scope* block_scope = NewScope(BLOCK_SCOPE);
 
   // Parse the statements and collect escaping labels.
   Expect(Token::LBRACE, CHECK_OK);
-  block_scope->set_start_position(scanner()->location().beg_pos);
   {
-    BlockState block_state(&scope_state_, block_scope);
+    BlockState block_state(&scope_state_);
+    block_state.set_start_position(scanner()->location().beg_pos);
     Target target(&this->target_stack_, body);
 
     while (peek() != Token::RBRACE) {
       Statement* stat = ParseStatementListItem(CHECK_OK);
       if (stat && !stat->IsEmpty()) {
         body->statements()->Add(stat, zone());
       }
     }
+
+    Expect(Token::RBRACE, CHECK_OK);
+    block_state.set_end_position(scanner()->location().end_pos);
+    body->set_scope(block_state.FinalizedBlockScope());

[adamk, 2016/07/21 17:48:05]

Calling FinalizeBlockScope while still inside the block_state is a little scary,
since scope() is now this weird detached thing. What are the chances we could
pass the Block to the BlockState constructor? I realize that wouldn't work
everywhere though.

[marja, 2016/07/22 07:17:00]

(I like this version because it has the set_scope and FinalizeBlockScope calls
here, instead of hiding them inside the *State helpers. If we push too much
logic there, the AST - Scope connections will become more difficult to follow.
But that's a matter of taste, I don't have a strong opinion about this.)

   }
-  Expect(Token::RBRACE, CHECK_OK);
-  block_scope->set_end_position(scanner()->location().end_pos);
-  block_scope = block_scope->FinalizeBlockScope();
-  body->set_scope(block_scope);
   return body;
 }
 
 
 Block* Parser::DeclarationParsingResult::BuildInitializationBlock(
     ZoneList<const AstRawString*>* names, bool* ok) {
   Block* result = descriptor.parser->factory()->NewBlock(
       NULL, 1, true, descriptor.declaration_pos);
   for (auto declaration : declarations) {
     PatternRewriter::DeclareAndInitializeVariables(
@@ skipping 577 matching lines @@
 
   // make statement: undefined;
   // This is needed so the tag isn't returned as the value, in case the switch
   // statements don't have a value.
   switch_block->statements()->Add(
       factory()->NewExpressionStatement(
           factory()->NewUndefinedLiteral(kNoSourcePosition), kNoSourcePosition),
       zone());
 
   Block* cases_block = factory()->NewBlock(NULL, 1, false, kNoSourcePosition);
-  Scope* cases_scope = NewScope(BLOCK_SCOPE);
-  cases_scope->SetNonlinear();
 
   SwitchStatement* switch_statement =
       factory()->NewSwitchStatement(labels, switch_pos);
 
-  cases_scope->set_start_position(scanner()->location().beg_pos);
   {
-    BlockState cases_block_state(&scope_state_, cases_scope);
+    BlockState cases_block_state(&scope_state_);
+    cases_block_state.set_start_position(scanner()->location().beg_pos);
+    cases_block_state.SetNonlinear();
     Target target(&this->target_stack_, switch_statement);
 
     Expression* tag_read = factory()->NewVariableProxy(tag_variable);
 
     bool default_seen = false;
     ZoneList<CaseClause*>* cases =
         new (zone()) ZoneList<CaseClause*>(4, zone());
     Expect(Token::LBRACE, CHECK_OK);
     while (peek() != Token::RBRACE) {
       CaseClause* clause = ParseCaseClause(&default_seen, CHECK_OK);
       cases->Add(clause, zone());
     }
     switch_statement->Initialize(tag_read, cases);
     cases_block->statements()->Add(switch_statement, zone());
+    Expect(Token::RBRACE, CHECK_OK);
+
+    cases_block_state.set_end_position(scanner()->location().end_pos);
+    cases_block->set_scope(cases_block_state.FinalizedBlockScope());
   }
-  Expect(Token::RBRACE, CHECK_OK);
-
-  cases_scope->set_end_position(scanner()->location().end_pos);
-  cases_scope = cases_scope->FinalizeBlockScope();
-  cases_block->set_scope(cases_scope);
 
   switch_block->statements()->Add(cases_block, zone());
 
   return switch_block;
 }
 
 
 Statement* Parser::ParseThrowStatement(bool* ok) {
   // ThrowStatement ::
   //   'throw' Expression ';'
@@ skipping 56 matching lines @@
     {
       CollectExpressionsInTailPositionToListScope
           collect_tail_call_expressions_scope(
               function_state_, &tail_call_expressions_in_catch_block);
       BlockState block_state(&scope_state_, catch_scope);
 
       catch_block = factory()->NewBlock(nullptr, 16, false, kNoSourcePosition);
 
       // Create a block scope to hold any lexical declarations created
       // as part of destructuring the catch parameter.
-      Scope* block_scope = NewScope(BLOCK_SCOPE);
-      block_scope->set_start_position(scanner()->location().beg_pos);
       {
-        BlockState block_state(&scope_state_, block_scope);
+        BlockState block_state(&scope_state_);
+        block_state.set_start_position(scanner()->location().beg_pos);
         Target target(&this->target_stack_, catch_block);
 
         const AstRawString* name = ast_value_factory()->dot_catch_string();
         Expression* pattern = nullptr;
         if (peek_any_identifier()) {
           name = ParseIdentifier(kDontAllowRestrictedIdentifiers, CHECK_OK);
         } else {
           ExpressionClassifier pattern_classifier(this);
           pattern = ParsePrimaryExpression(&pattern_classifier, CHECK_OK);
           ValidateBindingPattern(&pattern_classifier, CHECK_OK);
@@ skipping 45 matching lines @@
             Scanner::Location location =
                 position == kNoSourcePosition
                     ? Scanner::Location::invalid()
                     : Scanner::Location(position, position + 1);
             ParserTraits::ReportMessageAt(
                 location, MessageTemplate::kVarRedeclaration, name);
             *ok = false;
             return nullptr;
           }
         }
+        block_state.set_end_position(scanner()->location().end_pos);
+        catch_block->set_scope(block_state.FinalizedBlockScope());
       }
-      block_scope->set_end_position(scanner()->location().end_pos);
-      block_scope = block_scope->FinalizeBlockScope();
-      catch_block->set_scope(block_scope);
     }
 
     catch_scope->set_end_position(scanner()->location().end_pos);
     tok = peek();
   }
 
   Block* finally_block = NULL;
   DCHECK(tok == Token::FINALLY || catch_block != NULL);
   if (tok == Token::FINALLY) {
     Consume(Token::FINALLY);
@@ skipping 534 matching lines @@
                                         bool legacy, bool* ok) {
   if (is_strict(language_mode()) || peek() != Token::FUNCTION ||
       (legacy && allow_harmony_restrictive_declarations())) {
     return ParseSubStatement(labels, kDisallowLabelledFunctionStatement, ok);
   } else {
     if (legacy) {
       ++use_counts_[v8::Isolate::kLegacyFunctionDeclaration];
     }
     // Make a block around the statement for a lexical binding
     // is introduced by a FunctionDeclaration.
-    Scope* body_scope = NewScope(BLOCK_SCOPE);
-    body_scope->set_start_position(scanner()->location().beg_pos);
-    BlockState block_state(&scope_state_, body_scope);
+    BlockState block_state(&scope_state_);
+    block_state.set_start_position(scanner()->location().beg_pos);
     Block* block = factory()->NewBlock(NULL, 1, false, kNoSourcePosition);
     Statement* body = ParseFunctionDeclaration(CHECK_OK);
     block->statements()->Add(body, zone());
-    body_scope->set_end_position(scanner()->location().end_pos);
-    body_scope = body_scope->FinalizeBlockScope();
-    block->set_scope(body_scope);
+    block_state.set_end_position(scanner()->location().end_pos);
+    block->set_scope(block_state.FinalizedBlockScope());
     return block;
   }
 }
 
 Statement* Parser::ParseForStatement(ZoneList<const AstRawString*>* labels,
                                      bool* ok) {
   int stmt_pos = peek_position();
   Statement* init = NULL;
   ZoneList<const AstRawString*> bound_names(1, zone());
   bool bound_names_are_lexical = false;
 
   // Create an in-between scope for let-bound iteration variables.
-  Scope* for_scope = NewScope(BLOCK_SCOPE);
-
-  BlockState block_state(&scope_state_, for_scope);
+  BlockState for_state(&scope_state_);
   Expect(Token::FOR, CHECK_OK);
   Expect(Token::LPAREN, CHECK_OK);
-  for_scope->set_start_position(scanner()->location().beg_pos);
-  for_scope->set_is_hidden();
+  for_state.set_start_position(scanner()->location().beg_pos);
+  for_state.set_is_hidden();
   DeclarationParsingResult parsing_result;
   if (peek() != Token::SEMICOLON) {
     if (peek() == Token::VAR || peek() == Token::CONST ||
         (peek() == Token::LET && IsNextLetKeyword())) {
       ParseVariableDeclarations(kForStatement, &parsing_result, nullptr,
                                 CHECK_OK);
 
       ForEachStatement::VisitMode mode = ForEachStatement::ENUMERATE;
       int each_beg_pos = scanner()->location().beg_pos;
       int each_end_pos = scanner()->location().end_pos;
@@ skipping 78 matching lines @@
         if (mode == ForEachStatement::ITERATE) {
           ExpressionClassifier classifier(this);
           enumerable = ParseAssignmentExpression(true, &classifier, CHECK_OK);
           RewriteNonPattern(&classifier, CHECK_OK);
         } else {
           enumerable = ParseExpression(true, CHECK_OK);
         }
 
         Expect(Token::RPAREN, CHECK_OK);
 
-        Scope* body_scope = NewScope(BLOCK_SCOPE);
-        body_scope->set_start_position(scanner()->location().beg_pos);
 
         Block* body_block =
             factory()->NewBlock(NULL, 3, false, kNoSourcePosition);
 
         Statement* final_loop;
         {
           ReturnExprScope no_tail_calls(function_state_,
                                         ReturnExprContext::kInsideForInOfBody);
-          BlockState block_state(&scope_state_, body_scope);
+          BlockState block_state(&scope_state_);
+          block_state.set_start_position(scanner()->location().beg_pos);
 
           Statement* body = ParseScopedStatement(NULL, true, CHECK_OK);
 
           auto each_initialization_block =
               factory()->NewBlock(nullptr, 1, true, kNoSourcePosition);
           {
             auto descriptor = parsing_result.descriptor;
             descriptor.declaration_pos = kNoSourcePosition;
             descriptor.initialization_pos = kNoSourcePosition;
             decl.initializer = factory()->NewVariableProxy(temp);
@@ skipping 37 matching lines @@
               }
             }
           }
 
           body_block->statements()->Add(each_initialization_block, zone());
           body_block->statements()->Add(body, zone());
           VariableProxy* temp_proxy =
               factory()->NewVariableProxy(temp, each_beg_pos, each_end_pos);
           final_loop = InitializeForEachStatement(
               loop, temp_proxy, enumerable, body_block, each_keyword_position);
+          block_state.set_end_position(scanner()->location().end_pos);
+          body_block->set_scope(block_state.FinalizedBlockScope());
         }
-        body_scope->set_end_position(scanner()->location().end_pos);
-        body_scope = body_scope->FinalizeBlockScope();
-        body_block->set_scope(body_scope);
 
         // Create a TDZ for any lexically-bound names.
         if (bound_names_are_lexical) {
           DCHECK_NULL(init_block);
 
           init_block =
               factory()->NewBlock(nullptr, 1, false, kNoSourcePosition);
 
           for (int i = 0; i < bound_names.length(); ++i) {
             // TODO(adamk): This needs to be some sort of special
             // INTERNAL variable that's invisible to the debugger
             // but visible to everything else.
             VariableProxy* tdz_proxy = NewUnresolved(bound_names[i], LET);
             Declaration* tdz_decl = factory()->NewVariableDeclaration(
                 tdz_proxy, LET, scope(), kNoSourcePosition);
             Variable* tdz_var = Declare(
                 tdz_decl, DeclarationDescriptor::NORMAL, true, CHECK_OK);
             tdz_var->set_initializer_position(position());
           }
         }
 
-        for_scope->set_end_position(scanner()->location().end_pos);
-        for_scope = for_scope->FinalizeBlockScope();
+        for_state.set_end_position(scanner()->location().end_pos);
+        Scope* for_scope = for_state.FinalizedBlockScope();
         // Parsed for-in loop w/ variable declarations.
         if (init_block != nullptr) {
           init_block->statements()->Add(final_loop, zone());
           init_block->set_scope(for_scope);
           return init_block;
         } else {
           DCHECK_NULL(for_scope);
           return final_loop;
         }
       } else {
@@ skipping 42 matching lines @@
         }
 
         Expect(Token::RPAREN, CHECK_OK);
 
         // For legacy compat reasons, give for loops similar treatment to
         // if statements in allowing a function declaration for a body
         Statement* body = ParseScopedStatement(NULL, true, CHECK_OK);
         Statement* final_loop = InitializeForEachStatement(
             loop, expression, enumerable, body, each_keyword_position);
 
-        for_scope->set_end_position(scanner()->location().end_pos);
-        for_scope = for_scope->FinalizeBlockScope();
-        DCHECK(for_scope == nullptr);
+        DCHECK_NULL(for_state.FinalizedBlockScope());
         return final_loop;
 
       } else {
         init = factory()->NewExpressionStatement(expression, lhs_beg_pos);
       }
     }
   }
 
   // Standard 'for' loop
   ForStatement* loop = factory()->NewForStatement(labels, stmt_pos);
   Target target(&this->target_stack_, loop);
 
   // Parsed initializer at this point.
   Expect(Token::SEMICOLON, CHECK_OK);
 
   Expression* cond = NULL;
   Statement* next = NULL;
   Statement* body = NULL;
 
   // If there are let bindings, then condition and the next statement of the
   // for loop must be parsed in a new scope.
   Scope* inner_scope = scope();
+  // TODO(verwaest): Allocate this through a ScopeState as well.
   if (bound_names_are_lexical && bound_names.length() > 0) {
-    inner_scope = NewScopeWithParent(for_scope, BLOCK_SCOPE);
+    inner_scope = NewScopeWithParent(inner_scope, BLOCK_SCOPE);
     inner_scope->set_start_position(scanner()->location().beg_pos);
   }
   {
     BlockState block_state(&scope_state_, inner_scope);
 
     if (peek() != Token::SEMICOLON) {
       cond = ParseExpression(true, CHECK_OK);
     }
     Expect(Token::SEMICOLON, CHECK_OK);
 
     if (peek() != Token::RPAREN) {
       Expression* exp = ParseExpression(true, CHECK_OK);
       next = factory()->NewExpressionStatement(exp, exp->position());
     }
     Expect(Token::RPAREN, CHECK_OK);
 
     body = ParseScopedStatement(NULL, true, CHECK_OK);
   }
 
   Statement* result = NULL;
   if (bound_names_are_lexical && bound_names.length() > 0) {
-    BlockState block_state(&scope_state_, for_scope);
     result = DesugarLexicalBindingsInForStatement(
         inner_scope, parsing_result.descriptor.mode, &bound_names, loop, init,
         cond, next, body, CHECK_OK);
-    for_scope->set_end_position(scanner()->location().end_pos);
+    for_state.set_end_position(scanner()->location().end_pos);
   } else {
-    for_scope->set_end_position(scanner()->location().end_pos);
-    for_scope = for_scope->FinalizeBlockScope();
+    for_state.set_end_position(scanner()->location().end_pos);
+    Scope* for_scope = for_state.FinalizedBlockScope();
     if (for_scope) {
       // Rewrite a for statement of the form
       //   for (const x = i; c; n) b
       //
       // into
       //
       //   {
       //     const x = i;
       //     for (; c; n) b
       //   }
@@ skipping 997 matching lines @@
 ClassLiteral* Parser::ParseClassLiteral(ExpressionClassifier* classifier,
                                         const AstRawString* name,
                                         Scanner::Location class_name_location,
                                         bool name_is_strict_reserved, int pos,
                                         bool* ok) {
   // All parts of a ClassDeclaration and ClassExpression are strict code.
   if (name_is_strict_reserved) {
     ReportMessageAt(class_name_location,
                     MessageTemplate::kUnexpectedStrictReserved);
     *ok = false;
-    return NULL;
+    return nullptr;
   }
   if (IsEvalOrArguments(name)) {
     ReportMessageAt(class_name_location, MessageTemplate::kStrictEvalArguments);
     *ok = false;
-    return NULL;
+    return nullptr;
   }
 
-  Scope* block_scope = NewScope(BLOCK_SCOPE);
-  BlockState block_state(&scope_state_, block_scope);
+  BlockState block_state(&scope_state_);
   RaiseLanguageMode(STRICT);
   scope()->SetScopeName(name);
 
-  VariableProxy* proxy = NULL;
-  if (name != NULL) {
+  VariableProxy* proxy = nullptr;
+  if (name != nullptr) {
     proxy = NewUnresolved(name, CONST);
-    Declaration* declaration =
-        factory()->NewVariableDeclaration(proxy, CONST, block_scope, pos);
+    // TODO(verwaest): declare via block_state.
+    Declaration* declaration = factory()->NewVariableDeclaration(
+        proxy, CONST, block_state.scope(), pos);
     Declare(declaration, DeclarationDescriptor::NORMAL, true, CHECK_OK);
   }
 
-  Expression* extends = NULL;
+  Expression* extends = nullptr;
   if (Check(Token::EXTENDS)) {
-    block_scope->set_start_position(scanner()->location().end_pos);
+    block_state.set_start_position(scanner()->location().end_pos);
     ExpressionClassifier extends_classifier(this);
     extends = ParseLeftHandSideExpression(&extends_classifier, CHECK_OK);
     CheckNoTailCallExpressions(&extends_classifier, CHECK_OK);
     RewriteNonPattern(&extends_classifier, CHECK_OK);
     if (classifier != nullptr) {
       classifier->Accumulate(&extends_classifier,
                              ExpressionClassifier::ExpressionProductions);
     }
   } else {
-    block_scope->set_start_position(scanner()->location().end_pos);
+    block_state.set_start_position(scanner()->location().end_pos);
   }
 
 
   ClassLiteralChecker checker(this);
   ZoneList<ObjectLiteral::Property*>* properties = NewPropertyList(4, zone());
-  FunctionLiteral* constructor = NULL;
+  FunctionLiteral* constructor = nullptr;
   bool has_seen_constructor = false;
 
   Expect(Token::LBRACE, CHECK_OK);
 
   const bool has_extends = extends != nullptr;
   while (peek() != Token::RBRACE) {
     if (Check(Token::SEMICOLON)) continue;
     FuncNameInferrer::State fni_state(fni_);
     const bool in_class = true;
     bool is_computed_name = false;  // Classes do not care about computed
                                     // property names here.
     ExpressionClassifier property_classifier(this);
     const AstRawString* property_name = nullptr;
     ObjectLiteral::Property* property = ParsePropertyDefinition(
         &checker, in_class, has_extends, MethodKind::Normal, &is_computed_name,
         &has_seen_constructor, &property_classifier, &property_name, CHECK_OK);
     RewriteNonPattern(&property_classifier, CHECK_OK);
     if (classifier != nullptr) {
       classifier->Accumulate(&property_classifier,
                              ExpressionClassifier::ExpressionProductions);
     }
 
-    if (has_seen_constructor && constructor == NULL) {
+    if (has_seen_constructor && constructor == nullptr) {
       constructor = GetPropertyValue(property)->AsFunctionLiteral();
       DCHECK_NOT_NULL(constructor);
       constructor->set_raw_name(
           name != nullptr ? name : ast_value_factory()->empty_string());
     } else {
       properties->Add(property, zone());
     }
 
-    if (fni_ != NULL) fni_->Infer();
+    if (fni_ != nullptr) fni_->Infer();
 
     if (property_name != ast_value_factory()->constructor_string()) {
       SetFunctionNameFromPropertyName(property, property_name);
     }
   }
 
   Expect(Token::RBRACE, CHECK_OK);
   int end_pos = scanner()->location().end_pos;
 
-  if (constructor == NULL) {
-    DCHECK_EQ(this->scope(), block_scope);
+  if (constructor == nullptr) {
     constructor = DefaultConstructor(name, has_extends, pos, end_pos,
-                                     block_scope->language_mode());
+                                     block_state.language_mode());
   }
 
   // Note that we do not finalize this block scope because it is
   // used as a sentinel value indicating an anonymous class.
-  block_scope->set_end_position(end_pos);
+  block_state.set_end_position(end_pos);
 
-  if (name != NULL) {
+  if (name != nullptr) {
     DCHECK_NOT_NULL(proxy);
     proxy->var()->set_initializer_position(end_pos);
   }
 
+  // TODO(verwaest): Use FinalizedBlockScope() once supported.
+  Scope* block_scope = block_state.scope();
   return factory()->NewClassLiteral(block_scope, proxy, extends, constructor,
                                     properties, pos, end_pos);
 }
 
 
 Expression* Parser::ParseV8Intrinsic(bool* ok) {
   // CallRuntime ::
   //   '%' Identifier Arguments
 
   int pos = peek_position();
@@ skipping 1957 matching lines @@
   node->Print(Isolate::Current());
 }
 #endif  // DEBUG
 
 #undef CHECK_OK
 #undef CHECK_OK_VOID
 #undef CHECK_FAILED
 
 }  // namespace internal
 }  // namespace v8