[parser|cleanup] Remove unnecessary ExpressionClassifying.

ExpressionClassifier was used just for transmitting information back and forth
to DeclareFormalParameters.

As a bonus, we now do the Scope::IsDeclaredParameter check only when we're going
to use the information it produces.

BUG=v8:6092,v8:6474

Change-Id: Ib5ac6a779705caa74e933e1c6f03eaaf0f49bf05
Reviewed-on: https://chromium-review.googlesource.com/455836
Commit-Queue: Marja Hölttä <marja@chromium.org>
Reviewed-by: Toon Verwaest <verwaest@chromium.org>
Cr-Commit-Position: refs/heads/master@{#45809}

src/ast/scopes.cc

@@ -1023,7 +1023,9 @@ Variable* DeclarationScope::DeclareParameter(
     DCHECK_EQ(mode, VAR);
     var = Declare(zone(), name, mode);
     // TODO(wingo): Avoid O(n^2) check.
-    *is_duplicate = IsDeclaredParameter(name);
+    if (is_duplicate != nullptr) {
+      *is_duplicate = *is_duplicate || IsDeclaredParameter(name);
+    }
   }
   has_rest_ = is_rest;
   var->set_initializer_position(position);

src/parsing/parser-base.h

@@ -3763,7 +3763,8 @@ void ParserBase<Impl>::ParseFormalParameterList(FormalParametersT* parameters,
     }
   }
 
-  impl()->DeclareFormalParameters(parameters->scope, parameters->params);
+  impl()->DeclareFormalParameters(parameters->scope, parameters->params,
+                                  parameters->is_simple);
 }
 
 template <typename Impl>

src/parsing/parser.cc

@@ -682,7 +682,7 @@ FunctionLiteral* Parser::DoParseProgram(ParseInfo* info) {
     if (parsing_module_) {
       // Declare the special module parameter.
       auto name = ast_value_factory()->empty_string();
-      bool is_duplicate;
+      bool is_duplicate = false;
       bool is_rest = false;
       bool is_optional = false;
       auto var =
@@ -901,7 +901,10 @@ FunctionLiteral* Parser::DoParseFunction(ParseInfo* info) {
         } else {
           // BindingIdentifier
           ParseFormalParameter(&formals, &ok);
-          if (ok) DeclareFormalParameters(formals.scope, formals.params);
+          if (ok) {
+            DeclareFormalParameters(formals.scope, formals.params,
+                                    formals.is_simple);
+          }
         }
       }
 
@@ -2506,15 +2509,11 @@ void Parser::DeclareArrowFunctionFormalParameters(
     return;
   }
 
-  ExpressionClassifier classifier(this);
-  if (!parameters->is_simple) {
-    this->classifier()->RecordNonSimpleParameter();
-  }
-  DeclareFormalParameters(parameters->scope, parameters->params);
-  if (!this->classifier()
-           ->is_valid_formal_parameter_list_without_duplicates()) {
-    *duplicate_loc =
-        this->classifier()->duplicate_formal_parameter_error().location;
+  bool has_duplicate = false;
+  DeclareFormalParameters(parameters->scope, parameters->params,
+                          parameters->is_simple, &has_duplicate);
+  if (has_duplicate) {
+    *duplicate_loc = scanner()->location();
   }
   DCHECK_EQ(parameters->is_simple, parameters->scope->has_simple_parameters());
 }

src/parsing/parser.h

@@ -1085,11 +1085,10 @@ class V8_EXPORT_PRIVATE Parser : public NON_EXPORTED_BASE(ParserBase<Parser>) {
 
   V8_INLINE void DeclareFormalParameters(
       DeclarationScope* scope,
-      const ThreadedList<ParserFormalParameters::Parameter>& parameters) {
-    bool is_simple = classifier()->is_simple_parameter_list();
+      const ThreadedList<ParserFormalParameters::Parameter>& parameters,
+      bool is_simple, bool* has_duplicate = nullptr) {
     if (!is_simple) scope->SetHasNonSimpleParameters();
     for (auto parameter : parameters) {
-      bool is_duplicate = false;
       bool is_optional = parameter->initializer != nullptr;
       // If the parameter list is simple, declare the parameters normally with
       // their names. If the parameter list is not simple, declare a temporary
@@ -1098,12 +1097,7 @@ class V8_EXPORT_PRIVATE Parser : public NON_EXPORTED_BASE(ParserBase<Parser>) {
       scope->DeclareParameter(
           is_simple ? parameter->name : ast_value_factory()->empty_string(),
           is_simple ? VAR : TEMPORARY, is_optional, parameter->is_rest,
-          &is_duplicate, ast_value_factory(), parameter->position);
-      if (is_duplicate &&
-          classifier()->is_valid_formal_parameter_list_without_duplicates()) {
-        classifier()->RecordDuplicateFormalParameterError(
-            scanner()->location());
-      }
+          has_duplicate, ast_value_factory(), parameter->position);
     }
   }
 

src/parsing/preparser.h

@@ -1619,8 +1619,8 @@ class PreParser : public ParserBase<PreParser> {
 
   V8_INLINE void DeclareFormalParameters(
       DeclarationScope* scope,
-      const ThreadedList<PreParserFormalParameters::Parameter>& parameters) {
-    bool is_simple = classifier()->is_simple_parameter_list();
+      const ThreadedList<PreParserFormalParameters::Parameter>& parameters,
+      bool is_simple) {
     if (!is_simple) scope->SetHasNonSimpleParameters();
     if (track_unresolved_variables_) {
       DCHECK(FLAG_lazy_inner_functions);

test/mjsunit/duplicate-parameters.js

@@ -126,7 +126,7 @@ function testHelper(type, strict, lazy, duplicate_params_string, ok) {
 
 function test(type, strict, lazy, ok_if_param_list_simple) {
   // Simple duplicate params.
-  testHelper(type, strict, lazy, "dup, dup", ok_if_param_list_simple)
+  testHelper(type, strict, lazy, "a, dup, dup, b", ok_if_param_list_simple)
 
   if (strict != Strictness.STRICT_FUNCTION) {
     // Generate test cases where the duplicate parameter occurs because of
@@ -134,12 +134,12 @@ function test(type, strict, lazy, ok_if_param_list_simple) {
     // parameters are only allowed in simple parameter lists. These tests are
     // not possible if a function declares itself strict, since non-simple
     // parameters are not allowed then.
-    testHelper(type, strict, lazy, "[dup], dup", false);
-    testHelper(type, strict, lazy, "dup, {a: dup}", false);
-    testHelper(type, strict, lazy, "{dup}, [dup]", false);
-    testHelper(type, strict, lazy, "dup, ...dup", false);
-    testHelper(type, strict, lazy, "dup, dup, ...rest", false);
-    testHelper(type, strict, lazy, "dup, dup, a = 1", false);
+    testHelper(type, strict, lazy, "a, [dup], dup, b", false);
+    testHelper(type, strict, lazy, "a, dup, {b: dup}, c", false);
+    testHelper(type, strict, lazy, "a, {dup}, [dup], b", false);
+    testHelper(type, strict, lazy, "a, dup, ...dup", false);
+    testHelper(type, strict, lazy, "a, dup, dup, ...rest", false);
+    testHelper(type, strict, lazy, "a, dup, dup, b = 1", false);
   }
 }