Allow trailing comma in parameter and argument lists.

runtime/vm/parser.cc

 // Copyright (c) 2012, the Dart project authors.  Please see the AUTHORS file
 // for details. All rights reserved. Use of this source code is governed by a
 // BSD-style license that can be found in the LICENSE file.
 
 #include "vm/parser.h"
 #include "vm/flags.h"
 
 #ifndef DART_PRECOMPILED_RUNTIME
 
 #include "lib/invocation_mirror.h"
@@ skipping 2013 matching lines @@
       params->has_optional_positional_parameters = true;
       return;
     }
     if (!params->has_optional_positional_parameters &&
         !params->has_optional_named_parameters &&
         (CurrentToken() == Token::kLBRACE)) {
       // End of normal parameters, start of optional named parameters.
       params->has_optional_named_parameters = true;
       return;
     }
+    Token::Kind terminator =
+       params->has_optional_positional_parameters ? Token::kRBRACK :
+       params->has_optional_named_parameters ? Token::kRBRACE :
+       Token :: kRPAREN;
+    if (CurrentToken() == terminator) {

[hausner, 2016/06/22 17:50:25]

You are allowing an empty list of optional positional or named arguments with
this. In other words, you would allow a formal parameter list like this:

foo(a, [])

Which is probably not what you intended. On the other hand, I don't have a
authoritative spec.

[Lasse Reichstein Nielsen, 2016/06/24 11:43:04]

Good point. 
Curiously, it's only an empty named block that gets accepted, something must be
catching the empty positional one elsewhere.

+      // Allow a trailing comma.
+      break;
+    }
     ParseFormalParameter(allow_explicit_default_values,
                          evaluate_metadata,
                          params);
   } while (CurrentToken() == Token::kCOMMA);
 }
 
 
 void Parser::ParseFormalParameterList(bool allow_explicit_default_values,
                                       bool evaluate_metadata,
                                       ParamList* params) {
@@ skipping 9068 matching lines @@
   const bool saved_mode = SetAllowFunctionLiterals(true);
   ArgumentListNode* arguments;
   if (implicit_arguments == NULL) {
     arguments = new(Z) ArgumentListNode(TokenPos());
   } else {
     arguments = implicit_arguments;
   }
   const GrowableObjectArray& names =
       GrowableObjectArray::Handle(Z, GrowableObjectArray::New(Heap::kOld));
   bool named_argument_seen = false;
   if (LookaheadToken(1) != Token::kRPAREN) {

[hausner, 2016/06/22 17:50:25]

This special case for the empty argument list is no longer necessary with your
change. The new check you added will handle it correctly. 

[Lasse Reichstein Nielsen, 2016/06/24 11:43:04]

True. It won't when I add code to prevent the empty optional parameter list,
though.

     String& arg_name = String::Handle(Z);
     do {
       ASSERT((CurrentToken() == Token::kLPAREN) ||
              (CurrentToken() == Token::kCOMMA));
       ConsumeToken();
+      if (CurrentToken() == Token::kRPAREN) {
+        // Allow trailing comma.
+        break;
+      }
       if (IsIdentifier() && (LookaheadToken(1) == Token::kCOLON)) {
         named_argument_seen = true;
         // The canonicalization of the arguments descriptor array built in
         // the code generator requires that the names are symbols, i.e.
         // canonicalized strings.
         ASSERT(CurrentLiteral()->IsSymbol());
         for (int i = 0; i < names.Length(); i++) {
           arg_name ^= names.At(i);
           if (CurrentLiteral()->Equals(arg_name)) {
             ReportError("duplicate named argument");
@@ skipping 3362 matching lines @@
     const ArgumentListNode& function_args,
     const LocalVariable* temp_for_last_arg,
     bool is_super_invocation) {
   UNREACHABLE();
   return NULL;
 }
 
 }  // namespace dart
 
 #endif  // DART_PRECOMPILED_RUNTIME