From 3fcda0e5769a338f447ae771a2e82190ecbd2dad Mon Sep 17 00:00:00 2001 From: "ulan@chromium.org" Date: Mon, 26 May 2014 08:07:02 +0000 Subject: [PATCH] Make let variables fresh in each iteration of a for-loop. BUG=v8:2198 LOG=N TEST=mjsunit/harmony/block-for R=rossberg@chromium.org Review URL: https://codereview.chromium.org/292743009 git-svn-id: https://v8.googlecode.com/svn/branches/bleeding_edge@21480 ce2b1a6d-e550-0410-aec6-3dcde31c8c00 --- src/parser.cc | 211 ++++++++++++++++++++++++++---- src/parser.h | 4 + test/mjsunit/harmony/block-for.js | 40 +++++- test/mjsunit/harmony/debug-blockscopes.js | 12 +- 4 files changed, 236 insertions(+), 31 deletions(-) diff --git a/src/parser.cc b/src/parser.cc index 572b0f4..a08148c 100644 --- a/src/parser.cc +++ b/src/parser.cc @@ -2837,12 +2837,175 @@ void Parser::InitializeForEachStatement(ForEachStatement* stmt, } +Statement* Parser::DesugarLetBindingsInForStatement( + Scope* inner_scope, ZoneStringList* names, ForStatement* loop, + Statement* init, Expression* cond, Statement* next, Statement* body, + bool* ok) { + // ES6 13.6.3.4 specifies that on each loop iteration the let variables are + // copied into a new environment. After copying, the "next" statement of the + // loop is executed to update the loop variables. The loop condition is + // checked and the loop body is executed. + // + // We rewrite a for statement of the form + // + // for (let x = i; cond; next) body + // + // into + // + // { + // let x = i; + // temp_x = x; + // flag = 1; + // for (;;) { + // let x = temp_x; + // if (flag == 1) { + // flag = 0; + // } else { + // next; + // } + // if (cond) { + // + // } else { + // break; + // } + // b + // temp_x = x; + // } + // } + + ASSERT(names->length() > 0); + Scope* for_scope = scope_; + ZoneList temps(names->length(), zone()); + + Block* outer_block = factory()->NewBlock(NULL, names->length() + 3, false, + RelocInfo::kNoPosition); + outer_block->AddStatement(init, zone()); + + Handle temp_name = isolate()->factory()->dot_for_string(); + Handle smi0 = handle(Smi::FromInt(0), isolate()); + Handle smi1 = handle(Smi::FromInt(1), isolate()); + + + // For each let variable x: + // make statement: temp_x = x. + for (int i = 0; i < names->length(); i++) { + VariableProxy* proxy = + NewUnresolved(names->at(i), LET, Interface::NewValue()); + Variable* temp = scope_->DeclarationScope()->NewTemporary(temp_name); + VariableProxy* temp_proxy = factory()->NewVariableProxy(temp); + Assignment* assignment = factory()->NewAssignment( + Token::ASSIGN, temp_proxy, proxy, RelocInfo::kNoPosition); + Statement* assignment_statement = factory()->NewExpressionStatement( + assignment, RelocInfo::kNoPosition); + outer_block->AddStatement(assignment_statement, zone()); + temps.Add(temp, zone()); + } + + Variable* flag = scope_->DeclarationScope()->NewTemporary(temp_name); + // Make statement: flag = 1. + { + VariableProxy* flag_proxy = factory()->NewVariableProxy(flag); + Expression* const1 = factory()->NewLiteral(smi1, RelocInfo::kNoPosition); + Assignment* assignment = factory()->NewAssignment( + Token::ASSIGN, flag_proxy, const1, RelocInfo::kNoPosition); + Statement* assignment_statement = factory()->NewExpressionStatement( + assignment, RelocInfo::kNoPosition); + outer_block->AddStatement(assignment_statement, zone()); + } + + outer_block->AddStatement(loop, zone()); + outer_block->set_scope(for_scope); + scope_ = inner_scope; + + Block* inner_block = factory()->NewBlock(NULL, 2 * names->length() + 3, + false, RelocInfo::kNoPosition); + int pos = scanner()->location().beg_pos; + ZoneList inner_vars(names->length(), zone()); + + // For each let variable x: + // make statement: let x = temp_x. + for (int i = 0; i < names->length(); i++) { + VariableProxy* proxy = + NewUnresolved(names->at(i), LET, Interface::NewValue()); + Declaration* declaration = + factory()->NewVariableDeclaration(proxy, LET, scope_, pos); + Declare(declaration, true, CHECK_OK); + inner_vars.Add(declaration->proxy()->var(), zone()); + VariableProxy* temp_proxy = factory()->NewVariableProxy(temps.at(i)); + Assignment* assignment = factory()->NewAssignment( + Token::INIT_LET, proxy, temp_proxy, pos); + Statement* assignment_statement = factory()->NewExpressionStatement( + assignment, pos); + proxy->var()->set_initializer_position(pos); + inner_block->AddStatement(assignment_statement, zone()); + } + + // Make statement: if (flag == 1) { flag = 0; } else { next; }. + { + Expression* compare = NULL; + // Make compare expresion: flag == 1. + { + Expression* const1 = factory()->NewLiteral(smi1, RelocInfo::kNoPosition); + VariableProxy* flag_proxy = factory()->NewVariableProxy(flag); + compare = factory()->NewCompareOperation( + Token::EQ, flag_proxy, const1, RelocInfo::kNoPosition); + } + Statement* clear_flag = NULL; + // Make statement: flag = 0. + { + VariableProxy* flag_proxy = factory()->NewVariableProxy(flag); + Expression* const0 = factory()->NewLiteral(smi0, RelocInfo::kNoPosition); + Assignment* assignment = factory()->NewAssignment( + Token::ASSIGN, flag_proxy, const0, RelocInfo::kNoPosition); + clear_flag = factory()->NewExpressionStatement(assignment, pos); + } + Statement* clear_flag_or_next = factory()->NewIfStatement( + compare, clear_flag, next, RelocInfo::kNoPosition); + inner_block->AddStatement(clear_flag_or_next, zone()); + } + + + // Make statement: if (cond) { } else { break; }. + { + Statement* empty = factory()->NewEmptyStatement(RelocInfo::kNoPosition); + BreakableStatement* t = LookupBreakTarget(Handle(), CHECK_OK); + Statement* stop = factory()->NewBreakStatement(t, RelocInfo::kNoPosition); + Statement* if_not_cond_break = factory()->NewIfStatement( + cond, empty, stop, RelocInfo::kNoPosition); + inner_block->AddStatement(if_not_cond_break, zone()); + } + + inner_block->AddStatement(body, zone()); + + // For each let variable x: + // make statement: temp_x = x; + for (int i = 0; i < names->length(); i++) { + VariableProxy* temp_proxy = factory()->NewVariableProxy(temps.at(i)); + int pos = scanner()->location().end_pos; + VariableProxy* proxy = factory()->NewVariableProxy(inner_vars.at(i), pos); + Assignment* assignment = factory()->NewAssignment( + Token::ASSIGN, temp_proxy, proxy, RelocInfo::kNoPosition); + Statement* assignment_statement = factory()->NewExpressionStatement( + assignment, RelocInfo::kNoPosition); + inner_block->AddStatement(assignment_statement, zone()); + } + + inner_scope->set_end_position(scanner()->location().end_pos); + inner_block->set_scope(inner_scope); + scope_ = for_scope; + + loop->Initialize(NULL, NULL, NULL, inner_block); + return outer_block; +} + + Statement* Parser::ParseForStatement(ZoneStringList* labels, bool* ok) { // ForStatement :: // 'for' '(' Expression? ';' Expression? ';' Expression? ')' Statement int pos = peek_position(); Statement* init = NULL; + ZoneStringList let_bindings(1, zone()); // Create an in-between scope for let-bound iteration variables. Scope* saved_scope = scope_; @@ -2894,8 +3057,8 @@ Statement* Parser::ParseForStatement(ZoneStringList* labels, bool* ok) { Handle name; VariableDeclarationProperties decl_props = kHasNoInitializers; Block* variable_statement = - ParseVariableDeclarations(kForStatement, &decl_props, NULL, &name, - CHECK_OK); + ParseVariableDeclarations(kForStatement, &decl_props, &let_bindings, + &name, CHECK_OK); bool accept_IN = !name.is_null() && decl_props != kHasInitializers; bool accept_OF = decl_props == kHasNoInitializers; ForEachStatement::VisitMode mode; @@ -2998,6 +3161,15 @@ Statement* Parser::ParseForStatement(ZoneStringList* labels, bool* ok) { // Parsed initializer at this point. Expect(Token::SEMICOLON, CHECK_OK); + // If there are let bindings, then condition and the next statement of the + // for loop must be parsed in a new scope. + Scope* inner_scope = NULL; + if (let_bindings.length() > 0) { + inner_scope = NewScope(for_scope, BLOCK_SCOPE); + inner_scope->set_start_position(scanner()->location().beg_pos); + scope_ = inner_scope; + } + Expression* cond = NULL; if (peek() != Token::SEMICOLON) { cond = ParseExpression(true, CHECK_OK); @@ -3012,31 +3184,22 @@ Statement* Parser::ParseForStatement(ZoneStringList* labels, bool* ok) { Expect(Token::RPAREN, CHECK_OK); Statement* body = ParseStatement(NULL, CHECK_OK); - scope_ = saved_scope; - for_scope->set_end_position(scanner()->location().end_pos); - for_scope = for_scope->FinalizeBlockScope(); - if (for_scope != NULL) { - // Rewrite a for statement of the form - // - // for (let x = i; c; n) b - // - // into - // - // { - // let x = i; - // for (; c; n) b - // } - ASSERT(init != NULL); - Block* result = factory()->NewBlock(NULL, 2, false, RelocInfo::kNoPosition); - result->AddStatement(init, zone()); - result->AddStatement(loop, zone()); - result->set_scope(for_scope); - loop->Initialize(NULL, cond, next, body); - return result; + + Statement* result = NULL; + if (let_bindings.length() > 0) { + scope_ = for_scope; + result = DesugarLetBindingsInForStatement(inner_scope, &let_bindings, loop, + init, cond, next, body, CHECK_OK); + scope_ = saved_scope; + for_scope->set_end_position(scanner()->location().end_pos); } else { loop->Initialize(init, cond, next, body); - return loop; + result = loop; + scope_ = saved_scope; + for_scope->set_end_position(scanner()->location().end_pos); + for_scope->FinalizeBlockScope(); } + return result; } diff --git a/src/parser.h b/src/parser.h index 10be170..7f9f9f2 100644 --- a/src/parser.h +++ b/src/parser.h @@ -718,6 +718,10 @@ class Parser : public ParserBase { Expression* each, Expression* subject, Statement* body); + Statement* DesugarLetBindingsInForStatement( + Scope* inner_scope, ZoneStringList* names, ForStatement* loop, + Statement* init, Expression* cond, Statement* next, Statement* body, + bool* ok); FunctionLiteral* ParseFunctionLiteral( Handle name, diff --git a/test/mjsunit/harmony/block-for.js b/test/mjsunit/harmony/block-for.js index e84f0d2..2c11246 100644 --- a/test/mjsunit/harmony/block-for.js +++ b/test/mjsunit/harmony/block-for.js @@ -102,7 +102,7 @@ assertThrows("function foo() { 'use strict'; for (let x, y = 4 in {}) { } }", Sy assertThrows("function foo() { 'use strict'; for (let x = 3, y = 4 in {}) { } }", SyntaxError); -// In a normal for statement the iteration variable is not +// In a normal for statement the iteration variable is // freshly allocated for each iteration. function closures1() { let a = []; @@ -110,7 +110,7 @@ function closures1() { a.push(function () { return i; }); } for (let j = 0; j < 5; ++j) { - assertEquals(5, a[j]()); + assertEquals(j, a[j]()); } } closures1(); @@ -123,13 +123,45 @@ function closures2() { b.push(function () { return j; }); } for (let k = 0; k < 5; ++k) { - assertEquals(5, a[k]()); - assertEquals(15, b[k]()); + assertEquals(k, a[k]()); + assertEquals(k + 10, b[k]()); } } closures2(); +function closure_in_for_init() { + let a = []; + for (let i = 0, f = function() { return i }; i < 5; ++i) { + a.push(f); + } + for (let k = 0; k < 5; ++k) { + assertEquals(0, a[k]()); + } +} +closure_in_for_init(); + + +function closure_in_for_cond() { + let a = []; + for (let i = 0; a.push(function () { return i; }), i < 5; ++i) { } + for (let k = 0; k < 5; ++k) { + assertEquals(k, a[k]()); + } +} +closure_in_for_next(); + + +function closure_in_for_next() { + let a = []; + for (let i = 0; i < 5; a.push(function () { return i; }), ++i) { } + for (let k = 0; k < 5; ++k) { + assertEquals(k + 1, a[k]()); + } +} +closure_in_for_next(); + + // In a for-in statement the iteration variable is fresh // for earch iteration. function closures3(x) { diff --git a/test/mjsunit/harmony/debug-blockscopes.js b/test/mjsunit/harmony/debug-blockscopes.js index f56a306..ee281d9 100644 --- a/test/mjsunit/harmony/debug-blockscopes.js +++ b/test/mjsunit/harmony/debug-blockscopes.js @@ -412,10 +412,12 @@ function for_loop_3() { listener_delegate = function(exec_state) { CheckScopeChain([debug.ScopeType.Block, + debug.ScopeType.Block, debug.ScopeType.Local, debug.ScopeType.Global], exec_state); CheckScopeContent({x:3}, 0, exec_state); - CheckScopeContent({}, 1, exec_state); + CheckScopeContent({x:3}, 1, exec_state); + CheckScopeContent({}, 2, exec_state); }; for_loop_3(); EndTest(); @@ -434,11 +436,13 @@ function for_loop_4() { listener_delegate = function(exec_state) { CheckScopeChain([debug.ScopeType.Block, debug.ScopeType.Block, + debug.ScopeType.Block, debug.ScopeType.Local, debug.ScopeType.Global], exec_state); CheckScopeContent({x:5}, 0, exec_state); CheckScopeContent({x:3}, 1, exec_state); - CheckScopeContent({}, 2, exec_state); + CheckScopeContent({x:3}, 2, exec_state); + CheckScopeContent({}, 3, exec_state); }; for_loop_4(); EndTest(); @@ -455,10 +459,12 @@ function for_loop_5() { listener_delegate = function(exec_state) { CheckScopeChain([debug.ScopeType.Block, + debug.ScopeType.Block, debug.ScopeType.Local, debug.ScopeType.Global], exec_state); CheckScopeContent({x:3,y:5}, 0, exec_state); - CheckScopeContent({}, 1, exec_state); + CheckScopeContent({x:3,y:5}, 1, exec_state); + CheckScopeContent({}, 2, exec_state); }; for_loop_5(); EndTest(); -- 2.7.4