[es6] support AssignmentPattern as LHS in for-in/of loops

BUG=v8:811, v8:4599
LOG=N
R=adamk@chromium.org, rossberg@chromium.org

Review URL: https://codereview.chromium.org/1517973002

Cr-Commit-Position: refs/heads/master@{#32814}

src/ast/ast.cc

@@ -137,6 +137,10 @@ static void AssignVectorSlots(Expression* expr, FeedbackVectorSpec* spec,
 void ForEachStatement::AssignFeedbackVectorSlots(
     Isolate* isolate, FeedbackVectorSpec* spec,
     FeedbackVectorSlotCache* cache) {
+  // TODO(adamk): for-of statements do not make use of this feedback slot.
+  // The each_slot_ should be specific to ForInStatement, and this work moved
+  // there.
+  if (IsForOfStatement()) return;
   AssignVectorSlots(each(), spec, &each_slot_);
 }
 

src/parsing/parser.cc

@@ -3327,9 +3327,10 @@ Expression* Parser::BuildIteratorNextResult(Expression* iterator,
 
 
 void Parser::InitializeForEachStatement(ForEachStatement* stmt,
-                                        Expression* each,
-                                        Expression* subject,
-                                        Statement* body) {
+                                        Expression* each, Expression* subject,
+                                        Statement* body,
+                                        bool is_destructuring) {
+  DCHECK(!is_destructuring || allow_harmony_destructuring_assignment());
   ForOfStatement* for_of = stmt->AsForOfStatement();
 
   if (for_of != NULL) {
@@ -3380,6 +3381,10 @@ void Parser::InitializeForEachStatement(ForEachStatement* stmt,
           result_proxy, value_literal, RelocInfo::kNoPosition);
       assign_each = factory()->NewAssignment(Token::ASSIGN, each, result_value,
                                              RelocInfo::kNoPosition);
+      if (is_destructuring) {
+        assign_each = PatternRewriter::RewriteDestructuringAssignment(
+            this, assign_each->AsAssignment(), scope_);
+      }
     }
 
     for_of->Initialize(each, subject, body,
@@ -3388,6 +3393,23 @@ void Parser::InitializeForEachStatement(ForEachStatement* stmt,
                        result_done,
                        assign_each);
   } else {
+    if (is_destructuring) {
+      Variable* temp =
+          scope_->NewTemporary(ast_value_factory()->empty_string());
+      VariableProxy* temp_proxy = factory()->NewVariableProxy(temp);
+      Expression* assign_each = PatternRewriter::RewriteDestructuringAssignment(
+          this, factory()->NewAssignment(Token::ASSIGN, each, temp_proxy,
+                                         RelocInfo::kNoPosition),
+          scope_);
+      auto block =
+          factory()->NewBlock(nullptr, 2, false, RelocInfo::kNoPosition);
+      block->statements()->Add(factory()->NewExpressionStatement(
+                                   assign_each, RelocInfo::kNoPosition),
+                               zone());
+      block->statements()->Add(body, zone());
+      body = block;
+      each = factory()->NewVariableProxy(temp);
+    }
     stmt->Initialize(each, subject, body);
   }
 }
@@ -3772,7 +3794,8 @@ Statement* Parser::ParseForStatement(ZoneList<const AstRawString*>* labels,
         body_block->statements()->Add(body, zone());
         VariableProxy* temp_proxy =
             factory()->NewVariableProxy(temp, each_beg_pos, each_end_pos);
-        InitializeForEachStatement(loop, temp_proxy, enumerable, body_block);
+        InitializeForEachStatement(loop, temp_proxy, enumerable, body_block,
+                                   false);
         scope_ = for_scope;
         body_scope->set_end_position(scanner()->location().end_pos);
         body_scope = body_scope->FinalizeBlockScope();
@@ -3823,7 +3846,8 @@ Statement* Parser::ParseForStatement(ZoneList<const AstRawString*>* labels,
       }
     } else {
       int lhs_beg_pos = peek_position();
-      Expression* expression = ParseExpression(false, CHECK_OK);
+      ExpressionClassifier classifier;
+      Expression* expression = ParseExpression(false, &classifier, CHECK_OK);
       int lhs_end_pos = scanner()->location().end_pos;
       ForEachStatement::VisitMode mode;
       is_let_identifier_expression =
@@ -3831,11 +3855,24 @@ Statement* Parser::ParseForStatement(ZoneList<const AstRawString*>* labels,
           expression->AsVariableProxy()->raw_name() ==
               ast_value_factory()->let_string();
 
-      if (CheckInOrOf(&mode, ok)) {
+      bool is_for_each = CheckInOrOf(&mode, ok);
       if (!*ok) return nullptr;
+      bool is_destructuring =
+          is_for_each && allow_harmony_destructuring_assignment() &&
+          (expression->IsArrayLiteral() || expression->IsObjectLiteral());
+
+      if (is_destructuring) {
+        ValidateAssignmentPattern(&classifier, CHECK_OK);
+      } else {
+        ValidateExpression(&classifier, CHECK_OK);
+      }
+
+      if (is_for_each) {
+        if (!is_destructuring) {
           expression = this->CheckAndRewriteReferenceExpression(
               expression, lhs_beg_pos, lhs_end_pos,
               MessageTemplate::kInvalidLhsInFor, kSyntaxError, CHECK_OK);
+        }
 
         ForEachStatement* loop =
             factory()->NewForEachStatement(mode, labels, stmt_pos);
@@ -3856,7 +3893,8 @@ Statement* Parser::ParseForStatement(ZoneList<const AstRawString*>* labels,
             factory()->NewBlock(NULL, 1, false, RelocInfo::kNoPosition);
         Statement* body = ParseSubStatement(NULL, CHECK_OK);
         block->statements()->Add(body, zone());
-        InitializeForEachStatement(loop, expression, enumerable, block);
+        InitializeForEachStatement(loop, expression, enumerable, block,
+                                   is_destructuring);
         scope_ = saved_scope;
         body_scope->set_end_position(scanner()->location().end_pos);
         body_scope = body_scope->FinalizeBlockScope();
@@ -4556,10 +4594,8 @@ class InitializerRewriter : public AstExpressionVisitor {
         expr->AsRewritableAssignmentExpression();
     if (to_rewrite == nullptr || to_rewrite->is_rewritten()) return;
 
-    bool ok = true;
     Parser::PatternRewriter::RewriteDestructuringAssignment(parser_, to_rewrite,
-                                                            scope_, &ok);
-    DCHECK(ok);
+                                                            scope_);
   }
 
  private:
@@ -6529,10 +6565,7 @@ void Parser::RewriteDestructuringAssignments() {
     Scope* scope = pair.scope;
     DCHECK_NOT_NULL(to_rewrite);
     if (!to_rewrite->is_rewritten()) {
-      bool ok = true;
-      PatternRewriter::RewriteDestructuringAssignment(this, to_rewrite, scope,
-                                                      &ok);
-      DCHECK(ok);
+      PatternRewriter::RewriteDestructuringAssignment(this, to_rewrite, scope);
     }
   }
 }

src/parsing/parser.h

@@ -1047,8 +1047,11 @@ class Parser : public ParserBase<ParserTraits> {
         ZoneList<const AstRawString*>* names, bool* ok);
 
     static void RewriteDestructuringAssignment(
-        Parser* parser, RewritableAssignmentExpression* expr, Scope* Scope,
-        bool* ok);
+        Parser* parser, RewritableAssignmentExpression* expr, Scope* Scope);
+
+    static Expression* RewriteDestructuringAssignment(Parser* parser,
+                                                      Assignment* assignment,
+                                                      Scope* scope);
 
     void set_initializer_position(int pos) { initializer_position_ = pos; }
 
@@ -1144,10 +1147,9 @@ class Parser : public ParserBase<ParserTraits> {
 
 
   // Initialize the components of a for-in / for-of statement.
-  void InitializeForEachStatement(ForEachStatement* stmt,
-                                  Expression* each,
-                                  Expression* subject,
-                                  Statement* body);
+  void InitializeForEachStatement(ForEachStatement* stmt, Expression* each,
+                                  Expression* subject, Statement* body,
+                                  bool is_destructuring);
   Statement* DesugarLexicalBindingsInForStatement(
       Scope* inner_scope, bool is_const, ZoneList<const AstRawString*>* names,
       ForStatement* loop, Statement* init, Expression* cond, Statement* next,

src/parsing/pattern-rewriter.cc

@@ -32,12 +32,12 @@ void Parser::PatternRewriter::DeclareAndInitializeVariables(
 
 
 void Parser::PatternRewriter::RewriteDestructuringAssignment(
-    Parser* parser, RewritableAssignmentExpression* to_rewrite, Scope* scope,
-    bool* ok) {
+    Parser* parser, RewritableAssignmentExpression* to_rewrite, Scope* scope) {
   PatternRewriter rewriter;
 
   DCHECK(!to_rewrite->is_rewritten());
 
+  bool ok = true;
   rewriter.scope_ = scope;
   rewriter.parser_ = parser;
   rewriter.context_ = ASSIGNMENT;
@@ -45,9 +45,21 @@ void Parser::PatternRewriter::RewriteDestructuringAssignment(
   rewriter.block_ = nullptr;
   rewriter.descriptor_ = nullptr;
   rewriter.names_ = nullptr;
-  rewriter.ok_ = ok;
+  rewriter.ok_ = &ok;
 
   rewriter.RecurseIntoSubpattern(rewriter.pattern_, nullptr);
+  DCHECK(ok);
+}
+
+
+Expression* Parser::PatternRewriter::RewriteDestructuringAssignment(
+    Parser* parser, Assignment* assignment, Scope* scope) {
+  DCHECK_NOT_NULL(assignment);
+  DCHECK_EQ(Token::ASSIGN, assignment->op());
+  auto to_rewrite =
+      parser->factory()->NewRewritableAssignmentExpression(assignment);
+  RewriteDestructuringAssignment(parser, to_rewrite, scope);
+  return to_rewrite->expression();
 }
 
 

src/parsing/preparser.cc

@@ -956,15 +956,29 @@ PreParser::Statement PreParser::ParseForStatement(bool* ok) {
       }
     } else {
       int lhs_beg_pos = peek_position();
-      Expression lhs = ParseExpression(false, CHECK_OK);
+      ExpressionClassifier classifier;
+      Expression lhs = ParseExpression(false, &classifier, CHECK_OK);
       int lhs_end_pos = scanner()->location().end_pos;
       is_let_identifier_expression =
           lhs.IsIdentifier() && lhs.AsIdentifier().IsLet();
-      if (CheckInOrOf(&mode, ok)) {
+      bool is_for_each = CheckInOrOf(&mode, ok);
       if (!*ok) return Statement::Default();
+      bool is_destructuring = is_for_each &&
+                              allow_harmony_destructuring_assignment() &&
+                              (lhs->IsArrayLiteral() || lhs->IsObjectLiteral());
+
+      if (is_destructuring) {
+        ValidateAssignmentPattern(&classifier, CHECK_OK);
+      } else {
+        ValidateExpression(&classifier, CHECK_OK);
+      }
+
+      if (is_for_each) {
+        if (!is_destructuring) {
           lhs = CheckAndRewriteReferenceExpression(
               lhs, lhs_beg_pos, lhs_end_pos, MessageTemplate::kInvalidLhsInFor,
               kSyntaxError, CHECK_OK);
+        }
         ParseExpression(true, CHECK_OK);
         Expect(Token::RPAREN, CHECK_OK);
         ParseSubStatement(CHECK_OK);

test/cctest/test-parsing.cc

@@ -6828,6 +6828,25 @@ TEST(DestructuringAssignmentPositiveTests) {
       {"'use strict'; let x, y, z; for (x of ", " = {});"},
       {"var x, y, z; for (x in ", " = {});"},
       {"var x, y, z; for (x of ", " = {});"},
+      {"var x, y, z; for (", " in {});"},
+      {"var x, y, z; for (", " of {});"},
+      {"'use strict'; var x, y, z; for (", " in {});"},
+      {"'use strict'; var x, y, z; for (", " of {});"},
+      {NULL, NULL}};
+
+  const char* mixed_assignments_context_data[][2] = {
+      {"'use strict'; let x, y, z; (", " = z = {});"},
+      {"var x, y, z; (", " = z = {});"},
+      {"'use strict'; let x, y, z; (x = ", " = z = {});"},
+      {"var x, y, z; (x = ", " = z = {});"},
+      {"'use strict'; let x, y, z; for (x in ", " = z = {});"},
+      {"'use strict'; let x, y, z; for (x in x = ", " = z = {});"},
+      {"'use strict'; let x, y, z; for (x of ", " = z = {});"},
+      {"'use strict'; let x, y, z; for (x of x = ", " = z = {});"},
+      {"var x, y, z; for (x in ", " = z = {});"},
+      {"var x, y, z; for (x in x = ", " = z = {});"},
+      {"var x, y, z; for (x of ", " = z = {});"},
+      {"var x, y, z; for (x of x = ", " = z = {});"},
       {NULL, NULL}};
 
   // clang-format off
@@ -6901,8 +6920,6 @@ TEST(DestructuringAssignmentPositiveTests) {
     "[ [ foo()[x] = 10 ] = {} ]",
     "[ [ x.y = 10 ] = {} ]",
     "[ [ x[y] = 10 ] = {} ]",
-
-    "{ x : y }",
     "{ x : y = 1 }",
     "{ x }",
     "{ x, y, z }",
@@ -6945,12 +6962,8 @@ TEST(DestructuringAssignmentPositiveTests) {
     "[...x]",
     "[x,y,...z]",
     "[x,,...z]",
-    "{ x: y } = z",
-    "[x, y] = z",
-    "{ x: y } = { z }",
-    "[x, y] = { z }",
-    "{ x: y } = [ z ]",
-    "[x, y] = [ z ]",
+    "{ x: y }",
+    "[x, y]",
     "[((x, y) => z).x]",
     "{x: ((y, z) => z).x}",
     "[((x, y) => z)['x']]",
@@ -6966,6 +6979,9 @@ TEST(DestructuringAssignmentPositiveTests) {
   RunParserSyncTest(context_data, data, kSuccess, NULL, 0, always_flags,
                     arraysize(always_flags));
 
+  RunParserSyncTest(mixed_assignments_context_data, data, kSuccess, NULL, 0,
+                    always_flags, arraysize(always_flags));
+
   const char* empty_context_data[][2] = {
       {"'use strict';", ""}, {"", ""}, {NULL, NULL}};
 
@@ -7755,3 +7771,12 @@ TEST(EscapedKeywords) {
   RunModuleParserSyncTest(strict_context_data, valid_data, kError, NULL, 0,
                           always_flags, arraysize(always_flags));
 }
+
+
+TEST(MiscSyntaxErrors) {
+  const char* context_data[][2] = {
+      {"'use strict'", ""}, {"", ""}, {NULL, NULL}};
+  const char* error_data[] = {"for (();;) {}", NULL};
+
+  RunParserSyncTest(context_data, error_data, kError, NULL, 0, NULL, 0);
+}

test/mjsunit/harmony/destructuring-assignment.js

@@ -428,3 +428,55 @@ assertEquals(oz, [1, 2, 3, 4, 5]);
   assertThrows(() => { ({ a: [ c ] } = { a: [ "nope!" ] }); }, TypeError);
   assertEquals("untouchable", c);
 })();
+
+(function testForIn() {
+  var log = [];
+  var x = {};
+  var object = {
+    "Apenguin": 1,
+    "\u{1F382}cake": 2,
+    "Bpuppy": 3,
+    "Cspork": 4
+  };
+  for ([x.firstLetter, ...x.rest] in object) {
+    if (x.firstLetter === "A") {
+      assertEquals(["p", "e", "n", "g", "u", "i", "n"], x.rest);
+      continue;
+    }
+    if (x.firstLetter === "C") {
+      assertEquals(["s", "p", "o", "r", "k"], x.rest);
+      break;
+    }
+    log.push({ firstLetter: x.firstLetter, rest: x.rest });
+  }
+  assertEquals([
+    { firstLetter: "\u{1F382}", rest: ["c", "a", "k", "e"] },
+    { firstLetter: "B", rest: ["p", "u", "p", "p", "y"] },
+  ], log);
+})();
+
+(function testForOf() {
+  var log = [];
+  var x = {};
+  var names = [
+    "Apenguin",
+    "\u{1F382}cake",
+    "Bpuppy",
+    "Cspork"
+  ];
+  for ([x.firstLetter, ...x.rest] of names) {
+    if (x.firstLetter === "A") {
+      assertEquals(["p", "e", "n", "g", "u", "i", "n"], x.rest);
+      continue;
+    }
+    if (x.firstLetter === "C") {
+      assertEquals(["s", "p", "o", "r", "k"], x.rest);
+      break;
+    }
+    log.push({ firstLetter: x.firstLetter, rest: x.rest });
+  }
+  assertEquals([
+    { firstLetter: "\u{1F382}", rest: ["c", "a", "k", "e"] },
+    { firstLetter: "B", rest: ["p", "u", "p", "p", "y"] },
+  ], log);
+})();