From: marja@chromium.org Date: Fri, 7 Feb 2014 12:44:45 +0000 (+0000) Subject: Unify function parameter checking in PreParser and Parser. X-Git-Tag: upstream/4.7.83~10816 X-Git-Url: http://review.tizen.org/git/?a=commitdiff_plain;h=caa727711f34d7039818e86b6ecd8753a566c992;p=platform%2Fupstream%2Fv8.git Unify function parameter checking in PreParser and Parser. This also makes the "delayed strict mode violations" mechanism in PreParser unnecessary. The attached tests didn't pass before this CL but now they do. BUG=v8:3126 LOG=N R=mstarzinger@chromium.org Review URL: https://codereview.chromium.org/157453003 git-svn-id: http://v8.googlecode.com/svn/branches/bleeding_edge@19197 ce2b1a6d-e550-0410-aec6-3dcde31c8c00 --- diff --git a/src/parser.cc b/src/parser.cc index 21beacc..54a83e8 100644 --- a/src/parser.cc +++ b/src/parser.cc @@ -4068,8 +4068,12 @@ FunctionLiteral* Parser::ParseFunctionLiteral( // '(' (Identifier)*[','] ')' Expect(Token::LPAREN, CHECK_OK); scope->set_start_position(scanner().location().beg_pos); - Scanner::Location name_loc = Scanner::Location::invalid(); - Scanner::Location dupe_loc = Scanner::Location::invalid(); + + // 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_error_log = Scanner::Location::invalid(); + Scanner::Location dupe_error_loc = Scanner::Location::invalid(); Scanner::Location reserved_loc = Scanner::Location::invalid(); bool done = (peek() == Token::RPAREN); @@ -4079,16 +4083,16 @@ FunctionLiteral* Parser::ParseFunctionLiteral( ParseIdentifierOrStrictReservedWord(&is_strict_reserved, CHECK_OK); // Store locations for possible future error reports. - if (!name_loc.IsValid() && IsEvalOrArguments(param_name)) { - name_loc = scanner().location(); - } - if (!dupe_loc.IsValid() && top_scope_->IsDeclared(param_name)) { - duplicate_parameters = FunctionLiteral::kHasDuplicateParameters; - dupe_loc = scanner().location(); + if (!eval_args_error_log.IsValid() && IsEvalOrArguments(param_name)) { + eval_args_error_log = scanner().location(); } if (!reserved_loc.IsValid() && is_strict_reserved) { reserved_loc = scanner().location(); } + if (!dupe_error_loc.IsValid() && top_scope_->IsDeclared(param_name)) { + duplicate_parameters = FunctionLiteral::kHasDuplicateParameters; + dupe_error_loc = scanner().location(); + } top_scope_->DeclareParameter(param_name, VAR); num_parameters++; @@ -4256,7 +4260,8 @@ FunctionLiteral* Parser::ParseFunctionLiteral( scope->set_end_position(scanner().location().end_pos); } - // Validate strict mode. + // Validate strict mode. We can do this only after parsing the function, + // since the function can declare itself strict. if (!top_scope_->is_classic_mode()) { if (IsEvalOrArguments(function_name)) { ReportMessageAt(function_name_location, @@ -4271,14 +4276,14 @@ FunctionLiteral* Parser::ParseFunctionLiteral( *ok = false; return NULL; } - if (name_loc.IsValid()) { - ReportMessageAt(name_loc, "strict_eval_arguments", + if (eval_args_error_log.IsValid()) { + ReportMessageAt(eval_args_error_log, "strict_eval_arguments", Vector::empty()); *ok = false; return NULL; } - if (dupe_loc.IsValid()) { - ReportMessageAt(dupe_loc, "strict_param_dupe", + if (dupe_error_loc.IsValid()) { + ReportMessageAt(dupe_error_loc, "strict_param_dupe", Vector::empty()); *ok = false; return NULL; diff --git a/src/preparser.cc b/src/preparser.cc index 4caf906..49d6cd2 100644 --- a/src/preparser.cc +++ b/src/preparser.cc @@ -75,9 +75,6 @@ PreParser::PreParseResult PreParser::PreParseLazyFunction( if (!scope_->is_classic_mode()) { int end_pos = scanner()->location().end_pos; CheckOctalLiteral(start_position, end_pos, &ok); - if (ok) { - CheckDelayedStrictModeViolation(start_position, end_pos, &ok); - } } } return kPreParseSuccess; @@ -1295,7 +1292,7 @@ PreParser::Arguments PreParser::ParseArguments(bool* ok) { } PreParser::Expression PreParser::ParseFunctionLiteral( - Identifier name, + Identifier function_name, Scanner::Location function_name_location, bool name_is_strict_reserved, bool is_generator, @@ -1314,8 +1311,23 @@ PreParser::Expression PreParser::ParseFunctionLiteral( int start_position = position(); bool done = (peek() == Token::RPAREN); 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_error_loc = Scanner::Location::invalid(); + Scanner::Location dupe_error_loc = Scanner::Location::invalid(); + Scanner::Location reserved_error_loc = Scanner::Location::invalid(); while (!done) { - ParseIdentifier(kDontAllowEvalOrArguments, CHECK_OK); + bool is_strict_reserved = false; + Identifier param_name = + ParseIdentifierOrStrictReservedWord(&is_strict_reserved, CHECK_OK); + if (!eval_args_error_loc.IsValid() && param_name.IsEvalOrArguments()) { + eval_args_error_loc = scanner()->location(); + } + if (!reserved_error_loc.IsValid() && is_strict_reserved) { + reserved_error_loc = scanner()->location(); + } + int prev_value; if (scanner()->is_literal_ascii()) { prev_value = @@ -1325,11 +1337,10 @@ PreParser::Expression PreParser::ParseFunctionLiteral( duplicate_finder.AddUtf16Symbol(scanner()->literal_utf16_string(), 1); } - if (prev_value != 0) { - SetStrictModeViolation(scanner()->location(), - "strict_param_dupe", - CHECK_OK); + if (!dupe_error_loc.IsValid() && prev_value != 0) { + dupe_error_loc = scanner()->location(); } + done = (peek() == Token::RPAREN); if (!done) { Expect(Token::COMMA, CHECK_OK); @@ -1353,9 +1364,10 @@ PreParser::Expression PreParser::ParseFunctionLiteral( } Expect(Token::RBRACE, CHECK_OK); - // Validate strict mode. + // Validate strict mode. We can do this only after parsing the function, + // since the function can declare itself strict. if (!scope_->is_classic_mode()) { - if (name.IsEvalOrArguments()) { + if (function_name.IsEvalOrArguments()) { ReportMessageAt(function_name_location, "strict_eval_arguments", NULL); *ok = false; return Expression::Default(); @@ -1366,10 +1378,27 @@ PreParser::Expression PreParser::ParseFunctionLiteral( *ok = false; return Expression::Default(); } + if (eval_args_error_loc.IsValid()) { + ReportMessageAt(eval_args_error_loc, "strict_eval_arguments", + Vector::empty()); + *ok = false; + return Expression::Default(); + } + if (dupe_error_loc.IsValid()) { + ReportMessageAt(dupe_error_loc, "strict_param_dupe", + Vector::empty()); + *ok = false; + return Expression::Default(); + } + if (reserved_error_loc.IsValid()) { + ReportMessageAt(reserved_error_loc, "unexpected_strict_reserved", + Vector::empty()); + *ok = false; + return Expression::Default(); + } int end_position = scanner()->location().end_pos; CheckOctalLiteral(start_position, end_position, CHECK_OK); - CheckDelayedStrictModeViolation(start_position, end_position, CHECK_OK); return Expression::StrictFunction(); } @@ -1475,7 +1504,8 @@ PreParser::Identifier PreParser::ParseIdentifier( PreParser::Identifier name = GetIdentifierSymbol(); if (allow_eval_or_arguments == kDontAllowEvalOrArguments && !scope_->is_classic_mode() && name.IsEvalOrArguments()) { - StrictModeIdentifierViolation(scanner()->location(), name, ok); + ReportMessageAt(scanner()->location(), "strict_eval_arguments", NULL); + *ok = false; } return name; } else if (scope_->is_classic_mode() && @@ -1490,58 +1520,6 @@ PreParser::Identifier PreParser::ParseIdentifier( } -void PreParser::SetStrictModeViolation(Scanner::Location location, - const char* type, - bool* ok) { - if (!scope_->is_classic_mode()) { - ReportMessageAt(location, type, NULL); - *ok = false; - return; - } - // Delay report in case this later turns out to be strict code - // (i.e., for function names and parameters prior to a "use strict" - // directive). - // It's safe to overwrite an existing violation. - // It's either from a function that turned out to be non-strict, - // or it's in the current function (and we just need to report - // one error), or it's in a unclosed nesting function that wasn't - // strict (otherwise we would already be in strict mode). - strict_mode_violation_location_ = location; - strict_mode_violation_type_ = type; -} - - -void PreParser::CheckDelayedStrictModeViolation(int beg_pos, - int end_pos, - bool* ok) { - Scanner::Location location = strict_mode_violation_location_; - if (location.IsValid() && - location.beg_pos > beg_pos && location.end_pos < end_pos) { - ReportMessageAt(location, strict_mode_violation_type_, NULL); - *ok = false; - } -} - - -void PreParser::StrictModeIdentifierViolation(Scanner::Location location, - Identifier identifier, - bool* ok) { - const char* type = "strict_eval_arguments"; - if (identifier.IsFutureReserved()) { - type = "unexpected_reserved"; - } else if (identifier.IsFutureStrictReserved() || identifier.IsYield()) { - type = "unexpected_strict_reserved"; - } - if (!scope_->is_classic_mode()) { - ReportMessageAt(location, type, NULL); - *ok = false; - return; - } - strict_mode_violation_location_ = location; - strict_mode_violation_type_ = type; -} - - // Parses and identifier or a strict mode future reserved word, and indicate // whether it is strict mode future reserved. PreParser::Identifier PreParser::ParseIdentifierOrStrictReservedWord( diff --git a/src/preparser.h b/src/preparser.h index 0f417f1..bcaab74 100644 --- a/src/preparser.h +++ b/src/preparser.h @@ -243,8 +243,6 @@ class PreParser : public ParserBase { : ParserBase(scanner, stack_limit), log_(log), scope_(NULL), - strict_mode_violation_location_(Scanner::Location::invalid()), - strict_mode_violation_type_(NULL), parenthesized_function_(false) { } ~PreParser() {} @@ -655,20 +653,8 @@ class PreParser : public ParserBase { bool CheckInOrOf(bool accept_OF); - void SetStrictModeViolation(Scanner::Location, - const char* type, - bool* ok); - - void CheckDelayedStrictModeViolation(int beg_pos, int end_pos, bool* ok); - - void StrictModeIdentifierViolation(Scanner::Location, - Identifier identifier, - bool* ok); - ParserRecorder* log_; Scope* scope_; - Scanner::Location strict_mode_violation_location_; - const char* strict_mode_violation_type_; bool parenthesized_function_; }; diff --git a/test/cctest/test-parsing.cc b/test/cctest/test-parsing.cc index 4dd7971..85b2f3c 100644 --- a/test/cctest/test-parsing.cc +++ b/test/cctest/test-parsing.cc @@ -1940,3 +1940,40 @@ TEST(DontRegressPreParserDataSizes) { CHECK(!data.has_error()); } } + + +TEST(FunctionDeclaresItselfStrict) { + // Tests that we produce the right kinds of errors when a function declares + // itself strict (we cannot produce there errors as soon as we see the + // offending identifiers, because we don't know at that point whether the + // function is strict or not). + const char* context_data[][2] = { + {"function eval() {", "}"}, + {"function arguments() {", "}"}, + {"function yield() {", "}"}, + {"function interface() {", "}"}, + {"function foo(eval) {", "}"}, + {"function foo(arguments) {", "}"}, + {"function foo(yield) {", "}"}, + {"function foo(interface) {", "}"}, + {"function foo(bar, eval) {", "}"}, + {"function foo(bar, arguments) {", "}"}, + {"function foo(bar, yield) {", "}"}, + {"function foo(bar, interface) {", "}"}, + {"function foo(bar, bar) {", "}"}, + { NULL, NULL } + }; + + const char* strict_statement_data[] = { + "\"use strict\";", + NULL + }; + + const char* non_strict_statement_data[] = { + ";", + NULL + }; + + RunParserSyncTest(context_data, strict_statement_data, kError); + RunParserSyncTest(context_data, non_strict_statement_data, kSuccess); +}