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

@@ -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) {
+  //          <empty>
+  //        } else {
+  //          break;
+  //        }
+  //        b
+  //        temp_x = x;
+  //     }
+  //  }
+
+  ASSERT(names->length() > 0);
+  Scope* for_scope = scope_;
+  ZoneList<Variable*> temps(names->length(), zone());
+
+  Block* outer_block = factory()->NewBlock(NULL, names->length() + 3, false,
+                                           RelocInfo::kNoPosition);
+  outer_block->AddStatement(init, zone());
+
+  Handle<String> temp_name = isolate()->factory()->dot_for_string();
+  Handle<Smi> smi0 = handle(Smi::FromInt(0), isolate());
+  Handle<Smi> 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<Variable*> 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<String>(), 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<String> 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;
 }
 
 

src/parser.h

@@ -718,6 +718,10 @@ class Parser : public ParserBase<ParserTraits> {
                                   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<String> name,

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) {

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();
@@ -433,12 +435,14 @@ 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();