Revert "Factor formal argument parsing into ParserBase"

Revert https://codereview.chromium.org/1078093002/ and follow-on parser
patches due to a perf regression.

This reverts commit 53ddccfc.
This reverts commit 71d3213a.
This reverts commit 0f432ebb.
This reverts commit 1dbc4327.

R=marja@chromium.org

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

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

src/messages.js

@@ -195,11 +195,7 @@ var kMessages = {
   array_not_subclassable:        ["Subclassing Arrays is not currently supported."],
   for_in_loop_initializer:       ["for-in loop variable declaration may not have an initializer."],
   for_of_loop_initializer:       ["for-of loop variable declaration may not have an initializer."],
-  for_inof_loop_multi_bindings:  ["Invalid left-hand side in ", "%0", " loop: Must have a single binding."],
-  bad_getter_arity:              ["Getter must not have any formal parameters."],
-  bad_setter_arity:              ["Setter must have exactly one formal parameter."],
-  this_formal_parameter:         ["'this' is not a valid formal parameter name"],
-  duplicate_arrow_function_formal_parameter: ["Arrow function may not have duplicate parameter names"]
+  for_inof_loop_multi_bindings:  ["Invalid left-hand side in ", "%0", " loop: Must have a single binding."]
 };
 
 

src/parser.h

@@ -557,8 +557,6 @@ class ParserTraits {
     typedef ObjectLiteral::Property* ObjectLiteralProperty;
     typedef ZoneList<v8::internal::Expression*>* ExpressionList;
     typedef ZoneList<ObjectLiteral::Property*>* PropertyList;
-    typedef const v8::internal::AstRawString* FormalParameter;
-    typedef ZoneList<const v8::internal::AstRawString*>* FormalParameterList;
     typedef ZoneList<v8::internal::Statement*>* StatementList;
 
     // For constructing objects returned by the traversing functions.
@@ -696,6 +694,7 @@ class ParserTraits {
   static Expression* EmptyExpression() {
     return NULL;
   }
+  static Expression* EmptyArrowParamList() { return NULL; }
   static Literal* EmptyLiteral() {
     return NULL;
   }
@@ -706,10 +705,6 @@ class ParserTraits {
   static ZoneList<Expression*>* NullExpressionList() {
     return NULL;
   }
-  static const AstRawString* EmptyFormalParameter() { return NULL; }
-  static ZoneList<const AstRawString*>* NullFormalParameterList() {
-    return NULL;
-  }
 
   // Non-NULL empty string.
   V8_INLINE const AstRawString* EmptyIdentifierString();
@@ -745,20 +740,14 @@ class ParserTraits {
   ZoneList<v8::internal::Statement*>* NewStatementList(int size, Zone* zone) {
     return new(zone) ZoneList<v8::internal::Statement*>(size, zone);
   }
-  ZoneList<const v8::internal::AstRawString*>* NewFormalParameterList(
-      int size, Zone* zone) {
-    return new (zone) ZoneList<const v8::internal::AstRawString*>(size, zone);
-  }
   V8_INLINE Scope* NewScope(Scope* parent_scope, ScopeType scope_type,
                             FunctionKind kind = kNormalFunction);
 
-  void RecordArrowFunctionParameter(ZoneList<const AstRawString*>* params,
-                                    VariableProxy* proxy,
-                                    FormalParameterErrorLocations* error_locs,
-                                    bool* ok);
-  ZoneList<const AstRawString*>* ParseArrowFunctionFormalParameterList(
-      Expression* params, const Scanner::Location& params_loc,
-      FormalParameterErrorLocations* error_locs, bool* is_rest, bool* ok);
+  // Utility functions
+  int DeclareArrowParametersFromExpression(Expression* expression, Scope* scope,
+                                           Scanner::Location* undefined_loc,
+                                           Scanner::Location* dupe_loc,
+                                           bool* ok);
 
   // Temporary glue; these functions will move to ParserBase.
   Expression* ParseV8Intrinsic(bool* ok);
@@ -779,9 +768,6 @@ class ParserTraits {
                                   bool name_is_strict_reserved, int pos,
                                   bool* ok);
 
-  int DeclareFormalParameters(ZoneList<const AstRawString*>* params,
-                              Scope* scope, bool has_rest);
-
   V8_INLINE void CheckConflictingVarDeclarations(v8::internal::Scope* scope,
                                                  bool* ok);
 
@@ -1059,6 +1045,8 @@ class Parser : public ParserBase<ParserTraits> {
   ScriptCompiler::CompileOptions compile_options_;
   ParseData* cached_parse_data_;
 
+  bool parsing_lazy_arrow_parameters_;  // for lazily parsed arrow functions.
+
   PendingCompilationErrorHandler pending_error_handler_;
 
   // Other information which will be stored in Parser and moved to Isolate after

src/preparser.cc

@@ -926,19 +926,62 @@ PreParser::Expression PreParser::ParseFunctionLiteral(
   PreParserFactory factory(NULL);
   FunctionState function_state(&function_state_, &scope_, function_scope, kind,
                                &factory);
-  FormalParameterErrorLocations error_locs;
+  //  FormalParameterList ::
+  //    '(' (Identifier)*[','] ')'
+  Expect(Token::LPAREN, CHECK_OK);
+  int start_position = position();
+  DuplicateFinder duplicate_finder(scanner()->unicode_cache());
+  // We don't yet know if the function will be strict, so we cannot yet produce
+  // errors for parameter names or duplicates. However, we remember the
+  // locations of these errors if they occur and produce the errors later.
+  Scanner::Location eval_args_loc = Scanner::Location::invalid();
+  Scanner::Location dupe_loc = Scanner::Location::invalid();
+  Scanner::Location reserved_loc = Scanner::Location::invalid();
+
+  // Similarly for strong mode.
+  Scanner::Location undefined_loc = Scanner::Location::invalid();
 
   bool is_rest = false;
-  Expect(Token::LPAREN, CHECK_OK);
-  int start_position = scanner()->location().beg_pos;
-  PreParserFormalParameterList params =
-      ParseFormalParameterList(&error_locs, &is_rest, CHECK_OK);
-  Expect(Token::RPAREN, CHECK_OK);
-  int formals_end_position = scanner()->location().end_pos;
+  bool done = arity_restriction == FunctionLiteral::GETTER_ARITY ||
+      (peek() == Token::RPAREN &&
+       arity_restriction != FunctionLiteral::SETTER_ARITY);
+  while (!done) {
+    bool is_strict_reserved = false;
+    is_rest = peek() == Token::ELLIPSIS && allow_harmony_rest_params();
+    if (is_rest) {
+      Consume(Token::ELLIPSIS);
+    }
+
+    Identifier param_name =
+        ParseIdentifierOrStrictReservedWord(&is_strict_reserved, CHECK_OK);
+    if (!eval_args_loc.IsValid() && param_name.IsEvalOrArguments()) {
+      eval_args_loc = scanner()->location();
+    }
+    if (!undefined_loc.IsValid() && param_name.IsUndefined()) {
+      undefined_loc = scanner()->location();
+    }
+    if (!reserved_loc.IsValid() && is_strict_reserved) {
+      reserved_loc = scanner()->location();
+    }
 
-  CheckArityRestrictions(params->length(), arity_restriction, start_position,
-                         formals_end_position, ok);
-  if (!*ok) return Expression::Default();
+    int prev_value = scanner()->FindSymbol(&duplicate_finder, 1);
+
+    if (!dupe_loc.IsValid() && prev_value != 0) {
+      dupe_loc = scanner()->location();
+    }
+
+    if (arity_restriction == FunctionLiteral::SETTER_ARITY) break;
+    done = (peek() == Token::RPAREN);
+    if (!done) {
+      if (is_rest) {
+        ReportMessageAt(scanner()->peek_location(), "param_after_rest");
+        *ok = false;
+        return Expression::Default();
+      }
+      Expect(Token::COMMA, CHECK_OK);
+    }
+  }
+  Expect(Token::RPAREN, CHECK_OK);
 
   // See Parser::ParseFunctionLiteral for more information about lazy parsing
   // and lazy compilation.
@@ -959,8 +1002,8 @@ PreParser::Expression PreParser::ParseFunctionLiteral(
   CheckFunctionName(language_mode(), kind, function_name,
                     name_is_strict_reserved, function_name_location, CHECK_OK);
   const bool use_strict_params = is_rest || IsConciseMethod(kind);
-  CheckFunctionParameterNames(language_mode(), use_strict_params, error_locs,
-                              CHECK_OK);
+  CheckFunctionParameterNames(language_mode(), use_strict_params, eval_args_loc,
+                              undefined_loc, dupe_loc, reserved_loc, CHECK_OK);
 
   if (is_strict(language_mode())) {
     int end_position = scanner()->location().end_pos;

test/cctest/test-parsing.cc

@@ -3502,23 +3502,20 @@ TEST(ErrorsArrowFunctions) {
     "(x, (y, z)) => 0",
     "((x, y), z) => 0",
 
-    // Arrow function formal parameters are parsed as StrictFormalParameters,
-    // which confusingly only implies that there are no duplicates.  Words
-    // reserved in strict mode, and eval or arguments, are indeed valid in
-    // sloppy mode.
-    "eval => { 'use strict'; 0 }",
-    "arguments => { 'use strict'; 0 }",
-    "yield => { 'use strict'; 0 }",
-    "interface => { 'use strict'; 0 }",
-    "(eval) => { 'use strict'; 0 }",
-    "(arguments) => { 'use strict'; 0 }",
-    "(yield) => { 'use strict'; 0 }",
-    "(interface) => { 'use strict'; 0 }",
-    "(eval, bar) => { 'use strict'; 0 }",
-    "(bar, eval) => { 'use strict'; 0 }",
-    "(bar, arguments) => { 'use strict'; 0 }",
-    "(bar, yield) => { 'use strict'; 0 }",
-    "(bar, interface) => { 'use strict'; 0 }",
+    // Parameter lists are always validated as strict, so those are errors.
+    "eval => {}",
+    "arguments => {}",
+    "yield => {}",
+    "interface => {}",
+    "(eval) => {}",
+    "(arguments) => {}",
+    "(yield) => {}",
+    "(interface) => {}",
+    "(eval, bar) => {}",
+    "(bar, eval) => {}",
+    "(bar, arguments) => {}",
+    "(bar, yield) => {}",
+    "(bar, interface) => {}",
     // TODO(aperez): Detecting duplicates does not work in PreParser.
     // "(bar, bar) => {}",
 
@@ -3625,66 +3622,6 @@ TEST(NoErrorsArrowFunctions) {
 }
 
 
-TEST(ArrowFunctionsSloppyParameterNames) {
-  const char* strong_context_data[][2] = {
-    {"'use strong'; ", ";"},
-    {"'use strong'; bar ? (", ") : baz;"},
-    {"'use strong'; bar ? baz : (", ");"},
-    {"'use strong'; bar, ", ";"},
-    {"'use strong'; ", ", bar;"},
-    {NULL, NULL}
-  };
-
-  const char* strict_context_data[][2] = {
-    {"'use strict'; ", ";"},
-    {"'use strict'; bar ? (", ") : baz;"},
-    {"'use strict'; bar ? baz : (", ");"},
-    {"'use strict'; bar, ", ";"},
-    {"'use strict'; ", ", bar;"},
-    {NULL, NULL}
-  };
-
-  const char* sloppy_context_data[][2] = {
-    {"", ";"},
-    {"bar ? (", ") : baz;"},
-    {"bar ? baz : (", ");"},
-    {"bar, ", ";"},
-    {"", ", bar;"},
-    {NULL, NULL}
-  };
-
-  const char* statement_data[] = {
-    "eval => {}",
-    "arguments => {}",
-    "yield => {}",
-    "interface => {}",
-    "(eval) => {}",
-    "(arguments) => {}",
-    "(yield) => {}",
-    "(interface) => {}",
-    "(eval, bar) => {}",
-    "(bar, eval) => {}",
-    "(bar, arguments) => {}",
-    "(bar, yield) => {}",
-    "(bar, interface) => {}",
-    "(interface, eval) => {}",
-    "(interface, arguments) => {}",
-    "(eval, interface) => {}",
-    "(arguments, interface) => {}",
-    NULL
-  };
-
-  static const ParserFlag always_flags[] = { kAllowHarmonyArrowFunctions,
-                                             kAllowStrongMode};
-  RunParserSyncTest(strong_context_data, statement_data, kError, NULL, 0,
-                    always_flags, arraysize(always_flags));
-  RunParserSyncTest(strict_context_data, statement_data, kError, NULL, 0,
-                    always_flags, arraysize(always_flags));
-  RunParserSyncTest(sloppy_context_data, statement_data, kSuccess, NULL, 0,
-                    always_flags, arraysize(always_flags));
-}
-
-
 TEST(SuperNoErrors) {
   // Tests that parser and preparser accept 'super' keyword in right places.
   const char* context_data[][2] = {

test/message/formal-parameters-bad-rest.js

-// 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
-
-function foo(...b, a) { return a }

test/message/formal-parameters-bad-rest.out

-*%(basename)s:7: SyntaxError: Rest parameter must be last formal parameter
-function foo(...b, a) { return a }
-                 ^
-SyntaxError: Rest parameter must be last formal parameter

test/message/formal-parameters-strict-body.js

-// 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.
-
-function foo(b, eval) { "use strict"; return b }

test/message/formal-parameters-strict-body.out

-*%(basename)s:5: SyntaxError: Unexpected eval or arguments in strict mode
-function foo(b, eval) { "use strict"; return b }
-                ^^^^
-SyntaxError: Unexpected eval or arguments in strict mode

test/message/formal-parameters-trailing-comma.js

-// 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.
-
-function foo(b, a, a,) { return a }

test/message/formal-parameters-trailing-comma.out

-*%(basename)s:5: SyntaxError: Unexpected token )
-function foo(b, a, a,) { return a }
-                     ^
-SyntaxError: Unexpected token )

test/message/strict-formal-parameters.js

-// 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.
-
-"use strict";
-function foo(b, a, a, d) { return a }

test/message/strict-formal-parameters.out

-*%(basename)s:6: SyntaxError: Strict mode function may not have duplicate parameter names
-function foo(b, a, a, d) { return a }
-                   ^
-SyntaxError: Strict mode function may not have duplicate parameter names