Make let variables fresh in each iteration of a for-loop.

src/parser.cc

Index: src/parser.cc
diff --git a/src/parser.cc b/src/parser.cc
index 5efcba6119225365a42fc50172420ae121757857..9557910c8413c441b00c8fd8d2fb1bd787764c5d 100644
--- a/src/parser.cc
+++ b/src/parser.cc
@@ -2837,12 +2837,175 @@ void Parser::InitializeForEachStatement(ForEachStatement* stmt,
 }
 
 
+Statement* Parser::Assign(Token::Value op,
+                          VariableProxy* dst,
+                          Expression* src,
+                          int pos) {
+  Assignment* x = factory()->NewAssignment(op, dst, src, pos);
+  return factory()->NewExpressionStatement(x, pos);
+}
+
+
+Variable* Parser::Temporary(Handle<String> name) {
+    return scope_->DeclarationScope()->NewTemporary(name);
+}
+
+Statement* Parser::DesugarLetBindingsInForStatement(
+    Scope* inner_scope, ZoneStringList* names, ForStatement* loop,
+    Statement* init, Expression* cond, Statement* next, 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 the loop condition is

[rossberg, 2014/05/22 12:55:44]

Nit: "The the"

[ulan, 2014/05/23 14:29:03]

Done.

+  // 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;
+  //     }
+  //  }

[rossberg, 2014/05/22 12:55:44]

So, eventually, there should be a comment here along the lines of:

"Similarly for other mutable lexical declarations (e.g. function). Immutable
declarations (like const) do not need to be rewritten."

[ulan, 2014/05/23 14:29:03]

As discussed offline, only lets and consts are allowed here.

+
+  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 = Temporary(temp_name);
+    VariableProxy* temp_proxy = factory()->NewVariableProxy(temp);
+    Statement* assignment = Assign(Token::ASSIGN, temp_proxy, proxy);
+    outer_block->AddStatement(assignment, zone());
+    temps.Add(temp, zone());
+  }
+
+  Variable* flag = Temporary(temp_name);
+  // Make statement: flag = 1.
+  {
+    VariableProxy* flag_proxy = factory()->NewVariableProxy(flag);
+    Expression* const1 = factory()->NewLiteral(smi1, RelocInfo::kNoPosition);
+    Statement* assignment = Assign(Token::ASSIGN, flag_proxy, const1);
+    outer_block->AddStatement(assignment, zone());
+  }
+
+  outer_block->AddStatement(loop, zone());
+  outer_block->set_scope(for_scope);
+  scope_ = inner_scope;
+
+  Statement* body = ParseStatement(NULL, CHECK_OK);

[rossberg, 2014/05/22 12:55:44]

It's somewhat inappropriate to do actual parsing in a function called Desugar. I
would pass this in as an argument.

[ulan, 2014/05/23 14:29:03]

Done.

+
+  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));
+    Statement* statement = Assign(Token::INIT_LET, proxy, temp_proxy, pos);
+    proxy->var()->set_initializer_position(pos);
+    inner_block->AddStatement(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);
+      clear_flag = Assign(Token::ASSIGN, flag_proxy, const0);
+    }
+    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);
+    Statement* statement = Assign(Token::ASSIGN, temp_proxy, proxy);
+    inner_block->AddStatement(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());

[rossberg, 2014/05/22 12:55:44]

lexical_bindings

[ulan, 2014/05/23 14:29:03]

Leaving as is.

 
   // 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

[rossberg, 2014/05/22 12:55:44]

"let bindings" -> "lexical declarations"

[ulan, 2014/05/23 14:29:03]

Leaving as is.

+  // 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);
@@ -3011,32 +3183,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, CHECK_OK);
+    scope_ = saved_scope;
+    for_scope->set_end_position(scanner()->location().end_pos);
   } else {
+    Statement* body = ParseStatement(NULL, CHECK_OK);
     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;
 }