treewide: enforce strict clippy check

This commit is contained in:
2026-08-22 18:41:23 +08:00
parent 5f494983b8
commit cb38031f85
51 changed files with 1237 additions and 568 deletions
+76 -30
View File
@@ -1,4 +1,7 @@
#![allow(dead_code)]
#![allow(
dead_code,
reason = "boxing layer exposes a full API surface; some helpers are used only on specific targets or not yet wired up"
)]
use std::fmt;
use std::num::NonZeroU8;
@@ -79,41 +82,48 @@ int_store!(i16);
int_store!(i32);
fn store_ptr<P: Strict + Copy>(value: &mut Value, ptr: P) {
#[cfg(target_pointer_width = "64")]
{
assert!(
ptr.addr() <= 0x0000_FFFF_FFFF_FFFF,
"Pointer too large to store in NaN box"
);
cfg_select! {
target_pointer_width = "64" => {
assert!(
ptr.addr() <= 0x0000_FFFF_FFFF_FFFF,
"Pointer too large to store in NaN box"
);
let val = (unsafe { value.whole_mut() } as *mut [u8; 8]).cast::<P>();
// SAFETY: `Value` is `#[repr(C, align(8))]` and exactly 8 bytes, so it is
// sound to reinterpret its storage as a `[u8; 8]` that we then treat as `P`
// (a pointer-sized value).
let val = (unsafe { value.whole_mut() } as *mut [u8; 8]).cast::<P>();
let ptr = Strict::map_addr(ptr, |addr| {
addr | (usize::from(value.header().into_raw()) << 48)
});
let ptr = Strict::map_addr(ptr, |addr| {
addr | (usize::from(value.header().into_raw()) << 48)
});
unsafe { val.write(ptr) };
}
#[cfg(target_pointer_width = "32")]
{
let _ = (value, ptr);
unimplemented!("32-bit pointer storage not supported");
// SAFETY: `val` points to the `Value`'s 8-byte storage, which is valid and
// suitably aligned to hold `P`.
unsafe { val.write(ptr) };
}
_ => {
compile_error!("unsupported pointer width");
}
}
}
fn load_ptr<P: Strict>(value: &Value) -> P {
#[cfg(target_pointer_width = "64")]
{
let val = (unsafe { value.whole() } as *const [u8; 8]).cast::<P>();
let ptr = unsafe { val.read() };
Strict::map_addr(ptr, |addr| addr & 0x0000_FFFF_FFFF_FFFF)
}
#[cfg(target_pointer_width = "32")]
{
let _ = value;
unimplemented!("32-bit pointer storage not supported");
cfg_select! {
target_pointer_width = "64" => {
// SAFETY: `Value` is `#[repr(C, align(8))]` and exactly 8 bytes, so it
// is sound to reinterpret its storage as a `[u8; 8]` cast to `P`; the
// pointer was originally written through this same `P` layout by
// `store_ptr`.
let val = (unsafe { value.whole() } as *const [u8; 8]).cast::<P>();
// SAFETY: `val` points to the `Value`'s 8-byte storage, which is valid
// and suitably aligned to hold `P`.
let ptr = unsafe { val.read() };
Strict::map_addr(ptr, |addr| addr & 0x0000_FFFF_FFFF_FFFF)
}
_ => {
compile_error!("unsupported pointer width");
}
}
}
@@ -179,6 +189,10 @@ impl RawTag {
#[inline]
#[must_use]
pub(crate) fn new(neg: bool, val: NonZeroU8) -> RawTag {
// SAFETY: masking a `NonZeroU8` with `0x07` yields a value in `0..8`;
// callers only construct tags from valid tag discriminants in `1..8`,
// so the low three bits are in the `1..8` range required by
// `new_unchecked`.
unsafe { Self::new_unchecked(neg, val.get() & 0x07) }
}
@@ -229,6 +243,9 @@ impl RawTag {
(true, 6) => TagVal::_N6,
(true, 7) => TagVal::_N7,
// SAFETY: the caller guarantees `val` is in `1..8`; every
// `(neg, val)` combination in that range is matched above, so this
// arm cannot be reached.
_ => unsafe { core::hint::unreachable_unchecked() },
})
}
@@ -302,6 +319,10 @@ impl Header {
#[inline]
const fn tag(self) -> RawTag {
// SAFETY: a `Header` is only ever constructed by `Header::new` from a
// `RawTag` whose value is in `1..8`, stored in the low three bits;
// `get_tag` recovers exactly those bits, so the argument passed to
// `new_unchecked` is in the required `1..8` range.
unsafe { RawTag::new_unchecked(self.get_sign(), self.get_tag()) }
}
@@ -323,7 +344,7 @@ impl Header {
#[derive(Copy, Clone, Debug, PartialEq)]
#[repr(C, align(8))]
pub struct Value {
pub(crate) struct Value {
#[cfg(target_endian = "big")]
header: Header,
data: [u8; 6],
@@ -387,6 +408,9 @@ impl Value {
#[must_use]
unsafe fn whole(&self) -> &[u8; 8] {
let ptr = (self as *const Value).cast::<[u8; 8]>();
// SAFETY: `Value` is `#[repr(C, align(8))]` and exactly 8 bytes with no
// padding, so it shares its layout with `[u8; 8]`; `ptr` is derived
// from a valid `&Value`, so the reference is valid for reads.
unsafe { &*ptr }
}
@@ -394,6 +418,10 @@ impl Value {
#[must_use]
unsafe fn whole_mut(&mut self) -> &mut [u8; 8] {
let ptr = (self as *mut Value).cast::<[u8; 8]>();
// SAFETY: `Value` is `#[repr(C, align(8))]` and exactly 8 bytes with no
// padding, so it shares its layout with `[u8; 8]`; `ptr` is derived
// from a unique `&mut Value`, so the reference is valid for reads and
// writes.
unsafe { &mut *ptr }
}
}
@@ -435,6 +463,9 @@ impl RawBox {
#[must_use]
pub(crate) const fn tag(&self) -> Option<RawTag> {
if self.is_value() {
// SAFETY: `is_value()` returned true, so the union holds a `Value`
// (a tagged-NaN bit pattern) rather than a float, making the read
// of the `value` field sound.
Some(unsafe { self.value.tag() })
} else {
None
@@ -444,12 +475,18 @@ impl RawBox {
#[inline]
#[must_use]
pub(crate) fn is_float(&self) -> bool {
// SAFETY: every 8-byte pattern is simultaneously a valid `f64` and a
// valid `u64`, so reading the `float` and `bits` union fields is
// always sound.
(unsafe { !self.float.is_nan() } || unsafe { self.bits & SIGN_MASK == QUIET_NAN })
}
#[inline]
#[must_use]
pub(crate) const fn is_value(&self) -> bool {
// SAFETY: every 8-byte pattern is simultaneously a valid `f64` and a
// valid `u64`, so reading the `float` and `bits` union fields is
// always sound.
(unsafe { self.float.is_nan() } && unsafe { self.bits & SIGN_MASK != QUIET_NAN })
}
@@ -457,6 +494,8 @@ impl RawBox {
#[must_use]
pub(crate) fn float(&self) -> Option<&f64> {
if self.is_float() {
// SAFETY: reading the `float` field is sound because any 8-byte
// pattern is a valid `f64`.
Some(unsafe { &self.float })
} else {
None
@@ -467,6 +506,9 @@ impl RawBox {
#[must_use]
pub(crate) fn value(&self) -> Option<&Value> {
if self.is_value() {
// SAFETY: `is_value()` returned true, so the union holds a `Value`;
// `Value` has no invalid bit patterns, so reading the `value` field
// is sound.
Some(unsafe { &self.value })
} else {
None
@@ -475,12 +517,16 @@ impl RawBox {
#[inline]
pub(crate) fn into_float_unchecked(self) -> f64 {
// SAFETY: reading the `float` field is sound because any 8-byte pattern
// is a valid `f64`.
unsafe { self.float }
}
#[inline]
#[must_use]
pub(crate) fn to_bits(self) -> u64 {
// SAFETY: reading the `bits` field is sound because any 8-byte pattern
// is a valid `u64`.
unsafe { self.bits }
}
}
+13 -13
View File
@@ -2,8 +2,8 @@ use fix_lang::StringId;
use gc_arena::Mutation;
use crate::{
AttrSet, Break, BytecodeReader, Closure, List, Machine, NixNum, NixString, NixType, Null,
PrimOp, PrimOpApp, Step, StrictValue, ValueVariant,
AttrSet, BytecodeReader, Closure, List, Machine, NixNum, NixString, NixType, Null, PrimOp,
PrimOpApp, Step, StrictValue, ValueVariant,
};
pub trait Forced<'gc>: Sized {
@@ -63,11 +63,11 @@ macro_rules! impl_forced {
) -> Step {
m.force_slot_to_pc(base_depth, reader, mc, resume_pc)?;
let v = m.peek_forced(base_depth);
if v.downcast::<$ty>().is_none() {
let _: Step = m.finish_type_err(<$ty as ValueVariant>::TYPE, v.ty());
return Step::Break(Break::Done);
if !v.is::<$ty>() {
m.finish_type_err(<$ty as ValueVariant>::TYPE, v.ty())
} else {
Step::Continue(())
}
Step::Continue(())
}
#[inline(always)]
@@ -109,10 +109,10 @@ impl<'gc> Forced<'gc> for NixNum {
m.force_slot_to_pc(base_depth, reader, mc, resume_pc)?;
let v = m.peek_forced(base_depth);
if v.downcast_num().is_none() {
let _: Step = m.finish_type_err(NixType::Int, v.ty());
return Step::Break(Break::Done);
m.finish_type_err(NixType::Int, v.ty())
} else {
Step::Continue(())
}
Step::Continue(())
}
#[inline(always)]
@@ -136,11 +136,11 @@ impl<'gc> Forced<'gc> for f64 {
) -> Step {
m.force_slot_to_pc(base_depth, reader, mc, resume_pc)?;
let v = m.peek_forced(base_depth);
if v.downcast::<f64>().is_none() {
let _: Step = m.finish_type_err(NixType::Float, v.ty());
return Step::Break(Break::Done);
if !v.is::<f64>() {
m.finish_type_err(NixType::Float, v.ty())
} else {
Step::Continue(())
}
Step::Continue(())
}
#[inline(always)]
+3 -5
View File
@@ -1,6 +1,6 @@
use fix_bytecode::InstructionPtr;
use fix_error::Source;
use fix_lang::{self, BUILTINS, StringId};
use fix_lang::{self, StringId};
use hashbrown::HashSet;
use crate::{
@@ -153,11 +153,9 @@ impl<T: VmRuntimeCtx> ConvertValueWithSeen for T {
Value::Thunk
}
} else if let Some(primop) = val.downcast::<PrimOp>() {
let name = BUILTINS[primop.id as usize].0;
Value::PrimOp(name.strip_prefix("__").unwrap_or(name))
Value::PrimOp(primop.id.info().name)
} else if let Some(app) = val.downcast::<PrimOpApp>() {
let name = BUILTINS[app.primop.id as usize].0;
Value::PrimOpApp(name.strip_prefix("__").unwrap_or(name))
Value::PrimOpApp(app.primop.id.info().name)
} else {
Value::Null
}
+12 -4
View File
@@ -3,11 +3,11 @@ use std::path::{Path, PathBuf};
use fix_error::Error;
use fix_lang::{self, StringId};
use gc_arena::Mutation;
use gc_arena::{Gc, Mutation};
use crate::{
Break, BytecodeReader, CallFrame, ForceMode, Forced, GcEnv, NixType, PendingLoad, Step,
StrictValue, Value, VmError,
AttrSet, Break, BytecodeReader, CallFrame, ForceMode, Forced, GcEnv, NixType, PendingLoad,
Step, StrictValue, Value, VmError,
};
/// Abstract VM-side operations consumed by instruction handlers and primops.
@@ -65,6 +65,10 @@ pub trait Machine<'gc> {
) -> Step;
#[inline(always)]
#[expect(
clippy::unreachable,
reason = "a primop only returns via `return_from_primop` while its call frame is still on the stack, so `pop_call_frame` is always `Some`"
)]
fn return_from_primop(&mut self, val: Value<'gc>, reader: &mut BytecodeReader<'_>) -> Step {
self.push(val);
let Some(CallFrame {
@@ -91,6 +95,10 @@ pub trait Machine<'gc> {
fn set_env(&mut self, env: GcEnv<'gc>);
#[inline(always)]
#[expect(
clippy::indexing_slicing,
reason = "codegen guarantees the local index is within the resolved frame's `locals`"
)]
fn local(&self, layer: u8, idx: u32) -> Value<'gc> {
let mut cur = self.env();
for _ in 0..layer {
@@ -109,7 +117,7 @@ pub trait Machine<'gc> {
self.finish_err(err.into_error())
}
fn builtins(&self) -> Value<'gc>;
fn builtins(&self) -> Gc<'gc, AttrSet<'gc>>;
fn functor_sym(&self) -> StringId;
fn empty_list(&self) -> Value<'gc>;
fn empty_attrs(&self) -> Value<'gc>;
+2 -10
View File
@@ -20,20 +20,12 @@ pub fn resolve_operand<'gc, M: Machine<'gc>>(
Const(id) => ctx.get_const(id).into(),
BigInt(val) => Value::new(Gc::new(mc, val)),
Local { layer, idx } => m.local(layer, idx),
#[allow(clippy::unwrap_used)]
BuiltinConst(id) => m
.builtins()
.downcast::<AttrSet>()
.unwrap()
.lookup(id)
.unwrap(),
Builtins => m.builtins(),
BuiltinConst(id) => m.builtins().lookup(id).expect("builtin const must exist"),
Builtins => m.builtins().into(),
ReplBinding(_id) => todo!(),
ScopedImportBinding { slot_id, name } => {
let scope = m.scope_slot(slot_id);
#[allow(clippy::unwrap_used)]
let attrs = scope.downcast::<AttrSet>().expect("scope must be attrset");
#[allow(clippy::unwrap_used)]
attrs.lookup(name).expect("scoped binding not found")
}
}
-2
View File
@@ -8,7 +8,6 @@ use hashbrown::HashSet;
use crate::{GcEnv, Thunk};
#[allow(dead_code)]
pub enum VmError {
Catchable(String),
Uncatchable(Box<Error>),
@@ -51,7 +50,6 @@ pub enum Break {
pub type Step = ControlFlow<Break>;
#[allow(dead_code)]
pub struct ErrorFrame {
pub span_id: u32,
pub message: Option<String>,
+7 -3
View File
@@ -31,10 +31,10 @@ impl StringContextElem {
drv_path: drv_path.into(),
}
} else if let Some(rest) = encoded.strip_prefix('!') {
if let Some(second_bang) = rest.find('!') {
if let Some((output, drv_path)) = rest.split_once('!') {
Self::Built {
output: rest[..second_bang].into(),
drv_path: rest[second_bang + 1..].into(),
output: output.into(),
drv_path: drv_path.into(),
}
} else {
Self::Opaque {
@@ -117,6 +117,10 @@ impl StringContext {
}
}
#[expect(
clippy::indexing_slicing,
reason = "`i`/`j` stay strictly below their lengths inside the loop, and the trailing slices use those in-bounds cursors as start indices"
)]
pub fn merge(&self, other: &Self) -> Self {
if self.data.is_empty() {
return other.clone();
+75 -5
View File
@@ -4,7 +4,9 @@ use std::marker::PhantomData;
use std::mem::size_of;
use std::ops::Deref;
use fix_bytecode::Continuation;
use fix_lang::*;
use fix_macros::unelide_lifetimes;
use gc_arena::barrier::Unlock;
use gc_arena::collect::Trace;
use gc_arena::{Collect, Gc, GcRefLock, Mutation, RefLock};
@@ -20,7 +22,10 @@ mod private {
}
pub trait ValueVariant: private::Cealed {
#[allow(private_bounds)]
#[expect(
private_bounds,
reason = "Storable is a sealed implementation detail of the value system"
)]
type Ty<'gc>: Storable + 'gc;
const TYPE: NixType;
}
@@ -52,6 +57,9 @@ macro_rules! define_value_types {
}
#[inline(always)]
unsafe fn from_raw_box(raw: RawBox) -> Self {
// SAFETY: the caller guarantees `raw` represents a valid
// `Self` of this inline type, so it holds a `Value` (making
// `value()` `Some`) whose payload decodes to this type.
unsafe { <Self as RawStore>::from_val(raw.value().unwrap_unchecked()) }
}
}
@@ -73,13 +81,17 @@ macro_rules! define_value_types {
}
#[inline(always)]
unsafe fn from_raw_box(raw: RawBox) -> Self {
// SAFETY: the caller guarantees `raw` represents a valid
// `Self` of this GC type, so it holds a `Value` (making
// `value()` `Some`) whose payload is the pointer originally
// produced by `Gc::as_ptr` in `to_raw_box`.
unsafe { Gc::from_ptr(<*mut $gtype as RawStore>::from_val(raw.value().unwrap_unchecked())) }
}
}
impl private::Cealed for Gc<'_, $gtype> {}
impl private::Cealed for $gtype {}
impl ValueVariant for $gtype {
type Ty<'gc> = Gc<'gc, fix_macros::unelide_lifetimes!('gc; $gtype)>;
type Ty<'gc> = Gc<'gc, unelide_lifetimes!('gc; $gtype)>;
const TYPE: NixType = $gty;
}
)*
@@ -93,6 +105,7 @@ macro_rules! define_value_types {
let mut mask_true: u8 = 0;
let mut i = 0;
while i < tags.len() {
#[expect(clippy::indexing_slicing, reason = "loop condition guarantees `i < tags.len()`")]
let (neg, val) = tags[i];
let bit = 1 << val;
if neg {
@@ -106,12 +119,17 @@ macro_rules! define_value_types {
}
};
// SAFETY: `trace` visits every reachable `Gc` pointer: for each GC
// tag it downcasts to the concrete `Gc` type and forwards `trace`,
// while inline tags hold no GC pointers and need no tracing.
unsafe impl<'gc> Collect<'gc> for Value<'gc> {
const NEEDS_TRACE: bool = true;
fn trace<T: Trace<'gc>>(&self, cc: &mut T) {
let Some(tag) = self.raw.tag() else { return };
match tag {
$($gtag => unsafe {
// SAFETY: `tag` matched `$gtag`, so `downcast` to the
// corresponding GC type is guaranteed to be `Some`.
self.downcast::<$gtype>().unwrap_unchecked().trace(cc)
},)*
$($itag => (),)*
@@ -123,9 +141,13 @@ macro_rules! define_value_types {
impl fmt::Debug for Value<'_> {
fn fmt(&self, f: &mut fmt::Formatter<'_>) -> fmt::Result {
match self.tag() {
// SAFETY: `tag()` is `None`, meaning the `RawBox` holds a
// float, so `float()` is guaranteed to be `Some`.
None => write!(f, "Float({:?})", unsafe {
self.raw.float().unwrap_unchecked()
}),
// SAFETY: `tag()` matched `$itag`, so `downcast` to the
// corresponding inline type is guaranteed to be `Some`.
$(Some($itag) => write!(f, "{}({:?})", $iname, unsafe {
self.downcast::<$itype>().unwrap_unchecked()
}),)*
@@ -171,6 +193,8 @@ impl Storable for f64 {
}
#[inline(always)]
unsafe fn from_raw_box(raw: RawBox) -> Self {
// SAFETY: the caller guarantees `raw` represents a valid `f64`, so
// `float()` is guaranteed to be `Some`.
unsafe { raw.float().copied().unwrap_unchecked() }
}
}
@@ -180,6 +204,12 @@ impl ValueVariant for f64 {
const TYPE: NixType = NixType::Float;
}
impl<'gc, T: Storable + 'gc> From<T> for Value<'gc> {
fn from(value: T) -> Self {
Value::new(value)
}
}
/// # Nix runtime value representation
///
/// NaN-boxed value fitting in 8 bytes.
@@ -206,7 +236,10 @@ impl<'gc> Value<'gc> {
impl<'gc> Value<'gc> {
#[inline]
#[allow(private_bounds)]
#[expect(
private_bounds,
reason = "Storable is a sealed implementation detail of the value system"
)]
pub fn new<T: Storable>(val: T) -> Self {
Self {
raw: val.to_raw_box(),
@@ -230,7 +263,10 @@ impl<'gc> Value<'gc> {
#[inline]
pub fn downcast<T: ValueVariant>(self) -> Option<T::Ty<'gc>> {
self.is::<T>().then(|| unsafe { T::Ty::from_raw_box(self.raw) })
self.is::<T>()
// SAFETY: `is::<T>()` returned true, so `self.raw` represents a
// valid `T::Ty`.
.then(|| unsafe { T::Ty::from_raw_box(self.raw) })
}
#[inline]
@@ -259,6 +295,10 @@ impl<'gc> Value<'gc> {
}
#[inline]
#[expect(
clippy::unreachable,
reason = "the preceding `if`/`else if` chain exhausts every registered value tag"
)]
pub fn ty(self) -> NixType {
if self.is::<f64>() {
NixType::Float
@@ -316,7 +356,10 @@ impl<'gc> From<StaticValue> for Value<'gc> {
impl StaticValue {
#[inline]
#[allow(private_bounds)]
#[expect(
private_bounds,
reason = "Storable is a sealed implementation detail of the value system"
)]
pub fn new<T: Storable + 'static>(val: T) -> Self {
Self(Value::new(val))
}
@@ -429,6 +472,10 @@ impl<'gc> AttrSet<'gc> {
Self { entries }
}
#[expect(
clippy::indexing_slicing,
reason = "index comes from a successful `binary_search_by_key`, so it is a valid entry index"
)]
pub fn lookup(&self, key: StringId) -> Option<Value<'gc>> {
self.entries
.binary_search_by_key(&key, |(k, _)| *k)
@@ -440,6 +487,10 @@ impl<'gc> AttrSet<'gc> {
self.entries.binary_search_by_key(&key, |(k, _)| *k).is_ok()
}
#[expect(
clippy::indexing_slicing,
reason = "`i`/`j` stay strictly below their lengths inside the loop, and the trailing slices use those in-bounds cursors as start indices"
)]
pub fn merge(&self, other: &Self, mc: &Mutation<'gc>) -> Gc<'gc, Self> {
use std::cmp::Ordering::*;
@@ -500,6 +551,9 @@ impl<'gc> List<'gc> {
impl<'gc> Unlock for List<'gc> {
type Unlocked = RefCell<SmallVec<[Value<'gc>; 4]>>;
unsafe fn unlock_unchecked(&self) -> &Self::Unlocked {
// SAFETY: the caller upholds the `Unlock` contract (mutation happens
// behind a write barrier); we forward that obligation to the inner
// `RefLock`'s `unlock_unchecked`.
unsafe { self.inner.unlock_unchecked() }
}
}
@@ -539,6 +593,10 @@ impl<'gc> Env<'gc> {
}
}
#[expect(
clippy::indexing_slicing,
reason = "`locals` was just created with `1 + n_locals` elements, so index 0 is always valid"
)]
pub fn with_arg(arg: Value<'gc>, n_locals: u32, prev: Gc<'gc, RefLock<Env<'gc>>>) -> Self {
let mut locals = smallvec::smallvec![Value::default(); 1 + n_locals as usize];
locals[0] = arg;
@@ -576,6 +634,18 @@ pub struct PrimOp {
pub dispatch_ip: u32,
}
impl From<BuiltinId> for PrimOp {
fn from(id: BuiltinId) -> Self {
let BuiltinInfo { arity, .. } = id.info();
let dispatch_ip = Continuation::entry_for_builtin(id).ip();
Self {
id,
arity,
dispatch_ip,
}
}
}
impl RawStore for PrimOp {
fn to_val(self, value: &mut RawValue) {
let bytes = self.dispatch_ip.to_le_bytes();