Skip to content

Commit 79c290c

Browse files
HalidOdatHalidOdat
committed
Close iterator when array destructoring
1 parent 8391c80 commit 79c290c

File tree

7 files changed

+126
-25
lines changed

7 files changed

+126
-25
lines changed

‎boa_engine/src/bytecompiler/declaration/declaration_pattern.rs‎

Lines changed: 41 additions & 7 deletions
Original file line numberDiff line numberDiff line change
@@ -180,11 +180,45 @@ impl ByteCompiler<'_, '_> {
180180
self.emit_opcode(Opcode::ValueNotNullOrUndefined);
181181
self.emit_opcode(Opcode::GetIterator);
182182

183+
// TODO: maybe, refactor this to be more condensed.
184+
let handler_index = self.push_handler();
183185
for element in pattern.bindings() {
184186
self.compile_array_pattern_element(element, def);
185187
}
188+
let handler_end_address = self.next_opcode_location();
186189

190+
self.emit_opcode(Opcode::PushFalse);
191+
192+
let exit = self.jump();
193+
let handler_address = self.next_opcode_location();
194+
self.emit_opcode(Opcode::Exception);
195+
self.emit_opcode(Opcode::PushTrue);
196+
self.patch_jump(exit);
197+
198+
self.handlers[handler_index as usize].handler = handler_address;
199+
self.handlers[handler_index as usize].range.end = handler_end_address;
200+
201+
let iterator_close_handler = self.push_handler();
187202
self.iterator_close(false);
203+
204+
let exit = self.jump();
205+
let iterator_close_handler_address = self.next_opcode_location();
206+
{
207+
let jump = self.jump_if_false();
208+
self.emit_opcode(Opcode::Throw);
209+
self.patch_jump(jump);
210+
}
211+
self.emit_opcode(Opcode::ReThrow);
212+
self.patch_jump(exit);
213+
214+
let jump = self.jump_if_false();
215+
self.emit_opcode(Opcode::Throw);
216+
self.patch_jump(jump);
217+
218+
self.handlers[iterator_close_handler as usize].handler =
219+
iterator_close_handler_address;
220+
self.handlers[iterator_close_handler as usize].range.end =
221+
iterator_close_handler_address;
188222
}
189223
}
190224
}
@@ -198,15 +232,15 @@ impl ByteCompiler<'_, '_> {
198232
match element {
199233
// ArrayBindingPattern : [ Elision ]
200234
Elision => {
201-
self.emit_opcode(Opcode::IteratorNext);
235+
self.emit_opcode(Opcode::IteratorNextWithoutPop);
202236
}
203237
// SingleNameBinding : BindingIdentifier Initializer[opt]
204238
SingleName {
205239
ident,
206240
default_init,
207241
} => {
208-
self.emit_opcode(Opcode::IteratorNext);
209-
self.emit_opcode(Opcode::IteratorValue);
242+
self.emit_opcode(Opcode::IteratorNextWithoutPop);
243+
self.emit_opcode(Opcode::IteratorValueWithoutPop);
210244
if let Some(init) = default_init {
211245
let skip = self.emit_opcode_with_operand(Opcode::JumpIfNotUndefined);
212246
self.compile_expr(init, true);
@@ -216,17 +250,17 @@ impl ByteCompiler<'_, '_> {
216250
}
217251
PropertyAccess { access } => {
218252
self.access_set(Access::Property { access }, false, |compiler, _level| {
219-
compiler.emit_opcode(Opcode::IteratorNext);
220-
compiler.emit_opcode(Opcode::IteratorValue);
253+
compiler.emit_opcode(Opcode::IteratorNextWithoutPop);
254+
compiler.emit_opcode(Opcode::IteratorValueWithoutPop);
221255
});
222256
}
223257
// BindingElement : BindingPattern Initializer[opt]
224258
Pattern {
225259
pattern,
226260
default_init,
227261
} => {
228-
self.emit_opcode(Opcode::IteratorNext);
229-
self.emit_opcode(Opcode::IteratorValue);
262+
self.emit_opcode(Opcode::IteratorNextWithoutPop);
263+
self.emit_opcode(Opcode::IteratorValueWithoutPop);
230264

231265
if let Some(init) = default_init {
232266
let skip = self.emit_opcode_with_operand(Opcode::JumpIfNotUndefined);

‎boa_engine/src/bytecompiler/statement/loop.rs‎

Lines changed: 4 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -376,7 +376,7 @@ impl ByteCompiler<'_, '_> {
376376

377377
self.emit_opcode(Opcode::Exception);
378378

379-
// NOTE: Capture throws of the iterator close and handle it.
379+
// NOTE: Capture throw of the iterator close and ignore it.
380380
{
381381
let handler_index = self.push_handler();
382382
self.iterator_close(for_of_loop.r#await());
@@ -385,12 +385,12 @@ impl ByteCompiler<'_, '_> {
385385
self.handlers[handler_index as usize].range.end = end_address;
386386
}
387387

388-
self.emit_opcode(Opcode::Throw);
389-
self.patch_jump(exit);
390-
391388
let handler_index = handler_index as usize;
392389
self.handlers[handler_index].handler = handler_address;
393390
self.handlers[handler_index].range.end = end_address;
391+
392+
self.emit_opcode(Opcode::Throw);
393+
self.patch_jump(exit);
394394
}
395395

396396
self.compile_stmt(for_of_loop.body(), use_expr, true);

‎boa_engine/src/vm/code_block.rs‎

Lines changed: 5 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -240,10 +240,10 @@ impl CodeBlock {
240240
}
241241

242242
pub(crate) fn find_handler(&self, pc: u32) -> Option<&Handler> {
243-
dbg!(self.handlers
243+
self.handlers
244244
.iter()
245245
.rev()
246-
.find(|&handler| handler.range.contains(&pc)))
246+
.find(|&handler| handler.range.contains(&pc))
247247
}
248248
}
249249

@@ -556,8 +556,10 @@ impl CodeBlock {
556556
| Opcode::GetAsyncIterator
557557
| Opcode::GeneratorResumeReturn
558558
| Opcode::IteratorNext
559+
| Opcode::IteratorNextWithoutPop
559560
| Opcode::IteratorFinishAsyncNext
560561
| Opcode::IteratorValue
562+
| Opcode::IteratorValueWithoutPop
561563
| Opcode::IteratorResult
562564
| Opcode::IteratorDone
563565
| Opcode::IteratorToArray
@@ -654,9 +656,7 @@ impl CodeBlock {
654656
| Opcode::Reserved57
655657
| Opcode::Reserved58
656658
| Opcode::Reserved59
657-
| Opcode::Reserved60
658-
| Opcode::Reserved61
659-
| Opcode::Reserved62 => unreachable!("Reserved opcodes are unrechable"),
659+
| Opcode::Reserved60 => unreachable!("Reserved opcodes are unrechable"),
660660
}
661661
}
662662
}

‎boa_engine/src/vm/flowgraph/mod.rs‎

Lines changed: 3 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -552,8 +552,10 @@ impl CodeBlock {
552552
| Opcode::GetIterator
553553
| Opcode::GetAsyncIterator
554554
| Opcode::IteratorNext
555+
| Opcode::IteratorNextWithoutPop
555556
| Opcode::IteratorFinishAsyncNext
556557
| Opcode::IteratorValue
558+
| Opcode::IteratorValueWithoutPop
557559
| Opcode::IteratorResult
558560
| Opcode::IteratorDone
559561
| Opcode::IteratorToArray
@@ -670,9 +672,7 @@ impl CodeBlock {
670672
| Opcode::Reserved57
671673
| Opcode::Reserved58
672674
| Opcode::Reserved59
673-
| Opcode::Reserved60
674-
| Opcode::Reserved61
675-
| Opcode::Reserved62 => unreachable!("Reserved opcodes are unrechable"),
675+
| Opcode::Reserved60 => unreachable!("Reserved opcodes are unrechable"),
676676
}
677677
}
678678

‎boa_engine/src/vm/opcode/control_flow/throw.rs‎

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -51,7 +51,7 @@ impl Operation for ReThrow {
5151
fn execute(context: &mut Context<'_>) -> JsResult<CompletionType> {
5252
assert!(context.vm.err.is_some(), "Need exception to rethrow");
5353

54-
let pc = context.vm.frame().pc;
54+
let pc = context.vm.frame().pc.saturating_sub(1);
5555
if let Some(handler) = context.vm.frame().code_block().find_handler(pc).cloned() {
5656
let env_fp = context.vm.frame().env_fp;
5757

‎boa_engine/src/vm/opcode/iteration/iterator.rs‎

Lines changed: 57 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -34,6 +34,35 @@ impl Operation for IteratorNext {
3434
}
3535
}
3636

37+
/// `IteratorNextWithoutPop` implements the Opcode Operation for `Opcode::IteratorNextWithoutPop`
38+
///
39+
/// Operation:
40+
/// - Calls the `next` method of `iterator`, updating its record with the next value.
41+
#[derive(Debug, Clone, Copy)]
42+
pub(crate) struct IteratorNextWithoutPop;
43+
44+
impl Operation for IteratorNextWithoutPop {
45+
const NAME: &'static str = "IteratorNextWithoutPop";
46+
const INSTRUCTION: &'static str = "INST - IteratorNextWithoutPop";
47+
48+
fn execute(context: &mut Context<'_>) -> JsResult<CompletionType> {
49+
let mut iterator = context
50+
.vm
51+
.frame_mut()
52+
.iterators
53+
.pop()
54+
.expect("iterator stack should have at least an iterator");
55+
56+
let result = iterator.step(context);
57+
58+
context.vm.frame_mut().iterators.push(iterator);
59+
60+
result?;
61+
62+
Ok(CompletionType::Normal)
63+
}
64+
}
65+
3766
/// `IteratorFinishAsyncNext` implements the Opcode Operation for `Opcode::IteratorFinishAsyncNext`.
3867
///
3968
/// Operation:
@@ -129,6 +158,34 @@ impl Operation for IteratorValue {
129158
}
130159
}
131160

161+
/// `IteratorValueWithoutPop` implements the Opcode Operation for `Opcode::IteratorValueWithoutPop`
162+
///
163+
/// Operation:
164+
/// - Gets the `value` property of the current iterator record.
165+
#[derive(Debug, Clone, Copy)]
166+
pub(crate) struct IteratorValueWithoutPop;
167+
168+
impl Operation for IteratorValueWithoutPop {
169+
const NAME: &'static str = "IteratorValueWithoutPop";
170+
const INSTRUCTION: &'static str = "INST - IteratorValueWithoutPop";
171+
172+
fn execute(context: &mut Context<'_>) -> JsResult<CompletionType> {
173+
let mut iterator = context
174+
.vm
175+
.frame_mut()
176+
.iterators
177+
.pop()
178+
.expect("iterator on the call frame must exist");
179+
180+
let value = iterator.value(context);
181+
context.vm.frame_mut().iterators.push(iterator);
182+
183+
context.vm.push(value?);
184+
185+
Ok(CompletionType::Normal)
186+
}
187+
}
188+
132189
/// `IteratorDone` implements the Opcode Operation for `Opcode::IteratorDone`
133190
///
134191
/// Operation:

‎boa_engine/src/vm/opcode/mod.rs‎

Lines changed: 15 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -1407,6 +1407,13 @@ generate_impl! {
14071407
/// Iterator Stack: `iterator` **=>** `iterator`
14081408
IteratorNext,
14091409

1410+
/// Calls the `next` method of `iterator`, updating its record with the next value.
1411+
///
1412+
/// Operands:
1413+
///
1414+
/// Iterator Stack: `iterator` **=>** `iterator`
1415+
IteratorNextWithoutPop,
1416+
14101417
/// Returns `true` if the current iterator is done, or `false` otherwise
14111418
///
14121419
/// Stack: **=>** done
@@ -1424,13 +1431,20 @@ generate_impl! {
14241431
/// Iterator Stack: iterator **=>** iterator
14251432
IteratorFinishAsyncNext,
14261433

1427-
/// - Gets the `value` property of the current iterator record.
1434+
/// Gets the `value` property of the current iterator record.
14281435
///
14291436
/// Stack: **=>** `value`
14301437
///
14311438
/// Iterator Stack: `iterator` **=>** `iterator`
14321439
IteratorValue,
14331440

1441+
/// Gets the `value` property of the current iterator record.
1442+
///
1443+
/// Stack: **=>** `value`
1444+
///
1445+
/// Iterator Stack: `iterator` **=>** `iterator`
1446+
IteratorValueWithoutPop,
1447+
14341448
/// Gets the last iteration result of the current iterator record.
14351449
///
14361450
/// Stack: **=>** `result`
@@ -1756,10 +1770,6 @@ generate_impl! {
17561770
Reserved59 => Reserved,
17571771
/// Reserved [`Opcode`].
17581772
Reserved60 => Reserved,
1759-
/// Reserved [`Opcode`].
1760-
Reserved61 => Reserved,
1761-
/// Reserved [`Opcode`].
1762-
Reserved62 => Reserved,
17631773
}
17641774
}
17651775

0 commit comments

Comments
 (0)