[destructuring] Refactor duplicate parameter name detection.

src/preparser.h

 // Copyright 2012 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_PREPARSER_H
 #define V8_PREPARSER_H
 
 #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"
 #include "src/scanner.h"
 #include "src/scopes.h"
 #include "src/token.h"
 
 namespace v8 {
 namespace internal {
 
-
 // Common base class shared between parser and pre-parser. Traits encapsulate
 // the differences between Parser and PreParser:
 
 // - Return types: For example, Parser functions return Expression* and
 // PreParser functions return PreParserExpression.
 
 // - Creating parse tree nodes: Parser generates an AST during the recursive
 // descent. PreParser doesn't create a tree. Instead, it passes around minimal
 // data objects (PreParserExpression, PreParserIdentifier etc.) which contain
 // just enough data for the upper layer functions. PreParserFactory is
@@ skipping 15 matching lines @@
 //     // Return types for traversing functions.
 //     typedef Identifier;
 //     typedef Expression;
 //     typedef FunctionLiteral;
 //     typedef ClassLiteral;
 //     typedef ObjectLiteralProperty;
 //     typedef Literal;
 //     typedef ExpressionList;
 //     typedef PropertyList;
 //     typedef FormalParameter;
-//     typedef FormalParameterScope;
 //     // For constructing objects returned by the traversing functions.
 //     typedef Factory;
 //   };
 //   // ...
 // };
 
 template <typename Traits>
 class ParserBase : public Traits {
  public:
   // Shorten type names defined by Traits.
   typedef typename Traits::Type::Expression ExpressionT;
   typedef typename Traits::Type::Identifier IdentifierT;
   typedef typename Traits::Type::FormalParameter FormalParameterT;
-  typedef typename Traits::Type::FormalParameterScope FormalParameterScopeT;
   typedef typename Traits::Type::FunctionLiteral FunctionLiteralT;
   typedef typename Traits::Type::Literal LiteralT;
   typedef typename Traits::Type::ObjectLiteralProperty ObjectLiteralPropertyT;
 
   ParserBase(Zone* zone, Scanner* scanner, uintptr_t stack_limit,
              v8::Extension* extension, AstValueFactory* ast_value_factory,
              ParserRecorder* log, typename Traits::Type::Parser this_object)
       : Traits(this_object),
         parenthesized_function_(false),
         scope_(NULL),
@@ skipping 457 matching lines @@
   void ReportMessageAt(Scanner::Location location,
                        MessageTemplate::Template message,
                        ParseErrorType error_type = kSyntaxError) {
     Traits::ReportMessageAt(location, message, reinterpret_cast<const char*>(0),
                             error_type);
   }
 
   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);
   }
 
   void ValidateExpression(const ExpressionClassifier* classifier, bool* ok) {
     if (!classifier->is_valid_expression()) {
       ReportClassifierError(classifier->expression_error());
       *ok = false;
     }
   }
@@ skipping 131 matching lines @@
   ExpressionT ParseTemplateLiteral(ExpressionT tag, int start,
                                    ExpressionClassifier* classifier, bool* ok);
   void AddTemplateExpression(ExpressionT);
   ExpressionT ParseSuperExpression(bool is_new,
                                    ExpressionClassifier* classifier, bool* ok);
   ExpressionT ParseStrongInitializationExpression(
       ExpressionClassifier* classifier, bool* ok);
   ExpressionT ParseStrongSuperCallExpression(ExpressionClassifier* classifier,
                                              bool* ok);
 
-  void ParseFormalParameter(FormalParameterScopeT* scope, bool is_rest,
+  void ParseFormalParameter(Scope* scope, bool is_rest,
                             ExpressionClassifier* classifier, bool* ok);
-  int ParseFormalParameterList(FormalParameterScopeT* scope, bool* has_rest,
+  int ParseFormalParameterList(Scope* scope, bool* has_rest,
                                ExpressionClassifier* classifier, bool* ok);
   void CheckArityRestrictions(
       int param_count, FunctionLiteral::ArityRestriction arity_restriction,
       bool has_rest, int formals_start_pos, int formals_end_pos, bool* ok);
 
   // Checks if the expression is a valid reference expression (e.g., on the
   // left-hand side of assignments). Although ruled out by ECMA as early errors,
   // we allow calls for web compatibility and rewrite them to a runtime throw.
   ExpressionT CheckAndRewriteReferenceExpression(
       ExpressionT expression, Scanner::Location location,
@@ skipping 595 matching lines @@
     typedef PreParserIdentifier Identifier;
     typedef PreParserExpression Expression;
     typedef PreParserExpression YieldExpression;
     typedef PreParserExpression FunctionLiteral;
     typedef PreParserExpression ClassLiteral;
     typedef PreParserExpression ObjectLiteralProperty;
     typedef PreParserExpression Literal;
     typedef PreParserExpressionList ExpressionList;
     typedef PreParserExpressionList PropertyList;
     typedef PreParserIdentifier FormalParameter;
-    typedef DuplicateFinder FormalParameterScope;
     typedef PreParserStatementList StatementList;
 
     // For constructing objects returned by the traversing functions.
     typedef PreParserFactory Factory;
   };
 
   explicit PreParserTraits(PreParser* pre_parser) : pre_parser_(pre_parser) {}
 
   // Helper functions for recursive descent.
   static bool IsEval(PreParserIdentifier identifier) {
@@ skipping 252 matching lines @@
     return EmptyExpression();
   }
   inline void MaterializeTemplateCallsiteLiterals();
   PreParserExpression NoTemplateTag() {
     return PreParserExpression::NoTemplateTag();
   }
   static bool IsTaggedTemplate(const PreParserExpression tag) {
     return !tag.IsNoTemplateTag();
   }
 
-  V8_INLINE bool DeclareFormalParameter(DuplicateFinder* scope,
-                                        PreParserIdentifier param,
-                                        bool is_rest);
+  void DeclareFormalParameter(Scope* scope, PreParserIdentifier param,
+                              ExpressionClassifier* classifier, bool is_rest) {}
 
   void CheckConflictingVarDeclarations(Scope* scope, bool* ok) {}
 
   // Temporary glue; these functions will move to ParserBase.
   PreParserExpression ParseV8Intrinsic(bool* ok);
   PreParserExpression ParseFunctionLiteral(
       PreParserIdentifier name, Scanner::Location function_name_location,
       bool name_is_strict_reserved, FunctionKind kind,
       int function_token_position, FunctionLiteral::FunctionType type,
       FunctionLiteral::ArityRestriction arity_restriction, bool* ok);
@@ skipping 173 matching lines @@
   return pre_parser_->factory()->NewCall(function, args, pos);
 }
 
 PreParserExpression PreParserTraits::SpreadCallNew(PreParserExpression function,
                                                    PreParserExpressionList args,
                                                    int pos) {
   return pre_parser_->factory()->NewCallNew(function, args, pos);
 }
 
 
-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,
     Scanner::Location* duplicate_loc, bool* ok) {
   // TODO(wingo): Detect duplicated identifiers in paramlists.  Detect parameter
   // lists that are too long.
 }
 
 
 PreParserStatementList PreParser::ParseEagerFunctionBody(
@@ skipping 152 matching lines @@
       classifier->RecordStrongModeFormalParameterError(
           scanner()->location(), MessageTemplate::kStrongUndefined);
       if (is_strong(language_mode())) {
         // TODO(dslomov): allow 'undefined' in nested patterns.
         classifier->RecordBindingPatternError(
             scanner()->location(), MessageTemplate::kStrongUndefined);
         classifier->RecordAssignmentPatternError(
             scanner()->location(), MessageTemplate::kStrongUndefined);
       }
     }
+
+    if (classifier->duplicate_finder() != nullptr &&
+        scanner()->FindSymbol(classifier->duplicate_finder(), 1) != 0) {
+      classifier->RecordDuplicateFormalParameterError(scanner()->location());
+    }

[wingo, 2015/06/09 14:53:08]

Do we actually want to detect formal parameter errors at all in the preparser,
given our recent discussions?

[Dmitry Lomov (no reviews), 2015/06/09 14:56:11]

Good question. I am shooting for "no functionality regressions" in this CL, but
overall maybe it has to go.

     return name;
   } else if (is_sloppy(language_mode()) &&
              (next == Token::FUTURE_STRICT_RESERVED_WORD ||
               next == Token::LET || next == Token::STATIC ||
               (next == Token::YIELD && !is_generator()))) {
     classifier->RecordStrictModeFormalParameterError(
         scanner()->location(), MessageTemplate::kUnexpectedStrictReserved);
     return this->GetSymbol(scanner());
   } else {
     this->ReportUnexpectedToken(next);
@@ skipping 1470 matching lines @@
       default:
         return expression;
     }
   }
   DCHECK(false);
   return this->EmptyExpression();
 }
 
 
 template <class Traits>
-void ParserBase<Traits>::ParseFormalParameter(FormalParameterScopeT* scope,
-                                              bool is_rest,
+void ParserBase<Traits>::ParseFormalParameter(Scope* scope, bool is_rest,
                                               ExpressionClassifier* classifier,
                                               bool* ok) {
   // FormalParameter[Yield,GeneratorParameter] :
   //   BindingElement[?Yield, ?GeneratorParameter]
   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);
 }
 
 
 template <class Traits>
 int ParserBase<Traits>::ParseFormalParameterList(
-    FormalParameterScopeT* scope, bool* is_rest,
-    ExpressionClassifier* classifier, bool* ok) {
+    Scope* scope, bool* is_rest, ExpressionClassifier* classifier, bool* ok) {
   // FormalParameters[Yield,GeneratorParameter] :
   //   [empty]
   //   FormalParameterList[?Yield, ?GeneratorParameter]
   //
   // FormalParameterList[Yield,GeneratorParameter] :
   //   FunctionRestParameter[?Yield]
   //   FormalsList[?Yield, ?GeneratorParameter]
   //   FormalsList[?Yield, ?GeneratorParameter] , FunctionRestParameter[?Yield]
   //
   // FormalsList[Yield,GeneratorParameter] :
@@ skipping 336 matching lines @@
       *ok = false;
       return;
     }
     has_seen_constructor_ = true;
     return;
   }
 }
 } }  // v8::internal
 
 #endif  // V8_PREPARSER_H