Move ParseArguments to ParserBase and add tests.

Notes:
- PreParser didn't produce "too_many_arguments"; now it does.
- The argument count in the error message was wrong; fixed it.

BUG=v8:3126
LOG=N
R=ulan@chromium.org

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

git-svn-id: http://v8.googlecode.com/svn/branches/bleeding_edge@19812 ce2b1a6d-e550-0410-aec6-3dcde31c8c00

src/messages.js

@@ -155,8 +155,8 @@ var kMessages = {
   invalid_preparser_data:        ["Invalid preparser data for function ", "%0"],
   strict_mode_with:              ["Strict mode code may not include a with statement"],
   strict_eval_arguments:         ["Unexpected eval or arguments in strict mode"],
-  too_many_arguments:            ["Too many arguments in function call (only 32766 allowed)"],
-  too_many_parameters:           ["Too many parameters in function definition (only 32766 allowed)"],
+  too_many_arguments:            ["Too many arguments in function call (only 65535 allowed)"],
+  too_many_parameters:           ["Too many parameters in function definition (only 65535 allowed)"],
   too_many_variables:            ["Too many variables declared (only 131071 allowed)"],
   strict_param_dupe:             ["Strict mode function may not have duplicate parameter names"],
   strict_octal_literal:          ["Octal literals are not allowed in strict mode."],

src/parser.cc

@@ -3501,29 +3501,6 @@ Handle<FixedArray> CompileTimeValue::GetElements(Handle<FixedArray> value) {
 }
 
 
-ZoneList<Expression*>* Parser::ParseArguments(bool* ok) {
-  // Arguments ::
-  //   '(' (AssignmentExpression)*[','] ')'
-
-  ZoneList<Expression*>* result = new(zone()) ZoneList<Expression*>(4, zone());
-  Expect(Token::LPAREN, CHECK_OK);
-  bool done = (peek() == Token::RPAREN);
-  while (!done) {
-    Expression* argument = ParseAssignmentExpression(true, CHECK_OK);
-    result->Add(argument, zone());
-    if (result->length() > Code::kMaxArguments) {
-      ReportMessageAt(scanner()->location(), "too_many_arguments");
-      *ok = false;
-      return NULL;
-    }
-    done = (peek() == Token::RPAREN);
-    if (!done) Expect(Token::COMMA, CHECK_OK);
-  }
-  Expect(Token::RPAREN, CHECK_OK);
-  return result;
-}
-
-
 class SingletonLogger : public ParserRecorder {
  public:
   SingletonLogger() : has_error_(false), start_(-1), end_(-1) { }

src/parser.h

@@ -488,6 +488,9 @@ class ParserTraits {
   static Literal* EmptyLiteral() {
     return NULL;
   }
+  static ZoneList<Expression*>* NullExpressionList() {
+    return NULL;
+  }
 
   // Odd-ball literal creators.
   Literal* GetLiteralTheHole(int position,
@@ -687,7 +690,6 @@ class Parser : public ParserBase<ParserTraits> {
                                   Expression* subject,
                                   Statement* body);
 
-  ZoneList<Expression*>* ParseArguments(bool* ok);
   FunctionLiteral* ParseFunctionLiteral(
       Handle<String> name,
       Scanner::Location function_name_location,

src/preparser.cc

@@ -1135,28 +1135,6 @@ PreParser::Expression PreParser::ParseMemberExpressionContinuation(
 }
 
 
-PreParser::Arguments PreParser::ParseArguments(bool* ok) {
-  // Arguments ::
-  //   '(' (AssignmentExpression)*[','] ')'
-
-  Expect(Token::LPAREN, ok);
-  if (!*ok) return -1;
-  bool done = (peek() == Token::RPAREN);
-  int argc = 0;
-  while (!done) {
-    ParseAssignmentExpression(true, ok);
-    if (!*ok) return -1;
-    argc++;
-    done = (peek() == Token::RPAREN);
-    if (!done) {
-      Expect(Token::COMMA, ok);
-      if (!*ok) return -1;
-    }
-  }
-  Expect(Token::RPAREN, ok);
-  return argc;
-}
-
 PreParser::Expression PreParser::ParseFunctionLiteral(
     Identifier function_name,
     Scanner::Location function_name_location,

src/preparser.h

@@ -342,6 +342,7 @@ class ParserBase : public Traits {
   typename Traits::Type::Expression ParseExpression(bool accept_IN, bool* ok);
   typename Traits::Type::Expression ParseArrayLiteral(bool* ok);
   typename Traits::Type::Expression ParseObjectLiteral(bool* ok);
+  typename Traits::Type::ExpressionList ParseArguments(bool* ok);
 
   // Used to detect duplicates in object literals. Each of the values
   // kGetterProperty, kSetterProperty and kValueProperty represents
@@ -556,8 +557,12 @@ class PreParserExpression {
 class PreParserExpressionList {
  public:
   // These functions make list->Add(some_expression) work (and do nothing).
+  PreParserExpressionList() : length_(0) {}
   PreParserExpressionList* operator->() { return this; }
-  void Add(PreParserExpression, void*) { }
+  void Add(PreParserExpression, void*) { ++length_; }
+  int length() const { return length_; }
+ private:
+  int length_;
 };
 
 
@@ -720,6 +725,9 @@ class PreParserTraits {
   static PreParserExpression EmptyLiteral() {
     return PreParserExpression::Default();
   }
+  static PreParserExpressionList NullExpressionList() {
+    return PreParserExpressionList();
+  }
 
   // Odd-ball literal creators.
   static PreParserExpression GetLiteralTheHole(int position,
@@ -910,8 +918,6 @@ class PreParser : public ParserBase<PreParserTraits> {
     kUnknownSourceElements
   };
 
-  typedef int Arguments;
-
   // All ParseXXX functions take as the last argument an *ok parameter
   // which is set to false if parsing failed; it is unchanged otherwise.
   // By making the 'exception handling' explicit, we are forced to check
@@ -955,7 +961,6 @@ class PreParser : public ParserBase<PreParserTraits> {
   Expression ParseObjectLiteral(bool* ok);
   Expression ParseV8Intrinsic(bool* ok);
 
-  Arguments ParseArguments(bool* ok);
   Expression ParseFunctionLiteral(
       Identifier name,
       Scanner::Location function_name_location,
@@ -1153,6 +1158,13 @@ ParserBase<Traits>::ParseRegExpLiteral(bool seen_equal, bool* ok) {
 #define DUMMY )  // to make indentation work
 #undef DUMMY
 
+// Used in functions where the return type is not Traits::Type::Expression.
+#define CHECK_OK_CUSTOM(x) ok); \
+  if (!*ok) return this->x(); \
+  ((void)0
+#define DUMMY )  // to make indentation work
+#undef DUMMY
+
 template <class Traits>
 typename Traits::Type::Expression ParserBase<Traits>::ParsePrimaryExpression(
     bool* ok) {
@@ -1370,7 +1382,10 @@ typename Traits::Type::Expression ParserBase<Traits>::ParseObjectLiteral(
             number_of_boilerplate_properties++;
           }
           properties->Add(property, zone());
-          if (peek() != Token::RBRACE) Expect(Token::COMMA, CHECK_OK);
+          if (peek() != Token::RBRACE) {
+            // Need {} because of the CHECK_OK macro.
+            Expect(Token::COMMA, CHECK_OK);
+          }
 
           if (fni_ != NULL) {
             fni_->Infer();
@@ -1437,7 +1452,10 @@ typename Traits::Type::Expression ParserBase<Traits>::ParseObjectLiteral(
     properties->Add(property, zone());
 
     // TODO(1240767): Consider allowing trailing comma.
-    if (peek() != Token::RBRACE) Expect(Token::COMMA, CHECK_OK);
+    if (peek() != Token::RBRACE) {
+      // Need {} because of the CHECK_OK macro.
+      Expect(Token::COMMA, CHECK_OK);
+    }
 
     if (fni_ != NULL) {
       fni_->Infer();
@@ -1457,8 +1475,39 @@ typename Traits::Type::Expression ParserBase<Traits>::ParseObjectLiteral(
 }
 
 
+template <class Traits>
+typename Traits::Type::ExpressionList ParserBase<Traits>::ParseArguments(
+    bool* ok) {
+  // Arguments ::
+  //   '(' (AssignmentExpression)*[','] ')'
+
+  typename Traits::Type::ExpressionList result =
+      this->NewExpressionList(4, zone_);
+  Expect(Token::LPAREN, CHECK_OK_CUSTOM(NullExpressionList));
+  bool done = (peek() == Token::RPAREN);
+  while (!done) {
+    typename Traits::Type::Expression argument =
+        this->ParseAssignmentExpression(true,
+                                        CHECK_OK_CUSTOM(NullExpressionList));
+    result->Add(argument, zone_);
+    if (result->length() > Code::kMaxArguments) {
+      ReportMessageAt(scanner()->location(), "too_many_arguments");
+      *ok = false;
+      return this->NullExpressionList();
+    }
+    done = (peek() == Token::RPAREN);
+    if (!done) {
+      // Need {} because of the CHECK_OK_CUSTOM macro.
+      Expect(Token::COMMA, CHECK_OK_CUSTOM(NullExpressionList));
+    }
+  }
+  Expect(Token::RPAREN, CHECK_OK_CUSTOM(NullExpressionList));
+  return result;
+}
+
 
 #undef CHECK_OK
+#undef CHECK_OK_CUSTOM
 
 
 template <typename Traits>

test/cctest/test-parsing.cc

@@ -35,6 +35,7 @@
 #include "compiler.h"
 #include "execution.h"
 #include "isolate.h"
+#include "objects.h"
 #include "parser.h"
 #include "preparser.h"
 #include "scanner-character-streams.h"
@@ -1463,7 +1464,9 @@ TEST(PreparserStrictOctal) {
 
 void RunParserSyncTest(const char* context_data[][2],
                        const char* statement_data[],
-                       ParserSyncTestResult result) {
+                       ParserSyncTestResult result,
+                       const ParserFlag* flags = NULL,
+                       int flags_len = 0) {
   v8::HandleScope handles(CcTest::isolate());
   v8::Handle<v8::Context> context = v8::Context::New(CcTest::isolate());
   v8::Context::Scope context_scope(context);
@@ -1472,10 +1475,14 @@ void RunParserSyncTest(const char* context_data[][2],
   CcTest::i_isolate()->stack_guard()->SetStackLimit(
       reinterpret_cast<uintptr_t>(&marker) - 128 * 1024);
 
-  static const ParserFlag flags[] = {
+  static const ParserFlag default_flags[] = {
     kAllowLazy, kAllowHarmonyScoping, kAllowModules, kAllowGenerators,
     kAllowForOf, kAllowNativesSyntax
   };
+  if (!flags) {
+    flags = default_flags;
+    flags_len = ARRAY_SIZE(default_flags);
+  }
   for (int i = 0; context_data[i][0] != NULL; ++i) {
     for (int j = 0; statement_data[j] != NULL; ++j) {
       int kPrefixLen = i::StrLength(context_data[i][0]);
@@ -1493,7 +1500,7 @@ void RunParserSyncTest(const char* context_data[][2],
       CHECK(length == kProgramSize);
       TestParserSync(program.start(),
                      flags,
-                     ARRAY_SIZE(flags),
+                     flags_len,
                      result);
     }
   }
@@ -2320,3 +2327,27 @@ TEST(NoErrorsObjectLiteralChecking) {
 
   RunParserSyncTest(context_data, statement_data, kSuccess);
 }
+
+
+TEST(TooManyArguments) {
+  const char* context_data[][2] = {
+    {"foo(", "0)"},
+    { NULL, NULL }
+  };
+
+  using v8::internal::Code;
+  char statement[Code::kMaxArguments * 2];
+  for (int i = 0; i < Code::kMaxArguments; ++i) {
+    statement[2 * i] = '0';
+    statement[2 * i + 1] = ',';
+  }
+
+  const char* statement_data[] = {
+    statement,
+    NULL
+  };
+
+  // The test is quite slow, so run it with a reduced set of flags.
+  static const ParserFlag empty_flags[] = {kAllowLazy};
+  RunParserSyncTest(context_data, statement_data, kError, empty_flags, 1);
+}