[es6] support rest parameters in arrow functions

src/preparser.h

Index: src/preparser.h
diff --git a/src/preparser.h b/src/preparser.h
index 7df271f6baad4152aa359f3377e6d5609b97bbe6..0fceadb2198c16303931eedbeb52e998e786adce 100644
--- a/src/preparser.h
+++ b/src/preparser.h
@@ -338,6 +338,11 @@ class ParserBase : public Traits {
     return scanner()->peek();
   }
 
+  INLINE(Token::Value peek(int n)) {
+    if (stack_overflow_) return Token::ILLEGAL;
+    return scanner()->peek(n);
+  }
+
   INLINE(Token::Value Next()) {
     if (stack_overflow_) return Token::ILLEGAL;
     {
@@ -351,6 +356,8 @@ class ParserBase : public Traits {
     return scanner()->Next();
   }
 
+  void PeekScan(int until);
+
   void Consume(Token::Value token) {
     Token::Value next = Next();
     USE(next);
@@ -539,6 +546,7 @@ class ParserBase : public Traits {
   }
 
   void ReportUnexpectedToken(Token::Value token);
+  void ReportUnexpectedTokenAt(Scanner::Location location, Token::Value token);
 
   // Recursive descent functions:
 
@@ -783,6 +791,11 @@ class PreParserExpression {
                                IdentifierTypeField::encode(id.type_));
   }
 
+  static PreParserExpression SpreadOperation(PreParserExpression expr) {
+    return PreParserExpression(TypeField::encode(
+        kSpreadExpression) | expr.code_);
+  }
+
   static PreParserExpression BinaryOperation(PreParserExpression left,
                                              Token::Value op,
                                              PreParserExpression right) {
@@ -897,6 +910,15 @@ class PreParserExpression {
                kMultiParenthesizedExpression;
   }
 
+  bool IsSpreadOperation() const {
+    return TypeField::decode(code_) & kSpreadExpression;

[marja, 2015/03/10 09:11:19]

Hmm... so this works because kSpreadExpression happens to be 4 (which is a
single bit), right? But it's fragile and wouldn't work if somebody reorders the
Type enum...

+  }
+
+  PreParserExpression SpreadExpression() const {
+    DCHECK(IsSpreadOperation());
+    return PreParserExpression(TypeField::decode(code_) & ~kSpreadExpression);
+  }
+
   // At the moment PreParser doesn't track these expression types.
   bool IsFunctionLiteral() const { return false; }
   bool IsCallNew() const { return false; }
@@ -938,7 +960,8 @@ class PreParserExpression {
     kExpression,
     kIdentifierExpression,
     kStringLiteralExpression,
-    kBinaryOperationExpression
+    kBinaryOperationExpression,
+    kSpreadExpression

[marja, 2015/03/10 09:11:19]

So maybe add "= 1 << 3 " here to hint that kSpreadExpression needs to be a
single-bit thing.

   };
 
   enum Parenthesization {
@@ -959,13 +982,16 @@ class PreParserExpression {
       : code_(expression_code) {}
 
   V8_INLINE bool IsValidArrowParams() const {
+    if (IsSpreadOperation()) {
+      return SpreadExpression().IsValidArrowParams();
+    }
     return IsBinaryOperation()
                ? IsValidArrowParamListField::decode(code_)
                : (IsIdentifier() && AsIdentifier().IsValidArrowParam());
   }
 
   // The first four bits are for the Type and Parenthesization.
-  typedef BitField<Type, 0, 2> TypeField;
+  typedef BitField<Type, 0, 3> TypeField;
   typedef BitField<Parenthesization, TypeField::kNext, 2> ParenthesizationField;
 
   // The rest of the bits are interpreted depending on the value
@@ -1181,6 +1207,9 @@ class PreParserFactory {
       int position) {
     return PreParserExpression::Default();
   }
+  PreParserExpression NewSpreadOperation(PreParserExpression expr, int pos) {
+    return PreParserExpression::SpreadOperation(expr);
+  }
 
   // Return the object itself as AstVisitor and implement the needed
   // dummy method right in this class.
@@ -1682,37 +1711,40 @@ ParserBase<Traits>::FunctionState::~FunctionState() {
 
 template<class Traits>
 void ParserBase<Traits>::ReportUnexpectedToken(Token::Value token) {
-  Scanner::Location source_location = scanner()->location();
+  return ReportUnexpectedTokenAt(scanner()->location(), token);
+}
+
 
+template <class Traits>
+void ParserBase<Traits>::ReportUnexpectedTokenAt(Scanner::Location location,
+                                                 Token::Value token) {
   // Four of the tokens are treated specially
   switch (token) {
     case Token::EOS:
-      return ReportMessageAt(source_location, "unexpected_eos");
+      return ReportMessageAt(location, "unexpected_eos");
     case Token::SMI:
     case Token::NUMBER:
-      return ReportMessageAt(source_location, "unexpected_token_number");
+      return ReportMessageAt(location, "unexpected_token_number");
     case Token::STRING:
-      return ReportMessageAt(source_location, "unexpected_token_string");
+      return ReportMessageAt(location, "unexpected_token_string");
     case Token::IDENTIFIER:
-      return ReportMessageAt(source_location, "unexpected_token_identifier");
+      return ReportMessageAt(location, "unexpected_token_identifier");
     case Token::FUTURE_RESERVED_WORD:
-      return ReportMessageAt(source_location, "unexpected_reserved");
+      return ReportMessageAt(location, "unexpected_reserved");
     case Token::LET:
     case Token::STATIC:
     case Token::YIELD:
     case Token::FUTURE_STRICT_RESERVED_WORD:
-      return ReportMessageAt(source_location,
-                             is_strict(language_mode())
-                                 ? "unexpected_strict_reserved"
-                                 : "unexpected_token_identifier");
+      return ReportMessageAt(location, is_strict(language_mode())
+                                           ? "unexpected_strict_reserved"
+                                           : "unexpected_token_identifier");
     case Token::TEMPLATE_SPAN:
     case Token::TEMPLATE_TAIL:
-      return Traits::ReportMessageAt(source_location,
-          "unexpected_template_string");
+      return Traits::ReportMessageAt(location, "unexpected_template_string");
     default:
       const char* name = Token::String(token);
       DCHECK(name != NULL);
-      Traits::ReportMessageAt(source_location, "unexpected_token", name);
+      Traits::ReportMessageAt(location, "unexpected_token", name);
   }
 }
 
@@ -1859,6 +1891,7 @@ ParserBase<Traits>::ParsePrimaryExpression(bool* ok) {
   //   ClassLiteral
   //   '(' Expression ')'
   //   TemplateLiteral
+  //   ... Identifier ')' '=>'

[arv (Not doing code reviews), 2015/03/10 13:44:02]

Why ')' '=>'?

[caitp (gmail), 2015/03/10 14:47:51]

So, you're right that we can just check for the `)`, but this means there are
potentially a lot more places where we have to look for problems. I liked doing
it this way because it meant there was only one place where a syntax error could
occur

 
   int beg_pos = scanner()->peek_location().beg_pos;
   int end_pos = scanner()->peek_location().end_pos;
@@ -1931,6 +1964,10 @@ ParserBase<Traits>::ParsePrimaryExpression(bool* ok) {
         result = this->ParseExpression(true, CHECK_OK);
         result->increase_parenthesization_level();
         Expect(Token::RPAREN, CHECK_OK);
+        if (peek() == Token::ARROW) {
+          int x = 5;

[marja, 2015/03/10 09:11:19]

?

[arv (Not doing code reviews), 2015/03/10 13:44:02]

Looks like debug code... remove?

[caitp (gmail), 2015/03/10 14:47:51]

Hah, yeah --- I'll get rid of that =)

+          USE(x);
+        }
       }
       break;
 
@@ -1970,6 +2007,39 @@ ParserBase<Traits>::ParsePrimaryExpression(bool* ok) {
       // If we're not allowing special syntax we fall-through to the
       // default case.
 
+    case Token::ELLIPSIS:
+      // Not a valid primary expression, but valid in parenthesized arrow
+      // function.
+      //
+      // Fail if name, closing parenthesis and arrow token are not found.
+      if (allow_harmony_rest_params() && allow_harmony_arrow_functions()) {
+        Consume(Token::ELLIPSIS);
+        bool is_strict_reserved_name = false;
+        IdentifierT name = ParseIdentifierOrStrictReservedWord(
+            &is_strict_reserved_name, CHECK_OK);

[arv (Not doing code reviews), 2015/03/10 13:44:03]

I don't see where is_strict_reserved_name is being checked?
ArrowFormalParameters are always strict.

[caitp (gmail), 2015/03/10 14:47:51]

to match the function signature... pretty much just trying to match what's
happening in CheckAndDeclareArrowFunctionParameters or whatever it's called.

+        if (peek() == Token::COMMA) {
+          *ok = false;
+          ReportMessageAt(scanner_->peek_location(), "param_after_rest");
+          break;
+        } else if (peek() != Token::RPAREN) {
+          ReportUnexpectedTokenAt(scanner_->peek_location(), peek());
+          *ok = false;
+          break;
+        }
+
+        if (peek(1) != Token::ARROW) {
+          ReportUnexpectedTokenAt(scanner_->peek_location(1), peek(1));

[arv (Not doing code reviews), 2015/03/10 13:44:03]

Another option here is to report the invalid `...`

+          *ok = false;
+          break;
+        }
+
+        Scanner::Location name_pos = scanner_->location();
+        result = this->ExpressionFromIdentifier(
+            name, name_pos.beg_pos, name_pos.end_pos, scope_, factory());
+
+        return factory()->NewSpreadOperation(result, beg_pos);
+      }
+
     default: {
       Next();
       ReportUnexpectedToken(token);
@@ -2765,6 +2835,12 @@ ParserBase<Traits>::ParseMemberExpression(bool* ok) {
     result = ParseSuperExpression(is_new, CHECK_OK);
   } else {
     result = ParsePrimaryExpression(CHECK_OK);
+    if (result->IsSpreadOperation()) {
+      // SpreadOperation is only parsed here as the last parameter of an
+      // arrow function. It's not a valid MemberExpression, and there is no
+      // continuation.
+      return result;
+    }
   }
 
   result = ParseMemberExpressionContinuation(result, CHECK_OK);