Support rest parameters in arrow functions

R=dslomov@chromium.org, rossberg@chromium.org
LOG=Y
BUG=v8:2700

Review URL: https://codereview.chromium.org/1178523002

Cr-Commit-Position: refs/heads/master@{#28908}

src/parser.cc

@@ -3759,9 +3759,9 @@ Handle<FixedArray> CompileTimeValue::GetElements(Handle<FixedArray> value) {
 }
 
 
-void ParserTraits::DeclareArrowFunctionParameters(
+void ParserTraits::ParseArrowFunctionFormalParameters(
     Scope* scope, Expression* expr, const Scanner::Location& params_loc,
-    Scanner::Location* duplicate_loc, bool* ok) {
+    bool* has_rest, Scanner::Location* duplicate_loc, bool* ok) {
   if (scope->num_parameters() >= Code::kMaxArguments) {
     ReportMessageAt(params_loc, MessageTemplate::kMalformedArrowFunParamList);
     *ok = false;
@@ -3769,15 +3769,18 @@ void ParserTraits::DeclareArrowFunctionParameters(
   }
 
   // ArrowFunctionFormals ::
-  //    Binary(Token::COMMA, ArrowFunctionFormals, VariableProxy)
+  //    Binary(Token::COMMA, NonTailArrowFunctionFormals, Tail)
+  //    Tail
+  // NonTailArrowFunctionFormals ::
+  //    Binary(Token::COMMA, NonTailArrowFunctionFormals, VariableProxy)
   //    VariableProxy
+  // Tail ::
+  //    VariableProxy
+  //    Spread(VariableProxy)
   //
   // As we need to visit the parameters in left-to-right order, we recurse on
   // the left-hand side of comma expressions.
   //
-  // Sadly, for the various malformed_arrow_function_parameter_list errors, we
-  // can't be more specific on the error message or on the location because we
-  // need to match the pre-parser's behavior.
   if (expr->IsBinaryOperation()) {
     BinaryOperation* binop = expr->AsBinaryOperation();
     // The classifier has already run, so we know that the expression is a valid
@@ -3785,13 +3788,21 @@ void ParserTraits::DeclareArrowFunctionParameters(
     DCHECK_EQ(binop->op(), Token::COMMA);
     Expression* left = binop->left();
     Expression* right = binop->right();
-    DeclareArrowFunctionParameters(scope, left, params_loc, duplicate_loc, ok);
+    ParseArrowFunctionFormalParameters(scope, left, params_loc, has_rest,
+                                       duplicate_loc, ok);
     if (!*ok) return;
     // LHS of comma expression should be unparenthesized.
     expr = right;
   }
 
-  // TODO(wingo): Support rest parameters.
+  // Only the right-most expression may be a rest parameter.
+  DCHECK(!*has_rest);
+
+  if (expr->IsSpread()) {
+    *has_rest = true;
+    expr = expr->AsSpread()->expression();
+  }
+
   DCHECK(expr->IsVariableProxy());
   DCHECK(!expr->AsVariableProxy()->is_this());
 
@@ -3804,17 +3815,11 @@ void ParserTraits::DeclareArrowFunctionParameters(
   // parse-time side-effect.
   parser_->scope_->RemoveUnresolved(expr->AsVariableProxy());
 
-  bool is_rest = false;
   ExpressionClassifier classifier;
-  DeclareFormalParameter(scope, raw_name, &classifier, is_rest);
+  DeclareFormalParameter(scope, raw_name, &classifier, *has_rest);
+  if (!duplicate_loc->IsValid()) {
     *duplicate_loc = classifier.duplicate_formal_parameter_error().location;
-}
-
-
-void ParserTraits::ParseArrowFunctionFormalParameters(
-    Scope* scope, Expression* params, const Scanner::Location& params_loc,
-    bool* is_rest, Scanner::Location* duplicate_loc, bool* ok) {
-  DeclareArrowFunctionParameters(scope, params, params_loc, duplicate_loc, ok);
+  }
 }
 
 

src/parser.h

@@ -757,13 +757,9 @@ class ParserTraits {
   V8_INLINE void DeclareFormalParameter(Scope* scope, const AstRawString* name,
                                         ExpressionClassifier* classifier,
                                         bool is_rest);
-  void DeclareArrowFunctionParameters(Scope* scope, Expression* expr,
-                                      const Scanner::Location& params_loc,
-                                      Scanner::Location* duplicate_loc,
-                                      bool* ok);
   void ParseArrowFunctionFormalParameters(Scope* scope, Expression* params,
                                           const Scanner::Location& params_loc,
-                                          bool* is_rest,
+                                          bool* has_rest,
                                           Scanner::Location* duplicate_loc,
                                           bool* ok);
 

src/preparser.h

@@ -1528,7 +1528,7 @@ class PreParserTraits {
 
   V8_INLINE void ParseArrowFunctionFormalParameters(
       Scope* scope, PreParserExpression expression,
-      const Scanner::Location& params_loc, bool* is_rest,
+      const Scanner::Location& params_loc, bool* has_rest,
       Scanner::Location* duplicate_loc, bool* ok);
 
   struct TemplateLiteralState {};
@@ -1752,7 +1752,7 @@ PreParserExpression PreParserTraits::SpreadCallNew(PreParserExpression function,
 
 void PreParserTraits::ParseArrowFunctionFormalParameters(
     Scope* scope, PreParserExpression params,
-    const Scanner::Location& params_loc, bool* is_rest,
+    const Scanner::Location& params_loc, bool* has_rest,
     Scanner::Location* duplicate_loc, bool* ok) {
   // TODO(wingo): Detect duplicated identifiers in paramlists.  Detect parameter
   // lists that are too long.
@@ -2159,6 +2159,24 @@ ParserBase<Traits>::ParsePrimaryExpression(ExpressionClassifier* classifier,
         bool has_rest = false;
         result = this->ParseArrowFunctionLiteral(scope, has_rest,
                                                  args_classifier, CHECK_OK);
+      } else if (allow_harmony_arrow_functions() &&
+                 allow_harmony_rest_params() && Check(Token::ELLIPSIS)) {
+        // (...x) => y
+        Scope* scope =
+            this->NewScope(scope_, ARROW_SCOPE, FunctionKind::kArrowFunction);
+        scope->set_start_position(beg_pos);
+        ExpressionClassifier args_classifier;
+        const bool has_rest = true;
+        this->ParseFormalParameter(scope, has_rest, &args_classifier, CHECK_OK);
+        if (peek() == Token::COMMA) {
+          ReportMessageAt(scanner()->peek_location(),
+                          MessageTemplate::kParamAfterRest);
+          *ok = false;
+          return this->EmptyExpression();
+        }
+        Expect(Token::RPAREN, CHECK_OK);
+        result = this->ParseArrowFunctionLiteral(scope, has_rest,
+                                                 args_classifier, CHECK_OK);
       } else {
         // Heuristically try to detect immediately called functions before
         // seeing the call parentheses.
@@ -2239,11 +2257,29 @@ typename ParserBase<Traits>::ExpressionT ParserBase<Traits>::ParseExpression(
       this->ParseAssignmentExpression(accept_IN, &binding_classifier, CHECK_OK);
   classifier->Accumulate(binding_classifier,
                          ExpressionClassifier::AllProductions);
+  bool seen_rest = false;
   while (peek() == Token::COMMA) {
-    Expect(Token::COMMA, CHECK_OK);
+    if (seen_rest) {
+      // At this point the production can't possibly be valid, but we don't know
+      // which error to signal.
+      classifier->RecordArrowFormalParametersError(
+          scanner()->peek_location(), MessageTemplate::kParamAfterRest);
+    }
+    Consume(Token::COMMA);
+    bool is_rest = false;
+    if (allow_harmony_rest_params() && peek() == Token::ELLIPSIS) {
+      // 'x, y, ...z' in CoverParenthesizedExpressionAndArrowParameterList only
+      // as the formal parameters of'(x, y, ...z) => foo', and is not itself a
+      // valid expression or binding pattern.
+      ExpressionUnexpectedToken(classifier);
+      BindingPatternUnexpectedToken(classifier);
+      Consume(Token::ELLIPSIS);
+      seen_rest = is_rest = true;
+    }
     int pos = position();
     ExpressionT right = this->ParseAssignmentExpression(
         accept_IN, &binding_classifier, CHECK_OK);
+    if (is_rest) right = factory()->NewSpread(right, pos);
     classifier->Accumulate(binding_classifier,
                            ExpressionClassifier::AllProductions);
     result = factory()->NewBinaryOperation(Token::COMMA, result, right, pos);
@@ -2671,7 +2707,6 @@ ParserBase<Traits>::ParseAssignmentExpression(bool accept_IN,
   }
   ExpressionT expression = this->ParseConditionalExpression(
       accept_IN, &arrow_formals_classifier, CHECK_OK);
-  classifier->Accumulate(arrow_formals_classifier);
 
   if (allow_harmony_arrow_functions() && peek() == Token::ARROW) {
     checkpoint.Restore();
@@ -2679,11 +2714,11 @@ ParserBase<Traits>::ParseAssignmentExpression(bool accept_IN,
     ValidateArrowFormalParameters(&arrow_formals_classifier, expression,
                                   CHECK_OK);
     Scanner::Location loc(lhs_location.beg_pos, scanner()->location().end_pos);
-    bool has_rest = false;
     Scope* scope =
         this->NewScope(scope_, ARROW_SCOPE, FunctionKind::kArrowFunction);
     scope->set_start_position(lhs_location.beg_pos);
     Scanner::Location duplicate_loc = Scanner::Location::invalid();
+    bool has_rest = false;
     this->ParseArrowFunctionFormalParameters(scope, expression, loc, &has_rest,
                                              &duplicate_loc, CHECK_OK);
     if (duplicate_loc.IsValid()) {
@@ -2698,6 +2733,7 @@ ParserBase<Traits>::ParseAssignmentExpression(bool accept_IN,
   // "expression" was not itself an arrow function parameter list, but it might
   // form part of one.  Propagate speculative formal parameter error locations.
   classifier->Accumulate(arrow_formals_classifier,
+                         ExpressionClassifier::StandardProductions |
                              ExpressionClassifier::FormalParametersProductions);
 
   if (!Token::IsAssignmentOp(peek())) {

test/message/arrow-bare-rest-param.js

@@ -0,0 +1,7 @@
+// Copyright 2015 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.
+//
+// Flags: --harmony-rest-parameters --harmony-arrow-functions
+
+...x => 10

test/message/arrow-bare-rest-param.out

@@ -0,0 +1,4 @@
+*%(basename)s:7: SyntaxError: Unexpected token ...
+...x => 10
+^^^
+SyntaxError: Unexpected token ...

test/message/arrow-param-after-rest-2.js

@@ -0,0 +1,7 @@
+// Copyright 2015 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.
+//
+// Flags: --harmony-rest-parameters --harmony-arrow-functions
+
+(w, ...x, y) => 10

test/message/arrow-param-after-rest-2.out

@@ -0,0 +1,4 @@
+*%(basename)s:7: SyntaxError: Rest parameter must be last formal parameter
+(w, ...x, y) => 10
+        ^
+SyntaxError: Rest parameter must be last formal parameter

test/message/arrow-param-after-rest.js

@@ -0,0 +1,7 @@
+// Copyright 2015 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.
+//
+// Flags: --harmony-rest-parameters --harmony-arrow-functions
+
+(...x, y) => 10

test/message/arrow-param-after-rest.out

@@ -0,0 +1,4 @@
+*%(basename)s:7: SyntaxError: Rest parameter must be last formal parameter
+(...x, y) => 10
+     ^
+SyntaxError: Rest parameter must be last formal parameter

test/message/arrow-two-rest-params.js

@@ -0,0 +1,7 @@
+// Copyright 2015 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.
+//
+// Flags: --harmony-rest-parameters --harmony-arrow-functions
+
+(w, ...x, ...y) => 10

test/message/arrow-two-rest-params.out

@@ -0,0 +1,4 @@
+*%(basename)s:7: SyntaxError: Rest parameter must be last formal parameter
+(w, ...x, ...y) => 10
+        ^
+SyntaxError: Rest parameter must be last formal parameter

test/message/invalid-spread-2.js

@@ -0,0 +1,7 @@
+// Copyright 2015 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.
+//
+// Flags: --harmony-rest-parameters --harmony-arrow-functions
+
+(x, ...y, z)

test/message/invalid-spread-2.out

@@ -0,0 +1,4 @@
+*%(basename)s:7: SyntaxError: Unexpected token ...
+(x, ...y, z)
+    ^^^
+SyntaxError: Unexpected token ...

test/message/invalid-spread.js

@@ -0,0 +1,7 @@
+// Copyright 2015 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.
+//
+// Flags: --harmony-rest-parameters --harmony-arrow-functions
+
+(x, ...y)

test/message/invalid-spread.out

@@ -0,0 +1,4 @@
+*%(basename)s:7: SyntaxError: Unexpected token ...
+(x, ...y)
+    ^^^
+SyntaxError: Unexpected token ...

test/mjsunit/harmony/arrow-rest-params.js

@@ -0,0 +1,142 @@
+// Copyright 2014 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.
+
+// Flags: --harmony-rest-parameters --harmony-arrow-functions
+
+(function testRestIndex() {
+  assertEquals(5, ((...args) => args.length)(1,2,3,4,5));
+  assertEquals(4, ((a, ...args) => args.length)(1,2,3,4,5));
+  assertEquals(3, ((a, b, ...args) => args.length)(1,2,3,4,5));
+  assertEquals(2, ((a, b, c, ...args) => args.length)(1,2,3,4,5));
+  assertEquals(1, ((a, b, c, d, ...args) => args.length)(1,2,3,4,5));
+  assertEquals(0, ((a, b, c, d, e, ...args) => args.length)(1,2,3,4,5));
+})();
+
+// strictTest and sloppyTest should be called with descending natural
+// numbers, as in:
+//
+//   strictTest(6,5,4,3,2,1)
+//
+var strictTest = (a, b, ...c) => {
+  "use strict";
+  assertEquals(Array, c.constructor);
+  assertTrue(Array.isArray(c));
+
+  var expectedLength = (a === undefined) ? 0 : a - 2;
+  assertEquals(expectedLength, c.length);
+
+  for (var i = 2; i < a; ++i) {
+    assertEquals(c[i - 2], a - i);
+  }
+}
+
+var sloppyTest = (a, b, ...c) => {
+  assertEquals(Array, c.constructor);
+  assertTrue(Array.isArray(c));
+
+  var expectedLength = (a === undefined) ? 0 : a - 2;
+  assertEquals(expectedLength, c.length);
+
+  for (var i = 2; i < a; ++i) {
+    assertEquals(c[i - 2], a - i);
+  }
+}
+
+
+var O = {
+  strict: strictTest,
+  sloppy: sloppyTest
+};
+
+(function testStrictRestParamArity() {
+  assertEquals(2, strictTest.length);
+  assertEquals(2, O.strict.length);
+})();
+
+
+(function testRestParamsStrictMode() {
+  strictTest();
+  strictTest(2, 1);
+  strictTest(6, 5, 4, 3, 2, 1);
+  strictTest(3, 2, 1);
+  O.strict();
+  O.strict(2, 1);
+  O.strict(6, 5, 4, 3, 2, 1);
+  O.strict(3, 2, 1);
+})();
+
+
+(function testRestParamsStrictModeApply() {
+  strictTest.apply(null, []);
+  strictTest.apply(null, [2, 1]);
+  strictTest.apply(null, [6, 5, 4, 3, 2, 1]);
+  strictTest.apply(null, [3, 2, 1]);
+  O.strict.apply(O, []);
+  O.strict.apply(O, [2, 1]);
+  O.strict.apply(O, [6, 5, 4, 3, 2, 1]);
+  O.strict.apply(O, [3, 2, 1]);
+})();
+
+
+(function testRestParamsStrictModeCall() {
+  strictTest.call(null);
+  strictTest.call(null, 2, 1);
+  strictTest.call(null, 6, 5, 4, 3, 2, 1);
+  strictTest.call(null, 3, 2, 1);
+  O.strict.call(O);
+  O.strict.call(O, 2, 1);
+  O.strict.call(O, 6, 5, 4, 3, 2, 1);
+  O.strict.call(O, 3, 2, 1);
+})();
+
+
+(function testsloppyRestParamArity() {
+  assertEquals(2, sloppyTest.length);
+  assertEquals(2, O.sloppy.length);
+})();
+
+
+(function testRestParamssloppyMode() {
+  sloppyTest();
+  sloppyTest(2, 1);
+  sloppyTest(6, 5, 4, 3, 2, 1);
+  sloppyTest(3, 2, 1);
+  O.sloppy();
+  O.sloppy(2, 1);
+  O.sloppy(6, 5, 4, 3, 2, 1);
+  O.sloppy(3, 2, 1);
+})();
+
+
+(function testRestParamssloppyModeApply() {
+  sloppyTest.apply(null, []);
+  sloppyTest.apply(null, [2, 1]);
+  sloppyTest.apply(null, [6, 5, 4, 3, 2, 1]);
+  sloppyTest.apply(null, [3, 2, 1]);
+  O.sloppy.apply(O, []);
+  O.sloppy.apply(O, [2, 1]);
+  O.sloppy.apply(O, [6, 5, 4, 3, 2, 1]);
+  O.sloppy.apply(O, [3, 2, 1]);
+})();
+
+
+(function testRestParamssloppyModeCall() {
+  sloppyTest.call(null);
+  sloppyTest.call(null, 2, 1);
+  sloppyTest.call(null, 6, 5, 4, 3, 2, 1);
+  sloppyTest.call(null, 3, 2, 1);
+  O.sloppy.call(O);
+  O.sloppy.call(O, 2, 1);
+  O.sloppy.call(O, 6, 5, 4, 3, 2, 1);
+  O.sloppy.call(O, 3, 2, 1);
+})();
+
+
+(function testUnmappedArguments() {
+  // Normal functions make their arguments object unmapped, but arrow
+  // functions don't have an arguments object anyway.  Check that the
+  // right thing happens for arguments in arrow functions with rest
+  // parameters.
+  assertSame(arguments, ((...rest) => arguments)());
+})();

test/mjsunit/harmony/regress/regress-arrow-duplicate-params.js

@@ -0,0 +1,7 @@
+// Copyright 2015 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.
+
+// Flags: --harmony-arrow-functions
+
+assertThrows("(x, x, y) => 10;", SyntaxError);

test/mjsunit/harmony/rest-params-lazy-parsing.js

@@ -2,7 +2,8 @@
 // Use of this source code is governed by a BSD-style license that can be
 // found in the LICENSE file.
 
-// Flags: --harmony-rest-parameters --min-preparse-length=0
+// Flags: --harmony-rest-parameters --harmony-arrow-functions
+// Flags: --min-preparse-length=0
 
 function variadic(co, ...values) {
   var sum = 0;
@@ -12,6 +13,21 @@ function variadic(co, ...values) {
   return sum;
 }
 
+var arrowVariadic = (co, ...values) => {
+  var sum = 0;
+  while (values.length) {
+    sum += co * values.pop();
+  }
+  return sum;
+}
+
+assertEquals(1, variadic.length);
+assertEquals(1, arrowVariadic.length);
+
 assertEquals(90, variadic(2, 1, 2, 3, 4, 5, 6, 7, 8, 9));
 assertEquals(74, variadic(2, 1, 2, 3, 4, 5, 6, 7, 9));
 assertEquals(110, variadic(2, 1, 2, 3, 4, 5, 6, 7, 8, 9, 10));
+
+assertEquals(90, arrowVariadic(2, 1, 2, 3, 4, 5, 6, 7, 8, 9));
+assertEquals(74, arrowVariadic(2, 1, 2, 3, 4, 5, 6, 7, 9));
+assertEquals(110, arrowVariadic(2, 1, 2, 3, 4, 5, 6, 7, 8, 9, 10));