Revision: 19197
Author: [email protected]
Date: Fri Feb 7 12:44:45 2014 UTC
Log: 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
[email protected]
Review URL: https://codereview.chromium.org/157453003
http://code.google.com/p/v8/source/detail?r=19197
Modified:
/branches/bleeding_edge/src/parser.cc
/branches/bleeding_edge/src/preparser.cc
/branches/bleeding_edge/src/preparser.h
/branches/bleeding_edge/test/cctest/test-parsing.cc
=======================================
--- /branches/bleeding_edge/src/parser.cc Fri Feb 7 10:47:01 2014 UTC
+++ /branches/bleeding_edge/src/parser.cc Fri Feb 7 12:44:45 2014 UTC
@@ -4068,8 +4068,12 @@
// '(' (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 @@
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 @@
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 @@
*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<const char*>::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<const char*>::empty());
*ok = false;
return NULL;
=======================================
--- /branches/bleeding_edge/src/preparser.cc Fri Feb 7 10:47:01 2014 UTC
+++ /branches/bleeding_edge/src/preparser.cc Fri Feb 7 12:44:45 2014 UTC
@@ -75,9 +75,6 @@
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::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 @@
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 @@
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 @@
}
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 @@
*ok = false;
return Expression::Default();
}
+ if (eval_args_error_loc.IsValid()) {
+ ReportMessageAt(eval_args_error_loc, "strict_eval_arguments",
+ Vector<const char*>::empty());
+ *ok = false;
+ return Expression::Default();
+ }
+ if (dupe_error_loc.IsValid()) {
+ ReportMessageAt(dupe_error_loc, "strict_param_dupe",
+ Vector<const char*>::empty());
+ *ok = false;
+ return Expression::Default();
+ }
+ if (reserved_error_loc.IsValid()) {
+ ReportMessageAt(reserved_error_loc, "unexpected_strict_reserved",
+ Vector<const char*>::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 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() &&
@@ -1488,58 +1518,6 @@
return Identifier::Default();
}
}
-
-
-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
=======================================
--- /branches/bleeding_edge/src/preparser.h Fri Feb 7 10:47:01 2014 UTC
+++ /branches/bleeding_edge/src/preparser.h Fri Feb 7 12:44:45 2014 UTC
@@ -243,8 +243,6 @@
: 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 @@
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_;
};
=======================================
--- /branches/bleeding_edge/test/cctest/test-parsing.cc Fri Feb 7 10:28:00
2014 UTC
+++ /branches/bleeding_edge/test/cctest/test-parsing.cc Fri Feb 7 12:44:45
2014 UTC
@@ -1940,3 +1940,40 @@
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);
+}
--
--
v8-dev mailing list
[email protected]
http://groups.google.com/group/v8-dev
---
You received this message because you are subscribed to the Google Groups "v8-dev" group.
To unsubscribe from this group and stop receiving emails from it, send an email
to [email protected].
For more options, visit https://groups.google.com/groups/opt_out.
