Apply review

boa_engine/src/builtins/eval/mod.rs

@@ -18,7 +18,7 @@ use crate::{
     object::JsObject,
     realm::Realm,
     string::common::StaticJsStrings,
-    vm::{CallFrame, Opcode},
+    vm::{CallFrame, CallFrameFlags, Opcode},
     Context, JsArgs, JsResult, JsString, JsValue,
 };
 use boa_ast::operations::{contains, contains_arguments, ContainsSymbol};
@@ -258,9 +258,11 @@ impl Eval {
         let env_fp = context.vm.environments.len() as u32;
         let environments = context.vm.environments.clone();
         let realm = context.realm().clone();
-        context
-            .vm
-            .push_frame(CallFrame::new(code_block, None, environments, realm).with_env_fp(env_fp));
+        context.vm.push_frame(
+            CallFrame::new(code_block, None, environments, realm)
+                .with_env_fp(env_fp)
+                .with_flags(CallFrameFlags::EXIT_EARLY),
+        );
 
         context.vm.push(JsValue::undefined()); // Push `this` value.
         context.vm.push(JsValue::undefined()); // No function object, so push undefined.

boa_engine/src/builtins/generator/mod.rs

@@ -101,7 +101,7 @@ impl GeneratorContext {
             .push_frame(self.call_frame.take().expect("should have a call frame"));
 
         context.vm.frame_mut().fp = 0;
-        context.vm.frame_mut().exit_early = true;
+        context.vm.frame_mut().set_exit_early(true);
 
         if let Some(value) = value {
             context.vm.push(value);

boa_engine/src/builtins/json/mod.rs

@@ -29,7 +29,7 @@ use crate::{
     string::{common::StaticJsStrings, utf16, CodePoint},
     symbol::JsSymbol,
     value::IntegerOrInfinity,
-    vm::CallFrame,
+    vm::{CallFrame, CallFrameFlags},
     Context, JsArgs, JsResult, JsString, JsValue,
 };
 use boa_gc::Gc;
@@ -134,7 +134,8 @@ impl Json {
         let env_fp = context.vm.environments.len() as u32;
         context.vm.push_frame(
             CallFrame::new(code_block, None, context.vm.environments.clone(), realm)
-                .with_env_fp(env_fp),
+                .with_env_fp(env_fp)
+                .with_flags(CallFrameFlags::EXIT_EARLY),
         );
 
         context.vm.push(JsValue::undefined()); // Push `this` value.

boa_engine/src/module/source.rs

@@ -29,7 +29,7 @@ use crate::{
     realm::Realm,
     vm::{
         create_function_object_fast, create_generator_function_object, ActiveRunnable, CallFrame,
-        CodeBlock, CompletionRecord, Opcode,
+        CallFrameFlags, CodeBlock, CompletionRecord, Opcode,
     },
     Context, JsArgs, JsError, JsNativeError, JsObject, JsResult, JsString, JsValue, NativeFunction,
 };
@@ -1738,7 +1738,8 @@ impl SourceTextModule {
             environments,
             realm,
         )
-        .with_env_fp(env_fp);
+        .with_env_fp(env_fp)
+        .with_flags(CallFrameFlags::EXIT_EARLY);
         callframe.promise_capability = capability;
 
         // 8. Suspend the running execution context.

boa_engine/src/object/internal_methods/function.rs

@@ -6,7 +6,7 @@ use crate::{
         internal_methods::{InternalObjectMethods, ORDINARY_INTERNAL_METHODS},
         JsObject, ObjectData, ObjectKind,
     },
-    vm::CallFrame,
+    vm::{CallFrame, CallFrameFlags},
     Context, JsNativeError, JsResult, JsValue,
 };
 
@@ -64,19 +64,16 @@ pub(crate) fn function_call(
 
     let env_fp = environments.len() as u32;
 
-    let mut frame = CallFrame::new(code.clone(), script_or_module, environments, realm)
+    let frame = CallFrame::new(code.clone(), script_or_module, environments, realm)
         .with_argument_count(argument_count as u32)
         .with_env_fp(env_fp);
 
-    frame.exit_early = false;
-
     context.vm.push_frame(frame);
 
-    let at = context.vm.stack.len() - argument_count;
-
-    context.vm.frame_mut().fp = at as u32 - 2;
+    let fp = context.vm.stack.len() - argument_count - CallFrame::FUCNTION_PROLOGUE;
+    context.vm.frame_mut().fp = fp as u32;
 
-    let this = context.vm.stack[at - 2].clone();
+    let this = context.vm.stack[fp + CallFrame::THIS_POSITION].clone();
 
     let lexical_this_mode = code.this_mode == ThisMode::Lexical;
 
@@ -134,7 +131,7 @@ pub(crate) fn function_call(
         //              that don't have a rest parameter, any parameter
         //              default value initializers, or any destructured parameters.
         //     ii. Let ao be CreateMappedArgumentsObject(func, formals, argumentsList, env).
-        let args = context.vm.stack[at..].to_vec();
+        let args = context.vm.stack[(fp + CallFrame::FIRST_ARGUMENT_POSITION)..].to_vec();
         let arguments_obj = if code.strict() || !code.params.is_simple() {
             Arguments::create_unmapped_arguments_object(&args, context)
         } else {
@@ -213,12 +210,10 @@ fn function_construct(
         None
     };
 
-    let mut frame = CallFrame::new(code.clone(), script_or_module, environments, realm)
+    let frame = CallFrame::new(code.clone(), script_or_module, environments, realm)
         .with_argument_count(argument_count as u32)
-        .with_env_fp(env_fp);
-
-    frame.exit_early = false;
-    frame.construct = true;
+        .with_env_fp(env_fp)
+        .with_flags(CallFrameFlags::CONSTRUCT);
 
     context.vm.push_frame(frame);
 

boa_engine/src/object/internal_methods/mod.rs

@@ -305,7 +305,7 @@ pub(crate) struct InternalObjectMethods {
 
 /// The return value of an internal method (`[[Call]]` or `[[Construct]]`).
 ///
-/// This is done to avoid recusion.
+/// This is done to avoid recursion.
 pub(crate) enum CallValue {
     /// Calling is ready, the frames have been setup.
     ///

boa_engine/src/object/operations.rs

@@ -335,7 +335,7 @@ impl JsObject {
             return Ok(context.vm.pop());
         }
 
-        context.vm.frames[frame_index].exit_early = true;
+        context.vm.frames[frame_index].set_exit_early(true);
 
         let result = context.run().consume();
 
@@ -385,7 +385,7 @@ impl JsObject {
                 .clone());
         }
 
-        context.vm.frames[frame_index].exit_early = true;
+        context.vm.frames[frame_index].set_exit_early(true);
 
         let result = context.run().consume();
 

boa_engine/src/script.rs

@@ -19,7 +19,7 @@ use rustc_hash::FxHashMap;
 use crate::{
     bytecompiler::ByteCompiler,
     realm::Realm,
-    vm::{ActiveRunnable, CallFrame, CodeBlock},
+    vm::{ActiveRunnable, CallFrame, CallFrameFlags, CodeBlock},
     Context, HostDefined, JsResult, JsString, JsValue, Module,
 };
 
@@ -156,7 +156,8 @@ impl Script {
                 context.vm.environments.clone(),
                 self.inner.realm.clone(),
             )
-            .with_env_fp(env_fp),
+            .with_env_fp(env_fp)
+            .with_flags(CallFrameFlags::EXIT_EARLY),
         );
 
         context.vm.push(JsValue::undefined()); // Push `this` value.

boa_engine/src/vm/call_frame/mod.rs

@@ -15,6 +15,19 @@ use thin_vec::ThinVec;
 
 use super::{ActiveRunnable, Vm};
 
+bitflags::bitflags! {
+    /// Flags associated with a [`CallFrame`].
+    #[derive(Debug, Default, Clone, Copy)]
+    pub(crate) struct CallFrameFlags: u8 {
+        /// When we return from this [`CallFrame`] to stop execution and
+        /// return from [`crate::Context::run()`], and leave the remaining [`CallFrame`]s unchanged.
+        const EXIT_EARLY = 0b0000_0001;
+
+        /// Was this [`CallFrame`] created from the `__construct__()` internal object method?
+        const CONSTRUCT = 0b0000_0010;
+    }
+}
+
 /// A `CallFrame` holds the state of a function call.
 #[derive(Clone, Debug, Finalize, Trace)]
 pub struct CallFrame {
@@ -43,12 +56,15 @@ pub struct CallFrame {
     /// \[\[ScriptOrModule\]\]
     pub(crate) active_runnable: Option<ActiveRunnable>,
 
+    /// \[\[Environment\]\]
     pub(crate) environments: EnvironmentStack,
 
+    /// \[\[Realm\]\]
     pub(crate) realm: Realm,
 
-    pub(crate) exit_early: bool,
-    pub(crate) construct: bool,
+    // SAFETY: Nothing in `CallFrameFlags` requires tracing, so this is safe.
+    #[unsafe_ignore_trace]
+    pub(crate) flags: CallFrameFlags,
 }
 
 /// ---- `CallFrame` public API ----
@@ -63,8 +79,25 @@ impl CallFrame {
 
 /// ---- `CallFrame` creation methods ----
 impl CallFrame {
-    pub(crate) const THIS_POSITION: u32 = 0;
-    pub(crate) const FUNCTION_POSITION: u32 = 1;
+    /// This is the size of the function prologue.
+    ///
+    /// The position of the elements are relative to the [`CallFrame::fp`].
+    ///
+    /// ```text
+    ///                                                 arguments
+    ///   --- frame pointer                                ^
+    ///  /                      __________________________/
+    /// /                      /                          \
+    /// | 0: this | 1: func | 2: arg1 | ... | (2 + N): argN |
+    ///    \            /
+    ///     ------------
+    ///     |
+    /// function prolugue
+    /// ```
+    pub(crate) const FUCNTION_PROLOGUE: usize = 2;
+    pub(crate) const THIS_POSITION: usize = 0;
+    pub(crate) const FUNCTION_POSITION: usize = 1;
+    pub(crate) const FIRST_ARGUMENT_POSITION: usize = 2;
 
     /// Creates a new `CallFrame` with the provided `CodeBlock`.
     pub(crate) fn new(
@@ -87,8 +120,7 @@ impl CallFrame {
             active_runnable,
             environments,
             realm,
-            exit_early: true,
-            construct: false,
+            flags: CallFrameFlags::empty(),
         }
     }
 
@@ -104,19 +136,38 @@ impl CallFrame {
         self
     }
 
+    /// Updates a `CallFrame`'s `flags` field with the value provided.
+    pub(crate) fn with_flags(mut self, flags: CallFrameFlags) -> Self {
+        self.flags = flags;
+        self
+    }
+
     pub(crate) fn this(&self, vm: &Vm) -> JsValue {
-        let this_index = self.fp + Self::THIS_POSITION;
-        vm.stack[this_index as usize].clone()
+        let this_index = self.fp as usize + Self::THIS_POSITION;
+        vm.stack[this_index].clone()
     }
 
     pub(crate) fn function(&self, vm: &Vm) -> Option<JsObject> {
-        let function_index = self.fp + Self::FUNCTION_POSITION;
-        if let Some(object) = vm.stack[function_index as usize].as_object() {
+        let function_index = self.fp as usize + Self::FUNCTION_POSITION;
+        if let Some(object) = vm.stack[function_index].as_object() {
             return Some(object.clone());
         }
 
         None
     }
+
+    /// Does this have the [`CallFrameFlags::EXIT_EARLY`] flag.
+    pub(crate) fn exit_early(&self) -> bool {
+        self.flags.contains(CallFrameFlags::EXIT_EARLY)
+    }
+    /// Set the [`CallFrameFlags::EXIT_EARLY`] flag.
+    pub(crate) fn set_exit_early(&mut self, early_exit: bool) {
+        self.flags.set(CallFrameFlags::EXIT_EARLY, early_exit);
+    }
+    /// Does this have the [`CallFrameFlags::CONSTRUCT`] flag.
+    pub(crate) fn construct(&self) -> bool {
+        self.flags.contains(CallFrameFlags::CONSTRUCT)
+    }
 }
 
 /// ---- `CallFrame` stack methods ----

boa_engine/src/vm/mod.rs

@@ -38,6 +38,7 @@ pub use {
 };
 
 pub(crate) use {
+    call_frame::CallFrameFlags,
     code_block::{
         create_function_object, create_function_object_fast, create_generator_function_object,
         CodeBlockFlags, Handler,
@@ -403,7 +404,7 @@ impl Context<'_> {
                     self.vm.stack.truncate(self.vm.frame().fp as usize);
 
                     let result = self.vm.take_return_value();
-                    if self.vm.frame().exit_early {
+                    if self.vm.frame().exit_early() {
                         return CompletionRecord::Normal(result);
                     }
 
@@ -413,7 +414,7 @@ impl Context<'_> {
                 Ok(CompletionType::Throw) => {
                     self.vm.stack.truncate(self.vm.frame().fp as usize);
 
-                    if self.vm.frame().exit_early {
+                    if self.vm.frame().exit_early() {
                         return CompletionRecord::Throw(
                             self.vm
                                 .pending_exception
@@ -427,7 +428,7 @@ impl Context<'_> {
                     while let Some(frame) = self.vm.frames.last_mut() {
                         let pc = frame.pc;
                         let fp = frame.fp;
-                        let exit_early = frame.exit_early;
+                        let exit_early = frame.exit_early();
 
                         if self.vm.handle_exception_at(pc) {
                             continue 'instruction;
@@ -449,7 +450,7 @@ impl Context<'_> {
                 // Early return immediately.
                 Ok(CompletionType::Yield) => {
                     let result = self.vm.take_return_value();
-                    if self.vm.frame().exit_early {
+                    if self.vm.frame().exit_early() {
                         return CompletionRecord::Return(result);
                     }
 
@@ -473,7 +474,7 @@ impl Context<'_> {
                                 let mut fp = self.vm.stack.len();
                                 let mut env_fp = self.vm.environments.len();
                                 while let Some(frame) = self.vm.frames.last() {
-                                    if frame.exit_early {
+                                    if frame.exit_early() {
                                         break;
                                     }
 
@@ -496,7 +497,7 @@ impl Context<'_> {
                         continue;
                     }
 
-                    if self.vm.frame().exit_early {
+                    if self.vm.frame().exit_early() {
                         self.vm
                             .environments
                             .truncate(self.vm.frame().env_fp as usize);
@@ -513,7 +514,7 @@ impl Context<'_> {
                         let pc = frame.pc;
                         let fp = frame.fp;
                         let env_fp = frame.env_fp;
-                        let exit_early = frame.exit_early;
+                        let exit_early = frame.exit_early();
 
                         if self.vm.handle_exception_at(pc) {
                             self.vm.pending_exception = Some(err);

boa_engine/src/vm/opcode/control_flow/return.rs

@@ -31,7 +31,7 @@ impl Operation for CheckReturn {
     const INSTRUCTION: &'static str = "INST - CheckReturn";
 
     fn execute(context: &mut Context<'_>) -> JsResult<CompletionType> {
-        if !context.vm.frame().construct {
+        if !context.vm.frame().construct() {
             return Ok(CompletionType::Normal);
         }
         let frame = context.vm.frame();

boa_engine/src/vm/opcode/generator/mod.rs

@@ -48,7 +48,10 @@ impl Operation for Generator {
         dummy_call_frame.pc = pc;
         let mut call_frame = std::mem::replace(context.vm.frame_mut(), dummy_call_frame);
 
-        context.vm.frame_mut().exit_early = call_frame.exit_early;
+        context
+            .vm
+            .frame_mut()
+            .set_exit_early(call_frame.exit_early());
 
         call_frame.environments = context.vm.environments.clone();
         call_frame.realm = context.realm().clone();