[es6] Implement for-of iterator finalization

src/parsing/parser.cc

Index: src/parsing/parser.cc
diff --git a/src/parsing/parser.cc b/src/parsing/parser.cc
index 444b20cbf87352cdd4b214938e837c2a7b3b2b52..4600719dbb0ef45d1f9802eeaea2458ea8536a35 100644
--- a/src/parsing/parser.cc
+++ b/src/parsing/parser.cc
@@ -3336,6 +3336,7 @@ void Parser::InitializeForEachStatement(ForEachStatement* stmt,
     }
 
     for_of->Initialize(each, subject, body,
+                       iterator,
                        assign_iterator,
                        next_result,
                        result_done,
@@ -3613,9 +3614,6 @@ Statement* Parser::DesugarLexicalBindingsInForStatement(
 
 Statement* Parser::ParseForStatement(ZoneList<const AstRawString*>* labels,
                                      bool* ok) {
-  // ForStatement ::
-  //   'for' '(' Expression? ';' Expression? ';' Expression? ')' Statement
-
   int stmt_pos = peek_position();
   Statement* init = NULL;
   ZoneList<const AstRawString*> lexical_bindings(1, zone());
@@ -3752,39 +3750,44 @@ Statement* Parser::ParseForStatement(ZoneList<const AstRawString*>* labels,
         }
         body_scope->set_end_position(scanner()->location().end_pos);
         body_scope = body_scope->FinalizeBlockScope();
-          body_block->set_scope(body_scope);
-
-          // Create a TDZ for any lexically-bound names.
-          if (IsLexicalVariableMode(parsing_result.descriptor.mode)) {
-            DCHECK_NULL(init_block);
-
-            init_block =
-                factory()->NewBlock(nullptr, 1, false, RelocInfo::kNoPosition);
-
-            for (int i = 0; i < lexical_bindings.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.
-              VariableProxy* tdz_proxy =
-                  NewUnresolved(lexical_bindings[i], LET);
-              Declaration* tdz_decl = factory()->NewVariableDeclaration(
-                  tdz_proxy, LET, scope_, RelocInfo::kNoPosition);
-              Variable* tdz_var = Declare(
-                  tdz_decl, DeclarationDescriptor::NORMAL, true, CHECK_OK);
-              tdz_var->set_initializer_position(position());
-            }
+        body_block->set_scope(body_scope);
+
+        // Create a TDZ for any lexically-bound names.
+        if (IsLexicalVariableMode(parsing_result.descriptor.mode)) {
+          DCHECK_NULL(init_block);
+
+          init_block =
+              factory()->NewBlock(nullptr, 1, false, RelocInfo::kNoPosition);
+
+          for (int i = 0; i < lexical_bindings.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.
+            VariableProxy* tdz_proxy =
+                NewUnresolved(lexical_bindings[i], LET);
+            Declaration* tdz_decl = factory()->NewVariableDeclaration(
+                tdz_proxy, LET, scope_, RelocInfo::kNoPosition);
+            Variable* tdz_var = Declare(
+                tdz_decl, DeclarationDescriptor::NORMAL, true, CHECK_OK);
+            tdz_var->set_initializer_position(position());
           }
+        }
+
+        Statement* final_loop = loop->IsForOfStatement()
+            ? FinalizeForOfStatement(
+                loop->AsForOfStatement(), RelocInfo::kNoPosition)
+            : loop;

[Dan Ehrenberg, 2016/02/16 21:18:30]

I'm not sure if any of this is risky (the nopos part?); if so, we may want to do
it only behind a flag.

[rossberg, 2016/02/17 12:54:45]

Good point, added flag.

 
         for_scope->set_end_position(scanner()->location().end_pos);
         for_scope = for_scope->FinalizeBlockScope();
         // Parsed for-in loop w/ variable declarations.
         if (init_block != nullptr) {
-          init_block->statements()->Add(loop, zone());
+          init_block->statements()->Add(final_loop, zone());
           init_block->set_scope(for_scope);
           return init_block;
         } else {
           DCHECK_NULL(for_scope);
-          return loop;
+          return final_loop;
         }
       } else {
         init = parsing_result.BuildInitializationBlock(
@@ -3847,21 +3850,28 @@ Statement* Parser::ParseForStatement(ZoneList<const AstRawString*>* labels,
         // expressions in head of the loop should actually have variables
         // resolved in the outer scope.
         Scope* body_scope = NewScope(for_scope, BLOCK_SCOPE);
-        BlockState block_state(&scope_, body_scope);
-        Block* block =
-            factory()->NewBlock(NULL, 1, false, RelocInfo::kNoPosition);
-        Statement* body = ParseSubStatement(NULL, CHECK_OK);
-        block->statements()->Add(body, zone());
-        InitializeForEachStatement(loop, expression, enumerable, block,
-                                   is_destructuring);
-        body_scope->set_end_position(scanner()->location().end_pos);
-        body_scope = body_scope->FinalizeBlockScope();
-        block->set_scope(body_scope);
+        {
+          BlockState block_state(&scope_, body_scope);
+          Block* block =
+              factory()->NewBlock(NULL, 1, false, RelocInfo::kNoPosition);
+          Statement* body = ParseSubStatement(NULL, CHECK_OK);
+          block->statements()->Add(body, zone());
+          InitializeForEachStatement(loop, expression, enumerable, block,
+                                     is_destructuring);
+          body_scope->set_end_position(scanner()->location().end_pos);
+          body_scope = body_scope->FinalizeBlockScope();
+          block->set_scope(body_scope);
+        }
+
+        Statement* final_loop = loop->IsForOfStatement()
+            ? FinalizeForOfStatement(
+                loop->AsForOfStatement(), RelocInfo::kNoPosition)
+            : loop;
+
         for_scope->set_end_position(scanner()->location().end_pos);
         for_scope = for_scope->FinalizeBlockScope();
         DCHECK(for_scope == nullptr);
-        // Parsed for-in loop.
-        return loop;
+        return final_loop;
 
       } else {
         init = factory()->NewExpressionStatement(expression, lhs_beg_pos);
@@ -5741,7 +5751,7 @@ Expression* Parser::RewriteSpreads(ArrayLiteral* lit) {
           ForEachStatement::ITERATE, nullptr, RelocInfo::kNoPosition);
       ForOfStatement* for_of = loop->AsForOfStatement();
       for_of->Initialize(factory()->NewVariableProxy(each), subject,
-                         append_body, assign_iterator, next_element,
+                         append_body, iterator, assign_iterator, next_element,
                          element_done, assign_each);
       do_block->statements()->Add(for_of, zone());
     }
@@ -5899,7 +5909,6 @@ Expression* ParserTraits::RewriteYieldStar(
   auto scope = parser_->scope_;
   auto zone = parser_->zone();
 
-  Statement* skip = factory->NewEmptyStatement(nopos);
 
   // Forward definition for break/continue statements.
   WhileStatement* loop = factory->NewWhileStatement(nullptr, nopos);
@@ -5971,8 +5980,8 @@ Expression* ParserTraits::RewriteYieldStar(
       throw_call = factory->NewExpressionStatement(call, nopos);
     }
 
-    validate_iterator =
-        factory->NewIfStatement(is_receiver_call, skip, throw_call, nopos);
+    validate_iterator = factory->NewIfStatement(
+        is_receiver_call, factory->NewEmptyStatement(nopos), throw_call, nopos);
   }
 
 
@@ -6015,8 +6024,8 @@ Expression* ParserTraits::RewriteYieldStar(
       throw_call = factory->NewExpressionStatement(call, nopos);
     }
 
-    validate_next_output =
-        factory->NewIfStatement(is_receiver_call, skip, throw_call, nopos);
+    validate_next_output = factory->NewIfStatement(
+        is_receiver_call, factory->NewEmptyStatement(nopos), throw_call, nopos);
   }
 
 
@@ -6052,11 +6061,12 @@ Expression* ParserTraits::RewriteYieldStar(
     Statement* throw_call = factory->NewExpressionStatement(call, nopos);
 
     Block* then = factory->NewBlock(nullptr, 4+1, false, nopos);
-    BuildIteratorClose(then->statements(), var_iterator, Nothing<Variable*>(),
-                       Nothing<Variable*>());
+    Variable* var_tmp = scope->NewTemporary(avfactory->empty_string());
+    BuildIteratorClose(
+        then->statements(), var_iterator, Nothing<Variable*>(), var_tmp);
     then->statements()->Add(throw_call, zone);
-    check_throw =
-        factory->NewIfStatement(condition, then, skip, nopos);
+    check_throw = factory->NewIfStatement(
+        condition, then, factory->NewEmptyStatement(nopos), nopos);
   }
 
 
@@ -6095,8 +6105,8 @@ Expression* ParserTraits::RewriteYieldStar(
       throw_call = factory->NewExpressionStatement(call, nopos);
     }
 
-    validate_throw_output =
-        factory->NewIfStatement(is_receiver_call, skip, throw_call, nopos);
+    validate_throw_output = factory->NewIfStatement(
+        is_receiver_call, factory->NewEmptyStatement(nopos), throw_call, nopos);
   }
 
 
@@ -6108,7 +6118,8 @@ Expression* ParserTraits::RewriteYieldStar(
         factory->NewStringLiteral(avfactory->done_string(), nopos);
     Expression* property = factory->NewProperty(output_proxy, literal, nopos);
     BreakStatement* break_loop = factory->NewBreakStatement(loop, nopos);
-    if_done = factory->NewIfStatement(property, break_loop, skip, nopos);
+    if_done = factory->NewIfStatement(
+        property, break_loop, factory->NewEmptyStatement(nopos), nopos);
   }
 
 
@@ -6227,8 +6238,7 @@ Expression* ParserTraits::RewriteYieldStar(
     case_next->Add(factory->NewBreakStatement(switch_mode, nopos), zone);
 
     auto case_return = new (zone) ZoneList<Statement*>(5, zone);
-    BuildIteratorClose(
-        case_return, var_iterator, Just(var_input), Just(var_output));
+    BuildIteratorClose(case_return, var_iterator, Just(var_input), var_output);
     case_return->Add(factory->NewBreakStatement(switch_mode, nopos), zone);
 
     auto case_throw = new (zone) ZoneList<Statement*>(5, zone);
@@ -6295,16 +6305,28 @@ Expression* ParserTraits::RewriteYieldStar(
 void ParserTraits::BuildIteratorClose(ZoneList<Statement*>* statements,
                                       Variable* iterator,
                                       Maybe<Variable*> input,
-                                      Maybe<Variable*> output) {
+                                      Variable* var_output) {
+  //
+  // This function adds four statements to [statements], corresponding to the
+  // following code:
+  //
+  //   let iteratorReturn = iterator.return;
+  //   if (IS_NULL_OR_UNDEFINED(iteratorReturn) return |input|;
+  //   output = %_Call(iteratorReturn, iterator|, input|);
+  //   if (!IS_RECEIVER(output)) %ThrowIterResultNotAnObject(output);
+  //
+  // Here, |...| denotes optional parts, depending on the presence of the

[Dan Ehrenberg, 2016/02/16 21:18:30]

Given the below comment (iteratorReturn shouldn't be passed input per spec), the
caller could set input to undefined when it's not provided, rather than making
this a Maybe.

[rossberg, 2016/02/17 12:54:45]

Done.

+  // input variable.
+  //
+
   const int nopos = RelocInfo::kNoPosition;
   auto factory = parser_->factory();
   auto avfactory = parser_->ast_value_factory();
-  auto scope = parser_->scope_;
   auto zone = parser_->zone();
-  Statement* skip = factory->NewEmptyStatement(nopos);
+
 
   // let iteratorReturn = iterator.return;
-  Variable* var = scope->NewTemporary(avfactory->empty_string());
+  Variable* var_return = var_output;  // Reusing the output variable.
   Statement* get_return;
   {
     Expression* iterator_proxy = factory->NewVariableProxy(iterator);
@@ -6312,18 +6334,18 @@ void ParserTraits::BuildIteratorClose(ZoneList<Statement*>* statements,
         factory->NewStringLiteral(avfactory->return_string(), nopos);
     Expression* property =
         factory->NewProperty(iterator_proxy, literal, nopos);
-    Expression* return_proxy = factory->NewVariableProxy(var);
+    Expression* return_proxy = factory->NewVariableProxy(var_return);
     Expression* assignment = factory->NewAssignment(
         Token::ASSIGN, return_proxy, property, nopos);
     get_return = factory->NewExpressionStatement(assignment, nopos);
   }
 
-  // if (IS_NULL_OR_UNDEFINED(iteratorReturn) return;  OR
-  // if (IS_NULL_OR_UNDEFINED(iteratorReturn) return input;
+
+  // if (IS_NULL_OR_UNDEFINED(iteratorReturn) return |input|;
   Statement* check_return;
   {
     Expression* condition = factory->NewCompareOperation(
-        Token::EQ, factory->NewVariableProxy(var),
+        Token::EQ, factory->NewVariableProxy(var_return),
         factory->NewNullLiteral(nopos), nopos);
 
     Expression* value = input.IsJust() ?
@@ -6332,16 +6354,16 @@ void ParserTraits::BuildIteratorClose(ZoneList<Statement*>* statements,
 
     Statement* return_undefined = factory->NewReturnStatement(value, nopos);
 
-    check_return =
-        factory->NewIfStatement(condition, return_undefined, skip, nopos);
+    check_return = factory->NewIfStatement(
+        condition, return_undefined, factory->NewEmptyStatement(nopos), nopos);
   }
 
-  // let output = %_Call(iteratorReturn, iterator);  OR
-  // output = %_Call(iteratorReturn, iterator, input);
+
+  // output = %_Call(iteratorReturn, iterator|, input|);

[Dan Ehrenberg, 2016/02/16 21:18:30]

Not sure why you're passing input to return. This doesn't seem to be there in
the spec: IteratorClose always calls the "return" method with no arguments:
http://tc39.github.io/ecma262/#sec-iteratorclose

[rossberg, 2016/02/17 12:54:45]

Fixed.

   Statement* call_return;
   {
     auto args = new (zone) ZoneList<Expression*>(3, zone);
-    args->Add(factory->NewVariableProxy(var), zone);
+    args->Add(factory->NewVariableProxy(var_return), zone);
     args->Add(factory->NewVariableProxy(iterator), zone);
     if (input.IsJust()) {
       args->Add(factory->NewVariableProxy(input.FromJust()), zone);
@@ -6349,20 +6371,20 @@ void ParserTraits::BuildIteratorClose(ZoneList<Statement*>* statements,
 
     Expression* call =
         factory->NewCallRuntime(Runtime::kInlineCall, args, nopos);
-    Expression* output_proxy = factory->NewVariableProxy(
-        output.IsJust() ? output.FromJust() : var);
+    Expression* output_proxy = factory->NewVariableProxy(var_output);
     Expression* assignment = factory->NewAssignment(
         Token::ASSIGN, output_proxy, call, nopos);
     call_return = factory->NewExpressionStatement(assignment, nopos);
   }
 
-  // if (!IS_RECEIVER(output)) %ThrowIterResultNotAnObject(output);
+
+  // if (!IS_RECEIVER(output)) %ThrowIteratorResultNotAnObject(output);
   Statement* validate_output;
   {
     Expression* is_receiver_call;
     {
       auto args = new (zone) ZoneList<Expression*>(1, zone);
-      args->Add(factory->NewVariableProxy(var), zone);
+      args->Add(factory->NewVariableProxy(var_output), zone);
       is_receiver_call =
           factory->NewCallRuntime(Runtime::kInlineIsJSReceiver, args, nopos);
     }
@@ -6370,16 +6392,17 @@ void ParserTraits::BuildIteratorClose(ZoneList<Statement*>* statements,
     Statement* throw_call;
     {
       auto args = new (zone) ZoneList<Expression*>(1, zone);
-      args->Add(factory->NewVariableProxy(var), zone);
+      args->Add(factory->NewVariableProxy(var_output), zone);
       Expression* call = factory->NewCallRuntime(
           Runtime::kThrowIteratorResultNotAnObject, args, nopos);
       throw_call = factory->NewExpressionStatement(call, nopos);
     }
 
-    validate_output =
-        factory->NewIfStatement(is_receiver_call, skip, throw_call, nopos);
+    validate_output = factory->NewIfStatement(
+        is_receiver_call, factory->NewEmptyStatement(nopos), throw_call, nopos);
   }
 
+
   statements->Add(get_return, zone);
   statements->Add(check_return, zone);
   statements->Add(call_return, zone);
@@ -6387,5 +6410,353 @@ void ParserTraits::BuildIteratorClose(ZoneList<Statement*>* statements,
 }
 
 
+// Runtime encoding of different completion modes.
+enum ForOfLoopCompletion { THROW, RETURN_OR_BREAK, NORMAL };

[Dan Ehrenberg, 2016/02/16 21:18:30]

This seems to correspond to JSGeneratorObject::ResumeMode. Can you reuse that
somehow (maybe putting it elsewhere if the names feel funny)?

[rossberg, 2016/02/17 12:54:45]

Here it only talks about the completion of the loop body. Also, it cannot
distinguish RETURN and BREAK, which would make it a bit misleading to reuse the
other. Changed names to clarify.

+
+void ParserTraits::BuildIteratorCloseForCompletion(
+    ZoneList<Statement*>* statements, Variable* iterator,
+    Variable* completion) {
+  //
+  // This function adds two statements to [statements], corresponding to the
+  // following code:
+  //
+  //   let iteratorReturn = iterator.return;
+  //   if (!IS_NULL_OR_UNDEFINED(iteratorReturn)) {
+  //     let output;
+  //     if (completion === THROW) {
+  //       if (!IS_CALLABLE(iteratorReturn)) {
+  //         throw MakeTypeError(kReturnMethodNotCallable);
+  //       }
+  //       try { output = %_Call(iteratorReturn, iterator) } catch (_) { }
+  //     } else {
+  //       output = %_Call(iteratorReturn, iterator);
+  //     }
+  //     if (!IS_RECEIVER(output)) %ThrowIterResultNotAnObject(output);
+  //   }
+  //
+
+  const int nopos = RelocInfo::kNoPosition;
+  auto factory = parser_->factory();
+  auto avfactory = parser_->ast_value_factory();
+  auto scope = parser_->scope_;
+  auto zone = parser_->zone();
+
+
+  // let output;
+  Variable* var_output = scope->NewTemporary(avfactory->empty_string());
+
+
+  // let iteratorReturn = iterator.return;
+  Variable* var_return = var_output;  // Reusing the output variable.
+  Statement* get_return;
+  {
+    Expression* iterator_proxy = factory->NewVariableProxy(iterator);
+    Expression* literal =
+        factory->NewStringLiteral(avfactory->return_string(), nopos);
+    Expression* property =
+        factory->NewProperty(iterator_proxy, literal, nopos);
+    Expression* return_proxy = factory->NewVariableProxy(var_return);
+    Expression* assignment = factory->NewAssignment(
+        Token::ASSIGN, return_proxy, property, nopos);
+    get_return = factory->NewExpressionStatement(assignment, nopos);
+  }
+
+
+  // if (!IS_CALLABLE(iteratorReturn)) {
+  //   throw MakeTypeError(kReturnMethodNotCallable);
+  // }
+  Statement* check_return_callable;
+  {
+    Expression* type_of = factory->NewUnaryOperation(
+        Token::TYPEOF, factory->NewVariableProxy(var_return), nopos);
+    Expression* function_literal = factory->NewStringLiteral(
+        avfactory->function_string(), nopos);
+    Expression* condition = factory->NewCompareOperation(
+        Token::EQ_STRICT, type_of, function_literal, nopos);

[Dan Ehrenberg, 2016/02/16 21:18:30]

FYI mvstanton@: Here's how you can do IsCallable in a desugaring in an
optimizable way.

Seems like multiple desugarings may need to do this IsCallable/throw type error
thing (also @@hasInstance). Should we factor this out?

[mvstanton, 2016/02/16 22:55:46]

Thanks Dan! Over in my CL I'll put this into a method in parser.cc for handy
reuse...

[rossberg, 2016/02/17 12:54:45]

Makes sense. Okay, whoever lands second should remember to change these lines to
use your method.

+
+    Expression* call = NewThrowTypeError(
+        MessageTemplate::kReturnMethodNotCallable,
+        avfactory->empty_string(), nopos);
+    Statement* throw_call = factory->NewExpressionStatement(call, nopos);
+
+    check_return_callable = factory->NewIfStatement(
+        condition, factory->NewEmptyStatement(nopos), throw_call, nopos);
+  }
+
+
+  // output = %_Call(iteratorReturn, iterator);
+  Statement* call_return;
+  {
+    auto args = new (zone) ZoneList<Expression*>(2, zone);
+    args->Add(factory->NewVariableProxy(var_return), zone);
+    args->Add(factory->NewVariableProxy(iterator), zone);
+    Expression* call =
+        factory->NewCallRuntime(Runtime::kInlineCall, args, nopos);
+
+    Expression* output_proxy = factory->NewVariableProxy(var_output);
+    Expression* assignment = factory->NewAssignment(
+        Token::ASSIGN, output_proxy, call, nopos);
+    call_return = factory->NewExpressionStatement(assignment, nopos);
+  }
+
+
+  // try { output = %_Call(iteratorReturn, iterator) } catch (_) { }
+  Statement* try_call_return;
+  {
+    auto args = new (zone) ZoneList<Expression*>(2, zone);
+    args->Add(factory->NewVariableProxy(var_return), zone);
+    args->Add(factory->NewVariableProxy(iterator), zone);
+
+    Expression* call =
+        factory->NewCallRuntime(Runtime::kInlineCall, args, nopos);
+    Expression* assignment = factory->NewAssignment(
+        Token::ASSIGN, factory->NewVariableProxy(var_output), call, nopos);
+
+    Block* try_block = factory->NewBlock(nullptr, 1, false, nopos);
+    try_block->statements()->Add(
+        factory->NewExpressionStatement(assignment, nopos), zone);
+
+    Block* catch_block = factory->NewBlock(nullptr, 0, false, nopos);
+
+    Scope* catch_scope = NewScope(scope, CATCH_SCOPE);
+    Variable* catch_variable = catch_scope->DeclareLocal(
+        avfactory->dot_catch_string(), VAR, kCreatedInitialized,
+        Variable::NORMAL);
+
+    try_call_return = factory->NewTryCatchStatement(
+        try_block, catch_scope, catch_variable, catch_block, nopos);
+  }
+
+
+  // if (completion === THROW) {
+  //   #check_return_callable;
+  //   #try_call_return;
+  // } else {
+  //   #call_return;
+  // }
+  Statement* call_return_carefully;
+  {
+    Expression* condition = factory->NewCompareOperation(
+        Token::EQ_STRICT, factory->NewVariableProxy(completion),
+        factory->NewSmiLiteral(THROW, nopos), nopos);
+
+    Block* then_block = factory->NewBlock(nullptr, 2, false, nopos);
+    then_block->statements()->Add(check_return_callable, zone);
+    then_block->statements()->Add(try_call_return, zone);
+
+    call_return_carefully =
+        factory->NewIfStatement(condition, then_block, call_return, nopos);
+  }
+
+
+  // if (!IS_RECEIVER(output)) %ThrowIteratorResultNotAnObject(output);
+  Statement* validate_output;
+  {
+    Expression* is_receiver_call;
+    {
+      auto args = new (zone) ZoneList<Expression*>(1, zone);
+      args->Add(factory->NewVariableProxy(var_output), zone);
+      is_receiver_call =
+          factory->NewCallRuntime(Runtime::kInlineIsJSReceiver, args, nopos);
+    }
+
+    Statement* throw_call;
+    {
+      auto args = new (zone) ZoneList<Expression*>(1, zone);
+      args->Add(factory->NewVariableProxy(var_output), zone);
+      Expression* call = factory->NewCallRuntime(
+          Runtime::kThrowIteratorResultNotAnObject, args, nopos);
+      throw_call = factory->NewExpressionStatement(call, nopos);
+    }
+
+    validate_output = factory->NewIfStatement(
+        is_receiver_call, factory->NewEmptyStatement(nopos), throw_call, nopos);
+  }
+
+
+  // if (!IS_NULL_OR_UNDEFINED(iteratorReturn)) { ... }

[Dan Ehrenberg, 2016/02/16 21:18:30]

Nit: All your other comments fully say what statements are included, except this
one. It's so pretty in general! Keep it up!

[rossberg, 2016/02/17 12:54:45]

Actually, not true. The convention (see also RewriteYieldStar) is to use dots
for the parts that are not actually constructed in this block. I think that
makes sense.

+  Statement* maybe_call_return;
+  {
+    Expression* condition = factory->NewCompareOperation(
+        Token::EQ, factory->NewVariableProxy(var_return),
+        factory->NewNullLiteral(nopos), nopos);
+
+    Block* block = factory->NewBlock(nullptr, 2, false, nopos);
+    block->statements()->Add(call_return_carefully, zone);
+    block->statements()->Add(validate_output, zone);  // GAGA

[Dan Ehrenberg, 2016/02/16 21:18:30]

GAGA? I don't really see anything funny about this code.

[rossberg, 2016/02/17 12:54:45]

I missed that one, removed. You'll have to ask Georg what it meant.

+
+    maybe_call_return = factory->NewIfStatement(
+        condition, factory->NewEmptyStatement(nopos), block, nopos);
+  }
+
+
+  statements->Add(get_return, zone);
+  statements->Add(maybe_call_return, zone);
+}
+
+
+Statement* ParserTraits::FinalizeForOfStatement(ForOfStatement* loop, int pos) {
+  //
+  // This function replaces the loop with the following wrapping:
+  //
+  //   let completion = RETURN_OR_BREAK;
+  //   try {
+  //     #loop;
+  //   } catch(e) {
+  //     completion = THROW;
+  //     throw e;
+  //   } finally {
+  //     if (!(completion === NORMAL || IS_UNDEFINED(#iterator))) {
+  //       #BuildIteratorClose(#iterator, completion)  // See above.
+  //     }
+  //   }
+  //
+  // where the loop's body is wrapped as follows:
+  //
+  //   {
+  //     {{completion = RETURN_OR_BREAK;}}
+  //     #loop-body
+  //     {{completion = NORMAL;}}
+  //   }
+
+  const int nopos = RelocInfo::kNoPosition;

[Dan Ehrenberg, 2016/02/16 21:18:30]

This desugaring uses nopos for everything. Is that really OK? Do we maybe want
to ascribe things to the beginning or end of the block, position-wise?

Style nit: Usually, I like putting things in variables, but here, I think it's
nice to see explicit references to RelocInfo::kNoPosition; it makes it more
clear what's going on, and you can grep for it to find where we're skimping on
positions. It's just a single long name, not a method call or anything, so I
think it makes sense to repeat around.

[rossberg, 2016/02/17 12:54:45]

This is done in various other functions already. I think it makes sense to save
on the verbosity.

+  auto factory = parser_->factory();
+  auto avfactory = parser_->ast_value_factory();
+  auto scope = parser_->scope_;
+  auto zone = parser_->zone();
+
+  // let completion = RETURN_OR_BREAK;
+  Variable* var_completion = scope->NewTemporary(avfactory->empty_string());
+  Statement* initialize_completion;
+  {
+    Expression* proxy = factory->NewVariableProxy(var_completion);
+    Expression* assignment = factory->NewAssignment(
+        Token::ASSIGN, proxy,
+        factory->NewSmiLiteral(RETURN_OR_BREAK, nopos), nopos);
+    initialize_completion =
+      factory->NewExpressionStatement(assignment, nopos);
+  }
+
+  // completion = THROW;
+  Statement* set_completion_throw;
+  {
+    Expression* proxy = factory->NewVariableProxy(var_completion);
+    Expression* assignment = factory->NewAssignment(
+        Token::ASSIGN, proxy, factory->NewSmiLiteral(THROW, nopos), nopos);
+    set_completion_throw = factory->NewExpressionStatement(assignment, nopos);
+  }
+
+  // if (!(completion === NORMAL || IS_UNDEFINED(#iterator))) {
+  //   #BuildIteratorClose(#iterator, completion)
+  // }
+  Block* maybe_close;
+  {
+    Expression* condition1 = factory->NewCompareOperation(
+        Token::EQ_STRICT, factory->NewVariableProxy(var_completion),
+        factory->NewSmiLiteral(NORMAL, nopos), nopos);
+    Expression* condition2 = factory->NewCompareOperation(
+        Token::EQ_STRICT, factory->NewVariableProxy(loop->iterator()),
+        factory->NewUndefinedLiteral(nopos), nopos);
+    Expression* condition = factory->NewBinaryOperation(
+        Token::OR, condition1, condition2, nopos);
+
+    Block* block = factory->NewBlock(nullptr, 2, false, nopos);
+    BuildIteratorClose(
+        block->statements(), loop->iterator(), var_completion);
+    DCHECK(block->statements()->length() == 2);
+
+    maybe_close = factory->NewBlock(nullptr, 1, false, nopos);
+    maybe_close->statements()->Add(factory->NewIfStatement(
+        condition, factory->NewEmptyStatement(nopos), block, nopos), zone);
+  }
+
+  // try { #try_block }
+  // catch(e) {
+  //   #set_completion_throw;
+  //   throw e;
+  // }
+  Statement* try_catch;
+  {
+    Scope* catch_scope = NewScope(scope, CATCH_SCOPE);
+    Variable* catch_variable = catch_scope->DeclareLocal(
+        avfactory->dot_catch_string(), VAR, kCreatedInitialized,
+        Variable::NORMAL);
+
+    Statement* rethrow;
+    {
+      Expression* proxy = factory->NewVariableProxy(catch_variable);
+      rethrow = factory->NewExpressionStatement(
+          factory->NewThrow(proxy, nopos), nopos);
+    }
+
+    Block* try_block = factory->NewBlock(nullptr, 1, false, nopos);
+    try_block->statements()->Add(loop, zone);
+
+    Block* catch_block = factory->NewBlock(nullptr, 2, false, nopos);
+    catch_block->statements()->Add(set_completion_throw, zone);
+    catch_block->statements()->Add(rethrow, zone);
+
+    try_catch = factory->NewTryCatchStatement(
+        try_block, catch_scope, catch_variable, catch_block, nopos);
+  }
+
+  // try { #try_catch } finally { #maybe_close }
+  Statement* try_finally;
+  {
+    Block* try_block = factory->NewBlock(nullptr, 1, false, nopos);
+    try_block->statements()->Add(try_catch, zone);
+
+    try_finally =
+        factory->NewTryFinallyStatement(try_block, maybe_close, nopos);
+  }
+
+  // #initialize_completion;
+  // #try_finally;
+  Statement* final_loop;
+  {
+    Block* block = factory->NewBlock(nullptr, 2, false, nopos);
+    block->statements()->Add(initialize_completion, zone);
+    block->statements()->Add(try_finally, zone);
+    final_loop = block;
+  }
+
+  // {{completion = RETURN_OR_BREAK;}}
+  Statement* set_completion_break;
+  {
+    Expression* proxy = factory->NewVariableProxy(var_completion);
+    Expression* assignment = factory->NewAssignment(
+        Token::ASSIGN, proxy,
+        factory->NewSmiLiteral(RETURN_OR_BREAK, nopos), nopos);
+
+    Block* block = factory->NewBlock(nullptr, 1, true, nopos);
+    block->statements()->Add(
+        factory->NewExpressionStatement(assignment, nopos), zone);
+    set_completion_break = block;
+  }
+
+  // {{completion = NORMAL;}}
+  Statement* set_completion_normal;
+  {
+    Expression* proxy = factory->NewVariableProxy(var_completion);
+    Expression* assignment = factory->NewAssignment(
+        Token::ASSIGN, proxy, factory->NewSmiLiteral(NORMAL, nopos), nopos);
+
+    Block* block = factory->NewBlock(nullptr, 1, true, nopos);
+    block->statements()->Add(
+        factory->NewExpressionStatement(assignment, nopos), zone);
+    set_completion_normal = block;
+  }
+
+  // { #set_completion_break; #loop-body; #set_completion_normal }
+  Block* new_body = factory->NewBlock(nullptr, 2, false, nopos);
+  new_body->statements()->Add(set_completion_break, zone);
+  new_body->statements()->Add(loop->body(), zone);
+  new_body->statements()->Add(set_completion_normal, zone);
+
+  loop->set_body(new_body);
+  return final_loop;
+}
+
+
 }  // namespace internal
 }  // namespace v8