Ensure IteratorClose is called for errors in non-declaring assignments

There was a bug in for-of loops without newly declared variables: If,
in performing the assignment, an exception were thrown, then
IteratorClose would not be called. The problem was that the assignment
is done as part of assign_each, which happens before the loop is put
back in the state which is recognized to be breaking/throwing/returning
early.

This patch modifies the for-of desugaring by setting the loop state
before, rather than after, evaluating the assign_each portion, which is
responsible for evaluating the assignment in for-of loops which do not
have a declaration.

This patch, together with https://codereview.chromium.org/1728973002 ,
allow all test262 iterator return-related tests to pass.

R=rossberg
BUG=v8:4776
LOG=Y

Review URL: https://codereview.chromium.org/1731773003

Cr-Commit-Position: refs/heads/master@{#34262}

src/parsing/parser.cc

@@ -6740,6 +6740,7 @@ Statement* ParserTraits::FinalizeForOfStatement(ForOfStatement* loop, int pos) {
   // This function replaces the loop with the following wrapping:
   //
   //   let completion = BODY_COMPLETED;
+  //   let each;
   //   try {
   //     #loop;
   //   } catch(e) {
@@ -6754,10 +6755,16 @@ Statement* ParserTraits::FinalizeForOfStatement(ForOfStatement* loop, int pos) {
   // where the loop's body is wrapped as follows:
   //
   //   {
-  //     {{completion = BODY_ABORTED;}}
   //     #loop-body
   //     {{completion = BODY_COMPLETED;}}
   //   }
+  //
+  // and assign_each is wrapped as follows
+  //
+  //   do {
+  //     {{completion = BODY_ABORTED;}}
+  //     #assign-each
+  //   } into each
 
   const int nopos = RelocInfo::kNoPosition;
   auto factory = parser_->factory();
@@ -6777,6 +6784,18 @@ Statement* ParserTraits::FinalizeForOfStatement(ForOfStatement* loop, int pos) {
         factory->NewExpressionStatement(assignment, nopos);
   }
 
+  // let each;
+  Variable* var_each = scope->NewTemporary(avfactory->empty_string());
+  Statement* initialize_each;
+  {
+    Expression* proxy = factory->NewVariableProxy(var_each);
+    Expression* assignment = factory->NewAssignment(
+        Token::ASSIGN, proxy,
+        factory->NewUndefinedLiteral(nopos), nopos);
+    initialize_each =
+        factory->NewExpressionStatement(assignment, nopos);
+  }
+
   // if (completion === BODY_ABORTED) completion = BODY_THREW;
   Statement* set_completion_throw;
   {
@@ -6858,29 +6877,17 @@ Statement* ParserTraits::FinalizeForOfStatement(ForOfStatement* loop, int pos) {
   }
 
   // #initialize_completion;
+  // #initialize_each;
   // #try_finally;
   Statement* final_loop;
   {
     Block* block = factory->NewBlock(nullptr, 2, false, nopos);
     block->statements()->Add(initialize_completion, zone);
+    block->statements()->Add(initialize_each, zone);
     block->statements()->Add(try_finally, zone);
     final_loop = block;
   }
 
-  // {{completion = BODY_ABORTED;}}
-  Statement* set_completion_break;
-  {
-    Expression* proxy = factory->NewVariableProxy(var_completion);
-    Expression* assignment = factory->NewAssignment(
-        Token::ASSIGN, proxy,
-        factory->NewSmiLiteral(BODY_ABORTED, nopos), nopos);
-
-    Block* block = factory->NewBlock(nullptr, 1, true, nopos);
-    block->statements()->Add(
-        factory->NewExpressionStatement(assignment, nopos), zone);
-    set_completion_break = block;
-  }
-
   // {{completion = BODY_COMPLETED;}}
   Statement* set_completion_normal;
   {
@@ -6895,13 +6902,37 @@ Statement* ParserTraits::FinalizeForOfStatement(ForOfStatement* loop, int pos) {
     set_completion_normal = block;
   }
 
-  // { #set_completion_break; #loop-body; #set_completion_normal }
+  // { #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);
+
+  // {{completion = BODY_ABORTED;}}
+  Statement* set_completion_break;
+  {
+    Expression* proxy = factory->NewVariableProxy(var_completion);
+    Expression* assignment = factory->NewAssignment(
+        Token::ASSIGN, proxy, factory->NewSmiLiteral(BODY_ABORTED, nopos),
+        nopos);
+
+    Block* block = factory->NewBlock(nullptr, 1, true, nopos);
+    block->statements()->Add(factory->NewExpressionStatement(assignment, nopos),
+                             zone);
+    set_completion_break = block;
+  }
+
+  // { #set_completion_break; #assign-each }
+  Block* new_assign_each = factory->NewBlock(nullptr, 2, false, nopos);
+  new_assign_each->statements()->Add(set_completion_break, zone);
+  new_assign_each->statements()->Add(
+      factory->NewExpressionStatement(loop->assign_each(), nopos), zone);
+
+  Expression* do_each =
+      factory->NewDoExpression(new_assign_each, var_each, nopos);
+  loop->set_assign_each(do_each);
+
   return final_loop;
 }
 

test/mjsunit/harmony/iterator-close.js

@@ -155,6 +155,13 @@ function* g() { yield 42; return 88 };
   log = [];
   assertEquals(42, eval('for (x of g()) { x; }'));
   assertEquals([], log);
+
+  // Even if doing the assignment throws, still call return
+  x = { set attr(_) { throw 1234; } };
+  assertThrowsEquals(() => {
+    for (x.attr of g()) { throw 456; }
+  }, 1234);
+  assertEquals([[]], log);
 }
 
 

test/test262/test262.status

@@ -64,10 +64,6 @@
   'language/computed-property-names/class/static/method-symbol': [FAIL, FAIL_SLOPPY],
   'language/computed-property-names/class/static/method-string': [FAIL, FAIL_SLOPPY],
 
-  # https://bugs.chromium.org/p/v8/issues/detail?id=4776
-  'language/statements/for-of/body-dstr-assign-error': [FAIL],
-  'language/statements/for-of/body-put-error': [FAIL],
-
   # We do not expose Array.prototype.values
   # https://code.google.com/p/v8/issues/detail?id=4247
   'built-ins/Array/prototype/Symbol.iterator': [FAIL],