From: rossberg Date: Wed, 5 Aug 2015 12:00:41 +0000 (-0700) Subject: [es6] Implement proper TDZ for parameters X-Git-Tag: upstream/4.7.83~1015 X-Git-Url: http://review.tizen.org/git/?a=commitdiff_plain;h=4273f66e9882f1239c6b7846c31730d47b870d81;p=platform%2Fupstream%2Fv8.git [es6] Implement proper TDZ for parameters Previously, examples like (({a = x}, x) => {})({}, 0) did not throw a ReferenceError like they should. This CL - Splits up DeclareFormalParameters such that the formals can be recorded first and declared later. - Declaration then takes the complete parameter list into account. If it is not simple, temporaries are introduced for all parameters. - BuildParameterInitializationBlock desugars all parameters from non-simple lists into let-bindings. - Refactored Pre/ParserFormalParameters, so that the arity information is no longer duplicated in Parser. - Rest is currently handled specially, until rest-via-destructuring has landed. R=adamk@chromium.org, littledan@chromium.org BUG=v8:811 LOG=N Review URL: https://codereview.chromium.org/1259283002 Cr-Commit-Position: refs/heads/master@{#30025} --- diff --git a/src/parser.cc b/src/parser.cc index 311d192..9e3d672 100644 --- a/src/parser.cc +++ b/src/parser.cc @@ -1203,6 +1203,11 @@ FunctionLiteral* Parser::ParseLazy(Isolate* isolate, ParseInfo* info, // BindingIdentifier const bool is_rest = false; ParseFormalParameter(is_rest, &formals, &formals_classifier, &ok); + if (ok) { + DeclareFormalParameter( + formals.scope, formals.at(0), formals.is_simple, + &formals_classifier); + } } } @@ -2223,10 +2228,7 @@ Statement* Parser::ParseFunctionDeclaration( is_strong(language_mode()) ? CONST : (is_strict(language_mode()) || allow_harmony_sloppy()) && - !(scope_->is_script_scope() || scope_->is_eval_scope() || - scope_->is_function_scope()) - ? LET - : VAR; + !scope_->is_declaration_scope() ? LET : VAR; VariableProxy* proxy = NewUnresolved(name, mode); Declaration* declaration = factory()->NewFunctionDeclaration(proxy, mode, fun, scope_, pos); @@ -3854,7 +3856,7 @@ void ParserTraits::ParseArrowFunctionFormalParameters( ParserFormalParameters* parameters, Expression* expr, const Scanner::Location& params_loc, Scanner::Location* duplicate_loc, bool* ok) { - if (parameters->arity >= Code::kMaxArguments) { + if (parameters->Arity() >= Code::kMaxArguments) { ReportMessageAt(params_loc, MessageTemplate::kMalformedArrowFunParamList); *ok = false; return; @@ -3908,11 +3910,26 @@ void ParserTraits::ParseArrowFunctionFormalParameters( parser_->scope_->RemoveUnresolved(expr->AsVariableProxy()); } - ++parameters->arity; - ExpressionClassifier classifier; - DeclareFormalParameter(parameters, expr, is_rest, &classifier); - if (!duplicate_loc->IsValid()) { - *duplicate_loc = classifier.duplicate_formal_parameter_error().location; + AddFormalParameter(parameters, expr, is_rest); +} + + +void ParserTraits::ParseArrowFunctionFormalParameterList( + ParserFormalParameters* parameters, Expression* expr, + const Scanner::Location& params_loc, + Scanner::Location* duplicate_loc, bool* ok) { + ParseArrowFunctionFormalParameters(parameters, expr, params_loc, + duplicate_loc, ok); + if (!*ok) return; + + for (int i = 0; i < parameters->Arity(); ++i) { + auto parameter = parameters->at(i); + ExpressionClassifier classifier; + DeclareFormalParameter( + parameters->scope, parameter, parameters->is_simple, &classifier); + if (!duplicate_loc->IsValid()) { + *duplicate_loc = classifier.duplicate_formal_parameter_error().location; + } } } @@ -4032,8 +4049,7 @@ FunctionLiteral* Parser::ParseFunctionLiteral( scope_->set_start_position(start_position); ParserFormalParameters formals(scope); ParseFormalParameterList(&formals, &formals_classifier, CHECK_OK); - arity = formals.arity; - DCHECK(arity == formals.params.length()); + arity = formals.Arity(); Expect(Token::RPAREN, CHECK_OK); int formals_end_position = scanner()->location().end_pos; @@ -4299,7 +4315,8 @@ Block* Parser::BuildParameterInitializationBlock( factory()->NewBlock(NULL, 1, true, RelocInfo::kNoPosition); for (int i = 0; i < parameters.params.length(); ++i) { auto parameter = parameters.params[i]; - if (parameter.pattern == nullptr) continue; + // TODO(caitp,rossberg): Remove special handling for rest once desugared. + if (parameter.is_rest) break; DeclarationDescriptor descriptor; descriptor.declaration_kind = DeclarationDescriptor::PARAMETER; descriptor.parser = this; diff --git a/src/parser.h b/src/parser.h index 2086ee9..4a77d91 100644 --- a/src/parser.h +++ b/src/parser.h @@ -539,7 +539,7 @@ class Parser; class SingletonLogger; -struct ParserFormalParameters : public PreParserFormalParameters { +struct ParserFormalParameters : FormalParametersBase { struct Parameter { Parameter(const AstRawString* name, Expression* pattern, bool is_rest) : name(name), pattern(pattern), is_rest(is_rest) {} @@ -549,15 +549,11 @@ struct ParserFormalParameters : public PreParserFormalParameters { }; explicit ParserFormalParameters(Scope* scope) - : PreParserFormalParameters(scope), params(4, scope->zone()) {} - + : FormalParametersBase(scope), params(4, scope->zone()) {} ZoneList params; - void AddParameter( - const AstRawString* name, Expression* pattern, bool is_rest) { - params.Add(Parameter(name, pattern, is_rest), scope->zone()); - DCHECK_EQ(arity, params.length()); - } + int Arity() const { return params.length(); } + const Parameter& at(int i) const { return params[i]; } }; @@ -782,13 +778,19 @@ class ParserTraits { V8_INLINE Scope* NewScope(Scope* parent_scope, ScopeType scope_type, FunctionKind kind = kNormalFunction); + V8_INLINE void AddFormalParameter( + ParserFormalParameters* parameters, Expression* pattern, bool is_rest); V8_INLINE void DeclareFormalParameter( - ParserFormalParameters* parameters, Expression* pattern, bool is_rest, - ExpressionClassifier* classifier); + Scope* scope, const ParserFormalParameters::Parameter& parameter, + bool is_simple, ExpressionClassifier* classifier); void ParseArrowFunctionFormalParameters( ParserFormalParameters* parameters, Expression* params, const Scanner::Location& params_loc, Scanner::Location* duplicate_loc, bool* ok); + void ParseArrowFunctionFormalParameterList( + ParserFormalParameters* parameters, Expression* params, + const Scanner::Location& params_loc, + Scanner::Location* duplicate_loc, bool* ok); void ReindexLiterals(const ParserFormalParameters& parameters); @@ -1310,25 +1312,35 @@ Expression* ParserTraits::SpreadCallNew( } -void ParserTraits::DeclareFormalParameter( - ParserFormalParameters* parameters, Expression* pattern, bool is_rest, - ExpressionClassifier* classifier) { - bool is_duplicate = false; +void ParserTraits::AddFormalParameter( + ParserFormalParameters* parameters, Expression* pattern, bool is_rest) { bool is_simple = pattern->IsVariableProxy(); - DCHECK(parser_->allow_harmony_destructuring() || is_simple); - + DCHECK(parser_->allow_harmony_destructuring() || + parser_->allow_harmony_rest_parameters() || is_simple); const AstRawString* name = is_simple ? pattern->AsVariableProxy()->raw_name() : parser_->ast_value_factory()->empty_string(); - VariableMode mode = is_simple ? VAR : TEMPORARY; + parameters->params.Add( + ParserFormalParameters::Parameter(name, pattern, is_rest), + parameters->scope->zone()); +} + + +void ParserTraits::DeclareFormalParameter( + Scope* scope, const ParserFormalParameters::Parameter& parameter, + bool is_simple, ExpressionClassifier* classifier) { + bool is_duplicate = false; + // TODO(caitp): Remove special handling for rest once desugaring is in. + auto name = is_simple || parameter.is_rest + ? parameter.name : parser_->ast_value_factory()->empty_string(); + auto mode = is_simple || parameter.is_rest ? VAR : TEMPORARY; Variable* var = - parameters->scope->DeclareParameter(name, mode, is_rest, &is_duplicate); - parameters->AddParameter(name, is_simple ? nullptr : pattern, is_rest); + scope->DeclareParameter(name, mode, parameter.is_rest, &is_duplicate); if (is_duplicate) { classifier->RecordDuplicateFormalParameterError( parser_->scanner()->location()); } - if (is_sloppy(parameters->scope->language_mode())) { + 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. diff --git a/src/preparser.h b/src/preparser.h index 3bd0953..d240db1 100644 --- a/src/preparser.h +++ b/src/preparser.h @@ -27,6 +27,15 @@ enum FunctionNameValidity { }; +struct FormalParametersBase { + explicit FormalParametersBase(Scope* scope) : scope(scope) {} + Scope* scope; + bool has_rest = false; + bool is_simple = true; + int materialized_literals_count = 0; +}; + + // Common base class shared between parser and pre-parser. Traits encapsulate // the differences between Parser and PreParser: @@ -1312,18 +1321,13 @@ class PreParserFactory { }; -struct PreParserFormalParameters { +struct PreParserFormalParameters : FormalParametersBase { explicit PreParserFormalParameters(Scope* scope) - : scope(scope), - arity(0), - has_rest(false), - is_simple(true), - materialized_literals_count(0) {} - Scope* scope; - int arity; - bool has_rest; - bool is_simple; - int materialized_literals_count; + : FormalParametersBase(scope) {} + int arity = 0; + + int Arity() const { return arity; } + PreParserIdentifier at(int i) { return PreParserIdentifier(); } // Dummy }; @@ -1606,7 +1610,7 @@ class PreParserTraits { const PreParserFormalParameters& parameters, FunctionKind kind, FunctionLiteral::FunctionType function_type, bool* ok); - V8_INLINE void ParseArrowFunctionFormalParameters( + V8_INLINE void ParseArrowFunctionFormalParameterList( PreParserFormalParameters* parameters, PreParserExpression expression, const Scanner::Location& params_loc, Scanner::Location* duplicate_loc, bool* ok); @@ -1637,8 +1641,13 @@ class PreParserTraits { return !tag.IsNoTemplateTag(); } - void DeclareFormalParameter(PreParserFormalParameters* parameters, - PreParserExpression pattern, bool is_rest, + void AddFormalParameter( + PreParserFormalParameters* parameters, PreParserExpression pattern, + bool is_rest) { + ++parameters->arity; + } + void DeclareFormalParameter(Scope* scope, PreParserIdentifier parameter, + bool is_simple, ExpressionClassifier* classifier) {} void CheckConflictingVarDeclarations(Scope* scope, bool* ok) {} @@ -1835,7 +1844,7 @@ PreParserExpression PreParserTraits::SpreadCallNew(PreParserExpression function, } -void PreParserTraits::ParseArrowFunctionFormalParameters( +void PreParserTraits::ParseArrowFunctionFormalParameterList( PreParserFormalParameters* parameters, PreParserExpression params, const Scanner::Location& params_loc, Scanner::Location* duplicate_loc, bool* ok) { @@ -2280,12 +2289,15 @@ ParserBase::ParsePrimaryExpression(ExpressionClassifier* classifier, // (...x) => y Scope* scope = this->NewScope(scope_, ARROW_SCOPE, FunctionKind::kArrowFunction); - FormalParametersT parameters(scope); + FormalParametersT formals(scope); scope->set_start_position(beg_pos); - ExpressionClassifier args_classifier; + ExpressionClassifier formals_classifier; const bool is_rest = true; - this->ParseFormalParameter(is_rest, ¶meters, &args_classifier, + this->ParseFormalParameter(is_rest, &formals, &formals_classifier, CHECK_OK); + Traits::DeclareFormalParameter( + formals.scope, formals.at(0), formals.is_simple, + &formals_classifier); if (peek() == Token::COMMA) { ReportMessageAt(scanner()->peek_location(), MessageTemplate::kParamAfterRest); @@ -2293,7 +2305,7 @@ ParserBase::ParsePrimaryExpression(ExpressionClassifier* classifier, return this->EmptyExpression(); } Expect(Token::RPAREN, CHECK_OK); - result = this->ParseArrowFunctionLiteral(parameters, args_classifier, + result = this->ParseArrowFunctionLiteral(formals, formals_classifier, CHECK_OK); } else { // Heuristically try to detect immediately called functions before @@ -2845,8 +2857,8 @@ ParserBase::ParseAssignmentExpression(bool accept_IN, scope->set_start_position(lhs_location.beg_pos); Scanner::Location duplicate_loc = Scanner::Location::invalid(); - this->ParseArrowFunctionFormalParameters(¶meters, expression, loc, - &duplicate_loc, CHECK_OK); + this->ParseArrowFunctionFormalParameterList(¶meters, expression, loc, + &duplicate_loc, CHECK_OK); if (duplicate_loc.IsValid()) { arrow_formals_classifier.RecordDuplicateFormalParameterError( duplicate_loc); @@ -3647,8 +3659,7 @@ void ParserBase::ParseFormalParameter( *ok = false; return; } - ++parameters->arity; - Traits::DeclareFormalParameter(parameters, pattern, is_rest, classifier); + Traits::AddFormalParameter(parameters, pattern, is_rest); } @@ -3669,11 +3680,11 @@ void ParserBase::ParseFormalParameterList( // FormalsList[?Yield, ?GeneratorParameter] , // FormalParameter[?Yield,?GeneratorParameter] - DCHECK_EQ(0, parameters->arity); + DCHECK_EQ(0, parameters->Arity()); if (peek() != Token::RPAREN) { do { - if (parameters->arity > Code::kMaxArguments) { + if (parameters->Arity() > Code::kMaxArguments) { ReportMessage(MessageTemplate::kTooManyParameters); *ok = false; return; @@ -3687,8 +3698,15 @@ void ParserBase::ParseFormalParameterList( ReportMessageAt(scanner()->peek_location(), MessageTemplate::kParamAfterRest); *ok = false; + return; } } + + for (int i = 0; i < parameters->Arity(); ++i) { + auto parameter = parameters->at(i); + Traits::DeclareFormalParameter( + parameters->scope, parameter, parameters->is_simple, classifier); + } } diff --git a/test/mjsunit/harmony/destructuring.js b/test/mjsunit/harmony/destructuring.js index f9b6203..41b366a 100644 --- a/test/mjsunit/harmony/destructuring.js +++ b/test/mjsunit/harmony/destructuring.js @@ -806,16 +806,70 @@ assertEquals(1, g20({a: 1, b: 2})); // var g21 = ({[eval('y')]: x}) => { var y = 'b'; return x; }; // assertEquals(1, g21({a: 1, b: 2})); +})(); + + +(function TestParameterTDZ() { + function f1({a = x}, x) { return a } + assertThrows(() => f1({}, 4), ReferenceError); + assertEquals(4, f1({a: 4}, 5)); + // TODO(rossberg): eval in default expressions is not working yet. + // function f2({a = eval("x")}, x) { return a } + // assertThrows(() => f2({}, 4), ReferenceError); + // assertEquals(4, f2({a: 4}, 5)); + // function f3({a = eval("x")}, x) { 'use strict'; return a } + // assertThrows(() => f3({}, 4), ReferenceError); + // assertEquals(4, f3({a: 4}, 5)); + // function f4({a = eval("'use strict'; x")}, x) { return a } + // assertThrows(() => f4({}, 4), ReferenceError); + // assertEquals(4, f4({a: 4}, 5)); + + function f5({a = () => x}, x) { return a() } + assertEquals(4, f5({a: () => 4}, 5)); + // TODO(rossberg): eval in default expressions is not working yet. + // function f6({a = () => eval("x")}, x) { return a() } + // assertEquals(4, f6({a: () => 4}, 5)); + // function f7({a = () => eval("x")}, x) { 'use strict'; return a() } + // assertEquals(4, f7({a: () => 4}, 5)); + // function f8({a = () => eval("'use strict'; x")}, x) { return a() } + // assertEquals(4, f8({a: () => 4}, 5)); + + function f11({a = b}, {b}) { return a } + assertThrows(() => f11({}, {b: 4}), ReferenceError); + assertEquals(4, f11({a: 4}, {b: 5})); + // function f12({a = eval("b")}, {b}) { return a } + // assertThrows(() => f12({}, {b: 4}), ReferenceError); + // assertEquals(4, f12({a: 4}, {b: 5})); + // function f13({a = eval("b")}, {b}) { 'use strict'; return a } + // assertThrows(() => f13({}, {b: 4}), ReferenceError); + // assertEquals(4, f13({a: 4}, {b: 5})); + // function f14({a = eval("'use strict'; b")}, {b}) { return a } + // assertThrows(() => f14({}, {b: 4}), ReferenceError); + // assertEquals(4, f14({a: 4}, {b: 5})); + + function f15({a = () => b}, {b}) { return a() } + assertEquals(4, f15({a: () => 4}, {b: 5})); + // function f16({a = () => eval("b")}, {b}) { return a() } + // assertEquals(4, f16({a: () => 4}, {b: 5})); + // function f17({a = () => eval("b")}, {b}) { 'use strict'; return a() } + // assertEquals(4, f17({a: () => 4}, {b: 5})); + // function f18({a = () => eval("'use strict'; b")}, {b}) { return a() } + // assertEquals(4, f18({a: () => 4}, {b: 5})); // TODO(caitp): TDZ for rest parameters is not working yet. - // function f30({x = a}, ...a) {} + // function f30({x = a}, ...a) { return x[0] } // assertThrows(() => f30({}), ReferenceError); - // function f31({x = eval("a")}, ...a) {} + // assertEquals(4, f30({a: [4]}, 5)); + // function f31({x = eval("a")}, ...a) { return x[0] } // assertThrows(() => f31({}), ReferenceError); - // function f32({x = eval("a")}, ...a) { 'use strict'; } + // assertEquals(4, f31({a: [4]}, 5)); + // function f32({x = eval("a")}, ...a) { 'use strict'; return x[0] } // assertThrows(() => f32({}), ReferenceError); - // function f33({x = eval("'use strict'; a")}, ...a) {} + // assertEquals(4, f32({a: [4]}, 5)); + // function f33({x = eval("'use strict'; a")}, ...a) { return x[0] } // assertThrows(() => f33({}), ReferenceError); + // assertEquals(4, f33({a: [4]}, 5)); + function f34({x = function() { return a }}, ...a) { return x()[0] } assertEquals(4, f34({}, 4)); function f35({x = () => a}, ...a) { return x()[0] } diff --git a/test/test262-es6/test262-es6.status b/test/test262-es6/test262-es6.status index 29ecd6c..8696f96 100644 --- a/test/test262-es6/test262-es6.status +++ b/test/test262-es6/test262-es6.status @@ -436,6 +436,9 @@ 'language/computed-property-names/class/static/method-symbol': [FAIL, FAIL_SLOPPY], 'language/computed-property-names/class/static/method-string': [FAIL, FAIL_SLOPPY], + # This should work as soon as rest parameters are re-implemented via desaguring. + 'language/expressions/arrow-function/syntax/early-errors/arrowparameters-cover-no-duplicates-rest': [PASS, FAIL], + # https://code.google.com/p/v8/issues/detail?id=2160 'language/expressions/arrow-function/syntax/arrowparameters-cover-initialize-1': [FAIL], 'language/expressions/arrow-function/syntax/arrowparameters-cover-initialize-2': [FAIL],