Move return_value to VM

boa-dev · Aug 1, 2023 · facc473 · facc473
1 parent 04c8402
commit facc473
Show file tree

Hide file tree

Showing 11 changed files with 33 additions and 33 deletions.
diff --git a/boa_engine/src/builtins/eval/mod.rs b/boa_engine/src/builtins/eval/mod.rs
@@ -259,8 +259,6 @@ impl Eval {
             .push_frame(CallFrame::new(code_block).with_env_fp(env_fp));
         context.realm().resize_global_env();
 
-        // Push return value.
-        context.vm.push(JsValue::undefined());
         let record = context.run();
         context.vm.pop_frame();
 

diff --git a/boa_engine/src/builtins/json/mod.rs b/boa_engine/src/builtins/json/mod.rs
@@ -133,8 +133,6 @@ impl Json {
             .vm
             .push_frame(CallFrame::new(code_block).with_env_fp(env_fp));
         context.realm().resize_global_env();
-        // Push return value.
-        context.vm.push(JsValue::undefined());
         let record = context.run();
         context.vm.pop_frame();
 

diff --git a/boa_engine/src/bytecompiler/expression/mod.rs b/boa_engine/src/bytecompiler/expression/mod.rs
@@ -204,8 +204,7 @@ impl ByteCompiler<'_, '_> {
                     }
                     self.close_active_iterators();
 
-                    self.emit_opcode(Opcode::SetReturnValue);
-                    self.r#return();
+                    self.r#return(true);
 
                     self.patch_jump(throw_method_undefined);
                     self.iterator_close(self.in_async());

diff --git a/boa_engine/src/bytecompiler/function.rs b/boa_engine/src/bytecompiler/function.rs
@@ -136,6 +136,9 @@ impl FunctionCompiler {
             // ExecuteAsyncModule ( module ): <https://tc39.es/ecma262/#sec-execute-async-module>
             compiler.emit_opcode(Opcode::CreatePromiseCapability);
 
+            // Note: We set it to one so we don't pop return value when we return.
+            compiler.current_stack_value_count += 1;
+
             // 2. Let declResult be Completion(FunctionDeclarationInstantiation(functionObject, argumentsList)).
             //
             // Note: We push an exception handler so we catch exceptions that are thrown by the
@@ -159,6 +162,10 @@ impl FunctionCompiler {
         // Note: We do handle exceptions thrown by generator body in `AsyncGeneratorStart`.
         if compiler.in_generator() {
             assert!(compiler.async_handler.is_none());
+
+            // Note: We set it to one so we don't pop return value when we return.
+            compiler.current_stack_value_count += 1;
+
             if compiler.in_async() {
                 // Patched in `ByteCompiler::finish()`.
                 compiler.async_handler = Some(compiler.push_handler());

diff --git a/boa_engine/src/bytecompiler/jump_control.rs b/boa_engine/src/bytecompiler/jump_control.rs
@@ -66,7 +66,7 @@ pub(crate) enum JumpRecordAction {
 pub(crate) enum JumpRecordKind {
     Break,
     Continue,
-    Return,
+    Return { return_value_on_stack: bool },
 }
 
 /// This represents a local control flow handling. See [`JumpRecordKind`] for types.
@@ -122,7 +122,13 @@ impl JumpRecord {
         match self.kind {
             JumpRecordKind::Break => compiler.patch_jump(self.label),
             JumpRecordKind::Continue => compiler.patch_jump_with_target(self.label, start_address),
-            JumpRecordKind::Return => {
+            JumpRecordKind::Return {
+                return_value_on_stack,
+            } => {
+                if return_value_on_stack {
+                    compiler.emit_opcode(Opcode::SetReturnValue);
+                }
+
                 match (compiler.in_async(), compiler.in_generator()) {
                     // Taken from:
                     //  - 27.6.3.2 AsyncGeneratorStart ( generator, generatorBody ): https://tc39.es/ecma262/#sec-asyncgeneratorstart
@@ -137,6 +143,7 @@ impl JumpRecord {
                     (true, false) => compiler.emit_opcode(Opcode::CompletePromiseCapability),
                     (_, _) => {}
                 }
+
                 compiler.emit_opcode(Opcode::Return);
             }
         }

diff --git a/boa_engine/src/bytecompiler/mod.rs b/boa_engine/src/bytecompiler/mod.rs
@@ -301,9 +301,7 @@ impl<'ctx, 'host> ByteCompiler<'ctx, 'host> {
             compile_environments: Vec::default(),
             current_open_environments_count: 0,
 
-            // Note: We set it to one so we don't pop return value,
-            // which is allocated after frame pointer.
-            current_stack_value_count: 1,
+            current_stack_value_count: 0,
             code_block_flags,
             handlers: ThinVec::default(),
 
@@ -1422,7 +1420,7 @@ impl<'ctx, 'host> ByteCompiler<'ctx, 'host> {
         if let Some(async_handler) = self.async_handler {
             self.patch_handler(async_handler);
         }
-        self.r#return();
+        self.r#return(false);
 
         let name = self
             .context

diff --git a/boa_engine/src/bytecompiler/statement/mod.rs b/boa_engine/src/bytecompiler/statement/mod.rs
@@ -72,9 +72,8 @@ impl ByteCompiler<'_, '_> {
                 } else {
                     self.emit_opcode(Opcode::PushUndefined);
                 }
-                self.emit_opcode(Opcode::SetReturnValue);
 
-                self.r#return();
+                self.r#return(true);
             }
             Statement::Try(t) => self.compile_try(t, use_expr),
             Statement::Expression(expr) => {
@@ -88,10 +87,16 @@ impl ByteCompiler<'_, '_> {
         }
     }
 
-    pub(crate) fn r#return(&mut self) {
+    pub(crate) fn r#return(&mut self, return_value_on_stack: bool) {
         let actions = self.return_jump_record_actions();
 
-        JumpRecord::new(JumpRecordKind::Return, actions).perform_actions(Self::DUMMY_ADDRESS, self);
+        JumpRecord::new(
+            JumpRecordKind::Return {
+                return_value_on_stack,
+            },
+            actions,
+        )
+        .perform_actions(Self::DUMMY_ADDRESS, self);
     }
 
     fn return_jump_record_actions(&self) -> Vec<JumpRecordAction> {

diff --git a/boa_engine/src/module/source.rs b/boa_engine/src/module/source.rs
@@ -1763,9 +1763,6 @@ impl SourceTextModule {
         // 10. Else,
         //    a. Assert: capability is a PromiseCapability Record.
         //    b. Perform AsyncBlockStart(capability, module.[[ECMAScriptCode]], moduleContext).
-
-        // Push return value.
-        context.vm.push(JsValue::undefined());
         let result = context.run();
 
         std::mem::swap(&mut context.vm.environments, &mut environments);

diff --git a/boa_engine/src/script.rs b/boa_engine/src/script.rs
@@ -150,9 +150,6 @@ impl Script {
 
         // TODO: Here should be https://tc39.es/ecma262/#sec-globaldeclarationinstantiation
 
-        // Push return value.
-        context.vm.push(JsValue::undefined());
-
         self.realm().resize_global_env();
         let record = context.run();
         context.vm.pop_frame();

diff --git a/boa_engine/src/vm/code_block.rs b/boa_engine/src/vm/code_block.rs
@@ -1241,9 +1241,6 @@ impl JsObject {
 
         context.vm.push_frame(frame);
 
-        // Push return value.
-        context.vm.push(JsValue::undefined());
-
         // Push function arguments to the stack.
         for _ in argument_count..parameters_count {
             context.vm.push(JsValue::undefined());
@@ -1443,9 +1440,6 @@ impl JsObject {
                         .with_env_fp(environments_len as u32),
                 );
 
-                // Push return value.
-                context.vm.push(JsValue::undefined());
-
                 // Push function arguments to the stack.
                 for _ in argument_count..parameters_count {
                     context.vm.push(JsValue::undefined());

diff --git a/boa_engine/src/vm/mod.rs b/boa_engine/src/vm/mod.rs
@@ -53,6 +53,7 @@ mod tests;
 pub struct Vm {
     pub(crate) frames: Vec<CallFrame>,
     pub(crate) stack: Vec<JsValue>,
+    pub(crate) return_value: JsValue,
 
     /// When an error is thrown, the pending exception is set.
     ///
@@ -94,6 +95,7 @@ impl Vm {
         Self {
             frames: Vec::with_capacity(16),
             stack: Vec::with_capacity(1024),
+            return_value: JsValue::undefined(),
             environments: EnvironmentStack::new(global),
             pending_exception: None,
             runtime_limits: RuntimeLimits::default(),
@@ -183,13 +185,11 @@ impl Vm {
     }
 
     pub(crate) fn get_return_value(&self) -> JsValue {
-        let fp = self.frame().fp;
-        self.stack[fp as usize].clone()
+        self.return_value.clone()
     }
 
     pub(crate) fn set_return_value(&mut self, value: JsValue) {
-        let fp = self.frame().fp;
-        self.stack[fp as usize] = value;
+        self.return_value = value;
     }
 }
 
@@ -338,8 +338,8 @@ impl Context<'_> {
             match result {
                 Ok(CompletionType::Normal) => {}
                 Ok(CompletionType::Return) => {
-                    let execution_result = self.vm.get_return_value();
                     self.vm.stack.truncate(self.vm.frame().fp as usize);
+                    let execution_result = std::mem::take(&mut self.vm.return_value);
                     return CompletionRecord::Normal(execution_result);
                 }
                 Ok(CompletionType::Throw) => {