[es6] Re-implement rest parameters via desugaring.

src/parser.cc

 // Copyright 2012 the V8 project authors. All rights reserved.
 // Use of this source code is governed by a BSD-style license that can be
 // found in the LICENSE file.
 
 #include "src/parser.h"
 
 #include "src/api.h"
 #include "src/ast.h"
 #include "src/ast-literal-reindexer.h"
 #include "src/bailout-reason.h"
@@ skipping 3924 matching lines @@
     expr = right;
   }
 
   // Only the right-most expression may be a rest parameter.
   DCHECK(!parameters->has_rest);
 
   bool is_rest = expr->IsSpread();
   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;
   if (expr->IsVariableProxy()) {
     // When the formal parameter was originally seen, it was parsed as a
     // VariableProxy and recorded as unresolved in the scope.  Here we undo that
     // parse-time side-effect for parameters that are single-names (not
     // patterns; for patterns that happens uniformly in
     // PatternRewriter::VisitVariableProxy).
     parser_->scope_->RemoveUnresolved(expr->AsVariableProxy());
@@ skipping 33 matching lines @@
 }
 
 
 void ParserTraits::ReindexLiterals(const ParserFormalParameters& parameters) {
   if (parser_->function_state_->materialized_literal_count() > 0) {
     AstLiteralReindexer reindexer;
 
     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());
   }
 }
 
 
 FunctionLiteral* Parser::ParseFunctionLiteral(
     const AstRawString* function_name, Scanner::Location function_name_location,
     FunctionNameValidity function_name_validity, FunctionKind kind,
     int function_token_pos, FunctionLiteral::FunctionType function_type,
@@ skipping 149 matching lines @@
     // parsing if it suspect that wasn't a good idea. If so, or if we didn't
     // try to lazy parse in the first place, we'll have to parse eagerly.
     Scanner::BookmarkScope bookmark(scanner());
     if (is_lazily_parsed) {
       Scanner::BookmarkScope* maybe_bookmark =
           bookmark.Set() ? &bookmark : nullptr;
       SkipLazyFunctionBody(&materialized_literal_count,
                            &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;
 
         // This is probably an initialization function. Inform the compiler it
         // should also eager-compile this function, and that we expect it to be
         // used once.
         eager_compile_hint = FunctionLiteral::kShouldEagerCompile;
         should_be_used_once_hint = true;
       }
@@ skipping 44 matching lines @@
 
   FunctionLiteral* function_literal = factory()->NewFunctionLiteral(
       function_name, ast_value_factory(), scope, body,
       materialized_literal_count, expected_property_count, arity,
       duplicate_parameters, function_type, FunctionLiteral::kIsFunction,
       eager_compile_hint, kind, pos);
   function_literal->set_function_token_position(function_token_pos);
   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;
 }
 
 
 void Parser::SkipLazyFunctionBody(int* materialized_literal_count,
                                   int* expected_property_count, bool* ok,
                                   Scanner::BookmarkScope* bookmark) {
   DCHECK_IMPLIES(bookmark, bookmark->HasBeenSet());
   if (produce_cached_parse_data()) CHECK(log_);
@@ skipping 114 matching lines @@
 
 
 Block* Parser::BuildParameterInitializationBlock(
     const ParserFormalParameters& parameters, bool* ok) {
   DCHECK(!parameters.is_simple);
   DCHECK(scope_->is_function_scope());
   Block* init_block =
       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;
     descriptor.declaration_scope = scope_;
     descriptor.scope = scope_;
     descriptor.hoist_scope = nullptr;
     descriptor.mode = LET;
     descriptor.is_const = false;
     descriptor.needs_init = true;
     descriptor.declaration_pos = parameter.pattern->position();
     descriptor.initialization_pos = parameter.pattern->position();
     descriptor.init_op = Token::INIT_LET;
     Expression* initial_value =
         factory()->NewVariableProxy(parameters.scope->parameter(i));
     if (parameter.initializer != nullptr) {
       // IS_UNDEFINED($param) ? initializer : $param
       auto condition = factory()->NewCompareOperation(
           Token::EQ_STRICT,
           factory()->NewVariableProxy(parameters.scope->parameter(i)),
           factory()->NewUndefinedLiteral(RelocInfo::kNoPosition),
           RelocInfo::kNoPosition);
       initial_value = factory()->NewConditional(
           condition, parameter.initializer, initial_value,
           RelocInfo::kNoPosition);
       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()) {
       param_scope = NewScope(scope_, BLOCK_SCOPE);
       param_scope->set_is_declaration_scope();
       param_scope->set_start_position(parameter.pattern->position());
       param_scope->set_end_position(RelocInfo::kNoPosition);
       param_scope->RecordEvalCall();
       param_block = factory()->NewBlock(NULL, 8, true, RelocInfo::kNoPosition);
       param_block->set_scope(param_scope);
@@ skipping 1694 matching lines @@
 
 Expression* Parser::SpreadCallNew(Expression* function,
                                   ZoneList<v8::internal::Expression*>* args,
                                   int pos) {
   args->InsertAt(0, function, zone());
 
   return factory()->NewCallRuntime(Context::REFLECT_CONSTRUCT_INDEX, args, pos);
 }
 }  // namespace internal
 }  // namespace v8