From 90d310b184ecae4d4e13379f803815ba3c948b59 Mon Sep 17 00:00:00 2001 From: Kevin Date: Thu, 19 Jan 2023 19:56:20 +0000 Subject: [PATCH] Labelled ByteCompiler Fix (#2534) This Pull Request addresses #2295, and another case that I came across when I was adding `Break` to the `ByteCompiler` I did have a question that came up during this regarding the spec. We currently don't implement the [BreakableStatement](https://tc39.es/ecma262/#prod-BreakableStatement). Any thoughts on whether we should be? Especially since `BreakableStatement` seems to be a bit of a inaccurate since `LabelledStatement` is breakable too. It changes the following: - Moves handling of label jump out of `compile_block` and into `compile_labelled`. - Adds a couple more tests to keep track of `LabelledStatement` breaks. Co-authored-by: Ness --- boa_engine/src/bytecompiler/jump_control.rs | 16 ++++------ .../src/bytecompiler/statement/block.rs | 13 +------- .../src/bytecompiler/statement/labelled.rs | 13 +++----- boa_engine/src/bytecompiler/statement/mod.rs | 2 +- boa_engine/src/tests.rs | 32 +++++++++++++++++++ 5 files changed, 46 insertions(+), 30 deletions(-) diff --git a/boa_engine/src/bytecompiler/jump_control.rs b/boa_engine/src/bytecompiler/jump_control.rs index 7dc30b38bc..f8c52fa20d 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,10 @@ 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()); - - self.emit_opcode(Opcode::PopEnvironment); + assert!(info.is_labelled()); 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 4de6f7c718..512f5d3f77 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 f398bfabeb..8c4e944ac1 100644 --- a/boa_engine/src/bytecompiler/statement/labelled.rs +++ b/boa_engine/src/bytecompiler/statement/labelled.rs @@ -16,6 +16,9 @@ impl ByteCompiler<'_, '_> { use_expr: bool, configurable_globals: bool, ) -> JsResult<()> { + 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,14 +52,6 @@ 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) => { @@ -64,6 +59,8 @@ impl ByteCompiler<'_, '_> { } } + 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 23c89cb9fc..5f509c41f2 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 4a8e7e6248..5fac76f7d0 100644 --- a/boa_engine/src/tests.rs +++ b/boa_engine/src/tests.rs @@ -2198,6 +2198,38 @@ 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.