From 9e9abcfff46084f60ad83acb26f56f13ac04347f Mon Sep 17 00:00:00 2001 From: adamk Date: Mon, 2 May 2016 18:55:27 -0700 Subject: [PATCH] Properly disallow 'yield' in class expressions and arrow parameters Yield expressions are not allowed in formal parameter initializers of generators, but we weren't properly catching the case where the yield expression appeared in the 'extends' clause of a class expression. They also aren't allowed in arrow functions, which we were failing to catch due to not looking at the obscurely-named "FormalParameterInitializerError" bit of ExpressionClassifier. This patch passes along an ExpressionClassifier when parsing class expressions and accumulates the proper error for that case. For the arrow function case, the fix is simply to check for the "formal parameter initializer" error once we know we've parsed an arrow function. The error message used for this has also been made specific to yield expressions. Tests are added both for the error case and the non-error cases (where yield is used in such a position inside the class body). BUG=v8:4966, v8:4968, v8:4974 LOG=n Review-Url: https://codereview.chromium.org/1941823003 Cr-Commit-Position: refs/heads/master@{#35957} --- src/messages.h | 4 +-- src/parsing/parser-base.h | 21 ++++++++-------- src/parsing/parser.cc | 30 +++++++++++++---------- src/parsing/parser.h | 7 +++--- src/parsing/preparser.cc | 28 ++++++++++++--------- src/parsing/preparser.h | 6 +++-- test/cctest/test-parsing.cc | 21 +++++++++++++++- test/message/yield-in-arrow-param.js | 5 ++++ test/message/yield-in-arrow-param.out | 4 +++ test/message/yield-in-generator-param.js | 5 ++++ test/message/yield-in-generator-param.out | 4 +++ 11 files changed, 91 insertions(+), 44 deletions(-) create mode 100644 test/message/yield-in-arrow-param.js create mode 100644 test/message/yield-in-arrow-param.out create mode 100644 test/message/yield-in-generator-param.js create mode 100644 test/message/yield-in-generator-param.out diff --git a/src/messages.h b/src/messages.h index b4e7528974..caf1200ef8 100644 --- a/src/messages.h +++ b/src/messages.h @@ -117,8 +117,7 @@ class CallSite { T(DebuggerType, "Debugger: Parameters have wrong types.") \ T(DeclarationMissingInitializer, "Missing initializer in % declaration") \ T(DefineDisallowed, "Cannot define property:%, object is not extensible.") \ - T(DetachedOperation, \ - "Cannot perform % on a detached ArrayBuffer") \ + T(DetachedOperation, "Cannot perform % on a detached ArrayBuffer") \ T(DuplicateTemplateProperty, "Object template has duplicate property '%'") \ T(ExtendsValueGenerator, \ "Class extends value % may not be a generator function") \ @@ -463,6 +462,7 @@ class CallSite { T(InvalidHexEscapeSequence, "Invalid hexadecimal escape sequence") \ T(InvalidUnicodeEscapeSequence, "Invalid Unicode escape sequence") \ T(UndefinedUnicodeCodePoint, "Undefined Unicode code-point") \ + T(YieldInParameter, "Yield expression not allowed in formal parameter") \ /* EvalError */ \ T(CodeGenFromStrings, "%") \ /* URIError */ \ diff --git a/src/parsing/parser-base.h b/src/parsing/parser-base.h index 7202c981e5..b475f1f15b 100644 --- a/src/parsing/parser-base.h +++ b/src/parsing/parser-base.h @@ -784,15 +784,6 @@ class ParserBase : public Traits { classifier->RecordArrowFormalParametersError(location, message, arg); } - void FormalParameterInitializerUnexpectedToken( - ExpressionClassifier* classifier) { - MessageTemplate::Template message = MessageTemplate::kUnexpectedToken; - const char* arg; - Scanner::Location location = scanner()->peek_location(); - GetUnexpectedTokenMessage(peek(), &message, &location, &arg); - classifier->RecordFormalParameterInitializerError(location, message, arg); - } - // Recursive descent functions: // Parses an identifier that is valid for the current scope, in particular it @@ -1415,7 +1406,7 @@ ParserBase::ParsePrimaryExpression(ExpressionClassifier* classifier, CHECK_OK); class_name_location = scanner()->location(); } - return this->ParseClassLiteral(name, class_name_location, + return this->ParseClassLiteral(classifier, name, class_name_location, is_strict_reserved_name, class_token_position, ok); } @@ -1990,6 +1981,13 @@ ParserBase::ParseAssignmentExpression(bool accept_IN, Token::String(Token::ARROW)); ValidateArrowFormalParameters(&arrow_formals_classifier, expression, parenthesized_formals, CHECK_OK); + // This reads strangely, but is correct: it checks whether any + // sub-expression of the parameter list failed to be a valid formal + // parameter initializer. Since YieldExpressions are banned anywhere + // in an arrow parameter list, this is correct. + // TODO(adamk): Rename "FormalParameterInitializerError" to refer to + // "YieldExpression", which is its only use. + ValidateFormalParameterInitializer(&arrow_formals_classifier, ok); Scanner::Location loc(lhs_beg_pos, scanner()->location().end_pos); Scope* scope = this->NewScope(scope_, FUNCTION_SCOPE, FunctionKind::kArrowFunction); @@ -2126,7 +2124,8 @@ ParserBase::ParseYieldExpression(bool accept_IN, int pos = peek_position(); classifier->RecordPatternError(scanner()->peek_location(), MessageTemplate::kInvalidDestructuringTarget); - FormalParameterInitializerUnexpectedToken(classifier); + classifier->RecordFormalParameterInitializerError( + scanner()->peek_location(), MessageTemplate::kYieldInParameter); Expect(Token::YIELD, CHECK_OK); ExpressionT generator_object = factory()->NewVariableProxy(function_state_->generator_object_variable()); diff --git a/src/parsing/parser.cc b/src/parsing/parser.cc index 25126e1cf3..91d3fb8f0f 100644 --- a/src/parsing/parser.cc +++ b/src/parsing/parser.cc @@ -753,11 +753,11 @@ FunctionLiteral* ParserTraits::ParseFunctionLiteral( function_token_position, type, language_mode, ok); } - ClassLiteral* ParserTraits::ParseClassLiteral( - const AstRawString* name, Scanner::Location class_name_location, - bool name_is_strict_reserved, int pos, bool* ok) { - return parser_->ParseClassLiteral(name, class_name_location, + Type::ExpressionClassifier* classifier, const AstRawString* name, + Scanner::Location class_name_location, bool name_is_strict_reserved, + int pos, bool* ok) { + return parser_->ParseClassLiteral(classifier, name, class_name_location, name_is_strict_reserved, pos, ok); } @@ -1564,9 +1564,9 @@ Statement* Parser::ParseExportDefault(bool* ok) { if (peek() == Token::EXTENDS || peek() == Token::LBRACE) { // ClassDeclaration[+Default] :: // 'class' ('extends' LeftHandExpression)? '{' ClassBody '}' - default_export = - ParseClassLiteral(default_string, Scanner::Location::invalid(), - false, position(), CHECK_OK); + default_export = ParseClassLiteral(nullptr, default_string, + Scanner::Location::invalid(), false, + position(), CHECK_OK); result = factory()->NewEmptyStatement(RelocInfo::kNoPosition); } else { result = ParseClassDeclaration(&names, CHECK_OK); @@ -2130,7 +2130,7 @@ Statement* Parser::ParseClassDeclaration(ZoneList* names, bool is_strict_reserved = false; const AstRawString* name = ParseIdentifierOrStrictReservedWord(&is_strict_reserved, CHECK_OK); - ClassLiteral* value = ParseClassLiteral(name, scanner()->location(), + ClassLiteral* value = ParseClassLiteral(nullptr, name, scanner()->location(), is_strict_reserved, pos, CHECK_OK); VariableProxy* proxy = NewUnresolved(name, LET); @@ -4630,8 +4630,8 @@ PreParser::PreParseResult Parser::ParseLazyFunctionBodyWithPreParser( return result; } - -ClassLiteral* Parser::ParseClassLiteral(const AstRawString* name, +ClassLiteral* Parser::ParseClassLiteral(ExpressionClassifier* classifier, + const AstRawString* name, Scanner::Location class_name_location, bool name_is_strict_reserved, int pos, bool* ok) { @@ -4664,9 +4664,13 @@ ClassLiteral* Parser::ParseClassLiteral(const AstRawString* name, Expression* extends = NULL; if (Check(Token::EXTENDS)) { block_scope->set_start_position(scanner()->location().end_pos); - ExpressionClassifier classifier(this); - extends = ParseLeftHandSideExpression(&classifier, CHECK_OK); - RewriteNonPattern(&classifier, CHECK_OK); + ExpressionClassifier extends_classifier(this); + extends = ParseLeftHandSideExpression(&extends_classifier, CHECK_OK); + RewriteNonPattern(&extends_classifier, CHECK_OK); + if (classifier != nullptr) { + classifier->Accumulate(&extends_classifier, + ExpressionClassifier::ExpressionProductions); + } } else { block_scope->set_start_position(scanner()->location().end_pos); } diff --git a/src/parsing/parser.h b/src/parsing/parser.h index 13b8a276a6..9417a76bfd 100644 --- a/src/parsing/parser.h +++ b/src/parsing/parser.h @@ -567,7 +567,8 @@ class ParserTraits { const ParserFormalParameters& parameters, FunctionKind kind, FunctionLiteral::FunctionType function_type, bool* ok); - ClassLiteral* ParseClassLiteral(const AstRawString* name, + ClassLiteral* ParseClassLiteral(Type::ExpressionClassifier* classifier, + const AstRawString* name, Scanner::Location class_name_location, bool name_is_strict_reserved, int pos, bool* ok); @@ -958,8 +959,8 @@ class Parser : public ParserBase { int function_token_position, FunctionLiteral::FunctionType type, LanguageMode language_mode, bool* ok); - - ClassLiteral* ParseClassLiteral(const AstRawString* name, + ClassLiteral* ParseClassLiteral(ExpressionClassifier* classifier, + const AstRawString* name, Scanner::Location class_name_location, bool name_is_strict_reserved, int pos, bool* ok); diff --git a/src/parsing/preparser.cc b/src/parsing/preparser.cc index db8fefd579..af1580b081 100644 --- a/src/parsing/preparser.cc +++ b/src/parsing/preparser.cc @@ -140,11 +140,11 @@ PreParser::PreParseResult PreParser::PreParseLazyFunction( return kPreParseSuccess; } - PreParserExpression PreParserTraits::ParseClassLiteral( - PreParserIdentifier name, Scanner::Location class_name_location, - bool name_is_strict_reserved, int pos, bool* ok) { - return pre_parser_->ParseClassLiteral(name, class_name_location, + Type::ExpressionClassifier* classifier, PreParserIdentifier name, + Scanner::Location class_name_location, bool name_is_strict_reserved, + int pos, bool* ok) { + return pre_parser_->ParseClassLiteral(classifier, name, class_name_location, name_is_strict_reserved, pos, ok); } @@ -418,8 +418,8 @@ PreParser::Statement PreParser::ParseClassDeclaration(bool* ok) { bool is_strict_reserved = false; Identifier name = ParseIdentifierOrStrictReservedWord(&is_strict_reserved, CHECK_OK); - ParseClassLiteral(name, scanner()->location(), is_strict_reserved, pos, - CHECK_OK); + ParseClassLiteral(nullptr, name, scanner()->location(), is_strict_reserved, + pos, CHECK_OK); return Statement::Default(); } @@ -1136,10 +1136,10 @@ void PreParser::ParseLazyFunctionLiteralBody(bool* ok, scope_->uses_super_property(), scope_->calls_eval()); } - PreParserExpression PreParser::ParseClassLiteral( - PreParserIdentifier name, Scanner::Location class_name_location, - bool name_is_strict_reserved, int pos, bool* ok) { + ExpressionClassifier* classifier, PreParserIdentifier name, + Scanner::Location class_name_location, bool name_is_strict_reserved, + int pos, bool* ok) { // All parts of a ClassDeclaration and ClassExpression are strict code. if (name_is_strict_reserved) { ReportMessageAt(class_name_location, @@ -1163,9 +1163,13 @@ PreParserExpression PreParser::ParseClassLiteral( bool has_extends = Check(Token::EXTENDS); if (has_extends) { - ExpressionClassifier classifier(this); - ParseLeftHandSideExpression(&classifier, CHECK_OK); - ValidateExpression(&classifier, CHECK_OK); + ExpressionClassifier extends_classifier(this); + ParseLeftHandSideExpression(&extends_classifier, CHECK_OK); + ValidateExpression(&extends_classifier, CHECK_OK); + if (classifier != nullptr) { + classifier->Accumulate(&extends_classifier, + ExpressionClassifier::ExpressionProductions); + } } ClassLiteralChecker checker(this); diff --git a/src/parsing/preparser.h b/src/parsing/preparser.h index 3fd3ce8480..75d5c18a54 100644 --- a/src/parsing/preparser.h +++ b/src/parsing/preparser.h @@ -891,7 +891,8 @@ class PreParserTraits { int function_token_position, FunctionLiteral::FunctionType type, LanguageMode language_mode, bool* ok); - PreParserExpression ParseClassLiteral(PreParserIdentifier name, + PreParserExpression ParseClassLiteral(Type::ExpressionClassifier* classifier, + PreParserIdentifier name, Scanner::Location class_name_location, bool name_is_strict_reserved, int pos, bool* ok); @@ -1096,7 +1097,8 @@ class PreParser : public ParserBase { void ParseLazyFunctionLiteralBody(bool* ok, Scanner::BookmarkScope* bookmark = nullptr); - PreParserExpression ParseClassLiteral(PreParserIdentifier name, + PreParserExpression ParseClassLiteral(ExpressionClassifier* classifier, + PreParserIdentifier name, Scanner::Location class_name_location, bool name_is_strict_reserved, int pos, bool* ok); diff --git a/test/cctest/test-parsing.cc b/test/cctest/test-parsing.cc index a28c235c5e..7f297b3d67 100644 --- a/test/cctest/test-parsing.cc +++ b/test/cctest/test-parsing.cc @@ -2332,6 +2332,10 @@ TEST(NoErrorsGenerator) { "(yield) \n ? yield : yield", // If there is a newline before the next token, we don't look for RHS. "yield\nfor (;;) {}", + "x = class extends (yield) {}", + "x = class extends f(yield) {}", + "x = class extends (null, yield) { }", + "x = class extends (a ? null : yield) { }", NULL }; // clang-format on @@ -2396,6 +2400,7 @@ TEST(ErrorsYieldGenerator) { "for (yield 'x' of {});", "for (yield 'x' in {} in {});", "for (yield 'x' in {} of {});", + "class C extends yield { }", NULL }; // clang-format on @@ -6730,6 +6735,9 @@ TEST(DefaultParametersYieldInInitializers) { const char* generator_context_data[][2] = { {"'use strict'; (function *g(", ") { });"}, {"(function *g(", ") { });"}, + // Arrow function within generator has the same rules. + {"'use strict'; (function *g() { (", ") => {} });"}, + {"(function *g() { (", ") => {} });"}, {NULL, NULL} }; @@ -6760,6 +6768,17 @@ TEST(DefaultParametersYieldInInitializers) { NULL }; + // Because classes are always in strict mode, these are always errors. + const char* always_error_param_data[] = { + "x = class extends (yield) { }", + "x = class extends f(yield) { }", + "x = class extends (null, yield) { }", + "x = class extends (a ? null : yield) { }", + "[x] = [class extends (a ? null : yield) { }]", + "[x = class extends (a ? null : yield) { }]", + "[x = class extends (a ? null : yield) { }] = [null]", + NULL + }; // clang-format on RunParserSyncTest(sloppy_function_context_data, parameter_data, kSuccess); @@ -6769,9 +6788,9 @@ TEST(DefaultParametersYieldInInitializers) { RunParserSyncTest(strict_arrow_context_data, parameter_data, kError); RunParserSyncTest(generator_context_data, parameter_data, kError); + RunParserSyncTest(generator_context_data, always_error_param_data, kError); } - TEST(SpreadArray) { const char* context_data[][2] = { {"'use strict';", ""}, {"", ""}, {NULL, NULL}}; diff --git a/test/message/yield-in-arrow-param.js b/test/message/yield-in-arrow-param.js new file mode 100644 index 0000000000..c815fe7603 --- /dev/null +++ b/test/message/yield-in-arrow-param.js @@ -0,0 +1,5 @@ +// Copyright 2016 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* g() { (a = yield) => {} } diff --git a/test/message/yield-in-arrow-param.out b/test/message/yield-in-arrow-param.out new file mode 100644 index 0000000000..8eeb0df5f6 --- /dev/null +++ b/test/message/yield-in-arrow-param.out @@ -0,0 +1,4 @@ +*%(basename)s:5: SyntaxError: Yield expression not allowed in formal parameter +function* g() { (a = yield) => {} } + ^^^^^ +SyntaxError: Yield expression not allowed in formal parameter diff --git a/test/message/yield-in-generator-param.js b/test/message/yield-in-generator-param.js new file mode 100644 index 0000000000..1a8f8420f6 --- /dev/null +++ b/test/message/yield-in-generator-param.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* g(a = yield) {} diff --git a/test/message/yield-in-generator-param.out b/test/message/yield-in-generator-param.out new file mode 100644 index 0000000000..ec46f478c9 --- /dev/null +++ b/test/message/yield-in-generator-param.out @@ -0,0 +1,4 @@ +*%(basename)s:5: SyntaxError: Yield expression not allowed in formal parameter +function* g(a = yield) {} + ^^^^^ +SyntaxError: Yield expression not allowed in formal parameter