diff --git a/boa_ast/src/operations.rs b/boa_ast/src/operations.rs index 2504bc0aefd..09d7634d06b 100644 --- a/boa_ast/src/operations.rs +++ b/boa_ast/src/operations.rs @@ -2098,3 +2098,61 @@ impl<'ast> Visitor<'ast> for AnnexBFunctionDeclarationNamesVisitor<'_> { self.visit(node.statement()) } } + +/// Returns `true` if the given statement returns a value. +#[must_use] +pub fn returns_value<'a, N>(node: &'a N) -> bool +where + &'a N: Into>, +{ + ReturnsValueVisitor.visit(node.into()).is_break() +} + +/// The [`Visitor`] used for [`returns_value`]. +#[derive(Debug)] +struct ReturnsValueVisitor; + +impl<'ast> Visitor<'ast> for ReturnsValueVisitor { + type BreakTy = (); + + fn visit_block(&mut self, node: &'ast crate::statement::Block) -> ControlFlow { + for statement in node.statement_list().statements() { + match statement { + StatementListItem::Declaration(_) => {} + StatementListItem::Statement(node) => try_break!(self.visit(node)), + } + } + ControlFlow::Continue(()) + } + + fn visit_statement(&mut self, node: &'ast Statement) -> ControlFlow { + match node { + Statement::Empty | Statement::Var(_) => {} + Statement::Block(node) => try_break!(self.visit(node)), + Statement::Labelled(node) => try_break!(self.visit(node)), + _ => return ControlFlow::Break(()), + } + ControlFlow::Continue(()) + } + + fn visit_case(&mut self, node: &'ast crate::statement::Case) -> ControlFlow { + for statement in node.body().statements() { + match statement { + StatementListItem::Declaration(_) => {} + StatementListItem::Statement(node) => try_break!(self.visit(node)), + } + } + ControlFlow::Continue(()) + } + + fn visit_labelled( + &mut self, + node: &'ast crate::statement::Labelled, + ) -> ControlFlow { + match node.item() { + LabelledItem::Statement(node) => try_break!(self.visit(node)), + LabelledItem::Function(_) => {} + } + ControlFlow::Continue(()) + } +} diff --git a/boa_ast/src/statement/mod.rs b/boa_ast/src/statement/mod.rs index f810c285edf..d6c920c4925 100644 --- a/boa_ast/src/statement/mod.rs +++ b/boa_ast/src/statement/mod.rs @@ -162,17 +162,6 @@ 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 b2d5a3a3c81..996a6acaa38 100644 --- a/boa_engine/src/bytecompiler/mod.rs +++ b/boa_engine/src/bytecompiler/mod.rs @@ -31,6 +31,7 @@ use boa_ast::{ ArrowFunction, AsyncArrowFunction, AsyncFunction, AsyncGenerator, Class, FormalParameterList, Function, FunctionBody, Generator, PrivateName, }, + operations::returns_value, pattern::Pattern, Declaration, Expression, Statement, StatementList, StatementListItem, }; @@ -734,34 +735,34 @@ 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 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| { - matches!( - item, - &&StatementListItem::Statement(Statement::Empty | Statement::Var(_)) - | &&StatementListItem::Declaration(_) - ) - }) - .count(); - - if expr_index == 0 && !list.statements().is_empty() { + let mut has_returns_value = false; + let mut use_expr_index = 0; + let mut first_return_is_abrupt = false; + for (i, statement) in list.statements().iter().enumerate() { + match statement { + StatementListItem::Statement(Statement::Break(_) | Statement::Continue(_)) => { + if !has_returns_value { + first_return_is_abrupt = true; + } + break; + } + StatementListItem::Statement(Statement::Empty | Statement::Var(_)) + | StatementListItem::Declaration(_) => {} + StatementListItem::Statement(Statement::Block(block)) + if !returns_value(block) => {} + StatementListItem::Statement(_) => { + has_returns_value = true; + use_expr_index = i; + } + } + } + + if first_return_is_abrupt { self.emit_opcode(Opcode::PushUndefined); } for (i, item) in list.statements().iter().enumerate() { - self.compile_stmt_list_item(item, i + 1 == expr_index, block); + self.compile_stmt_list_item(item, i == use_expr_index, block); } } else { for item in list.statements() { diff --git a/boa_engine/src/bytecompiler/statement/break.rs b/boa_engine/src/bytecompiler/statement/break.rs index 342a9fa14a7..157a9a9924d 100644 --- a/boa_engine/src/bytecompiler/statement/break.rs +++ b/boa_engine/src/bytecompiler/statement/break.rs @@ -1,5 +1,5 @@ use crate::{ - bytecompiler::{ByteCompiler, Label}, + bytecompiler::{ByteCompiler, JumpControlInfo}, vm::Opcode, }; use boa_ast::statement::Break; @@ -29,10 +29,11 @@ impl ByteCompiler<'_, '_> { let (break_label, target_jump_label) = self.emit_opcode_with_two_operands(opcode); if let Some(node_label) = node.label() { - self.search_jump_info_label(target_jump_label, node_label); + let info = self.jump_info_label(node_label); + info.push_break_label(target_jump_label); if !has_finally_or_is_finally { - self.search_jump_info_label(break_label, node_label); + info.push_break_label(break_label); return; } } else { @@ -55,29 +56,32 @@ impl ByteCompiler<'_, '_> { // Emit the break opcode -> (Label, Label) let (break_label, target_label) = self.emit_opcode_with_two_operands(opcode); if let Some(label) = node.label() { - self.search_jump_info_label(break_label, label); - self.search_jump_info_label(target_label, label); + let info = self.jump_info_label(label); + info.push_break_label(break_label); + info.push_break_label(target_label); return; } - let info = self - .jump_info - .last_mut() - .expect("unlabeled break must be inside loop or switch"); - + let info = self.jump_info_non_labelled(); info.push_break_label(break_label); info.push_break_label(target_label); } - fn search_jump_info_label(&mut self, address: Label, node_label: Sym) { - let mut found = false; + fn jump_info_non_labelled(&mut self) -> &mut JumpControlInfo { + for info in self.jump_info.iter_mut().rev() { + if !info.is_labelled() { + return info; + } + } + panic!("Jump info without label must exist"); + } + + fn jump_info_label(&mut self, label: Sym) -> &mut JumpControlInfo { for info in self.jump_info.iter_mut().rev() { - if info.label() == Some(node_label) { - info.push_break_label(address); - found = true; - break; + if info.label() == Some(label) { + return info; } } - assert!(found, "Cannot use the undeclared label"); + panic!("Jump info with label must exist"); } } diff --git a/boa_engine/src/bytecompiler/statement/if.rs b/boa_engine/src/bytecompiler/statement/if.rs index a0183628b19..1838d7985a0 100644 --- a/boa_engine/src/bytecompiler/statement/if.rs +++ b/boa_engine/src/bytecompiler/statement/if.rs @@ -1,12 +1,12 @@ use crate::{bytecompiler::ByteCompiler, vm::Opcode}; -use boa_ast::statement::If; +use boa_ast::{operations::returns_value, 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 !node.body().returns_value() { + if !returns_value(node.body()) { self.emit_opcode(Opcode::PushUndefined); } self.compile_stmt(node.body(), true); @@ -18,7 +18,7 @@ impl ByteCompiler<'_, '_> { self.emit_opcode(Opcode::PushUndefined); } Some(else_body) => { - if !else_body.returns_value() { + if !returns_value(else_body) { self.emit_opcode(Opcode::PushUndefined); } self.compile_stmt(else_body, true); diff --git a/boa_engine/src/bytecompiler/statement/loop.rs b/boa_engine/src/bytecompiler/statement/loop.rs index e5b005b0f8e..0c64acb20fd 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, + operations::{bound_names, returns_value}, statement::{ iteration::{ForLoopInitializer, IterableLoopInitializer}, DoWhileLoop, ForInLoop, ForLoop, ForOfLoop, WhileLoop, @@ -95,7 +95,7 @@ impl ByteCompiler<'_, '_> { } let exit = self.jump_if_false(); - if !for_loop.body().returns_value() { + if !returns_value(for_loop.body()) { self.emit_opcode(Opcode::PushUndefined); } self.compile_stmt(for_loop.body(), true); @@ -229,7 +229,7 @@ impl ByteCompiler<'_, '_> { } } - if !for_in_loop.body().returns_value() { + if !returns_value(for_in_loop.body()) { self.emit_opcode(Opcode::PushUndefined); } self.compile_stmt(for_in_loop.body(), true); @@ -375,7 +375,7 @@ impl ByteCompiler<'_, '_> { } } - if !for_of_loop.body().returns_value() { + if !returns_value(for_of_loop.body()) { self.emit_opcode(Opcode::PushUndefined); } self.compile_stmt(for_of_loop.body(), true); @@ -416,7 +416,7 @@ impl ByteCompiler<'_, '_> { self.compile_expr(while_loop.condition(), true); let exit = self.jump_if_false(); - if !while_loop.body().returns_value() { + if !returns_value(while_loop.body()) { self.emit_opcode(Opcode::PushUndefined); } self.compile_stmt(while_loop.body(), true); @@ -454,7 +454,7 @@ impl ByteCompiler<'_, '_> { self.patch_jump(initial_label); - if !do_while_loop.body().returns_value() { + if !returns_value(do_while_loop.body()) { self.emit_opcode(Opcode::PushUndefined); } self.compile_stmt(do_while_loop.body(), true); diff --git a/boa_engine/src/bytecompiler/statement/switch.rs b/boa_engine/src/bytecompiler/statement/switch.rs index 3f14914f97c..e0b67a00651 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::statement::Switch; +use boa_ast::{operations::returns_value, statement::Switch}; impl ByteCompiler<'_, '_> { /// Compile a [`Switch`] `boa_ast` node @@ -44,7 +44,7 @@ impl ByteCompiler<'_, '_> { }; self.patch_jump(label); self.compile_statement_list(case.body(), true, true); - if !case.body().statements().is_empty() { + if returns_value(case) { self.emit_opcode(Opcode::LoopUpdateReturnValue); } } diff --git a/boa_engine/src/bytecompiler/statement/try.rs b/boa_engine/src/bytecompiler/statement/try.rs index aa22012438c..082bbcd075b 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, + operations::{bound_names, returns_value}, statement::{Catch, Finally, Try}, }; @@ -21,7 +21,7 @@ impl ByteCompiler<'_, '_> { self.push_try_control_info(t.finally().is_some(), try_start); self.compile_block(t.block(), true); - if t.block().statement_list().statements().is_empty() { + if !returns_value(t.block()) { self.emit_opcode(Opcode::PushUndefined); } if !use_expr { @@ -80,7 +80,7 @@ impl ByteCompiler<'_, '_> { } self.compile_block(catch.block(), true); - if catch.block().statement_list().statements().is_empty() { + if !returns_value(catch.block()) { self.emit_opcode(Opcode::PushUndefined); } if !use_expr { @@ -103,7 +103,7 @@ impl ByteCompiler<'_, '_> { pub(crate) fn compile_finally_stmt(&mut self, finally: &Finally, finally_end_label: Label) { self.compile_block(finally.block(), true); - if !finally.block().statement_list().statements().is_empty() { + if returns_value(finally.block()) { self.emit_opcode(Opcode::Pop); } diff --git a/boa_engine/src/bytecompiler/statement/with.rs b/boa_engine/src/bytecompiler/statement/with.rs index dccbacca4f4..3d42c15886a 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::statement::With; +use boa_ast::{operations::returns_value, statement::With}; impl ByteCompiler<'_, '_> { /// Compile a [`With`] `boa_ast` node @@ -8,7 +8,7 @@ impl ByteCompiler<'_, '_> { self.push_compile_environment(false); self.emit_opcode(Opcode::PushObjectEnvironment); - if !with.statement().returns_value() { + if !returns_value(with.statement()) { self.emit_opcode(Opcode::PushUndefined); } self.compile_stmt(with.statement(), true); diff --git a/boa_engine/src/vm/tests.rs b/boa_engine/src/vm/tests.rs index 954d2f3fd08..b1072538046 100644 --- a/boa_engine/src/vm/tests.rs +++ b/boa_engine/src/vm/tests.rs @@ -324,3 +324,36 @@ fn arguments_object_constructor_valid_index() { "object", )]); } + +#[test] +fn empty_return_values() { + run_test_actions([ + TestAction::run(indoc! {r#"do {{}} while (false);"#}), + TestAction::run(indoc! {r#"do try {{}} catch {} while (false);"#}), + TestAction::run(indoc! {r#"do {} while (false);"#}), + TestAction::run(indoc! {r#"do try {{}{}} catch {} while (false);"#}), + TestAction::run(indoc! {r#"do {{}{}} while (false);"#}), + TestAction::run(indoc! {r#"do {;{}} while (false);"#}), + TestAction::run(indoc! {r#"do {e: {}} while (false);"#}), + TestAction::run(indoc! {r#"do {e: ;} while (false);"#}), + TestAction::run(indoc! {r#"do { break } while (false);"#}), + TestAction::run(indoc! {r#"while (true) a: break"#}), + TestAction::run(indoc! {r#"while (true) a: {"a"; break};"#}), + TestAction::run(indoc! {r#"do {"a";{}} while (false);"#}), + TestAction::run(indoc! {r#" + switch (false) { + default: {} + } + "#}), + TestAction::run(indoc! {r#" + switch (false) { + default: {}{} + } + "#}), + TestAction::run(indoc! {r#" + switch (false) { + default: ;{}{} + } + "#}), + ]); +}