optimize: avoid generating drop glue for StepResult

fix-vm/src/bytecode_reader.rs

@@ -77,6 +77,7 @@ impl<'a> BytecodeReader<'a> {
     #[inline(always)]
     pub(crate) fn read_string_id(&mut self) -> StringId {
         let raw = self.read_u32();
+        #[allow(clippy::unwrap_used)]
         StringId(string_interner::symbol::SymbolU32::try_from_usize(raw as usize).unwrap())
     }
 

fix-vm/src/instructions/arithmetic.rs

@@ -32,7 +32,7 @@ impl<'gc> crate::Vm<'gc> {
                 self.push_stack(val);
                 StepResult::Continue
             }
-            Err(e) => e.into_step_result(),
+            Err(e) => self.finish_vm_err(e),
         }
     }
 
@@ -76,7 +76,7 @@ impl<'gc> crate::Vm<'gc> {
                 self.push_stack(val);
                 StepResult::Continue
             }
-            Err(e) => e.into_step_result(),
+            Err(e) => self.finish_vm_err(e),
         }
     }
 
@@ -96,10 +96,9 @@ impl<'gc> crate::Vm<'gc> {
         let lhs = self.pop_stack_forced();
         match (get_num(rhs), get_num(lhs)) {
             (_, Some(NixNum::Int(0))) | (_, Some(NixNum::Float(0.))) => {
-                return VmError::Uncatchable(fix_error::Error::eval_error(
+                return self.finish_vm_err(VmError::Uncatchable(fix_error::Error::eval_error(
                     "division by zero",
-                ))
+                )));
-                .into_step_result();
             }
             _ => {}
         }
@@ -109,7 +108,7 @@ impl<'gc> crate::Vm<'gc> {
                 self.push_stack(val);
                 StepResult::Continue
             }
-            Err(e) => e.into_step_result(),
+            Err(e) => self.finish_vm_err(e),
         }
     }
 
@@ -128,7 +127,7 @@ impl<'gc> crate::Vm<'gc> {
         }
         let eq = match self.values_equal(ctx) {
             Ok(eq) => eq,
-            Err(e) => return e.into_step_result(),
+            Err(e) => return self.finish_vm_err(e),
         };
         self.push_stack(Value::new_inline(eq));
         StepResult::Continue
@@ -149,7 +148,7 @@ impl<'gc> crate::Vm<'gc> {
         }
         let eq = match self.values_equal(ctx) {
             Ok(eq) => eq,
-            Err(e) => return e.into_step_result(),
+            Err(e) => return self.finish_vm_err(e),
         };
         self.push_stack(Value::new_inline(!eq));
         StepResult::Continue
@@ -210,7 +209,7 @@ impl<'gc> crate::Vm<'gc> {
         }
         match self.compare_values_inner(ctx, pred) {
             Ok(()) => StepResult::Continue,
-            Err(e) => e.into_step_result(),
+            Err(e) => self.finish_vm_err(e),
         }
     }
 
@@ -229,16 +228,14 @@ impl<'gc> crate::Vm<'gc> {
         let rhs = self.pop_stack_forced();
         let lhs = self.pop_stack_forced();
         let Some(l) = lhs.as_gc::<crate::List>() else {
-            return VmError::Uncatchable(fix_error::Error::eval_error(
+            return self.finish_err(fix_error::Error::eval_error(
                 "cannot concatenate: left operand is not a list",
-            ))
+            ));
-            .into_step_result();
         };
         let Some(r) = rhs.as_gc::<crate::List>() else {
-            return VmError::Uncatchable(fix_error::Error::eval_error(
+            return self.finish_err(fix_error::Error::eval_error(
                 "cannot concatenate: right operand is not a list",
-            ))
+            ));
-            .into_step_result();
         };
         let mut items = smallvec::SmallVec::new();
         items.extend_from_slice(&l);
@@ -262,16 +259,14 @@ impl<'gc> crate::Vm<'gc> {
         let rhs = self.pop_stack_forced();
         let lhs = self.pop_stack_forced();
         let Some(l) = lhs.as_gc::<crate::AttrSet>() else {
-            return VmError::Uncatchable(fix_error::Error::eval_error(
+            return self.finish_err(fix_error::Error::eval_error(
                 "cannot update: left operand is not a set",
-            ))
+            ));
-            .into_step_result();
         };
         let Some(r) = rhs.as_gc::<crate::AttrSet>() else {
-            return VmError::Uncatchable(fix_error::Error::eval_error(
+            return self.finish_err(fix_error::Error::eval_error(
                 "cannot update: right operand is not a set",
-            ))
+            ));
-            .into_step_result();
         };
         self.push_stack(Value::new_gc(l.merge(&r, mc)));
         StepResult::Continue

fix-vm/src/instructions/calls.rs

@@ -18,9 +18,9 @@ impl<'gc> crate::Vm<'gc> {
             return step;
         }
         if self.call_depth > 10000 {
-            return StepResult::Done(Err(Error::eval_error(
+            return self.finish_err(Error::eval_error(
                 "stack overflow; max-call-depth exceeded",
-            )));
+            ));
         }
         self.call_depth += 1;
         let func = self.pop_stack();
@@ -56,10 +56,7 @@ impl<'gc> crate::Vm<'gc> {
         reader: &mut BytecodeReader<'_>,
         mc: &Mutation<'gc>,
     ) -> StepResult<'gc> {
-        match self.handle_return(reader, ctx, mc) {
+        self.handle_return(reader, ctx, mc)
-            Some(result) => StepResult::Done(result),
-            None => StepResult::Continue,
-        }
     }
 
     pub(crate) fn handle_return<C: crate::VmContext>(
@@ -67,7 +64,7 @@ impl<'gc> crate::Vm<'gc> {
         reader: &mut BytecodeReader<'_>,
         ctx: &C,
         mc: &Mutation<'gc>,
-    ) -> Option<std::result::Result<fix_common::Value, Box<Error>>> {
+    ) -> StepResult<'gc> {
         let ret_inst_pc = reader.pc() - 1;
         let Some(CallFrame {
             pc: ret_pc,
@@ -78,7 +75,7 @@ impl<'gc> crate::Vm<'gc> {
         }) = self.call_stack.pop()
         else {
             let val = self.pop_stack();
-            return Some(Ok(ctx.convert_value(val)));
+            return self.finish_ok(ctx.convert_value(val));
         };
         reader.set_pc(ret_pc);
         if let Some(outer_thunk) = thunk {
@@ -121,7 +118,7 @@ impl<'gc> crate::Vm<'gc> {
                             reader.set_pc(inner_ip);
                             self.env = inner_env;
                             self.with_env = inner_with_env;
-                            return None;
+                            return StepResult::Continue;
                         }
                         ThunkState::Evaluated(val) => {
                             *outer_thunk.borrow_mut(mc) = ThunkState::Evaluated(val);
@@ -136,9 +133,9 @@ impl<'gc> crate::Vm<'gc> {
                         }
                         ThunkState::Apply { func: _, arg: _ } => todo!("force Apply thunk"),
                         ThunkState::Blackhole => {
-                            return Some(Err(Error::eval_error(
+                            return self.finish_err(Error::eval_error(
                                 "infinite recursion encountered",
-                            )));
+                            ));
                         }
                     }
                 }
@@ -148,6 +145,6 @@ impl<'gc> crate::Vm<'gc> {
         }
         self.env = env;
         self.with_env = with_env;
-        None
+        StepResult::Continue
     }
 }

fix-vm/src/instructions/collections.rs

@@ -63,9 +63,9 @@ impl<'gc> crate::Vm<'gc> {
 
         let attrs = self.peek_stack(0).restrict().expect("forced");
         let Some(attrset) = attrs.as_gc::<AttrSet>() else {
-            return StepResult::Done(Err(Error::eval_error(
+            return self.finish_err(Error::eval_error(
                 "value is not a set while a set was expected",
-            )));
+            ));
         };
 
         match attrset.lookup(key) {
@@ -85,9 +85,9 @@ impl<'gc> crate::Vm<'gc> {
                         break;
                     } else {
                         let name = ctx.resolve_string(key);
-                        return StepResult::Done(Err(Error::eval_error(format!(
+                        return self.finish_err(Error::eval_error(format!(
                             "attribute '{name}' missing"
-                        ))));
+                        )));
                     }
                 }
             }
@@ -119,16 +119,16 @@ impl<'gc> crate::Vm<'gc> {
         } else if let Some(ns) = key_val.as_gc::<NixString>() {
             ctx.intern_string(ns.as_str())
         } else {
-            return StepResult::Done(Err(Error::eval_error(
+            return self.finish_err(Error::eval_error(
                 "dynamic select key must be a string",
-            )));
+            ));
         };
 
         let attrset_val = self.stack[self.stack.len() - 2].restrict().expect("forced");
         let Some(attrset) = attrset_val.as_gc::<AttrSet>() else {
-            return StepResult::Done(Err(Error::eval_error(
+            return self.finish_err(Error::eval_error(
                 "value is not a set while a set was expected",
-            )));
+            ));
         };
 
         match attrset.lookup(key_sid) {
@@ -138,9 +138,9 @@ impl<'gc> crate::Vm<'gc> {
             }
             None => {
                 let name = ctx.resolve_string(key_sid);
-                return StepResult::Done(Err(Error::eval_error(format!(
+                return self.finish_err(Error::eval_error(format!(
                     "attribute '{name}' missing"
-                ))));
+                )));
             }
         }
         StepResult::Continue

fix-vm/src/instructions/with_scope.rs

@@ -59,10 +59,10 @@ impl<'gc> crate::Vm<'gc> {
                 unreachable!()
             };
             self.with_env = with_env;
-            return StepResult::Done(Err(Error::eval_error(format!(
+            return self.finish_err(Error::eval_error(format!(
                 "undefined variable '{}'",
                 Symbol::from(ctx.resolve_string(name))
-            ))));
+            )));
         };
         self.push_stack(env);
         if let Some(step) = self.try_force_resolved(0, reader.inst_start_pc(), mc) {

fix-vm/src/lib.rs

@@ -38,10 +38,10 @@ impl From<Box<Error>> for VmError {
 }
 
 impl VmError {
-    fn into_step_result<'gc>(self) -> StepResult<'gc> {
+    fn into_error(self) -> Box<Error> {
         match self {
             VmError::Catchable(_) => todo!("Check for tryEval catch frames"),
-            VmError::Uncatchable(e) => StepResult::Done(Err(e)),
+            VmError::Uncatchable(e) => e,
         }
     }
 }
@@ -128,7 +128,7 @@ impl<T: VmContext> VmContextExt for T {
 pub(crate) enum StepResult<'gc> {
     Continue,
     ForceThunk(ForceInfo<'gc>),
-    Done(Result<fix_common::Value>),
+    Done,
 }
 
 pub(crate) struct ForceInfo<'gc> {
@@ -159,6 +159,9 @@ pub struct Vm<'gc> {
     pub(crate) empty_attrs: Value<'gc>,
 
     pub(crate) force_mode: ForceMode,
+
+    #[collect(require_static)]
+    pub(crate) result: Option<Result<fix_common::Value>>,
 }
 
 pub(crate) enum OperandData {
@@ -264,9 +267,28 @@ impl<'gc> Vm<'gc> {
             empty_attrs: Value::new_gc(Gc::new(mc, AttrSet::default())),
 
             force_mode,
+
+            result: None,
         }
     }
 
+    #[inline(always)]
+    pub(crate) fn finish_ok(&mut self, val: fix_common::Value) -> StepResult<'gc> {
+        self.result = Some(Ok(val));
+        StepResult::Done
+    }
+
+    #[inline(always)]
+    pub(crate) fn finish_err(&mut self, err: Box<Error>) -> StepResult<'gc> {
+        self.result = Some(Err(err));
+        StepResult::Done
+    }
+
+    #[inline(always)]
+    pub(crate) fn finish_vm_err(&mut self, err: VmError) -> StepResult<'gc> {
+        self.finish_err(err.into_error())
+    }
+
     #[inline(always)]
     pub(crate) fn push_stack(&mut self, val: Value<'gc>) {
         self.stack.push(val);
@@ -339,7 +361,7 @@ impl<'gc> Vm<'gc> {
                 }
                 ThunkState::Apply { .. } => todo!("force apply"),
                 ThunkState::Blackhole => {
-                    StepResult::Done(Err(Error::eval_error("infinite recursion encountered")))
+                    self.finish_err(Error::eval_error("infinite recursion encountered"))
                 }
             }
         } else {
@@ -526,7 +548,11 @@ impl<'gc> Vm<'gc> {
                     self.env = info.env;
                     self.with_env = info.with_env;
                 }
-                StepResult::Done(result) => return Action::Done(result),
+                StepResult::Done => {
+                    return Action::Done(
+                        self.result.take().expect("StepResult::Done without result"),
+                    );
+                }
             }
         }
     }