fix force

fix/src/runtime/value.rs

@@ -478,6 +478,12 @@ impl<'gc> AttrSet<'gc> {
 pub(crate) struct List<'gc> {
     pub(crate) inner: SmallVec<[Value<'gc>; 4]>,
 }
+impl<'gc> Deref for List<'gc> {
+    type Target = SmallVec<[Value<'gc>; 4]>;
+    fn deref(&self) -> &Self::Target {
+        &self.inner
+    }
+}
 
 pub(crate) type Thunk<'gc> = RefLock<ThunkState<'gc>>;
 

fix/src/runtime/vm.rs

@@ -95,7 +95,11 @@ impl<'gc> GcRoot<'gc> {
 
     #[inline(always)]
     pub(super) fn pop_stack_forced(&mut self) -> StrictValue<'gc> {
-        self.stack.pop().expect("stack underflow").restrict().expect("forced")
+        self.stack
+            .pop()
+            .expect("stack underflow")
+            .restrict()
+            .expect("forced")
     }
 }
 
@@ -196,7 +200,7 @@ enum SelectKeyData {
 }
 
 macro_rules! try_vm {
-    ($self:ident, $expr:expr) => {
+    ($self:ident; $expr:expr) => {
         match $expr {
             Ok(v) => v,
             Err(e) => return Runtime::handle_vm_error($self, e),
@@ -529,8 +533,7 @@ impl Runtime {
                         &SelectKeyData::Static(sid) => sid,
                         SelectKeyData::Dynamic => {
                             self.arena.mutate_root(|_, root| {
-                                let v =
+                                let v = root.temp_stack.pop().expect("missing dynamic key");
-                                    root.temp_stack.pop().expect("missing dynamic key");
                                 root.push_stack(v);
                             });
                             self.force_tos();
@@ -555,9 +558,7 @@ impl Runtime {
                     let result = self.arena.mutate_root(|_, root| {
                         let val = root.pop_stack();
                         let Some(attrset) = val.as_gc::<AttrSet<'_>>() else {
-                            return Err(vm_err(
+                            return Err(vm_err("value is not a set while a set was expected"));
-                                "value is not a set while a set was expected",
-                            ));
                         };
                         match attrset.lookup(key_sid) {
                             Some(v) => {
@@ -577,8 +578,7 @@ impl Runtime {
                         Ok(false) => {
                             self.arena
                                 .mutate_root(|_, root| root.temp_stack.truncate(temp_base));
-                            let name =
+                            let name = self.strings.resolve(key_sid.0).unwrap_or("«unknown»");
-                                self.strings.resolve(key_sid.0).unwrap_or("«unknown»");
                             return Runtime::handle_vm_error(
                                 self,
                                 vm_err(format!("attribute '{name}' missing")),
@@ -626,8 +626,7 @@ impl Runtime {
                         &SelectKeyData::Static(sid) => sid,
                         SelectKeyData::Dynamic => {
                             self.arena.mutate_root(|_, root| {
-                                let v =
+                                let v = root.temp_stack.pop().expect("missing dynamic key");
-                                    root.temp_stack.pop().expect("missing dynamic key");
                                 root.push_stack(v);
                             });
                             self.force_tos();
@@ -652,11 +651,12 @@ impl Runtime {
                     let found = self.arena.mutate_root(|_, root| {
                         let val = root.pop_stack();
                         if let Some(attrset) = val.as_gc::<AttrSet<'_>>()
-                            && let Some(v) = attrset.lookup(key_sid) {
+                            && let Some(v) = attrset.lookup(key_sid)
+                        {
                             root.push_stack(v);
                             return true;
                         }
-                        // Not a set or key missing → use default
+                        // Not a set or key missing: use default
                         false
                     });
 
@@ -706,24 +706,118 @@ impl Runtime {
                 self.push_empty_list();
             }
 
-            OpAdd | OpSub | OpMul | OpDiv | OpEq | OpNeq | OpLt | OpGt | OpLeq | OpGeq
+            OpAdd => {
-            | OpConcat | OpUpdate => {
+                self.force_n(2);
-                let tag = match op {
+                let strings = &self.strings;
-                    OpAdd => BinOpTag::Add,
+                let res = self.arena.mutate_root(|mc, root| {
-                    OpSub => BinOpTag::Sub,
+                    let rhs = root.pop_stack_forced();
-                    OpMul => BinOpTag::Mul,
+                    let lhs = root.pop_stack_forced();
-                    OpDiv => BinOpTag::Div,
+                    // FIXME: path & string context
-                    OpEq => BinOpTag::Eq,
+                    if let (Some(ls), Some(rs)) = (
-                    OpNeq => BinOpTag::Neq,
+                        Self::get_string(strings, lhs),
-                    OpLt => BinOpTag::Lt,
+                        Self::get_string(strings, rhs),
-                    OpGt => BinOpTag::Gt,
+                    ) {
-                    OpLeq => BinOpTag::Leq,
+                        let ns = Gc::new(mc, NixString::new(format!("{ls}{rs}")));
-                    OpGeq => BinOpTag::Geq,
+                        root.push_stack(Value::new_gc(ns));
-                    OpConcat => BinOpTag::Concat,
+                        return Ok(());
-                    OpUpdate => BinOpTag::Update,
+                    }
+                    let res = Self::numeric_binop(lhs, rhs, mc, i64::wrapping_add, |a, b| a + b)?;
+                    root.push_stack(res);
+                    VmResult::Ok(())
+                });
+                try_vm!(self; res);
+            }
+            OpSub | OpMul => {
+                self.force_n(2);
+                #[allow(clippy::type_complexity)]
+                let func: (fn(i64, i64) -> i64, fn(f64, f64) -> f64) = match op {
+                    OpSub => (i64::wrapping_sub, |a, b| a - b),
+                    OpMul => (i64::wrapping_mul, |a, b| a * b),
                     _ => unreachable!(),
                 };
-                try_vm!(self, self.compute_binop(tag))
+                let res = self.arena.mutate_root(|mc, root| {
+                    let rhs = root.pop_stack_forced();
+                    let lhs = root.pop_stack_forced();
+                    let res = Self::numeric_binop(lhs, rhs, mc, func.0, func.1)?;
+                    root.push_stack(res);
+                    VmResult::Ok(())
+                });
+                try_vm!(self; res);
+            }
+            OpDiv => {
+                self.force_n(2);
+                let res = self.arena.mutate_root(|mc, root| {
+                    let rhs = root.pop_stack_forced();
+                    let lhs = root.pop_stack_forced();
+                    match (Self::get_num(lhs), Self::get_num(rhs)) {
+                        (_, Some(NixNum::Int(0))) => Err(vm_err("division by zero")),
+                        (_, Some(NixNum::Float(0.))) => Err(vm_err("division by zero")),
+                        _ => Ok(()),
+                    }?;
+                    let res = Self::numeric_binop(lhs, rhs, mc, |a, b| a / b, |a, b| a / b)?;
+                    root.push_stack(res);
+                    VmResult::Ok(())
+                });
+                try_vm!(self; res);
+            }
+            OpEq | OpNeq => {
+                self.force_n(2);
+                let map: fn(bool) -> bool = match op {
+                    OpEq => |a| a,
+                    OpNeq => |a| !a,
+                    _ => unreachable!(),
+                };
+                let eq = try_vm!(self; self.values_equal());
+                self.push_stack(|_| Value::new_inline(map(eq)));
+            }
+            OpLt | OpGt | OpLeq | OpGeq => {
+                use std::cmp::Ordering;
+                self.force_n(2);
+                let pred: fn(Ordering) -> bool = match op {
+                    OpLt => Ordering::is_lt,
+                    OpGt => Ordering::is_gt,
+                    OpLeq => Ordering::is_le,
+                    OpGeq => Ordering::is_ge,
+                    _ => unreachable!(),
+                };
+                try_vm!(self; self.compare_values(pred));
+            }
+            OpConcat => {
+                self.force_n(2);
+                let res = self.arena.mutate_root(|mc, root| {
+                    let rhs = root.pop_stack_forced();
+                    let lhs = root.pop_stack_forced();
+                    // TODO: better type-assert ergonomic
+                    let Some(l) = lhs.as_gc::<List<'_>>() else {
+                        return Err(vm_err("cannot concatenate: left operand is not a list"));
+                    };
+                    let Some(r) = rhs.as_gc::<List<'_>>() else {
+                        return Err(vm_err("cannot concatenate: right operand is not a list"));
+                    };
+                    let mut items = SmallVec::new();
+                    items.extend_from_slice(&l);
+                    items.extend_from_slice(&r);
+                    root.push_stack(Value::new_gc(Gc::new(mc, List { inner: items })));
+                    VmResult::Ok(())
+                });
+                try_vm!(self; res);
+            }
+            OpUpdate => {
+                self.force_n(2);
+                let res = self.arena.mutate_root(|mc, root| {
+                    let rhs = root.pop_stack_forced();
+                    let lhs = root.pop_stack_forced();
+                    // TODO: better type-assert ergonomic
+                    let Some(l) = lhs.as_gc::<AttrSet<'_>>() else {
+                        return Err(vm_err("cannot update: left operand is not a set"));
+                    };
+                    let Some(r) = rhs.as_gc::<AttrSet<'_>>() else {
+                        return Err(vm_err("cannot update: right operand is not a set"));
+                    };
+                    root.push_stack(Value::new_gc(l.merge(&r, mc)));
+                    VmResult::Ok(())
+                });
+                try_vm!(self; res);
             }
 
             OpNeg => {
@@ -858,119 +952,7 @@ impl Runtime {
         }
     }
 
-    fn compute_binop(&mut self, op: BinOpTag) -> VmResult<()> {
+    #[inline]
-        self.force_n(2);
-
-        match op {
-            BinOpTag::Add => {
-                let strings = &self.strings;
-                self.arena.mutate_root(|mc, root| {
-                    let rhs = root.pop_stack_forced();
-                    let lhs = root.pop_stack_forced();
-                    // FIXME: path & string context
-                    if let (Some(ls), Some(rs)) = (
-                        Self::get_string(strings, lhs),
-                        Self::get_string(strings, rhs),
-                    ) {
-                        let ns = Gc::new(mc, NixString::new(format!("{ls}{rs}")));
-                        root.push_stack(Value::new_gc(ns));
-                        return Ok(());
-                    }
-                    let res = Self::numeric_binop(lhs, rhs, mc, i64::wrapping_add, |a, b| a + b)?;
-                    root.push_stack(res);
-                    VmResult::Ok(())
-                })?;
-            }
-            BinOpTag::Sub => {
-                self.arena.mutate_root(|mc, root| {
-                    let rhs = root.pop_stack_forced();
-                    let lhs = root.pop_stack_forced();
-                    let res = Self::numeric_binop(lhs, rhs, mc, i64::wrapping_sub, |a, b| a - b)?;
-                    root.push_stack(res);
-                    VmResult::Ok(())
-                })?;
-            }
-            BinOpTag::Mul => {
-                self.arena.mutate_root(|mc, root| {
-                    let rhs = root.pop_stack_forced();
-                    let lhs = root.pop_stack_forced();
-                    let res = Self::numeric_binop(lhs, rhs, mc, i64::wrapping_mul, |a, b| a * b)?;
-                    root.push_stack(res);
-                    VmResult::Ok(())
-                })?;
-            }
-            BinOpTag::Div => {
-                self.arena.mutate_root(|mc, root| {
-                    let rhs = root.pop_stack_forced();
-                    let lhs = root.pop_stack_forced();
-                    let res = match (Self::get_num(lhs), Self::get_num(rhs)) {
-                        (_, Some(NixNum::Int(0))) => Err(vm_err("division by zero")),
-                        (_, Some(NixNum::Float(0.))) => Err(vm_err("division by zero")),
-                        (Some(NixNum::Int(a)), Some(NixNum::Int(b))) => {
-                            Ok(Value::make_int(a.wrapping_div(b), mc))
-                        }
-                        (Some(NixNum::Float(a)), Some(NixNum::Float(b))) => {
-                            Ok(Value::new_float(a / b))
-                        }
-                        (Some(NixNum::Int(a)), Some(NixNum::Float(b))) => {
-                            Ok(Value::new_float(a as f64 / b))
-                        }
-                        (Some(NixNum::Float(a)), Some(NixNum::Int(b))) => {
-                            Ok(Value::new_float(a / b as f64))
-                        }
-                        _ => Err(vm_err("cannot divide non-numbers")),
-                    }?;
-                    root.push_stack(res);
-                    VmResult::Ok(())
-                })?;
-            }
-            BinOpTag::Eq => {
-                let eq = self.values_equal()?;
-                self.push_stack(|_| Value::new_inline(eq));
-            }
-            BinOpTag::Neq => {
-                let eq = self.values_equal()?;
-                self.push_stack(|_| Value::new_inline(!eq));
-            }
-            BinOpTag::Lt => self.compare_values(|o| o.is_lt())?,
-            BinOpTag::Gt => self.compare_values(|o| o.is_gt())?,
-            BinOpTag::Leq => self.compare_values(|o| o.is_le())?,
-            BinOpTag::Geq => self.compare_values(|o| o.is_ge())?,
-            BinOpTag::Concat => {
-                self.arena.mutate_root(|mc, root| {
-                    let rhs = root.pop_stack_forced();
-                    let lhs = root.pop_stack_forced();
-                    let Some(l) = lhs.as_gc::<List<'_>>() else {
-                        return Err(vm_err("cannot concatenate: left operand is not a list"));
-                    };
-                    let Some(r) = rhs.as_gc::<List<'_>>() else {
-                        return Err(vm_err("cannot concatenate: right operand is not a list"));
-                    };
-                    let mut items = SmallVec::new();
-                    items.extend(l.inner.iter().cloned());
-                    items.extend(r.inner.iter().cloned());
-                    root.push_stack(Value::new_gc(Gc::new(mc, List { inner: items })));
-                    VmResult::Ok(())
-                })?;
-            }
-            BinOpTag::Update => {
-                self.arena.mutate_root(|mc, root| {
-                    let rhs = root.pop_stack_forced();
-                    let lhs = root.pop_stack_forced();
-                    let Some(l) = lhs.as_gc::<AttrSet<'_>>() else {
-                        return Err(vm_err("cannot update: left operand is not a set"));
-                    };
-                    let Some(r) = rhs.as_gc::<AttrSet<'_>>() else {
-                        return Err(vm_err("cannot update: right operand is not a set"));
-                    };
-                    root.push_stack(Value::new_gc(l.merge(&r, mc)));
-                    VmResult::Ok(())
-                })?;
-            }
-        }
-        Ok(())
-    }
-
     fn numeric_binop<'gc>(
         lhs: StrictValue<'gc>,
         rhs: StrictValue<'gc>,
@@ -1056,8 +1038,8 @@ impl Runtime {
             State::List(len) | State::AttrSet(len) => {
                 for i in 0..len {
                     self.arena.mutate_root(|_, root| {
-                        let y = root.temp_stack.pop().unwrap();
+                        let y = root.temp_stack.pop().expect("stack underflow");
-                        let x = root.temp_stack.pop().unwrap();
+                        let x = root.temp_stack.pop().expect("stack underflow");
                         root.push_stack(x);
                         root.push_stack(y);
                     });
@@ -1186,13 +1168,10 @@ impl Runtime {
     pub(super) fn force_tos(&mut self) -> Action {
         loop {
             let run = self.arena.mutate_root(|_mc, root| {
-                let thunk = root
+                let thunk = root.stack.tos_mut().expect("stack underflow");
-                    .stack
-                    .tos_mut()
-                    .expect("stack underflow");
 
                 let Some(thunk_state) = thunk.as_gc::<Thunk>() else {
-                    return false
+                    return false;
                 };
                 match *thunk_state.borrow() {
                     ThunkState::Pending { ip, env } => {
@@ -1210,7 +1189,7 @@ impl Runtime {
                     ThunkState::Evaluated(val) => {
                         *thunk = val;
                         false
-                    },
+                    }
                     ThunkState::Blackhole => todo!("force_tos"),
                 }
             });
@@ -1276,21 +1255,18 @@ impl Runtime {
             ForceMode::AsIs => (),
             ForceMode::Shallow => {
                 if let done @ Action::Done(_) = self.force_tos_shallow() {
-                    return done
+                    return done;
                 }
             }
             ForceMode::Deep => {
                 if let done @ Action::Done(_) = self.force_tos_shallow() {
-                    return done
+                    return done;
                 }
             }
         }
         let val = self.arena.mutate_root(|_, root| {
             root.current_env = None;
-            convert_value(
+            convert_value(root.stack.pop().expect("stack underflow"), &self.strings)
-                root.stack.pop().expect("stack underflow"),
-                &self.strings,
-            )
         });
         Action::Done(Ok(val))
     }