Refactor formal parameter error locations into a class

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/func-name-inferrer.h"
 #include "src/hashmap.h"
 #include "src/scanner.h"
 #include "src/scopes.h"
 #include "src/token.h"
 
 namespace v8 {
 namespace internal {
 
+
+// When parsing the formal parameters of a function, we usually don't yet know
+// if the function will be strict, so we cannot yet produce errors for
+// parameter names or duplicates.  Instead, we remember the locations of these
+// errors if they occur and produce the errors later.
+class FormalParameterErrorLocations BASE_EMBEDDED {

[rossberg, 2015/04/13 09:12:16]

Could this be a local (protected?) class of PreParserBase?

+ public:
+  FormalParameterErrorLocations()
+      : eval_or_arguments_(Scanner::Location::invalid()),
+        undefined_(Scanner::Location::invalid()),
+        duplicate_(Scanner::Location::invalid()),
+        reserved_(Scanner::Location::invalid()) {}
+
+  Scanner::Location eval_or_arguments_;

[arv (Not doing code reviews), 2015/04/13 14:24:17]

Nit: public fields should not end with underscore.

+  Scanner::Location undefined_;
+  Scanner::Location duplicate_;
+  Scanner::Location reserved_;
+};
+
+
 // 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 462 matching lines @@
       Traits::ReportMessageAt(function_name_loc, "strong_undefined");
       *ok = false;
       return;
     }
   }
 
   // Checking the parameter names of a function literal. This has to be done
   // after parsing the function, since the function can declare itself strict.
   void CheckFunctionParameterNames(LanguageMode language_mode,
                                    bool strict_params,
-                                   const Scanner::Location& eval_args_loc,
-                                   const Scanner::Location& undefined_loc,
-                                   const Scanner::Location& dupe_loc,
-                                   const Scanner::Location& reserved_loc,
+                                   const FormalParameterErrorLocations& locs,
                                    bool* ok) {
     if (is_sloppy(language_mode) && !strict_params) return;
-    if (is_strict(language_mode) && eval_args_loc.IsValid()) {
-      Traits::ReportMessageAt(eval_args_loc, "strict_eval_arguments");
+    if (is_strict(language_mode) && locs.eval_or_arguments_.IsValid()) {
+      Traits::ReportMessageAt(locs.eval_or_arguments_, "strict_eval_arguments");
       *ok = false;
       return;
     }
-    if (is_strong(language_mode) && undefined_loc.IsValid()) {
-      Traits::ReportMessageAt(undefined_loc, "strong_undefined");
+    if (is_strong(language_mode) && locs.undefined_.IsValid()) {
+      Traits::ReportMessageAt(locs.undefined_, "strong_undefined");
       *ok = false;
       return;
     }
     // TODO(arv): When we add support for destructuring in setters we also need
     // to check for duplicate names.
-    if (dupe_loc.IsValid()) {
-      Traits::ReportMessageAt(dupe_loc, "strict_param_dupe");
+    if (locs.duplicate_.IsValid()) {
+      Traits::ReportMessageAt(locs.duplicate_, "strict_param_dupe");
       *ok = false;
       return;
     }
-    if (reserved_loc.IsValid()) {
-      Traits::ReportMessageAt(reserved_loc, "unexpected_strict_reserved");
+    if (locs.reserved_.IsValid()) {
+      Traits::ReportMessageAt(locs.reserved_, "unexpected_strict_reserved");
       *ok = false;
       return;
     }
   }
 
   // Determine precedence of given token.
   static int Precedence(Token::Value token, bool accept_IN) {
     if (token == Token::IN && !accept_IN)
       return 0;  // 0 precedence will terminate binary expression parsing
     return Token::Precedence(token);
@@ skipping 67 matching lines @@
   ExpressionT ParseMemberExpression(bool* ok);
   ExpressionT ParseMemberExpressionContinuation(ExpressionT expression,
                                                 bool* ok);
   ExpressionT ParseArrowFunctionLiteral(int start_pos, ExpressionT params_ast,
                                         bool* ok);
   ExpressionT ParseTemplateLiteral(ExpressionT tag, int start, bool* ok);
   void AddTemplateExpression(ExpressionT);
   ExpressionT ParseSuperExpression(bool is_new, bool* ok);
 
   FormalParameterT ParseFormalParameter(DuplicateFinder* duplicate_finder,
-                                        Scanner::Location* eval_args_error_loc,
-                                        Scanner::Location* undefined_error_loc,
-                                        Scanner::Location* dupe_error_loc,
-                                        Scanner::Location* reserved_error_loc,
+                                        FormalParameterErrorLocations* locs,
                                         bool* ok);
   FormalParameterListT ParseFormalParameterList(
-      Scanner::Location* eval_args_error_loc,
-      Scanner::Location* undefined_error_loc, Scanner::Location* dupe_error_loc,
-      Scanner::Location* reserved_error_loc, bool* is_rest, bool* ok);
+      FormalParameterErrorLocations* locs, bool* is_rest, bool* ok);
   void CheckArityRestrictions(
       int param_count, FunctionLiteral::ArityRestriction arity_restriction,
       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, const char* message, bool* ok);
@@ skipping 289 matching lines @@
 
   bool IsCall() const {
     return TypeField::decode(code_) == kExpression &&
            ExpressionTypeField::decode(code_) == kCallExpression;
   }
 
   bool IsValidReferenceExpression() const {
     return IsIdentifier() || IsProperty();
   }
 
-  bool IsValidArrowParamList(Scanner::Location* undefined_loc) const {
+  bool IsValidArrowParamList(FormalParameterErrorLocations* locs) const {
     ValidArrowParam valid = ValidateArrowParams();
     if (ParenthesizationField::decode(code_) == kMultiParenthesizedExpression) {
       return false;
     }
     if (valid == kValidArrowParam) {
       return true;
     } else if (valid == kInvalidStrongArrowParam) {
       // Return true for now regardless of strong mode for compatibility with
       // parser.
-      *undefined_loc = Scanner::Location();
+      locs->undefined_ = Scanner::Location();
       return true;
     } else {
       return false;
     }
   }
 
   // At the moment PreParser doesn't track these expression types.
   bool IsFunctionLiteral() const { return false; }
   bool IsCallNew() const { return false; }
 
@@ skipping 584 matching lines @@
                                       int* expected_property_count, bool* ok) {
     UNREACHABLE();
   }
 
   V8_INLINE PreParserStatementList
   ParseEagerFunctionBody(PreParserIdentifier function_name, int pos,
                          Variable* fvar, Token::Value fvar_init_op,
                          FunctionKind kind, bool* ok);
 
   // Utility functions
-  int DeclareArrowParametersFromExpression(PreParserExpression expression,
-                                           Scope* scope,
-                                           Scanner::Location* undefined_loc,
-                                           Scanner::Location* dupe_loc,
-                                           bool* ok) {
-    // TODO(aperez): Detect duplicated identifiers in paramlists.
-    *ok = expression.IsValidArrowParamList(undefined_loc);
+  V8_INLINE int DeclareArrowParametersFromExpression(
+      PreParserExpression expression, Scope* scope,
+      FormalParameterErrorLocations* locs, bool* ok) {
+    // TODO(wingo): Detect duplicated identifiers in paramlists.  Detect eval or
+    // arguments.  Detect reserved words.
+    *ok = expression.IsValidArrowParamList(locs);
     return 0;
   }
 
   struct TemplateLiteralState {};
 
   TemplateLiteralState OpenTemplateLiteral(int pos) {
     return TemplateLiteralState();
   }
   void AddTemplateSpan(TemplateLiteralState*, bool) {}
   void AddTemplateExpression(TemplateLiteralState*, PreParserExpression) {}
@@ skipping 1484 matching lines @@
     }
   }
   DCHECK(false);
   return this->EmptyExpression();
 }
 
 
 template <class Traits>
 typename ParserBase<Traits>::FormalParameterT
 ParserBase<Traits>::ParseFormalParameter(DuplicateFinder* duplicate_finder,
-                                         Scanner::Location* eval_args_error_loc,
-                                         Scanner::Location* undefined_error_loc,
-                                         Scanner::Location* dupe_error_loc,
-                                         Scanner::Location* reserved_error_loc,
+                                         FormalParameterErrorLocations* locs,
                                          bool* ok) {
   // FormalParameter[Yield,GeneratorParameter] :
   //   BindingElement[?Yield, ?GeneratorParameter]
   bool is_strict_reserved;
   IdentifierT name =
       ParseIdentifierOrStrictReservedWord(&is_strict_reserved, ok);
   if (!*ok) return this->EmptyFormalParameter();
 
   // Store locations for possible future error reports.
-  if (!eval_args_error_loc->IsValid() && this->IsEvalOrArguments(name)) {
-    *eval_args_error_loc = scanner()->location();
+  if (!locs->eval_or_arguments_.IsValid() && this->IsEvalOrArguments(name)) {
+    locs->eval_or_arguments_ = scanner()->location();
   }
-  if (!undefined_error_loc->IsValid() && this->IsUndefined(name)) {
-    *undefined_error_loc = scanner()->location();
+  if (!locs->undefined_.IsValid() && this->IsUndefined(name)) {
+    locs->undefined_ = scanner()->location();
   }
-  if (!reserved_error_loc->IsValid() && is_strict_reserved) {
-    *reserved_error_loc = scanner()->location();
+  if (!locs->reserved_.IsValid() && is_strict_reserved) {
+    locs->reserved_ = scanner()->location();
   }
-  if (!dupe_error_loc->IsValid()) {
+  if (!locs->duplicate_.IsValid()) {
     int prev_value = scanner()->FindSymbol(duplicate_finder, 1);
-    if (prev_value != 0) *dupe_error_loc = scanner()->location();
+    if (prev_value != 0) locs->duplicate_ = scanner()->location();
   }
 
   return name;
 }
 
 
 template <class Traits>
 typename ParserBase<Traits>::FormalParameterListT
 ParserBase<Traits>::ParseFormalParameterList(
-    Scanner::Location* eval_args_error_loc,
-    Scanner::Location* undefined_error_loc, Scanner::Location* dupe_error_loc,
-    Scanner::Location* reserved_error_loc, bool* is_rest, bool* ok) {
+    FormalParameterErrorLocations* locs, bool* is_rest, 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] :
   //   FormalParameter[?Yield, ?GeneratorParameter]
   //   FormalsList[?Yield, ?GeneratorParameter] ,
   //     FormalParameter[?Yield,?GeneratorParameter]
 
   FormalParameterListT result = this->NewFormalParameterList(4, zone_);
   DuplicateFinder duplicate_finder(scanner()->unicode_cache());
 
   if (peek() != Token::RPAREN) {
     do {
       *is_rest = allow_harmony_rest_params() && Check(Token::ELLIPSIS);
-      FormalParameterT param = ParseFormalParameter(
-          &duplicate_finder, eval_args_error_loc, undefined_error_loc,
-          dupe_error_loc, reserved_error_loc, ok);
+      FormalParameterT param =
+          ParseFormalParameter(&duplicate_finder, locs, ok);
       if (!*ok) return this->NullFormalParameterList();
       result->Add(param, zone());
       if (result->length() > Code::kMaxArguments) {
         ReportMessage("too_many_parameters");
         *ok = false;
         return this->NullFormalParameterList();
       }
     } while (!*is_rest && Check(Token::COMMA));
   }
 
@@ skipping 50 matching lines @@
   int num_parameters = -1;
   int materialized_literal_count = -1;
   int expected_property_count = -1;
   int handler_count = 0;
   Scanner::Location super_loc;
 
   {
     typename Traits::Type::Factory function_factory(ast_value_factory());
     FunctionState function_state(&function_state_, &scope_, scope,
                                  kArrowFunction, &function_factory);
-    Scanner::Location undefined_loc = Scanner::Location::invalid();
-    Scanner::Location dupe_loc = Scanner::Location::invalid();
-    // TODO(arv): Pass in eval_args_loc and reserved_loc here.
+    FormalParameterErrorLocations error_locs;
     num_parameters = Traits::DeclareArrowParametersFromExpression(
-        params_ast, scope_, &undefined_loc, &dupe_loc, ok);
+        params_ast, scope_, &error_locs, ok);
     if (!*ok) {
       ReportMessageAt(
           Scanner::Location(start_pos, scanner()->location().beg_pos),
           "malformed_arrow_function_parameter_list");
       return this->EmptyExpression();
     }
 
-    if (undefined_loc.IsValid()) {
+    if (error_locs.undefined_.IsValid()) {
       // Workaround for preparser not keeping track of positions.
-      undefined_loc = Scanner::Location(start_pos,
-                                        scanner()->location().end_pos);
+      error_locs.undefined_ =
+          Scanner::Location(start_pos, scanner()->location().end_pos);
     }
     if (num_parameters > Code::kMaxArguments) {
       ReportMessageAt(Scanner::Location(params_ast->position(), position()),
                       "too_many_parameters");
       *ok = false;
       return this->EmptyExpression();
     }
 
     Expect(Token::ARROW, CHECK_OK);
 
@@ skipping 25 matching lines @@
       body->Add(factory()->NewReturnStatement(expression, pos), zone());
       materialized_literal_count = function_state.materialized_literal_count();
       expected_property_count = function_state.expected_property_count();
       handler_count = function_state.handler_count();
     }
     super_loc = function_state.super_call_location();
 
     scope->set_start_position(start_pos);
     scope->set_end_position(scanner()->location().end_pos);
 
-    // Arrow function *parameter lists* are always checked as in strict mode.
-    // TODO(arv): eval_args_loc and reserved_loc needs to be set by
-    // DeclareArrowParametersFromExpression.
-    Scanner::Location eval_args_loc = Scanner::Location::invalid();
-    Scanner::Location reserved_loc = Scanner::Location::invalid();
+    // 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 use_strict_params = true;
     this->CheckFunctionParameterNames(language_mode(), use_strict_params,
-                                      eval_args_loc, undefined_loc, dupe_loc,
-                                      reserved_loc, CHECK_OK);
+                                      error_locs, CHECK_OK);
 
     // Validate strict mode.
     if (is_strict(language_mode())) {
       CheckStrictOctalLiteral(start_pos, scanner()->location().end_pos,
                               CHECK_OK);
       this->CheckConflictingVarDeclarations(scope, CHECK_OK);
     }
   }
 
   FunctionLiteralT function_literal = factory()->NewFunctionLiteral(
@@ skipping 192 matching lines @@
       *ok = false;
       return;
     }
     has_seen_constructor_ = true;
     return;
   }
 }
 } }  // v8::internal
 
 #endif  // V8_PREPARSER_H