Add spread rewriting

src/parsing/parser.cc

Index: src/parsing/parser.cc
diff --git a/src/parsing/parser.cc b/src/parsing/parser.cc
index 12ea1e285ee1ac70dc8fe2291039c897fa5d8732..8cda452cc6401b8b9698060e2fae3b2d856f94d5 100644
--- a/src/parsing/parser.cc
+++ b/src/parsing/parser.cc
@@ -6,6 +6,7 @@
 
 #include "src/api.h"
 #include "src/ast/ast.h"
+#include "src/ast/ast-expression-rewriter.h"
 #include "src/ast/ast-expression-visitor.h"
 #include "src/ast/ast-literal-reindexer.h"
 #include "src/ast/scopeinfo.h"
@@ -5404,9 +5405,44 @@ ObjectLiteralProperty* ParserTraits::RewriteObjectLiteralProperty(
 }
 
 
+class SpreadRewriter : public AstExpressionRewriter {
+ public:
+  SpreadRewriter(uintptr_t stack_limit, Parser* parser)
+      : AstExpressionRewriter(stack_limit), parser_(parser) {}
+  ~SpreadRewriter() override {}
+
+ private:
+  bool RewriteExpression(Expression* expr) override {
+    // Rewrite only what could have been a pattern.
+    // The actual work is in array literals.
+    if (expr->IsArrayLiteral()) {
+      ArrayLiteral* lit = expr->AsArrayLiteral();
+      VisitExpressions(lit->values());
+      replacement_ = parser_->RewriteSpreads(lit);
+      return false;
+    }
+    if (expr->IsObjectLiteral()) {
+      return true;
+    }
+    if (expr->IsBinaryOperation() &&
+        expr->AsBinaryOperation()->op() == Token::COMMA) {
+      return true;
+    }
+    // Everything else does not need rewriting.
+    return false;
+  }
+
+  Parser* parser_;
+};
+
+
 Expression* Parser::RewriteExpression(Expression* expr) {
-  // TODO(nikolaos): For the time being, this does no rewriting at all.
-  return expr;
+  SpreadRewriter rewriter(stack_limit_, this);
+  AstNode* node = rewriter.Rewrite(expr);
+  DCHECK_NOT_NULL(node);

[rossberg, 2016/01/11 12:50:14]

Nit: having 2 null checks here may be a bit over-defensive. Doesn't the latter
one suffice?

[nickie, 2016/01/13 10:15:21]

Acknowledged.  I will remove the first.  It was probably left from when I was
debugging.

+  Expression* result = reinterpret_cast<Expression*>(node);
+  DCHECK_NOT_NULL(result);
+  return result;
 }
 
 
@@ -5442,6 +5478,104 @@ void Parser::RewriteDestructuringAssignments() {
 }
 
 
+Expression* Parser::RewriteSpreads(ArrayLiteral* lit) {
+  ZoneList<Expression*>::iterator s = lit->FirstSpread();
+  if (s == lit->EndValue()) return nullptr;
+  // TODO(nikolaos): Check and fix the positions for the generated AST.
+  Variable* result =
+      scope_->NewTemporary(ast_value_factory()->dot_result_string());
+  Expression* init_result =
+      factory()->NewAssignment(Token::INIT, factory()->NewVariableProxy(result),
+                               lit, RelocInfo::kNoPosition);
+  Block* do_block =
+      factory()->NewBlock(nullptr, 16, false, RelocInfo::kNoPosition);
+  do_block->statements()->Add(
+      factory()->NewExpressionStatement(init_result, RelocInfo::kNoPosition),
+      zone());
+  while (s != lit->EndValue()) {
+    Expression* value = *s++;
+    Spread* spread = value->AsSpread();
+    if (spread == nullptr) {

[rossberg, 2016/01/11 12:50:14]

Nit: add comments to both branches of this conditional that give the overall
rewriting.

[nickie, 2016/01/13 10:15:21]

Acknowledged.

+      ZoneList<Expression*>* append_element_args = NewExpressionList(2, zone());
+      append_element_args->Add(factory()->NewVariableProxy(result), zone());
+      append_element_args->Add(value, zone());
+      do_block->statements()->Add(
+          factory()->NewExpressionStatement(
+              factory()->NewCallRuntime(Runtime::kAppendElement,
+                                        append_element_args,
+                                        RelocInfo::kNoPosition),
+              RelocInfo::kNoPosition),
+          zone());
+    } else {
+      Variable* each =
+          scope_->NewTemporary(ast_value_factory()->dot_for_string());
+      Expression* subject = spread->expression();
+      Variable* iterator =
+          scope_->NewTemporary(ast_value_factory()->dot_iterator_string());
+      Variable* element =
+          scope_->NewTemporary(ast_value_factory()->dot_result_string());
+      // iterator = subject[Symbol.iterator]()
+      Expression* assign_iterator = factory()->NewAssignment(
+          Token::ASSIGN, factory()->NewVariableProxy(iterator),
+          GetIterator(subject, factory(), spread->position()),
+          spread->position());
+      // !%_IsJSReceiver(element = iterator.next()) &&
+      //     %ThrowIteratorResultNotAnObject(element)
+      Expression* next_element;
+      {
+        // element = iterator.next()
+        Expression* iterator_proxy = factory()->NewVariableProxy(iterator);
+        next_element = BuildIteratorNextResult(iterator_proxy, element,
+                                               spread->position());
+      }
+      // element.done
+      Expression* element_done;
+      {
+        Expression* done_literal = factory()->NewStringLiteral(
+            ast_value_factory()->done_string(), RelocInfo::kNoPosition);
+        Expression* element_proxy = factory()->NewVariableProxy(element);
+        element_done = factory()->NewProperty(element_proxy, done_literal,
+                                              RelocInfo::kNoPosition);
+      }
+      // each = element.value
+      Expression* assign_each;
+      {
+        Expression* value_literal = factory()->NewStringLiteral(
+            ast_value_factory()->value_string(), RelocInfo::kNoPosition);
+        Expression* element_proxy = factory()->NewVariableProxy(element);
+        Expression* element_value = factory()->NewProperty(
+            element_proxy, value_literal, RelocInfo::kNoPosition);
+        assign_each = factory()->NewAssignment(
+            Token::ASSIGN, factory()->NewVariableProxy(each), element_value,
+            RelocInfo::kNoPosition);
+      }
+      // append each to the result
+      Statement* append_body;
+      {
+        ZoneList<Expression*>* append_element_args =
+            NewExpressionList(2, zone());
+        append_element_args->Add(factory()->NewVariableProxy(result), zone());
+        append_element_args->Add(factory()->NewVariableProxy(each), zone());
+        append_body = factory()->NewExpressionStatement(
+            factory()->NewCallRuntime(Runtime::kAppendElement,
+                                      append_element_args,
+                                      RelocInfo::kNoPosition),
+            RelocInfo::kNoPosition);
+      }
+      ForEachStatement* loop = factory()->NewForEachStatement(
+          ForEachStatement::ITERATE, nullptr, spread->position());
+      ForOfStatement* for_of = loop->AsForOfStatement();
+      for_of->Initialize(factory()->NewVariableProxy(each), subject,
+                         append_body, assign_iterator, next_element,
+                         element_done, assign_each);
+      do_block->statements()->Add(for_of, zone());
+    }
+  }
+  lit->RewindSpreads();

[rossberg, 2016/01/11 12:50:14]

Why is this needed?

[nickie, 2016/01/13 10:15:21]

This is very important.  Line 5487 initializes the result of the do expression
(the temporary variable containing the array that we build).  The initial value
assigned there is the array literal (lit), spreads and all.  But it must not be
the whole array literal; it must be just the values up to (and excluding)
first_spread_index_, as the rest will be added with the calls to %AppendElement.
 So, I'm "rewinding" the array literal, truncating everything from
first_spread_index_ until the end.  This is done by ArrayLiteral::RewindSpreads,
defined in ast.h:1693.

I'll add some comments to the code, to explain all this.

+  return factory()->NewDoExpression(do_block, result, lit->position());
+}
+
+
 void ParserTraits::QueueDestructuringAssignmentForRewriting(Expression* expr) {
   DCHECK(expr->IsRewritableAssignmentExpression());
   parser_->function_state_->AddDestructuringAssignment(