Close for-of iterator when the loop body throws (#3734)

core/engine/src/bytecompiler/statement/loop.rs

@@ -312,7 +312,8 @@ impl ByteCompiler<'_> {
             self.emit_with_varying_operand(Opcode::PushDeclarativeEnvironment, env_index);
         };
 
-        let mut handler_index = None;
+        let handler_index = self.push_handler();
+
         match for_of_loop.initializer() {
             IterableLoopInitializer::Identifier(ref ident) => {
                 let ident = ident.to_js_string(self.interner());
@@ -331,7 +332,6 @@ impl ByteCompiler<'_> {
                 }
             }
             IterableLoopInitializer::Access(access) => {
-                handler_index = Some(self.push_handler());
                 self.access_set(
                     Access::Property { access },
                     false,
@@ -384,15 +384,13 @@ impl ByteCompiler<'_> {
                 }
             },
             IterableLoopInitializer::Pattern(pattern) => {
-                handler_index = Some(self.push_handler());
                 self.compile_declaration_pattern(pattern, BindingOpcode::SetName);
             }
         }
 
-        // If the left-hand side is not a lexical binding and the assignment produces
-        // an error, the iterator should be closed and the error forwarded to the
-        // runtime.
-        if let Some(handler_index) = handler_index {
+        self.compile_stmt(for_of_loop.body(), use_expr, true);
+
+        {
             let exit = self.jump();
             self.patch_handler(handler_index);
 
@@ -411,8 +409,6 @@ impl ByteCompiler<'_> {
             self.patch_jump(exit);
         }
 
-        self.compile_stmt(for_of_loop.body(), use_expr, true);
-
         if let Some(old_lex_env) = old_lex_env {
             self.pop_compile_environment();
             self.lexical_environment = old_lex_env;

core/engine/src/bytecompiler/utils.rs

@@ -17,14 +17,9 @@ impl ByteCompiler<'_> {
     /// [iter]: https://tc39.es/ecma262/#sec-iteratorclose
     /// [async]: https://tc39.es/ecma262/#sec-asynciteratorclose
     pub(super) fn iterator_close(&mut self, async_: bool) {
-        self.emit_opcode(Opcode::IteratorDone);
-
-        let skip_return = self.jump_if_true();
-
-        // iterator didn't finish iterating.
         self.emit_opcode(Opcode::IteratorReturn);
 
-        // `iterator` didn't have a `return` method, so we can early exit.
+        // `iterator` didn't have a `return` method, is already done or is not on the iterator stack.
         let early_exit = self.jump_if_false();
         if async_ {
             self.emit_opcode(Opcode::Await);
@@ -38,9 +33,6 @@ impl ByteCompiler<'_> {
         )));
         self.emit_with_varying_operand(Opcode::ThrowNewTypeError, error_msg);
 
-        self.patch_jump(skip_return);
-        self.emit_opcode(Opcode::IteratorPop);
-
         self.patch_jump(skip_throw);
         self.patch_jump(early_exit);
     }

core/engine/src/vm/code_block.rs

@@ -622,7 +622,6 @@ impl CodeBlock {
             | Instruction::IteratorResult
             | Instruction::IteratorDone
             | Instruction::IteratorToArray
-            | Instruction::IteratorPop
             | Instruction::IteratorReturn
             | Instruction::IteratorStackEmpty
             | Instruction::RequireObjectCoercible
@@ -717,7 +716,8 @@ impl CodeBlock {
             | Instruction::Reserved55
             | Instruction::Reserved56
             | Instruction::Reserved57
-            | Instruction::Reserved58 => unreachable!("Reserved opcodes are unrechable"),
+            | Instruction::Reserved58
+            | Instruction::Reserved59 => unreachable!("Reserved opcodes are unrechable"),
         }
     }
 }

core/engine/src/vm/flowgraph/mod.rs

@@ -409,7 +409,6 @@ impl CodeBlock {
                 | Instruction::IteratorResult
                 | Instruction::IteratorDone
                 | Instruction::IteratorToArray
-                | Instruction::IteratorPop
                 | Instruction::IteratorReturn
                 | Instruction::IteratorStackEmpty
                 | Instruction::RequireObjectCoercible
@@ -516,7 +515,8 @@ impl CodeBlock {
                 | Instruction::Reserved55
                 | Instruction::Reserved56
                 | Instruction::Reserved57
-                | Instruction::Reserved58 => unreachable!("Reserved opcodes are unrechable"),
+                | Instruction::Reserved58
+                | Instruction::Reserved59 => unreachable!("Reserved opcodes are unrechable"),
             }
         }
 

core/engine/src/vm/opcode/iteration/iterator.rs

@@ -235,12 +235,15 @@ impl Operation for IteratorReturn {
     const COST: u8 = 8;
 
     fn execute(context: &mut Context) -> JsResult<CompletionType> {
-        let record = context
-            .vm
-            .frame_mut()
-            .iterators
-            .pop()
-            .expect("iterator on the call frame must exist");
+        let Some(record) = context.vm.frame_mut().iterators.pop() else {
+            context.vm.push(false);
+            return Ok(CompletionType::Normal);
+        };
+
+        if record.done() {
+            context.vm.push(false);
+            return Ok(CompletionType::Normal);
+        }
 
         let Some(ret) = record
             .iterator()
@@ -317,24 +320,6 @@ impl Operation for IteratorToArray {
     }
 }
 
-/// `IteratorPop` implements the Opcode Operation for `Opcode::IteratorPop`
-///
-/// Operation:
-///  - Pop an iterator from the call frame close iterator stack.
-#[derive(Debug, Clone, Copy)]
-pub(crate) struct IteratorPop;
-
-impl Operation for IteratorPop {
-    const NAME: &'static str = "IteratorPop";
-    const INSTRUCTION: &'static str = "INST - IteratorPop";
-    const COST: u8 = 1;
-
-    fn execute(context: &mut Context) -> JsResult<CompletionType> {
-        context.vm.frame_mut().iterators.pop();
-        Ok(CompletionType::Normal)
-    }
-}
-
 /// `IteratorStackEmpty` implements the Opcode Operation for `Opcode::IteratorStackEmpty`
 ///
 /// Operation:

core/engine/src/vm/opcode/mod.rs

@@ -1879,12 +1879,6 @@ generate_opcodes! {
     /// Iterator Stack: `iterator` **=>** `iterator`
     IteratorToArray,
 
-    /// Pop an iterator from the call frame close iterator stack.
-    ///
-    /// Iterator Stack:
-    /// - `iterator` **=>**
-    IteratorPop,
-
     /// Pushes `true` to the stack if the iterator stack is empty.
     ///
     /// Stack:
@@ -2222,6 +2216,8 @@ generate_opcodes! {
     Reserved57 => Reserved,
     /// Reserved [`Opcode`].
     Reserved58 => Reserved,
+    /// Reserved [`Opcode`].
+    Reserved59 => Reserved,
 }
 
 /// Specific opcodes for bindings.