[strong] Implement static restrictions on binding 'undefined' in arrow functions

Implements the strong mode proposal's static restrictions on the use of the
identifier 'undefined', for arrow functions. Assumes these restrictions are
intended to be identical to the restrictions on the use of 'eval and 'arguments'
in strict mode. In addition, Location variables inconsistantly named (e.g.
dupe_error_loc vs dupe_loc) are now consistently named the shorter way.

Baseline: https://codereview.chromium.org/1070633002

BUG=v8:3956
LOG=N

Review URL: https://codereview.chromium.org/1060883004

Cr-Commit-Position: refs/heads/master@{#27756}
This commit is contained in:
conradw 2015-04-10 11:27:05 -07:00 committed by Commit bot
parent 7898475e92
commit 3d5717a71b
6 changed files with 162 additions and 81 deletions

View File

@ -3706,6 +3706,7 @@ Handle<FixedArray> CompileTimeValue::GetElements(Handle<FixedArray> value) {
bool CheckAndDeclareArrowParameter(ParserTraits* traits, Expression* expression,
Scope* scope, int* num_params,
Scanner::Location* undefined_loc,
Scanner::Location* dupe_loc) {
// Case for empty parameter lists:
// () => ...
@ -3725,7 +3726,10 @@ 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(
expression->position(), expression->position() + raw_name->length());
}
if (scope->IsDeclared(raw_name)) {
*dupe_loc = Scanner::Location(
expression->position(), expression->position() + raw_name->length());
@ -3750,9 +3754,9 @@ bool CheckAndDeclareArrowParameter(ParserTraits* traits, Expression* expression,
return false;
return CheckAndDeclareArrowParameter(traits, binop->left(), scope,
num_params, dupe_loc) &&
num_params, undefined_loc, dupe_loc) &&
CheckAndDeclareArrowParameter(traits, binop->right(), scope,
num_params, dupe_loc);
num_params, undefined_loc, dupe_loc);
}
// Any other kind of expression is not a valid parameter list.
@ -3761,15 +3765,15 @@ bool CheckAndDeclareArrowParameter(ParserTraits* traits, Expression* expression,
int ParserTraits::DeclareArrowParametersFromExpression(
Expression* expression, Scope* scope, Scanner::Location* dupe_loc,
bool* ok) {
Expression* expression, Scope* scope, Scanner::Location* undefined_loc,
Scanner::Location* dupe_loc, 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, becauseonly top-level functions
// 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,
dupe_loc);
undefined_loc, dupe_loc);
return num_params;
}
@ -3877,12 +3881,12 @@ FunctionLiteral* Parser::ParseFunctionLiteral(
// 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();
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_error_loc = Scanner::Location::invalid();
Scanner::Location undefined_loc = Scanner::Location::invalid();
bool is_rest = false;
bool done = arity_restriction == FunctionLiteral::GETTER_ARITY ||
@ -3899,19 +3903,19 @@ FunctionLiteral* Parser::ParseFunctionLiteral(
ParseIdentifierOrStrictReservedWord(&is_strict_reserved, CHECK_OK);
// Store locations for possible future error reports.
if (!eval_args_error_loc.IsValid() && IsEvalOrArguments(param_name)) {
eval_args_error_loc = scanner()->location();
if (!eval_args_loc.IsValid() && IsEvalOrArguments(param_name)) {
eval_args_loc = scanner()->location();
}
if (!undefined_error_loc.IsValid() && IsUndefined(param_name)) {
undefined_error_loc = scanner()->location();
if (!undefined_loc.IsValid() && IsUndefined(param_name)) {
undefined_loc = scanner()->location();
}
if (!reserved_error_loc.IsValid() && is_strict_reserved) {
reserved_error_loc = scanner()->location();
if (!reserved_loc.IsValid() && is_strict_reserved) {
reserved_loc = scanner()->location();
}
if (!dupe_error_loc.IsValid() &&
if (!dupe_loc.IsValid() &&
scope_->IsDeclaredParameter(param_name)) {
duplicate_parameters = FunctionLiteral::kHasDuplicateParameters;
dupe_error_loc = scanner()->location();
dupe_loc = scanner()->location();
}
Variable* var = scope_->DeclareParameter(param_name, VAR, is_rest);
@ -4023,8 +4027,8 @@ FunctionLiteral* Parser::ParseFunctionLiteral(
CHECK_OK);
const bool use_strict_params = is_rest || IsConciseMethod(kind);
CheckFunctionParameterNames(language_mode(), use_strict_params,
eval_args_error_loc, undefined_error_loc,
dupe_error_loc, reserved_error_loc, CHECK_OK);
eval_args_loc, undefined_loc, dupe_loc,
reserved_loc, CHECK_OK);
if (is_strict(language_mode())) {
CheckStrictOctalLiteral(scope->start_position(), scope->end_position(),

View File

@ -745,6 +745,7 @@ class ParserTraits {
// Utility functions
int DeclareArrowParametersFromExpression(Expression* expression, Scope* scope,
Scanner::Location* undefined_loc,
Scanner::Location* dupe_loc,
bool* ok);

View File

@ -916,12 +916,12 @@ PreParser::Expression PreParser::ParseFunctionLiteral(
// 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();
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_error_loc = Scanner::Location::invalid();
Scanner::Location undefined_loc = Scanner::Location::invalid();
bool is_rest = false;
bool done = arity_restriction == FunctionLiteral::GETTER_ARITY ||
@ -936,20 +936,20 @@ PreParser::Expression PreParser::ParseFunctionLiteral(
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 (!eval_args_loc.IsValid() && param_name.IsEvalOrArguments()) {
eval_args_loc = scanner()->location();
}
if (!undefined_error_loc.IsValid() && param_name.IsUndefined()) {
undefined_error_loc = scanner()->location();
if (!undefined_loc.IsValid() && param_name.IsUndefined()) {
undefined_loc = scanner()->location();
}
if (!reserved_error_loc.IsValid() && is_strict_reserved) {
reserved_error_loc = scanner()->location();
if (!reserved_loc.IsValid() && is_strict_reserved) {
reserved_loc = scanner()->location();
}
int prev_value = scanner()->FindSymbol(&duplicate_finder, 1);
if (!dupe_error_loc.IsValid() && prev_value != 0) {
dupe_error_loc = scanner()->location();
if (!dupe_loc.IsValid() && prev_value != 0) {
dupe_loc = scanner()->location();
}
if (arity_restriction == FunctionLiteral::SETTER_ARITY) break;
@ -984,9 +984,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_error_loc, undefined_error_loc,
dupe_error_loc, reserved_error_loc, CHECK_OK);
CheckFunctionParameterNames(language_mode(), use_strict_params, eval_args_loc,
undefined_loc, dupe_loc, reserved_loc, CHECK_OK);
if (is_strict(language_mode())) {
int end_position = scanner()->location().end_pos;

View File

@ -495,27 +495,26 @@ 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_error_loc,
const Scanner::Location& undefined_error_loc,
const Scanner::Location& dupe_error_loc,
const Scanner::Location& eval_args_loc,
const Scanner::Location& undefined_loc,
const Scanner::Location& dupe_loc,
const Scanner::Location& reserved_loc,
bool* ok) {
if (is_sloppy(language_mode) && !strict_params) return;
if (is_strict(language_mode) && eval_args_error_loc.IsValid()) {
Traits::ReportMessageAt(eval_args_error_loc, "strict_eval_arguments");
if (is_strict(language_mode) && eval_args_loc.IsValid()) {
Traits::ReportMessageAt(eval_args_loc, "strict_eval_arguments");
*ok = false;
return;
}
if (is_strong(language_mode) && undefined_error_loc.IsValid()) {
Traits::ReportMessageAt(eval_args_error_loc, "strong_undefined");
if (is_strong(language_mode) && undefined_loc.IsValid()) {
Traits::ReportMessageAt(undefined_loc, "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_error_loc.IsValid()) {
Traits::ReportMessageAt(dupe_error_loc, "strict_param_dupe");
if (dupe_loc.IsValid()) {
Traits::ReportMessageAt(dupe_loc, "strict_param_dupe");
*ok = false;
return;
}
@ -757,11 +756,6 @@ class PreParserIdentifier {
type_ == kLetIdentifier || type_ == kStaticIdentifier ||
type_ == kYieldIdentifier;
}
V8_INLINE bool IsValidArrowParam() const {
// A valid identifier can be an arrow function parameter
// except for eval, arguments, yield, and reserved keywords.
return !(IsEval() || IsArguments() || IsFutureStrictReserved());
}
// Allow identifier->name()[->length()] to work. The preparser
// does not need the actual positions/lengths of the identifiers.
@ -785,6 +779,7 @@ class PreParserIdentifier {
kPrototypeIdentifier,
kConstructorIdentifier
};
explicit PreParserIdentifier(Type type) : type_(type) {}
Type type_;
@ -810,10 +805,11 @@ class PreParserExpression {
static PreParserExpression BinaryOperation(PreParserExpression left,
Token::Value op,
PreParserExpression right) {
bool valid_arrow_param_list =
op == Token::COMMA && !left.is_parenthesized() &&
!right.is_parenthesized() && left.IsValidArrowParams() &&
right.IsValidArrowParams();
ValidArrowParam valid_arrow_param_list =
(op == Token::COMMA && !left.is_parenthesized() &&
!right.is_parenthesized()) ?
std::min(left.ValidateArrowParams(), right.ValidateArrowParams())
: kInvalidArrowParam;
return PreParserExpression(
TypeField::encode(kBinaryOperationExpression) |
IsValidArrowParamListField::encode(valid_arrow_param_list));
@ -915,10 +911,21 @@ class PreParserExpression {
return IsIdentifier() || IsProperty();
}
bool IsValidArrowParamList() const {
return IsValidArrowParams() &&
ParenthesizationField::decode(code_) !=
kMultiParenthesizedExpression;
bool IsValidArrowParamList(Scanner::Location* undefined_loc) 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();
return true;
} else {
return false;
}
}
// At the moment PreParser doesn't track these expression types.
@ -984,13 +991,34 @@ class PreParserExpression {
kNoTemplateTagExpression
};
enum ValidArrowParam {
kInvalidArrowParam,
kInvalidStrongArrowParam,
kValidArrowParam
};
explicit PreParserExpression(uint32_t expression_code)
: code_(expression_code) {}
V8_INLINE bool IsValidArrowParams() const {
return IsBinaryOperation()
? IsValidArrowParamListField::decode(code_)
: (IsIdentifier() && AsIdentifier().IsValidArrowParam());
V8_INLINE ValidArrowParam ValidateArrowParams() const {
if (IsBinaryOperation()) {
return IsValidArrowParamListField::decode(code_);
}
if (!IsIdentifier()) {
return kInvalidArrowParam;
}
PreParserIdentifier ident = AsIdentifier();
// A valid identifier can be an arrow function parameter
// except for eval, arguments, yield, and reserved keywords.
if (ident.IsEval() || ident.IsArguments() ||
ident.IsFutureStrictReserved()) {
return kInvalidArrowParam;
}
// In strong mode, 'undefined' is similarly restricted.
if (ident.IsUndefined()) {
return kInvalidStrongArrowParam;
}
return kValidArrowParam;
}
// The first five bits are for the Type and Parenthesization.
@ -1003,7 +1031,7 @@ class PreParserExpression {
ExpressionTypeField;
typedef BitField<bool, ParenthesizationField::kNext, 1> IsUseStrictField;
typedef BitField<bool, IsUseStrictField::kNext, 1> IsUseStrongField;
typedef BitField<bool, ParenthesizationField::kNext, 1>
typedef BitField<ValidArrowParam, ParenthesizationField::kNext, 2>
IsValidArrowParamListField;
typedef BitField<PreParserIdentifier::Type, ParenthesizationField::kNext, 10>
IdentifierTypeField;
@ -1491,10 +1519,11 @@ class PreParserTraits {
// 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();
*ok = expression.IsValidArrowParamList(undefined_loc);
return 0;
}
@ -3022,10 +3051,11 @@ ParserBase<Traits>::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 dupe_error_loc = Scanner::Location::invalid();
// TODO(arv): Pass in eval_args_error_loc and reserved_loc here.
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.
num_parameters = Traits::DeclareArrowParametersFromExpression(
params_ast, scope_, &dupe_error_loc, ok);
params_ast, scope_, &undefined_loc, &dupe_loc, ok);
if (!*ok) {
ReportMessageAt(
Scanner::Location(start_pos, scanner()->location().beg_pos),
@ -3033,6 +3063,11 @@ ParserBase<Traits>::ParseArrowFunctionLiteral(int start_pos,
return this->EmptyExpression();
}
if (undefined_loc.IsValid()) {
// Workaround for preparser not keeping track of positions.
undefined_loc = Scanner::Location(start_pos,
scanner()->location().end_pos);
}
if (num_parameters > Code::kMaxArguments) {
ReportMessageAt(Scanner::Location(params_ast->position(), position()),
"too_many_parameters");
@ -3043,7 +3078,7 @@ ParserBase<Traits>::ParseArrowFunctionLiteral(int start_pos,
Expect(Token::ARROW, CHECK_OK);
if (peek() == Token::LBRACE) {
// Multiple statemente body
// Multiple statement body
Consume(Token::LBRACE);
bool is_lazily_parsed =
(mode() == PARSE_LAZILY && scope_->AllowsLazyCompilation());
@ -3078,15 +3113,14 @@ ParserBase<Traits>::ParseArrowFunctionLiteral(int 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_error_loc, undefined_error_loc, and reserved_loc
// needs to be set by DeclareArrowParametersFromExpression.
Scanner::Location eval_args_error_loc = Scanner::Location::invalid();
Scanner::Location undefined_error_loc = Scanner::Location::invalid();
// 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();
const bool use_strict_params = true;
this->CheckFunctionParameterNames(language_mode(), use_strict_params,
eval_args_error_loc, undefined_error_loc,
dupe_error_loc, reserved_loc, CHECK_OK);
eval_args_loc, undefined_loc, dupe_loc,
reserved_loc, CHECK_OK);
// Validate strict mode.
if (is_strict(language_mode())) {

View File

@ -5869,13 +5869,48 @@ TEST(StrongUndefinedLocal) {
"(class undefined {'use strong'});",
NULL};
static const ParserFlag always_flags[] = {kAllowStrongMode};
static const ParserFlag always_flags[] = {
kAllowStrongMode, kAllowHarmonySloppy
};
RunParserSyncTest(context_data, data, kError, NULL, 0,
always_flags, arraysize(always_flags));
}
TEST(StrongUndefinedArrow) {
const char* sloppy_context_data[][2] = {{"", ""}, {NULL}};
const char* strict_context_data[][2] = {{"'use strict';", ""}, {NULL}};
const char* strong_context_data[][2] = {{"'use strong';", ""}, {NULL}};
const char* data[] = {
"(undefined => {return});",
"((undefined, b, c) => {return});",
"((a, undefined, c) => {return});",
"((a, b, undefined) => {return});",
NULL};
const char* local_strong[] = {
"(undefined => {'use strong';});",
"((undefined, b, c) => {'use strong';});",
"((a, undefined, c) => {'use strong';});",
"((a, b, undefined) => {'use strong';});",
NULL};
static const ParserFlag always_flags[] = {
kAllowStrongMode, kAllowHarmonyArrowFunctions
};
RunParserSyncTest(sloppy_context_data, data, kSuccess, NULL, 0, always_flags,
arraysize(always_flags));
RunParserSyncTest(strict_context_data, data, kSuccess, NULL, 0, always_flags,
arraysize(always_flags));
RunParserSyncTest(strong_context_data, data, kError, NULL, 0, always_flags,
arraysize(always_flags));
RunParserSyncTest(sloppy_context_data, local_strong, kError, NULL, 0,
always_flags, arraysize(always_flags));
}
TEST(ArrowFunctionASIErrors) {
const char* context_data[][2] = {{"'use strict';", ""}, {"", ""},
{NULL, NULL}};

View File

@ -107,11 +107,6 @@ assertThrows("({ foo(a, b, undefined, c, d) {'use strong';} });", SyntaxError);
CheckStrongMode("({ *foo(a, b, undefined, c, d) {} });");
assertThrows("({ *foo(a, b, undefined, c, d) {'use strong';} });", SyntaxError);
// Arrow function expression with 'undefined' param
// TODO(conradw): Checking arrow function heads is hard to modify just now
// CheckStrongMode("(undefined => {})");
// assertThrows("(undefined => {'use strong';})");
// Class declaration named 'undefined'
CheckStrongMode("class undefined {}");
assertThrows("class undefined {'use strong'}", SyntaxError);
@ -190,3 +185,16 @@ assertDoesNotThrow(function() {
assertThrows(function() {
Function("undefined", "'use strong';");
}, SyntaxError);
// Arrow functions with undefined parameters
CheckStrongMode("(undefined => {return});");
assertThrows("(undefined => {'use strong';});");
CheckStrongMode("((undefined, b, c) => {return});");
assertThrows("((undefined, b, c) => {'use strong';});");
CheckStrongMode("((a, undefined, c) => {return});");
assertThrows("((a, undefined, c) => {'use strong';});");
CheckStrongMode("((a, b, undefined) => {return});");
assertThrows("((a, b, undefined) => {'use strong';});");