Allow trailing commas in function parameter lists

src/parsing/parser-base.h

Index: src/parsing/parser-base.h
diff --git a/src/parsing/parser-base.h b/src/parsing/parser-base.h
index dc08f5c5803923dd9ee29bf92228af10d5f0efe3..6938f32fca0df01679d70dcb24f426a4eedf3caa 100644
--- a/src/parsing/parser-base.h
+++ b/src/parsing/parser-base.h
@@ -199,7 +199,8 @@ class ParserBase : public Traits {
         allow_harmony_function_name_(false),
         allow_harmony_function_sent_(false),
         allow_harmony_async_await_(false),
-        allow_harmony_restrictive_generators_(false) {}
+        allow_harmony_restrictive_generators_(false),
+        allow_harmony_trailing_commas_in_parameters_(false) {}
 
 #define ALLOW_ACCESSORS(name)                           \
   bool allow_##name() const { return allow_##name##_; } \
@@ -221,6 +222,7 @@ class ParserBase : public Traits {
   ALLOW_ACCESSORS(harmony_function_sent);
   ALLOW_ACCESSORS(harmony_async_await);
   ALLOW_ACCESSORS(harmony_restrictive_generators);
+  ALLOW_ACCESSORS(harmony_trailing_commas_in_parameters);
   SCANNER_ACCESSORS(harmony_exponentiation_operator);
 
 #undef SCANNER_ACCESSORS
@@ -1192,6 +1194,7 @@ class ParserBase : public Traits {
   bool allow_harmony_function_sent_;
   bool allow_harmony_async_await_;
   bool allow_harmony_restrictive_generators_;
+  bool allow_harmony_trailing_commas_in_parameters_;
 };
 
 template <class Traits>
@@ -1700,7 +1703,11 @@ typename ParserBase<Traits>::ExpressionT ParserBase<Traits>::ParseExpression(
     }
     Consume(Token::COMMA);
     bool is_rest = false;
-    if (peek() == Token::ELLIPSIS) {
+    if (allow_harmony_trailing_commas_in_parameters() &&
+        peek() == Token::RPAREN && PeekAhead() == Token::ARROW) {
+      // a trailing comma is allowed at the end of an arrow parameter list

[caitp (gmail), 2016/06/22 18:56:20]

Is this approach using PeekAhead() faster than just using the peek() and
ExpressionClassifier? We should try both and measure

[adamk, 2016/06/28 00:39:47]

I have the same question :)

I'll be sad if this slows down the parser at all, actually.

+      break;
+    } else if (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.
@@ -2191,6 +2198,11 @@ typename Traits::Type::ExpressionList ParserBase<Traits>::ParseArguments(
     done = (peek() != Token::COMMA);
     if (!done) {
       Next();
+      if (allow_harmony_trailing_commas_in_parameters() &&
+          peek() == Token::RPAREN) {
+        // allow trailing comma
+        done = true;
+      }
     }
   }
   Scanner::Location location = scanner_->location();
@@ -3212,7 +3224,7 @@ void ParserBase<Traits>::ParseFormalParameterList(
   DCHECK_EQ(0, parameters->Arity());
 
   if (peek() != Token::RPAREN) {
-    do {
+    while (true) {
       if (parameters->Arity() > Code::kMaxArguments) {
         ReportMessage(MessageTemplate::kTooManyParameters);
         *ok = false;
@@ -3221,16 +3233,23 @@ void ParserBase<Traits>::ParseFormalParameterList(
       parameters->has_rest = Check(Token::ELLIPSIS);
       ParseFormalParameter(parameters, classifier, ok);
       if (!*ok) return;
-    } while (!parameters->has_rest && Check(Token::COMMA));
 
-    if (parameters->has_rest) {
-      parameters->is_simple = false;
-      classifier->RecordNonSimpleParameter();
-      if (peek() == Token::COMMA) {
-        ReportMessageAt(scanner()->peek_location(),
-                      MessageTemplate::kParamAfterRest);
-        *ok = false;
-        return;
+      if (parameters->has_rest) {
+        parameters->is_simple = false;
+        classifier->RecordNonSimpleParameter();
+        if (peek() == Token::COMMA) {
+          ReportMessageAt(scanner()->peek_location(),
+                          MessageTemplate::kParamAfterRest);
+          *ok = false;
+          return;
+        }
+        break;
+      }
+      if (!Check(Token::COMMA)) break;
+      if (allow_harmony_trailing_commas_in_parameters() &&

[caitp (gmail), 2016/06/22 18:56:20]

For a lot of these, I feel like a WebKit-ism of defining helper macros which
more clearly explain what this is doing would be good, like
`breakIfTrailingComma()` or something.

I don't think it really fits into the chromium code style though, so I am fine
with it as-is.

+          peek() == Token::RPAREN) {
+        // allow the trailing comma
+        break;
       }
     }
   }