Labelled ByteCompiler Fix (#2534)

<!---
Thank you for contributing to Boa! Please fill out the template below, and remove or add any
information as you feel necessary.
--->

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 <Kevin.Ness@Staples.com>

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);

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<Sym>,
         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();
-        }
 
         Ok(())
     }

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(())
     }
 }

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)?;

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.