[es6] Re-implement rest parameters via desugaring.

src/parser.cc

Index: src/parser.cc
diff --git a/src/parser.cc b/src/parser.cc
index 3f635c70df275ea837606aa81354f6daea36a882..bf5128e8dddfa6d5acd860a11da42dd8307c9079 100644
--- a/src/parser.cc
+++ b/src/parser.cc
@@ -3942,9 +3942,19 @@ void ParserTraits::ParseArrowFunctionFormalParameters(
   if (is_rest) {
     expr = expr->AsSpread()->expression();
     parameters->has_rest = true;
+
+    if (!expr->IsVariableProxy()) {
+      ReportMessageAt(params_loc, MessageTemplate::kMalformedArrowFunParamList);

[wingo, 2015/08/31 13:07:31]

Why is this necessary?  ParseArrowFunctionFormalParameters should only be called
on a valid production.

[caitp (gmail), 2015/08/31 13:23:40]

You're right, I realized that this morning but haven't fixed it yet. Going to
airport soon, but I'll update with a new patchset with a fix when I get back

+      *ok = false;
+      return;
+    }
+
+    parameters->rest_array_literal_index =
+        parser_->function_state_->NextMaterializedLiteralIndex();
+    ++parameters->materialized_literals_count;
   }
   if (parameters->is_simple) {
-    parameters->is_simple = !is_rest && expr->IsVariableProxy();
+    parameters->is_simple = expr->IsVariableProxy();
   }
 
   Expression* initializer = nullptr;
@@ -3998,6 +4008,11 @@ void ParserTraits::ReindexLiterals(const ParserFormalParameters& parameters) {
     for (const auto p : parameters.params) {
       if (p.pattern != nullptr) reindexer.Reindex(p.pattern);
     }
+
+    if (parameters.has_rest) {
+      parameters.rest_array_literal_index = reindexer.NextIndex();
+    }
+
     DCHECK(reindexer.count() <=
            parser_->function_state_->materialized_literal_count());
   }
@@ -4167,6 +4182,10 @@ FunctionLiteral* Parser::ParseFunctionLiteral(
                            &expected_property_count, /*CHECK_OK*/ ok,
                            maybe_bookmark);
 
+      if (formals.materialized_literals_count > 0) {
+        materialized_literal_count += formals.materialized_literals_count;
+      }

[wingo, 2015/08/31 13:07:31]

Is this bit specific to this patch or is it a general fix?

[caitp (gmail), 2015/08/31 13:23:40]

Well, it's a mix. See https://code.google.com/p/v8/issues/detail?id=4400 for
other bugs related to this.

I'm hoping the various literal counter strategies can be sort of unified later
on to make things simpler, but this was the simplest way to do it for this CL

+
       if (bookmark.HasBeenReset()) {
         // Trigger eager (re-)parsing, just below this block.
         is_lazily_parsed = false;
@@ -4231,11 +4250,6 @@ FunctionLiteral* Parser::ParseFunctionLiteral(
   if (should_be_used_once_hint)
     function_literal->set_should_be_used_once_hint();
 
-  if (scope->has_rest_parameter()) {
-    // TODO(caitp): enable optimization of functions with rest params
-    function_literal->set_dont_optimize_reason(kRestParameter);
-  }
-
   if (fni_ != NULL && should_infer_name) fni_->AddFunction(function_literal);
   return function_literal;
 }
@@ -4370,8 +4384,6 @@ Block* Parser::BuildParameterInitializationBlock(
       factory()->NewBlock(NULL, 1, true, RelocInfo::kNoPosition);
   for (int i = 0; i < parameters.params.length(); ++i) {
     auto parameter = parameters.params[i];
-    // TODO(caitp,rossberg): Remove special handling for rest once desugared.
-    if (parameter.is_rest) break;
     DeclarationDescriptor descriptor;
     descriptor.declaration_kind = DeclarationDescriptor::PARAMETER;
     descriptor.parser = this;
@@ -4399,6 +4411,86 @@ Block* Parser::BuildParameterInitializationBlock(
       descriptor.initialization_pos = parameter.initializer->position();
     }
 
+    if (parameter.is_rest) {

[wingo, 2015/08/31 13:07:31]

This should probably be "else if" and the previous case should
DCHECK(!parameter.is_rest), I think

[adamk, 2015/08/31 21:17:28]

Agreed on the else-if suggestion here.

+      // $rest = [];
+      // for (var $argument_index = $rest_index;
+      //      $argument_index < %_ArgumentsLength();
+      //      ++$argument_index) {
+      //   %AppendElement($rest, %_Arguments($argument_index));
+      // }
+      // let <param> = $rest;
+      DCHECK_NULL(parameter.initializer);

[adamk, 2015/08/31 21:17:28]

That'll get rid of this DCHECK (or rather replace it with the DCHECK proposed by
wingo)

[caitp (gmail), 2015/08/31 22:43:15]

Right, forgot about this one. Done in the last patchset (also includes a rebase
though, so the interdiff will be bad)

+      DCHECK(parameter.pattern->IsVariableProxy());
+      DCHECK_EQ(i, parameters.params.length() - 1);
+
+      int pos = parameter.pattern->position();
+      auto var = parameters.scope->parameter(i);
+      auto empty_values = new (zone()) ZoneList<Expression*>(0, zone());
+      auto empty_array = factory()->NewArrayLiteral(
+          empty_values, parameters.rest_array_literal_index,

[wingo, 2015/08/31 13:07:31]

Is it necessary to reserve a literal index in the reindexer?  Since we make it
here and we're in the context of the function, seems to me we might be able to
reserve the index with the normal mechanism.

[caitp (gmail), 2015/08/31 13:23:40]

It seems to be necessary. Without it, an extra slot is allocated in the
context's literals array, which makes things hard for lazy parsing, and is
potentially a bug farm too. I haven't gotten it working without saving the
literal index (either in an AST node or here) and rewriting it correctly

+          is_strong(language_mode()), RelocInfo::kNoPosition);
+
+      auto init_array = factory()->NewAssignment(
+          Token::INIT_VAR, factory()->NewVariableProxy(var), empty_array,
+          RelocInfo::kNoPosition);
+
+      auto loop = factory()->NewForStatement(NULL, RelocInfo::kNoPosition);
+
+      auto argument_index =
+          parameters.scope->NewTemporary(ast_value_factory()->empty_string());
+      auto init = factory()->NewExpressionStatement(
+          factory()->NewAssignment(
+              Token::INIT_VAR, factory()->NewVariableProxy(argument_index),
+              factory()->NewSmiLiteral(i, RelocInfo::kNoPosition),
+              RelocInfo::kNoPosition),
+          RelocInfo::kNoPosition);
+
+      auto empty_arguments = new (zone()) ZoneList<Expression*>(0, zone());
+
+      // $arguments_index < arguments.length
+      auto cond = factory()->NewCompareOperation(
+          Token::LT, factory()->NewVariableProxy(argument_index),
+          factory()->NewCallRuntime(Runtime::kInlineArgumentsLength,
+                                    empty_arguments, RelocInfo::kNoPosition),
+          RelocInfo::kNoPosition);
+
+      // ++argument_index
+      auto next = factory()->NewExpressionStatement(
+          factory()->NewCountOperation(
+              Token::INC, true, factory()->NewVariableProxy(argument_index),
+              RelocInfo::kNoPosition),
+          RelocInfo::kNoPosition);
+
+      // %_Arguments($arguments_index)
+      auto arguments_args = new (zone()) ZoneList<Expression*>(1, zone());
+      arguments_args->Add(factory()->NewVariableProxy(argument_index), zone());
+
+      // %AppendElement($rest, %_Arguments($arguments_index))
+      auto append_element_args = new (zone()) ZoneList<Expression*>(2, zone());
+
+      append_element_args->Add(factory()->NewVariableProxy(var), zone());
+      append_element_args->Add(
+          factory()->NewCallRuntime(Runtime::kInlineArguments, arguments_args,
+                                    RelocInfo::kNoPosition),
+          zone());
+
+      auto body = factory()->NewExpressionStatement(
+          factory()->NewCallRuntime(Runtime::kAppendElement,
+                                    append_element_args,
+                                    RelocInfo::kNoPosition),
+          RelocInfo::kNoPosition);
+
+      loop->Initialize(init, cond, next, body);
+
+      init_block->AddStatement(
+          factory()->NewExpressionStatement(init_array, RelocInfo::kNoPosition),
+          zone());
+
+      init_block->AddStatement(loop, zone());
+
+      descriptor.initialization_pos = pos;
+    }
+
     Scope* param_scope = scope_;
     Block* param_block = init_block;
     if (!parameter.is_simple() && scope_->calls_sloppy_eval()) {