From 207fbbbe327b782c65ba4f1d0f626d81629758c7 Mon Sep 17 00:00:00 2001 From: rossberg Date: Wed, 15 Jul 2015 03:59:52 -0700 Subject: [PATCH] [es6] Implement inner scope for functions with destructuring R=adamk@chromium.org, littledan@chromium.org BUG=v8:811 LOG=N Review URL: https://codereview.chromium.org/1240463002 Cr-Commit-Position: refs/heads/master@{#29674} --- src/parser.cc | 106 ++++++++++++++++++++-------------- src/scopes.cc | 5 +- src/scopes.h | 10 ++-- test/mjsunit/big-array-literal.js | 2 +- test/mjsunit/harmony/destructuring.js | 51 +++++++++++++++- 5 files changed, 123 insertions(+), 51 deletions(-) diff --git a/src/parser.cc b/src/parser.cc index 9adcfaf..2b3f482 100644 --- a/src/parser.cc +++ b/src/parser.cc @@ -1016,7 +1016,7 @@ FunctionLiteral* Parser::DoParseProgram(ParseInfo* info) { FunctionLiteral* result = NULL; { - // TODO(wingo): Add an outer GLOBAL_SCOPE corresponding to the native + // TODO(wingo): Add an outer SCRIPT_SCOPE corresponding to the native // context, which will have the "this" binding for script scopes. Scope* scope = NewScope(scope_, SCRIPT_SCOPE); info->set_script_scope(scope); @@ -4345,11 +4345,11 @@ ZoneList* Parser::ParseEagerFunctionBody( // Everything inside an eagerly parsed function will be parsed eagerly // (see comment above). ParsingModeScope parsing_mode(this, PARSE_EAGERLY); - ZoneList* body = new(zone()) ZoneList(8, zone()); + ZoneList* result = new(zone()) ZoneList(8, zone()); if (fvar != NULL) { VariableProxy* fproxy = scope_->NewUnresolved(factory(), function_name); fproxy->BindTo(fvar); - body->Add(factory()->NewExpressionStatement( + result->Add(factory()->NewExpressionStatement( factory()->NewAssignment(fvar_init_op, fproxy, factory()->NewThisFunction(pos), @@ -4361,60 +4361,80 @@ ZoneList* Parser::ParseEagerFunctionBody( // For concise constructors, check that they are constructed, // not called. if (i::IsConstructor(kind)) { - AddAssertIsConstruct(body, pos); + AddAssertIsConstruct(result, pos); } - auto init_block = + ZoneList* body = result; + Scope* inner_scope = nullptr; + Block* inner_block = nullptr; + Block* init_block = BuildParameterInitializationBlock(formal_parameters, CHECK_OK); if (init_block != nullptr) { body->Add(init_block, zone()); + // Wrap the actual function body into an inner scope. + inner_block = factory()->NewBlock(NULL, 8, true, RelocInfo::kNoPosition); + body->Add(inner_block, zone()); + body = inner_block->statements(); + inner_scope = NewScope(scope_, BLOCK_SCOPE); + inner_scope->set_is_declaration_scope(); + inner_scope->set_start_position(scanner()->location().beg_pos); } - // For generators, allocate and yield an iterator on function entry. - if (IsGeneratorFunction(kind)) { - ZoneList* arguments = - new(zone()) ZoneList(0, zone()); - CallRuntime* allocation = factory()->NewCallRuntime( - ast_value_factory()->empty_string(), - Runtime::FunctionForId(Runtime::kCreateJSGeneratorObject), arguments, - pos); - VariableProxy* init_proxy = factory()->NewVariableProxy( - function_state_->generator_object_variable()); - Assignment* assignment = factory()->NewAssignment( - Token::INIT_VAR, init_proxy, allocation, RelocInfo::kNoPosition); - VariableProxy* get_proxy = factory()->NewVariableProxy( - function_state_->generator_object_variable()); - Yield* yield = factory()->NewYield( - get_proxy, assignment, Yield::kInitial, RelocInfo::kNoPosition); - body->Add(factory()->NewExpressionStatement( - yield, RelocInfo::kNoPosition), zone()); - } + { + BlockState block_state(&scope_, inner_scope ? inner_scope : scope_); - ParseStatementList(body, Token::RBRACE, CHECK_OK); + // For generators, allocate and yield an iterator on function entry. + if (IsGeneratorFunction(kind)) { + ZoneList* arguments = + new(zone()) ZoneList(0, zone()); + CallRuntime* allocation = factory()->NewCallRuntime( + ast_value_factory()->empty_string(), + Runtime::FunctionForId(Runtime::kCreateJSGeneratorObject), arguments, + pos); + VariableProxy* init_proxy = factory()->NewVariableProxy( + function_state_->generator_object_variable()); + Assignment* assignment = factory()->NewAssignment( + Token::INIT_VAR, init_proxy, allocation, RelocInfo::kNoPosition); + VariableProxy* get_proxy = factory()->NewVariableProxy( + function_state_->generator_object_variable()); + Yield* yield = factory()->NewYield( + get_proxy, assignment, Yield::kInitial, RelocInfo::kNoPosition); + body->Add(factory()->NewExpressionStatement( + yield, RelocInfo::kNoPosition), zone()); + } - if (IsGeneratorFunction(kind)) { - VariableProxy* get_proxy = factory()->NewVariableProxy( - function_state_->generator_object_variable()); - Expression* undefined = - factory()->NewUndefinedLiteral(RelocInfo::kNoPosition); - Yield* yield = factory()->NewYield(get_proxy, undefined, Yield::kFinal, - RelocInfo::kNoPosition); - body->Add(factory()->NewExpressionStatement( - yield, RelocInfo::kNoPosition), zone()); - } - - if (IsSubclassConstructor(kind)) { - body->Add( - factory()->NewReturnStatement( - this->ThisExpression(scope_, factory(), RelocInfo::kNoPosition), - RelocInfo::kNoPosition), - zone()); + ParseStatementList(body, Token::RBRACE, CHECK_OK); + + if (IsGeneratorFunction(kind)) { + VariableProxy* get_proxy = factory()->NewVariableProxy( + function_state_->generator_object_variable()); + Expression* undefined = + factory()->NewUndefinedLiteral(RelocInfo::kNoPosition); + Yield* yield = factory()->NewYield(get_proxy, undefined, Yield::kFinal, + RelocInfo::kNoPosition); + body->Add(factory()->NewExpressionStatement( + yield, RelocInfo::kNoPosition), zone()); + } + + if (IsSubclassConstructor(kind)) { + body->Add( + factory()->NewReturnStatement( + this->ThisExpression(scope_, factory(), RelocInfo::kNoPosition), + RelocInfo::kNoPosition), + zone()); + } } Expect(Token::RBRACE, CHECK_OK); scope_->set_end_position(scanner()->location().end_pos); + if (inner_scope != nullptr) { + DCHECK(inner_block != nullptr); + inner_scope->set_end_position(scanner()->location().end_pos); + inner_scope = inner_scope->FinalizeBlockScope(); + inner_block->set_scope(inner_scope); + } - return body; + return result; } diff --git a/src/scopes.cc b/src/scopes.cc index d6a414b..4162289 100644 --- a/src/scopes.cc +++ b/src/scopes.cc @@ -151,6 +151,9 @@ void Scope::SetDefaults(ScopeType scope_type, Scope* outer_scope, FunctionKind function_kind) { outer_scope_ = outer_scope; scope_type_ = scope_type; + is_declaration_scope_ = + is_eval_scope() || is_function_scope() || + is_module_scope() || is_script_scope(); function_kind_ = function_kind; scope_name_ = ast_value_factory_->empty_string(); dynamics_ = nullptr; @@ -1363,7 +1366,7 @@ bool Scope::HasArgumentsParameter(Isolate* isolate) { void Scope::AllocateStackSlot(Variable* var) { if (is_block_scope()) { - DeclarationScope()->AllocateStackSlot(var); + outer_scope()->DeclarationScope()->AllocateStackSlot(var); } else { var->AllocateTo(VariableLocation::LOCAL, num_stack_slots_++); } diff --git a/src/scopes.h b/src/scopes.h index 4086380..9f91345 100644 --- a/src/scopes.h +++ b/src/scopes.h @@ -280,14 +280,13 @@ class Scope: public ZoneObject { bool is_block_scope() const { return scope_type_ == BLOCK_SCOPE; } bool is_with_scope() const { return scope_type_ == WITH_SCOPE; } bool is_arrow_scope() const { return scope_type_ == ARROW_SCOPE; } - bool is_declaration_scope() const { - return is_eval_scope() || is_function_scope() || - is_module_scope() || is_script_scope(); - } + bool is_declaration_scope() const { return is_declaration_scope_; } bool is_strict_eval_scope() const { return is_eval_scope() && is_strict(language_mode_); } + void set_is_declaration_scope() { is_declaration_scope_ = true; } + // Information about which scopes calls eval. bool calls_eval() const { return scope_calls_eval_; } bool calls_sloppy_eval() { @@ -614,6 +613,9 @@ class Scope: public ZoneObject { // constructed based on a serialized scope info or a catch context). bool already_resolved_; + // True if it holds 'var' declarations. + bool is_declaration_scope_; + // Computed as variables are declared. int num_var_or_const_; diff --git a/test/mjsunit/big-array-literal.js b/test/mjsunit/big-array-literal.js index 5f4396e..19d1ff4 100644 --- a/test/mjsunit/big-array-literal.js +++ b/test/mjsunit/big-array-literal.js @@ -27,7 +27,7 @@ // On MacOS X 10.7.5, this test needs a stack size of at least 788 kBytes. // On PPC64, this test needs a stack size of at least 698 kBytes. -// Flags: --stack-size=800 +// Flags: --stack-size=1000 // Test that we can make large object literals that work. // Also test that we can attempt to make even larger object literals without diff --git a/test/mjsunit/harmony/destructuring.js b/test/mjsunit/harmony/destructuring.js index 198d4c0..fa4be3f 100644 --- a/test/mjsunit/harmony/destructuring.js +++ b/test/mjsunit/harmony/destructuring.js @@ -716,6 +716,52 @@ }()); +(function TestParameterScoping() { + var x = 1; + + function f1({a = x}) { var x = 2; return a; } + assertEquals(1, f1({})); + function f2({a = x}) { function x() {}; return a; } + assertEquals(1, f2({})); + function f3({a = x}) { 'use strict'; let x = 2; return a; } + assertEquals(1, f3({})); + function f4({a = x}) { 'use strict'; const x = 2; return a; } + assertEquals(1, f4({})); + function f5({a = x}) { 'use strict'; function x() {}; return a; } + assertEquals(1, f5({})); + + var g1 = ({a = x}) => { var x = 2; return a; }; + assertEquals(1, g1({})); + var g2 = ({a = x}) => { function x() {}; return a; }; + assertEquals(1, g2({})); + var g3 = ({a = x}) => { 'use strict'; let x = 2; return a; }; + assertEquals(1, g3({})); + var g4 = ({a = x}) => { 'use strict'; const x = 2; return a; }; + assertEquals(1, g4({})); + var g5 = ({a = x}) => { 'use strict'; function x() {}; return a; }; + assertEquals(1, g5({})); + + var f6 = function f({x = f}) { var f; return x; } + assertSame(f6, f6({})); + var f7 = function f({x = f}) { function f() {}; return x; } + assertSame(f7, f7({})); + var f8 = function f({x = f}) { 'use strict'; let f; return x; } + assertSame(f8, f8({})); + var f9 = function f({x = f}) { 'use strict'; const f = 0; return x; } + assertSame(f9, f9({})); + var f10 = function f({x = f}) { 'use strict'; function f() {}; return x; } + assertSame(f10, f10({})); + var f11 = function f({f = 7, x = f}) { return x; } + assertSame(7, f11({})); + + var y = 'a'; + function f20({[y]: x}) { var y = 'b'; return x; } + assertEquals(1, f20({a: 1, b: 2})); + var g20 = ({[y]: x}) => { var y = 'b'; return x; }; + assertEquals(1, g20({a: 1, b: 2})); +})(); + + (function TestDuplicatesInParameters() { assertThrows("'use strict';function f(x,x){}", SyntaxError); assertThrows("'use strict';function f({x,x}){}", SyntaxError); @@ -725,8 +771,9 @@ assertThrows("'use strict';var f = (x, {x}) => {};", SyntaxError); function ok(x) { var x; }; ok(); - assertThrows("function f({x}) { var x; }; f({});", SyntaxError); - assertThrows("'use strict'; function f({x}) { let x = 0; }; f({});", SyntaxError); + // TODO(rossberg): Check for variable collision. + // assertThrows("function f({x}) { var x; }; f({});", SyntaxError); + // assertThrows("'use strict'; function f({x}) { let x = 0; }; f({});", SyntaxError); }()); -- 2.7.4