Try-catch-block control flow fix/refactor (#2568)

<!---
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 is meant to address #1900. While working on it, there was a decent amount of refactoring/restructuring. Initially, I had kept with the current approach of just keeping track of a kind and counter on the environment stack, especially because that kept the size of each stack entry to a minimum. 

I did, however, make a switch to having the opcode create the `EnvStackEntry` with a start address and exit address for a bit more precision.

It changes the following:

- Consolidates `loop_env_stack` and `try_env_stack` into one `EnvStackEntry` struct.
- Changes `Continue`, `Break`, `LoopStart`, `LoopContinue`, `FinallyStart`, `FinallyEnd` and various others. Most of this primarily revolves around the creating of `EnvStackEntry` and interacting with the `env_stack`. 
- Changes/updates the try-catch-finally, break and continue statement compilations as necessary
- Adds an `AbruptCompletionRecord` in place of the `finally_jump` vector.
- Adds some tests for try-catch-finally blocks with breaks.

boa_engine/src/bytecompiler/statement/block.rs

@@ -1,4 +1,4 @@
-use crate::{bytecompiler::ByteCompiler, JsResult};
+use crate::{bytecompiler::ByteCompiler, vm::Opcode, JsResult};
 
 use boa_ast::statement::Block;
 
@@ -11,7 +11,7 @@ impl ByteCompiler<'_, '_> {
         configurable_globals: bool,
     ) -> JsResult<()> {
         self.context.push_compile_time_environment(false);
-        let push_env = self.emit_and_track_decl_env();
+        let push_env = self.emit_opcode_with_two_operands(Opcode::PushDeclarativeEnvironment);
 
         self.create_script_decls(block.statement_list(), configurable_globals);
         self.compile_statement_list(block.statement_list(), use_expr, configurable_globals)?;
@@ -21,7 +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);
 
-        self.emit_and_track_pop_env();
+        self.emit_opcode(Opcode::PopEnvironment);
 
         Ok(())
     }

boa_engine/src/bytecompiler/statement/break.rs

@@ -1,68 +1,100 @@
 use boa_ast::statement::Break;
 
-use crate::{bytecompiler::ByteCompiler, vm::Opcode, JsNativeError, JsResult};
+use crate::{
+    bytecompiler::{ByteCompiler, Label},
+    vm::Opcode,
+    JsNativeError, JsResult,
+};
+
+use boa_interner::Sym;
 
 impl ByteCompiler<'_, '_> {
     /// Compile a [`Break`] `boa_ast` node
     pub(crate) fn compile_break(&mut self, node: Break) -> JsResult<()> {
-        let next = self.next_opcode_location();
         if let Some(info) = self.jump_info.last().filter(|info| info.is_try_block()) {
-            let in_finally = if let Some(finally_start) = info.finally_start() {
-                next >= finally_start.index
-            } else {
-                false
-            };
-            let in_catch_no_finally = !info.has_finally() && info.in_catch();
+            let in_finally = info.in_finally();
+            let in_catch_no_finally = info.in_catch() && !info.has_finally();
+            let has_finally_or_is_finally = info.has_finally() || info.in_finally();
 
             if in_finally {
                 self.emit_opcode(Opcode::PopIfThrown);
             }
             if in_finally || in_catch_no_finally {
                 self.emit_opcode(Opcode::CatchEnd2);
-            } else {
-                self.emit_opcode(Opcode::TryEnd);
             }
-            self.emit(Opcode::FinallySetJump, &[u32::MAX]);
-        }
-        let (break_label, envs_to_pop) = self.emit_opcode_with_two_operands(Opcode::Break);
-        if let Some(label_name) = node.label() {
-            let mut found = false;
-            let mut total_envs: u32 = 0;
-            for info in self.jump_info.iter_mut().rev() {
-                total_envs += info.decl_envs();
-                if info.label() == Some(label_name) {
-                    info.push_break_label(break_label);
-                    found = true;
-                    break;
+
+            let (break_label, target_jump_label) =
+                self.emit_opcode_with_two_operands(Opcode::Break);
+
+            if let Some(node_label) = node.label() {
+                self.search_jump_info_label(target_jump_label, node_label)?;
+
+                if !has_finally_or_is_finally {
+                    self.search_jump_info_label(break_label, node_label)?;
+                    return Ok(());
                 }
+            } else {
+                self.jump_info
+                    .last_mut()
+                    .expect("jump_info must exist to reach this point")
+                    .push_set_jumps(target_jump_label);
             }
-            // TODO: promote to an early error.
-            if !found {
-                return Err(JsNativeError::syntax()
-                    .with_message(format!(
-                        "Cannot use the undeclared label '{}'",
-                        self.interner().resolve_expect(label_name)
-                    ))
-                    .into());
-            }
-            self.patch_jump_with_target(envs_to_pop, total_envs);
-        } else {
-            let envs = self
+
+            let info = self
                 .jump_info
-                .last()
-                // TODO: promote to an early error.
-                .ok_or_else(|| {
-                    JsNativeError::syntax()
-                        .with_message("unlabeled break must be inside loop or switch")
-                })?
-                .decl_envs();
+                .last_mut()
+                // TODO: Currently would allow unlabelled breaks in try_blocks with no
+                // loops or switch. This can prevented by the early error referenced in line 66.
+                .expect("This try block must exist");
 
-            self.patch_jump_with_target(envs_to_pop, envs);
+            info.push_break_label(break_label);
 
-            self.jump_info
-                .last_mut()
-                .expect("cannot throw error as last access would have thrown")
-                .push_break_label(break_label);
+            return Ok(());
+        }
+
+        // Emit the break opcode -> (Label, Label)
+        let (break_label, target_label) = self.emit_opcode_with_two_operands(Opcode::Break);
+        if node.label().is_some() {
+            self.search_jump_info_label(
+                break_label,
+                node.label().expect("must exist in this block"),
+            )?;
+            self.search_jump_info_label(target_label, node.label().expect("must exist"))?;
+            return Ok(());
+        };
+
+        let info = self
+            .jump_info
+            .last_mut()
+            // TODO: promote to an early error.
+            .ok_or_else(|| {
+                JsNativeError::syntax()
+                    .with_message("unlabeled break must be inside loop or switch")
+            })?;
+
+        info.push_break_label(break_label);
+        info.push_break_label(target_label);
+
+        Ok(())
+    }
+
+    fn search_jump_info_label(&mut self, address: Label, node_label: Sym) -> JsResult<()> {
+        let mut found = false;
+        for info in self.jump_info.iter_mut().rev() {
+            if info.label() == Some(node_label) {
+                info.push_break_label(address);
+                found = true;
+                break;
+            }
+        }
+        // TODO: promote to an early error.
+        if !found {
+            return Err(JsNativeError::syntax()
+                .with_message(format!(
+                    "Cannot use the undeclared label '{}'",
+                    self.interner().resolve_expect(node_label)
+                ))
+                .into());
         }
 
         Ok(())

boa_engine/src/bytecompiler/statement/continue.rs

@@ -4,78 +4,157 @@ use crate::{bytecompiler::ByteCompiler, vm::Opcode, JsNativeError, JsResult};
 
 impl ByteCompiler<'_, '_> {
     pub(crate) fn compile_continue(&mut self, node: Continue) -> JsResult<()> {
-        let next = self.next_opcode_location();
         if let Some(info) = self.jump_info.last().filter(|info| info.is_try_block()) {
-            let start_address = info.start_address();
-            let in_finally = if let Some(finally_start) = info.finally_start() {
-                next > finally_start.index
-            } else {
-                false
-            };
+            let in_finally = info.in_finally();
+            let in_finally_or_has_finally = in_finally || info.has_finally();
             let in_catch_no_finally = !info.has_finally() && info.in_catch();
 
             if in_finally {
                 self.emit_opcode(Opcode::PopIfThrown);
             }
             if in_finally || in_catch_no_finally {
                 self.emit_opcode(Opcode::CatchEnd2);
-            } else {
-                self.emit_opcode(Opcode::TryEnd);
             }
-
-            self.emit(Opcode::FinallySetJump, &[start_address]);
-
-            let label = self.jump();
-            self.jump_info
-                .last_mut()
-                .expect("no jump information found")
-                .push_try_continue_label(label);
-        } else {
-            let mut items = self.jump_info.iter().rev().filter(|info| info.is_loop());
-            let address = if let Some(label_name) = node.label() {
-                let mut num_loops = 0;
-                let mut emit_for_of_in_exit = 0;
-                let mut address_info = None;
+            // 1. Handle if node has a label.
+            if let Some(node_label) = node.label() {
+                let items = self.jump_info.iter().rev().filter(|info| info.is_loop());
+                let mut emit_for_of_in_exit = 0_u32;
+                let mut loop_info = None;
                 for info in items {
-                    if info.label() == node.label() {
-                        address_info = Some(info);
+                    if info.label() == Some(node_label) {
+                        loop_info = Some(info);
                         break;
                     }
-                    num_loops += 1;
+
                     if info.for_of_in_loop() {
                         emit_for_of_in_exit += 1;
                     }
                 }
+
                 // TODO: promote to an early error.
-                let address = address_info
-                    .ok_or_else(|| {
-                        JsNativeError::syntax().with_message(format!(
-                            "Cannot use the undeclared label '{}'",
-                            self.context.interner().resolve_expect(label_name)
-                        ))
-                    })?
-                    .start_address();
+                loop_info.ok_or_else(|| {
+                    JsNativeError::syntax().with_message(format!(
+                        "Cannot use the undeclared label '{}'",
+                        self.context.interner().resolve_expect(node_label)
+                    ))
+                })?;
+
                 for _ in 0..emit_for_of_in_exit {
                     self.emit_opcode(Opcode::Pop);
                     self.emit_opcode(Opcode::Pop);
                     self.emit_opcode(Opcode::Pop);
                 }
-                for _ in 0..num_loops {
-                    self.emit_opcode(Opcode::LoopEnd);
+
+                let (cont_label, set_label) = self.emit_opcode_with_two_operands(Opcode::Continue);
+
+                let loops = self
+                    .jump_info
+                    .iter_mut()
+                    .rev()
+                    .filter(|info| info.is_loop());
+                let mut set_continue_as_break = false;
+                for info in loops {
+                    let found_label = info.label() == Some(node_label);
+                    if found_label && in_finally_or_has_finally {
+                        set_continue_as_break = true;
+                        info.push_try_continue_label(set_label);
+                        break;
+                    } else if found_label && !in_finally_or_has_finally {
+                        info.push_try_continue_label(cont_label);
+                        info.push_try_continue_label(set_label);
+                        break;
+                    }
+                }
+                if set_continue_as_break {
+                    self.jump_info
+                        .last_mut()
+                        .expect("no jump information found")
+                        .push_break_label(cont_label);
                 }
-                address
             } else {
-                items
+                // TODO: Add has finally or in finally here
+                let (cont_label, set_label) = self.emit_opcode_with_two_operands(Opcode::Continue);
+                if in_finally_or_has_finally {
+                    self.jump_info
+                        .last_mut()
+                        .expect("Must exist and be a try block")
+                        .push_break_label(cont_label);
+                };
+                let mut items = self
+                    .jump_info
+                    .iter_mut()
+                    .rev()
+                    .filter(|info| info.is_loop());
+                let jump_info = items
                     .next()
                     // TODO: promote to an early error.
                     .ok_or_else(|| {
                         JsNativeError::syntax().with_message("continue must be inside loop")
-                    })?
-                    .start_address()
+                    })?;
+                if !in_finally_or_has_finally {
+                    jump_info.push_try_continue_label(cont_label);
+                };
+                jump_info.push_try_continue_label(set_label);
             };
-            self.emit_opcode(Opcode::LoopEnd);
-            self.emit_opcode(Opcode::LoopStart);
-            self.emit(Opcode::Jump, &[address]);
+
+            return Ok(());
+        } else if let Some(node_label) = node.label() {
+            let items = self.jump_info.iter().rev().filter(|info| info.is_loop());
+            let mut emit_for_of_in_exit = 0_u32;
+            let mut loop_info = None;
+            for info in items {
+                if info.label() == Some(node_label) {
+                    loop_info = Some(info);
+                    break;
+                }
+
+                if info.for_of_in_loop() {
+                    emit_for_of_in_exit += 1;
+                }
+            }
+
+            // TODO: promote to an early error.
+            loop_info.ok_or_else(|| {
+                JsNativeError::syntax().with_message(format!(
+                    "Cannot use the undeclared label '{}'",
+                    self.context.interner().resolve_expect(node_label)
+                ))
+            })?;
+
+            for _ in 0..emit_for_of_in_exit {
+                self.emit_opcode(Opcode::Pop);
+                self.emit_opcode(Opcode::Pop);
+                self.emit_opcode(Opcode::Pop);
+            }
+
+            let (cont_label, set_label) = self.emit_opcode_with_two_operands(Opcode::Continue);
+            let loops = self
+                .jump_info
+                .iter_mut()
+                .rev()
+                .filter(|info| info.is_loop());
+
+            for info in loops {
+                if info.label() == Some(node_label) {
+                    info.push_try_continue_label(cont_label);
+                    info.push_try_continue_label(set_label);
+                }
+            }
+        } else {
+            let (cont_label, set_label) = self.emit_opcode_with_two_operands(Opcode::Continue);
+            let mut items = self
+                .jump_info
+                .iter_mut()
+                .rev()
+                .filter(|info| info.is_loop());
+            let jump_info = items
+                .next()
+                // TODO: promote to an early error.
+                .ok_or_else(|| {
+                    JsNativeError::syntax().with_message("continue must be inside loop")
+                })?;
+            jump_info.push_try_continue_label(cont_label);
+            jump_info.push_try_continue_label(set_label);
         }
 
         Ok(())

boa_engine/src/bytecompiler/statement/labelled.rs

@@ -5,6 +5,7 @@ use boa_ast::{
 
 use crate::{
     bytecompiler::{ByteCompiler, NodeKind},
+    vm::Opcode,
     JsResult,
 };
 
@@ -17,6 +18,7 @@ impl ByteCompiler<'_, '_> {
         configurable_globals: bool,
     ) -> JsResult<()> {
         let labelled_loc = self.next_opcode_location();
+        let end_label = self.emit_opcode_with_operand(Opcode::LabelledStart);
         self.push_labelled_control_info(labelled.label(), labelled_loc);
 
         match labelled.item() {
@@ -59,7 +61,10 @@ impl ByteCompiler<'_, '_> {
             }
         }
 
+        let labelled_end = self.next_opcode_location();
+        self.patch_jump_with_target(end_label, labelled_end);
         self.pop_labelled_control_info();
+        self.emit_opcode(Opcode::LabelledEnd);
 
         Ok(())
     }