[parser] Parse declarations in the scope that they belong in ParseForStatement

That way we can drop PatternRewriter::scope_ and just use parser_->scope()
instead.

Change-Id: I66137d3ff8e7b805afc7108fd2d55537f69f11e6
Reviewed-on: https://chromium-review.googlesource.com/c/1387500Reviewed-by: Camillo Bruni <cbruni@chromium.org>
Commit-Queue: Toon Verwaest <verwaest@chromium.org>
Cr-Commit-Position: refs/heads/master@{#58432}

src/parsing/parser-base.h

@@ -483,7 +483,6 @@ class ParserBase {
 
   struct DeclarationDescriptor {
     enum Kind { NORMAL, PARAMETER, FOR_EACH };
-    Scope* scope;
     VariableMode mode;
     int declaration_pos;
     int initialization_pos;
@@ -3522,8 +3521,6 @@ typename ParserBase<Impl>::BlockT ParserBase<Impl>::ParseVariableDeclarations(
       break;
   }
 
-  parsing_result->descriptor.scope = scope();
-
   int bindings_start = peek_position();
   do {
     // Parse binding pattern.
@@ -5385,15 +5382,23 @@ typename ParserBase<Impl>::StatementT ParserBase<Impl>::ParseForStatement(
 
     Expect(Token::SEMICOLON);
 
-    StatementT init = impl()->BuildInitializationBlock(&for_info.parsing_result,
-                                                       &for_info.bound_names);
+    // Parse the remaining code in the inner block scope since the declaration
+    // above was parsed there. We'll finalize the unnecessary outer block scope
+    // after parsing the rest of the loop.
+    StatementT result = impl()->NullStatement();
+    inner_block_scope->set_start_position(scope()->start_position());
+    {
+      BlockState inner_state(&scope_, inner_block_scope);
+      StatementT init = impl()->BuildInitializationBlock(
+          &for_info.parsing_result, &for_info.bound_names);
 
-    Scope* finalized = inner_block_scope->FinalizeBlockScope();
-    // No variable declarations will have been created in inner_block_scope.
+      result = ParseStandardForLoopWithLexicalDeclarations(
+          stmt_pos, init, &for_info, labels, own_labels);
+    }
+    Scope* finalized = scope()->FinalizeBlockScope();
     DCHECK_NULL(finalized);
     USE(finalized);
-    return ParseStandardForLoopWithLexicalDeclarations(
-        stmt_pos, init, &for_info, labels, own_labels);
+    return result;
   }
 
   StatementT init = impl()->NullStatement();

src/parsing/parser.cc

@@ -1574,7 +1574,6 @@ Block* Parser::RewriteCatchPattern(CatchInfo* catch_info) {
   DCHECK_NOT_NULL(catch_info->pattern);
   DeclarationDescriptor descriptor;
   descriptor.declaration_kind = DeclarationDescriptor::NORMAL;
-  descriptor.scope = scope();
   descriptor.mode = VariableMode::kLet;
   descriptor.declaration_pos = catch_info->pattern->position();
   descriptor.initialization_pos = catch_info->pattern->position();
@@ -1885,7 +1884,6 @@ void Parser::DesugarBindingInForEachStatement(ForInfo* for_info,
     auto descriptor = for_info->parsing_result.descriptor;
     descriptor.declaration_pos = kNoSourcePosition;
     descriptor.initialization_pos = kNoSourcePosition;
-    descriptor.scope = scope();
     decl.initializer = factory()->NewVariableProxy(temp);
 
     bool is_for_var_of =
@@ -2819,7 +2817,6 @@ Block* Parser::BuildParameterInitializationBlock(
   for (auto parameter : parameters.params) {
     DeclarationDescriptor descriptor;
     descriptor.declaration_kind = DeclarationDescriptor::PARAMETER;
-    descriptor.scope = scope();
     descriptor.mode = VariableMode::kLet;
     descriptor.declaration_pos = parameter->pattern->position();
     // The position that will be used by the AssignmentExpression
@@ -2856,9 +2853,6 @@ Block* Parser::BuildParameterInitializationBlock(
       param_scope->RecordEvalCall();
       param_block = factory()->NewBlock(8, true);
       param_block->set_scope(param_scope);
-      // Pass the appropriate scope in so that PatternRewriter can appropriately
-      // rewrite inner initializers of the pattern to param_scope
-      descriptor.scope = param_scope;
       // Rewrite the outer initializer to point to param_scope
       ReparentExpressionScope(stack_limit(), initial_value, param_scope);
     }

src/parsing/pattern-rewriter.cc

@@ -32,20 +32,18 @@ class PatternRewriter final : public AstVisitor<PatternRewriter> {
       ZonePtrList<const AstRawString>* names);
 
   static Expression* RewriteDestructuringAssignment(Parser* parser,
-                                                    Assignment* to_rewrite,
-                                                    Scope* scope);
+                                                    Assignment* to_rewrite);
 
  private:
   enum PatternContext : uint8_t { BINDING, ASSIGNMENT };
 
-  PatternRewriter(Scope* scope, Parser* parser, PatternContext context,
+  PatternRewriter(Parser* parser, PatternContext context,
                   const DeclarationDescriptor* descriptor = nullptr,
                   ZonePtrList<const AstRawString>* names = nullptr,
                   int initializer_position = kNoSourcePosition,
                   int value_beg_position = kNoSourcePosition,
                   bool declares_parameter_containing_sloppy_eval = false)
-      : scope_(scope),
-        parser_(parser),
+      : parser_(parser),
         block_(nullptr),
         descriptor_(descriptor),
         names_(names),
@@ -114,9 +112,8 @@ class PatternRewriter final : public AstVisitor<PatternRewriter> {
   std::vector<void*>* pointer_buffer() { return parser_->pointer_buffer(); }
 
   Zone* zone() const { return parser_->zone(); }
-  Scope* scope() const { return scope_; }
+  Scope* scope() const { return parser_->scope(); }
 
-  Scope* const scope_;
   Parser* const parser_;
   Block* block_;
   const DeclarationDescriptor* descriptor_;
@@ -143,16 +140,15 @@ void Parser::DeclareAndInitializeVariables(
 void Parser::RewriteDestructuringAssignment(RewritableExpression* to_rewrite) {
   DCHECK(!to_rewrite->is_rewritten());
   Assignment* assignment = to_rewrite->expression()->AsAssignment();
-  Expression* result = PatternRewriter::RewriteDestructuringAssignment(
-      this, assignment, scope());
+  Expression* result =
+      PatternRewriter::RewriteDestructuringAssignment(this, assignment);
   to_rewrite->Rewrite(result);
 }
 
 Expression* Parser::RewriteDestructuringAssignment(Assignment* assignment) {
   DCHECK_NOT_NULL(assignment);
   DCHECK_EQ(Token::ASSIGN, assignment->op());
-  return PatternRewriter::RewriteDestructuringAssignment(this, assignment,
-                                                         scope());
+  return PatternRewriter::RewriteDestructuringAssignment(this, assignment);
 }
 
 void PatternRewriter::DeclareAndInitializeVariables(
@@ -162,13 +158,12 @@ void PatternRewriter::DeclareAndInitializeVariables(
     ZonePtrList<const AstRawString>* names) {
   DCHECK(block->ignore_completion_value());
 
-  Scope* scope = declaration_descriptor->scope;
-  PatternRewriter rewriter(scope, parser, BINDING, declaration_descriptor,
-                           names, declaration->initializer_position,
+  PatternRewriter rewriter(parser, BINDING, declaration_descriptor, names,
+                           declaration->initializer_position,
                            declaration->value_beg_position,
                            declaration_descriptor->declaration_kind ==
                                    DeclarationDescriptor::PARAMETER &&
-                               scope->is_block_scope());
+                               parser->scope()->is_block_scope());
   rewriter.block_ = block;
 
   rewriter.RecurseIntoSubpattern(declaration->pattern,
@@ -176,10 +171,10 @@ void PatternRewriter::DeclareAndInitializeVariables(
 }
 
 Expression* PatternRewriter::RewriteDestructuringAssignment(
-    Parser* parser, Assignment* to_rewrite, Scope* scope) {
-  DCHECK(!scope->HasBeenRemoved());
+    Parser* parser, Assignment* to_rewrite) {
+  DCHECK(!parser->scope()->HasBeenRemoved());
 
-  PatternRewriter rewriter(scope, parser, ASSIGNMENT);
+  PatternRewriter rewriter(parser, ASSIGNMENT);
   return rewriter.Rewrite(to_rewrite);
 }
 
@@ -199,16 +194,11 @@ void PatternRewriter::VisitVariableProxy(VariableProxy* proxy) {
   DCHECK_NOT_NULL(block_);
   DCHECK_NOT_NULL(descriptor_);
 
-  // scope() isn't guaranteed to be the same as parser_->scope(). scope() is
-  // where the pattern was parsed, whereas parser_->scope() is where the pattern
-  // is rewritten into as a declaration.
-  Scope* target_scope = parser_->scope();
+  Scope* target_scope = scope();
   if (declares_parameter_containing_sloppy_eval_) {
-    target_scope = scope()->outer_scope();
-    target_scope->DeleteUnresolved(proxy);
-  } else {
-    scope()->DeleteUnresolved(proxy);
+    target_scope = target_scope->outer_scope();
   }
+  target_scope->DeleteUnresolved(proxy);
 
   // Declare variable.
   // Note that we *always* must treat the initial value via a separate init
@@ -234,12 +224,11 @@ void PatternRewriter::VisitVariableProxy(VariableProxy* proxy) {
   DCHECK_NE(initializer_position_, kNoSourcePosition);
   var->set_initializer_position(initializer_position_);
   Scope* declaration_scope = var->scope();
-  DCHECK_EQ(declaration_scope,
-            declares_parameter_containing_sloppy_eval_
-                ? scope()->outer_scope()
-                : (IsLexicalVariableMode(descriptor_->mode)
-                       ? parser_->scope()
-                       : parser_->scope()->GetDeclarationScope()));
+  DCHECK_EQ(declaration_scope, declares_parameter_containing_sloppy_eval_
+                                   ? scope()->outer_scope()
+                                   : (IsLexicalVariableMode(descriptor_->mode)
+                                          ? scope()
+                                          : scope()->GetDeclarationScope()));
   if (declaration_scope->num_var() > kMaxNumFunctionLocals) {
     parser_->ReportMessage(MessageTemplate::kTooManyVariables);
     return;

src/parsing/preparser.cc

@@ -434,14 +434,14 @@ void PreParser::DeclareAndInitializeVariables(
     ZonePtrList<const AstRawString>* names) {
   if (declaration->pattern.variables_ != nullptr) {
     for (auto variable : *(declaration->pattern.variables_)) {
-      declaration_descriptor->scope->DeleteUnresolved(variable);
+      scope()->DeleteUnresolved(variable);
       Variable* var = scope()->DeclareVariableName(
           variable->raw_name(), declaration_descriptor->mode);
       if (var == nullptr) {
         ReportUnidentifiableError();
         return;
       }
-      MarkLoopVariableAsAssigned(declaration_descriptor->scope, var,
+      MarkLoopVariableAsAssigned(scope(), var,
                                  declaration_descriptor->declaration_kind);
       // This is only necessary if there is an initializer, but we don't have
       // that information here.  Consequently, the preparser sometimes says