From: wingo Date: Tue, 21 Apr 2015 11:09:53 +0000 (-0700) Subject: Factor formal argument parsing into ParserBase X-Git-Tag: upstream/4.7.83~3080 X-Git-Url: http://review.tizen.org/git/?a=commitdiff_plain;h=636cb4f365afe93de37685a69a9b8bfd16a9f70f;p=platform%2Fupstream%2Fv8.git Factor formal argument parsing into ParserBase This commit is a precursor to making lazy arrow function parsing use similar logic to function(){} argument parsing. Originally landed in these three CLs: https://codereview.chromium.org/1078093002 https://codereview.chromium.org/1083623002 https://codereview.chromium.org/1083953002 These were rolled out due to a performance regression on CodeLoad. This patchset will fix that by avoiding creation of a DuplicateFinder in the full parser. R=marja@chromium.org BUG= LOG=N Review URL: https://codereview.chromium.org/1100713002 Cr-Commit-Position: refs/heads/master@{#27960} --- diff --git a/src/messages.js b/src/messages.js index 6dece2e..048732d 100644 --- a/src/messages.js +++ b/src/messages.js @@ -164,7 +164,9 @@ var kMessages = { array_not_subclassable: ["Subclassing Arrays is not currently supported."], for_in_loop_initializer: ["for-in loop variable declaration may not have an initializer."], for_of_loop_initializer: ["for-of loop variable declaration may not have an initializer."], - for_inof_loop_multi_bindings: ["Invalid left-hand side in ", "%0", " loop: Must have a single binding."] + for_inof_loop_multi_bindings: ["Invalid left-hand side in ", "%0", " loop: Must have a single binding."], + bad_getter_arity: ["Getter must not have any formal parameters."], + bad_setter_arity: ["Setter must have exactly one formal parameter."] }; diff --git a/src/parser.cc b/src/parser.cc index fb52eef..b7bead7 100644 --- a/src/parser.cc +++ b/src/parser.cc @@ -3734,8 +3734,7 @@ Handle CompileTimeValue::GetElements(Handle value) { bool CheckAndDeclareArrowParameter(ParserTraits* traits, Expression* expression, Scope* scope, int* num_params, - Scanner::Location* undefined_loc, - Scanner::Location* dupe_loc) { + FormalParameterErrorLocations* locs) { // Case for empty parameter lists: // () => ... if (expression == NULL) return true; @@ -3754,21 +3753,24 @@ bool CheckAndDeclareArrowParameter(ParserTraits* traits, Expression* expression, if (traits->IsEvalOrArguments(raw_name) || traits->IsFutureStrictReserved(raw_name)) return false; - if (traits->IsUndefined(raw_name) && !undefined_loc->IsValid()) { - *undefined_loc = Scanner::Location( + if (traits->IsUndefined(raw_name) && !locs->undefined.IsValid()) { + locs->undefined = Scanner::Location( expression->position(), expression->position() + raw_name->length()); } - if (scope->IsDeclared(raw_name)) { - *dupe_loc = Scanner::Location( - expression->position(), expression->position() + raw_name->length()); - return false; - } // When the variable was seen, it was recorded as unresolved in the outer // scope. But it's really not unresolved. scope->outer_scope()->RemoveUnresolved(expression->AsVariableProxy()); - scope->DeclareParameter(raw_name, VAR); + bool is_rest = false; + bool is_duplicate = false; + scope->DeclareParameter(raw_name, VAR, is_rest, &is_duplicate); + if (is_duplicate) { + locs->duplicate = Scanner::Location( + expression->position(), expression->position() + raw_name->length()); + return false; + } + ++(*num_params); return true; } @@ -3782,9 +3784,9 @@ bool CheckAndDeclareArrowParameter(ParserTraits* traits, Expression* expression, return false; return CheckAndDeclareArrowParameter(traits, binop->left(), scope, - num_params, undefined_loc, dupe_loc) && + num_params, locs) && CheckAndDeclareArrowParameter(traits, binop->right(), scope, - num_params, undefined_loc, dupe_loc); + num_params, locs); } // Any other kind of expression is not a valid parameter list. @@ -3793,15 +3795,15 @@ bool CheckAndDeclareArrowParameter(ParserTraits* traits, Expression* expression, int ParserTraits::DeclareArrowParametersFromExpression( - Expression* expression, Scope* scope, Scanner::Location* undefined_loc, - Scanner::Location* dupe_loc, bool* ok) { + Expression* expression, Scope* scope, FormalParameterErrorLocations* locs, + bool* ok) { int num_params = 0; // Always reset the flag: It only needs to be set for the first expression // parsed as arrow function parameter list, because only top-level functions // are parsed lazily. parser_->parsing_lazy_arrow_parameters_ = false; - *ok = CheckAndDeclareArrowParameter(this, expression, scope, &num_params, - undefined_loc, dupe_loc); + *ok = + CheckAndDeclareArrowParameter(this, expression, scope, &num_params, locs); return num_params; } @@ -3875,8 +3877,7 @@ FunctionLiteral* Parser::ParseFunctionLiteral( int materialized_literal_count = -1; int expected_property_count = -1; int handler_count = 0; - FunctionLiteral::ParameterFlag duplicate_parameters = - FunctionLiteral::kNoDuplicateParameters; + FormalParameterErrorLocations error_locs; FunctionLiteral::IsParenthesizedFlag parenthesized = parenthesized_function_ ? FunctionLiteral::kIsParenthesized : FunctionLiteral::kNotParenthesized; @@ -3901,77 +3902,17 @@ FunctionLiteral* Parser::ParseFunctionLiteral( function_state.set_generator_object_variable(temp); } - // FormalParameterList :: - // '(' (Identifier)*[','] ')' + bool has_rest = false; Expect(Token::LPAREN, CHECK_OK); - scope->set_start_position(scanner()->location().beg_pos); - - // We don't yet know if the function will be strict, so we cannot yet - // produce errors for parameter names or duplicates. However, we remember - // the locations of these errors if they occur and produce the errors later. - Scanner::Location eval_args_loc = Scanner::Location::invalid(); - Scanner::Location dupe_loc = Scanner::Location::invalid(); - Scanner::Location reserved_loc = Scanner::Location::invalid(); - - // Similarly for strong mode. - Scanner::Location undefined_loc = Scanner::Location::invalid(); - - bool is_rest = false; - bool done = arity_restriction == FunctionLiteral::GETTER_ARITY || - (peek() == Token::RPAREN && - arity_restriction != FunctionLiteral::SETTER_ARITY); - while (!done) { - bool is_strict_reserved = false; - is_rest = peek() == Token::ELLIPSIS && allow_harmony_rest_params(); - if (is_rest) { - Consume(Token::ELLIPSIS); - } - - const AstRawString* param_name = - ParseIdentifierOrStrictReservedWord(&is_strict_reserved, CHECK_OK); - - // Store locations for possible future error reports. - if (!eval_args_loc.IsValid() && IsEvalOrArguments(param_name)) { - eval_args_loc = scanner()->location(); - } - if (!undefined_loc.IsValid() && IsUndefined(param_name)) { - undefined_loc = scanner()->location(); - } - if (!reserved_loc.IsValid() && is_strict_reserved) { - reserved_loc = scanner()->location(); - } - if (!dupe_loc.IsValid() && - scope_->IsDeclaredParameter(param_name)) { - duplicate_parameters = FunctionLiteral::kHasDuplicateParameters; - dupe_loc = scanner()->location(); - } - - Variable* var = scope_->DeclareParameter(param_name, VAR, is_rest); - if (is_sloppy(scope->language_mode())) { - // TODO(sigurds) Mark every parameter as maybe assigned. This is a - // conservative approximation necessary to account for parameters - // that are assigned via the arguments array. - var->set_maybe_assigned(); - } - - num_parameters++; - if (num_parameters > Code::kMaxArguments) { - ReportMessage("too_many_parameters"); - *ok = false; - return NULL; - } - if (arity_restriction == FunctionLiteral::SETTER_ARITY) break; - done = (peek() == Token::RPAREN); - if (!done) { - if (is_rest) { - ReportMessageAt(scanner()->peek_location(), "param_after_rest"); - *ok = false; - return NULL; - } - Expect(Token::COMMA, 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); 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); @@ -4053,10 +3994,9 @@ FunctionLiteral* Parser::ParseFunctionLiteral( CheckFunctionName(language_mode(), kind, function_name, name_is_strict_reserved, function_name_location, CHECK_OK); - const bool use_strict_params = is_rest || IsConciseMethod(kind); - CheckFunctionParameterNames(language_mode(), use_strict_params, - eval_args_loc, undefined_loc, dupe_loc, - reserved_loc, CHECK_OK); + const bool use_strict_params = has_rest || IsConciseMethod(kind); + CheckFunctionParameterNames(language_mode(), use_strict_params, error_locs, + CHECK_OK); if (is_strict(language_mode())) { CheckStrictOctalLiteral(scope->start_position(), scope->end_position(), @@ -4075,6 +4015,10 @@ FunctionLiteral* Parser::ParseFunctionLiteral( } } + FunctionLiteral::ParameterFlag duplicate_parameters = + error_locs.duplicate.IsValid() ? FunctionLiteral::kHasDuplicateParameters + : FunctionLiteral::kNoDuplicateParameters; + FunctionLiteral* function_literal = factory()->NewFunctionLiteral( function_name, ast_value_factory(), scope, body, materialized_literal_count, expected_property_count, handler_count, diff --git a/src/parser.h b/src/parser.h index 8d557ae..a985d71 100644 --- a/src/parser.h +++ b/src/parser.h @@ -557,6 +557,8 @@ class ParserTraits { typedef ObjectLiteral::Property* ObjectLiteralProperty; typedef ZoneList* ExpressionList; typedef ZoneList* PropertyList; + typedef const v8::internal::AstRawString* FormalParameter; + typedef Scope FormalParameterScope; typedef ZoneList* StatementList; // For constructing objects returned by the traversing functions. @@ -705,6 +707,7 @@ class ParserTraits { static ZoneList* NullExpressionList() { return NULL; } + static const AstRawString* EmptyFormalParameter() { return NULL; } // Non-NULL empty string. V8_INLINE const AstRawString* EmptyIdentifierString(); @@ -745,10 +748,22 @@ class ParserTraits { // Utility functions int DeclareArrowParametersFromExpression(Expression* expression, Scope* scope, - Scanner::Location* undefined_loc, - Scanner::Location* dupe_loc, + FormalParameterErrorLocations* locs, bool* ok); + bool DeclareFormalParameter(Scope* scope, const AstRawString* name, + bool is_rest) { + bool is_duplicate = false; + Variable* var = scope->DeclareParameter(name, VAR, is_rest, &is_duplicate); + if (is_sloppy(scope->language_mode())) { + // TODO(sigurds) Mark every parameter as maybe assigned. This is a + // conservative approximation necessary to account for parameters + // that are assigned via the arguments array. + var->set_maybe_assigned(); + } + return is_duplicate; + } + // Temporary glue; these functions will move to ParserBase. Expression* ParseV8Intrinsic(bool* ok); FunctionLiteral* ParseFunctionLiteral( diff --git a/src/preparser.cc b/src/preparser.cc index 4371cdb..4a8c37d 100644 --- a/src/preparser.cc +++ b/src/preparser.cc @@ -926,62 +926,23 @@ PreParser::Expression PreParser::ParseFunctionLiteral( PreParserFactory factory(NULL); FunctionState function_state(&function_state_, &scope_, function_scope, kind, &factory); - // FormalParameterList :: - // '(' (Identifier)*[','] ')' - Expect(Token::LPAREN, CHECK_OK); - int start_position = position(); - DuplicateFinder duplicate_finder(scanner()->unicode_cache()); - // We don't yet know if the function will be strict, so we cannot yet produce - // errors for parameter names or duplicates. However, we remember the - // locations of these errors if they occur and produce the errors later. - Scanner::Location eval_args_loc = Scanner::Location::invalid(); - Scanner::Location dupe_loc = Scanner::Location::invalid(); - Scanner::Location reserved_loc = Scanner::Location::invalid(); - - // Similarly for strong mode. - Scanner::Location undefined_loc = Scanner::Location::invalid(); + FormalParameterErrorLocations error_locs; bool is_rest = false; - bool done = arity_restriction == FunctionLiteral::GETTER_ARITY || - (peek() == Token::RPAREN && - arity_restriction != FunctionLiteral::SETTER_ARITY); - while (!done) { - bool is_strict_reserved = false; - is_rest = peek() == Token::ELLIPSIS && allow_harmony_rest_params(); - if (is_rest) { - Consume(Token::ELLIPSIS); - } - - Identifier param_name = - ParseIdentifierOrStrictReservedWord(&is_strict_reserved, CHECK_OK); - if (!eval_args_loc.IsValid() && param_name.IsEvalOrArguments()) { - eval_args_loc = scanner()->location(); - } - if (!undefined_loc.IsValid() && param_name.IsUndefined()) { - undefined_loc = scanner()->location(); - } - if (!reserved_loc.IsValid() && is_strict_reserved) { - reserved_loc = scanner()->location(); - } - - int prev_value = scanner()->FindSymbol(&duplicate_finder, 1); - - if (!dupe_loc.IsValid() && prev_value != 0) { - dupe_loc = scanner()->location(); - } - - if (arity_restriction == FunctionLiteral::SETTER_ARITY) break; - done = (peek() == Token::RPAREN); - if (!done) { - if (is_rest) { - ReportMessageAt(scanner()->peek_location(), "param_after_rest"); - *ok = false; - return Expression::Default(); - } - Expect(Token::COMMA, CHECK_OK); - } + Expect(Token::LPAREN, CHECK_OK); + int start_position = scanner()->location().beg_pos; + function_scope->set_start_position(start_position); + int num_parameters; + { + DuplicateFinder duplicate_finder(scanner()->unicode_cache()); + num_parameters = ParseFormalParameterList(&duplicate_finder, &error_locs, + &is_rest, 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); // See Parser::ParseFunctionLiteral for more information about lazy parsing // and lazy compilation. @@ -1002,8 +963,8 @@ PreParser::Expression PreParser::ParseFunctionLiteral( CheckFunctionName(language_mode(), kind, function_name, name_is_strict_reserved, function_name_location, CHECK_OK); const bool use_strict_params = is_rest || IsConciseMethod(kind); - CheckFunctionParameterNames(language_mode(), use_strict_params, eval_args_loc, - undefined_loc, dupe_loc, reserved_loc, CHECK_OK); + CheckFunctionParameterNames(language_mode(), use_strict_params, error_locs, + CHECK_OK); if (is_strict(language_mode())) { int end_position = scanner()->location().end_pos; diff --git a/src/preparser.h b/src/preparser.h index 33c737f..e112e4a 100644 --- a/src/preparser.h +++ b/src/preparser.h @@ -17,6 +17,26 @@ 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 { + 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; + 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: @@ -51,6 +71,8 @@ namespace internal { // typedef Literal; // typedef ExpressionList; // typedef PropertyList; +// typedef FormalParameter; +// typedef FormalParameterScope; // // For constructing objects returned by the traversing functions. // typedef Factory; // }; @@ -63,6 +85,8 @@ class ParserBase : public Traits { // 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; @@ -495,31 +519,28 @@ class ParserBase : public Traits { // 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; } @@ -606,6 +627,16 @@ class ParserBase : public Traits { void AddTemplateExpression(ExpressionT); ExpressionT ParseSuperExpression(bool is_new, bool* ok); + void ParseFormalParameter(FormalParameterScopeT* scope, + FormalParameterErrorLocations* locs, bool is_rest, + bool* ok); + int ParseFormalParameterList(FormalParameterScopeT* scope, + FormalParameterErrorLocations* locs, + bool* has_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. @@ -911,7 +942,7 @@ class PreParserExpression { 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; @@ -921,7 +952,7 @@ class PreParserExpression { } 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; @@ -1040,20 +1071,24 @@ class PreParserExpression { }; -// PreParserExpressionList doesn't actually store the expressions because -// PreParser doesn't need to. -class PreParserExpressionList { +// The pre-parser doesn't need to build lists of expressions, identifiers, or +// the like. +template +class PreParserList { public: // These functions make list->Add(some_expression) work (and do nothing). - PreParserExpressionList() : length_(0) {} - PreParserExpressionList* operator->() { return this; } - void Add(PreParserExpression, void*) { ++length_; } + PreParserList() : length_(0) {} + PreParserList* operator->() { return this; } + void Add(T, void*) { ++length_; } int length() const { return length_; } private: int length_; }; +typedef PreParserList PreParserExpressionList; + + class PreParserStatement { public: static PreParserStatement Default() { @@ -1118,16 +1153,7 @@ class PreParserStatement { }; - -// PreParserStatementList doesn't actually store the statements because -// the PreParser does not need them. -class PreParserStatementList { - public: - // These functions make list->Add(some_expression) work as no-ops. - PreParserStatementList() {} - PreParserStatementList* operator->() { return this; } - void Add(PreParserStatement, void*) {} -}; +typedef PreParserList PreParserStatementList; class PreParserFactory { @@ -1292,6 +1318,8 @@ class PreParserTraits { 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. @@ -1526,13 +1554,12 @@ class PreParserTraits { 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; } @@ -1560,6 +1587,10 @@ class PreParserTraits { return !tag.IsNoTemplateTag(); } + V8_INLINE bool DeclareFormalParameter(DuplicateFinder* scope, + PreParserIdentifier param, + bool is_rest); + void CheckConflictingVarDeclarations(Scope* scope, bool* ok) {} // Temporary glue; these functions will move to ParserBase. @@ -1748,6 +1779,13 @@ 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; +} + + PreParserStatementList PreParser::ParseEagerFunctionBody( PreParserIdentifier function_name, int pos, Variable* fvar, Token::Value fvar_init_op, FunctionKind kind, bool* ok) { @@ -3041,6 +3079,101 @@ ParserBase::ParseMemberExpressionContinuation(ExpressionT expression, template +void ParserBase::ParseFormalParameter( + FormalParameterScopeT* scope, FormalParameterErrorLocations* locs, + bool is_rest, bool* ok) { + // FormalParameter[Yield,GeneratorParameter] : + // BindingElement[?Yield, ?GeneratorParameter] + bool is_strict_reserved; + IdentifierT name = + ParseIdentifierOrStrictReservedWord(&is_strict_reserved, ok); + if (!*ok) return; + + // Store locations for possible future error reports. + if (!locs->eval_or_arguments.IsValid() && this->IsEvalOrArguments(name)) { + locs->eval_or_arguments = scanner()->location(); + } + if (!locs->undefined.IsValid() && this->IsUndefined(name)) { + locs->undefined = scanner()->location(); + } + if (!locs->reserved.IsValid() && is_strict_reserved) { + locs->reserved = scanner()->location(); + } + bool was_declared = Traits::DeclareFormalParameter(scope, name, is_rest); + if (!locs->duplicate.IsValid() && was_declared) { + locs->duplicate = scanner()->location(); + } +} + + +template +int ParserBase::ParseFormalParameterList( + FormalParameterScopeT* scope, 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] + + int parameter_count = 0; + + if (peek() != Token::RPAREN) { + do { + if (++parameter_count > Code::kMaxArguments) { + ReportMessage("too_many_parameters"); + *ok = false; + return -1; + } + *is_rest = allow_harmony_rest_params() && Check(Token::ELLIPSIS); + ParseFormalParameter(scope, locs, *is_rest, ok); + if (!*ok) return -1; + } while (!*is_rest && Check(Token::COMMA)); + + if (*is_rest && peek() == Token::COMMA) { + ReportMessageAt(scanner()->peek_location(), "param_after_rest"); + *ok = false; + return -1; + } + } + + return parameter_count; +} + + +template +void ParserBase::CheckArityRestrictions( + int param_count, FunctionLiteral::ArityRestriction arity_restriction, + int formals_start_pos, int formals_end_pos, bool* ok) { + switch (arity_restriction) { + case FunctionLiteral::GETTER_ARITY: + if (param_count != 0) { + ReportMessageAt(Scanner::Location(formals_start_pos, formals_end_pos), + "bad_getter_arity"); + *ok = false; + } + break; + case FunctionLiteral::SETTER_ARITY: + if (param_count != 1) { + ReportMessageAt(Scanner::Location(formals_start_pos, formals_end_pos), + "bad_setter_arity"); + *ok = false; + } + break; + default: + break; + } +} + +template typename ParserBase::ExpressionT ParserBase::ParseArrowFunctionLiteral(int start_pos, ExpressionT params_ast, @@ -3066,11 +3199,9 @@ ParserBase::ParseArrowFunctionLiteral(int start_pos, 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), @@ -3078,10 +3209,10 @@ ParserBase::ParseArrowFunctionLiteral(int start_pos, 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()), @@ -3127,15 +3258,13 @@ ParserBase::ParseArrowFunctionLiteral(int start_pos, 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())) { diff --git a/src/scopes.cc b/src/scopes.cc index 364f2f1..be991a1 100644 --- a/src/scopes.cc +++ b/src/scopes.cc @@ -452,7 +452,7 @@ Variable* Scope::Lookup(const AstRawString* name) { Variable* Scope::DeclareParameter(const AstRawString* name, VariableMode mode, - bool is_rest) { + bool is_rest, bool* is_duplicate) { DCHECK(!already_resolved()); DCHECK(is_function_scope()); Variable* var = variables_.Declare(this, name, mode, Variable::NORMAL, @@ -462,6 +462,8 @@ Variable* Scope::DeclareParameter(const AstRawString* name, VariableMode mode, rest_parameter_ = var; rest_index_ = num_parameters(); } + // TODO(wingo): Avoid O(n^2) check. + *is_duplicate = IsDeclaredParameter(name); params_.Add(var, zone()); return var; } diff --git a/src/scopes.h b/src/scopes.h index 7b372d9..409fa2f 100644 --- a/src/scopes.h +++ b/src/scopes.h @@ -125,7 +125,7 @@ class Scope: public ZoneObject { // 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_rest = false); + bool is_rest, bool* is_duplicate); // Declare a local variable in this scope. If the variable has been // declared before, the previously declared variable is returned. diff --git a/test/message/formal-parameters-bad-rest.js b/test/message/formal-parameters-bad-rest.js new file mode 100644 index 0000000..c67e1de --- /dev/null +++ b/test/message/formal-parameters-bad-rest.js @@ -0,0 +1,7 @@ +// Copyright 2015 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. +// +// Flags: --harmony-rest-parameters + +function foo(...b, a) { return a } diff --git a/test/message/formal-parameters-bad-rest.out b/test/message/formal-parameters-bad-rest.out new file mode 100644 index 0000000..562b6ad --- /dev/null +++ b/test/message/formal-parameters-bad-rest.out @@ -0,0 +1,4 @@ +*%(basename)s:7: SyntaxError: Rest parameter must be last formal parameter +function foo(...b, a) { return a } + ^ +SyntaxError: Rest parameter must be last formal parameter diff --git a/test/message/formal-parameters-strict-body.js b/test/message/formal-parameters-strict-body.js new file mode 100644 index 0000000..c5af740 --- /dev/null +++ b/test/message/formal-parameters-strict-body.js @@ -0,0 +1,5 @@ +// Copyright 2015 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. + +function foo(b, eval) { "use strict"; return b } diff --git a/test/message/formal-parameters-strict-body.out b/test/message/formal-parameters-strict-body.out new file mode 100644 index 0000000..bb0d7e0 --- /dev/null +++ b/test/message/formal-parameters-strict-body.out @@ -0,0 +1,4 @@ +*%(basename)s:5: SyntaxError: Unexpected eval or arguments in strict mode +function foo(b, eval) { "use strict"; return b } + ^^^^ +SyntaxError: Unexpected eval or arguments in strict mode diff --git a/test/message/formal-parameters-trailing-comma.js b/test/message/formal-parameters-trailing-comma.js new file mode 100644 index 0000000..09cdb89 --- /dev/null +++ b/test/message/formal-parameters-trailing-comma.js @@ -0,0 +1,5 @@ +// Copyright 2015 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. + +function foo(b, a, a,) { return a } diff --git a/test/message/formal-parameters-trailing-comma.out b/test/message/formal-parameters-trailing-comma.out new file mode 100644 index 0000000..4b930ec --- /dev/null +++ b/test/message/formal-parameters-trailing-comma.out @@ -0,0 +1,4 @@ +*%(basename)s:5: SyntaxError: Unexpected token ) +function foo(b, a, a,) { return a } + ^ +SyntaxError: Unexpected token ) diff --git a/test/message/strict-formal-parameters.js b/test/message/strict-formal-parameters.js new file mode 100644 index 0000000..a6c7531 --- /dev/null +++ b/test/message/strict-formal-parameters.js @@ -0,0 +1,6 @@ +// Copyright 2015 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. + +"use strict"; +function foo(b, a, a, d) { return a } diff --git a/test/message/strict-formal-parameters.out b/test/message/strict-formal-parameters.out new file mode 100644 index 0000000..3648d38 --- /dev/null +++ b/test/message/strict-formal-parameters.out @@ -0,0 +1,4 @@ +*%(basename)s:6: SyntaxError: Strict mode function may not have duplicate parameter names +function foo(b, a, a, d) { return a } + ^ +SyntaxError: Strict mode function may not have duplicate parameter names