[parser] Rewrite duplicate formal detection

Now duplicate parameter detection depends on tracking of unresolved references.
This also fixes finding duplicate parameters of arrow functions nested in other
arrow functions.

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

BUILD.gn

@@ -2304,7 +2304,6 @@ v8_source_set("v8_base") {
     "src/optimized-compilation-info.h",
     "src/ostreams.cc",
     "src/ostreams.h",
-    "src/parsing/duplicate-finder.h",
     "src/parsing/expression-classifier.h",
     "src/parsing/expression-scope-reparenter.cc",
     "src/parsing/expression-scope-reparenter.h",

src/ast/scopes.cc

@@ -1020,9 +1020,11 @@ Variable* Scope::Lookup(const AstRawString* name) {
   return nullptr;
 }
 
-Variable* DeclarationScope::DeclareParameter(
-    const AstRawString* name, VariableMode mode, bool is_optional, bool is_rest,
-    bool* is_duplicate, AstValueFactory* ast_value_factory, int position) {
+Variable* DeclarationScope::DeclareParameter(const AstRawString* name,
+                                             VariableMode mode,
+                                             bool is_optional, bool is_rest,
+                                             AstValueFactory* ast_value_factory,
+                                             int position) {
   DCHECK(!already_resolved_);
   DCHECK(is_function_scope() || is_module_scope());
   DCHECK(!has_rest_);
@@ -1035,10 +1037,6 @@ Variable* DeclarationScope::DeclareParameter(
   } else {
     DCHECK_EQ(mode, VariableMode::kVar);
     var = Declare(zone(), name, mode);
-    // TODO(wingo): Avoid O(n^2) check.
-    if (is_duplicate != nullptr) {
-      *is_duplicate = *is_duplicate || IsDeclaredParameter(name);
-    }
   }
   has_rest_ = is_rest;
   var->set_initializer_position(position);

src/ast/scopes.h

@@ -729,7 +729,7 @@ class V8_EXPORT_PRIVATE DeclarationScope : public Scope {
   // parameters the rightmost one 'wins'.  However, the implementation
   // expects all parameters to be declared and from left to right.
   Variable* DeclareParameter(const AstRawString* name, VariableMode mode,
-                             bool is_optional, bool is_rest, bool* is_duplicate,
+                             bool is_optional, bool is_rest,
                              AstValueFactory* ast_value_factory, int position);
 
   // Declares that a parameter with the name exists. Creates a Variable and

src/parsing/duplicate-finder.h

-// Copyright 2011 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.
-
-#ifndef V8_PARSING_DUPLICATE_FINDER_H_
-#define V8_PARSING_DUPLICATE_FINDER_H_
-
-#include <set>
-
-namespace v8 {
-namespace internal {
-
-class Scanner;
-
-// DuplicateFinder : Helper class to discover duplicate symbols.
-//
-// Allocate a DuplicateFinder for each set of symbols you want to check
-// for duplicates and then pass this instance into
-// Scanner::IsDuplicateSymbol(..).
-//
-// This class only holds the data; all actual logic is in
-// Scanner::IsDuplicateSymbol.
-class DuplicateFinder {
- public:
-  DuplicateFinder() = default;
-
- private:
-  friend class Scanner;
-
-  std::set<const void*> known_symbols_;
-};
-
-}  // namespace internal
-}  // namespace v8
-
-#endif  // V8_PARSING_DUPLICATE_FINDER_H_

src/parsing/expression-classifier.h

@@ -111,10 +111,8 @@ class ExpressionClassifierBase {
   };
   // clang-format on
 
-  explicit ExpressionClassifierBase(typename Types::Base* base,
-                                    DuplicateFinder* duplicate_finder = nullptr)
+  explicit ExpressionClassifierBase(typename Types::Base* base)
       : base_(base),
-        duplicate_finder_(duplicate_finder),
         invalid_productions_(0),
         is_non_simple_parameter_list_(0) {}
 
@@ -124,10 +122,6 @@ class ExpressionClassifierBase {
     return (invalid_productions_ & productions) == 0;
   }
 
-  V8_INLINE DuplicateFinder* duplicate_finder() const {
-    return duplicate_finder_;
-  }
-
   V8_INLINE bool is_valid_expression() const {
     return is_valid(ExpressionProduction);
   }
@@ -211,7 +205,6 @@ class ExpressionClassifierBase {
 
  protected:
   typename Types::Base* base_;
-  DuplicateFinder* duplicate_finder_;
   unsigned invalid_productions_ : kUnusedError;
   STATIC_ASSERT(kUnusedError <= 15);
   unsigned is_non_simple_parameter_list_ : 1;
@@ -228,9 +221,8 @@ class ExpressionClassifierErrorTracker
   using typename BaseClassType::ErrorKind;
   using TP = typename BaseClassType::TargetProduction;
 
-  ExpressionClassifierErrorTracker(typename Types::Base* base,
-                                   DuplicateFinder* duplicate_finder)
-      : BaseClassType(base, duplicate_finder),
+  explicit ExpressionClassifierErrorTracker(typename Types::Base* base)
+      : BaseClassType(base),
         reported_errors_(base->impl()->GetReportedErrorList()) {
     reported_errors_begin_ = reported_errors_end_ = reported_errors_->length();
   }
@@ -366,9 +358,8 @@ class ExpressionClassifierEmptyErrorTracker
   using typename BaseClassType::ErrorKind;
   using TP = typename BaseClassType::TargetProduction;
 
-  ExpressionClassifierEmptyErrorTracker(typename Types::Base* base,
-                                        DuplicateFinder* duplicate_finder)
-      : BaseClassType(base, duplicate_finder) {}
+  explicit ExpressionClassifierEmptyErrorTracker(typename Types::Base* base)
+      : BaseClassType(base) {}
 
   V8_INLINE void Discard() {}
 
@@ -411,12 +402,11 @@ class ExpressionClassifier
   using typename BaseClassType::ErrorKind;
   using TP = typename BaseClassType::TargetProduction;
 
-  explicit ExpressionClassifier(typename Types::Base* base,
-                                DuplicateFinder* duplicate_finder = nullptr)
-      : std::conditional<Types::ExpressionClassifierReportErrors,
-                         ExpressionClassifierErrorTracker<Types>,
-                         ExpressionClassifierEmptyErrorTracker<Types>>::
-            type(base, duplicate_finder),
+  explicit ExpressionClassifier(typename Types::Base* base)
+      : std::conditional<
+            Types::ExpressionClassifierReportErrors,
+            ExpressionClassifierErrorTracker<Types>,
+            ExpressionClassifierEmptyErrorTracker<Types>>::type(base),
         previous_(base->classifier_) {
     base->classifier_ = this;
   }

src/parsing/parser-base.h

@@ -1709,12 +1709,6 @@ ParserBase<Impl>::ParseAndClassifyIdentifier(bool* ok) {
       }
     }
 
-    if (classifier()->duplicate_finder() != nullptr &&
-        scanner()->IsDuplicateSymbol(classifier()->duplicate_finder(),
-                                     ast_value_factory())) {
-      classifier()->RecordDuplicateFormalParameterError(scanner()->location());
-    }
-
     return name;
   } else if (next == Token::AWAIT && !parsing_module_ && !is_async_function()) {
     classifier()->RecordAsyncArrowFormalParametersError(
@@ -2602,13 +2596,6 @@ ParserBase<Impl>::ParseObjectPropertyDefinition(ObjectLiteralChecker* checker,
 
       DCHECK(!*is_computed_name);
 
-      if (classifier()->duplicate_finder() != nullptr &&
-          scanner()->IsDuplicateSymbol(classifier()->duplicate_finder(),
-                                       ast_value_factory())) {
-        classifier()->RecordDuplicateFormalParameterError(
-            scanner()->location());
-      }
-
       if (impl()->IsEvalOrArguments(name) && is_strict(language_mode())) {
         classifier()->RecordBindingPatternError(
             scanner()->location(), MessageTemplate::kStrictEvalArguments);
@@ -2869,8 +2856,7 @@ ParserBase<Impl>::ParseAssignmentExpression(bool accept_IN, bool* ok) {
   }
 
   FuncNameInferrerState fni_state(&fni_);
-  ExpressionClassifier arrow_formals_classifier(
-      this, classifier()->duplicate_finder());
+  ExpressionClassifier arrow_formals_classifier(this);
 
   Scope::Snapshot scope_snapshot(scope());
   int rewritable_length = static_cast<int>(
@@ -2932,12 +2918,8 @@ ParserBase<Impl>::ParseAssignmentExpression(bool accept_IN, bool* ok) {
     }
 
     scope->set_start_position(lhs_beg_pos);
-    Scanner::Location duplicate_loc = Scanner::Location::invalid();
     impl()->DeclareArrowFunctionFormalParameters(&parameters, expression, loc,
-                                                 &duplicate_loc, CHECK_OK);
-    if (duplicate_loc.IsValid()) {
-      classifier()->RecordDuplicateFormalParameterError(duplicate_loc);
-    }
+                                                 CHECK_OK);
     expression = ParseArrowFunctionLiteral(accept_IN, parameters,
                                            rewritable_length, CHECK_OK);
     Accumulate(ExpressionClassifier::AsyncArrowFormalParametersProduction);
@@ -4253,7 +4235,17 @@ void ParserBase<Impl>::ParseFunctionBody(
 
   scope()->set_end_position(end_position());
 
-  if (!parameters.is_simple) {
+  bool allow_duplicate_parameters = false;
+
+  if (parameters.is_simple) {
+    DCHECK_EQ(inner_scope, function_scope);
+    if (is_sloppy(function_scope->language_mode())) {
+      impl()->InsertSloppyBlockFunctionVarBindings(function_scope);
+    }
+    allow_duplicate_parameters = is_sloppy(function_scope->language_mode()) &&
+                                 !IsConciseMethod(kind) &&
+                                 !IsArrowFunction(kind);
+  } else {
     DCHECK_NOT_NULL(inner_scope);
     DCHECK_EQ(function_scope, scope());
     DCHECK_EQ(function_scope, inner_scope->outer_scope());
@@ -4281,13 +4273,11 @@ void ParserBase<Impl>::ParseFunctionBody(
 
     result->Add(init_block, zone());
     result->Add(inner_block, zone());
-  } else {
-    DCHECK_EQ(inner_scope, function_scope);
-    if (is_sloppy(function_scope->language_mode())) {
-      impl()->InsertSloppyBlockFunctionVarBindings(function_scope);
-    }
   }
 
+  ValidateFormalParameters(language_mode(), allow_duplicate_parameters,
+                           CHECK_OK_VOID);
+
   if (!IsArrowFunction(kind)) {
     // Declare arguments after parsing the function since lexical 'arguments'
     // masks the arguments object. Declare arguments before declaring the
@@ -4437,6 +4427,10 @@ ParserBase<Impl>::ParseArrowFunctionLiteral(
             formal_parameters.scope, &dummy_num_parameters,
             &produced_preparsed_scope_data, false, false, &hint, CHECK_OK);
 
+        // Validate parameter names. We can do this only after preparsing the
+        // function, since the function can declare itself strict.
+        ValidateFormalParameters(language_mode(), false, CHECK_OK);
+
         DCHECK_NULL(produced_preparsed_scope_data);
 
         if (did_preparse_successfully) {
@@ -4478,14 +4472,6 @@ ParserBase<Impl>::ParseArrowFunctionLiteral(
 
     formal_parameters.scope->set_end_position(end_position());
 
-    // Arrow function formal parameters are parsed as StrictFormalParameterList,
-    // which is not the same as "parameters of a strict function"; it only means
-    // that duplicates are not allowed.  Of course, the arrow function may
-    // itself be strict as well.
-    const bool allow_duplicate_parameters = false;
-    ValidateFormalParameters(language_mode(), allow_duplicate_parameters,
-                             CHECK_OK);
-
     // Validate strict mode.
     if (is_strict(language_mode())) {
       CheckStrictOctalLiteral(formal_parameters.scope->start_position(),

src/parsing/parser.cc

@@ -18,7 +18,6 @@
 #include "src/log.h"
 #include "src/messages.h"
 #include "src/objects/scope-info.h"
-#include "src/parsing/duplicate-finder.h"
 #include "src/parsing/expression-scope-reparenter.h"
 #include "src/parsing/parse-info.h"
 #include "src/parsing/rewriter.h"
@@ -51,12 +50,11 @@ FunctionLiteral* Parser::DefaultConstructor(const AstRawString* name,
     if (call_super) {
       // Create a SuperCallReference and handle in BytecodeGenerator.
       auto constructor_args_name = ast_value_factory()->empty_string();
-      bool is_duplicate;
       bool is_rest = true;
       bool is_optional = false;
       Variable* constructor_args = function_scope->DeclareParameter(
           constructor_args_name, VariableMode::kTemporary, is_optional, is_rest,
-          &is_duplicate, ast_value_factory(), pos);
+          ast_value_factory(), pos);
 
       ZonePtrList<Expression>* args =
           new (zone()) ZonePtrList<Expression>(1, zone());
@@ -521,13 +519,10 @@ FunctionLiteral* Parser::DoParseProgram(Isolate* isolate, ParseInfo* info) {
       DCHECK(info->is_module());
       // Declare the special module parameter.
       auto name = ast_value_factory()->empty_string();
-      bool is_duplicate = false;
       bool is_rest = false;
       bool is_optional = false;
       auto var = scope->DeclareParameter(name, VariableMode::kVar, is_optional,
-                                         is_rest, &is_duplicate,
-                                         ast_value_factory(), beg_pos);
-      DCHECK(!is_duplicate);
+                                         is_rest, ast_value_factory(), beg_pos);
       var->AllocateTo(VariableLocation::PARAMETER, 0);
 
       PrepareGeneratorVariables();
@@ -2381,8 +2376,7 @@ void Parser::AddArrowFunctionFormalParameters(
 
 void Parser::DeclareArrowFunctionFormalParameters(
     ParserFormalParameters* parameters, Expression* expr,
-    const Scanner::Location& params_loc, Scanner::Location* duplicate_loc,
-    bool* ok) {
+    const Scanner::Location& params_loc, bool* ok) {
   if (expr->IsEmptyParentheses()) return;
 
   AddArrowFunctionFormalParameters(parameters, expr, params_loc.end_pos,
@@ -2394,12 +2388,8 @@ void Parser::DeclareArrowFunctionFormalParameters(
     return;
   }
 
-  bool has_duplicate = false;
   DeclareFormalParameters(parameters->scope, parameters->params,
-                          parameters->is_simple, &has_duplicate);
-  if (has_duplicate) {
-    *duplicate_loc = scanner()->location();
-  }
+                          parameters->is_simple);
   DCHECK_EQ(parameters->is_simple, parameters->scope->has_simple_parameters());
 }
 
@@ -2978,8 +2968,7 @@ ZonePtrList<Statement>* Parser::ParseFunction(
 
   bool is_wrapped = function_type == FunctionLiteral::kWrapped;
 
-  DuplicateFinder duplicate_finder;
-  ExpressionClassifier formals_classifier(this, &duplicate_finder);
+  ExpressionClassifier formals_classifier(this);
 
   int expected_parameters_end_pos = parameters_end_pos_;
   if (expected_parameters_end_pos != kNoSourcePosition) {
@@ -3042,14 +3031,6 @@ ZonePtrList<Statement>* Parser::ParseFunction(
   ParseFunctionBody(body, function_name, pos, formals, kind, function_type,
                     FunctionBodyType::kBlock, true, ok);
 
-  // Validate parameter names. We can do this only after parsing the function,
-  // since the function can declare itself strict.
-  const bool allow_duplicate_parameters =
-      is_sloppy(function_scope->language_mode()) && formals.is_simple &&
-      !IsConciseMethod(kind);
-  ValidateFormalParameters(function_scope->language_mode(),
-                           allow_duplicate_parameters, CHECK_OK);
-
   RewriteDestructuringAssignments();
 
   *has_duplicate_parameters =

src/parsing/parser.h

@@ -922,7 +922,7 @@ class V8_EXPORT_PRIVATE Parser : public NON_EXPORTED_BASE(ParserBase<Parser>) {
   V8_INLINE void DeclareFormalParameters(
       DeclarationScope* scope,
       const base::ThreadedList<ParserFormalParameters::Parameter>& parameters,
-      bool is_simple, bool* has_duplicate = nullptr) {
+      bool is_simple) {
     if (!is_simple) scope->SetHasNonSimpleParameters();
     for (auto parameter : parameters) {
       bool is_optional = parameter->initializer != nullptr;
@@ -930,10 +930,15 @@ class V8_EXPORT_PRIVATE Parser : public NON_EXPORTED_BASE(ParserBase<Parser>) {
       // their names. If the parameter list is not simple, declare a temporary
       // for each parameter - the corresponding named variable is declared by
       // BuildParamerterInitializationBlock.
+      if (is_simple && scope->LookupLocal(parameter->name)) {
+        classifier()->RecordDuplicateFormalParameterError(
+            Scanner::Location(parameter->position,
+                              parameter->position + parameter->name->length()));
+      }
       scope->DeclareParameter(
           is_simple ? parameter->name : ast_value_factory()->empty_string(),
           is_simple ? VariableMode::kVar : VariableMode::kTemporary,
-          is_optional, parameter->is_rest, has_duplicate, ast_value_factory(),
+          is_optional, parameter->is_rest, ast_value_factory(),
           parameter->position);
     }
   }
@@ -941,7 +946,6 @@ class V8_EXPORT_PRIVATE Parser : public NON_EXPORTED_BASE(ParserBase<Parser>) {
   void DeclareArrowFunctionFormalParameters(ParserFormalParameters* parameters,
                                             Expression* params,
                                             const Scanner::Location& params_loc,
-                                            Scanner::Location* duplicate_loc,
                                             bool* ok);
 
   Expression* ExpressionListToExpression(ZonePtrList<Expression>* args);

src/parsing/preparser.cc

@@ -9,7 +9,6 @@
 #include "src/conversions-inl.h"
 #include "src/conversions.h"
 #include "src/globals.h"
-#include "src/parsing/duplicate-finder.h"
 #include "src/parsing/parser-base.h"
 #include "src/parsing/preparsed-scope-data.h"
 #include "src/parsing/preparser.h"
@@ -158,13 +157,12 @@ PreParser::PreParseResult PreParser::PreParseFunction(
   bool* ok = &ok_holder;
 
   PreParserFormalParameters formals(function_scope);
-  DuplicateFinder duplicate_finder;
   std::unique_ptr<ExpressionClassifier> formals_classifier;
 
   // Parse non-arrow function parameters. For arrow functions, the parameters
   // have already been parsed.
   if (!IsArrowFunction(kind)) {
-    formals_classifier.reset(new ExpressionClassifier(this, &duplicate_finder));
+    formals_classifier.reset(new ExpressionClassifier(this));
     // We return kPreParseSuccess in failure cases too - errors are retrieved
     // separately by Parser::SkipLazyFunctionBody.
     ParseFormalParameterList(
@@ -194,7 +192,17 @@ PreParser::PreParseResult PreParser::PreParseFunction(
     result = ParseStatementListAndLogFunction(&formals, may_abort, ok);
   }
 
-  if (!formals.is_simple) {
+  bool allow_duplicate_parameters = false;
+
+  if (formals.is_simple) {
+    if (is_sloppy(function_scope->language_mode())) {
+      function_scope->HoistSloppyBlockFunctions(nullptr);
+    }
+
+    allow_duplicate_parameters = is_sloppy(function_scope->language_mode()) &&
+                                 !IsConciseMethod(kind) &&
+                                 !IsArrowFunction(kind);
+  } else {
     BuildParameterInitializationBlock(formals, ok);
 
     if (is_sloppy(inner_scope->language_mode())) {
@@ -204,10 +212,6 @@ PreParser::PreParseResult PreParser::PreParseFunction(
     SetLanguageMode(function_scope, inner_scope->language_mode());
     inner_scope->set_end_position(scanner()->peek_location().end_pos);
     inner_scope->FinalizeBlockScope();
-  } else {
-    if (is_sloppy(function_scope->language_mode())) {
-      function_scope->HoistSloppyBlockFunctions(nullptr);
-    }
   }
 
   use_counts_ = nullptr;
@@ -230,11 +234,7 @@ PreParser::PreParseResult PreParser::PreParseFunction(
     if (!IsArrowFunction(kind)) {
       // Validate parameter names. We can do this only after parsing the
       // function, since the function can declare itself strict.
-      const bool allow_duplicate_parameters =
-          is_sloppy(function_scope->language_mode()) && formals.is_simple &&
-          !IsConciseMethod(kind);
-      ValidateFormalParameters(function_scope->language_mode(),
-                               allow_duplicate_parameters, ok);
+      ValidateFormalParameters(language_mode(), allow_duplicate_parameters, ok);
       if (!*ok) {
         if (pending_error_handler()->ErrorUnidentifiableByPreParser()) {
           return kPreParseNotIdentifiableError;
@@ -322,8 +322,7 @@ PreParser::Expression PreParser::ParseFunctionLiteral(
   }
 
   FunctionState function_state(&function_state_, &scope_, function_scope);
-  DuplicateFinder duplicate_finder;
-  ExpressionClassifier formals_classifier(this, &duplicate_finder);
+  ExpressionClassifier formals_classifier(this);
   int func_id = GetNextFunctionLiteralId();
 
   Expect(Token::LPAREN, CHECK_OK);
@@ -357,9 +356,6 @@ PreParser::Expression PreParser::ParseFunctionLiteral(
   // function, since the function can declare itself strict.
   CheckFunctionName(language_mode, function_name, function_name_validity,
                     function_name_location, CHECK_OK);
-  const bool allow_duplicate_parameters =
-      is_sloppy(language_mode) && formals.is_simple && !IsConciseMethod(kind);
-  ValidateFormalParameters(language_mode, allow_duplicate_parameters, CHECK_OK);
 
   int end_position = scanner()->location().end_pos;
   if (is_strict(language_mode)) {

src/parsing/preparser.h

@@ -995,7 +995,7 @@ class PreParser : public ParserBase<PreParser> {
                               runtime_call_stats, logger, script_id,
                               parsing_module, parsing_on_main_thread),
         use_counts_(nullptr),
-        track_unresolved_variables_(false),
+        track_unresolved_variables_(true),
         preparsed_scope_data_builder_(nullptr) {}
 
   static bool IsPreParser() { return true; }
@@ -1721,11 +1721,22 @@ class PreParser : public ParserBase<PreParser> {
       for (auto parameter : parameters) {
         DCHECK_IMPLIES(is_simple, parameter->variables_ != nullptr);
         DCHECK_IMPLIES(is_simple, parameter->variables_->LengthForTest() == 1);
-        // Make sure each parameter is added only once even if it's a
-        // destructuring parameter which contains multiple names.
-        bool add_parameter = true;
-        if (parameter->variables_ != nullptr) {
+        if (parameter->variables_ == nullptr) {
+          // No names were declared; declare a dummy one here to up the
+          // parameter count.
+          scope->DeclareParameterName(ast_value_factory()->empty_string(),
+                                      parameter->is_rest, ast_value_factory(),
+                                      false, true);
+        } else {
+          // Make sure each parameter is added only once even if it's a
+          // destructuring parameter which contains multiple names.
+          bool add_parameter = true;
           for (auto variable : (*parameter->variables_)) {
+            // Find duplicates in simple and complex parameter lists.
+            if (scope->LookupLocal(variable->raw_name())) {
+              classifier()->RecordDuplicateFormalParameterError(
+                  Scanner::Location::invalid());
+            }
             // We declare the parameter name for all names, but only create a
             // parameter entry for the first one.
             scope->DeclareParameterName(variable->raw_name(),
@@ -1734,30 +1745,23 @@ class PreParser : public ParserBase<PreParser> {
             add_parameter = false;
           }
         }
-        if (add_parameter) {
-          // No names were declared; declare a dummy one here to up the
-          // parameter count.
-          DCHECK(!is_simple);
-          scope->DeclareParameterName(ast_value_factory()->empty_string(),
-                                      parameter->is_rest, ast_value_factory(),
-                                      false, add_parameter);
-        }
       }
     }
   }
 
   V8_INLINE void DeclareArrowFunctionFormalParameters(
       PreParserFormalParameters* parameters, const PreParserExpression& params,
-      const Scanner::Location& params_loc, Scanner::Location* duplicate_loc,
-      bool* ok) {
-    // TODO(wingo): Detect duplicated identifiers in paramlists.  Detect
-    // parameter lists that are too long.
+      const Scanner::Location& params_loc, bool* ok) {
     if (track_unresolved_variables_) {
       DCHECK(FLAG_lazy_inner_functions);
       if (params.variables_ != nullptr) {
+        Scope* scope = parameters->scope;
         for (auto variable : *params.variables_) {
-          parameters->scope->DeclareVariableName(variable->raw_name(),
-                                                 VariableMode::kVar);
+          if (scope->LookupLocal(variable->raw_name())) {
+            classifier()->RecordDuplicateFormalParameterError(
+                Scanner::Location::invalid());
+          }
+          scope->DeclareVariableName(variable->raw_name(), VariableMode::kVar);
         }
       }
     }

src/parsing/scanner.cc

@@ -13,7 +13,6 @@
 #include "src/ast/ast-value-factory.h"
 #include "src/conversions-inl.h"
 #include "src/objects/bigint.h"
-#include "src/parsing/duplicate-finder.h"  // For Scanner::FindSymbol
 #include "src/parsing/scanner-inl.h"
 
 namespace v8 {
@@ -1188,14 +1187,6 @@ const char* Scanner::CurrentLiteralAsCString(Zone* zone) const {
   return buffer;
 }
 
-bool Scanner::IsDuplicateSymbol(DuplicateFinder* duplicate_finder,
-                                AstValueFactory* ast_value_factory) const {
-  DCHECK_NOT_NULL(duplicate_finder);
-  DCHECK_NOT_NULL(ast_value_factory);
-  const AstRawString* string = CurrentSymbol(ast_value_factory);
-  return !duplicate_finder->known_symbols_.insert(string).second;
-}
-
 void Scanner::SeekNext(size_t position) {
   // Use with care: This cleanly resets most, but not all scanner state.
   // TODO(vogelheim): Fix this, or at least DCHECK the relevant conditions.

src/parsing/scanner.h

@@ -335,12 +335,6 @@ class Scanner {
            CurrentMatchesContextualEscaped(Token::LET);
   }
 
-  // Check whether the CurrentSymbol() has already been seen.
-  // The DuplicateFinder holds the data, so different instances can be used
-  // for different sets of duplicates to check for.
-  bool IsDuplicateSymbol(DuplicateFinder* duplicate_finder,
-                         AstValueFactory* ast_value_factory) const;
-
   UnicodeCache* unicode_cache() const { return unicode_cache_; }
 
   // Returns the location of the last seen octal literal.

test/mjsunit/regress/regress-preparse-inner-arrow-duplicate-parameter.js

+// Copyright 2018 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.
+
+assertThrows("()=>{ (x,x)=>1 }", SyntaxError)