Apply review and some clean-up

core/engine/src/builtins/function/mod.rs

@@ -1035,12 +1035,12 @@ fn function_construct(
         .with_env_fp(env_fp)
         .with_flags(CallFrameFlags::CONSTRUCT);
 
-    context.vm.push_frame(frame);
-
     let len = context.vm.stack.len();
 
+    context.vm.push_frame(frame);
+
     // NOTE(HalidOdat): +1 because we insert `this` value below.
-    context.vm.frame_mut().fp = len as u32 + 1;
+    context.vm.frame_mut().rp = len as u32 + 1;
 
     let mut last_env = 0;
 

core/engine/src/builtins/generator/mod.rs

@@ -69,13 +69,13 @@ impl GeneratorContext {
         let mut frame = context.vm.frame().clone();
         frame.environments = context.vm.environments.clone();
         frame.realm = context.realm().clone();
-        let fp = frame.restore_fp() as usize;
+        let fp = frame.fp() as usize;
         let stack = context.vm.stack.split_off(fp);
 
-        frame.fp = CallFrame::FUNCTION_PROLOGUE + frame.argument_count;
+        frame.rp = CallFrame::FUNCTION_PROLOGUE + frame.argument_count;
 
         // NOTE: Since we get a pre-built call frame with stack, and we reuse them.
-        //       So we don't need to push the locals in subsuquent calls.
+        //       So we don't need to push the locals in subsequent calls.
         frame.flags |= CallFrameFlags::LOCALS_ALREADY_PUSHED;
 
         Self {
@@ -93,10 +93,10 @@ impl GeneratorContext {
     ) -> CompletionRecord {
         std::mem::swap(&mut context.vm.stack, &mut self.stack);
         let frame = self.call_frame.take().expect("should have a call frame");
-        let fp = frame.fp;
+        let rp = frame.rp;
         context.vm.push_frame(frame);
 
-        context.vm.frame_mut().fp = fp;
+        context.vm.frame_mut().rp = rp;
         context.vm.frame_mut().set_exit_early(true);
 
         if let Some(value) = value {

core/engine/src/vm/call_frame/mod.rs

@@ -36,11 +36,10 @@ bitflags::bitflags! {
 pub struct CallFrame {
     pub(crate) code_block: Gc<CodeBlock>,
     pub(crate) pc: u32,
-    pub(crate) fp: u32,
-    pub(crate) env_fp: u32,
-    // Tracks the number of environments in environment entry.
-    // On abrupt returns this is used to decide how many environments need to be pop'ed.
+    /// The register pointer, points to the first register in the stack.
+    pub(crate) rp: u32,
     pub(crate) argument_count: u32,
+    pub(crate) env_fp: u32,
     pub(crate) promise_capability: Option<PromiseCapability>,
 
     // Iterators and their `[[Done]]` flags that must be closed when an abrupt completion is thrown.
@@ -80,16 +79,18 @@ impl CallFrame {
 impl CallFrame {
     /// This is the size of the function prologue.
     ///
-    /// The position of the elements are relative to the [`CallFrame::fp`] (frame pointer).
+    /// The position of the elements are relative to the [`CallFrame::fp`] (register pointer).
     ///
     /// ```text
     ///                      Setup by the caller
-    ///   ┌─────────────────────────────────────────────────────────┐ ┌───── frame pointer
+    ///   ┌─────────────────────────────────────────────────────────┐ ┌───── register pointer
     ///   ▼                                                         ▼ ▼
     /// | -(2 + N): this | -(1 + N): func | -N: arg1 | ... | -1: argN | 0: local1 | ... | K: localK |
     ///   ▲                              ▲   ▲                      ▲   ▲                         ▲
     ///   └──────────────────────────────┘   └──────────────────────┘   └─────────────────────────┘
-    ///         function prolugue                    arguments              Setup by the callee
+    ///         function prologue                    arguments              Setup by the callee
+    ///   ▲
+    ///   └─ Frame pointer
     /// ```
     ///
     /// ### Example
@@ -107,17 +108,22 @@ impl CallFrame {
     /// ```
     ///
     /// ```text
-    ///   caller prolugue     caller arguments   callee prolugue   callee arguments
-    ///   ______|____________   _____|_____   _________|_________   __|_
-    ///  /                   \ /           \ /                   \ /    \
-    /// | 0: undefined | 1: y | 2: 1 | 3: 2 | 4: undefined | 5: x | 6: 3 |
-    ///                                     /                            ^
-    ///            caller frame pointer  --+                  callee frame pointer
+    ///     caller prologue    caller arguments   callee prologue   callee arguments
+    ///   ┌─────────────────┐   ┌─────────┐   ┌─────────────────┐  ┌──────┐
+    ///   ▼                 ▼   ▼         ▼   │                 ▼  ▼      ▼
+    /// | 0: undefined | 1: y | 2: 1 | 3: 2 | 4: undefined | 5: x | 6:  3  |
+    /// ▲                                   ▲                            ▲
+    /// │       caller register pointer ────┤                            │
+    /// │                                   │                callee register pointer
+    /// │                             callee frame pointer
+    /// │
+    /// └─────  caller frame pointer
+    ///
     /// ```
     ///
     /// Some questions:
     ///
-    /// - Who is responsible for cleaning up the stack after a call? The caller.
+    /// - Who is responsible for cleaning up the stack after a call? The rust caller.
     ///
     pub(crate) const FUNCTION_PROLOGUE: u32 = 2;
     pub(crate) const THIS_POSITION: u32 = 2;
@@ -134,7 +140,7 @@ impl CallFrame {
         Self {
             code_block,
             pc: 0,
-            fp: 0,
+            rp: 0,
             env_fp: 0,
             argument_count: 0,
             promise_capability: None,
@@ -167,12 +173,12 @@ impl CallFrame {
     }
 
     pub(crate) fn this(&self, vm: &Vm) -> JsValue {
-        let this_index = self.fp - self.argument_count - Self::THIS_POSITION;
+        let this_index = self.rp - self.argument_count - Self::THIS_POSITION;
         vm.stack[this_index as usize].clone()
     }
 
     pub(crate) fn function(&self, vm: &Vm) -> Option<JsObject> {
-        let function_index = self.fp - self.argument_count - Self::FUNCTION_POSITION;
+        let function_index = self.rp - self.argument_count - Self::FUNCTION_POSITION;
         if let Some(object) = vm.stack[function_index as usize].as_object() {
             return Some(object.clone());
         }
@@ -181,22 +187,22 @@ impl CallFrame {
     }
 
     pub(crate) fn arguments<'stack>(&self, vm: &'stack Vm) -> &'stack [JsValue] {
-        let fp = self.fp as usize;
+        let rp = self.rp as usize;
         let argument_count = self.argument_count as usize;
-        let arguments_start = fp - argument_count;
-        &vm.stack[arguments_start..fp]
+        let arguments_start = rp - argument_count;
+        &vm.stack[arguments_start..rp]
     }
 
     pub(crate) fn argument<'stack>(&self, index: usize, vm: &'stack Vm) -> Option<&'stack JsValue> {
         self.arguments(vm).get(index)
     }
 
-    pub(crate) fn restore_fp(&self) -> u32 {
-        self.fp - self.argument_count - Self::FUNCTION_PROLOGUE
+    pub(crate) fn fp(&self) -> u32 {
+        self.rp - self.argument_count - Self::FUNCTION_PROLOGUE
     }
 
     pub(crate) fn restore_stack(&self, vm: &mut Vm) {
-        let fp = self.restore_fp();
+        let fp = self.fp();
         vm.stack.truncate(fp as usize);
     }
 
@@ -218,7 +224,7 @@ impl CallFrame {
     /// If the index is out of bounds.
     pub(crate) fn local<'stack>(&self, index: u32, stack: &'stack [JsValue]) -> &'stack JsValue {
         debug_assert!(index < self.code_block().locals_count);
-        let at = self.fp + index;
+        let at = self.rp + index;
         &stack[at as usize]
     }
 
@@ -242,8 +248,8 @@ impl CallFrame {
 
 /// ---- `CallFrame` stack methods ----
 impl CallFrame {
-    pub(crate) fn set_frame_pointer(&mut self, pointer: u32) {
-        self.fp = pointer;
+    pub(crate) fn set_register_pointer(&mut self, pointer: u32) {
+        self.rp = pointer;
     }
 }
 

core/engine/src/vm/mod.rs

@@ -158,7 +158,7 @@ impl Vm {
 
     pub(crate) fn push_frame(&mut self, mut frame: CallFrame) {
         let current_stack_length = self.stack.len();
-        frame.set_frame_pointer(current_stack_length as u32);
+        frame.set_register_pointer(current_stack_length as u32);
         std::mem::swap(&mut self.environments, &mut frame.environments);
         std::mem::swap(&mut self.realm, &mut frame.realm);
 
@@ -210,7 +210,7 @@ impl Vm {
 
         let catch_address = handler.handler();
         let environment_sp = frame.env_fp + handler.environment_count;
-        let sp = frame.fp + handler.stack_count;
+        let sp = frame.rp + handler.stack_count;
 
         // Go to handler location.
         frame.pc = catch_address;
@@ -325,7 +325,12 @@ impl Context {
         let result = self.execute_instruction(f);
         let duration = instant.elapsed();
 
-        let fp = self.vm.frames.last().map(|frame| frame.fp as usize);
+        let fp = self
+            .vm
+            .frames
+            .last()
+            .map(CallFrame::fp)
+            .map(|fp| fp as usize);
 
         let stack = {
             let mut stack = String::from("[ ");
@@ -427,7 +432,7 @@ impl Context {
                             break;
                         }
 
-                        fp = frame.restore_fp() as usize;
+                        fp = frame.fp() as usize;
                         env_fp = frame.env_fp as usize;
                         self.vm.pop_frame();
                     }
@@ -454,8 +459,8 @@ impl Context {
         match result {
             CompletionType::Normal => {}
             CompletionType::Return => {
-                let restore_fp = self.vm.frame().restore_fp() as usize;
-                self.vm.stack.truncate(restore_fp);
+                let fp = self.vm.frame().fp() as usize;
+                self.vm.stack.truncate(fp);
 
                 let result = self.vm.take_return_value();
                 if self.vm.frame().exit_early() {
@@ -466,7 +471,7 @@ impl Context {
                 self.vm.pop_frame();
             }
             CompletionType::Throw => {
-                let mut fp = self.vm.frame().restore_fp();
+                let mut fp = self.vm.frame().fp();
                 let mut env_fp = self.vm.frame().env_fp;
                 if self.vm.frame().exit_early() {
                     self.vm.environments.truncate(env_fp as usize);
@@ -482,7 +487,7 @@ impl Context {
                 self.vm.pop_frame();
 
                 while let Some(frame) = self.vm.frames.last_mut() {
-                    fp = frame.restore_fp();
+                    fp = frame.fp();
                     env_fp = frame.env_fp;
                     let pc = frame.pc;
                     let exit_early = frame.exit_early();

core/engine/src/vm/opcode/generator/mod.rs

@@ -82,7 +82,7 @@ impl Operation for Generator {
             let fp = frame
                 .call_frame
                 .as_ref()
-                .map_or(0, |frame| frame.fp as usize);
+                .map_or(0, |frame| frame.rp as usize);
             frame.stack[fp] = generator.clone().into();
 
             let mut gen = generator

core/engine/src/vm/opcode/rest_parameter/mod.rs

@@ -17,11 +17,11 @@ impl Operation for RestParameterInit {
     const COST: u8 = 6;
 
     fn execute(context: &mut Context) -> JsResult<CompletionType> {
-        let arg_count = context.vm.frame().argument_count as usize;
+        let argument_count = context.vm.frame().argument_count as usize;
         let param_count = context.vm.frame().code_block().params.as_ref().len();
 
-        let array = if arg_count >= param_count {
-            let rest_count = arg_count - param_count + 1;
+        let array = if argument_count >= param_count {
+            let rest_count = argument_count - param_count + 1;
             let args = context.vm.pop_n_values(rest_count);
             Array::create_array_from_list(args, context)
         } else {