Refactor environment, exception handling and jumping in VM (#3059)

* Refactor environment, exception handling, generators and jumping in VM

* Add helper for JumpIfNotResumeKind opcode

* Better handler display for CodeBlock

* Update documentaion

* Factor exception handling from throw opcodes

* Simplify try compilation

* Only push try statement jump controll info if needed

* Add helper functions checks in async and generator

* Run prettier

* Remove redundant check for end of bytecode.

We always emit a `Return` opcode at the end of a function, so this should never be reached.

* Fix doc link

* Implement stack_count calculation of handlers

* Fix typo

* Rename `LoopContinue` to `IncrementLoopIteration`

* Fix typo

* Remove #[allow(unused)] from Handler field

boa_engine/src/builtins/eval/mod.rs

@@ -253,7 +253,10 @@ impl Eval {
             context.vm.environments.extend_outer_function_environment();
         }
 
-        context.vm.push_frame(CallFrame::new(code_block));
+        let env_fp = context.vm.environments.len() as u32;
+        context
+            .vm
+            .push_frame(CallFrame::new(code_block).with_env_fp(env_fp));
         context.realm().resize_global_env();
         let record = context.run();
         context.vm.pop_frame();

boa_engine/src/builtins/generator/mod.rs

@@ -108,10 +108,11 @@ impl GeneratorContext {
         context
             .vm
             .push_frame(self.call_frame.take().expect("should have a call frame"));
-        context.vm.frame_mut().generator_resume_kind = resume_kind;
+
         if let Some(value) = value {
             context.vm.push(value);
         }
+        context.vm.push(resume_kind);
 
         let result = context.run();
 

boa_engine/src/builtins/json/mod.rs

@@ -128,7 +128,10 @@ impl Json {
             Gc::new(compiler.finish())
         };
 
-        context.vm.push_frame(CallFrame::new(code_block));
+        let env_fp = context.vm.environments.len() as u32;
+        context
+            .vm
+            .push_frame(CallFrame::new(code_block).with_env_fp(env_fp));
         context.realm().resize_global_env();
         let record = context.run();
         context.vm.pop_frame();

boa_engine/src/bytecompiler/class.rs

@@ -69,14 +69,12 @@ impl ByteCompiler<'_, '_> {
             compiler.pop_compile_environment();
             compiler.code_block_flags |= CodeBlockFlags::IS_CLASS_CONSTRUCTOR;
         }
+        compiler.emit_opcode(Opcode::SetReturnValue);
 
         if class.name().is_some() && class.has_binding_identifier() {
             compiler.pop_compile_environment();
         }
 
-        compiler.emit_opcode(Opcode::SetReturnValue);
-        compiler.emit_opcode(Opcode::Return);
-
         let code = Gc::new(compiler.finish());
         let index = self.functions.len() as u32;
         self.functions.push(code);
@@ -268,12 +266,11 @@ impl ByteCompiler<'_, '_> {
                     } else {
                         field_compiler.emit_opcode(Opcode::PushUndefined);
                     }
+                    field_compiler.emit_opcode(Opcode::SetReturnValue);
+
                     field_compiler.pop_compile_environment();
                     field_compiler.pop_compile_environment();
 
-                    field_compiler.emit_opcode(Opcode::SetReturnValue);
-                    field_compiler.emit_opcode(Opcode::Return);
-
                     field_compiler.code_block_flags |= CodeBlockFlags::IN_CLASS_FIELD_INITIALIZER;
 
                     let code = field_compiler.finish();
@@ -306,7 +303,6 @@ impl ByteCompiler<'_, '_> {
                     field_compiler.pop_compile_environment();
 
                     field_compiler.emit_opcode(Opcode::SetReturnValue);
-                    field_compiler.emit_opcode(Opcode::Return);
 
                     field_compiler.code_block_flags |= CodeBlockFlags::IN_CLASS_FIELD_INITIALIZER;
 
@@ -346,12 +342,11 @@ impl ByteCompiler<'_, '_> {
                     } else {
                         field_compiler.emit_opcode(Opcode::PushUndefined);
                     }
+                    field_compiler.emit_opcode(Opcode::SetReturnValue);
+
                     field_compiler.pop_compile_environment();
                     field_compiler.pop_compile_environment();
 
-                    field_compiler.emit_opcode(Opcode::SetReturnValue);
-                    field_compiler.emit_opcode(Opcode::Return);
-
                     field_compiler.code_block_flags |= CodeBlockFlags::IN_CLASS_FIELD_INITIALIZER;
 
                     let code = field_compiler.finish();

boa_engine/src/bytecompiler/declaration/declaration_pattern.rs

@@ -180,11 +180,39 @@ impl ByteCompiler<'_, '_> {
                 self.emit_opcode(Opcode::ValueNotNullOrUndefined);
                 self.emit_opcode(Opcode::GetIterator);
 
+                // TODO: maybe, refactor this to be more condensed.
+                let handler_index = self.push_handler();
                 for element in pattern.bindings() {
                     self.compile_array_pattern_element(element, def);
                 }
 
+                self.emit_opcode(Opcode::PushFalse);
+
+                let exit = self.jump();
+                self.patch_handler(handler_index);
+                self.emit_opcode(Opcode::Exception);
+                self.emit_opcode(Opcode::PushTrue);
+                self.patch_jump(exit);
+
+                self.current_stack_value_count += 2;
+
+                let iterator_close_handler = self.push_handler();
                 self.iterator_close(false);
+
+                let exit = self.jump();
+                self.patch_handler(iterator_close_handler);
+                self.current_stack_value_count -= 2;
+                {
+                    let jump = self.jump_if_false();
+                    self.emit_opcode(Opcode::Throw);
+                    self.patch_jump(jump);
+                }
+                self.emit_opcode(Opcode::ReThrow);
+                self.patch_jump(exit);
+
+                let jump = self.jump_if_false();
+                self.emit_opcode(Opcode::Throw);
+                self.patch_jump(jump);
             }
         }
     }
@@ -198,15 +226,15 @@ impl ByteCompiler<'_, '_> {
         match element {
             // ArrayBindingPattern : [ Elision ]
             Elision => {
-                self.emit_opcode(Opcode::IteratorNext);
+                self.emit_opcode(Opcode::IteratorNextWithoutPop);
             }
             // SingleNameBinding : BindingIdentifier Initializer[opt]
             SingleName {
                 ident,
                 default_init,
             } => {
-                self.emit_opcode(Opcode::IteratorNext);
-                self.emit_opcode(Opcode::IteratorValue);
+                self.emit_opcode(Opcode::IteratorNextWithoutPop);
+                self.emit_opcode(Opcode::IteratorValueWithoutPop);
                 if let Some(init) = default_init {
                     let skip = self.emit_opcode_with_operand(Opcode::JumpIfNotUndefined);
                     self.compile_expr(init, true);
@@ -216,17 +244,17 @@ impl ByteCompiler<'_, '_> {
             }
             PropertyAccess { access } => {
                 self.access_set(Access::Property { access }, false, |compiler, _level| {
-                    compiler.emit_opcode(Opcode::IteratorNext);
-                    compiler.emit_opcode(Opcode::IteratorValue);
+                    compiler.emit_opcode(Opcode::IteratorNextWithoutPop);
+                    compiler.emit_opcode(Opcode::IteratorValueWithoutPop);
                 });
             }
             // BindingElement : BindingPattern Initializer[opt]
             Pattern {
                 pattern,
                 default_init,
             } => {
-                self.emit_opcode(Opcode::IteratorNext);
-                self.emit_opcode(Opcode::IteratorValue);
+                self.emit_opcode(Opcode::IteratorNextWithoutPop);
+                self.emit_opcode(Opcode::IteratorValueWithoutPop);
 
                 if let Some(init) = default_init {
                     let skip = self.emit_opcode_with_operand(Opcode::JumpIfNotUndefined);

boa_engine/src/bytecompiler/declarations.rs

@@ -1015,6 +1015,7 @@ impl ByteCompiler<'_, '_> {
             // Don't need to use `AsyncGeneratorYield` since
             // we just want to stop the execution of the generator.
             self.emit_opcode(Opcode::GeneratorYield);
+            self.emit_opcode(Opcode::Pop);
         }
 
         // 27. If hasParameterExpressions is false, then

boa_engine/src/bytecompiler/env.rs

@@ -7,6 +7,7 @@ use boa_ast::expression::Identifier;
 impl ByteCompiler<'_, '_> {
     /// Push either a new declarative or function environment on the compile time environment stack.
     pub(crate) fn push_compile_environment(&mut self, function_scope: bool) {
+        self.current_open_environments_count += 1;
         self.current_environment = Rc::new(CompileTimeEnvironment::new(
             self.current_environment.clone(),
             function_scope,
@@ -16,6 +17,7 @@ impl ByteCompiler<'_, '_> {
     /// Pops the top compile time environment and returns its index in the compile time environments array.
     #[track_caller]
     pub(crate) fn pop_compile_environment(&mut self) -> u32 {
+        self.current_open_environments_count -= 1;
         let index = self.compile_environments.len() as u32;
         self.compile_environments
             .push(self.current_environment.clone());

boa_engine/src/bytecompiler/expression/mod.rs

@@ -7,7 +7,7 @@ mod update;
 use super::{Access, Callable, NodeKind};
 use crate::{
     bytecompiler::{ByteCompiler, Literal},
-    vm::Opcode,
+    vm::{GeneratorResumeKind, Opcode},
 };
 use boa_ast::{
     expression::{
@@ -153,31 +153,41 @@ impl ByteCompiler<'_, '_> {
                 }
             }
             Expression::Yield(r#yield) => {
+                // stack:
                 if let Some(expr) = r#yield.target() {
                     self.compile_expr(expr, true);
                 } else {
                     self.emit_opcode(Opcode::PushUndefined);
                 }
 
+                // stack: value
+
                 if r#yield.delegate() {
-                    if self.in_async_generator {
+                    if self.in_async() {
                         self.emit_opcode(Opcode::GetAsyncIterator);
                     } else {
                         self.emit_opcode(Opcode::GetIterator);
                     }
 
+                    // stack:
                     self.emit_opcode(Opcode::PushUndefined);
+
+                    // stack: undefined
+                    self.emit_push_integer(GeneratorResumeKind::Normal as i32);
+
+                    // stack: resume_kind, undefined
                     let start_address = self.next_opcode_location();
                     let (throw_method_undefined, return_method_undefined) =
                         self.emit_opcode_with_two_operands(Opcode::GeneratorDelegateNext);
 
-                    if self.in_async_generator {
+                    if self.in_async() {
+                        self.emit_opcode(Opcode::Pop);
                         self.emit_opcode(Opcode::Await);
                     }
 
                     let (return_gen, exit) =
                         self.emit_opcode_with_two_operands(Opcode::GeneratorDelegateResume);
-                    if self.in_async_generator {
+                    if self.in_async() {
                         self.emit_opcode(Opcode::IteratorValue);
                         self.async_generator_yield();
                     } else {
@@ -188,16 +198,17 @@ impl ByteCompiler<'_, '_> {
 
                     self.patch_jump(return_gen);
                     self.patch_jump(return_method_undefined);
-                    if self.in_async_generator {
+                    if self.in_async() {
                         self.emit_opcode(Opcode::Await);
+                        self.emit_opcode(Opcode::Pop);
                     }
                     self.close_active_iterators();
 
                     self.emit_opcode(Opcode::SetReturnValue);
-                    self.emit_opcode(Opcode::GeneratorResumeReturn);
+                    self.r#return();
 
                     self.patch_jump(throw_method_undefined);
-                    self.iterator_close(self.in_async_generator);
+                    self.iterator_close(self.in_async());
                     self.emit_opcode(Opcode::Throw);
 
                     self.patch_jump(exit);

boa_engine/src/bytecompiler/function.rs

@@ -4,7 +4,7 @@ use crate::{
     builtins::function::ThisMode,
     bytecompiler::ByteCompiler,
     environments::CompileTimeEnvironment,
-    vm::{CodeBlock, CodeBlockFlags, Opcode},
+    vm::{CodeBlock, CodeBlockFlags},
     Context,
 };
 use boa_ast::function::{FormalParameterList, FunctionBody};
@@ -99,7 +99,8 @@ impl FunctionCompiler {
 
         let mut compiler = ByteCompiler::new(self.name, self.strict, false, outer_env, context);
         compiler.length = length;
-        compiler.in_async_generator = self.generator && self.r#async;
+        compiler.in_async = self.r#async;
+        compiler.in_generator = self.generator;
 
         if self.arrow {
             compiler.this_mode = ThisMode::Lexical;
@@ -151,15 +152,6 @@ impl FunctionCompiler {
 
         compiler.params = parameters.clone();
 
-        if compiler
-            .bytecode
-            .last()
-            .filter(|last| **last == Opcode::Return as u8)
-            .is_none()
-        {
-            compiler.emit_opcode(Opcode::Return);
-        }
-
         Gc::new(compiler.finish())
     }
 }