[parser] Alternative fix for chromium:740591

- Previous fix is https://chromium-review.googlesource.com/c/583531 but it
  diverges Scopes created by PreParser from Scopes created by Parser.

- This CL creates the inner block scope a bit earlier and (temporarily) pushes
  it into the scope chain for parsing the variable declarations in a for
  loop. The previous approach was to first parse the variable declarations and
  then reparent the AST nodes / Scopes created while parsing it afterwards.

- This CL partially reverts https://chromium-review.googlesource.com/c/583531;
  the new fix only touches parser-base.h (diff between patch sets 2 and 3 is the
  fix).

- The Ignition golden changes are basically undoing the changes done in that CL
  too.

Bug: chromium:740591
Change-Id: Iceff1383ef066317e754942bb5ff0c70a91bc937
Reviewed-on: https://chromium-review.googlesource.com/603787
Commit-Queue: Marja Hölttä <marja@chromium.org>
Reviewed-by: Adam Klein <adamk@chromium.org>
Reviewed-by: Georg Neis <neis@chromium.org>
Reviewed-by: Yang Guo <yangguo@chromium.org>
Cr-Commit-Position: refs/heads/master@{#47241}

src/parsing/expression-scope-reparenter.cc

@@ -93,17 +93,12 @@ void Reparenter::VisitWithStatement(WithStatement* stmt) {
 
 void ReparentExpressionScope(uintptr_t stack_limit, Expression* expr,
                              Scope* scope) {
-  // Both uses of this function should pass in a block scope.
+  // The only case that uses this code is block scopes for parameters containing
+  // sloppy eval.
   DCHECK(scope->is_block_scope());
-  // These hold for the sloppy parameters-with-eval case...
-  DCHECK_IMPLIES(scope->is_declaration_scope(), scope->calls_sloppy_eval());
-  DCHECK_IMPLIES(scope->is_declaration_scope(),
-                 scope->outer_scope()->is_function_scope());
-  // ...whereas these hold for lexical declarations in for-in/of loops.
-  DCHECK_IMPLIES(!scope->is_declaration_scope(),
-                 scope->outer_scope()->is_block_scope());
-  DCHECK_IMPLIES(!scope->is_declaration_scope(),
-                 scope->outer_scope()->is_hidden());
+  DCHECK(scope->is_declaration_scope());
+  DCHECK(scope->calls_sloppy_eval());
+  DCHECK(scope->outer_scope()->is_function_scope());
 
   Reparenter r(stack_limit, expr, scope);
   r.Run();

src/parsing/expression-scope-reparenter.h

@@ -16,8 +16,7 @@ class Scope;
 // When an extra declaration scope needs to be inserted to account for
 // a sloppy eval in a default parameter or function body, the expressions
 // needs to be in that new inner scope which was added after initial
-// parsing. We do the same rewriting for initializers of destructured
-// lexical declarations in for-in/of loops.
+// parsing.
 //
 // scope is the new inner scope, and its outer_scope() is assumed
 // to be the scope which was used during the initial parse.

src/parsing/parser-base.h

@@ -485,7 +485,7 @@ class ParserBase {
   };
 
   struct DeclarationDescriptor {
-    enum Kind { NORMAL, PARAMETER, LEXICAL_FOR_EACH };
+    enum Kind { NORMAL, PARAMETER };
     Scope* scope;
     VariableMode mode;
     int declaration_pos;
@@ -1224,7 +1224,7 @@ class ParserBase {
   StatementT ParseForStatement(ZoneList<const AstRawString*>* labels, bool* ok);
   StatementT ParseForEachStatementWithDeclarations(
       int stmt_pos, ForInfo* for_info, ZoneList<const AstRawString*>* labels,
-      bool* ok);
+      Scope* inner_block_scope, bool* ok);
   StatementT ParseForEachStatementWithoutDeclarations(
       int stmt_pos, ExpressionT expression, int lhs_beg_pos, int lhs_end_pos,
       ForInfo* for_info, ZoneList<const AstRawString*>* labels, bool* ok);
@@ -5533,21 +5533,33 @@ typename ParserBase<Impl>::StatementT ParserBase<Impl>::ParseForStatement(
   if (peek() == Token::VAR || peek() == Token::CONST ||
       (peek() == Token::LET && IsNextLetKeyword())) {
     // The initializer contains declarations.
-    ParseVariableDeclarations(kForStatement, &for_info.parsing_result, nullptr,
-                              CHECK_OK);
+
+    // Create an inner block scope which will be the parent scope of scopes
+    // possibly created by ParseVariableDeclarations.
+    Scope* inner_block_scope = NewScope(BLOCK_SCOPE);
+    {
+      BlockState inner_state(&scope_, inner_block_scope);
+      ParseVariableDeclarations(kForStatement, &for_info.parsing_result,
+                                nullptr, CHECK_OK);
+    }
     bound_names_are_lexical =
         IsLexicalVariableMode(for_info.parsing_result.descriptor.mode);
     for_info.position = scanner()->location().beg_pos;
 
     if (CheckInOrOf(&for_info.mode)) {
       return ParseForEachStatementWithDeclarations(stmt_pos, &for_info, labels,
-                                                   ok);
+                                                   inner_block_scope, ok);
     }
 
     // One or more declaration not followed by in/of.
     init = impl()->BuildInitializationBlock(
         &for_info.parsing_result,
         bound_names_are_lexical ? &for_info.bound_names : nullptr, CHECK_OK);
+
+    Scope* finalized = inner_block_scope->FinalizeBlockScope();
+    // No variable declarations will have been created in inner_block_scope.
+    DCHECK_NULL(finalized);
+    USE(finalized);
   } else if (peek() != Token::SEMICOLON) {
     // The initializer does not contain declarations.
     int lhs_beg_pos = peek_position();
@@ -5583,7 +5595,7 @@ template <typename Impl>
 typename ParserBase<Impl>::StatementT
 ParserBase<Impl>::ParseForEachStatementWithDeclarations(
     int stmt_pos, ForInfo* for_info, ZoneList<const AstRawString*>* labels,
-    bool* ok) {
+    Scope* inner_block_scope, bool* ok) {
   scope()->set_is_hidden();
   // Just one declaration followed by in/of.
   if (for_info->parsing_result.declarations.length() != 1) {
@@ -5624,7 +5636,7 @@ ParserBase<Impl>::ParseForEachStatementWithDeclarations(
 
   StatementT final_loop = impl()->NullStatement();
   {
-    BlockState block_state(zone(), &scope_);
+    BlockState block_state(&scope_, inner_block_scope);
     scope()->set_start_position(scanner()->location().beg_pos);
 
     SourceRange body_range;
@@ -5836,6 +5848,7 @@ typename ParserBase<Impl>::StatementT ParserBase<Impl>::ParseForAwaitStatement(
   ExpressionT each_variable = impl()->EmptyExpression();
 
   bool has_declarations = false;
+  Scope* inner_block_scope = NewScope(BLOCK_SCOPE);
 
   if (peek() == Token::VAR || peek() == Token::CONST ||
       (peek() == Token::LET && IsNextLetKeyword())) {
@@ -5845,8 +5858,12 @@ typename ParserBase<Impl>::StatementT ParserBase<Impl>::ParseForAwaitStatement(
     // 'for' 'await' '(' 'var' ForBinding 'of' AssignmentExpression ')'
     //     Statement
     has_declarations = true;
-    ParseVariableDeclarations(kForStatement, &for_info.parsing_result, nullptr,
-                              CHECK_OK);
+
+    {
+      BlockState inner_state(&scope_, inner_block_scope);
+      ParseVariableDeclarations(kForStatement, &for_info.parsing_result,
+                                nullptr, CHECK_OK);
+    }
     for_info.position = scanner()->location().beg_pos;
 
     // Only a single declaration is allowed in for-await-of loops
@@ -5871,6 +5888,7 @@ typename ParserBase<Impl>::StatementT ParserBase<Impl>::ParseForAwaitStatement(
     // 'for' 'await' '(' LeftHandSideExpression 'of' AssignmentExpression ')'
     //     Statement
     int lhs_beg_pos = peek_position();
+    BlockState inner_state(&scope_, inner_block_scope);
     ExpressionClassifier classifier(this);
     ExpressionT lhs = each_variable = ParseLeftHandSideExpression(CHECK_OK);
     int lhs_end_pos = scanner()->location().end_pos;
@@ -5902,7 +5920,7 @@ typename ParserBase<Impl>::StatementT ParserBase<Impl>::ParseForAwaitStatement(
   StatementT final_loop = impl()->NullStatement();
   Scope* for_scope = scope();
   {
-    BlockState block_state(zone(), &scope_);
+    BlockState block_state(&scope_, inner_block_scope);
     scope()->set_start_position(scanner()->location().beg_pos);
 
     SourceRange body_range;

src/parsing/parser.cc

@@ -2013,7 +2013,6 @@ void Parser::DesugarBindingInForEachStatement(ForInfo* for_info,
     descriptor.declaration_pos = kNoSourcePosition;
     descriptor.initialization_pos = kNoSourcePosition;
     descriptor.scope = scope();
-    descriptor.declaration_kind = DeclarationDescriptor::LEXICAL_FOR_EACH;
     decl.initializer = factory()->NewVariableProxy(temp);
 
     bool is_for_var_of =

src/parsing/pattern-rewriter.cc

@@ -289,13 +289,11 @@ void Parser::PatternRewriter::VisitRewritableExpression(
   set_context(old_context);
 }
 
-// When an extra declaration scope needs to be inserted to account for
-// a sloppy eval in a default parameter or function body, the expressions
-// needs to be in that new inner scope which was added after initial
-// parsing.
 bool Parser::PatternRewriter::DeclaresParameterContainingSloppyEval() const {
-  DCHECK(IsBindingContext());
-  if (descriptor_->declaration_kind == DeclarationDescriptor::PARAMETER &&
+  // Need to check for a binding context to make sure we have a descriptor.
+  if (IsBindingContext() &&
+      // Only relevant for parameters.
+      descriptor_->declaration_kind == DeclarationDescriptor::PARAMETER &&
       // And only when scope is a block scope;
       // without eval, it is a function scope.
       scope()->is_block_scope()) {
@@ -308,11 +306,12 @@ bool Parser::PatternRewriter::DeclaresParameterContainingSloppyEval() const {
   return false;
 }
 
+// When an extra declaration scope needs to be inserted to account for
+// a sloppy eval in a default parameter or function body, the expressions
+// needs to be in that new inner scope which was added after initial
+// parsing.
 void Parser::PatternRewriter::RewriteParameterScopes(Expression* expr) {
-  if (!IsBindingContext()) return;
-  if (DeclaresParameterContainingSloppyEval() ||
-      descriptor_->declaration_kind ==
-          DeclarationDescriptor::LEXICAL_FOR_EACH) {
+  if (DeclaresParameterContainingSloppyEval()) {
     ReparentExpressionScope(parser_->stack_limit(), expr, scope());
   }
 }

test/cctest/interpreter/bytecode_expectations/AsyncGenerators.golden

@@ -366,7 +366,7 @@ snippet: "
 "
 frame size: 23
 parameter count: 1
-bytecode array length: 697
+bytecode array length: 694
 bytecodes: [
                 B(Mov), R(new_target), R(10),
                 B(Ldar), R(new_target),
@@ -422,8 +422,6 @@ bytecodes: [
                 B(Star), R(12),
                 B(Mov), R(17), R(13),
                 B(JumpConstant), U8(24),
-                B(LdaTheHole),
-                B(Star), R(1),
                 B(LdaZero),
                 B(Star), R(6),
                 B(Mov), R(context), R(18),
@@ -661,9 +659,9 @@ bytecodes: [
 constant pool: [
   Smi [46],
   Smi [87],
-  Smi [160],
-  Smi [160],
-  Smi [532],
+  Smi [157],
+  Smi [157],
+  Smi [529],
   Smi [59],
   Smi [7],
   TUPLE2_TYPE,
@@ -683,17 +681,17 @@ constant pool: [
   Smi [6],
   Smi [14],
   FIXED_ARRAY_TYPE,
-  Smi [498],
+  Smi [495],
   Smi [6],
   Smi [28],
   Smi [35],
 ]
 handlers: [
-  [52, 624, 630],
-  [55, 585, 587],
-  [149, 387, 393],
-  [152, 347, 349],
-  [454, 464, 466],
+  [52, 621, 627],
+  [55, 582, 584],
+  [146, 384, 390],
+  [149, 344, 346],
+  [451, 461, 463],
 ]
 
 ---

test/cctest/interpreter/bytecode_expectations/ForAwaitOf.golden

@@ -16,7 +16,7 @@ snippet: "
 "
 frame size: 24
 parameter count: 1
-bytecode array length: 623
+bytecode array length: 620
 bytecodes: [
                 B(Mov), R(new_target), R(11),
                 B(Ldar), R(new_target),
@@ -43,8 +43,6 @@ bytecodes: [
                 B(Mov), R(2), R(11),
                 B(Mov), R(context), R(15),
                 B(Mov), R(context), R(16),
-                B(LdaTheHole),
-                B(Star), R(1),
                 B(LdaZero),
                 B(Star), R(6),
                 B(Mov), R(context), R(19),
@@ -283,9 +281,9 @@ bytecodes: [
   /*   57 S> */ B(Return),
 ]
 constant pool: [
-  Smi [105],
-  Smi [360],
-  Smi [440],
+  Smi [102],
+  Smi [357],
+  Smi [437],
   TUPLE2_TYPE,
   SYMBOL_TYPE,
   SYMBOL_TYPE,
@@ -303,11 +301,11 @@ constant pool: [
   Smi [9],
 ]
 handlers: [
-  [62, 578, 584],
-  [65, 533, 535],
-  [74, 283, 289],
-  [77, 243, 245],
-  [349, 407, 409],
+  [62, 575, 581],
+  [65, 530, 532],
+  [71, 280, 286],
+  [74, 240, 242],
+  [346, 404, 406],
 ]
 
 ---
@@ -319,7 +317,7 @@ snippet: "
 "
 frame size: 24
 parameter count: 1
-bytecode array length: 655
+bytecode array length: 652
 bytecodes: [
                 B(Mov), R(new_target), R(11),
                 B(Ldar), R(new_target),
@@ -346,8 +344,6 @@ bytecodes: [
                 B(Mov), R(2), R(11),
                 B(Mov), R(context), R(15),
                 B(Mov), R(context), R(16),
-                B(LdaTheHole),
-                B(Star), R(1),
                 B(LdaZero),
                 B(Star), R(6),
                 B(Mov), R(context), R(19),
@@ -598,9 +594,9 @@ bytecodes: [
   /*   68 S> */ B(Return),
 ]
 constant pool: [
-  Smi [105],
-  Smi [363],
-  Smi [443],
+  Smi [102],
+  Smi [360],
+  Smi [440],
   TUPLE2_TYPE,
   SYMBOL_TYPE,
   SYMBOL_TYPE,
@@ -621,11 +617,11 @@ constant pool: [
   Smi [25],
 ]
 handlers: [
-  [62, 594, 600],
-  [65, 548, 550],
-  [74, 285, 291],
-  [77, 245, 247],
-  [352, 410, 412],
+  [62, 591, 597],
+  [65, 545, 547],
+  [71, 282, 288],
+  [74, 242, 244],
+  [349, 407, 409],
 ]
 
 ---
@@ -640,7 +636,7 @@ snippet: "
 "
 frame size: 24
 parameter count: 1
-bytecode array length: 641
+bytecode array length: 638
 bytecodes: [
                 B(Mov), R(new_target), R(11),
                 B(Ldar), R(new_target),
@@ -667,8 +663,6 @@ bytecodes: [
                 B(Mov), R(2), R(11),
                 B(Mov), R(context), R(15),
                 B(Mov), R(context), R(16),
-                B(LdaTheHole),
-                B(Star), R(1),
                 B(LdaZero),
                 B(Star), R(6),
                 B(Mov), R(context), R(19),
@@ -915,9 +909,9 @@ bytecodes: [
   /*  114 S> */ B(Return),
 ]
 constant pool: [
-  Smi [105],
-  Smi [378],
-  Smi [458],
+  Smi [102],
+  Smi [375],
+  Smi [455],
   TUPLE2_TYPE,
   SYMBOL_TYPE,
   SYMBOL_TYPE,
@@ -935,11 +929,11 @@ constant pool: [
   Smi [9],
 ]
 handlers: [
-  [62, 596, 602],
-  [65, 551, 553],
-  [74, 301, 307],
-  [77, 261, 263],
-  [367, 425, 427],
+  [62, 593, 599],
+  [65, 548, 550],
+  [71, 298, 304],
+  [74, 258, 260],
+  [364, 422, 424],
 ]
 
 ---

test/cctest/interpreter/bytecode_expectations/ForOfLoop.golden

@@ -15,11 +15,9 @@ snippet: "
 "
 frame size: 16
 parameter count: 2
-bytecode array length: 263
+bytecode array length: 260
 bytecodes: [
   /*   10 E> */ B(StackCheck),
-                B(LdaTheHole),
-                B(Star), R(2),
                 B(LdaZero),
                 B(Star), R(6),
                 B(Mov), R(context), R(12),
@@ -142,9 +140,9 @@ constant pool: [
   FIXED_ARRAY_TYPE,
 ]
 handlers: [
-  [10, 127, 133],
-  [13, 91, 93],
-  [193, 203, 205],
+  [7, 124, 130],
+  [10, 88, 90],
+  [190, 200, 202],
 ]
 
 ---
@@ -338,11 +336,9 @@ snippet: "
 "
 frame size: 14
 parameter count: 2
-bytecode array length: 281
+bytecode array length: 278
 bytecodes: [
   /*   10 E> */ B(StackCheck),
-                B(LdaTheHole),
-                B(Star), R(0),
                 B(LdaZero),
                 B(Star), R(4),
                 B(Mov), R(context), R(10),
@@ -476,9 +472,9 @@ constant pool: [
   FIXED_ARRAY_TYPE,
 ]
 handlers: [
-  [10, 145, 151],
-  [13, 109, 111],
-  [211, 221, 223],
+  [7, 142, 148],
+  [10, 106, 108],
+  [208, 218, 220],
 ]
 
 ---
@@ -490,13 +486,9 @@ snippet: "
 "
 frame size: 19
 parameter count: 2
-bytecode array length: 304
+bytecode array length: 298
 bytecodes: [
   /*   10 E> */ B(StackCheck),
-                B(LdaTheHole),
-                B(Star), R(3),
-                B(LdaTheHole),
-                B(Star), R(4),
                 B(LdaZero),
                 B(Star), R(9),
                 B(Mov), R(context), R(15),
@@ -637,9 +629,9 @@ constant pool: [
   FIXED_ARRAY_TYPE,
 ]
 handlers: [
-  [13, 168, 174],
-  [16, 132, 134],
-  [234, 244, 246],
+  [7, 162, 168],
+  [10, 126, 128],
+  [228, 238, 240],
 ]
 
 ---
@@ -651,7 +643,7 @@ snippet: "
 "
 frame size: 20
 parameter count: 2
-bytecode array length: 354
+bytecode array length: 351
 bytecodes: [
                 B(Mov), R(new_target), R(11),
                 B(Ldar), R(new_target),
@@ -689,8 +681,6 @@ bytecodes: [
   /*   11 E> */ B(Throw),
                 B(Ldar), R(14),
   /*   55 S> */ B(Return),
-                B(LdaTheHole),
-                B(Star), R(2),
                 B(LdaZero),
                 B(Star), R(7),
                 B(Mov), R(context), R(16),
@@ -817,9 +807,9 @@ constant pool: [
   FIXED_ARRAY_TYPE,
 ]
 handlers: [
-  [100, 218, 224],
-  [103, 182, 184],
-  [284, 294, 296],
+  [97, 215, 221],
+  [100, 179, 181],
+  [281, 291, 293],
 ]
 
 ---
@@ -831,7 +821,7 @@ snippet: "
 "
 frame size: 19
 parameter count: 2
-bytecode array length: 428
+bytecode array length: 425
 bytecodes: [
                 B(Mov), R(new_target), R(10),
                 B(Ldar), R(new_target),
@@ -869,8 +859,6 @@ bytecodes: [
   /*   11 E> */ B(Throw),
                 B(Ldar), R(13),
   /*   49 S> */ B(Return),
-                B(LdaTheHole),
-                B(Star), R(1),
                 B(LdaZero),
                 B(Star), R(6),
                 B(Mov), R(context), R(15),
@@ -1012,7 +1000,7 @@ bytecodes: [
 ]
 constant pool: [
   Smi [46],
-  Smi [109],
+  Smi [106],
   Smi [10],
   Smi [7],
   SYMBOL_TYPE,
@@ -1031,9 +1019,9 @@ constant pool: [
   Smi [9],
 ]
 handlers: [
-  [100, 285, 291],
-  [103, 249, 251],
-  [352, 362, 364],
+  [97, 282, 288],
+  [100, 246, 248],
+  [349, 359, 361],
 ]
 
 ---
@@ -1045,7 +1033,7 @@ snippet: "
 "
 frame size: 23
 parameter count: 2
-bytecode array length: 400
+bytecode array length: 397
 bytecodes: [
                 B(CreateFunctionContext), U8(1),
                 B(PushContext), R(12),
@@ -1058,8 +1046,6 @@ bytecodes: [
                 B(Star), R(11),
                 B(Mov), R(context), R(15),
                 B(Mov), R(context), R(16),
-                B(LdaTheHole),
-                B(Star), R(2),
                 B(LdaZero),
                 B(Star), R(7),
                 B(Mov), R(context), R(19),
@@ -1243,11 +1229,11 @@ constant pool: [
   Smi [9],
 ]
 handlers: [
-  [21, 355, 361],
-  [24, 310, 312],
-  [33, 155, 161],
-  [36, 115, 117],
-  [221, 231, 233],
+  [21, 352, 358],
+  [24, 307, 309],
+  [30, 152, 158],
+  [33, 112, 114],
+  [218, 228, 230],
 ]
 
 ---
@@ -1259,7 +1245,7 @@ snippet: "
 "
 frame size: 25
 parameter count: 2
-bytecode array length: 514
+bytecode array length: 511
 bytecodes: [
                 B(Mov), R(new_target), R(11),
                 B(Ldar), R(new_target),
@@ -1290,8 +1276,6 @@ bytecodes: [
                 B(Mov), R(2), R(11),
                 B(Mov), R(context), R(16),
                 B(Mov), R(context), R(17),
-                B(LdaTheHole),
-                B(Star), R(1),
                 B(LdaZero),
                 B(Star), R(6),
                 B(Mov), R(context), R(20),
@@ -1486,7 +1470,7 @@ bytecodes: [
   /*   54 S> */ B(Return),
 ]
 constant pool: [
-  Smi [91],
+  Smi [88],
   SYMBOL_TYPE,
   Smi [85],
   ONE_BYTE_INTERNALIZED_STRING_TYPE ["next"],
@@ -1502,10 +1486,10 @@ constant pool: [
   Smi [9],
 ]
 handlers: [
-  [70, 469, 475],
-  [73, 424, 426],
-  [82, 269, 275],
-  [85, 229, 231],
-  [335, 345, 347],
+  [70, 466, 472],
+  [73, 421, 423],
+  [79, 266, 272],
+  [82, 226, 228],
+  [332, 342, 344],
 ]
 

test/cctest/interpreter/bytecode_expectations/Generators.golden

@@ -138,7 +138,7 @@ snippet: "
 "
 frame size: 18
 parameter count: 1
-bytecode array length: 422
+bytecode array length: 419
 bytecodes: [
                 B(Mov), R(new_target), R(10),
                 B(Ldar), R(new_target),
@@ -172,8 +172,6 @@ bytecodes: [
   /*   11 E> */ B(Throw),
                 B(Ldar), R(12),
   /*   44 S> */ B(Return),
-                B(LdaTheHole),
-                B(Star), R(1),
                 B(LdaZero),
                 B(Star), R(6),
                 B(Mov), R(context), R(14),
@@ -315,7 +313,7 @@ bytecodes: [
 ]
 constant pool: [
   Smi [38],
-  Smi [103],
+  Smi [100],
   Smi [10],
   Smi [7],
   TUPLE2_TYPE,
@@ -335,9 +333,9 @@ constant pool: [
   Smi [9],
 ]
 handlers: [
-  [92, 279, 285],
-  [95, 243, 245],
-  [346, 356, 358],
+  [89, 276, 282],
+  [92, 240, 242],
+  [343, 353, 355],
 ]
 
 ---