From 9b25ecf49156257f04c199505dd0ff6a647701e2 Mon Sep 17 00:00:00 2001 From: Haled Odat <8566042+HalidOdat@users.noreply.github.com> Date: Sun, 25 Jun 2023 20:58:08 +0200 Subject: [PATCH] Simplify/Refactor exception handling and last statement value (#3053) --- boa_engine/src/bytecompiler/class.rs | 7 + boa_engine/src/bytecompiler/expression/mod.rs | 4 +- boa_engine/src/bytecompiler/function.rs | 1 - boa_engine/src/bytecompiler/jump_control.rs | 116 ++++++++++------- boa_engine/src/bytecompiler/mod.rs | 5 +- .../src/bytecompiler/statement/break.rs | 22 +--- .../src/bytecompiler/statement/continue.rs | 9 +- boa_engine/src/bytecompiler/statement/if.rs | 25 +--- .../src/bytecompiler/statement/labelled.rs | 4 +- boa_engine/src/bytecompiler/statement/loop.rs | 62 ++------- boa_engine/src/bytecompiler/statement/mod.rs | 30 ++++- .../src/bytecompiler/statement/switch.rs | 13 +- boa_engine/src/bytecompiler/statement/try.rs | 42 ++---- boa_engine/src/bytecompiler/statement/with.rs | 11 +- boa_engine/src/vm/call_frame/env_stack.rs | 120 +++++++----------- boa_engine/src/vm/call_frame/mod.rs | 10 +- boa_engine/src/vm/code_block.rs | 18 +-- boa_engine/src/vm/flowgraph/mod.rs | 21 +-- boa_engine/src/vm/mod.rs | 24 ++-- .../src/vm/opcode/control_flow/break.rs | 60 +-------- .../src/vm/opcode/control_flow/catch.rs | 91 ------------- .../src/vm/opcode/control_flow/continue.rs | 14 +- boa_engine/src/vm/opcode/control_flow/mod.rs | 2 - .../src/vm/opcode/control_flow/return.rs | 36 ++++++ .../src/vm/opcode/control_flow/throw.rs | 22 ++-- boa_engine/src/vm/opcode/control_flow/try.rs | 4 +- boa_engine/src/vm/opcode/generator/mod.rs | 7 +- .../src/vm/opcode/iteration/loop_ops.rs | 28 +--- boa_engine/src/vm/opcode/mod.rs | 82 ++++-------- boa_engine/src/vm/opcode/pop/mod.rs | 57 --------- 30 files changed, 314 insertions(+), 633 deletions(-) delete mode 100644 boa_engine/src/vm/opcode/control_flow/catch.rs diff --git a/boa_engine/src/bytecompiler/class.rs b/boa_engine/src/bytecompiler/class.rs index 7ccd406881..65d5a53b48 100644 --- a/boa_engine/src/bytecompiler/class.rs +++ b/boa_engine/src/bytecompiler/class.rs @@ -74,6 +74,7 @@ impl ByteCompiler<'_, '_> { compiler.pop_compile_environment(); } + compiler.emit_opcode(Opcode::SetReturnValue); compiler.emit_opcode(Opcode::Return); let code = Gc::new(compiler.finish()); @@ -269,6 +270,8 @@ impl ByteCompiler<'_, '_> { } field_compiler.pop_compile_environment(); field_compiler.pop_compile_environment(); + + field_compiler.emit_opcode(Opcode::SetReturnValue); field_compiler.emit_opcode(Opcode::Return); field_compiler.code_block_flags |= CodeBlockFlags::IN_CLASS_FIELD_INITIALIZER; @@ -301,6 +304,8 @@ impl ByteCompiler<'_, '_> { } field_compiler.pop_compile_environment(); field_compiler.pop_compile_environment(); + + field_compiler.emit_opcode(Opcode::SetReturnValue); field_compiler.emit_opcode(Opcode::Return); field_compiler.code_block_flags |= CodeBlockFlags::IN_CLASS_FIELD_INITIALIZER; @@ -343,6 +348,8 @@ impl ByteCompiler<'_, '_> { } field_compiler.pop_compile_environment(); field_compiler.pop_compile_environment(); + + field_compiler.emit_opcode(Opcode::SetReturnValue); field_compiler.emit_opcode(Opcode::Return); field_compiler.code_block_flags |= CodeBlockFlags::IN_CLASS_FIELD_INITIALIZER; diff --git a/boa_engine/src/bytecompiler/expression/mod.rs b/boa_engine/src/bytecompiler/expression/mod.rs index 8d57537604..ef118a5cfa 100644 --- a/boa_engine/src/bytecompiler/expression/mod.rs +++ b/boa_engine/src/bytecompiler/expression/mod.rs @@ -98,7 +98,6 @@ impl ByteCompiler<'_, '_> { Expression::Conditional(op) => self.compile_conditional(op, use_expr), Expression::ArrayLiteral(array) => { self.emit_opcode(Opcode::PushNewArray); - self.emit_opcode(Opcode::PopOnReturnAdd); for element in array.as_ref() { if let Some(element) = element { @@ -114,7 +113,6 @@ impl ByteCompiler<'_, '_> { } } - self.emit_opcode(Opcode::PopOnReturnSub); if !use_expr { self.emit(Opcode::Pop, &[]); } @@ -194,6 +192,8 @@ impl ByteCompiler<'_, '_> { self.emit_opcode(Opcode::Await); } self.close_active_iterators(); + + self.emit_opcode(Opcode::SetReturnValue); self.emit_opcode(Opcode::GeneratorResumeReturn); self.patch_jump(throw_method_undefined); diff --git a/boa_engine/src/bytecompiler/function.rs b/boa_engine/src/bytecompiler/function.rs index cf88f6b1e3..b64e963e6f 100644 --- a/boa_engine/src/bytecompiler/function.rs +++ b/boa_engine/src/bytecompiler/function.rs @@ -157,7 +157,6 @@ impl FunctionCompiler { .filter(|last| **last == Opcode::Return as u8) .is_none() { - compiler.emit_opcode(Opcode::PushUndefined); compiler.emit_opcode(Opcode::Return); } diff --git a/boa_engine/src/bytecompiler/jump_control.rs b/boa_engine/src/bytecompiler/jump_control.rs index fe6019dfbb..0c86fbd0ba 100644 --- a/boa_engine/src/bytecompiler/jump_control.rs +++ b/boa_engine/src/bytecompiler/jump_control.rs @@ -32,11 +32,15 @@ bitflags! { const SWITCH = 0b0000_0010; const TRY_BLOCK = 0b0000_0100; const LABELLED = 0b0000_1000; - const IN_CATCH = 0b0001_0000; - const IN_FINALLY = 0b0010_0000; - const HAS_FINALLY = 0b0100_0000; - const ITERATOR_LOOP = 0b1000_0000; - const FOR_AWAIT_OF_LOOP = 0b1_0000_0000; + const IN_FINALLY = 0b0001_0000; + const HAS_FINALLY = 0b0010_0000; + const ITERATOR_LOOP = 0b0100_0000; + const FOR_AWAIT_OF_LOOP = 0b1000_0000; + + /// Is the statement compiled with use_expr set to true. + /// + /// This bitflag is inherited if the previous [`JumpControlInfo`]. + const USE_EXPR = 0b0001_0000_0000; } } @@ -134,10 +138,6 @@ impl JumpControlInfo { self.flags.contains(JumpControlInfoFlags::LABELLED) } - pub(crate) const fn in_catch(&self) -> bool { - self.flags.contains(JumpControlInfoFlags::IN_CATCH) - } - pub(crate) const fn in_finally(&self) -> bool { self.flags.contains(JumpControlInfoFlags::IN_FINALLY) } @@ -146,6 +146,10 @@ impl JumpControlInfo { self.flags.contains(JumpControlInfoFlags::HAS_FINALLY) } + pub(crate) const fn use_expr(&self) -> bool { + self.flags.contains(JumpControlInfoFlags::USE_EXPR) + } + pub(crate) const fn iterator_loop(&self) -> bool { self.flags.contains(JumpControlInfoFlags::ITERATOR_LOOP) } @@ -168,11 +172,6 @@ impl JumpControlInfo { self.start_address = start_address; } - /// Sets the `in_catch` field of `JumpControlInfo`. - pub(crate) fn set_in_catch(&mut self, value: bool) { - self.flags.set(JumpControlInfoFlags::IN_CATCH, value); - } - /// Set the `in_finally` field of `JumpControlInfo`. pub(crate) fn set_in_finally(&mut self, value: bool) { self.flags.set(JumpControlInfoFlags::IN_FINALLY, value); @@ -198,9 +197,9 @@ impl ByteCompiler<'_, '_> { /// Pushes a generic `JumpControlInfo` onto `ByteCompiler` /// /// Default `JumpControlInfoKind` is `JumpControlInfoKind::Loop` - pub(crate) fn push_empty_loop_jump_control(&mut self) { - self.jump_info - .push(JumpControlInfo::default().with_loop_flag(true)); + pub(crate) fn push_empty_loop_jump_control(&mut self, use_expr: bool) { + let new_info = JumpControlInfo::default().with_loop_flag(true); + self.push_contol_info(new_info, use_expr); } pub(crate) fn current_jump_control_mut(&mut self) -> Option<&mut JumpControlInfo> { @@ -212,37 +211,43 @@ impl ByteCompiler<'_, '_> { info.set_start_address(start_address); } - pub(crate) fn set_jump_control_in_finally(&mut self, value: bool) { - if !self.jump_info.is_empty() { - let info = self - .jump_info - .last_mut() - .expect("must have try control label"); - assert!(info.is_try_block()); - info.set_in_finally(value); + pub(crate) fn push_contol_info(&mut self, mut info: JumpControlInfo, use_expr: bool) { + info.flags.set(JumpControlInfoFlags::USE_EXPR, use_expr); + + if let Some(last) = self.jump_info.last() { + // Inherits the `JumpControlInfoFlags::USE_EXPR` flag. + info.flags |= last.flags & JumpControlInfoFlags::USE_EXPR; } + + self.jump_info.push(info); } - pub(crate) fn set_jump_control_in_catch(&mut self, value: bool) { - if !self.jump_info.is_empty() { - let info = self - .jump_info - .last_mut() - .expect("must have try control label"); - assert!(info.is_try_block()); - info.set_in_catch(value); + /// Does the jump control info have the `use_expr` flag set to true. + /// + /// See [`JumpControlInfoFlags`]. + pub(crate) fn jump_control_info_has_use_expr(&self) -> bool { + if let Some(last) = self.jump_info.last() { + return last.use_expr(); } + + false } // ---- Labelled Statement JumpControlInfo methods ---- // /// Pushes a `LabelledStatement`'s `JumpControlInfo` onto the `jump_info` stack. - pub(crate) fn push_labelled_control_info(&mut self, label: Sym, start_address: u32) { + pub(crate) fn push_labelled_control_info( + &mut self, + label: Sym, + start_address: u32, + use_expr: bool, + ) { let new_info = JumpControlInfo::default() .with_labelled_block_flag(true) .with_label(Some(label)) .with_start_address(start_address); - self.jump_info.push(new_info); + + self.push_contol_info(new_info, use_expr); } /// Pops and handles the info for a label's `JumpControlInfo` @@ -267,12 +272,18 @@ impl ByteCompiler<'_, '_> { // ---- `IterationStatement`'s `JumpControlInfo` methods ---- // /// Pushes an `WhileStatement`, `ForStatement` or `DoWhileStatement`'s `JumpControlInfo` on to the `jump_info` stack. - pub(crate) fn push_loop_control_info(&mut self, label: Option, start_address: u32) { + pub(crate) fn push_loop_control_info( + &mut self, + label: Option, + start_address: u32, + use_expr: bool, + ) { let new_info = JumpControlInfo::default() .with_loop_flag(true) .with_label(label) .with_start_address(start_address); - self.jump_info.push(new_info); + + self.push_contol_info(new_info, use_expr); } /// Pushes a `ForInOfStatement`'s `JumpControlInfo` on to the `jump_info` stack. @@ -280,19 +291,22 @@ impl ByteCompiler<'_, '_> { &mut self, label: Option, start_address: u32, + use_expr: bool, ) { let new_info = JumpControlInfo::default() .with_loop_flag(true) .with_label(label) .with_start_address(start_address) .with_iterator_loop(true); - self.jump_info.push(new_info); + + self.push_contol_info(new_info, use_expr); } pub(crate) fn push_loop_control_info_for_await_of_loop( &mut self, label: Option, start_address: u32, + use_expr: bool, ) { let new_info = JumpControlInfo::default() .with_loop_flag(true) @@ -300,7 +314,8 @@ impl ByteCompiler<'_, '_> { .with_start_address(start_address) .with_iterator_loop(true) .with_for_await_of_loop(true); - self.jump_info.push(new_info); + + self.push_contol_info(new_info, use_expr); } /// Pops and handles the info for a loop control block's `JumpControlInfo` @@ -327,12 +342,18 @@ impl ByteCompiler<'_, '_> { // ---- `SwitchStatement` `JumpControlInfo` methods ---- // /// Pushes a `SwitchStatement`'s `JumpControlInfo` on to the `jump_info` stack. - pub(crate) fn push_switch_control_info(&mut self, label: Option, start_address: u32) { + pub(crate) fn push_switch_control_info( + &mut self, + label: Option, + start_address: u32, + use_expr: bool, + ) { let new_info = JumpControlInfo::default() .with_switch_flag(true) .with_label(label) .with_start_address(start_address); - self.jump_info.push(new_info); + + self.push_contol_info(new_info, use_expr); } /// Pops and handles the info for a switch block's `JumpControlInfo` @@ -354,13 +375,18 @@ impl ByteCompiler<'_, '_> { // ---- `TryStatement`'s `JumpControlInfo` methods ---- // /// Pushes a `TryStatement`'s `JumpControlInfo` onto the `jump_info` stack. - pub(crate) fn push_try_control_info(&mut self, has_finally: bool, start_address: u32) { + pub(crate) fn push_try_control_info( + &mut self, + has_finally: bool, + start_address: u32, + use_expr: bool, + ) { let new_info = JumpControlInfo::default() .with_try_block_flag(true) .with_start_address(start_address) .with_has_finally(has_finally); - self.jump_info.push(new_info); + self.push_contol_info(new_info, use_expr); } /// Pops and handles the info for a try block's `JumpControlInfo` @@ -406,12 +432,12 @@ impl ByteCompiler<'_, '_> { } /// Pushes a `TryStatement`'s Finally block `JumpControlInfo` onto the `jump_info` stack. - pub(crate) fn push_init_finally_control_info(&mut self) { + pub(crate) fn push_init_finally_control_info(&mut self, use_expr: bool) { let mut new_info = JumpControlInfo::default().with_try_block_flag(true); new_info.set_in_finally(true); - self.jump_info.push(new_info); + self.push_contol_info(new_info, use_expr); } pub(crate) fn pop_finally_control_info(&mut self) { diff --git a/boa_engine/src/bytecompiler/mod.rs b/boa_engine/src/bytecompiler/mod.rs index b4274e74c9..b372fb0a55 100644 --- a/boa_engine/src/bytecompiler/mod.rs +++ b/boa_engine/src/bytecompiler/mod.rs @@ -756,7 +756,7 @@ impl<'ctx, 'host> ByteCompiler<'ctx, 'host> { /// Compile a [`StatementList`]. pub fn compile_statement_list(&mut self, list: &StatementList, use_expr: bool, block: bool) { - if use_expr { + if use_expr || self.jump_control_info_has_use_expr() { let mut has_returns_value = false; let mut use_expr_index = 0; let mut first_return_is_abrupt = false; @@ -781,6 +781,7 @@ impl<'ctx, 'host> ByteCompiler<'ctx, 'host> { if first_return_is_abrupt { self.emit_opcode(Opcode::PushUndefined); + self.emit_opcode(Opcode::SetReturnValue); } for (i, item) in list.statements().iter().enumerate() { @@ -1055,7 +1056,7 @@ impl<'ctx, 'host> ByteCompiler<'ctx, 'host> { fn compile_stmt_list_item(&mut self, item: &StatementListItem, use_expr: bool, block: bool) { match item { StatementListItem::Statement(stmt) => { - self.compile_stmt(stmt, use_expr); + self.compile_stmt(stmt, use_expr, false); } StatementListItem::Declaration(decl) => self.compile_decl(decl, block), } diff --git a/boa_engine/src/bytecompiler/statement/break.rs b/boa_engine/src/bytecompiler/statement/break.rs index 157a9a9924..eb51b429cd 100644 --- a/boa_engine/src/bytecompiler/statement/break.rs +++ b/boa_engine/src/bytecompiler/statement/break.rs @@ -7,26 +7,12 @@ use boa_interner::Sym; impl ByteCompiler<'_, '_> { /// Compile a [`Break`] `boa_ast` node - pub(crate) fn compile_break(&mut self, node: Break) { - let opcode = if node.label().is_some() { - Opcode::BreakLabel - } else { - Opcode::Break - }; - + pub(crate) fn compile_break(&mut self, node: Break, _use_expr: bool) { if let Some(info) = self.jump_info.last().filter(|info| info.is_try_block()) { - let in_finally = info.in_finally(); - let in_catch_no_finally = info.in_catch() && !info.has_finally(); let has_finally_or_is_finally = info.has_finally() || info.in_finally(); - if in_finally { - self.emit_opcode(Opcode::PopIfThrown); - } - if in_finally || in_catch_no_finally { - self.emit_opcode(Opcode::CatchEnd2); - } - - let (break_label, target_jump_label) = self.emit_opcode_with_two_operands(opcode); + let (break_label, target_jump_label) = + self.emit_opcode_with_two_operands(Opcode::Break); if let Some(node_label) = node.label() { let info = self.jump_info_label(node_label); @@ -54,7 +40,7 @@ impl ByteCompiler<'_, '_> { } // Emit the break opcode -> (Label, Label) - let (break_label, target_label) = self.emit_opcode_with_two_operands(opcode); + let (break_label, target_label) = self.emit_opcode_with_two_operands(Opcode::Break); if let Some(label) = node.label() { let info = self.jump_info_label(label); info.push_break_label(break_label); diff --git a/boa_engine/src/bytecompiler/statement/continue.rs b/boa_engine/src/bytecompiler/statement/continue.rs index 729e9e7e35..e6f68e9c5d 100644 --- a/boa_engine/src/bytecompiler/statement/continue.rs +++ b/boa_engine/src/bytecompiler/statement/continue.rs @@ -3,18 +3,11 @@ use boa_ast::statement::Continue; impl ByteCompiler<'_, '_> { #[allow(clippy::unnecessary_wraps)] - pub(crate) fn compile_continue(&mut self, node: Continue) { + pub(crate) fn compile_continue(&mut self, node: Continue, _use_expr: bool) { if let Some(info) = self.jump_info.last().filter(|info| info.is_try_block()) { let in_finally = info.in_finally(); let in_finally_or_has_finally = in_finally || info.has_finally(); - let in_catch_no_finally = !info.has_finally() && info.in_catch(); - if in_finally { - self.emit_opcode(Opcode::PopIfThrown); - } - if in_finally || in_catch_no_finally { - self.emit_opcode(Opcode::CatchEnd2); - } // 1. Handle if node has a label. if let Some(node_label) = node.label() { let items = self.jump_info.iter().rev().filter(|info| info.is_loop()); diff --git a/boa_engine/src/bytecompiler/statement/if.rs b/boa_engine/src/bytecompiler/statement/if.rs index 1838d7985a..d6b0c8e947 100644 --- a/boa_engine/src/bytecompiler/statement/if.rs +++ b/boa_engine/src/bytecompiler/statement/if.rs @@ -1,33 +1,18 @@ -use crate::{bytecompiler::ByteCompiler, vm::Opcode}; -use boa_ast::{operations::returns_value, statement::If}; +use crate::bytecompiler::ByteCompiler; +use boa_ast::statement::If; impl ByteCompiler<'_, '_> { pub(crate) fn compile_if(&mut self, node: &If, use_expr: bool) { self.compile_expr(node.cond(), true); let jelse = self.jump_if_false(); - if !returns_value(node.body()) { - self.emit_opcode(Opcode::PushUndefined); - } - self.compile_stmt(node.body(), true); + self.compile_stmt(node.body(), use_expr, true); let exit = self.jump(); self.patch_jump(jelse); - match node.else_node() { - None => { - self.emit_opcode(Opcode::PushUndefined); - } - Some(else_body) => { - if !returns_value(else_body) { - self.emit_opcode(Opcode::PushUndefined); - } - self.compile_stmt(else_body, true); - } + if let Some(else_body) = node.else_node() { + self.compile_stmt(else_body, use_expr, true); } self.patch_jump(exit); - - if !use_expr { - self.emit_opcode(Opcode::Pop); - } } } diff --git a/boa_engine/src/bytecompiler/statement/labelled.rs b/boa_engine/src/bytecompiler/statement/labelled.rs index 11f31583ac..8ae5612187 100644 --- a/boa_engine/src/bytecompiler/statement/labelled.rs +++ b/boa_engine/src/bytecompiler/statement/labelled.rs @@ -12,7 +12,7 @@ impl ByteCompiler<'_, '_> { pub(crate) fn compile_labelled(&mut self, labelled: &Labelled, use_expr: bool) { let labelled_loc = self.next_opcode_location(); let end_label = self.emit_opcode_with_operand(Opcode::LabelledStart); - self.push_labelled_control_info(labelled.label(), labelled_loc); + self.push_labelled_control_info(labelled.label(), labelled_loc, use_expr); match labelled.item() { LabelledItem::Statement(stmt) => match stmt { @@ -31,7 +31,7 @@ impl ByteCompiler<'_, '_> { Statement::DoWhileLoop(do_while_loop) => { self.compile_do_while_loop(do_while_loop, Some(labelled.label()), use_expr); } - stmt => self.compile_stmt(stmt, use_expr), + stmt => self.compile_stmt(stmt, use_expr, true), }, LabelledItem::Function(f) => { self.function_with_binding(f.into(), NodeKind::Declaration, false); diff --git a/boa_engine/src/bytecompiler/statement/loop.rs b/boa_engine/src/bytecompiler/statement/loop.rs index e2aa879bf5..e0b659041a 100644 --- a/boa_engine/src/bytecompiler/statement/loop.rs +++ b/boa_engine/src/bytecompiler/statement/loop.rs @@ -1,6 +1,6 @@ use boa_ast::{ declaration::Binding, - operations::{bound_names, returns_value}, + operations::bound_names, statement::{ iteration::{ForLoopInitializer, IterableLoopInitializer}, DoWhileLoop, ForInLoop, ForLoop, ForOfLoop, WhileLoop, @@ -56,7 +56,7 @@ impl ByteCompiler<'_, '_> { } } - self.push_empty_loop_jump_control(); + self.push_empty_loop_jump_control(use_expr); let (loop_start, loop_exit) = self.emit_opcode_with_two_operands(Opcode::LoopStart); let initial_jump = self.jump(); let start_address = self.next_opcode_location(); @@ -96,11 +96,7 @@ impl ByteCompiler<'_, '_> { } let exit = self.jump_if_false(); - if !returns_value(for_loop.body()) { - self.emit_opcode(Opcode::PushUndefined); - } - self.compile_stmt(for_loop.body(), true); - self.emit_opcode(Opcode::LoopUpdateReturnValue); + self.compile_stmt(for_loop.body(), use_expr, true); self.emit(Opcode::Jump, &[start_address]); @@ -109,10 +105,6 @@ impl ByteCompiler<'_, '_> { self.pop_loop_control_info(); self.emit_opcode(Opcode::LoopEnd); - if !use_expr { - self.emit_opcode(Opcode::Pop); - } - if let Some(env_labels) = env_labels { let env_index = self.pop_compile_environment(); self.patch_jump_with_target(env_labels, env_index); @@ -165,7 +157,7 @@ impl ByteCompiler<'_, '_> { let (loop_start, exit_label) = self.emit_opcode_with_two_operands(Opcode::IteratorLoopStart); let start_address = self.next_opcode_location(); - self.push_loop_control_info_for_of_in_loop(label, start_address); + self.push_loop_control_info_for_of_in_loop(label, start_address, use_expr); self.emit_opcode(Opcode::LoopContinue); self.patch_jump_with_target(loop_start, start_address); @@ -230,11 +222,7 @@ impl ByteCompiler<'_, '_> { } } - if !returns_value(for_in_loop.body()) { - self.emit_opcode(Opcode::PushUndefined); - } - self.compile_stmt(for_in_loop.body(), true); - self.emit_opcode(Opcode::LoopUpdateReturnValue); + self.compile_stmt(for_in_loop.body(), use_expr, true); if let Some(iteration_environment) = iteration_environment { let env_index = self.pop_compile_environment(); @@ -255,10 +243,6 @@ impl ByteCompiler<'_, '_> { self.patch_jump(early_exit); self.emit_opcode(Opcode::PushUndefined); self.patch_jump(skip_early_exit); - - if !use_expr { - self.emit_opcode(Opcode::Pop); - } } pub(crate) fn compile_for_of_loop( @@ -297,9 +281,9 @@ impl ByteCompiler<'_, '_> { let (loop_start, loop_exit) = self.emit_opcode_with_two_operands(Opcode::IteratorLoopStart); let start_address = self.next_opcode_location(); if for_of_loop.r#await() { - self.push_loop_control_info_for_await_of_loop(label, start_address); + self.push_loop_control_info_for_await_of_loop(label, start_address, use_expr); } else { - self.push_loop_control_info_for_of_in_loop(label, start_address); + self.push_loop_control_info_for_of_in_loop(label, start_address, use_expr); } self.emit_opcode(Opcode::LoopContinue); self.patch_jump_with_target(loop_start, start_address); @@ -386,11 +370,7 @@ impl ByteCompiler<'_, '_> { } } - if !returns_value(for_of_loop.body()) { - self.emit_opcode(Opcode::PushUndefined); - } - self.compile_stmt(for_of_loop.body(), true); - self.emit_opcode(Opcode::LoopUpdateReturnValue); + self.compile_stmt(for_of_loop.body(), use_expr, true); if let Some(iteration_environment) = iteration_environment { let env_index = self.pop_compile_environment(); @@ -406,10 +386,6 @@ impl ByteCompiler<'_, '_> { self.emit_opcode(Opcode::LoopEnd); self.iterator_close(for_of_loop.r#await()); - - if !use_expr { - self.emit_opcode(Opcode::Pop); - } } pub(crate) fn compile_while_loop( @@ -421,17 +397,13 @@ impl ByteCompiler<'_, '_> { let (loop_start, loop_exit) = self.emit_opcode_with_two_operands(Opcode::LoopStart); let start_address = self.next_opcode_location(); self.emit_opcode(Opcode::LoopContinue); - self.push_loop_control_info(label, start_address); + self.push_loop_control_info(label, start_address, use_expr); self.patch_jump_with_target(loop_start, start_address); self.compile_expr(while_loop.condition(), true); let exit = self.jump_if_false(); - if !returns_value(while_loop.body()) { - self.emit_opcode(Opcode::PushUndefined); - } - self.compile_stmt(while_loop.body(), true); - self.emit_opcode(Opcode::LoopUpdateReturnValue); + self.compile_stmt(while_loop.body(), use_expr, true); self.emit(Opcode::Jump, &[start_address]); @@ -439,9 +411,6 @@ impl ByteCompiler<'_, '_> { self.patch_jump(loop_exit); self.pop_loop_control_info(); self.emit_opcode(Opcode::LoopEnd); - if !use_expr { - self.emit_opcode(Opcode::Pop); - } } pub(crate) fn compile_do_while_loop( @@ -456,7 +425,7 @@ impl ByteCompiler<'_, '_> { let start_address = self.next_opcode_location(); self.patch_jump_with_target(loop_start, start_address); - self.push_loop_control_info(label, start_address); + self.push_loop_control_info(label, start_address, use_expr); let condition_label_address = self.next_opcode_location(); self.emit_opcode(Opcode::LoopContinue); @@ -465,11 +434,7 @@ impl ByteCompiler<'_, '_> { self.patch_jump(initial_label); - if !returns_value(do_while_loop.body()) { - self.emit_opcode(Opcode::PushUndefined); - } - self.compile_stmt(do_while_loop.body(), true); - self.emit_opcode(Opcode::LoopUpdateReturnValue); + self.compile_stmt(do_while_loop.body(), use_expr, true); self.emit(Opcode::Jump, &[condition_label_address]); self.patch_jump(exit); @@ -477,8 +442,5 @@ impl ByteCompiler<'_, '_> { self.pop_loop_control_info(); self.emit_opcode(Opcode::LoopEnd); - if !use_expr { - self.emit_opcode(Opcode::Pop); - } } } diff --git a/boa_engine/src/bytecompiler/statement/mod.rs b/boa_engine/src/bytecompiler/statement/mod.rs index 54b54c2e09..c9c3ea6fc8 100644 --- a/boa_engine/src/bytecompiler/statement/mod.rs +++ b/boa_engine/src/bytecompiler/statement/mod.rs @@ -14,7 +14,7 @@ mod with; impl ByteCompiler<'_, '_> { /// Compiles a [`Statement`] `boa_ast` node. - pub fn compile_stmt(&mut self, node: &Statement, use_expr: bool) { + pub fn compile_stmt(&mut self, node: &Statement, use_expr: bool, root_statement: bool) { match node { Statement::Var(var) => self.compile_var_decl(var), Statement::If(node) => self.compile_if(node, use_expr), @@ -39,8 +39,20 @@ impl ByteCompiler<'_, '_> { Statement::Labelled(labelled) => { self.compile_labelled(labelled, use_expr); } - Statement::Continue(node) => self.compile_continue(*node), - Statement::Break(node) => self.compile_break(*node), + Statement::Continue(node) => { + if root_statement && (use_expr || self.jump_control_info_has_use_expr()) { + self.emit_opcode(Opcode::PushUndefined); + self.emit_opcode(Opcode::SetReturnValue); + } + self.compile_continue(*node, use_expr); + } + Statement::Break(node) => { + if root_statement && (use_expr || self.jump_control_info_has_use_expr()) { + self.emit_opcode(Opcode::PushUndefined); + self.emit_opcode(Opcode::SetReturnValue); + } + self.compile_break(*node, use_expr); + } Statement::Throw(throw) => { self.compile_expr(throw.target(), true); self.emit(Opcode::Throw, &[]); @@ -56,12 +68,18 @@ impl ByteCompiler<'_, '_> { self.emit_opcode(Opcode::GeneratorNext); } } else { - self.emit(Opcode::PushUndefined, &[]); + self.emit_opcode(Opcode::PushUndefined); } - self.emit(Opcode::Return, &[]); + self.emit_opcode(Opcode::SetReturnValue); + self.emit_opcode(Opcode::Return); } Statement::Try(t) => self.compile_try(t, use_expr), - Statement::Expression(expr) => self.compile_expr(expr, use_expr), + Statement::Expression(expr) => { + self.compile_expr(expr, use_expr); + if use_expr { + self.emit_opcode(Opcode::SetReturnValue); + } + } Statement::With(with) => self.compile_with(with, use_expr), Statement::Empty => {} } diff --git a/boa_engine/src/bytecompiler/statement/switch.rs b/boa_engine/src/bytecompiler/statement/switch.rs index e0b67a0065..55eae91033 100644 --- a/boa_engine/src/bytecompiler/statement/switch.rs +++ b/boa_engine/src/bytecompiler/statement/switch.rs @@ -1,5 +1,5 @@ use crate::{bytecompiler::ByteCompiler, vm::Opcode}; -use boa_ast::{operations::returns_value, statement::Switch}; +use boa_ast::statement::Switch; impl ByteCompiler<'_, '_> { /// Compile a [`Switch`] `boa_ast` node @@ -14,7 +14,7 @@ impl ByteCompiler<'_, '_> { let (start_label, end_label) = self.emit_opcode_with_two_operands(Opcode::LoopStart); let start_address = self.next_opcode_location(); - self.push_switch_control_info(None, start_address); + self.push_switch_control_info(None, start_address, use_expr); self.patch_jump_with_target(start_label, start_address); let mut labels = Vec::with_capacity(switch.cases().len()); @@ -43,10 +43,8 @@ impl ByteCompiler<'_, '_> { label }; self.patch_jump(label); - self.compile_statement_list(case.body(), true, true); - if returns_value(case) { - self.emit_opcode(Opcode::LoopUpdateReturnValue); - } + + self.compile_statement_list(case.body(), use_expr, true); } if !default_label_set { @@ -56,9 +54,6 @@ impl ByteCompiler<'_, '_> { self.pop_switch_control_info(); self.patch_jump(end_label); self.emit_opcode(Opcode::LoopEnd); - if !use_expr { - self.emit_opcode(Opcode::Pop); - } let env_index = self.pop_compile_environment(); self.patch_jump_with_target(push_env, env_index); diff --git a/boa_engine/src/bytecompiler/statement/try.rs b/boa_engine/src/bytecompiler/statement/try.rs index 082bbcd075..6f9edfeb23 100644 --- a/boa_engine/src/bytecompiler/statement/try.rs +++ b/boa_engine/src/bytecompiler/statement/try.rs @@ -4,7 +4,7 @@ use crate::{ }; use boa_ast::{ declaration::Binding, - operations::{bound_names, returns_value}, + operations::bound_names, statement::{Catch, Finally, Try}, }; @@ -16,17 +16,11 @@ impl ByteCompiler<'_, '_> { // If there is a finally block, initialize the finally control block prior to pushing the try block jump_control if t.finally().is_some() { - self.push_init_finally_control_info(); + self.push_init_finally_control_info(use_expr); } - self.push_try_control_info(t.finally().is_some(), try_start); + self.push_try_control_info(t.finally().is_some(), try_start, use_expr); - self.compile_block(t.block(), true); - if !returns_value(t.block()) { - self.emit_opcode(Opcode::PushUndefined); - } - if !use_expr { - self.emit_opcode(Opcode::Pop); - } + self.compile_block(t.block(), use_expr); self.emit_opcode(Opcode::TryEnd); @@ -55,10 +49,7 @@ impl ByteCompiler<'_, '_> { } } - pub(crate) fn compile_catch_stmt(&mut self, catch: &Catch, finally: bool, use_expr: bool) { - self.set_jump_control_in_catch(true); - let catch_end = self.emit_opcode_with_operand(Opcode::CatchStart); - + pub(crate) fn compile_catch_stmt(&mut self, catch: &Catch, _finally: bool, use_expr: bool) { self.push_compile_environment(false); let push_env = self.emit_opcode_with_operand(Opcode::PushDeclarativeEnvironment); @@ -79,33 +70,18 @@ impl ByteCompiler<'_, '_> { self.emit_opcode(Opcode::Pop); } - self.compile_block(catch.block(), true); - if !returns_value(catch.block()) { - self.emit_opcode(Opcode::PushUndefined); - } - if !use_expr { - self.emit_opcode(Opcode::Pop); - } + self.compile_block(catch.block(), use_expr); let env_index = self.pop_compile_environment(); self.patch_jump_with_target(push_env, env_index); self.emit_opcode(Opcode::PopEnvironment); - - if finally { - self.emit_opcode(Opcode::CatchEnd); - } else { - self.emit_opcode(Opcode::CatchEnd2); - } - - self.patch_jump(catch_end); - self.set_jump_control_in_finally(false); } pub(crate) fn compile_finally_stmt(&mut self, finally: &Finally, finally_end_label: Label) { + // TODO: We could probably remove the Get/SetReturnValue if we check that there is no break/continues statements. + self.emit_opcode(Opcode::GetReturnValue); self.compile_block(finally.block(), true); - if returns_value(finally.block()) { - self.emit_opcode(Opcode::Pop); - } + self.emit_opcode(Opcode::SetReturnValue); self.pop_finally_control_info(); self.patch_jump(finally_end_label); diff --git a/boa_engine/src/bytecompiler/statement/with.rs b/boa_engine/src/bytecompiler/statement/with.rs index 3d42c15886..0b63edee26 100644 --- a/boa_engine/src/bytecompiler/statement/with.rs +++ b/boa_engine/src/bytecompiler/statement/with.rs @@ -1,5 +1,5 @@ use crate::{bytecompiler::ByteCompiler, vm::Opcode}; -use boa_ast::{operations::returns_value, statement::With}; +use boa_ast::statement::With; impl ByteCompiler<'_, '_> { /// Compile a [`With`] `boa_ast` node @@ -8,16 +8,9 @@ impl ByteCompiler<'_, '_> { self.push_compile_environment(false); self.emit_opcode(Opcode::PushObjectEnvironment); - if !returns_value(with.statement()) { - self.emit_opcode(Opcode::PushUndefined); - } - self.compile_stmt(with.statement(), true); + self.compile_stmt(with.statement(), use_expr, true); self.pop_compile_environment(); self.emit_opcode(Opcode::PopEnvironment); - - if !use_expr { - self.emit_opcode(Opcode::Pop); - } } } diff --git a/boa_engine/src/vm/call_frame/env_stack.rs b/boa_engine/src/vm/call_frame/env_stack.rs index 95b1ed7131..76cf451f4e 100644 --- a/boa_engine/src/vm/call_frame/env_stack.rs +++ b/boa_engine/src/vm/call_frame/env_stack.rs @@ -1,43 +1,28 @@ //! Module for implementing a `CallFrame`'s environment stacks -use crate::JsValue; -use boa_gc::{Finalize, Trace}; - -#[derive(Clone, Debug, Finalize, Trace)] +#[derive(Clone, Debug)] pub(crate) enum EnvEntryKind { Global, Loop { /// How many iterations a loop has done. iteration_count: u64, - /// The latest return value of the loop. - value: JsValue, - /// The index of the currently active iterator. iterator: Option, }, - Try, - Catch, + Try { + /// The length of the value stack when the try block was entered. + /// + /// This is used to pop exact amount values from the stack + /// when a throw happens. + fp: u32, + }, Finally, Labelled, } -impl PartialEq for EnvEntryKind { - fn eq(&self, other: &Self) -> bool { - matches!( - (self, other), - (Self::Global, Self::Global) - | (Self::Loop { .. }, Self::Loop { .. }) - | (Self::Try, Self::Try) - | (Self::Catch, Self::Catch) - | (Self::Finally, Self::Finally) - | (Self::Labelled, Self::Labelled) - ) - } -} - /// The `EnvStackEntry` tracks the environment count and relevant information for the current environment. -#[derive(Clone, Debug, Finalize, Trace)] +#[derive(Clone, Debug)] pub(crate) struct EnvStackEntry { start: u32, exit: u32, @@ -58,7 +43,7 @@ impl Default for EnvStackEntry { /// ---- `EnvStackEntry` creation methods ---- impl EnvStackEntry { - /// Creates a new `EnvStackEntry` with the supplied start addresses. + /// Creates a new [`EnvStackEntry`] with the supplied start addresses. pub(crate) const fn new(start_address: u32, exit_address: u32) -> Self { Self { start: start_address, @@ -68,48 +53,44 @@ impl EnvStackEntry { } } - /// Returns calling `EnvStackEntry` with `kind` field of `Try`. - pub(crate) fn with_try_flag(mut self) -> Self { - self.kind = EnvEntryKind::Try; + /// Returns calling [`EnvStackEntry`] with `kind` field of [`EnvEntryKind::Try`]. + pub(crate) const fn with_try_flag(mut self, fp: u32) -> Self { + self.kind = EnvEntryKind::Try { fp }; self } - /// Returns calling `EnvStackEntry` with `kind` field of `Loop`, loop iteration set to zero + /// Returns calling [`EnvStackEntry`] with `kind` field of [`EnvEntryKind::Loop`], loop iteration set to zero /// and iterator index set to `iterator`. - pub(crate) fn with_iterator_loop_flag(mut self, iteration_count: u64, iterator: u32) -> Self { + pub(crate) const fn with_iterator_loop_flag( + mut self, + iteration_count: u64, + iterator: u32, + ) -> Self { self.kind = EnvEntryKind::Loop { iteration_count, - value: JsValue::undefined(), iterator: Some(iterator), }; self } - /// Returns calling `EnvStackEntry` with `kind` field of `Loop`. + /// Returns calling [`EnvStackEntry`] with `kind` field of [`EnvEntryKind::Loop`]. /// And the loop iteration set to zero. - pub(crate) fn with_loop_flag(mut self, iteration_count: u64) -> Self { + pub(crate) const fn with_loop_flag(mut self, iteration_count: u64) -> Self { self.kind = EnvEntryKind::Loop { iteration_count, - value: JsValue::undefined(), iterator: None, }; self } - /// Returns calling `EnvStackEntry` with `kind` field of `Catch`. - pub(crate) fn with_catch_flag(mut self) -> Self { - self.kind = EnvEntryKind::Catch; - self - } - - /// Returns calling `EnvStackEntry` with `kind` field of `Finally`. - pub(crate) fn with_finally_flag(mut self) -> Self { + /// Returns calling [`EnvStackEntry`] with `kind` field of [`EnvEntryKind::Finally`]. + pub(crate) const fn with_finally_flag(mut self) -> Self { self.kind = EnvEntryKind::Finally; self } - /// Returns calling `EnvStackEntry` with `kind` field of `Labelled`. - pub(crate) fn with_labelled_flag(mut self) -> Self { + /// Returns calling [`EnvStackEntry`] with `kind` field of [`EnvEntryKind::Labelled`]. + pub(crate) const fn with_labelled_flag(mut self) -> Self { self.kind = EnvEntryKind::Labelled; self } @@ -132,8 +113,20 @@ impl EnvStackEntry { self.exit } - pub(crate) fn is_global_env(&self) -> bool { - self.kind == EnvEntryKind::Global + /// Returns the `fp` field of this [`EnvEntryKind::Try`]. + /// + /// # Panics + /// + /// If this [`EnvStackEntry`] is **not** a [`EnvEntryKind::Try`]. + pub(crate) fn try_env_frame_pointer(&self) -> u32 { + if let EnvEntryKind::Try { fp } = &self.kind { + return *fp; + } + unreachable!("trying to get frame pointer of a non-try environment") + } + + pub(crate) const fn is_global_env(&self) -> bool { + matches!(self.kind, EnvEntryKind::Global) } /// Returns the loop iteration count if `EnvStackEntry` is a loop. @@ -157,14 +150,6 @@ impl EnvStackEntry { } } - /// Returns the loop return value if `EnvStackEntry` is a loop. - pub(crate) const fn loop_env_value(&self) -> Option<&JsValue> { - if let EnvEntryKind::Loop { value, .. } = &self.kind { - return Some(value); - } - None - } - /// Returns the active iterator index if `EnvStackEntry` is an iterator loop. pub(crate) const fn iterator(&self) -> Option { if let EnvEntryKind::Loop { iterator, .. } = self.kind { @@ -174,22 +159,21 @@ impl EnvStackEntry { } /// Returns true if an `EnvStackEntry` is a try block - pub(crate) fn is_try_env(&self) -> bool { - self.kind == EnvEntryKind::Try + pub(crate) const fn is_try_env(&self) -> bool { + matches!(self.kind, EnvEntryKind::Try { .. }) } /// Returns true if an `EnvStackEntry` is a labelled block - pub(crate) fn is_labelled_env(&self) -> bool { - self.kind == EnvEntryKind::Labelled + pub(crate) const fn is_labelled_env(&self) -> bool { + matches!(self.kind, EnvEntryKind::Labelled) } - /// Returns true if an `EnvStackEntry` is a catch block - pub(crate) fn is_catch_env(&self) -> bool { - self.kind == EnvEntryKind::Catch + pub(crate) const fn is_finally_env(&self) -> bool { + matches!(self.kind, EnvEntryKind::Finally) } - pub(crate) fn is_finally_env(&self) -> bool { - self.kind == EnvEntryKind::Finally + pub(crate) const fn is_loop_env(&self) -> bool { + matches!(self.kind, EnvEntryKind::Loop { .. }) } /// Returns the current environment number for this entry. @@ -214,14 +198,4 @@ impl EnvStackEntry { pub(crate) fn dec_env_num(&mut self) { (self.env_num, _) = self.env_num.overflowing_sub(1); } - - /// Set the loop return value for the current `EnvStackEntry`. - pub(crate) fn set_loop_return_value(&mut self, value: JsValue) -> bool { - if let EnvEntryKind::Loop { value: v, .. } = &mut self.kind { - *v = value; - true - } else { - false - } - } } diff --git a/boa_engine/src/vm/call_frame/mod.rs b/boa_engine/src/vm/call_frame/mod.rs index 8bd151cd07..de97c2cb11 100644 --- a/boa_engine/src/vm/call_frame/mod.rs +++ b/boa_engine/src/vm/call_frame/mod.rs @@ -10,6 +10,7 @@ use crate::{ environments::BindingLocator, object::JsObject, vm::CodeBlock, + JsValue, }; use boa_gc::{Finalize, Gc, Trace}; use thin_vec::ThinVec; @@ -27,9 +28,9 @@ pub struct CallFrame { pub(crate) abrupt_completion: Option, #[unsafe_ignore_trace] pub(crate) r#yield: bool, - pub(crate) pop_on_return: u32, // Tracks the number of environments in environment entry. // On abrupt returns this is used to decide how many environments need to be pop'ed. + #[unsafe_ignore_trace] pub(crate) env_stack: Vec, pub(crate) argument_count: u32, #[unsafe_ignore_trace] @@ -45,6 +46,11 @@ pub struct CallFrame { // The stack of bindings being updated. pub(crate) binding_stack: Vec, + + /// The value that is returned from the function. + // + // TODO(HalidOdat): Remove this and put into the stack, maybe before frame pointer. + pub(crate) return_value: JsValue, } /// ---- `CallFrame` public API ---- @@ -65,7 +71,6 @@ impl CallFrame { code_block, pc: 0, fp: 0, - pop_on_return: 0, env_stack: Vec::from([EnvStackEntry::new(0, max_length)]), abrupt_completion: None, r#yield: false, @@ -75,6 +80,7 @@ impl CallFrame { async_generator: None, iterators: ThinVec::new(), binding_stack: Vec::new(), + return_value: JsValue::undefined(), } } diff --git a/boa_engine/src/vm/code_block.rs b/boa_engine/src/vm/code_block.rs index d0cd9ad397..c2cf7193c9 100644 --- a/boa_engine/src/vm/code_block.rs +++ b/boa_engine/src/vm/code_block.rs @@ -315,7 +315,6 @@ impl CodeBlock { | Opcode::JumpIfFalse | Opcode::JumpIfNotUndefined | Opcode::JumpIfNullOrUndefined - | Opcode::CatchStart | Opcode::FinallyStart | Opcode::LabelledStart | Opcode::Case @@ -339,7 +338,6 @@ impl CodeBlock { } Opcode::CopyDataProperties | Opcode::Break - | Opcode::BreakLabel | Opcode::Continue | Opcode::LoopStart | Opcode::IteratorLoopStart @@ -449,7 +447,6 @@ impl CodeBlock { format!("done: {done}") } Opcode::Pop - | Opcode::PopIfThrown | Opcode::Dup | Opcode::Swap | Opcode::PushZero @@ -516,8 +513,6 @@ impl CodeBlock { | Opcode::ToBoolean | Opcode::Throw | Opcode::TryEnd - | Opcode::CatchEnd - | Opcode::CatchEnd2 | Opcode::FinallyEnd | Opcode::This | Opcode::Super @@ -525,7 +520,6 @@ impl CodeBlock { | Opcode::PopEnvironment | Opcode::LoopEnd | Opcode::LoopContinue - | Opcode::LoopUpdateReturnValue | Opcode::LabelledEnd | Opcode::CreateForInIterator | Opcode::GetIterator @@ -548,8 +542,6 @@ impl CodeBlock { | Opcode::PushElisionToArray | Opcode::PushIteratorToArray | Opcode::PushNewArray - | Opcode::PopOnReturnAdd - | Opcode::PopOnReturnSub | Opcode::GeneratorYield | Opcode::AsyncGeneratorYield | Opcode::GeneratorNext @@ -570,6 +562,8 @@ impl CodeBlock { | Opcode::SetNameByLocator | Opcode::PopPrivateEnvironment | Opcode::ImportCall + | Opcode::GetReturnValue + | Opcode::SetReturnValue | Opcode::Nop => String::new(), Opcode::Reserved1 | Opcode::Reserved2 @@ -620,7 +614,13 @@ impl CodeBlock { | Opcode::Reserved47 | Opcode::Reserved48 | Opcode::Reserved49 - | Opcode::Reserved50 => unreachable!("Reserved opcodes are unrechable"), + | Opcode::Reserved50 + | Opcode::Reserved51 + | Opcode::Reserved52 + | Opcode::Reserved53 + | Opcode::Reserved54 + | Opcode::Reserved55 + | Opcode::Reserved56 => unreachable!("Reserved opcodes are unrechable"), } } } diff --git a/boa_engine/src/vm/flowgraph/mod.rs b/boa_engine/src/vm/flowgraph/mod.rs index 5e211a969b..6e632d66bb 100644 --- a/boa_engine/src/vm/flowgraph/mod.rs +++ b/boa_engine/src/vm/flowgraph/mod.rs @@ -154,7 +154,7 @@ impl CodeBlock { graph.add_node(previous_pc, NodeShape::None, label.into(), Color::Red); graph.add_edge(previous_pc, pc, None, Color::None, EdgeStyle::Line); } - Opcode::Break | Opcode::BreakLabel => { + Opcode::Break => { let jump_operand = self.read::(pc); pc += size_of::(); let target_operand = self.read::(pc); @@ -278,8 +278,7 @@ impl CodeBlock { EdgeStyle::Line, ); } - Opcode::CatchStart - | Opcode::CallEval + Opcode::CallEval | Opcode::Call | Opcode::New | Opcode::SuperCall @@ -518,7 +517,6 @@ impl CodeBlock { graph.add_edge(previous_pc, pc, None, Color::None, EdgeStyle::Line); } Opcode::Pop - | Opcode::PopIfThrown | Opcode::Dup | Opcode::Swap | Opcode::PushZero @@ -583,14 +581,11 @@ impl CodeBlock { | Opcode::DeleteSuperThrow | Opcode::ToPropertyKey | Opcode::ToBoolean - | Opcode::CatchEnd - | Opcode::CatchEnd2 | Opcode::FinallyEnd | Opcode::This | Opcode::Super | Opcode::LoopEnd | Opcode::LoopContinue - | Opcode::LoopUpdateReturnValue | Opcode::LabelledEnd | Opcode::CreateForInIterator | Opcode::GetIterator @@ -612,8 +607,6 @@ impl CodeBlock { | Opcode::PushElisionToArray | Opcode::PushIteratorToArray | Opcode::PushNewArray - | Opcode::PopOnReturnAdd - | Opcode::PopOnReturnSub | Opcode::GeneratorYield | Opcode::AsyncGeneratorYield | Opcode::GeneratorNext @@ -634,6 +627,8 @@ impl CodeBlock { | Opcode::PopPrivateEnvironment | Opcode::ImportCall | Opcode::GeneratorSetReturn + | Opcode::GetReturnValue + | Opcode::SetReturnValue | Opcode::Nop => { graph.add_node(previous_pc, NodeShape::None, label.into(), Color::None); graph.add_edge(previous_pc, pc, None, Color::None, EdgeStyle::Line); @@ -709,7 +704,13 @@ impl CodeBlock { | Opcode::Reserved47 | Opcode::Reserved48 | Opcode::Reserved49 - | Opcode::Reserved50 => unreachable!("Reserved opcodes are unrechable"), + | Opcode::Reserved50 + | Opcode::Reserved51 + | Opcode::Reserved52 + | Opcode::Reserved53 + | Opcode::Reserved54 + | Opcode::Reserved55 + | Opcode::Reserved56 => unreachable!("Reserved opcodes are unrechable"), } } diff --git a/boa_engine/src/vm/mod.rs b/boa_engine/src/vm/mod.rs index f9bca1d87a..27f81aaced 100644 --- a/boa_engine/src/vm/mod.rs +++ b/boa_engine/src/vm/mod.rs @@ -359,23 +359,15 @@ impl Context<'_> { println!("\n"); } - // Determine the execution result - let execution_result = if execution_completion == CompletionType::Throw { - self.vm.frame_mut().abrupt_completion = None; - self.vm.stack.truncate(self.vm.frame().fp as usize); - JsValue::undefined() - } else if execution_completion == CompletionType::Return { + if execution_completion == CompletionType::Throw + || execution_completion == CompletionType::Return + { self.vm.frame_mut().abrupt_completion = None; - let result = self.vm.pop(); - self.vm.stack.truncate(self.vm.frame().fp as usize); - result - } else if self.vm.stack.len() <= self.vm.frame().fp as usize { - JsValue::undefined() - } else { - let result = self.vm.pop(); - self.vm.stack.truncate(self.vm.frame().fp as usize); - result - }; + } + self.vm.stack.truncate(self.vm.frame().fp as usize); + + // Determine the execution result + let execution_result = self.vm.frame_mut().return_value.clone(); if let Some(promise) = promise_capability { match execution_completion { diff --git a/boa_engine/src/vm/opcode/control_flow/break.rs b/boa_engine/src/vm/opcode/control_flow/break.rs index ce1476383f..f940fb77bb 100644 --- a/boa_engine/src/vm/opcode/control_flow/break.rs +++ b/boa_engine/src/vm/opcode/control_flow/break.rs @@ -1,6 +1,6 @@ use crate::{ vm::{call_frame::AbruptCompletionRecord, opcode::Operation, CompletionType}, - Context, JsResult, JsValue, + Context, JsResult, }; /// `Break` implements the Opcode Operation for `Opcode::Break` @@ -17,14 +17,11 @@ impl Operation for Break { let jump_address = context.vm.read::(); let target_address = context.vm.read::(); - let value = context.vm.stack.pop().unwrap_or(JsValue::undefined()); - // 1. Iterate through Env stack looking for exit address. let mut envs_to_pop = 0; - let mut set_loop_result = false; let mut found_target = false; for i in (0..context.vm.frame().env_stack.len()).rev() { - if found_target && set_loop_result { + if found_target { break; } @@ -32,16 +29,10 @@ impl Operation for Break { break; }; - if found_target { - set_loop_result = env_entry.set_loop_return_value(value.clone()); - continue; - } - if jump_address == env_entry.exit_address() || (env_entry.is_finally_env() && jump_address == env_entry.start_address()) { found_target = true; - set_loop_result = env_entry.set_loop_return_value(value.clone()); continue; } @@ -61,50 +52,3 @@ impl Operation for Break { Ok(CompletionType::Normal) } } - -/// `BreakLabel` implements the Opcode Operation for `Opcode::BreakLabel` -/// -/// Operation: -/// - Pop required environments and jump to address. -pub(crate) struct BreakLabel; - -impl Operation for BreakLabel { - const NAME: &'static str = "BreakLabel"; - const INSTRUCTION: &'static str = "INST - BreakLabel"; - - fn execute(context: &mut Context<'_>) -> JsResult { - let jump_address = context.vm.read::(); - let target_address = context.vm.read::(); - - let value = context.vm.stack.pop().unwrap_or(JsValue::undefined()); - context.vm.push(value); - - // 1. Iterate through Env stack looking for exit address. - let mut envs_to_pop = 0; - for i in (0..context.vm.frame().env_stack.len()).rev() { - let Some(env_entry) = context.vm.frame_mut().env_stack.get_mut(i) else { - break; - }; - - if jump_address == env_entry.exit_address() - || (env_entry.is_finally_env() && jump_address == env_entry.start_address()) - { - break; - } - - envs_to_pop += env_entry.env_num(); - context.vm.frame_mut().env_stack.pop(); - } - - let env_truncation_len = context.vm.environments.len().saturating_sub(envs_to_pop); - context.vm.environments.truncate(env_truncation_len); - - // 2. Register target address in AbruptCompletionRecord. - let new_record = AbruptCompletionRecord::new_break().with_initial_target(target_address); - context.vm.frame_mut().abrupt_completion = Some(new_record); - - // 3. Set program counter and finally return fields. - context.vm.frame_mut().pc = jump_address; - Ok(CompletionType::Normal) - } -} diff --git a/boa_engine/src/vm/opcode/control_flow/catch.rs b/boa_engine/src/vm/opcode/control_flow/catch.rs deleted file mode 100644 index 3d7c693a54..0000000000 --- a/boa_engine/src/vm/opcode/control_flow/catch.rs +++ /dev/null @@ -1,91 +0,0 @@ -use crate::{ - vm::{call_frame::EnvStackEntry, opcode::Operation, CompletionType}, - Context, JsResult, -}; - -/// `CatchStart` implements the Opcode Operation for `Opcode::CatchStart` -/// -/// Operation: -/// - Start of a catch block. -#[derive(Debug, Clone, Copy)] -pub(crate) struct CatchStart; - -impl Operation for CatchStart { - const NAME: &'static str = "CatchStart"; - const INSTRUCTION: &'static str = "INST - CatchStart"; - - fn execute(context: &mut Context<'_>) -> JsResult { - let start = context.vm.frame().pc - 1; - let finally = context.vm.read::(); - - context - .vm - .frame_mut() - .env_stack - .push(EnvStackEntry::new(start, finally - 1).with_catch_flag()); - - context.vm.frame_mut().abrupt_completion = None; - Ok(CompletionType::Normal) - } -} - -/// `CatchEnd` implements the Opcode Operation for `Opcode::CatchEnd` -/// -/// Operation: -/// - End of a catch block. -#[derive(Debug, Clone, Copy)] -pub(crate) struct CatchEnd; - -impl Operation for CatchEnd { - const NAME: &'static str = "CatchEnd"; - const INSTRUCTION: &'static str = "INST - CatchEnd"; - - fn execute(context: &mut Context<'_>) -> JsResult { - let mut envs_to_pop = 0_usize; - while let Some(env_entry) = context.vm.frame_mut().env_stack.pop() { - envs_to_pop += env_entry.env_num(); - - if env_entry.is_catch_env() { - break; - } - } - - let env_truncation_len = context.vm.environments.len().saturating_sub(envs_to_pop); - context.vm.environments.truncate(env_truncation_len); - - Ok(CompletionType::Normal) - } -} - -/// `CatchEnd2` implements the Opcode Operation for `Opcode::CatchEnd2` -/// -/// Operation: -/// - End of a catch block -#[derive(Debug, Clone, Copy)] -pub(crate) struct CatchEnd2; - -impl Operation for CatchEnd2 { - const NAME: &'static str = "CatchEnd2"; - const INSTRUCTION: &'static str = "INST - CatchEnd2"; - - fn execute(context: &mut Context<'_>) -> JsResult { - if let Some(catch_entry) = context - .vm - .frame() - .env_stack - .last() - .filter(|entry| entry.is_catch_env()) - { - let env_truncation_len = context - .vm - .environments - .len() - .saturating_sub(catch_entry.env_num()); - context.vm.environments.truncate(env_truncation_len); - - context.vm.frame_mut().env_stack.pop(); - } - - Ok(CompletionType::Normal) - } -} diff --git a/boa_engine/src/vm/opcode/control_flow/continue.rs b/boa_engine/src/vm/opcode/control_flow/continue.rs index e751a3ec18..be97fedc23 100644 --- a/boa_engine/src/vm/opcode/control_flow/continue.rs +++ b/boa_engine/src/vm/opcode/control_flow/continue.rs @@ -1,6 +1,6 @@ use crate::{ vm::{call_frame::AbruptCompletionRecord, opcode::Operation, CompletionType}, - Context, JsResult, JsValue, + Context, JsResult, }; /// `Continue` implements the Opcode Operation for `Opcode::Continue` @@ -21,14 +21,11 @@ impl Operation for Continue { let jump_address = context.vm.read::(); let target_address = context.vm.read::(); - let value = context.vm.stack.pop().unwrap_or(JsValue::undefined()); - // 1. Iterate through Env stack looking for exit address. let mut envs_to_pop = 0; - let mut set_loop_result = false; let mut found_target = false; for i in (0..context.vm.frame().env_stack.len()).rev() { - if found_target && set_loop_result { + if found_target { break; } @@ -36,11 +33,6 @@ impl Operation for Continue { break; }; - if found_target { - set_loop_result = env_entry.set_loop_return_value(value.clone()); - continue; - } - // We check two conditions here where continue actually jumps to a higher address. // 1. When we have reached a finally env that matches the jump address we are moving to. // 2. When there is no finally, and we have reached the continue location. @@ -48,7 +40,6 @@ impl Operation for Continue { || (jump_address == target_address && jump_address == env_entry.start_address()) { found_target = true; - set_loop_result = env_entry.set_loop_return_value(value.clone()); continue; } @@ -56,7 +47,6 @@ impl Operation for Continue { // The below check determines whether we have continued from inside of a finally block. if jump_address > target_address && jump_address == env_entry.exit_address() { found_target = true; - set_loop_result = env_entry.set_loop_return_value(value.clone()); context.vm.frame_mut().env_stack.pop(); continue; } diff --git a/boa_engine/src/vm/opcode/control_flow/mod.rs b/boa_engine/src/vm/opcode/control_flow/mod.rs index 4f6429c852..fa3a4d7637 100644 --- a/boa_engine/src/vm/opcode/control_flow/mod.rs +++ b/boa_engine/src/vm/opcode/control_flow/mod.rs @@ -1,5 +1,4 @@ pub(crate) mod r#break; -pub(crate) mod catch; pub(crate) mod r#continue; pub(crate) mod finally; pub(crate) mod labelled; @@ -7,7 +6,6 @@ pub(crate) mod r#return; pub(crate) mod throw; pub(crate) mod r#try; -pub(crate) use catch::*; pub(crate) use finally::*; pub(crate) use labelled::*; pub(crate) use r#break::*; diff --git a/boa_engine/src/vm/opcode/control_flow/return.rs b/boa_engine/src/vm/opcode/control_flow/return.rs index 4738f2de96..b44a7f0558 100644 --- a/boa_engine/src/vm/opcode/control_flow/return.rs +++ b/boa_engine/src/vm/opcode/control_flow/return.rs @@ -50,3 +50,39 @@ impl Operation for Return { Ok(CompletionType::Return) } } + +/// `GetReturnValue` implements the Opcode Operation for `Opcode::GetReturnValue` +/// +/// Operation: +/// - Gets the return value of a function. +#[derive(Debug, Clone, Copy)] +pub(crate) struct GetReturnValue; + +impl Operation for GetReturnValue { + const NAME: &'static str = "GetReturnValue"; + const INSTRUCTION: &'static str = "INST - GetReturnValue"; + + fn execute(context: &mut Context<'_>) -> JsResult { + let value = context.vm.frame().return_value.clone(); + context.vm.push(value); + Ok(CompletionType::Normal) + } +} + +/// `SetReturnValue` implements the Opcode Operation for `Opcode::SetReturnValue` +/// +/// Operation: +/// - Sets the return value of a function. +#[derive(Debug, Clone, Copy)] +pub(crate) struct SetReturnValue; + +impl Operation for SetReturnValue { + const NAME: &'static str = "SetReturnValue"; + const INSTRUCTION: &'static str = "INST - SetReturnValue"; + + fn execute(context: &mut Context<'_>) -> JsResult { + let value = context.vm.pop(); + context.vm.frame_mut().return_value = value; + Ok(CompletionType::Normal) + } +} diff --git a/boa_engine/src/vm/opcode/control_flow/throw.rs b/boa_engine/src/vm/opcode/control_flow/throw.rs index fd466104d2..0a254d1869 100644 --- a/boa_engine/src/vm/opcode/control_flow/throw.rs +++ b/boa_engine/src/vm/opcode/control_flow/throw.rs @@ -29,8 +29,9 @@ impl Operation for Throw { let current_address = context.vm.frame().pc; let mut envs = context.vm.frame().env_stack.iter(); + // Handle catch block if let Some(idx) = - envs.rposition(|env| env.is_try_env() && env.start_address() < env.exit_address()) + envs.rposition(|env| env.is_try_env() && env.start_address() != env.exit_address()) { let active_iterator = context.vm.frame().env_stack[..idx] .iter() @@ -52,6 +53,10 @@ impl Operation for Throw { context.vm.err.take(); } + let try_env = &context.vm.frame().env_stack[idx]; + let try_env_frame_pointer = try_env.try_env_frame_pointer(); + context.vm.stack.truncate(try_env_frame_pointer as usize); + let catch_target = context.vm.frame().env_stack[idx].start_address(); let mut env_to_pop = 0; @@ -90,11 +95,6 @@ impl Operation for Throw { context.vm.frame_mut().pc = target_address; }; - for _ in 0..context.vm.frame().pop_on_return { - context.vm.pop(); - } - - context.vm.frame_mut().pop_on_return = 0; let record = AbruptCompletionRecord::new_throw().with_initial_target(catch_target); context.vm.frame_mut().abrupt_completion = Some(record); let err = error.to_opaque(context); @@ -147,6 +147,7 @@ impl Operation for Throw { .frame_mut() .iterators .split_off(active_iterator as usize + 1); + for iterator in inactive { if !iterator.done() { drop(iterator.close(Ok(JsValue::undefined()), context)); @@ -158,13 +159,8 @@ impl Operation for Throw { let env_truncation_len = context.vm.environments.len().saturating_sub(env_to_pop); context.vm.environments.truncate(env_truncation_len); - let previous_stack_size = context - .vm - .stack - .len() - .saturating_sub(context.vm.frame().pop_on_return as usize); - context.vm.stack.truncate(previous_stack_size); - context.vm.frame_mut().pop_on_return = 0; + // NOTE: There is could be leftover stack values, but this is fine, + // since we truncate to the call frams's frame pointer on return. context.vm.frame_mut().pc = address; let err = error.to_opaque(context); diff --git a/boa_engine/src/vm/opcode/control_flow/try.rs b/boa_engine/src/vm/opcode/control_flow/try.rs index 8e74378f67..b79b6dc10c 100644 --- a/boa_engine/src/vm/opcode/control_flow/try.rs +++ b/boa_engine/src/vm/opcode/control_flow/try.rs @@ -27,11 +27,13 @@ impl Operation for TryStart { ); } + let fp = context.vm.stack.len() as u32; + context .vm .frame_mut() .env_stack - .push(EnvStackEntry::new(catch, finally).with_try_flag()); + .push(EnvStackEntry::new(catch, finally).with_try_flag(fp)); Ok(CompletionType::Normal) } diff --git a/boa_engine/src/vm/opcode/generator/mod.rs b/boa_engine/src/vm/opcode/generator/mod.rs index d7818946d1..894a183c2d 100644 --- a/boa_engine/src/vm/opcode/generator/mod.rs +++ b/boa_engine/src/vm/opcode/generator/mod.rs @@ -13,6 +13,8 @@ use crate::{ pub(crate) use yield_stm::*; +use super::SetReturnValue; + /// `GeneratorNext` implements the Opcode Operation for `Opcode::GeneratorNext` /// /// Operation: @@ -28,7 +30,10 @@ impl Operation for GeneratorNext { match context.vm.frame().generator_resume_kind { GeneratorResumeKind::Normal => Ok(CompletionType::Normal), GeneratorResumeKind::Throw => Err(JsError::from_opaque(context.vm.pop())), - GeneratorResumeKind::Return => Return::execute(context), + GeneratorResumeKind::Return => { + SetReturnValue::execute(context)?; + Return::execute(context) + } } } } diff --git a/boa_engine/src/vm/opcode/iteration/loop_ops.rs b/boa_engine/src/vm/opcode/iteration/loop_ops.rs index dee19bac82..ef69491300 100644 --- a/boa_engine/src/vm/opcode/iteration/loop_ops.rs +++ b/boa_engine/src/vm/opcode/iteration/loop_ops.rs @@ -107,9 +107,7 @@ impl Operation for LoopEnd { while let Some(env_entry) = context.vm.frame_mut().env_stack.pop() { envs_to_pop += env_entry.env_num(); - if let Some(value) = env_entry.loop_env_value() { - context.vm.push(value.clone()); - + if env_entry.is_loop_env() { break; } } @@ -120,27 +118,3 @@ impl Operation for LoopEnd { Ok(CompletionType::Normal) } } - -/// `LoopUpdateReturnValue` implements the Opcode Operation for `Opcode::LoopUpdateReturnValue` -/// -/// Operation: -/// - Update the return value of a loop. -#[derive(Debug, Clone, Copy)] -pub(crate) struct LoopUpdateReturnValue; - -impl Operation for LoopUpdateReturnValue { - const NAME: &'static str = "LoopUpdateReturnValue"; - const INSTRUCTION: &'static str = "INST - LoopUpdateReturnValue"; - - fn execute(context: &mut Context<'_>) -> JsResult { - let value = context.vm.pop(); - context - .vm - .frame_mut() - .env_stack - .last_mut() - .expect("loop environment must be present") - .set_loop_return_value(value); - Ok(CompletionType::Normal) - } -} diff --git a/boa_engine/src/vm/opcode/mod.rs b/boa_engine/src/vm/opcode/mod.rs index 0a8e0a8f71..44622af89e 100644 --- a/boa_engine/src/vm/opcode/mod.rs +++ b/boa_engine/src/vm/opcode/mod.rs @@ -183,13 +183,6 @@ generate_impl! { /// Stack: value **=>** Pop = 0, - /// Pop the top value from the stack if the last try block has thrown a value. - /// - /// Operands: - /// - /// Stack: value **=>** - PopIfThrown, - /// Push a copy of the top value on the stack. /// /// Operands: @@ -1152,27 +1145,6 @@ generate_impl! { /// Stack: **=>** TryEnd, - /// Start of a catch block. - /// - /// Operands: finally_address: `u32` - /// - /// Stack: **=>** - CatchStart, - - /// End of a catch block. - /// - /// Operands: - /// - /// Stack: **=>** - CatchEnd, - - /// End of a catch block. - /// - /// Operands: - /// - /// Stack: **=>** - CatchEnd2, - /// Start of a finally block. /// /// Operands: @@ -1194,13 +1166,6 @@ generate_impl! { /// Stack: loop_return_value **=>** Break, - /// Jumps to a label target location and pops the environments involved. - /// - /// Operands: Jump Address: u32, Target address: u32 - /// - /// Stack: **=>** - BreakLabel, - /// Sets the `AbruptCompletionRecord` for a delayed continue /// /// Operands: Jump Address: u32, Target address: u32, @@ -1370,6 +1335,20 @@ generate_impl! { /// Stack: **=>** Return, + /// Get return value of a function. + /// + /// Operands: + /// + /// Stack: **=>** value + GetReturnValue, + + /// Set return value of a function. + /// + /// Operands: + /// + /// Stack: value **=>** + SetReturnValue, + /// Push a declarative environment. /// /// Operands: compile_environments_index: `u32` @@ -1421,13 +1400,6 @@ generate_impl! { /// Stack: **=>** value LoopEnd, - /// Update the return value of a loop. - /// - /// Operands: - /// - /// Stack: loop_return_value **=>** - LoopUpdateReturnValue, - /// Push labelled start marker. /// /// Operands: Exit Address: u32, @@ -1590,20 +1562,6 @@ generate_impl! { /// Stack: `argument_1` .. `argument_n` **=>** RestParameterPop, - /// Add one to the pop on return count. - /// - /// Operands: - /// - /// Stack: **=>** - PopOnReturnAdd, - - /// Subtract one from the pop on return count. - /// - /// Operands: - /// - /// Stack: **=>** - PopOnReturnSub, - /// Yields from the current generator execution. /// /// Operands: @@ -1826,6 +1784,18 @@ generate_impl! { Reserved49 => Reserved, /// Reserved [`Opcode`]. Reserved50 => Reserved, + /// Reserved [`Opcode`]. + Reserved51 => Reserved, + /// Reserved [`Opcode`]. + Reserved52 => Reserved, + /// Reserved [`Opcode`]. + Reserved53 => Reserved, + /// Reserved [`Opcode`]. + Reserved54 => Reserved, + /// Reserved [`Opcode`]. + Reserved55 => Reserved, + /// Reserved [`Opcode`]. + Reserved56 => Reserved, } } diff --git a/boa_engine/src/vm/opcode/pop/mod.rs b/boa_engine/src/vm/opcode/pop/mod.rs index e1fb43327c..ffadf54912 100644 --- a/boa_engine/src/vm/opcode/pop/mod.rs +++ b/boa_engine/src/vm/opcode/pop/mod.rs @@ -20,29 +20,6 @@ impl Operation for Pop { } } -/// `PopIfThrown` implements the Opcode Operation for `Opcode::PopIfThrown` -/// -/// Operation: -/// - Pop the top value from the stack if the last try block has thrown a value. -#[derive(Debug, Clone, Copy)] -pub(crate) struct PopIfThrown; - -impl Operation for PopIfThrown { - const NAME: &'static str = "PopIfThrown"; - const INSTRUCTION: &'static str = "INST - PopIfThrown"; - - fn execute(context: &mut Context<'_>) -> JsResult { - let frame = context.vm.frame(); - match frame.abrupt_completion { - Some(record) if record.is_throw() => { - context.vm.pop(); - } - _ => {} - }; - Ok(CompletionType::Normal) - } -} - /// `PopEnvironment` implements the Opcode Operation for `Opcode::PopEnvironment` /// /// Operation: @@ -60,37 +37,3 @@ impl Operation for PopEnvironment { Ok(CompletionType::Normal) } } - -/// `PopReturnAdd` implements the Opcode Operation for `Opcode::PopReturnAdd` -/// -/// Operation: -/// - Add one to the pop on return count. -#[derive(Debug, Clone, Copy)] -pub(crate) struct PopOnReturnAdd; - -impl Operation for PopOnReturnAdd { - const NAME: &'static str = "PopOnReturnAdd"; - const INSTRUCTION: &'static str = "INST - PopOnReturnAdd"; - - fn execute(context: &mut Context<'_>) -> JsResult { - context.vm.frame_mut().pop_on_return += 1; - Ok(CompletionType::Normal) - } -} - -/// `PopOnReturnSub` implements the Opcode Operation for `Opcode::PopOnReturnSub` -/// -/// Operation: -/// - Subtract one from the pop on return count. -#[derive(Debug, Clone, Copy)] -pub(crate) struct PopOnReturnSub; - -impl Operation for PopOnReturnSub { - const NAME: &'static str = "PopOnReturnSub"; - const INSTRUCTION: &'static str = "INST - PopOnReturnSub"; - - fn execute(context: &mut Context<'_>) -> JsResult { - context.vm.frame_mut().pop_on_return -= 1; - Ok(CompletionType::Normal) - } -}