[destructuring] Refactor duplicate parameter name detection.

src/preparser.h

Index: src/preparser.h
diff --git a/src/preparser.h b/src/preparser.h
index b89263229fe372368d3dcd9f20e7313f3fab9aa5..f0cdbefa2383205a4833c3ccb128f7a0e37c87f4 100644
--- a/src/preparser.h
+++ b/src/preparser.h
@@ -8,6 +8,7 @@
 #include "src/v8.h"
 
 #include "src/bailout-reason.h"
+#include "src/expression-classifier.h"
 #include "src/func-name-inferrer.h"
 #include "src/hashmap.h"
 #include "src/messages.h"
@@ -18,7 +19,6 @@
 namespace v8 {
 namespace internal {
 
-
 // Common base class shared between parser and pre-parser. Traits encapsulate
 // the differences between Parser and PreParser:
 
@@ -546,217 +546,8 @@ class ParserBase : public Traits {
   void ReportUnexpectedToken(Token::Value token);
   void ReportUnexpectedTokenAt(Scanner::Location location, Token::Value token);
 
-  class ExpressionClassifier {
-   public:
-    struct Error {
-      Error()
-          : location(Scanner::Location::invalid()),
-            message(MessageTemplate::kNone),
-            arg(nullptr) {}
-
-      Scanner::Location location;
-      MessageTemplate::Template message;
-      const char* arg;
-
-      bool HasError() const { return location.IsValid(); }
-    };
-
-    ExpressionClassifier() {}
-
-    bool is_valid_expression() const { return !expression_error_.HasError(); }
-
-    bool is_valid_binding_pattern() const {
-      return !binding_pattern_error_.HasError();
-    }
-
-    bool is_valid_assignment_pattern() const {
-      return !assignment_pattern_error_.HasError();
-    }
-
-    bool is_valid_arrow_formal_parameters() const {
-      return !arrow_formal_parameters_error_.HasError();
-    }
-
-    bool is_valid_formal_parameter_list_without_duplicates() const {
-      return !duplicate_formal_parameter_error_.HasError();
-    }
-
-    // Note: callers should also check
-    // is_valid_formal_parameter_list_without_duplicates().
-    bool is_valid_strict_mode_formal_parameters() const {
-      return !strict_mode_formal_parameter_error_.HasError();
-    }
-
-    // Note: callers should also check is_valid_strict_mode_formal_parameters()
-    // and is_valid_formal_parameter_list_without_duplicates().
-    bool is_valid_strong_mode_formal_parameters() const {
-      return !strong_mode_formal_parameter_error_.HasError();
-    }
-
-    const Error& expression_error() const { return expression_error_; }
-
-    const Error& binding_pattern_error() const {
-      return binding_pattern_error_;
-    }
-
-    const Error& assignment_pattern_error() const {
-      return assignment_pattern_error_;
-    }
-
-    const Error& arrow_formal_parameters_error() const {
-      return arrow_formal_parameters_error_;
-    }
-
-    const Error& duplicate_formal_parameter_error() const {
-      return duplicate_formal_parameter_error_;
-    }
-
-    const Error& strict_mode_formal_parameter_error() const {
-      return strict_mode_formal_parameter_error_;
-    }
-
-    const Error& strong_mode_formal_parameter_error() const {
-      return strong_mode_formal_parameter_error_;
-    }
-
-    void RecordExpressionError(const Scanner::Location& loc,
-                               MessageTemplate::Template message,
-                               const char* arg = nullptr) {
-      if (!is_valid_expression()) return;
-      expression_error_.location = loc;
-      expression_error_.message = message;
-      expression_error_.arg = arg;
-    }
-
-    void RecordBindingPatternError(const Scanner::Location& loc,
-                                   MessageTemplate::Template message,
-                                   const char* arg = nullptr) {
-      if (!is_valid_binding_pattern()) return;
-      binding_pattern_error_.location = loc;
-      binding_pattern_error_.message = message;
-      binding_pattern_error_.arg = arg;
-    }
-
-    void RecordAssignmentPatternError(const Scanner::Location& loc,
-                                      MessageTemplate::Template message,
-                                      const char* arg = nullptr) {
-      if (!is_valid_assignment_pattern()) return;
-      assignment_pattern_error_.location = loc;
-      assignment_pattern_error_.message = message;
-      assignment_pattern_error_.arg = arg;
-    }
-
-    void RecordArrowFormalParametersError(const Scanner::Location& loc,
-                                          MessageTemplate::Template message,
-                                          const char* arg = nullptr) {
-      if (!is_valid_arrow_formal_parameters()) return;
-      arrow_formal_parameters_error_.location = loc;
-      arrow_formal_parameters_error_.message = message;
-      arrow_formal_parameters_error_.arg = arg;
-    }
-
-    void RecordDuplicateFormalParameterError(const Scanner::Location& loc) {
-      if (!is_valid_formal_parameter_list_without_duplicates()) return;
-      duplicate_formal_parameter_error_.location = loc;
-      duplicate_formal_parameter_error_.message =
-          MessageTemplate::kStrictParamDupe;
-      duplicate_formal_parameter_error_.arg = nullptr;
-    }
-
-    // Record a binding that would be invalid in strict mode.  Confusingly this
-    // is not the same as StrictFormalParameterList, which simply forbids
-    // duplicate bindings.
-    void RecordStrictModeFormalParameterError(const Scanner::Location& loc,
-                                              MessageTemplate::Template message,
-                                              const char* arg = nullptr) {
-      if (!is_valid_strict_mode_formal_parameters()) return;
-      strict_mode_formal_parameter_error_.location = loc;
-      strict_mode_formal_parameter_error_.message = message;
-      strict_mode_formal_parameter_error_.arg = arg;
-    }
-
-    void RecordStrongModeFormalParameterError(const Scanner::Location& loc,
-                                              MessageTemplate::Template message,
-                                              const char* arg = nullptr) {
-      if (!is_valid_strong_mode_formal_parameters()) return;
-      strong_mode_formal_parameter_error_.location = loc;
-      strong_mode_formal_parameter_error_.message = message;
-      strong_mode_formal_parameter_error_.arg = arg;
-    }
-
-    enum TargetProduction {
-      ExpressionProduction = 1 << 0,
-      BindingPatternProduction = 1 << 1,
-      AssignmentPatternProduction = 1 << 2,
-      FormalParametersProduction = 1 << 3,
-      ArrowFormalParametersProduction = 1 << 4,
-      StandardProductions = (ExpressionProduction | BindingPatternProduction |
-                             AssignmentPatternProduction),
-      PatternProductions =
-          BindingPatternProduction | AssignmentPatternProduction,
-      AllProductions = (StandardProductions | FormalParametersProduction |
-                        ArrowFormalParametersProduction),
-    };
 
-    void Accumulate(const ExpressionClassifier& inner,
-                    unsigned productions = StandardProductions) {
-      if (productions & ExpressionProduction && is_valid_expression()) {
-        expression_error_ = inner.expression_error_;
-      }
-      if (productions & BindingPatternProduction &&
-          is_valid_binding_pattern()) {
-        binding_pattern_error_ = inner.binding_pattern_error_;
-      }
-      if (productions & AssignmentPatternProduction &&
-          is_valid_assignment_pattern()) {
-        assignment_pattern_error_ = inner.assignment_pattern_error_;
-      }
-      if (productions & FormalParametersProduction) {
-        if (is_valid_formal_parameter_list_without_duplicates()) {
-          duplicate_formal_parameter_error_ =
-              inner.duplicate_formal_parameter_error_;
-        }
-        if (is_valid_strict_mode_formal_parameters()) {
-          strict_mode_formal_parameter_error_ =
-              inner.strict_mode_formal_parameter_error_;
-        }
-        if (is_valid_strong_mode_formal_parameters()) {
-          strong_mode_formal_parameter_error_ =
-              inner.strong_mode_formal_parameter_error_;
-        }
-      }
-      if (productions & ArrowFormalParametersProduction &&
-          is_valid_arrow_formal_parameters()) {
-        // The result continues to be a valid arrow formal parameters if the
-        // inner expression is a valid binding pattern.
-        arrow_formal_parameters_error_ = inner.binding_pattern_error_;
-      }
-    }
-
-    void AccumulateReclassifyingAsPattern(const ExpressionClassifier& inner) {
-      Accumulate(inner, AllProductions & ~PatternProductions);
-      if (!inner.is_valid_expression()) {
-        if (is_valid_binding_pattern()) {
-          binding_pattern_error_ = inner.expression_error();
-        }
-        if (is_valid_assignment_pattern()) {
-          assignment_pattern_error_ = inner.expression_error();
-        }
-      }
-    }
-
-   private:
-    Error expression_error_;
-    Error binding_pattern_error_;
-    Error assignment_pattern_error_;
-    Error arrow_formal_parameters_error_;
-    Error duplicate_formal_parameter_error_;
-    Error strict_mode_formal_parameter_error_;
-    Error strong_mode_formal_parameter_error_;
-  };
-
-  void ReportClassifierError(
-      const typename ExpressionClassifier::Error& error) {
+  void ReportClassifierError(const ExpressionClassifier::Error& error) {
     Traits::ReportMessageAt(error.location, error.message, error.arg,
                             kSyntaxError);
   }
@@ -1526,7 +1317,7 @@ class PreParserTraits {
     typedef PreParserExpressionList ExpressionList;
     typedef PreParserExpressionList PropertyList;
     typedef PreParserIdentifier FormalParameter;
-    typedef DuplicateFinder FormalParameterScope;
+    typedef void* FormalParameterScope;
     typedef PreParserStatementList StatementList;
 
     // For constructing objects returned by the traversing functions.
@@ -1799,9 +1590,8 @@ class PreParserTraits {
     return !tag.IsNoTemplateTag();
   }
 
-  V8_INLINE bool DeclareFormalParameter(DuplicateFinder* scope,
-                                        PreParserIdentifier param,
-                                        bool is_rest);
+  void DeclareFormalParameter(void* scope, PreParserIdentifier param,

[arv (Not doing code reviews), 2015/06/09 14:17:47]

FormalParameterScope scope

[Dmitry Lomov (no reviews), 2015/06/09 14:38:30]

I did one better and removed that trait completely. It is now just Scope*.

+                              ExpressionClassifier* classifier, bool is_rest) {}
 
   void CheckConflictingVarDeclarations(Scope* scope, bool* ok) {}
 
@@ -1995,13 +1785,6 @@ PreParserExpression PreParserTraits::SpreadCallNew(PreParserExpression function,
 }
 
 
-bool PreParserTraits::DeclareFormalParameter(
-    DuplicateFinder* duplicate_finder, PreParserIdentifier current_identifier,
-    bool is_rest) {
-  return pre_parser_->scanner()->FindSymbol(duplicate_finder, 1) != 0;
-}
-
-
 void PreParserTraits::ParseArrowFunctionFormalParameters(
     Scope* scope, PreParserExpression params,
     const Scanner::Location& params_loc, bool* is_rest,
@@ -2174,6 +1957,11 @@ ParserBase<Traits>::ParseAndClassifyIdentifier(ExpressionClassifier* classifier,
             scanner()->location(), MessageTemplate::kStrongUndefined);
       }
     }
+
+    if (classifier->duplicate_finder() != nullptr &&
+        scanner()->FindSymbol(classifier->duplicate_finder(), 1) != 0) {
+      classifier->RecordDuplicateFormalParameterError(scanner()->location());
+    }
     return name;
   } else if (is_sloppy(language_mode()) &&
              (next == Token::FUTURE_STRICT_RESERVED_WORD ||
@@ -3673,10 +3461,7 @@ void ParserBase<Traits>::ParseFormalParameter(FormalParameterScopeT* scope,
   IdentifierT name = ParseAndClassifyIdentifier(classifier, ok);
   if (!*ok) return;
 
-  bool was_declared = Traits::DeclareFormalParameter(scope, name, is_rest);
-  if (was_declared) {
-    classifier->RecordDuplicateFormalParameterError(scanner()->location());
-  }
+  Traits::DeclareFormalParameter(scope, name, classifier, is_rest);
 }