Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Jump to bottom

[Merged by Bors] - Fix destructive for-of loop assignments #2803

Closed
wants to merge 2 commits into from
Closed

[Merged by Bors] - Fix destructive for-of loop assignments #2803

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
2 commits
Select commit Hold shift + click to select a range
345c133
Fix destructive for-of iterator assignments
raskad Apr 10, 2023
8850f3b
Switch to ThinVec
raskad Apr 11, 2023
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
56 changes: 40 additions & 16 deletions boa_engine/src/bytecompiler/declaration/declaration_pattern.rs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -176,6 +176,7 @@ impl ByteCompiler<'_, '_> {
Pattern::Array(pattern) => {
self.emit_opcode(Opcode::ValueNotNullOrUndefined);
self.emit_opcode(Opcode::GetIterator);
self.emit_opcode(Opcode::IteratorClosePush);
match pattern.bindings().split_last() {
None => self.emit_opcode(Opcode::PushFalse),
Some((last, rest)) => {
Expand All @@ -185,6 +186,7 @@ impl ByteCompiler<'_, '_> {
self.compile_array_pattern_element(last, def, true);
}
}
self.emit_opcode(Opcode::IteratorClosePop);
self.iterator_close(false);
}
}
Expand All @@ -209,7 +211,7 @@ impl ByteCompiler<'_, '_> {
match element {
// ArrayBindingPattern : [ Elision ]
Elision => {
self.emit_opcode(Opcode::IteratorNext);
self.emit_opcode(Opcode::IteratorNextSetDone);
if with_done {
self.emit_opcode(Opcode::IteratorUnwrapNext);
}
Expand All @@ -220,7 +222,7 @@ impl ByteCompiler<'_, '_> {
ident,
default_init,
} => {
self.emit_opcode(Opcode::IteratorNext);
self.emit_opcode(Opcode::IteratorNextSetDone);
self.emit_opcode(unwrapping);
if let Some(init) = default_init {
let skip = self.emit_opcode_with_operand(Opcode::JumpIfNotUndefined);
Expand All @@ -230,20 +232,31 @@ impl ByteCompiler<'_, '_> {
self.emit_binding(def, *ident);
}
PropertyAccess { access } => {
self.emit_opcode(Opcode::IteratorNext);
self.emit_opcode(unwrapping);
self.access_set(
Access::Property { access },
false,
ByteCompiler::access_set_top_of_stack_expr_fn,
);
self.access_set(Access::Property { access }, false, |compiler, level| {
if level != 0 {
compiler.emit_opcode(Opcode::RotateLeft);
compiler.emit_u8(level + 2);
compiler.emit_opcode(Opcode::RotateLeft);
compiler.emit_u8(level + 2);
}
compiler.emit_opcode(Opcode::IteratorNextSetDone);
compiler.emit_opcode(unwrapping);
if level != 0 {
compiler.emit_opcode(Opcode::RotateLeft);
compiler.emit_u8(level + 3 + u8::from(with_done));
compiler.emit_opcode(Opcode::RotateLeft);
compiler.emit_u8(level + 3 + u8::from(with_done));
compiler.emit_opcode(Opcode::RotateLeft);
compiler.emit_u8(level + 1);
}
});
}
// BindingElement : BindingPattern Initializer[opt]
Pattern {
pattern,
default_init,
} => {
self.emit_opcode(Opcode::IteratorNext);
self.emit_opcode(Opcode::IteratorNextSetDone);
self.emit_opcode(unwrapping);

if let Some(init) = default_init {
Expand All @@ -263,12 +276,23 @@ impl ByteCompiler<'_, '_> {
}
}
PropertyAccessRest { access } => {
self.emit_opcode(Opcode::IteratorToArray);
self.access_set(
Access::Property { access },
false,
ByteCompiler::access_set_top_of_stack_expr_fn,
);
self.access_set(Access::Property { access }, false, |compiler, level| {
if level != 0 {
compiler.emit_opcode(Opcode::RotateLeft);
compiler.emit_u8(level + 2);
compiler.emit_opcode(Opcode::RotateLeft);
compiler.emit_u8(level + 2);
}
compiler.emit_opcode(Opcode::IteratorToArray);
if level != 0 {
compiler.emit_opcode(Opcode::RotateLeft);
compiler.emit_u8(level + 3);
compiler.emit_opcode(Opcode::RotateLeft);
compiler.emit_u8(level + 3);
compiler.emit_opcode(Opcode::RotateLeft);
compiler.emit_u8(level + 1);
}
});
if with_done {
self.emit_opcode(Opcode::PushTrue);
}
Expand Down
77 changes: 47 additions & 30 deletions boa_engine/src/bytecompiler/statement/loop.rs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -89,15 +89,19 @@ impl ByteCompiler<'_, '_> {
label: Option<Sym>,
configurable_globals: bool,
) {
let init_bound_names = bound_names(for_in_loop.initializer());
if init_bound_names.is_empty() {
let initializer_bound_names = match for_in_loop.initializer() {
IterableLoopInitializer::Let(declaration)
| IterableLoopInitializer::Const(declaration) => bound_names(declaration),
_ => Vec::new(),
};
if initializer_bound_names.is_empty() {
self.compile_expr(for_in_loop.target(), true);
} else {
self.push_compile_environment(false);
let push_env = self.emit_opcode_with_two_operands(Opcode::PushDeclarativeEnvironment);

for name in init_bound_names {
self.create_mutable_binding(name, false, false);
for name in &initializer_bound_names {
self.create_mutable_binding(*name, false, false);
}
self.compile_expr(for_in_loop.target(), true);

Expand All @@ -119,12 +123,17 @@ impl ByteCompiler<'_, '_> {
self.patch_jump_with_target(continue_label, start_address);
self.patch_jump_with_target(loop_start, start_address);

self.push_compile_environment(false);
let push_env = self.emit_opcode_with_two_operands(Opcode::PushDeclarativeEnvironment);
self.emit_opcode(Opcode::Pop); // pop the `done` value.
self.emit_opcode(Opcode::IteratorNext);
let exit = self.emit_opcode_with_operand(Opcode::IteratorUnwrapNextOrJump);

let iteration_environment = if initializer_bound_names.is_empty() {
None
} else {
self.push_compile_environment(false);
Some(self.emit_opcode_with_two_operands(Opcode::PushDeclarativeEnvironment))
};

match for_in_loop.initializer() {
IterableLoopInitializer::Identifier(ident) => {
self.create_mutable_binding(*ident, true, true);
Expand Down Expand Up @@ -176,19 +185,18 @@ impl ByteCompiler<'_, '_> {
}
},
IterableLoopInitializer::Pattern(pattern) => {
for ident in bound_names(pattern) {
self.create_mutable_binding(ident, true, true);
}
self.compile_declaration_pattern(pattern, BindingOpcode::InitVar);
self.compile_declaration_pattern(pattern, BindingOpcode::SetName);
}
}

self.compile_stmt(for_in_loop.body(), false, configurable_globals);

let env_info = self.pop_compile_environment();
self.patch_jump_with_target(push_env.0, env_info.num_bindings as u32);
self.patch_jump_with_target(push_env.1, env_info.index as u32);
self.emit_opcode(Opcode::PopEnvironment);
if let Some(iteration_environment) = iteration_environment {
let env_info = self.pop_compile_environment();
self.patch_jump_with_target(iteration_environment.0, env_info.num_bindings as u32);
self.patch_jump_with_target(iteration_environment.1, env_info.index as u32);
self.emit_opcode(Opcode::PopEnvironment);
}

self.emit(Opcode::Jump, &[start_address]);

Expand All @@ -197,7 +205,9 @@ impl ByteCompiler<'_, '_> {
self.patch_jump(cont_exit_label);
self.pop_loop_control_info();
self.emit_opcode(Opcode::LoopEnd);
self.iterator_close(false);
self.emit_opcode(Opcode::Pop);
self.emit_opcode(Opcode::Pop);
self.emit_opcode(Opcode::Pop);

self.patch_jump(early_exit);
}
Expand All @@ -208,15 +218,19 @@ impl ByteCompiler<'_, '_> {
label: Option<Sym>,
configurable_globals: bool,
) {
let init_bound_names = bound_names(for_of_loop.initializer());
if init_bound_names.is_empty() {
let initializer_bound_names = match for_of_loop.initializer() {
IterableLoopInitializer::Let(declaration)
| IterableLoopInitializer::Const(declaration) => bound_names(declaration),
_ => Vec::new(),
};
if initializer_bound_names.is_empty() {
self.compile_expr(for_of_loop.iterable(), true);
} else {
self.push_compile_environment(false);
let push_env = self.emit_opcode_with_two_operands(Opcode::PushDeclarativeEnvironment);

for name in init_bound_names {
self.create_mutable_binding(name, false, false);
for name in &initializer_bound_names {
self.create_mutable_binding(*name, false, false);
}
self.compile_expr(for_of_loop.iterable(), true);

Expand All @@ -241,9 +255,6 @@ impl ByteCompiler<'_, '_> {
self.patch_jump_with_target(loop_start, start_address);
self.patch_jump_with_target(cont_label, start_address);

self.push_compile_environment(false);
let push_env = self.emit_opcode_with_two_operands(Opcode::PushDeclarativeEnvironment);

self.emit_opcode(Opcode::Pop); // pop the `done` value.
self.emit_opcode(Opcode::IteratorNext);
if for_of_loop.r#await() {
Expand All @@ -252,6 +263,13 @@ impl ByteCompiler<'_, '_> {
}
let exit = self.emit_opcode_with_operand(Opcode::IteratorUnwrapNextOrJump);

let iteration_environment = if initializer_bound_names.is_empty() {
None
} else {
self.push_compile_environment(false);
Some(self.emit_opcode_with_two_operands(Opcode::PushDeclarativeEnvironment))
};

match for_of_loop.initializer() {
IterableLoopInitializer::Identifier(ref ident) => {
self.create_mutable_binding(*ident, true, true);
Expand Down Expand Up @@ -303,19 +321,18 @@ impl ByteCompiler<'_, '_> {
}
},
IterableLoopInitializer::Pattern(pattern) => {
for ident in bound_names(pattern) {
self.create_mutable_binding(ident, true, true);
}
self.compile_declaration_pattern(pattern, BindingOpcode::InitVar);
self.compile_declaration_pattern(pattern, BindingOpcode::SetName);
}
}

self.compile_stmt(for_of_loop.body(), false, configurable_globals);

let env_info = self.pop_compile_environment();
self.patch_jump_with_target(push_env.0, env_info.num_bindings as u32);
self.patch_jump_with_target(push_env.1, env_info.index as u32);
self.emit_opcode(Opcode::PopEnvironment);
if let Some(iteration_environment) = iteration_environment {
let env_info = self.pop_compile_environment();
self.patch_jump_with_target(iteration_environment.0, env_info.num_bindings as u32);
self.patch_jump_with_target(iteration_environment.1, env_info.index as u32);
self.emit_opcode(Opcode::PopEnvironment);
}

self.emit(Opcode::Jump, &[start_address]);

Expand Down
12 changes: 9 additions & 3 deletions boa_engine/src/vm/call_frame/mod.rs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -2,14 +2,16 @@
//!
//! This module will provides everything needed to implement the `CallFrame`

use crate::{object::JsObject, vm::CodeBlock};
use boa_gc::{Finalize, Gc, Trace};

mod abrupt_record;
mod env_stack;

use crate::{object::JsObject, vm::CodeBlock};
use boa_gc::{Finalize, Gc, Trace};
use thin_vec::ThinVec;

pub(crate) use abrupt_record::AbruptCompletionRecord;
pub(crate) use env_stack::EnvStackEntry;

/// A `CallFrame` holds the state of a function call.
#[derive(Clone, Debug, Finalize, Trace)]
pub struct CallFrame {
Expand All @@ -33,6 +35,9 @@ pub struct CallFrame {
// When an async generator is resumed, the generator object is needed
// to fulfill the steps 4.e-j in [AsyncGeneratorStart](https://tc39.es/ecma262/#sec-asyncgeneratorstart).
pub(crate) async_generator: Option<JsObject>,

// Iterators and their `[[Done]]` flags that must be closed when an abrupt completion is thrown.
pub(crate) iterators: ThinVec<(JsObject, bool)>,
}

/// ---- `CallFrame` creation methods ----
Expand All @@ -52,6 +57,7 @@ impl CallFrame {
arg_count: 0,
generator_resume_kind: GeneratorResumeKind::Normal,
async_generator: None,
iterators: ThinVec::new(),
}
}

Expand Down
3 changes: 3 additions & 0 deletions boa_engine/src/vm/code_block.rs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -448,9 +448,12 @@ impl CodeBlock {
| Opcode::GetIterator
| Opcode::GetAsyncIterator
| Opcode::IteratorNext
| Opcode::IteratorNextSetDone
| Opcode::IteratorUnwrapNext
| Opcode::IteratorUnwrapValue
| Opcode::IteratorToArray
| Opcode::IteratorClosePush
| Opcode::IteratorClosePop
| Opcode::RequireObjectCoercible
| Opcode::ValueNotNullOrUndefined
| Opcode::RestParameterInit
Expand Down
3 changes: 3 additions & 0 deletions boa_engine/src/vm/flowgraph/mod.rs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -502,9 +502,12 @@ impl CodeBlock {
| Opcode::GetIterator
| Opcode::GetAsyncIterator
| Opcode::IteratorNext
| Opcode::IteratorNextSetDone
| Opcode::IteratorUnwrapNext
| Opcode::IteratorUnwrapValue
| Opcode::IteratorToArray
| Opcode::IteratorClosePush
| Opcode::IteratorClosePop
| Opcode::RequireObjectCoercible
| Opcode::ValueNotNullOrUndefined
| Opcode::RestParameterInit
Expand Down
16 changes: 16 additions & 0 deletions boa_engine/src/vm/opcode/control_flow/throw.rs
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -1,7 +1,9 @@
use crate::{
js_string,
vm::{call_frame::AbruptCompletionRecord, opcode::Operation, CompletionType},
Context, JsError, JsNativeError, JsResult,
};
use thin_vec::ThinVec;

/// `Throw` implements the Opcode Operation for `Opcode::Throw`
///
Expand All @@ -20,6 +22,20 @@ impl Operation for Throw {
} else {
JsError::from_opaque(context.vm.pop())
};

// Close all iterators that are still open.
let mut iterators = ThinVec::new();
std::mem::swap(&mut iterators, &mut context.vm.frame_mut().iterators);
for (iterator, done) in iterators {
if done {
continue;
}
if let Ok(Some(f)) = iterator.get_method(js_string!("return"), context) {
drop(f.call(&iterator.into(), &[], context));
}
}
context.vm.err.take();

// 1. Find the viable catch and finally blocks
let current_address = context.vm.frame().pc;
let viable_catch_candidates = context
Expand Down
Loading