[parser] Refactor of Parse*Statement*, part 8

src/parsing/parser.cc

Index: src/parsing/parser.cc
diff --git a/src/parsing/parser.cc b/src/parsing/parser.cc
index a406b66ae32490e15c9c66db05696864132cdaf1..a6bd065d92a5f1eddeac4e36bf4a5e2ed1ad615c 100644
--- a/src/parsing/parser.cc
+++ b/src/parsing/parser.cc
@@ -1995,6 +1995,124 @@ Statement* Parser::InitializeForEachStatement(ForEachStatement* stmt,
   return stmt;
 }
 
+// Special case for legacy for (var ... = ... in ...)

[marja, 2016/09/20 19:36:30]

Could this comment also explain what it is rewritten to (like the comment below
about for-in/of - that is very illustrative).

[nickie, 2016/09/21 09:20:02]

Done, with a bit of reverse-engineering.  You may want to see the comment I
added, in case I'm missing something.

+Block* Parser::RewriteForVarInLegacy(ForInfo* for_info) {
+  const DeclarationParsingResult::Declaration& decl =
+      for_info->parsing_result.declarations[0];
+  if (!for_info->bound_names_are_lexical && decl.pattern->IsVariableProxy() &&
+      decl.initializer != nullptr) {
+    DCHECK(!allow_harmony_for_in());
+    ++use_counts_[v8::Isolate::kForInInitializer];
+    const AstRawString* name = decl.pattern->AsVariableProxy()->raw_name();
+    VariableProxy* single_var = NewUnresolved(name);
+    Block* init_block = factory()->NewBlock(
+        nullptr, 2, true, for_info->parsing_result.descriptor.declaration_pos);
+    init_block->statements()->Add(
+        factory()->NewExpressionStatement(
+            factory()->NewAssignment(Token::ASSIGN, single_var,
+                                     decl.initializer, kNoSourcePosition),
+            kNoSourcePosition),
+        zone());
+    return init_block;
+  } else {

[marja, 2016/09/20 19:36:30]

Style nit: else { } around the return is unnecessary and doesn't make the code
any clearer.

[nickie, 2016/09/21 09:20:02]

Done.

+    return nullptr;
+  }
+}
+
+// Rewrite a for-in/of statement of the form
+//
+//   for (let/const/var x in/of e) b
+//
+// into
+//
+//   {
+//     <let x' be a temporary variable>
+//     for (x' in/of e) {
+//       let/const/var x;
+//       x = x';
+//       b;
+//     }
+//     let x;  // for TDZ
+//   }
+std::pair<Block*, Expression*> Parser::DesugarBindingInForEachStatement(
+    ForInfo* for_info, bool* ok) {
+  DeclarationParsingResult::Declaration& decl =
+      for_info->parsing_result.declarations[0];
+  Variable* temp = NewTemporary(ast_value_factory()->dot_for_string());
+  auto each_initialization_block =
+      factory()->NewBlock(nullptr, 1, true, kNoSourcePosition);
+  {
+    auto descriptor = for_info->parsing_result.descriptor;
+    descriptor.declaration_pos = kNoSourcePosition;
+    descriptor.initialization_pos = kNoSourcePosition;
+    decl.initializer = factory()->NewVariableProxy(temp);
+
+    bool is_for_var_of =
+        for_info->mode == ForEachStatement::ITERATE &&
+        for_info->parsing_result.descriptor.mode == VariableMode::VAR;
+
+    PatternRewriter::DeclareAndInitializeVariables(
+        this, each_initialization_block, &descriptor, &decl,
+        (for_info->bound_names_are_lexical || is_for_var_of)
+            ? &for_info->bound_names
+            : nullptr,
+        CHECK_OK_VALUE(std::make_pair(nullptr, nullptr)));
+
+    // Annex B.3.5 prohibits the form
+    // `try {} catch(e) { for (var e of {}); }`
+    // So if we are parsing a statement like `for (var ... of ...)`
+    // we need to walk up the scope chain and look for catch scopes
+    // which have a simple binding, then compare their binding against
+    // all of the names declared in the init of the for-of we're
+    // parsing.
+    if (is_for_var_of) {
+      Scope* catch_scope = scope();
+      while (catch_scope != nullptr && !catch_scope->is_declaration_scope()) {
+        if (catch_scope->is_catch_scope()) {
+          auto name = catch_scope->catch_variable_name();
+          if (name !=

[marja, 2016/09/20 19:36:30]

Style nit: I'd combine these two nested if's:

if (name != ... && for_info->bound_names.Contains(name)) {
  error
}

[nickie, 2016/09/21 09:20:02]

Done.  I think the part "name != ... dot_catch_string" may be redundant.  Is
there any way the for loop can declare such a name?

[marja, 2016/09/21 10:31:42]

I was thinking the same, wondering if it's redundant... Idk. Probably yes.
Didn't investigate.

[nickie, 2016/09/21 10:33:05]

OK, let's play it safe and leave it.

+              ast_value_factory()
+                  ->dot_catch_string()) {  // i.e. is a simple binding
+            if (for_info->bound_names.Contains(name)) {
+              ReportMessageAt(for_info->parsing_result.bindings_loc,
+                              MessageTemplate::kVarRedeclaration, name);
+              *ok = false;
+              return std::make_pair(nullptr, nullptr);
+            }
+          }
+        }
+        catch_scope = catch_scope->outer_scope();
+      }
+    }
+  }
+
+  Block* body_block = factory()->NewBlock(NULL, 3, false, kNoSourcePosition);
+  body_block->statements()->Add(each_initialization_block, zone());
+  VariableProxy* temp_proxy = factory()->NewVariableProxy(
+      temp, for_info->each_loc.beg_pos, for_info->each_loc.end_pos);
+  return std::make_pair(body_block, temp_proxy);
+}
+
+// Create a TDZ for any lexically-bound names in for in/of statements.
+Block* Parser::CreateForEachStatementTDZ(Block* init_block, ForInfo* for_info,
+                                         bool* ok) {
+  if (for_info->bound_names_are_lexical) {
+    DCHECK_NULL(init_block);
+
+    init_block = factory()->NewBlock(nullptr, 1, false, kNoSourcePosition);
+
+    for (int i = 0; i < for_info->bound_names.length(); ++i) {
+      // TODO(adamk): This needs to be some sort of special
+      // INTERNAL variable that's invisible to the debugger
+      // but visible to everything else.
+      Declaration* tdz_decl = DeclareVariable(for_info->bound_names[i], LET,
+                                              kNoSourcePosition, CHECK_OK);
+      tdz_decl->proxy()->var()->set_initializer_position(position());
+    }
+  }
+  return init_block;
+}
+
 Statement* Parser::InitializeForOfStatement(ForOfStatement* for_of,
                                             Expression* each,
                                             Expression* iterable,
@@ -2119,9 +2237,8 @@ Statement* Parser::InitializeForOfStatement(ForOfStatement* for_of,
 }
 
 Statement* Parser::DesugarLexicalBindingsInForStatement(
-    Scope* inner_scope, VariableMode mode, ZoneList<const AstRawString*>* names,
     ForStatement* loop, Statement* init, Expression* cond, Statement* next,
-    Statement* body, bool* ok) {
+    Statement* body, Scope* inner_scope, ForInfo* for_info, bool* ok) {
   // ES6 13.7.4.8 specifies that on each loop iteration the let variables are
   // copied into a new environment.  Moreover, the "next" statement must be
   // evaluated not in the environment of the just completed iteration but in
@@ -2159,11 +2276,11 @@ Statement* Parser::DesugarLexicalBindingsInForStatement(
   //    }
   //  }
 
-  DCHECK(names->length() > 0);
-  ZoneList<Variable*> temps(names->length(), zone());
+  DCHECK(for_info->bound_names.length() > 0);
+  ZoneList<Variable*> temps(for_info->bound_names.length(), zone());
 
-  Block* outer_block =
-      factory()->NewBlock(NULL, names->length() + 4, false, kNoSourcePosition);
+  Block* outer_block = factory()->NewBlock(
+      NULL, for_info->bound_names.length() + 4, false, kNoSourcePosition);
 
   // Add statement: let/const x = i.
   outer_block->statements()->Add(init, zone());
@@ -2172,8 +2289,8 @@ Statement* Parser::DesugarLexicalBindingsInForStatement(
 
   // For each lexical variable x:
   //   make statement: temp_x = x.
-  for (int i = 0; i < names->length(); i++) {
-    VariableProxy* proxy = NewUnresolved(names->at(i));
+  for (int i = 0; i < for_info->bound_names.length(); i++) {
+    VariableProxy* proxy = NewUnresolved(for_info->bound_names[i]);
     Variable* temp = NewTemporary(temp_name);
     VariableProxy* temp_proxy = factory()->NewVariableProxy(temp);
     Assignment* assignment = factory()->NewAssignment(Token::ASSIGN, temp_proxy,
@@ -2217,14 +2334,15 @@ Statement* Parser::DesugarLexicalBindingsInForStatement(
   {
     BlockState block_state(&scope_state_, inner_scope);
 
-    Block* ignore_completion_block =
-        factory()->NewBlock(NULL, names->length() + 3, true, kNoSourcePosition);
-    ZoneList<Variable*> inner_vars(names->length(), zone());
+    Block* ignore_completion_block = factory()->NewBlock(
+        NULL, for_info->bound_names.length() + 3, true, kNoSourcePosition);

[marja, 2016/09/20 19:36:30]

Nit: pls update to nullptr while anyway modifying this line

[nickie, 2016/09/21 09:20:02]

Done.  Here and elsewhere.

+    ZoneList<Variable*> inner_vars(for_info->bound_names.length(), zone());
     // For each let variable x:
     //    make statement: let/const x = temp_x.
-    for (int i = 0; i < names->length(); i++) {
-      Declaration* decl =
-          DeclareVariable(names->at(i), mode, kNoSourcePosition, CHECK_OK);
+    for (int i = 0; i < for_info->bound_names.length(); i++) {
+      Declaration* decl = DeclareVariable(
+          for_info->bound_names[i], for_info->parsing_result.descriptor.mode,
+          kNoSourcePosition, CHECK_OK);
       inner_vars.Add(decl->proxy()->var(), zone());
       VariableProxy* temp_proxy = factory()->NewVariableProxy(temps.at(i));
       Assignment* assignment = factory()->NewAssignment(
@@ -2308,7 +2426,7 @@ Statement* Parser::DesugarLexicalBindingsInForStatement(
 
       // Make the comma-separated list of temp_x = x assignments.
       int inner_var_proxy_pos = scanner()->location().beg_pos;
-      for (int i = 0; i < names->length(); i++) {
+      for (int i = 0; i < for_info->bound_names.length(); i++) {
         VariableProxy* temp_proxy = factory()->NewVariableProxy(temps.at(i));
         VariableProxy* proxy =
             factory()->NewVariableProxy(inner_vars.at(i), inner_var_proxy_pos);