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

src/parsing/parser.cc

Index: src/parsing/parser.cc
diff --git a/src/parsing/parser.cc b/src/parsing/parser.cc
index ccb106598db55002acf5d4dd0d0534bda03465cb..8e17f478dd1c14a6f8be6a755769e83da744032b 100644
--- a/src/parsing/parser.cc
+++ b/src/parsing/parser.cc
@@ -3326,9 +3326,9 @@ 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) {
   ForOfStatement* for_of = stmt->AsForOfStatement();
 
   if (for_of != NULL) {
@@ -3379,6 +3379,16 @@ 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) {
+        DCHECK(allow_harmony_destructuring_assignment());
+        bool ok = true;
+        auto rewrite =
+            factory()->NewRewritableAssignmentExpression(assign_each);

[adamk, 2015/12/10 17:28:03]

Making the callers here and below construct a RewritableAssignmentExpression is
a little ugly. Can you add a new entry point to PatternRewriter that
encapsulates this, taking an Expression and returning the rewritten one?

[caitp (gmail), 2015/12/10 18:19:35]

Done

+        PatternRewriter::RewriteDestructuringAssignment(this, rewrite, scope_,
+                                                        &ok);
+        DCHECK(ok);

[adamk, 2015/12/10 17:28:03]

It looks like all callers of RewriteDestructuringAssignment do a DCHECK. Can you
remove the ok argument and do that DCHECK to the end of
RewriteDestructuringAssignment?

[caitp (gmail), 2015/12/10 18:19:36]

Done.

+        assign_each = rewrite->expression();
+      }
     }
 
     for_of->Initialize(each, subject, body,
@@ -3387,6 +3397,27 @@ void Parser::InitializeForEachStatement(ForEachStatement* stmt,
                        result_done,
                        assign_each);
   } else {
+    if (is_destructuring) {
+      Variable* temp =
+          scope_->NewTemporary(ast_value_factory()->dot_result_string());

[adamk, 2015/12/10 17:28:03]

dot_result_string has a special usage (for completion value stuff). How about
just the empty string?

[caitp (gmail), 2015/12/10 18:19:36]

I think this is the completion-correct model, and matches what we do for for-of
loops. But, I've changed it to the empty string, let me know if you change your
mind

[adamk, 2015/12/10 19:23:33]

If there's a reason for it to be .result, I'd expect there's some test that
should catch it. Otherwise I'd always prefer empty string to make clear there's
no reason for .result.

+      VariableProxy* temp_proxy = factory()->NewVariableProxy(temp);
+      bool ok = true;
+      auto rewrite =
+          factory()->NewRewritableAssignmentExpression(factory()->NewAssignment(
+              Token::ASSIGN, each, temp_proxy, RelocInfo::kNoPosition));
+      PatternRewriter::RewriteDestructuringAssignment(this, rewrite, scope_,
+                                                      &ok);
+      DCHECK(ok);
+      auto block =
+          factory()->NewBlock(nullptr, 2, false, RelocInfo::kNoPosition);
+      block->statements()->Add(
+          factory()->NewExpressionStatement(rewrite->expression(),
+                                            RelocInfo::kNoPosition),
+          zone());
+      block->statements()->Add(body, zone());
+      body = block;
+      each = factory()->NewVariableProxy(temp);
+    }
     stmt->Initialize(each, subject, body);
   }
 }
@@ -3771,7 +3802,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();
@@ -3822,7 +3854,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 =
@@ -3832,9 +3865,17 @@ Statement* Parser::ParseForStatement(ZoneList<const AstRawString*>* labels,
 
       if (CheckInOrOf(&mode, ok)) {
         if (!*ok) return nullptr;
-        expression = this->CheckAndRewriteReferenceExpression(
-            expression, lhs_beg_pos, lhs_end_pos,
-            MessageTemplate::kInvalidLhsInFor, kSyntaxError, CHECK_OK);
+        bool is_destructuring =
+            allow_harmony_destructuring_assignment() &&
+            (expression->IsArrayLiteral() || expression->IsObjectLiteral());
+        if (is_destructuring) {
+          ValidateAssignmentPattern(&classifier, CHECK_OK);
+        } else {
+          ValidateExpression(&classifier, CHECK_OK);
+          expression = this->CheckAndRewriteReferenceExpression(
+              expression, lhs_beg_pos, lhs_end_pos,
+              MessageTemplate::kInvalidLhsInFor, kSyntaxError, CHECK_OK);
+        }
 
         ForEachStatement* loop =
             factory()->NewForEachStatement(mode, labels, stmt_pos);
@@ -3855,7 +3896,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();