Use ExpressionClassifier to identify valid arrow function formals

src/parser.cc

 // 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.
 
 #include "src/v8.h"
 
 #include "src/api.h"
 #include "src/ast.h"
 #include "src/bailout-reason.h"
 #include "src/base/platform/platform.h"
@@ skipping 1120 matching lines @@
     FunctionLiteral::FunctionType function_type = shared_info->is_expression()
         ? (shared_info->is_anonymous()
               ? FunctionLiteral::ANONYMOUS_EXPRESSION
               : FunctionLiteral::NAMED_EXPRESSION)
         : FunctionLiteral::DECLARATION;
     bool ok = true;
 
     if (shared_info->is_arrow()) {
       Scope* scope = NewScope(scope_, ARROW_SCOPE);
       scope->set_start_position(shared_info->start_position());
-      FormalParameterErrorLocations error_locs;
+      ExpressionClassifier formals_classifier;
       bool has_rest = false;
       if (Check(Token::LPAREN)) {
         // '(' StrictFormalParameters ')'
-        ParseFormalParameterList(scope, &error_locs, &has_rest, &ok);
+        ParseFormalParameterList(scope, &has_rest, &formals_classifier, &ok);
         if (ok) ok = Check(Token::RPAREN);
       } else {
         // BindingIdentifier
-        ParseFormalParameter(scope, &error_locs, has_rest, &ok);
+        ParseFormalParameter(scope, has_rest, &formals_classifier, &ok);
       }
 
       if (ok) {
-        ExpressionClassifier classifier;
-        Expression* expression = ParseArrowFunctionLiteral(
-            scope, error_locs, has_rest, &classifier, &ok);
-        ValidateExpression(&classifier, &ok);
+        Expression* expression =
+            ParseArrowFunctionLiteral(scope, has_rest, formals_classifier, &ok);
         if (ok) {
           // Scanning must end at the same position that was recorded
           // previously. If not, parsing has been interrupted due to a stack
           // overflow, at which point the partially parsed arrow function
           // concise body happens to be a valid expression. This is a problem
           // only for arrow functions with single expression bodies, since there
           // is no end token such as "}" for normal functions.
           if (scanner()->location().end_pos == shared_info->end_position()) {
             // The pre-parser saw an arrow function here, so the full parser
             // must produce a FunctionLiteral.
@@ skipping 2546 matching lines @@
 }
 
 
 Handle<FixedArray> CompileTimeValue::GetElements(Handle<FixedArray> value) {
   return Handle<FixedArray>(FixedArray::cast(value->get(kElementsSlot)));
 }
 
 
 void ParserTraits::DeclareArrowFunctionParameters(
     Scope* scope, Expression* expr, const Scanner::Location& params_loc,
-    FormalParameterErrorLocations* error_locs, bool* ok) {
+    Scanner::Location* duplicate_loc, bool* ok) {
   if (scope->num_parameters() >= Code::kMaxArguments) {
     ReportMessageAt(params_loc, "malformed_arrow_function_parameter_list");
     *ok = false;
     return;
   }
 
   // ArrowFunctionFormals ::
   //    Binary(Token::COMMA, ArrowFunctionFormals, VariableProxy)
   //    VariableProxy
   //
   // As we need to visit the parameters in left-to-right order, we recurse on
   // the left-hand side of comma expressions.
   //
   // Sadly, for the various malformed_arrow_function_parameter_list errors, we
   // can't be more specific on the error message or on the location because we
   // need to match the pre-parser's behavior.
   if (expr->IsBinaryOperation()) {
     BinaryOperation* binop = expr->AsBinaryOperation();
+    // TODO(wingo): These checks are now unnecessary, given the classifier.
     if (binop->op() != Token::COMMA) {
       ReportMessageAt(params_loc, "malformed_arrow_function_parameter_list");
       *ok = false;
       return;
     }
     Expression* left = binop->left();
     Expression* right = binop->right();
     if (left->is_single_parenthesized() || right->is_single_parenthesized()) {
       ReportMessageAt(params_loc, "malformed_arrow_function_parameter_list");
       *ok = false;
       return;
     }
-    DeclareArrowFunctionParameters(scope, left, params_loc, error_locs, ok);
+    DeclareArrowFunctionParameters(scope, left, params_loc, duplicate_loc, ok);
     if (!*ok) return;
     // LHS of comma expression should be unparenthesized.
     expr = right;
   }
 
   // TODO(wingo): Support rest parameters.
   if (!expr->IsVariableProxy()) {
     ReportMessageAt(params_loc, "malformed_arrow_function_parameter_list");
     *ok = false;
     return;
   }
 
   const AstRawString* raw_name = expr->AsVariableProxy()->raw_name();
   Scanner::Location param_location(expr->position(),
                                    expr->position() + raw_name->length());
 
   if (expr->AsVariableProxy()->is_this()) {
     ReportMessageAt(param_location, "this_formal_parameter");
     *ok = false;
     return;
   }
 
-  if (!error_locs->eval_or_arguments.IsValid() && IsEvalOrArguments(raw_name))
-    error_locs->eval_or_arguments = param_location;
-  if (!error_locs->reserved.IsValid() && IsFutureStrictReserved(raw_name))
-    error_locs->reserved = param_location;
-  if (!error_locs->undefined.IsValid() && IsUndefined(raw_name))
-    error_locs->undefined = param_location;
-
   // When the formal parameter was originally seen, it was parsed as a
   // VariableProxy and recorded as unresolved in the scope.  Here we undo that
   // parse-time side-effect.
   parser_->scope_->RemoveUnresolved(expr->AsVariableProxy());
 
   bool is_rest = false;
   bool is_duplicate = DeclareFormalParameter(scope, raw_name, is_rest);
 
-  if (is_duplicate) {
-    // Arrow function parameter lists are parsed as StrictFormalParameters,
-    // which means that they cannot have duplicates.  Note that this is a subset
-    // of the restrictions placed on parameters to functions whose body is
-    // strict.
-    ReportMessageAt(param_location,
-                    "duplicate_arrow_function_formal_parameter");
-    *ok = false;
-    return;
+  if (is_duplicate && !duplicate_loc->IsValid()) {
+    *duplicate_loc = param_location;
   }
 }
 
 
 void ParserTraits::ParseArrowFunctionFormalParameters(
     Scope* scope, Expression* params, const Scanner::Location& params_loc,
-    FormalParameterErrorLocations* error_locs, bool* is_rest, bool* ok) {
+    bool* is_rest, Scanner::Location* duplicate_loc, bool* ok) {
   // Too many parentheses around expression:
   //   (( ... )) => ...
   if (params->is_multi_parenthesized()) {
     // TODO(wingo): Make a better message.
     ReportMessageAt(params_loc, "malformed_arrow_function_parameter_list");
     *ok = false;
     return;
   }
 
-  DeclareArrowFunctionParameters(scope, params, params_loc, error_locs, ok);
+  DeclareArrowFunctionParameters(scope, params, params_loc, duplicate_loc, ok);
 }
 
 
 FunctionLiteral* Parser::ParseFunctionLiteral(
     const AstRawString* function_name, Scanner::Location function_name_location,
     bool name_is_strict_reserved, FunctionKind kind, int function_token_pos,
     FunctionLiteral::FunctionType function_type,
     FunctionLiteral::ArityRestriction arity_restriction, bool* ok) {
   // Function ::
   //   '(' FormalParameterList? ')' '{' FunctionBody '}'
@@ skipping 52 matching lines @@
   Scope* scope = function_type == FunctionLiteral::DECLARATION &&
                          is_sloppy(language_mode()) &&
                          (original_scope_ == original_declaration_scope ||
                           declaration_scope != original_declaration_scope)
                      ? NewScope(declaration_scope, FUNCTION_SCOPE, kind)
                      : NewScope(scope_, FUNCTION_SCOPE, kind);
   ZoneList<Statement*>* body = NULL;
   int materialized_literal_count = -1;
   int expected_property_count = -1;
   int handler_count = 0;
-  FormalParameterErrorLocations error_locs;
+  ExpressionClassifier formals_classifier;
   FunctionLiteral::EagerCompileHint eager_compile_hint =
       parenthesized_function_ ? FunctionLiteral::kShouldEagerCompile
                               : FunctionLiteral::kShouldLazyCompile;
   bool should_be_used_once_hint = false;
   // Parse function body.
   {
     AstNodeFactory function_factory(ast_value_factory());
     FunctionState function_state(&function_state_, &scope_, scope, kind,
                                  &function_factory);
     scope_->SetScopeName(function_name);
 
     if (is_generator) {
       // For generators, allocating variables in contexts is currently a win
       // because it minimizes the work needed to suspend and resume an
       // activation.
       scope_->ForceContextAllocation();
 
       // Calling a generator returns a generator object.  That object is stored
       // in a temporary variable, a definition that is used by "yield"
       // expressions. This also marks the FunctionState as a generator.
       Variable* temp = scope_->DeclarationScope()->NewTemporary(
           ast_value_factory()->dot_generator_object_string());
       function_state.set_generator_object_variable(temp);
     }
 
     bool has_rest = false;
     Expect(Token::LPAREN, CHECK_OK);
     int start_position = scanner()->location().beg_pos;
     scope_->set_start_position(start_position);
-    num_parameters =
-        ParseFormalParameterList(scope, &error_locs, &has_rest, CHECK_OK);
+    num_parameters = ParseFormalParameterList(scope, &has_rest,
+                                              &formals_classifier, CHECK_OK);
     Expect(Token::RPAREN, CHECK_OK);
     int formals_end_position = scanner()->location().end_pos;
 
     CheckArityRestrictions(num_parameters, arity_restriction, start_position,
                            formals_end_position, CHECK_OK);
 
     Expect(Token::LBRACE, CHECK_OK);
 
     // If we have a named function expression, we add a local variable
     // declaration to the body of the function with the name of the
@@ skipping 102 matching lines @@
         }
       }
     }
 
     // Validate name and parameter names. We can do this only after parsing the
     // function, since the function can declare itself strict.
     CheckFunctionName(language_mode(), kind, function_name,
                       name_is_strict_reserved, function_name_location,
                       CHECK_OK);
     const bool use_strict_params = has_rest || IsConciseMethod(kind);
-    CheckFunctionParameterNames(language_mode(), use_strict_params, error_locs,
-                                CHECK_OK);
+    const bool allow_duplicate_parameters =

[arv (Not doing code reviews), 2015/05/13 14:13:05]

Does this handle patterns?

// sloppy
function f(x  // ok
function f(x, x  // ok
function f(x, x, {  // no longer ok
function f(x, x =  // no longer ok

[wingo, 2015/05/15 12:30:30]

I guess it should but it doesn't.  In a similar way, duplicate bindings in
patterns are not yet detected, at least not in the preparser.  That will be
tricky to do given that a previous attempt to use the DuplicateFinder for both
the parser and the preparser regressed performance.  Perhaps changing the
DuplicateFinder to allocate in the zone instead of via malloc would improve
things, though.  In any case it's a problem we will need to solve for arrow
functions too:

  (x, x) => 10 // error, but currently not caught by the preparser

+        is_sloppy(language_mode()) && !use_strict_params;
+    ValidateFormalParameters(&formals_classifier, language_mode(),
+                             allow_duplicate_parameters, CHECK_OK);
 
     if (is_strict(language_mode())) {
       CheckStrictOctalLiteral(scope->start_position(), scope->end_position(),
                               CHECK_OK);
       CheckConflictingVarDeclarations(scope, CHECK_OK);
     }
   }
 
+  bool has_duplicate_parameters =
+      !formals_classifier.is_valid_formal_parameter_list_without_duplicates();
   FunctionLiteral::ParameterFlag duplicate_parameters =
-      error_locs.duplicate.IsValid() ? FunctionLiteral::kHasDuplicateParameters
-                                     : FunctionLiteral::kNoDuplicateParameters;
+      has_duplicate_parameters ? FunctionLiteral::kHasDuplicateParameters
+                               : FunctionLiteral::kNoDuplicateParameters;
 
   FunctionLiteral* function_literal = factory()->NewFunctionLiteral(
       function_name, ast_value_factory(), scope, body,
       materialized_literal_count, expected_property_count, handler_count,
       num_parameters, duplicate_parameters, function_type,
       FunctionLiteral::kIsFunction, eager_compile_hint, kind, pos);
   function_literal->set_function_token_position(function_token_pos);
   if (should_be_used_once_hint)
     function_literal->set_should_be_used_once_hint();
 
@@ skipping 1708 matching lines @@
 
 Expression* Parser::SpreadCallNew(Expression* function,
                                   ZoneList<v8::internal::Expression*>* args,
                                   int pos) {
   args->InsertAt(0, function, zone());
 
   return factory()->NewCallRuntime(
       ast_value_factory()->reflect_construct_string(), NULL, args, pos);
 }
 } }  // namespace v8::internal