[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..62d57cc6201a0df650c682839ebde81537659e13 100644
--- a/src/parser.cc
+++ b/src/parser.cc
@@ -3939,12 +3939,25 @@ void ParserTraits::ParseArrowFunctionFormalParameters(
   DCHECK(!parameters->has_rest);
 
   bool is_rest = expr->IsSpread();
+  int start_pos = expr->position();
+
   if (is_rest) {
     expr = expr->AsSpread()->expression();
     parameters->has_rest = true;
+
+    if (!expr->IsVariableProxy()) {
+      ReportMessageAt(params_loc, MessageTemplate::kMalformedArrowFunParamList);
+      *ok = false;
+      return;
+    }
+
+    expr = parser_->factory()->NewRestParameter(
+        expr->AsVariableProxy(),
+        parser_->function_state_->NextMaterializedLiteralIndex(), start_pos);
+    ++parameters->materialized_literals_count;
   }
   if (parameters->is_simple) {
-    parameters->is_simple = !is_rest && expr->IsVariableProxy();
+    parameters->is_simple = expr->IsVariableProxy();
   }
 
   Expression* initializer = nullptr;
@@ -3993,11 +4006,13 @@ void ParserTraits::ParseArrowFunctionFormalParameterList(
 
 void ParserTraits::ReindexLiterals(const ParserFormalParameters& parameters) {
   if (parser_->function_state_->materialized_literal_count() > 0) {
+    // Rest parameter array is always the first literal.
     AstLiteralReindexer reindexer;
 
     for (const auto p : parameters.params) {

[adamk, 2015/08/28 23:26:37]

So is const-correctness the only thing keeping you from handling rest specially
here, instead of adding a new AST node? The AST node adds a good bit of
cognitive overhead, and a very strange new type of "Expression" (though I guess
ExpressionClassifier already let that boat sail).

If you could put the logic here, it seems like all you'd need is a field in
ParserFormalParameters::Parameter to store the literal index, and a public
method on AstLiteralReindexer to do the updating.

[caitp (gmail), 2015/08/28 23:58:01]

Yeah, it's true that doing it all here could get rid of the new node --- but it
also means changing a lot of const references to non-const, and maybe mutating
stuff that you normally wouldn't expect to be mutated. It could lead to bugs,
but if the simplicity is worthwhile, I'm all for that

[adamk, 2015/08/29 01:01:07]

The current thing is cheating quite a bit, const-wise: it only works because the
parameters hold non-const pointers to Expression*. I'm not sure why
FormalParametersT is passed by const-ref everywhere while ExpressionClassifier,
for example, is passed by mutable-pointer. To get a minimal change going you
could even use the 'mutable' keyword on the bit you need to change in a
Parameter.

So, fwiw, my taste would be to avoid a new AST node here.

[caitp (gmail), 2015/08/29 15:51:59]

Done.

       if (p.pattern != nullptr) reindexer.Reindex(p.pattern);
     }
+
     DCHECK(reindexer.count() <=
            parser_->function_state_->materialized_literal_count());
   }
@@ -4167,6 +4182,11 @@ FunctionLiteral* Parser::ParseFunctionLiteral(
                            &expected_property_count, /*CHECK_OK*/ ok,
                            maybe_bookmark);
 
+      if (formals.has_rest) {
+        DCHECK(formals.materialized_literals_count > 0);
+        materialized_literal_count += formals.materialized_literals_count;
+      }
+
       if (bookmark.HasBeenReset()) {
         // Trigger eager (re-)parsing, just below this block.
         is_lazily_parsed = false;
@@ -4231,11 +4251,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 +4385,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;
@@ -4380,6 +4393,7 @@ Block* Parser::BuildParameterInitializationBlock(
     descriptor.hoist_scope = nullptr;
     descriptor.mode = LET;
     descriptor.is_const = false;
+    descriptor.is_rest = parameter.is_rest;
     descriptor.needs_init = true;
     descriptor.declaration_pos = parameter.pattern->position();
     descriptor.initialization_pos = parameter.pattern->position();
@@ -4399,6 +4413,80 @@ Block* Parser::BuildParameterInitializationBlock(
       descriptor.initialization_pos = parameter.initializer->position();
     }
 
+    if (parameter.is_rest) {

[adamk, 2015/08/28 23:26:37]

Please add a comment here with the full desugaring, it's hard for me to piece
together among all the parser boilerplate.

[caitp (gmail), 2015/08/29 15:51:59]

Done.

+      DCHECK_NULL(parameter.initializer);
+      DCHECK(parameter.pattern->IsRestParameter());
+      DCHECK_EQ(i, parameters.params.length() - 1);
+
+      auto rest_parameter = parameter.pattern->AsRestParameter();
+      int pos = rest_parameter->position();
+      auto var = parameters.scope->parameter(i);
+      auto empty_values = new (zone()) ZoneList<Expression*>(0, zone());
+      auto empty_array = factory()->NewArrayLiteral(
+          empty_values, rest_parameter->literal_index(),
+          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()) {