From 33e4da994bd4596b7e93b5259240f007971cae9c Mon Sep 17 00:00:00 2001 From: nekevss Date: Sun, 15 Jan 2023 11:46:11 -0500 Subject: [PATCH 1/4] Initial commit for fix --- boa_engine/src/bytecompiler/jump_control.rs | 17 +++++----- .../src/bytecompiler/statement/block.rs | 13 +------ .../src/bytecompiler/statement/labelled.rs | 14 ++++---- boa_engine/src/bytecompiler/statement/mod.rs | 2 +- boa_engine/src/tests.rs | 34 +++++++++++++++++++ 5 files changed, 51 insertions(+), 29 deletions(-) diff --git a/boa_engine/src/bytecompiler/jump_control.rs b/boa_engine/src/bytecompiler/jump_control.rs index 7dc30b38bc0..cb4196036a1 100644 --- a/boa_engine/src/bytecompiler/jump_control.rs +++ b/boa_engine/src/bytecompiler/jump_control.rs @@ -35,7 +35,7 @@ bitflags! { const LOOP = 0b0000_0001; const SWITCH = 0b0000_0010; const TRY_BLOCK = 0b0000_0100; - const LABELLED_BLOCK = 0b0000_1000; + const LABELLED = 0b0000_1000; const IN_CATCH = 0b0001_0000; const HAS_FINALLY = 0b0010_0000; const FOR_OF_IN_LOOP = 0b0100_0000; @@ -91,7 +91,7 @@ impl JumpControlInfo { } pub(crate) fn with_labelled_block_flag(mut self, value: bool) -> Self { - self.flags.set(JumpControlInfoFlags::LABELLED_BLOCK, value); + self.flags.set(JumpControlInfoFlags::LABELLED, value); self } @@ -129,8 +129,8 @@ impl JumpControlInfo { self.flags.contains(JumpControlInfoFlags::TRY_BLOCK) } - pub(crate) const fn is_labelled_block(&self) -> bool { - self.flags.contains(JumpControlInfoFlags::LABELLED_BLOCK) + pub(crate) const fn is_labelled(&self) -> bool { + self.flags.contains(JumpControlInfoFlags::LABELLED) } pub(crate) const fn in_catch(&self) -> bool { @@ -363,7 +363,7 @@ impl ByteCompiler<'_, '_> { } } - pub(crate) fn push_labelled_block_control_info(&mut self, label: Sym, start_address: u32) { + pub(crate) fn push_labelled_control_info(&mut self, label: Sym, start_address: u32) { let new_info = JumpControlInfo::default() .with_labelled_block_flag(true) .with_label(Some(label)) @@ -371,12 +371,13 @@ impl ByteCompiler<'_, '_> { self.jump_info.push(new_info); } - pub(crate) fn pop_labelled_block_control_info(&mut self) { + pub(crate) fn pop_labelled_control_info(&mut self) { let info = self.jump_info.pop().expect("no jump information found"); - assert!(info.is_labelled_block()); + assert!(info.is_labelled()); - self.emit_opcode(Opcode::PopEnvironment); + // We should no longer have to PopEnvironment here + // self.emit_opcode(Opcode::PopEnvironment); for label in info.breaks { self.patch_jump(label); diff --git a/boa_engine/src/bytecompiler/statement/block.rs b/boa_engine/src/bytecompiler/statement/block.rs index 4de6f7c718f..512f5d3f77a 100644 --- a/boa_engine/src/bytecompiler/statement/block.rs +++ b/boa_engine/src/bytecompiler/statement/block.rs @@ -1,22 +1,15 @@ use crate::{bytecompiler::ByteCompiler, JsResult}; use boa_ast::statement::Block; -use boa_interner::Sym; impl ByteCompiler<'_, '_> { /// Compile a [`Block`] `boa_ast` node pub(crate) fn compile_block( &mut self, block: &Block, - label: Option, use_expr: bool, configurable_globals: bool, ) -> JsResult<()> { - if let Some(label) = label { - let next = self.next_opcode_location(); - self.push_labelled_block_control_info(label, next); - } - self.context.push_compile_time_environment(false); let push_env = self.emit_and_track_decl_env(); @@ -28,11 +21,7 @@ impl ByteCompiler<'_, '_> { self.patch_jump_with_target(push_env.0, num_bindings as u32); self.patch_jump_with_target(push_env.1, index_compile_environment as u32); - if label.is_some() { - self.pop_labelled_block_control_info(); - } else { - self.emit_and_track_pop_env(); - } + self.emit_and_track_pop_env(); Ok(()) } diff --git a/boa_engine/src/bytecompiler/statement/labelled.rs b/boa_engine/src/bytecompiler/statement/labelled.rs index f398bfabebf..0329a62a91f 100644 --- a/boa_engine/src/bytecompiler/statement/labelled.rs +++ b/boa_engine/src/bytecompiler/statement/labelled.rs @@ -16,6 +16,10 @@ impl ByteCompiler<'_, '_> { use_expr: bool, configurable_globals: bool, ) -> JsResult<()> { + // Push a label jump control info + let labelled_loc = self.next_opcode_location(); + self.push_labelled_control_info(labelled.label(), labelled_loc); + match labelled.item() { LabelledItem::Statement(stmt) => match stmt { Statement::ForLoop(for_loop) => { @@ -49,20 +53,14 @@ impl ByteCompiler<'_, '_> { configurable_globals, )?; } - Statement::Block(block) => { - self.compile_block( - block, - Some(labelled.label()), - use_expr, - configurable_globals, - )?; - } stmt => self.compile_stmt(stmt, use_expr, configurable_globals)?, }, LabelledItem::Function(f) => { self.function(f.into(), NodeKind::Declaration, false)?; } } + + self.pop_labelled_control_info(); Ok(()) } diff --git a/boa_engine/src/bytecompiler/statement/mod.rs b/boa_engine/src/bytecompiler/statement/mod.rs index 23c89cb9fc9..5f509c41f2c 100644 --- a/boa_engine/src/bytecompiler/statement/mod.rs +++ b/boa_engine/src/bytecompiler/statement/mod.rs @@ -38,7 +38,7 @@ impl ByteCompiler<'_, '_> { self.compile_do_while_loop(do_while_loop, None, configurable_globals)?; } Statement::Block(block) => { - self.compile_block(block, None, use_expr, configurable_globals)?; + self.compile_block(block, use_expr, configurable_globals)?; } Statement::Labelled(labelled) => { self.compile_labelled(labelled, use_expr, configurable_globals)?; diff --git a/boa_engine/src/tests.rs b/boa_engine/src/tests.rs index 4a8e7e62482..6d1161d33e9 100644 --- a/boa_engine/src/tests.rs +++ b/boa_engine/src/tests.rs @@ -2198,6 +2198,40 @@ fn break_environment_gauntlet() { assert_eq!(&exec(scenario), "\"5601try_block\""); } +#[test] +fn break_labelled_if_statement() { + + let scenario = r#" + let result = ""; + bar: if(true) { + result = "foo"; + break bar; + result = 'this will not be executed'; + } + result + "#; + + assert_eq!(&exec(scenario), "\"foo\"") +} + +#[test] +fn break_labelled_try_statement() { + + let scenario = r#" + let result = "" + one: try { + result = "foo"; + break one; + result = "did not break" + } catch (err) { + console.log(err) + } + result + "#; + + assert_eq!(&exec(scenario), "\"foo\"") +} + #[test] fn while_loop_late_break() { // Ordering with statement before the break. From e5b5c74c8ddb8122c07bf2948c2880cae740354b Mon Sep 17 00:00:00 2001 From: nekevss Date: Sun, 15 Jan 2023 11:47:15 -0500 Subject: [PATCH 2/4] Forgot Rustfmt --- boa_engine/src/bytecompiler/statement/labelled.rs | 4 ++-- boa_engine/src/tests.rs | 6 ++---- 2 files changed, 4 insertions(+), 6 deletions(-) diff --git a/boa_engine/src/bytecompiler/statement/labelled.rs b/boa_engine/src/bytecompiler/statement/labelled.rs index 0329a62a91f..0275368e6e1 100644 --- a/boa_engine/src/bytecompiler/statement/labelled.rs +++ b/boa_engine/src/bytecompiler/statement/labelled.rs @@ -18,7 +18,7 @@ impl ByteCompiler<'_, '_> { ) -> JsResult<()> { // Push a label jump control info let labelled_loc = self.next_opcode_location(); - self.push_labelled_control_info(labelled.label(), labelled_loc); + self.push_labelled_control_info(labelled.label(), labelled_loc); match labelled.item() { LabelledItem::Statement(stmt) => match stmt { @@ -59,7 +59,7 @@ impl ByteCompiler<'_, '_> { self.function(f.into(), NodeKind::Declaration, false)?; } } - + self.pop_labelled_control_info(); Ok(()) diff --git a/boa_engine/src/tests.rs b/boa_engine/src/tests.rs index 6d1161d33e9..e45eccc86c0 100644 --- a/boa_engine/src/tests.rs +++ b/boa_engine/src/tests.rs @@ -2198,9 +2198,8 @@ fn break_environment_gauntlet() { assert_eq!(&exec(scenario), "\"5601try_block\""); } -#[test] +#[test] fn break_labelled_if_statement() { - let scenario = r#" let result = ""; bar: if(true) { @@ -2213,10 +2212,9 @@ fn break_labelled_if_statement() { assert_eq!(&exec(scenario), "\"foo\"") } - + #[test] fn break_labelled_try_statement() { - let scenario = r#" let result = "" one: try { From 7a07122a448c8a685b3ef6c04ad2d51f3c4d1a0c Mon Sep 17 00:00:00 2001 From: nekevss Date: Sun, 15 Jan 2023 12:04:38 -0500 Subject: [PATCH 3/4] Missed some semicolons --- boa_engine/src/tests.rs | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/boa_engine/src/tests.rs b/boa_engine/src/tests.rs index e45eccc86c0..5fac76f7d05 100644 --- a/boa_engine/src/tests.rs +++ b/boa_engine/src/tests.rs @@ -2210,7 +2210,7 @@ fn break_labelled_if_statement() { result "#; - assert_eq!(&exec(scenario), "\"foo\"") + assert_eq!(&exec(scenario), "\"foo\""); } #[test] @@ -2227,7 +2227,7 @@ fn break_labelled_try_statement() { result "#; - assert_eq!(&exec(scenario), "\"foo\"") + assert_eq!(&exec(scenario), "\"foo\""); } #[test] From 0335e1eb2d639d310f165a56d24e9091aadc6c46 Mon Sep 17 00:00:00 2001 From: Ness Date: Wed, 18 Jan 2023 09:24:41 -0500 Subject: [PATCH 4/4] Complete some cleanup --- boa_engine/src/bytecompiler/jump_control.rs | 3 --- boa_engine/src/bytecompiler/statement/labelled.rs | 1 - 2 files changed, 4 deletions(-) diff --git a/boa_engine/src/bytecompiler/jump_control.rs b/boa_engine/src/bytecompiler/jump_control.rs index cb4196036a1..f8c52fa20d5 100644 --- a/boa_engine/src/bytecompiler/jump_control.rs +++ b/boa_engine/src/bytecompiler/jump_control.rs @@ -376,9 +376,6 @@ impl ByteCompiler<'_, '_> { assert!(info.is_labelled()); - // We should no longer have to PopEnvironment here - // self.emit_opcode(Opcode::PopEnvironment); - for label in info.breaks { self.patch_jump(label); } diff --git a/boa_engine/src/bytecompiler/statement/labelled.rs b/boa_engine/src/bytecompiler/statement/labelled.rs index 0275368e6e1..8c4e944ac1e 100644 --- a/boa_engine/src/bytecompiler/statement/labelled.rs +++ b/boa_engine/src/bytecompiler/statement/labelled.rs @@ -16,7 +16,6 @@ impl ByteCompiler<'_, '_> { use_expr: bool, configurable_globals: bool, ) -> JsResult<()> { - // Push a label jump control info let labelled_loc = self.next_opcode_location(); self.push_labelled_control_info(labelled.label(), labelled_loc);