Rewrite scopes of initializers in for-in/of destructured declarations

Bug: chromium:740591
Change-Id: I869be41d8630b23704b9470c4d3db8a21bbde873
Reviewed-on: https://chromium-review.googlesource.com/583531Reviewed-by: Georg Neis <neis@chromium.org>
Commit-Queue: Adam Klein <adamk@chromium.org>
Cr-Commit-Position: refs/heads/master@{#46881}

src/parsing/parameter-initializer-rewriter.cc

@@ -17,9 +17,8 @@ namespace {
 
 class Rewriter final : public AstTraversalVisitor<Rewriter> {
  public:
-  Rewriter(uintptr_t stack_limit, Expression* initializer, Scope* param_scope)
-      : AstTraversalVisitor(stack_limit, initializer),
-        param_scope_(param_scope) {}
+  Rewriter(uintptr_t stack_limit, Expression* initializer, Scope* scope)
+      : AstTraversalVisitor(stack_limit, initializer), scope_(scope) {}
 
  private:
   // This is required so that the overriden Visit* methods can be
@@ -34,11 +33,11 @@ class Rewriter final : public AstTraversalVisitor<Rewriter> {
   void VisitTryCatchStatement(TryCatchStatement* stmt);
   void VisitWithStatement(WithStatement* stmt);
 
-  Scope* param_scope_;
+  Scope* scope_;
 };
 
 void Rewriter::VisitFunctionLiteral(FunctionLiteral* function_literal) {
-  function_literal->scope()->ReplaceOuterScope(param_scope_);
+  function_literal->scope()->ReplaceOuterScope(scope_);
 }
 
 
@@ -63,20 +62,20 @@ void Rewriter::VisitClassLiteral(ClassLiteral* class_literal) {
 
 void Rewriter::VisitVariableProxy(VariableProxy* proxy) {
   if (!proxy->is_resolved()) {
-    if (param_scope_->outer_scope()->RemoveUnresolved(proxy)) {
-      param_scope_->AddUnresolved(proxy);
+    if (scope_->outer_scope()->RemoveUnresolved(proxy)) {
+      scope_->AddUnresolved(proxy);
     }
   } else {
     // Ensure that temporaries we find are already in the correct scope.
     DCHECK(proxy->var()->mode() != TEMPORARY ||
-           proxy->var()->scope() == param_scope_->GetClosureScope());
+           proxy->var()->scope() == scope_->GetClosureScope());
   }
 }
 
 
 void Rewriter::VisitBlock(Block* stmt) {
   if (stmt->scope() != nullptr)
-    stmt->scope()->ReplaceOuterScope(param_scope_);
+    stmt->scope()->ReplaceOuterScope(scope_);
   else
     VisitStatements(stmt->statements());
 }
@@ -84,28 +83,33 @@ void Rewriter::VisitBlock(Block* stmt) {
 
 void Rewriter::VisitTryCatchStatement(TryCatchStatement* stmt) {
   Visit(stmt->try_block());
-  stmt->scope()->ReplaceOuterScope(param_scope_);
+  stmt->scope()->ReplaceOuterScope(scope_);
 }
 
 
 void Rewriter::VisitWithStatement(WithStatement* stmt) {
   Visit(stmt->expression());
-  stmt->scope()->ReplaceOuterScope(param_scope_);
+  stmt->scope()->ReplaceOuterScope(scope_);
 }
 
 
 }  // anonymous namespace
 
-void ReparentParameterExpressionScope(uintptr_t stack_limit, Expression* expr,
-                                      Scope* param_scope) {
-  // The only case that uses this code is block scopes for parameters containing
-  // sloppy eval.
-  DCHECK(param_scope->is_block_scope());
-  DCHECK(param_scope->is_declaration_scope());
-  DCHECK(param_scope->calls_sloppy_eval());
-  DCHECK(param_scope->outer_scope()->is_function_scope());
-
-  Rewriter rewriter(stack_limit, expr, param_scope);
+void ReparentExpressionScope(uintptr_t stack_limit, Expression* expr,
+                             Scope* scope) {
+  // Both uses of this function should pass in a block scope.
+  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());
+
+  Rewriter rewriter(stack_limit, expr, scope);
   rewriter.Run();
 }
 

src/parsing/parameter-initializer-rewriter.h

@@ -16,12 +16,13 @@ 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.
+// parsing. We do the same rewriting for initializers of destructured
+// lexical declarations in for-in/of loops.
 //
-// param_scope is the new inner scope, and its outer_scope() is assumed
-// to be the function scope which was used during the initial parse.
-void ReparentParameterExpressionScope(uintptr_t stack_limit, Expression* expr,
-                                      Scope* param_scope);
+// scope is the new inner scope, and its outer_scope() is assumed
+// to be the scope which was used during the initial parse.
+void ReparentExpressionScope(uintptr_t stack_limit, Expression* expr,
+                             Scope* scope);
 
 }  // namespace internal
 }  // namespace v8

src/parsing/parser-base.h

@@ -487,7 +487,7 @@ class ParserBase {
   };
 
   struct DeclarationDescriptor {
-    enum Kind { NORMAL, PARAMETER };
+    enum Kind { NORMAL, PARAMETER, LEXICAL_FOR_EACH };
     Scope* scope;
     VariableMode mode;
     int declaration_pos;

src/parsing/parser.cc

@@ -1991,10 +1991,9 @@ Block* Parser::RewriteForVarInLegacy(const ForInfo& for_info) {
 // into
 //
 //   {
-//     <let x' be a temporary variable>
-//     for (x' in/of e) {
-//       let/const/var x;
-//       x = x';
+//     var temp;
+//     for (temp in/of e) {
+//       let/const/var x = temp;
 //       b;
 //     }
 //     let x;  // for TDZ
@@ -2013,6 +2012,8 @@ void Parser::DesugarBindingInForEachStatement(ForInfo* for_info,
     auto descriptor = for_info->parsing_result.descriptor;
     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 =
@@ -3043,8 +3044,7 @@ Block* Parser::BuildParameterInitializationBlock(
       // rewrite inner initializers of the pattern to param_scope
       descriptor.scope = param_scope;
       // Rewrite the outer initializer to point to param_scope
-      ReparentParameterExpressionScope(stack_limit(), initial_value,
-                                       param_scope);
+      ReparentExpressionScope(stack_limit(), initial_value, param_scope);
     }
 
     BlockState block_state(&scope_, param_scope);

src/parsing/pattern-rewriter.cc

@@ -289,11 +289,13 @@ 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 {
-  // 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 &&
+  DCHECK(IsBindingContext());
+  if (descriptor_->declaration_kind == DeclarationDescriptor::PARAMETER &&
       // And only when scope is a block scope;
       // without eval, it is a function scope.
       scope()->is_block_scope()) {
@@ -306,13 +308,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 (DeclaresParameterContainingSloppyEval()) {
-    ReparentParameterExpressionScope(parser_->stack_limit(), expr, scope());
+  if (!IsBindingContext()) return;
+  if (DeclaresParameterContainingSloppyEval() ||
+      descriptor_->declaration_kind ==
+          DeclarationDescriptor::LEXICAL_FOR_EACH) {
+    ReparentExpressionScope(parser_->stack_limit(), expr, scope());
   }
 }
 

test/cctest/interpreter/bytecode_expectations/AsyncGenerators.golden

@@ -283,7 +283,7 @@ snippet: "
 "
 frame size: 23
 parameter count: 1
-bytecode array length: 570
+bytecode array length: 573
 bytecodes: [
                 B(Mov), R(new_target), R(10),
                 B(Ldar), R(new_target),
@@ -324,6 +324,8 @@ bytecodes: [
                 B(Star), R(12),
                 B(Mov), R(16), R(13),
                 B(JumpConstant), U8(20),
+                B(LdaTheHole),
+                B(Star), R(1),
                 B(LdaZero),
                 B(Star), R(6),
                 B(Mov), R(context), R(18),
@@ -529,7 +531,7 @@ bytecodes: [
 ]
 constant pool: [
   Smi [49],
-  Smi [116],
+  Smi [119],
   Smi [15],
   Smi [7],
   TUPLE2_TYPE,
@@ -548,17 +550,17 @@ constant pool: [
   Smi [6],
   Smi [14],
   FIXED_ARRAY_TYPE,
-  Smi [412],
+  Smi [415],
   Smi [6],
   Smi [28],
   Smi [35],
 ]
 handlers: [
-  [52, 497, 503],
-  [55, 458, 460],
-  [105, 304, 310],
-  [108, 264, 266],
-  [371, 381, 383],
+  [52, 500, 506],
+  [55, 461, 463],
+  [108, 307, 313],
+  [111, 267, 269],
+  [374, 384, 386],
 ]
 
 ---

test/cctest/interpreter/bytecode_expectations/ForAwaitOf.golden

@@ -16,7 +16,7 @@ snippet: "
 "
 frame size: 24
 parameter count: 1
-bytecode array length: 634
+bytecode array length: 637
 bytecodes: [
                 B(Mov), R(new_target), R(11),
                 B(Ldar), R(new_target),
@@ -43,6 +43,8 @@ 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),
@@ -290,9 +292,9 @@ bytecodes: [
   /*   57 S> */ B(Return),
 ]
 constant pool: [
-  Smi [102],
-  Smi [366],
-  Smi [451],
+  Smi [105],
+  Smi [369],
+  Smi [454],
   TUPLE2_TYPE,
   SYMBOL_TYPE,
   SYMBOL_TYPE,
@@ -310,11 +312,11 @@ constant pool: [
   Smi [9],
 ]
 handlers: [
-  [62, 589, 595],
-  [65, 544, 546],
-  [71, 284, 290],
-  [74, 244, 246],
-  [350, 413, 415],
+  [62, 592, 598],
+  [65, 547, 549],
+  [74, 287, 293],
+  [77, 247, 249],
+  [353, 416, 418],
 ]
 
 ---
@@ -326,7 +328,7 @@ snippet: "
 "
 frame size: 24
 parameter count: 1
-bytecode array length: 666
+bytecode array length: 669
 bytecodes: [
                 B(Mov), R(new_target), R(11),
                 B(Ldar), R(new_target),
@@ -353,6 +355,8 @@ 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),
@@ -612,9 +616,9 @@ bytecodes: [
   /*   68 S> */ B(Return),
 ]
 constant pool: [
-  Smi [102],
-  Smi [369],
-  Smi [454],
+  Smi [105],
+  Smi [372],
+  Smi [457],
   TUPLE2_TYPE,
   SYMBOL_TYPE,
   SYMBOL_TYPE,
@@ -635,11 +639,11 @@ constant pool: [
   Smi [25],
 ]
 handlers: [
-  [62, 605, 611],
-  [65, 559, 561],
-  [71, 286, 292],
-  [74, 246, 248],
-  [353, 416, 418],
+  [62, 608, 614],
+  [65, 562, 564],
+  [74, 289, 295],
+  [77, 249, 251],
+  [356, 419, 421],
 ]
 
 ---
@@ -654,7 +658,7 @@ snippet: "
 "
 frame size: 24
 parameter count: 1
-bytecode array length: 652
+bytecode array length: 655
 bytecodes: [
                 B(Mov), R(new_target), R(11),
                 B(Ldar), R(new_target),
@@ -681,6 +685,8 @@ 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),
@@ -936,9 +942,9 @@ bytecodes: [
   /*  114 S> */ B(Return),
 ]
 constant pool: [
-  Smi [102],
-  Smi [384],
-  Smi [469],
+  Smi [105],
+  Smi [387],
+  Smi [472],
   TUPLE2_TYPE,
   SYMBOL_TYPE,
   SYMBOL_TYPE,
@@ -956,11 +962,11 @@ constant pool: [
   Smi [9],
 ]
 handlers: [
-  [62, 607, 613],
-  [65, 562, 564],
-  [71, 302, 308],
-  [74, 262, 264],
-  [368, 431, 433],
+  [62, 610, 616],
+  [65, 565, 567],
+  [74, 305, 311],
+  [77, 265, 267],
+  [371, 434, 436],
 ]
 
 ---

test/cctest/interpreter/bytecode_expectations/ForOfLoop.golden

@@ -15,9 +15,11 @@ snippet: "
 "
 frame size: 16
 parameter count: 2
-bytecode array length: 260
+bytecode array length: 263
 bytecodes: [
   /*   10 E> */ B(StackCheck),
+                B(LdaTheHole),
+                B(Star), R(2),
                 B(LdaZero),
                 B(Star), R(6),
                 B(Mov), R(context), R(12),
@@ -140,9 +142,9 @@ constant pool: [
   FIXED_ARRAY_TYPE,
 ]
 handlers: [
-  [7, 124, 130],
-  [10, 88, 90],
-  [190, 200, 202],
+  [10, 127, 133],
+  [13, 91, 93],
+  [193, 203, 205],
 ]
 
 ---
@@ -336,9 +338,11 @@ snippet: "
 "
 frame size: 14
 parameter count: 2
-bytecode array length: 278
+bytecode array length: 281
 bytecodes: [
   /*   10 E> */ B(StackCheck),
+                B(LdaTheHole),
+                B(Star), R(0),
                 B(LdaZero),
                 B(Star), R(4),
                 B(Mov), R(context), R(10),
@@ -472,9 +476,9 @@ constant pool: [
   FIXED_ARRAY_TYPE,
 ]
 handlers: [
-  [7, 142, 148],
-  [10, 106, 108],
-  [208, 218, 220],
+  [10, 145, 151],
+  [13, 109, 111],
+  [211, 221, 223],
 ]
 
 ---
@@ -486,9 +490,13 @@ snippet: "
 "
 frame size: 19
 parameter count: 2
-bytecode array length: 298
+bytecode array length: 304
 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),
@@ -629,9 +637,9 @@ constant pool: [
   FIXED_ARRAY_TYPE,
 ]
 handlers: [
-  [7, 162, 168],
-  [10, 126, 128],
-  [228, 238, 240],
+  [13, 168, 174],
+  [16, 132, 134],
+  [234, 244, 246],
 ]
 
 ---
@@ -643,7 +651,7 @@ snippet: "
 "
 frame size: 20
 parameter count: 2
-bytecode array length: 357
+bytecode array length: 360
 bytecodes: [
                 B(Mov), R(new_target), R(11),
                 B(Ldar), R(new_target),
@@ -684,6 +692,8 @@ 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),
@@ -810,9 +820,9 @@ constant pool: [
   FIXED_ARRAY_TYPE,
 ]
 handlers: [
-  [103, 221, 227],
-  [106, 185, 187],
-  [287, 297, 299],
+  [106, 224, 230],
+  [109, 188, 190],
+  [290, 300, 302],
 ]
 
 ---
@@ -824,7 +834,7 @@ snippet: "
 "
 frame size: 19
 parameter count: 2
-bytecode array length: 436
+bytecode array length: 439
 bytecodes: [
                 B(Mov), R(new_target), R(10),
                 B(Ldar), R(new_target),
@@ -865,6 +875,8 @@ 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),
@@ -1009,7 +1021,7 @@ bytecodes: [
 ]
 constant pool: [
   Smi [52],
-  Smi [112],
+  Smi [115],
   Smi [10],
   Smi [7],
   SYMBOL_TYPE,
@@ -1028,9 +1040,9 @@ constant pool: [
   Smi [9],
 ]
 handlers: [
-  [103, 293, 299],
-  [106, 257, 259],
-  [360, 370, 372],
+  [106, 296, 302],
+  [109, 260, 262],
+  [363, 373, 375],
 ]
 
 ---
@@ -1042,7 +1054,7 @@ snippet: "
 "
 frame size: 23
 parameter count: 2
-bytecode array length: 397
+bytecode array length: 400
 bytecodes: [
                 B(CreateFunctionContext), U8(1),
                 B(PushContext), R(12),
@@ -1055,6 +1067,8 @@ 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),
@@ -1238,11 +1252,11 @@ constant pool: [
   Smi [9],
 ]
 handlers: [
-  [21, 352, 358],
-  [24, 307, 309],
-  [30, 152, 158],
-  [33, 112, 114],
-  [218, 228, 230],
+  [21, 355, 361],
+  [24, 310, 312],
+  [33, 155, 161],
+  [36, 115, 117],
+  [221, 231, 233],
 ]
 
 ---
@@ -1254,7 +1268,7 @@ snippet: "
 "
 frame size: 25
 parameter count: 2
-bytecode array length: 515
+bytecode array length: 518
 bytecodes: [
                 B(Mov), R(new_target), R(11),
                 B(Ldar), R(new_target),
@@ -1285,6 +1299,8 @@ 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),
@@ -1482,7 +1498,7 @@ bytecodes: [
   /*   54 S> */ B(Return),
 ]
 constant pool: [
-  Smi [88],
+  Smi [91],
   SYMBOL_TYPE,
   Smi [89],
   ONE_BYTE_INTERNALIZED_STRING_TYPE ["next"],
@@ -1498,10 +1514,10 @@ constant pool: [
   Smi [9],
 ]
 handlers: [
-  [70, 470, 476],
-  [73, 425, 427],
-  [79, 270, 276],
-  [82, 230, 232],
-  [336, 346, 348],
+  [70, 473, 479],
+  [73, 428, 430],
+  [82, 273, 279],
+  [85, 233, 235],
+  [339, 349, 351],
 ]
 

test/cctest/interpreter/bytecode_expectations/Generators.golden

@@ -147,7 +147,7 @@ snippet: "
 "
 frame size: 18
 parameter count: 1
-bytecode array length: 430
+bytecode array length: 433
 bytecodes: [
                 B(Mov), R(new_target), R(10),
                 B(Ldar), R(new_target),
@@ -184,6 +184,8 @@ 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),
@@ -328,7 +330,7 @@ bytecodes: [
 ]
 constant pool: [
   Smi [44],
-  Smi [106],
+  Smi [109],
   Smi [10],
   Smi [7],
   TUPLE2_TYPE,
@@ -348,9 +350,9 @@ constant pool: [
   Smi [9],
 ]
 handlers: [
-  [95, 287, 293],
-  [98, 251, 253],
-  [354, 364, 366],
+  [98, 290, 296],
+  [101, 254, 256],
+  [357, 367, 369],
 ]
 
 ---

test/mjsunit/regress/regress-crbug-740591.js

+// Copyright 2017 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.
+
+(function regressionCaseOne() {
+  var c;
+  for (let [a, b = c = function() { return a + b }] of [[0]]) {
+    function f() { return a };
+  }
+  c();
+})();
+
+(function testForInFunction() {
+  for (const {length: a, b = function() { return a, b }} in {foo: 42}) {
+    assertSame(b, (function() { return b() })());
+  }
+})();
+
+(function testForOfFunction() {
+  for (const [a, b = function() { return a, b }] of [[42]]) {
+    assertSame(b, (function() { return b() })());
+  }
+})();
+
+(function testForInVariableProxy() {
+  for (const {length: a, b = a} in {foo: 42}) {
+    assertEquals(3, a);
+    assertEquals(a, b);
+  }
+})();
+
+(function testForOfVariableProxy() {
+  for (const [a, b = a] of [[42]]) {
+    assertEquals(42, a);
+    assertEquals(a, b);
+  }
+})();