From 7605453cd78fed8215b13435b4895dfc23a600a8 Mon Sep 17 00:00:00 2001 From: raskad <32105367+raskad@users.noreply.github.com> Date: Mon, 8 May 2023 07:28:47 +0200 Subject: [PATCH] Add loop and switch return values (#2828) * Add loop and switch return values * Apply suggestions --- boa_ast/src/statement/mod.rs | 11 ++ boa_engine/src/bytecompiler/mod.rs | 15 ++- boa_engine/src/bytecompiler/statement/if.rs | 24 ++-- .../src/bytecompiler/statement/labelled.rs | 10 +- boa_engine/src/bytecompiler/statement/loop.rs | 114 +++++++++++++----- boa_engine/src/bytecompiler/statement/mod.rs | 14 +-- .../src/bytecompiler/statement/switch.rs | 10 +- boa_engine/src/bytecompiler/statement/try.rs | 22 +++- boa_engine/src/bytecompiler/statement/with.rs | 13 +- boa_engine/src/vm/call_frame/env_stack.rs | 80 +++++++++--- boa_engine/src/vm/call_frame/mod.rs | 1 - boa_engine/src/vm/code_block.rs | 3 +- boa_engine/src/vm/flowgraph/mod.rs | 4 +- .../src/vm/opcode/control_flow/break.rs | 29 ++++- .../src/vm/opcode/control_flow/continue.rs | 29 ++++- .../src/vm/opcode/iteration/loop_ops.rs | 112 ++++++++--------- boa_engine/src/vm/opcode/mod.rs | 17 ++- 17 files changed, 365 insertions(+), 143 deletions(-) diff --git a/boa_ast/src/statement/mod.rs b/boa_ast/src/statement/mod.rs index d6c920c492..f810c285ed 100644 --- a/boa_ast/src/statement/mod.rs +++ b/boa_ast/src/statement/mod.rs @@ -162,6 +162,17 @@ impl Statement { _ => false, } } + + /// Returns `true` if the statement returns a value. + #[inline] + #[must_use] + pub const fn returns_value(&self) -> bool { + match self { + Self::Block(block) if block.statement_list().statements().is_empty() => false, + Self::Empty | Self::Var(_) | Self::Break(_) | Self::Continue(_) => false, + _ => true, + } + } } impl ToIndentedString for Statement { diff --git a/boa_engine/src/bytecompiler/mod.rs b/boa_engine/src/bytecompiler/mod.rs index 8dea9078ea..bf95902c5c 100644 --- a/boa_engine/src/bytecompiler/mod.rs +++ b/boa_engine/src/bytecompiler/mod.rs @@ -745,8 +745,17 @@ 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 { - let expr_index = list + let list_until_loop_exit: Vec<_> = list .statements() + .iter() + .take_while(|item| { + !matches!( + item, + StatementListItem::Statement(Statement::Break(_) | Statement::Continue(_)) + ) + }) + .collect(); + let expr_index = list_until_loop_exit .iter() .rev() .skip_while(|item| { @@ -758,6 +767,10 @@ impl<'ctx, 'host> ByteCompiler<'ctx, 'host> { }) .count(); + if expr_index == 0 && !list.statements().is_empty() { + self.emit_opcode(Opcode::PushUndefined); + } + for (i, item) in list.statements().iter().enumerate() { self.compile_stmt_list_item(item, i + 1 == expr_index, block); } diff --git a/boa_engine/src/bytecompiler/statement/if.rs b/boa_engine/src/bytecompiler/statement/if.rs index ebeab44fe9..a0183628b1 100644 --- a/boa_engine/src/bytecompiler/statement/if.rs +++ b/boa_engine/src/bytecompiler/statement/if.rs @@ -1,4 +1,4 @@ -use crate::bytecompiler::ByteCompiler; +use crate::{bytecompiler::ByteCompiler, vm::Opcode}; use boa_ast::statement::If; impl ByteCompiler<'_, '_> { @@ -6,18 +6,28 @@ impl ByteCompiler<'_, '_> { self.compile_expr(node.cond(), true); let jelse = self.jump_if_false(); - self.compile_stmt(node.body(), use_expr); + if !node.body().returns_value() { + self.emit_opcode(Opcode::PushUndefined); + } + self.compile_stmt(node.body(), true); + let exit = self.jump(); + self.patch_jump(jelse); match node.else_node() { None => { - self.patch_jump(jelse); + self.emit_opcode(Opcode::PushUndefined); } Some(else_body) => { - let exit = self.jump(); - self.patch_jump(jelse); - self.compile_stmt(else_body, use_expr); - self.patch_jump(exit); + if !else_body.returns_value() { + self.emit_opcode(Opcode::PushUndefined); + } + self.compile_stmt(else_body, 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 a7c3565542..87a0eec3ca 100644 --- a/boa_engine/src/bytecompiler/statement/labelled.rs +++ b/boa_engine/src/bytecompiler/statement/labelled.rs @@ -17,19 +17,19 @@ impl ByteCompiler<'_, '_> { match labelled.item() { LabelledItem::Statement(stmt) => match stmt { Statement::ForLoop(for_loop) => { - self.compile_for_loop(for_loop, Some(labelled.label())); + self.compile_for_loop(for_loop, Some(labelled.label()), use_expr); } Statement::ForInLoop(for_in_loop) => { - self.compile_for_in_loop(for_in_loop, Some(labelled.label())); + self.compile_for_in_loop(for_in_loop, Some(labelled.label()), use_expr); } Statement::ForOfLoop(for_of_loop) => { - self.compile_for_of_loop(for_of_loop, Some(labelled.label())); + self.compile_for_of_loop(for_of_loop, Some(labelled.label()), use_expr); } Statement::WhileLoop(while_loop) => { - self.compile_while_loop(while_loop, Some(labelled.label())); + self.compile_while_loop(while_loop, Some(labelled.label()), use_expr); } Statement::DoWhileLoop(do_while_loop) => { - self.compile_do_while_loop(do_while_loop, Some(labelled.label())); + self.compile_do_while_loop(do_while_loop, Some(labelled.label()), use_expr); } stmt => self.compile_stmt(stmt, use_expr), }, diff --git a/boa_engine/src/bytecompiler/statement/loop.rs b/boa_engine/src/bytecompiler/statement/loop.rs index bee7402f32..8167ae3807 100644 --- a/boa_engine/src/bytecompiler/statement/loop.rs +++ b/boa_engine/src/bytecompiler/statement/loop.rs @@ -14,7 +14,12 @@ use crate::{ }; impl ByteCompiler<'_, '_> { - pub(crate) fn compile_for_loop(&mut self, for_loop: &ForLoop, label: Option) { + pub(crate) fn compile_for_loop( + &mut self, + for_loop: &ForLoop, + label: Option, + use_expr: bool, + ) { self.push_compile_environment(false); let push_env = self.emit_opcode_with_two_operands(Opcode::PushDeclarativeEnvironment); self.push_empty_loop_jump_control(); @@ -54,11 +59,8 @@ impl ByteCompiler<'_, '_> { .expect("jump_control must exist as it was just pushed") .set_start_address(start_address); - let (continue_start, continue_exit) = - self.emit_opcode_with_two_operands(Opcode::LoopContinue); - + self.emit_opcode(Opcode::LoopContinue); self.patch_jump_with_target(loop_start, start_address); - self.patch_jump_with_target(continue_start, start_address); if let Some(final_expr) = for_loop.final_expr() { self.compile_expr(final_expr, false); @@ -73,7 +75,11 @@ impl ByteCompiler<'_, '_> { } let exit = self.jump_if_false(); - self.compile_stmt(for_loop.body(), false); + if !for_loop.body().returns_value() { + self.emit_opcode(Opcode::PushUndefined); + } + self.compile_stmt(for_loop.body(), true); + self.emit_opcode(Opcode::LoopUpdateReturnValue); self.emit(Opcode::Jump, &[start_address]); @@ -83,13 +89,21 @@ impl ByteCompiler<'_, '_> { self.patch_jump(exit); self.patch_jump(loop_exit); - self.patch_jump(continue_exit); self.pop_loop_control_info(); self.emit_opcode(Opcode::LoopEnd); + + if !use_expr { + self.emit_opcode(Opcode::Pop); + } self.emit_opcode(Opcode::PopEnvironment); } - pub(crate) fn compile_for_in_loop(&mut self, for_in_loop: &ForInLoop, label: Option) { + pub(crate) fn compile_for_in_loop( + &mut self, + for_in_loop: &ForInLoop, + label: Option, + use_expr: bool, + ) { // Handle https://tc39.es/ecma262/#prod-annexB-ForInOfStatement if let IterableLoopInitializer::Var(var) = for_in_loop.initializer() { if let Binding::Identifier(ident) = var.binding() { @@ -128,9 +142,7 @@ impl ByteCompiler<'_, '_> { let (loop_start, exit_label) = self.emit_opcode_with_two_operands(Opcode::LoopStart); let start_address = self.next_opcode_location(); self.push_loop_control_info_for_of_in_loop(label, start_address); - let (continue_label, cont_exit_label) = - self.emit_opcode_with_two_operands(Opcode::LoopContinue); - self.patch_jump_with_target(continue_label, start_address); + self.emit_opcode(Opcode::LoopContinue); self.patch_jump_with_target(loop_start, start_address); self.emit_opcode(Opcode::Pop); // pop the `done` value. @@ -192,7 +204,11 @@ impl ByteCompiler<'_, '_> { } } - self.compile_stmt(for_in_loop.body(), false); + if !for_in_loop.body().returns_value() { + self.emit_opcode(Opcode::PushUndefined); + } + self.compile_stmt(for_in_loop.body(), true); + self.emit_opcode(Opcode::LoopUpdateReturnValue); if let Some(iteration_environment) = iteration_environment { let env_info = self.pop_compile_environment(); @@ -205,17 +221,30 @@ impl ByteCompiler<'_, '_> { self.patch_jump(exit); self.patch_jump(exit_label); - self.patch_jump(cont_exit_label); self.pop_loop_control_info(); self.emit_opcode(Opcode::LoopEnd); + self.emit_opcode(Opcode::RotateRight); + self.emit_u8(4); self.emit_opcode(Opcode::Pop); self.emit_opcode(Opcode::Pop); self.emit_opcode(Opcode::Pop); + let skip_early_exit = self.jump(); 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(&mut self, for_of_loop: &ForOfLoop, label: Option) { + pub(crate) fn compile_for_of_loop( + &mut self, + for_of_loop: &ForOfLoop, + label: Option, + use_expr: bool, + ) { let initializer_bound_names = match for_of_loop.initializer() { IterableLoopInitializer::Let(declaration) | IterableLoopInitializer::Const(declaration) => bound_names(declaration), @@ -248,10 +277,8 @@ impl ByteCompiler<'_, '_> { let (loop_start, loop_exit) = self.emit_opcode_with_two_operands(Opcode::LoopStart); let start_address = self.next_opcode_location(); self.push_loop_control_info_for_of_in_loop(label, start_address); - let (cont_label, cont_exit_label) = - self.emit_opcode_with_two_operands(Opcode::LoopContinue); + self.emit_opcode(Opcode::LoopContinue); self.patch_jump_with_target(loop_start, start_address); - self.patch_jump_with_target(cont_label, start_address); self.emit_opcode(Opcode::Pop); // pop the `done` value. self.emit_opcode(Opcode::IteratorNext); @@ -322,7 +349,11 @@ impl ByteCompiler<'_, '_> { } } - self.compile_stmt(for_of_loop.body(), false); + if !for_of_loop.body().returns_value() { + self.emit_opcode(Opcode::PushUndefined); + } + self.compile_stmt(for_of_loop.body(), true); + self.emit_opcode(Opcode::LoopUpdateReturnValue); if let Some(iteration_environment) = iteration_environment { let env_info = self.pop_compile_environment(); @@ -335,62 +366,83 @@ impl ByteCompiler<'_, '_> { self.patch_jump(exit); self.patch_jump(loop_exit); - self.patch_jump(cont_exit_label); self.pop_loop_control_info(); self.emit_opcode(Opcode::LoopEnd); + self.emit_opcode(Opcode::RotateRight); + self.emit_u8(4); self.iterator_close(for_of_loop.r#await()); + if !use_expr { + self.emit_opcode(Opcode::Pop); + } } - pub(crate) fn compile_while_loop(&mut self, while_loop: &WhileLoop, label: Option) { + pub(crate) fn compile_while_loop( + &mut self, + while_loop: &WhileLoop, + label: Option, + use_expr: bool, + ) { let (loop_start, loop_exit) = self.emit_opcode_with_two_operands(Opcode::LoopStart); let start_address = self.next_opcode_location(); - let (continue_start, continue_exit) = - self.emit_opcode_with_two_operands(Opcode::LoopContinue); - + self.emit_opcode(Opcode::LoopContinue); self.push_loop_control_info(label, start_address); self.patch_jump_with_target(loop_start, start_address); - self.patch_jump_with_target(continue_start, start_address); self.compile_expr(while_loop.condition(), true); let exit = self.jump_if_false(); - self.compile_stmt(while_loop.body(), false); + + if !while_loop.body().returns_value() { + self.emit_opcode(Opcode::PushUndefined); + } + self.compile_stmt(while_loop.body(), true); + self.emit_opcode(Opcode::LoopUpdateReturnValue); + self.emit(Opcode::Jump, &[start_address]); self.patch_jump(exit); self.patch_jump(loop_exit); - self.patch_jump(continue_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( &mut self, do_while_loop: &DoWhileLoop, label: Option, + use_expr: bool, ) { let (loop_start, loop_exit) = self.emit_opcode_with_two_operands(Opcode::LoopStart); let initial_label = self.jump(); let start_address = self.next_opcode_location(); + self.patch_jump_with_target(loop_start, start_address); self.push_loop_control_info(label, start_address); let condition_label_address = self.next_opcode_location(); - let (continue_start, continue_exit) = - self.emit_opcode_with_two_operands(Opcode::LoopContinue); - self.patch_jump_with_target(continue_start, start_address); + self.emit_opcode(Opcode::LoopContinue); self.compile_expr(do_while_loop.cond(), true); let exit = self.jump_if_false(); self.patch_jump(initial_label); - self.compile_stmt(do_while_loop.body(), false); + if !do_while_loop.body().returns_value() { + self.emit_opcode(Opcode::PushUndefined); + } + self.compile_stmt(do_while_loop.body(), true); + self.emit_opcode(Opcode::LoopUpdateReturnValue); + self.emit(Opcode::Jump, &[condition_label_address]); self.patch_jump(exit); self.patch_jump(loop_exit); - self.patch_jump(continue_exit); 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 75f5c3692d..43472fd86d 100644 --- a/boa_engine/src/bytecompiler/statement/mod.rs +++ b/boa_engine/src/bytecompiler/statement/mod.rs @@ -19,19 +19,19 @@ impl ByteCompiler<'_, '_> { Statement::Var(var) => self.compile_var_decl(var), Statement::If(node) => self.compile_if(node, use_expr), Statement::ForLoop(for_loop) => { - self.compile_for_loop(for_loop, None); + self.compile_for_loop(for_loop, None, use_expr); } Statement::ForInLoop(for_in_loop) => { - self.compile_for_in_loop(for_in_loop, None); + self.compile_for_in_loop(for_in_loop, None, use_expr); } Statement::ForOfLoop(for_of_loop) => { - self.compile_for_of_loop(for_of_loop, None); + self.compile_for_of_loop(for_of_loop, None, use_expr); } Statement::WhileLoop(while_loop) => { - self.compile_while_loop(while_loop, None); + self.compile_while_loop(while_loop, None, use_expr); } Statement::DoWhileLoop(do_while_loop) => { - self.compile_do_while_loop(do_while_loop, None); + self.compile_do_while_loop(do_while_loop, None, use_expr); } Statement::Block(block) => { self.compile_block(block, use_expr); @@ -46,7 +46,7 @@ impl ByteCompiler<'_, '_> { self.emit(Opcode::Throw, &[]); } Statement::Switch(switch) => { - self.compile_switch(switch); + self.compile_switch(switch, use_expr); } Statement::Return(ret) => { if let Some(expr) = ret.target() { @@ -58,7 +58,7 @@ impl ByteCompiler<'_, '_> { } Statement::Try(t) => self.compile_try(t, use_expr), Statement::Expression(expr) => self.compile_expr(expr, use_expr), - Statement::With(with) => self.compile_with(with), + 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 626da044d0..1e8566f71b 100644 --- a/boa_engine/src/bytecompiler/statement/switch.rs +++ b/boa_engine/src/bytecompiler/statement/switch.rs @@ -3,7 +3,7 @@ use boa_ast::statement::Switch; impl ByteCompiler<'_, '_> { /// Compile a [`Switch`] `boa_ast` node - pub(crate) fn compile_switch(&mut self, switch: &Switch) { + pub(crate) fn compile_switch(&mut self, switch: &Switch, use_expr: bool) { self.compile_expr(switch.val(), true); self.push_compile_environment(false); @@ -43,7 +43,10 @@ impl ByteCompiler<'_, '_> { label }; self.patch_jump(label); - self.compile_statement_list(case.body(), false, true); + self.compile_statement_list(case.body(), true, true); + if !case.body().statements().is_empty() { + self.emit_opcode(Opcode::LoopUpdateReturnValue); + } } if !default_label_set { @@ -53,6 +56,9 @@ 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_info = self.pop_compile_environment(); self.patch_jump_with_target(push_env.0, env_info.num_bindings as u32); diff --git a/boa_engine/src/bytecompiler/statement/try.rs b/boa_engine/src/bytecompiler/statement/try.rs index b05d2f3c70..51e2aa1205 100644 --- a/boa_engine/src/bytecompiler/statement/try.rs +++ b/boa_engine/src/bytecompiler/statement/try.rs @@ -20,7 +20,13 @@ impl ByteCompiler<'_, '_> { } self.push_try_control_info(t.finally().is_some(), try_start); - self.compile_block(t.block(), use_expr); + self.compile_block(t.block(), true); + if t.block().statement_list().statements().is_empty() { + self.emit_opcode(Opcode::PushUndefined); + } + if !use_expr { + self.emit_opcode(Opcode::Pop); + } self.emit_opcode(Opcode::TryEnd); @@ -76,12 +82,19 @@ impl ByteCompiler<'_, '_> { self.emit_opcode(Opcode::Pop); } - self.compile_block(catch.block(), use_expr); + self.compile_block(catch.block(), true); + if catch.block().statement_list().statements().is_empty() { + self.emit_opcode(Opcode::PushUndefined); + } + if !use_expr { + self.emit_opcode(Opcode::Pop); + } 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 parent_try.finally().is_some() { self.emit_opcode(Opcode::CatchEnd); } else { @@ -93,7 +106,10 @@ impl ByteCompiler<'_, '_> { } pub(crate) fn compile_finally_stmt(&mut self, finally: &Finally, finally_end_label: Label) { - self.compile_block(finally.block(), false); + self.compile_block(finally.block(), true); + if !finally.block().statement_list().statements().is_empty() { + self.emit_opcode(Opcode::Pop); + } 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 f48d851a36..dccbacca4f 100644 --- a/boa_engine/src/bytecompiler/statement/with.rs +++ b/boa_engine/src/bytecompiler/statement/with.rs @@ -3,12 +3,21 @@ use boa_ast::statement::With; impl ByteCompiler<'_, '_> { /// Compile a [`With`] `boa_ast` node - pub(crate) fn compile_with(&mut self, with: &With) { + pub(crate) fn compile_with(&mut self, with: &With, use_expr: bool) { self.compile_expr(with.expression(), true); self.push_compile_environment(false); self.emit_opcode(Opcode::PushObjectEnvironment); - self.compile_stmt(with.statement(), false); + + if !with.statement().returns_value() { + self.emit_opcode(Opcode::PushUndefined); + } + self.compile_stmt(with.statement(), 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 be34ea945a..fb71b23aa7 100644 --- a/boa_engine/src/vm/call_frame/env_stack.rs +++ b/boa_engine/src/vm/call_frame/env_stack.rs @@ -1,11 +1,17 @@ //! Module for implementing a `CallFrame`'s environment stacks -#[derive(Clone, Copy, Debug, PartialEq)] +use crate::JsValue; +use boa_gc::{Finalize, Trace}; + +#[derive(Clone, Debug, Finalize, Trace)] pub(crate) enum EnvEntryKind { Global, Loop { /// This is used to keep track of how many iterations a loop has done. iteration_count: u64, + + // This is the latest return value of the loop. + value: JsValue, }, Try, Catch, @@ -13,8 +19,22 @@ pub(crate) enum EnvEntryKind { Labelled, } -/// The `EnvStackEntry` tracks the environment count and relavant information for the current environment. -#[derive(Copy, Clone, Debug)] +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)] pub(crate) struct EnvStackEntry { start: u32, exit: u32, @@ -46,32 +66,35 @@ impl EnvStackEntry { } /// Returns calling `EnvStackEntry` with `kind` field of `Try`. - pub(crate) const fn with_try_flag(mut self) -> Self { + pub(crate) fn with_try_flag(mut self) -> Self { self.kind = EnvEntryKind::Try; self } /// Returns calling `EnvStackEntry` with `kind` field of `Loop`. /// And the loop iteration set to zero. - pub(crate) const fn with_loop_flag(mut self, iteration_count: u64) -> Self { - self.kind = EnvEntryKind::Loop { iteration_count }; + pub(crate) fn with_loop_flag(mut self, iteration_count: u64) -> Self { + self.kind = EnvEntryKind::Loop { + iteration_count, + value: JsValue::undefined(), + }; self } /// Returns calling `EnvStackEntry` with `kind` field of `Catch`. - pub(crate) const fn with_catch_flag(mut self) -> Self { + pub(crate) fn with_catch_flag(mut self) -> Self { self.kind = EnvEntryKind::Catch; self } /// Returns calling `EnvStackEntry` with `kind` field of `Finally`. - pub(crate) const fn with_finally_flag(mut self) -> Self { + pub(crate) fn with_finally_flag(mut self) -> Self { self.kind = EnvEntryKind::Finally; self } /// Returns calling `EnvStackEntry` with `kind` field of `Labelled`. - pub(crate) const fn with_labelled_flag(mut self) -> Self { + pub(crate) fn with_labelled_flag(mut self) -> Self { self.kind = EnvEntryKind::Labelled; self } @@ -98,14 +121,31 @@ impl EnvStackEntry { self.kind == EnvEntryKind::Global } - /// Returns true if an `EnvStackEntry` is a loop - pub(crate) const fn is_loop_env(&self) -> bool { - matches!(self.kind, EnvEntryKind::Loop { .. }) + /// Returns the loop iteration count if `EnvStackEntry` is a loop. + pub(crate) const fn as_loop_iteration_count(&self) -> Option { + if let EnvEntryKind::Loop { + iteration_count, .. + } = self.kind + { + return Some(iteration_count); + } + None } - pub(crate) const fn as_loop_iteration_count(self) -> Option { - if let EnvEntryKind::Loop { iteration_count } = self.kind { - return Some(iteration_count); + /// Increases loop iteration count if `EnvStackEntry` is a loop. + pub(crate) fn increase_loop_iteration_count(&mut self) { + if let EnvEntryKind::Loop { + iteration_count, .. + } = &mut self.kind + { + *iteration_count = iteration_count.wrapping_add(1); + } + } + + /// 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 } @@ -151,4 +191,14 @@ impl EnvStackEntry { pub(crate) fn dec_env_num(&mut self) { self.env_num -= 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 40f97aa844..2c2f4a020d 100644 --- a/boa_engine/src/vm/call_frame/mod.rs +++ b/boa_engine/src/vm/call_frame/mod.rs @@ -28,7 +28,6 @@ pub struct CallFrame { pub(crate) pop_on_return: usize, // 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) param_count: usize, pub(crate) arg_count: usize, diff --git a/boa_engine/src/vm/code_block.rs b/boa_engine/src/vm/code_block.rs index 844f292843..1c1e6506e1 100644 --- a/boa_engine/src/vm/code_block.rs +++ b/boa_engine/src/vm/code_block.rs @@ -290,7 +290,6 @@ impl CodeBlock { | Opcode::CopyDataProperties | Opcode::Break | Opcode::Continue - | Opcode::LoopContinue | Opcode::LoopStart | Opcode::TryStart | Opcode::AsyncGeneratorNext @@ -460,6 +459,8 @@ impl CodeBlock { | Opcode::Return | Opcode::PopEnvironment | Opcode::LoopEnd + | Opcode::LoopContinue + | Opcode::LoopUpdateReturnValue | Opcode::LabelledEnd | Opcode::CreateForInIterator | Opcode::GetIterator diff --git a/boa_engine/src/vm/flowgraph/mod.rs b/boa_engine/src/vm/flowgraph/mod.rs index 37933d8e50..f78a9ba90c 100644 --- a/boa_engine/src/vm/flowgraph/mod.rs +++ b/boa_engine/src/vm/flowgraph/mod.rs @@ -138,7 +138,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::LoopContinue | Opcode::LoopStart => { + Opcode::LoopStart => { let start_address = self.read::(pc); pc += size_of::(); let end_address = self.read::(pc); @@ -540,6 +540,8 @@ impl CodeBlock { | Opcode::This | Opcode::Super | Opcode::LoopEnd + | Opcode::LoopContinue + | Opcode::LoopUpdateReturnValue | Opcode::LabelledEnd | Opcode::CreateForInIterator | Opcode::GetIterator diff --git a/boa_engine/src/vm/opcode/control_flow/break.rs b/boa_engine/src/vm/opcode/control_flow/break.rs index e897f5a1e5..dfb1af2684 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, + Context, JsResult, JsValue, }; /// `Break` implements the Opcode Operation for `Opcode::Break` @@ -17,18 +17,39 @@ 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; - while let Some(env_entry) = context.vm.frame().env_stack.last() { + 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 { + break; + } + + let Some(env_entry) = context.vm.frame_mut().env_stack.get_mut(i) else { + 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()) { - break; + found_target = true; + set_loop_result = env_entry.set_loop_return_value(value.clone()); + continue; } // Checks for the break if we have jumped from inside of a finally block if jump_address == env_entry.exit_address() { - break; + found_target = true; + set_loop_result = env_entry.set_loop_return_value(value.clone()); + continue; } envs_to_pop += env_entry.env_num(); context.vm.frame_mut().env_stack.pop(); diff --git a/boa_engine/src/vm/opcode/control_flow/continue.rs b/boa_engine/src/vm/opcode/control_flow/continue.rs index d20572445d..80d62e4182 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, + Context, JsResult, JsValue, }; /// `Continue` implements the Opcode Operation for `Opcode::Continue` @@ -21,23 +21,44 @@ 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; - while let Some(env_entry) = context.vm.frame_mut().env_stack.last() { + 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 { + break; + } + + let Some(env_entry) = context.vm.frame_mut().env_stack.get_mut(i) else { + 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. if (env_entry.is_finally_env() && jump_address == env_entry.start_address()) || (jump_address == target_address && jump_address == env_entry.start_address()) { - break; + found_target = true; + set_loop_result = env_entry.set_loop_return_value(value.clone()); + continue; } envs_to_pop += env_entry.env_num(); // 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(); - break; + continue; } context.vm.frame_mut().env_stack.pop(); } diff --git a/boa_engine/src/vm/opcode/iteration/loop_ops.rs b/boa_engine/src/vm/opcode/iteration/loop_ops.rs index 6352da6582..fdacf20903 100644 --- a/boa_engine/src/vm/opcode/iteration/loop_ops.rs +++ b/boa_engine/src/vm/opcode/iteration/loop_ops.rs @@ -26,22 +26,6 @@ impl Operation for LoopStart { } } -/// This is a helper function used to clean the loop environment created by the -/// [`LoopStart`] and [`LoopContinue`] opcodes. -fn cleanup_loop_environment(context: &mut Context<'_>) { - 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_loop_env() { - break; - } - } - - let env_truncation_len = context.vm.environments.len().saturating_sub(envs_to_pop); - context.vm.environments.truncate(env_truncation_len); -} - /// `LoopContinue` implements the Opcode Operation for `Opcode::LoopContinue`. /// /// Operation: @@ -54,48 +38,31 @@ impl Operation for LoopContinue { const INSTRUCTION: &'static str = "INST - LoopContinue"; fn execute(context: &mut Context<'_>) -> JsResult { - let start = context.vm.read::(); - let exit = context.vm.read::(); - - let mut iteration_count = 0; - // 1. Clean up the previous environment. - if let Some(entry) = context + let env = context .vm - .frame() + .frame_mut() .env_stack - .last() - .filter(|entry| entry.exit_address() == exit) - { - let env_truncation_len = context - .vm - .environments - .len() - .saturating_sub(entry.env_num()); - context.vm.environments.truncate(env_truncation_len); - - // Pop loop environment and get it's iteration count. - let previous_entry = context.vm.frame_mut().env_stack.pop(); - if let Some(previous_iteration_count) = - previous_entry.and_then(EnvStackEntry::as_loop_iteration_count) - { - iteration_count = previous_iteration_count.wrapping_add(1); - - let max = context.vm.runtime_limits.loop_iteration_limit(); - if previous_iteration_count > max { - cleanup_loop_environment(context); - - return Err(JsNativeError::runtime_limit() - .with_message(format!("Maximum loop iteration limit {max} exceeded")) - .into()); - } + .last_mut() + .expect("loop environment must be present"); + + let env_num = env.env_num(); + env.clear_env_num(); + + if let Some(previous_iteration_count) = env.as_loop_iteration_count() { + env.increase_loop_iteration_count(); + let max = context.vm.runtime_limits.loop_iteration_limit(); + if previous_iteration_count > max { + let env_truncation_len = context.vm.environments.len().saturating_sub(env_num); + context.vm.environments.truncate(env_truncation_len); + return Err(JsNativeError::runtime_limit() + .with_message(format!("Maximum loop iteration limit {max} exceeded")) + .into()); } } - // 2. Push a new clean EnvStack. - let entry = EnvStackEntry::new(start, exit).with_loop_flag(iteration_count); - - context.vm.frame_mut().env_stack.push(entry); + let env_truncation_len = context.vm.environments.len().saturating_sub(env_num); + context.vm.environments.truncate(env_truncation_len); Ok(CompletionType::Normal) } @@ -104,7 +71,7 @@ impl Operation for LoopContinue { /// `LoopEnd` implements the Opcode Operation for `Opcode::LoopEnd` /// /// Operation: -/// - Clean up enviroments at the end of a lopp. +/// - Clean up environments at the end of a loop. #[derive(Debug, Clone, Copy)] pub(crate) struct LoopEnd; @@ -113,7 +80,44 @@ impl Operation for LoopEnd { const INSTRUCTION: &'static str = "INST - LoopEnd"; fn execute(context: &mut Context<'_>) -> JsResult { - cleanup_loop_environment(context); + 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 let Some(value) = env_entry.loop_env_value() { + context.vm.push(value.clone()); + + break; + } + } + + let env_truncation_len = context.vm.environments.len().saturating_sub(envs_to_pop); + context.vm.environments.truncate(env_truncation_len); + + 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 8336139ab4..612b63032e 100644 --- a/boa_engine/src/vm/opcode/mod.rs +++ b/boa_engine/src/vm/opcode/mod.rs @@ -1168,14 +1168,14 @@ generate_impl! { /// /// Operands: Jump Address: u32, Target address: u32 /// - /// Stack: **=>** + /// Stack: loop_return_value **=>** Break, /// Sets the `AbruptCompletionRecord` for a delayed continue /// /// Operands: Jump Address: u32, Target address: u32, /// - /// Stack: **=>** + /// Stack: loop_return_value **=>** Continue, /// Pops value converts it to boolean and pushes it back. @@ -1370,18 +1370,25 @@ generate_impl! { /// Clean up environments when a loop continues. /// - /// Operands: Start Address: `u32`, Exit Address: `u32` + /// Operands: /// /// Stack: **=>** LoopContinue, - /// Clean up environments at the end of a loop. + /// Clean up environments at the end of a loop and return it's value. /// /// Operands: /// - /// Stack: **=>** + /// Stack: **=>** value LoopEnd, + /// Update the return value of a loop. + /// + /// Operands: + /// + /// Stack: loop_return_value **=>** + LoopUpdateReturnValue, + /// Push labelled start marker. /// /// Operands: Exit Address: u32,