From 79c290ce709b884a527d4990dbdb4fc8b4656f2d Mon Sep 17 00:00:00 2001 From: Haled Odat <8566042+HalidOdat@users.noreply.github.com> Date: Sun, 9 Jul 2023 14:06:58 +0200 Subject: [PATCH] Close iterator when array destructoring --- .../declaration/declaration_pattern.rs | 48 +++++++++++++--- boa_engine/src/bytecompiler/statement/loop.rs | 8 +-- boa_engine/src/vm/code_block.rs | 10 ++-- boa_engine/src/vm/flowgraph/mod.rs | 6 +- .../src/vm/opcode/control_flow/throw.rs | 2 +- .../src/vm/opcode/iteration/iterator.rs | 57 +++++++++++++++++++ boa_engine/src/vm/opcode/mod.rs | 20 +++++-- 7 files changed, 126 insertions(+), 25 deletions(-) diff --git a/boa_engine/src/bytecompiler/declaration/declaration_pattern.rs b/boa_engine/src/bytecompiler/declaration/declaration_pattern.rs index 9801c63bd3d..98aa84b7fbf 100644 --- a/boa_engine/src/bytecompiler/declaration/declaration_pattern.rs +++ b/boa_engine/src/bytecompiler/declaration/declaration_pattern.rs @@ -180,11 +180,45 @@ impl ByteCompiler<'_, '_> { self.emit_opcode(Opcode::ValueNotNullOrUndefined); self.emit_opcode(Opcode::GetIterator); + // TODO: maybe, refactor this to be more condensed. + let handler_index = self.push_handler(); for element in pattern.bindings() { self.compile_array_pattern_element(element, def); } + let handler_end_address = self.next_opcode_location(); + self.emit_opcode(Opcode::PushFalse); + + let exit = self.jump(); + let handler_address = self.next_opcode_location(); + self.emit_opcode(Opcode::Exception); + self.emit_opcode(Opcode::PushTrue); + self.patch_jump(exit); + + self.handlers[handler_index as usize].handler = handler_address; + self.handlers[handler_index as usize].range.end = handler_end_address; + + let iterator_close_handler = self.push_handler(); self.iterator_close(false); + + let exit = self.jump(); + let iterator_close_handler_address = self.next_opcode_location(); + { + let jump = self.jump_if_false(); + self.emit_opcode(Opcode::Throw); + self.patch_jump(jump); + } + self.emit_opcode(Opcode::ReThrow); + self.patch_jump(exit); + + let jump = self.jump_if_false(); + self.emit_opcode(Opcode::Throw); + self.patch_jump(jump); + + self.handlers[iterator_close_handler as usize].handler = + iterator_close_handler_address; + self.handlers[iterator_close_handler as usize].range.end = + iterator_close_handler_address; } } } @@ -198,15 +232,15 @@ impl ByteCompiler<'_, '_> { match element { // ArrayBindingPattern : [ Elision ] Elision => { - self.emit_opcode(Opcode::IteratorNext); + self.emit_opcode(Opcode::IteratorNextWithoutPop); } // SingleNameBinding : BindingIdentifier Initializer[opt] SingleName { ident, default_init, } => { - self.emit_opcode(Opcode::IteratorNext); - self.emit_opcode(Opcode::IteratorValue); + self.emit_opcode(Opcode::IteratorNextWithoutPop); + self.emit_opcode(Opcode::IteratorValueWithoutPop); if let Some(init) = default_init { let skip = self.emit_opcode_with_operand(Opcode::JumpIfNotUndefined); self.compile_expr(init, true); @@ -216,8 +250,8 @@ impl ByteCompiler<'_, '_> { } PropertyAccess { access } => { self.access_set(Access::Property { access }, false, |compiler, _level| { - compiler.emit_opcode(Opcode::IteratorNext); - compiler.emit_opcode(Opcode::IteratorValue); + compiler.emit_opcode(Opcode::IteratorNextWithoutPop); + compiler.emit_opcode(Opcode::IteratorValueWithoutPop); }); } // BindingElement : BindingPattern Initializer[opt] @@ -225,8 +259,8 @@ impl ByteCompiler<'_, '_> { pattern, default_init, } => { - self.emit_opcode(Opcode::IteratorNext); - self.emit_opcode(Opcode::IteratorValue); + self.emit_opcode(Opcode::IteratorNextWithoutPop); + self.emit_opcode(Opcode::IteratorValueWithoutPop); if let Some(init) = default_init { let skip = self.emit_opcode_with_operand(Opcode::JumpIfNotUndefined); diff --git a/boa_engine/src/bytecompiler/statement/loop.rs b/boa_engine/src/bytecompiler/statement/loop.rs index c8f019fee65..381cb023906 100644 --- a/boa_engine/src/bytecompiler/statement/loop.rs +++ b/boa_engine/src/bytecompiler/statement/loop.rs @@ -376,7 +376,7 @@ impl ByteCompiler<'_, '_> { self.emit_opcode(Opcode::Exception); - // NOTE: Capture throws of the iterator close and handle it. + // NOTE: Capture throw of the iterator close and ignore it. { let handler_index = self.push_handler(); self.iterator_close(for_of_loop.r#await()); @@ -385,12 +385,12 @@ impl ByteCompiler<'_, '_> { self.handlers[handler_index as usize].range.end = end_address; } - self.emit_opcode(Opcode::Throw); - self.patch_jump(exit); - let handler_index = handler_index as usize; self.handlers[handler_index].handler = handler_address; self.handlers[handler_index].range.end = end_address; + + self.emit_opcode(Opcode::Throw); + self.patch_jump(exit); } self.compile_stmt(for_of_loop.body(), use_expr, true); diff --git a/boa_engine/src/vm/code_block.rs b/boa_engine/src/vm/code_block.rs index bdf921cf824..20cda388b1b 100644 --- a/boa_engine/src/vm/code_block.rs +++ b/boa_engine/src/vm/code_block.rs @@ -240,10 +240,10 @@ impl CodeBlock { } pub(crate) fn find_handler(&self, pc: u32) -> Option<&Handler> { - dbg!(self.handlers + self.handlers .iter() .rev() - .find(|&handler| handler.range.contains(&pc))) + .find(|&handler| handler.range.contains(&pc)) } } @@ -556,8 +556,10 @@ impl CodeBlock { | Opcode::GetAsyncIterator | Opcode::GeneratorResumeReturn | Opcode::IteratorNext + | Opcode::IteratorNextWithoutPop | Opcode::IteratorFinishAsyncNext | Opcode::IteratorValue + | Opcode::IteratorValueWithoutPop | Opcode::IteratorResult | Opcode::IteratorDone | Opcode::IteratorToArray @@ -654,9 +656,7 @@ impl CodeBlock { | Opcode::Reserved57 | Opcode::Reserved58 | Opcode::Reserved59 - | Opcode::Reserved60 - | Opcode::Reserved61 - | Opcode::Reserved62 => unreachable!("Reserved opcodes are unrechable"), + | Opcode::Reserved60 => unreachable!("Reserved opcodes are unrechable"), } } } diff --git a/boa_engine/src/vm/flowgraph/mod.rs b/boa_engine/src/vm/flowgraph/mod.rs index 02adec5b99e..985f2dbc180 100644 --- a/boa_engine/src/vm/flowgraph/mod.rs +++ b/boa_engine/src/vm/flowgraph/mod.rs @@ -552,8 +552,10 @@ impl CodeBlock { | Opcode::GetIterator | Opcode::GetAsyncIterator | Opcode::IteratorNext + | Opcode::IteratorNextWithoutPop | Opcode::IteratorFinishAsyncNext | Opcode::IteratorValue + | Opcode::IteratorValueWithoutPop | Opcode::IteratorResult | Opcode::IteratorDone | Opcode::IteratorToArray @@ -670,9 +672,7 @@ impl CodeBlock { | Opcode::Reserved57 | Opcode::Reserved58 | Opcode::Reserved59 - | Opcode::Reserved60 - | Opcode::Reserved61 - | Opcode::Reserved62 => unreachable!("Reserved opcodes are unrechable"), + | Opcode::Reserved60 => unreachable!("Reserved opcodes are unrechable"), } } diff --git a/boa_engine/src/vm/opcode/control_flow/throw.rs b/boa_engine/src/vm/opcode/control_flow/throw.rs index f389baeeb19..eabf98278d9 100644 --- a/boa_engine/src/vm/opcode/control_flow/throw.rs +++ b/boa_engine/src/vm/opcode/control_flow/throw.rs @@ -51,7 +51,7 @@ impl Operation for ReThrow { fn execute(context: &mut Context<'_>) -> JsResult { assert!(context.vm.err.is_some(), "Need exception to rethrow"); - let pc = context.vm.frame().pc; + let pc = context.vm.frame().pc.saturating_sub(1); if let Some(handler) = context.vm.frame().code_block().find_handler(pc).cloned() { let env_fp = context.vm.frame().env_fp; diff --git a/boa_engine/src/vm/opcode/iteration/iterator.rs b/boa_engine/src/vm/opcode/iteration/iterator.rs index 1a7b06deb70..1e36d4d5a46 100644 --- a/boa_engine/src/vm/opcode/iteration/iterator.rs +++ b/boa_engine/src/vm/opcode/iteration/iterator.rs @@ -34,6 +34,35 @@ impl Operation for IteratorNext { } } +/// `IteratorNextWithoutPop` implements the Opcode Operation for `Opcode::IteratorNextWithoutPop` +/// +/// Operation: +/// - Calls the `next` method of `iterator`, updating its record with the next value. +#[derive(Debug, Clone, Copy)] +pub(crate) struct IteratorNextWithoutPop; + +impl Operation for IteratorNextWithoutPop { + const NAME: &'static str = "IteratorNextWithoutPop"; + const INSTRUCTION: &'static str = "INST - IteratorNextWithoutPop"; + + fn execute(context: &mut Context<'_>) -> JsResult { + let mut iterator = context + .vm + .frame_mut() + .iterators + .pop() + .expect("iterator stack should have at least an iterator"); + + let result = iterator.step(context); + + context.vm.frame_mut().iterators.push(iterator); + + result?; + + Ok(CompletionType::Normal) + } +} + /// `IteratorFinishAsyncNext` implements the Opcode Operation for `Opcode::IteratorFinishAsyncNext`. /// /// Operation: @@ -129,6 +158,34 @@ impl Operation for IteratorValue { } } +/// `IteratorValueWithoutPop` implements the Opcode Operation for `Opcode::IteratorValueWithoutPop` +/// +/// Operation: +/// - Gets the `value` property of the current iterator record. +#[derive(Debug, Clone, Copy)] +pub(crate) struct IteratorValueWithoutPop; + +impl Operation for IteratorValueWithoutPop { + const NAME: &'static str = "IteratorValueWithoutPop"; + const INSTRUCTION: &'static str = "INST - IteratorValueWithoutPop"; + + fn execute(context: &mut Context<'_>) -> JsResult { + let mut iterator = context + .vm + .frame_mut() + .iterators + .pop() + .expect("iterator on the call frame must exist"); + + let value = iterator.value(context); + context.vm.frame_mut().iterators.push(iterator); + + context.vm.push(value?); + + Ok(CompletionType::Normal) + } +} + /// `IteratorDone` implements the Opcode Operation for `Opcode::IteratorDone` /// /// Operation: diff --git a/boa_engine/src/vm/opcode/mod.rs b/boa_engine/src/vm/opcode/mod.rs index 8d55fdd654b..ecb702be84b 100644 --- a/boa_engine/src/vm/opcode/mod.rs +++ b/boa_engine/src/vm/opcode/mod.rs @@ -1407,6 +1407,13 @@ generate_impl! { /// Iterator Stack: `iterator` **=>** `iterator` IteratorNext, + /// Calls the `next` method of `iterator`, updating its record with the next value. + /// + /// Operands: + /// + /// Iterator Stack: `iterator` **=>** `iterator` + IteratorNextWithoutPop, + /// Returns `true` if the current iterator is done, or `false` otherwise /// /// Stack: **=>** done @@ -1424,13 +1431,20 @@ generate_impl! { /// Iterator Stack: iterator **=>** iterator IteratorFinishAsyncNext, - /// - Gets the `value` property of the current iterator record. + /// Gets the `value` property of the current iterator record. /// /// Stack: **=>** `value` /// /// Iterator Stack: `iterator` **=>** `iterator` IteratorValue, + /// Gets the `value` property of the current iterator record. + /// + /// Stack: **=>** `value` + /// + /// Iterator Stack: `iterator` **=>** `iterator` + IteratorValueWithoutPop, + /// Gets the last iteration result of the current iterator record. /// /// Stack: **=>** `result` @@ -1756,10 +1770,6 @@ generate_impl! { Reserved59 => Reserved, /// Reserved [`Opcode`]. Reserved60 => Reserved, - /// Reserved [`Opcode`]. - Reserved61 => Reserved, - /// Reserved [`Opcode`]. - Reserved62 => Reserved, } }