[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 1b177080bb3c0f7c50c983bd4a22f441faef33b6..01dffc0a4d489374b7d826bfaa9465d003a0483d 100644
--- a/src/parser.cc
+++ b/src/parser.cc
@@ -4165,11 +4165,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;
 }
@@ -4326,6 +4321,99 @@ Block* Parser::BuildParameterInitializationBlock(
     PatternRewriter::DeclareAndInitializeVariables(init_block, &descriptor,
                                                    &decl, nullptr, CHECK_OK);
   }
+  if (formal_parameters.has_rest) {
+    auto rest_parameter = formal_parameters.params.last();
+    int rest_index = formal_parameters.params.length() - 1;
+    DCHECK_NULL(rest_parameter.pattern);
+    if (init_block == nullptr) {
+      init_block = factory()->NewBlock(NULL, 1, true, RelocInfo::kNoPosition);
+    }
+
+    // DESUGAR:
+    // let <rest_params> = [];

[Michael Starzinger, 2015/07/15 11:32:11]

Just throwing this out there, not saying we should do this (now):

Would it be a valid desugaring to pre-allocate the array to the correct length
and then use ordinary [[Put]] operations to populate it? Or am I missing some
JavaScript magic that makes this observable? Basically have something like the
following:

let <rest_length> = %_ArgumentsLength() - <rest_index>;
let <rest_params> = new $PremordialArray(<rest_length>);
for (int i = 0; i < <rest_length>; ++i) {
  <rest_params>[i] = %_Arguments(i + <rest_index>);
}

Again, not intended to be actionable feedback right now, just a question.

[caitp (gmail), 2015/07/15 11:36:32]

I don't think there's any problem doing it that way

[Michael Starzinger, 2015/07/15 11:41:15]

Awesome. Thanks! In that case I would be fine with this CL's approach as it is,
knowing we have the option available to go into that direction in the future.

[Toon Verwaest, 2015/07/15 18:57:39]

When you preallocate an array of a specific length you just have a holey array.
Any regular assignment will possibly trigger accessors up the prototype chain
(of Array.prototype and its prototype chain, normally Object.prototype); or fail
because there's a read-only element. %AddElement OTOH is a subset of
DefineProperty, which ignores the prototype chain.

[Michael Starzinger, 2015/07/15 19:07:43]

Yes, you are right. I should have been more explicit in my pseudo-code. Consider
the following ...

let <rest_length> = %_ArgumentsLength() - <rest_index>;
let <rest_params> = %_NewPrimordialArrayFilledWithUndefined(<rest_length>);
for (int i = 0; i < <rest_length>; ++i) {
  <rest_params>[i] = %_Arguments(i + <rest_index>);
}

+    // for (int i = <rest_index>; i < <arguments.length>; ++i) {
+    //   %AddElement(<rest_params>, <rest_params.length>, arguments[i++]);
+    // }
+    VariableProxy* proxy = factory()->NewVariableProxy(rest_parameter.var);
+    Declaration* declaration = factory()->NewVariableDeclaration(
+        proxy, VAR, scope_, RelocInfo::kNoPosition);

[rossberg, 2015/07/15 12:10:16]

Shouldn't this be LET?

[caitp (gmail), 2015/07/15 13:53:35]

There are 2 things that get in the way: missing position information, and the
var is already declared as VAR. can work around it but maybe better for a
followup?

[rossberg, 2015/07/15 14:54:31]

I'm fine with that. You can find a related TODO in the tests of the parameter
scoping CL that I uploaded earlier today. :)

+    Variable* var = this->Declare(declaration, DeclarationDescriptor::PARAMETER,
+                                  false, CHECK_OK);
+    proxy = factory()->NewVariableProxy(var);
+
+    // TODO(@caitp): use an array literal to avoid escape

[rossberg, 2015/07/15 12:10:16]

Can you do that in this CL? It would avoid adding another runtime function.

[caitp (gmail), 2015/07/15 13:53:35]

This is proving to be very difficult to accomplish, especially for arrow
functions. I think it will take some time to juggle the literal indexes just
right

[rossberg, 2015/07/15 14:54:31]

I see. Fine to leave for follow-up as well.

+    auto empty_values = new (zone()) ZoneList<Expression*>(0, zone());
+    Expression* empty_array = factory()->NewCallRuntime(
+        ast_value_factory()->empty_array_string(), nullptr, empty_values,
+        RelocInfo::kNoPosition);
+
+    Assignment* init_array = factory()->NewAssignment(
+        Token::INIT_VAR, proxy, empty_array, RelocInfo::kNoPosition);
+
+    auto loop = factory()->NewForStatement(NULL, RelocInfo::kNoPosition);
+
+    // argument_index = rest_index
+    Variable* argument_index =
+        scope_->NewTemporary(ast_value_factory()->dot_string());
+    auto init = factory()->NewExpressionStatement(
+        factory()->NewAssignment(
+            Token::INIT_VAR, factory()->NewVariableProxy(argument_index),
+            factory()->NewSmiLiteral(rest_index, RelocInfo::kNoPosition),
+            RelocInfo::kNoPosition),
+        RelocInfo::kNoPosition);
+
+    auto arguments_length = Runtime::FunctionForId(Runtime::kArgumentsLength);
+    auto empty_arguments = new (zone()) ZoneList<Expression*>(0, zone());
+
+    // argument_index < %_ArgumentsLength()
+    auto cond = factory()->NewCompareOperation(
+        Token::LT, factory()->NewVariableProxy(argument_index),
+        factory()->NewCallRuntime(ast_value_factory()->empty_string(),
+                                  arguments_length, 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);
+
+    auto arguments = Runtime::FunctionForId(Runtime::kArguments);
+    // %_Arguments(<argument_index>)
+    auto arguments_args = new (zone()) ZoneList<Expression*>(1, zone());
+    arguments_args->Add(factory()->NewVariableProxy(argument_index), zone());
+
+    // %AddElement(
+    //     <restParam>, <restParam.length>, %_Arguments(<argument_index>));
+    auto add_element = Runtime::FunctionForId(Runtime::kAddElement);
+    auto add_element_args = new (zone()) ZoneList<Expression*>(3, zone());
+    add_element_args->Add(factory()->NewVariableProxy(var), zone());
+    add_element_args->Add(
+        factory()->NewProperty(
+            factory()->NewVariableProxy(var),
+            factory()->NewStringLiteral(ast_value_factory()->length_string(),
+                                        RelocInfo::kNoPosition),
+            RelocInfo::kNoPosition),
+        zone());
+    add_element_args->Add(factory()->NewCallRuntime(
+                              ast_value_factory()->empty_string(), arguments,
+                              arguments_args, RelocInfo::kNoPosition),
+                          zone());
+
+    auto body = factory()->NewExpressionStatement(
+        factory()->NewCallRuntime(ast_value_factory()->empty_string(),
+                                  add_element, add_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());
+  }
   return init_block;
 }