diff --git a/Cargo.lock b/Cargo.lock index 609ad95..9a8357b 100644 --- a/Cargo.lock +++ b/Cargo.lock @@ -458,6 +458,7 @@ dependencies = [ "miette", "mimalloc", "rustyline", + "serial_test", "string-interner", "tempfile", "test-log", @@ -564,6 +565,41 @@ version = "0.2.0" source = "registry+https://github.com/rust-lang/crates.io-index" checksum = "77ce24cb58228fbb8aa041425bb1050850ac19177686ea6e0f41a70416f56fdb" +[[package]] +name = "futures-core" +version = "0.3.33" +source = "registry+https://github.com/rust-lang/crates.io-index" +checksum = "2cd50c473c80f6d7c3670a752354b8e569b1a7cbfdc0419ec88e5edad85e0dc7" + +[[package]] +name = "futures-executor" +version = "0.3.33" +source = "registry+https://github.com/rust-lang/crates.io-index" +checksum = "6754879cc9f2c66f88c6e5c35344bb0bdb0708b0352b1201815667c7eabc7458" +dependencies = [ + "futures-core", + "futures-task", + "futures-util", +] + +[[package]] +name = "futures-task" +version = "0.3.33" +source = "registry+https://github.com/rust-lang/crates.io-index" +checksum = "b231ed28831efb4a61a08580c4bc233ec56bc009f4cd8f52da2c3cb97df0c109" + +[[package]] +name = "futures-util" +version = "0.3.33" +source = "registry+https://github.com/rust-lang/crates.io-index" +checksum = "a77a90a256fce34da66415271e30f94ee91c57b04b8a2c042d9cf3220179deaa" +dependencies = [ + "futures-core", + "futures-task", + "pin-project-lite", + "slab", +] + [[package]] name = "gc-arena" version = "0.5.3" @@ -752,6 +788,15 @@ version = "0.12.1" source = "registry+https://github.com/rust-lang/crates.io-index" checksum = "32a66949e030da00e8c7d4434b251670a91556f4144941d37452769c25d58a53" +[[package]] +name = "lock_api" +version = "0.4.14" +source = "registry+https://github.com/rust-lang/crates.io-index" +checksum = "224399e74b87b5f3557511d98dff8b14089b3dadafcab6bb93eab67d3aace965" +dependencies = [ + "scopeguard", +] + [[package]] name = "log" version = "0.4.29" @@ -976,6 +1021,29 @@ dependencies = [ "winapi", ] +[[package]] +name = "parking_lot" +version = "0.12.5" +source = "registry+https://github.com/rust-lang/crates.io-index" +checksum = "93857453250e3077bd71ff98b6a65ea6621a19bb0f559a85248955ac12c45a1a" +dependencies = [ + "lock_api", + "parking_lot_core", +] + +[[package]] +name = "parking_lot_core" +version = "0.9.12" +source = "registry+https://github.com/rust-lang/crates.io-index" +checksum = "2621685985a2ebf1c516881c026032ac7deafcda1a2c9b7850dc81e3dfcb64c1" +dependencies = [ + "cfg-if", + "libc", + "redox_syscall", + "smallvec", + "windows-link", +] + [[package]] name = "pin-project-lite" version = "0.2.17" @@ -1094,6 +1162,15 @@ dependencies = [ "crossbeam-utils", ] +[[package]] +name = "redox_syscall" +version = "0.5.18" +source = "registry+https://github.com/rust-lang/crates.io-index" +checksum = "ed2bf2547551a7053d6fdfafda3f938979645c44812fbfcda098faae3f1a362d" +dependencies = [ + "bitflags", +] + [[package]] name = "regex" version = "1.12.3" @@ -1205,6 +1282,12 @@ dependencies = [ "winapi-util", ] +[[package]] +name = "scopeguard" +version = "1.2.0" +source = "registry+https://github.com/rust-lang/crates.io-index" +checksum = "94143f37725109f92c262ed2cf5e59bce7498c01bcc1502d7b9afe439a4e9f49" + [[package]] name = "semver" version = "1.0.28" @@ -1254,6 +1337,31 @@ dependencies = [ "zmij", ] +[[package]] +name = "serial_test" +version = "3.5.0" +source = "registry+https://github.com/rust-lang/crates.io-index" +checksum = "699f4197115b8a7e7ff19c9a315a4bd6fffec26cc4626ef45ecaea389e081c6d" +dependencies = [ + "futures-executor", + "futures-util", + "log", + "once_cell", + "parking_lot", + "serial_test_derive", +] + +[[package]] +name = "serial_test_derive" +version = "3.5.0" +source = "registry+https://github.com/rust-lang/crates.io-index" +checksum = "94e153fc76e1c6a068703d6d29c508a0b15c061c4b7e43da59cc097bc342673c" +dependencies = [ + "proc-macro2", + "quote", + "syn", +] + [[package]] name = "sharded-slab" version = "0.1.7" @@ -1269,6 +1377,12 @@ version = "1.3.0" source = "registry+https://github.com/rust-lang/crates.io-index" checksum = "0fda2ff0d084019ba4d7c6f371c95d8fd75ce3524c3cb8fb653a3023f6323e64" +[[package]] +name = "slab" +version = "0.4.12" +source = "registry+https://github.com/rust-lang/crates.io-index" +checksum = "0c790de23124f9ab44544d7ac05d60440adc586479ce501c1d6d7da3cd8c9cf5" + [[package]] name = "smallvec" version = "1.15.1" diff --git a/Cargo.toml b/Cargo.toml index ff8ad6b..4823c7b 100644 --- a/Cargo.toml +++ b/Cargo.toml @@ -34,6 +34,70 @@ git = "https://github.com/kyren/gc-arena" rev = "75671ae03f53718357b741ed4027560f14e90836" features = ["allocator-api2", "hashbrown", "smallvec"] +# https://emschwartz.me/your-clippy-config-should-be-stricter/ +# tombi: format.rules.table-keys-order.disabled = true +[workspace.lints.clippy] +# Don't Panic - prevent panics from unwraps and unsafe slicing or indexing +string_slice = "warn" +indexing_slicing = "warn" +unwrap_used = "warn" +panic = "warn" +# WIP evaluator: several features are deliberately stubbed with todo!/unimplemented! +todo = "allow" +unimplemented = "allow" +unreachable = "warn" +get_unwrap = "warn" +unwrap_in_result = "warn" +unchecked_time_subtraction = "warn" +panic_in_result_fn = "warn" +# Optional - see post for caveats +# expect_used = "warn" +# arithmetic_side_effects = "warn" + +# Don't Fail Silently - prevent dropped futures and swallowed errors +let_underscore_future = "warn" +let_underscore_must_use = "warn" +unused_result_ok = "warn" +map_err_ignore = "warn" +assertions_on_result_states = "warn" + +# Don't Do Bad Async Stuff - prevent deadlocks and concurrency bugs +await_holding_lock = "warn" +await_holding_refcell_ref = "warn" +if_let_mutex = "warn" # only relevant on editions before 2024 +large_futures = "warn" + +# Don't Do Unsafe Things with Memory +mem_forget = "warn" +undocumented_unsafe_blocks = "warn" +multiple_unsafe_ops_per_block = "warn" +unnecessary_safety_doc = "warn" +unnecessary_safety_comment = "warn" + +# Don't Do Potentially Incorrect Things with Numbers +float_cmp = "warn" +float_cmp_const = "warn" +lossy_float_literal = "warn" +cast_sign_loss = "warn" +invalid_upcast_comparisons = "warn" +# Optional - these effectively force you to document numeric invariants +# cast_possible_wrap = "warn" +# cast_precision_loss = "warn" +# cast_possible_truncation = "warn" + +# Don't Do Bad Things That are Easy to Avoid +rc_mutex = "warn" +debug_assert_with_mut_call = "warn" +iter_not_returning_iterator = "warn" +expl_impl_clone_on_copy = "warn" +infallible_try_from = "warn" +dbg_macro = "warn" + +# Don't `allow` Your Way Around These Lints - every suppression must be +# a deliberate #[expect(..., reason = "…")] rather than a silent #[allow] +allow_attributes = "warn" +allow_attributes_without_reason = "warn" + [profile.lto] inherits = "release" lto = true diff --git a/clippy.toml b/clippy.toml new file mode 100644 index 0000000..66eb2d7 --- /dev/null +++ b/clippy.toml @@ -0,0 +1,5 @@ +allow-indexing-slicing-in-tests = true +allow-panic-in-tests = true +allow-unwrap-in-tests = true +allow-expect-in-tests = true +allow-dbg-in-tests = true diff --git a/fix-bytecode/Cargo.toml b/fix-bytecode/Cargo.toml index 6961263..329dfb2 100644 --- a/fix-bytecode/Cargo.toml +++ b/fix-bytecode/Cargo.toml @@ -9,3 +9,6 @@ num_enum = { workspace = true } string-interner = { workspace = true } fix-lang = { path = "../fix-lang" } + +[lints] +workspace = true diff --git a/fix-bytecode/src/disassembler.rs b/fix-bytecode/src/disassembler.rs index c5e27ec..6979ca0 100644 --- a/fix-bytecode/src/disassembler.rs +++ b/fix-bytecode/src/disassembler.rs @@ -2,7 +2,7 @@ use std::fmt::Write; use colored::Colorize as _; -use crate::{Continuation, InstructionPtr, Op, OperandType}; +use crate::{BytecodeReader, Continuation, InstructionPtr, Op}; pub trait DisassemblerContext { fn resolve_string(&self, id: u32) -> &str; @@ -10,99 +10,15 @@ pub trait DisassemblerContext { } pub struct Disassembler<'a, Ctx> { - code: &'a [u8], + reader: BytecodeReader<'a>, ctx: &'a Ctx, - pc: usize, } impl<'a, Ctx: DisassemblerContext> Disassembler<'a, Ctx> { pub fn new(ip: InstructionPtr, ctx: &'a Ctx) -> Self { Self { - code: ctx.get_code(), + reader: BytecodeReader::new(ctx.get_code(), ip.0), ctx, - pc: ip.0, - } - } - - #[inline(always)] - fn read_u8(&mut self) -> u8 { - let b = self.code[self.pc]; - self.pc += 1; - b - } - - #[inline(always)] - fn read_u16(&mut self) -> u16 { - let bytes = self.code[self.pc..self.pc + 2] - .try_into() - .expect("no enough bytes"); - self.pc += 2; - u16::from_le_bytes(bytes) - } - - #[inline(always)] - fn read_u32(&mut self) -> u32 { - let bytes = self.code[self.pc..self.pc + 4] - .try_into() - .expect("no enough bytes"); - self.pc += 4; - u32::from_le_bytes(bytes) - } - - #[inline(always)] - fn read_i32(&mut self) -> i32 { - let bytes = self.code[self.pc..self.pc + 4] - .try_into() - .expect("no enough bytes"); - self.pc += 4; - i32::from_le_bytes(bytes) - } - - #[inline(always)] - fn read_i64(&mut self) -> i64 { - let bytes = self.code[self.pc..self.pc + 8] - .try_into() - .expect("no enough bytes"); - self.pc += 8; - i64::from_le_bytes(bytes) - } - - #[inline(always)] - fn read_f64(&mut self) -> f64 { - let bytes = self.code[self.pc..self.pc + 8] - .try_into() - .expect("no enough bytes"); - self.pc += 8; - f64::from_le_bytes(bytes) - } - - #[inline(always)] - fn read_operand_data(&mut self) { - use OperandType::*; - let tag = self.read_u8(); - let ty = OperandType::try_from(tag).expect("invalid operand type"); - match ty { - Const => { - self.read_u32(); - } - BigInt => { - self.read_i64(); - } - Local => { - self.read_u8(); - self.read_u32(); - } - BuiltinConst => { - self.read_u32(); - } - Builtins => {} - ReplBinding => { - self.read_u32(); - } - ScopedImportBinding => { - self.read_u32(); - self.read_u32(); - } } } @@ -114,6 +30,11 @@ impl<'a, Ctx: DisassemblerContext> Disassembler<'a, Ctx> { self.disassemble_impl(true) } + #[expect( + clippy::let_underscore_must_use, + clippy::indexing_slicing, + reason = "disassembler operates on well-formed bytecode; writes target an in-memory String via fmt::Write, which is infallible" + )] fn disassemble_impl(&mut self, color: bool) -> String { let mut out = String::new(); if color { @@ -122,24 +43,24 @@ impl<'a, Ctx: DisassemblerContext> Disassembler<'a, Ctx> { out, "{} {}", "Length:".white(), - format!("{} bytes", self.code.len()).cyan() + format!("{} bytes", self.reader.len()).cyan() ); } else { let _ = writeln!(out, "=== Bytecode Disassembly ==="); - let _ = writeln!(out, "Length: {} bytes", self.code.len()); + let _ = writeln!(out, "Length: {} bytes", self.reader.len()); } - while self.pc < self.code.len() { - let start_pos = self.pc; - let op_byte = self.read_u8(); + while self.reader.pc() < self.reader.len() { + let start_pos = self.reader.pc(); + let op_byte = self.reader.read_u8(); let (mnemonic, args) = self.decode_instruction(op_byte, start_pos); - let bytes_slice = &self.code[start_pos + 1..self.pc]; + let bytes_slice = &self.reader[start_pos + 1..self.reader.pc()]; let mut chunks = bytes_slice.chunks(4); let first_chunk = chunks.next().unwrap_or(&[]); let bytes_str = { - let mut temp = format!("{:02x}", self.code[start_pos]); + let mut temp = format!("{:02x}", self.reader[start_pos]); for b in first_chunk { let _ = write!(&mut temp, " {:02x}", b); } @@ -199,31 +120,36 @@ impl<'a, Ctx: DisassemblerContext> Disassembler<'a, Ctx> { out } + #[expect( + clippy::let_underscore_must_use, + clippy::cast_sign_loss, + reason = "disassembler operates on well-formed bytecode; jump targets are valid non-negative PCs and writes to String are infallible" + )] fn decode_instruction(&mut self, op_byte: u8, current_pc: usize) -> (&'static str, String) { let op = Op::try_from(op_byte).expect("invalid op code"); match op { Op::PushSmi => { - let val = self.read_i32(); + let val = self.reader.read_i32(); ("PushSmi", format!("{}", val)) } Op::PushBigInt => { - let val = self.read_i64(); + let val = self.reader.read_i64(); ("PushBigInt", format!("{}", val)) } Op::PushFloat => { - let val = self.read_f64(); + let val = self.reader.read_f64(); ("PushFloat", format!("{}", val)) } Op::PushString => { - let idx = self.read_u32(); + let idx = self.reader.read_u32(); let s = self.ctx.resolve_string(idx); let len = s.len(); let mut s_fmt = format!("{:?}", s); if s_fmt.len() > 60 { s_fmt.truncate(57); - #[allow(clippy::unwrap_used)] - write!(s_fmt, "...\" (total {len} bytes)").unwrap(); + write!(s_fmt, "...\" (total {len} bytes)") + .expect("writing to String is infallible"); } ("PushString", format!("@{} {}", idx, s_fmt)) } @@ -232,38 +158,38 @@ impl<'a, Ctx: DisassemblerContext> Disassembler<'a, Ctx> { Op::PushFalse => ("PushFalse", String::new()), Op::LoadLocal => { - let idx = self.read_u32(); + let idx = self.reader.read_u32(); ("LoadLocal", format!("[{}]", idx)) } Op::LoadOuter => { - let depth = self.read_u8(); - let idx = self.read_u32(); + let depth = self.reader.read_u8(); + let idx = self.reader.read_u32(); ("LoadOuter", format!("depth={} [{}]", depth, idx)) } Op::StoreLocal => { - let idx = self.read_u32(); + let idx = self.reader.read_u32(); ("StoreLocal", format!("[{}]", idx)) } Op::AllocLocals => { - let count = self.read_u32(); + let count = self.reader.read_u32(); ("AllocLocals", format!("count={}", count)) } Op::MakeThunk => { - let offset = self.read_u32(); + let offset = self.reader.read_u32(); ("MakeThunk", format!("-> {:04x}", offset)) } Op::MakeClosure => { - let offset = self.read_u32(); - let slots = self.read_u32(); + let offset = self.reader.read_u32(); + let slots = self.reader.read_u32(); ("MakeClosure", format!("-> {:04x} slots={}", offset, slots)) } Op::MakePatternClosure => { - let offset = self.read_u32(); - let slots = self.read_u32(); - let req_count = self.read_u16(); - let opt_count = self.read_u16(); - let ellipsis = self.read_u8() != 0; + let offset = self.reader.read_u32(); + let slots = self.reader.read_u32(); + let req_count = self.reader.read_u16(); + let opt_count = self.reader.read_u16(); + let ellipsis = self.reader.read_u8() != 0; let mut arg_str = format!( "-> {:04x} slots={} req={} opt={} ...={})", @@ -272,18 +198,18 @@ impl<'a, Ctx: DisassemblerContext> Disassembler<'a, Ctx> { arg_str.push_str(" Args=["); for _ in 0..req_count { - let idx = self.read_u32(); + let idx = self.reader.read_u32(); arg_str.push_str(&format!("Req({}) ", self.ctx.resolve_string(idx))); } for _ in 0..opt_count { - let idx = self.read_u32(); + let idx = self.reader.read_u32(); arg_str.push_str(&format!("Opt({}) ", self.ctx.resolve_string(idx))); } let total_args = req_count + opt_count; for _ in 0..total_args { - let _name_idx = self.read_u32(); - let _span_id = self.read_u32(); + let _name_idx = self.reader.read_u32(); + let _span_id = self.reader.read_u32(); } arg_str.push(']'); @@ -291,31 +217,32 @@ impl<'a, Ctx: DisassemblerContext> Disassembler<'a, Ctx> { } Op::Call => { - self.read_operand_data(); + let _ = self.reader.read_operand_data(); ("Call", "arg=?".into()) } Op::DispatchCont => { - let phase = Continuation::try_from(self.read_u8()).expect("invalid primop phase"); + let phase = + Continuation::try_from(self.reader.read_u8()).expect("invalid primop phase"); ("DispatchPrimOp", format!("phase={phase:?}")) } Op::MakeAttrs => { - let static_count = self.read_u32(); - let dynamic_count = self.read_u32(); + let static_count = self.reader.read_u32(); + let dynamic_count = self.reader.read_u32(); let mut args = format!("static={} dynamic={}", static_count, dynamic_count); for _ in 0..static_count { - let key_id = self.read_u32(); + let key_id = self.reader.read_u32(); let _ = write!(args, " [{}={}", self.ctx.resolve_string(key_id), key_id); - self.read_operand_data(); - let _span_id = self.read_u32(); + let _ = self.reader.read_operand_data(); + let _span_id = self.reader.read_u32(); args.push(']'); } for _ in 0..dynamic_count { let _ = write!(args, " [dyn"); - self.read_operand_data(); - let _span_id = self.read_u32(); + let _ = self.reader.read_operand_data(); + let _span_id = self.reader.read_u32(); args.push(']'); } @@ -324,31 +251,31 @@ impl<'a, Ctx: DisassemblerContext> Disassembler<'a, Ctx> { Op::MakeEmptyAttrs => ("MakeEmptyAttrs", String::new()), Op::SelectStatic => { - let span_id = self.read_u32(); - let key_id = self.read_u32(); + let span_id = self.reader.read_u32(); + let key_id = self.reader.read_u32(); ( "SelectStatic", format!("key={} span={}", self.ctx.resolve_string(key_id), span_id), ) } Op::SelectDynamic => { - let span_id = self.read_u32(); + let span_id = self.reader.read_u32(); ("SelectDynamic", format!("span={}", span_id)) } Op::HasAttrPathStatic => { - let span_id = self.read_u32(); - let key_id = self.read_u32(); + let span_id = self.reader.read_u32(); + let key_id = self.reader.read_u32(); ( "HasAttrPathStatic", format!("key={} span={}", self.ctx.resolve_string(key_id), span_id), ) } Op::HasAttrPathDynamic => { - let span_id = self.read_u32(); + let span_id = self.reader.read_u32(); ("HasAttrPathDynamic", format!("span={}", span_id)) } Op::HasAttrStatic => { - let key_id = self.read_u32(); + let key_id = self.reader.read_u32(); ( "HasAttrStatic", format!("key={}", self.ctx.resolve_string(key_id)), @@ -357,7 +284,7 @@ impl<'a, Ctx: DisassemblerContext> Disassembler<'a, Ctx> { Op::HasAttrDynamic => ("HasAttrDynamic", String::new()), Op::HasAttrResolve => ("HasAttrResolve", String::new()), Op::JumpIfSelectSucceeded => { - let offset = self.read_i32(); + let offset = self.reader.read_i32(); let target = (current_pc as isize + 1 + 4 + offset as isize) as usize; ( "JumpIfSelectSucceeded", @@ -365,7 +292,7 @@ impl<'a, Ctx: DisassemblerContext> Disassembler<'a, Ctx> { ) } Op::JumpIfSelectFailed => { - let offset = self.read_i32(); + let offset = self.reader.read_i32(); let target = (current_pc as isize + 1 + 4 + offset as isize) as usize; ( "JumpIfSelectFailed", @@ -374,9 +301,9 @@ impl<'a, Ctx: DisassemblerContext> Disassembler<'a, Ctx> { } Op::MakeList => { - let count = self.read_u32(); + let count = self.reader.read_u32(); for _ in 0..count { - self.read_operand_data(); + let _ = self.reader.read_operand_data(); } ("MakeList", format!("size={}", count)) } @@ -398,7 +325,7 @@ impl<'a, Ctx: DisassemblerContext> Disassembler<'a, Ctx> { Op::OpNot => ("OpNot", String::new()), Op::JumpIfFalse => { - let offset = self.read_i32(); + let offset = self.reader.read_i32(); let target = (current_pc as isize + 1 + 4 + offset as isize) as usize; ( "JumpIfFalse", @@ -406,55 +333,55 @@ impl<'a, Ctx: DisassemblerContext> Disassembler<'a, Ctx> { ) } Op::JumpIfTrue => { - let offset = self.read_i32(); + let offset = self.reader.read_i32(); let target = (current_pc as isize + 1 + 4 + offset as isize) as usize; ("JumpIfTrue", format!("-> {:04x} offset={}", target, offset)) } Op::Jump => { - let offset = self.read_i32(); + let offset = self.reader.read_i32(); let target = (current_pc as isize + 1 + 4 + offset as isize) as usize; ("Jump", format!("-> {:04x} offset={}", target, offset)) } Op::ConcatStrings => { - let count = self.read_u16(); - let force = self.read_u8(); + let count = self.reader.read_u16(); + let force = self.reader.read_u8(); ("ConcatStrings", format!("count={} force={}", count, force)) } Op::CoerceToString => ("CoerceToString", String::new()), Op::ResolvePath => { - let dir_id = self.read_u32(); + let dir_id = self.reader.read_u32(); let dir = self.ctx.resolve_string(dir_id); ("ResolvePath", format!("dir={:?}", dir)) } Op::Assert => { - let raw_idx = self.read_u32(); - let span_id = self.read_u32(); + let raw_idx = self.reader.read_u32(); + let span_id = self.reader.read_u32(); ("Assert", format!("text_id={} span={}", raw_idx, span_id)) } Op::LookupWith => { - let idx = self.read_u32(); + let idx = self.reader.read_u32(); let name = self.ctx.resolve_string(idx); - let n = self.read_u8(); + let n = self.reader.read_u8(); for _ in 0..n { - self.read_operand_data(); + let _ = self.reader.read_operand_data(); } ("LookupWith", format!("sym={:?} n={}", name, n)) } Op::LoadBuiltins => ("LoadBuiltins", String::new()), Op::LoadBuiltin => { - let id = self.read_u8(); + let id = self.reader.read_u8(); ("LoadBuiltin", format!("id={}", id)) } Op::LoadReplBinding => { - let idx = self.read_u32(); + let idx = self.reader.read_u32(); let name = self.ctx.resolve_string(idx); ("LoadReplBinding", format!("{:?}", name)) } Op::LoadScopedBinding => { - let slot = self.read_u32(); - let idx = self.read_u32(); + let slot = self.reader.read_u32(); + let idx = self.reader.read_u32(); let name = self.ctx.resolve_string(idx); ("LoadScopedBinding", format!("slot={} {:?}", slot, name)) } diff --git a/fix-bytecode/src/lib.rs b/fix-bytecode/src/lib.rs index 46935bd..a04f37e 100644 --- a/fix-bytecode/src/lib.rs +++ b/fix-bytecode/src/lib.rs @@ -1,4 +1,7 @@ -#![allow(dead_code)] +#![allow( + dead_code, + reason = "crate is under active development; some opcodes and helpers are not yet wired up" +)] use fix_lang::{BuiltinId, StringId}; use num_enum::TryFromPrimitive; @@ -10,8 +13,7 @@ pub mod disassembler; pub struct InstructionPtr(pub usize); #[repr(u8)] -#[derive(Debug, Clone, Copy, TryFromPrimitive)] -#[allow(clippy::enum_variant_names)] +#[derive(Debug, Clone, Copy)] pub enum Op { PushSmi, PushBigInt, @@ -88,6 +90,21 @@ pub enum Op { Illegal, } +impl TryFrom for Op { + type Error = u8; + + #[inline(always)] + fn try_from(value: u8) -> Result { + if (0..Self::Illegal as u8).contains(&value) { + // SAFETY: `value` is in `0..Illegal`, which are exactly the valid + // discriminants of this `#[repr(u8)]` enum, so the transmute is sound. + Ok(unsafe { std::mem::transmute::(value) }) + } else { + Err(value) + } + } +} + #[repr(u8)] #[derive(Debug, Clone, Copy, TryFromPrimitive)] pub enum OperandType { @@ -106,11 +123,7 @@ pub enum Const { Bool(bool), String(StringId), Path(StringId), - PrimOp { - id: BuiltinId, - arity: u8, - dispatch_ip: u32, - }, + PrimOp(BuiltinId), Null, } @@ -286,8 +299,11 @@ pub enum Continuation { impl TryFrom for Continuation { type Error = u8; + #[inline(always)] fn try_from(value: u8) -> Result { if (0..Self::Illegal as u8).contains(&value) { + // SAFETY: `value` is in `0..Illegal`, which are exactly the valid + // discriminants of this `#[repr(u8)]` enum, so the transmute is sound. Ok(unsafe { std::mem::transmute::(value) }) } else { Err(value) @@ -296,7 +312,7 @@ impl TryFrom for Continuation { } impl Continuation { - pub fn entry_for_builtin(id: BuiltinId) -> Self { + pub const fn entry_for_builtin(id: BuiltinId) -> Self { use BuiltinId::*; match id { Abort => Self::PAbort, @@ -405,7 +421,7 @@ impl Continuation { } } - pub fn ip(self) -> u32 { + pub const fn ip(self) -> u32 { self as u32 * 2 } } @@ -416,7 +432,129 @@ pub struct BytecodeReader<'a> { inst_start_pc: usize, } +impl std::ops::Deref for BytecodeReader<'_> { + type Target = [u8]; + fn deref(&self) -> &Self::Target { + self.bytecode + } +} + +pub trait FromBytecode { + #[must_use] + fn read(reader: &mut BytecodeReader<'_>) -> Self; +} + +impl FromBytecode for [u8; N] { + #[inline(always)] + #[expect( + clippy::indexing_slicing, + reason = "reads well-formed bytecode; an out-of-range PC indicates a codegen bug and should panic loudly" + )] + fn read(reader: &mut BytecodeReader<'_>) -> Self { + let ret = reader.bytecode[reader.pc..reader.pc + N] + .try_into() + .expect("read_array failed"); + reader.pc += N; + ret + } +} + +impl FromBytecode for u8 { + #[inline(always)] + fn read(reader: &mut BytecodeReader<'_>) -> Self { + let [byte] = reader.read(); + byte + } +} + +macro_rules! numbers_impl { + ( $($ty:ty),* ) => { + $( + impl FromBytecode for $ty { + #[inline(always)] + fn read(reader: &mut BytecodeReader<'_>) -> Self { + <$ty>::from_le_bytes(FromBytecode::read(reader)) + } + } + )* + }; +} + +numbers_impl!(u16, u32, i32, i64, f64); + +impl FromBytecode for Op { + #[inline(always)] + #[expect( + clippy::panic, + reason = "reads well-formed bytecode; an unknown opcode indicates a codegen bug and should panic loudly" + )] + fn read(reader: &mut BytecodeReader<'_>) -> Self { + reader.inst_start_pc = reader.pc; + let byte: u8 = reader.read(); + byte.try_into().unwrap_or_else(|byte| { + std::hint::cold_path(); + panic!("unknown opcode: {byte:#04x}") + }) + } +} + +impl FromBytecode for StringId { + #[inline(always)] + fn read(reader: &mut BytecodeReader<'_>) -> Self { + let raw: u32 = reader.read(); + StringId( + string_interner::symbol::SymbolU32::try_from_usize(raw as usize) + .expect("raw is a valid interner symbol index"), + ) + } +} + +impl FromBytecode for OperandData { + #[inline(always)] + #[expect( + clippy::panic, + reason = "an unknown operand tag in well-formed bytecode indicates a codegen bug and should panic loudly" + )] + fn read(reader: &mut BytecodeReader<'_>) -> Self { + let tag = reader.read(); + let ty = OperandType::try_from_primitive(tag).unwrap_or_else(|err| { + std::hint::cold_path(); + panic!("unknown operand tag: {:#04x}", err.number) + }); + match ty { + OperandType::Const => OperandData::Const(reader.read()), + OperandType::BigInt => OperandData::BigInt(reader.read()), + OperandType::Local => { + let layer = reader.read(); + let idx = reader.read(); + OperandData::Local { layer, idx } + } + OperandType::BuiltinConst => OperandData::BuiltinConst(reader.read()), + OperandType::Builtins => OperandData::Builtins, + OperandType::ReplBinding => OperandData::ReplBinding(reader.read()), + OperandType::ScopedImportBinding => { + let slot_id = reader.read(); + let name = reader.read(); + OperandData::ScopedImportBinding { slot_id, name } + } + } + } +} + +macro_rules! read_aliases { + { $($alias:ident -> $ty:ty),*$(,)? } => { + $( + #[inline(always)] + #[must_use] + pub fn $alias(&mut self) -> $ty { + <$ty>::read(self) + } + )* + }; +} + impl<'a> BytecodeReader<'a> { + #[must_use] pub fn new(bytecode: &'a [u8], pc: usize) -> Self { Self { bytecode, @@ -426,6 +564,7 @@ impl<'a> BytecodeReader<'a> { } #[inline(always)] + #[must_use] pub fn from_after_op(bytecode: &'a [u8], inst_start_pc: usize) -> Self { Self { bytecode, @@ -435,88 +574,21 @@ impl<'a> BytecodeReader<'a> { } #[inline(always)] - #[cfg_attr(debug_assertions, track_caller)] - fn read_array(&mut self) -> [u8; N] { - let ret = self.bytecode[self.pc..self.pc + N] - .try_into() - .expect("read_array failed"); - self.pc += N; - ret + #[must_use] + pub fn read(&mut self) -> T { + T::read(self) } - #[inline(always)] - pub fn read_op(&mut self) -> Op { - self.inst_start_pc = self.pc; - let byte = self.bytecode[self.pc]; - if !(0..Op::Illegal as u8).contains(&byte) { - std::hint::cold_path(); - panic!("unknown opcode: {byte:#04x}") - } - self.pc += 1; - unsafe { std::mem::transmute::(byte) } - } - - #[inline(always)] - pub fn read_u8(&mut self) -> u8 { - let val = self.bytecode[self.pc]; - self.pc += 1; - val - } - - #[inline(always)] - pub fn read_u16(&mut self) -> u16 { - u16::from_le_bytes(self.read_array()) - } - - #[inline(always)] - pub fn read_u32(&mut self) -> u32 { - u32::from_le_bytes(self.read_array()) - } - - #[inline(always)] - pub fn read_i32(&mut self) -> i32 { - i32::from_le_bytes(self.read_array()) - } - - #[inline(always)] - pub fn read_i64(&mut self) -> i64 { - i64::from_le_bytes(self.read_array()) - } - - #[inline(always)] - pub fn read_f64(&mut self) -> f64 { - f64::from_le_bytes(self.read_array()) - } - - #[inline(always)] - pub 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()) - } - - #[inline(always)] - pub fn read_operand_data(&mut self) -> OperandData { - let tag = self.read_u8(); - let Ok(ty) = OperandType::try_from_primitive(tag) - .map_err(|err| panic!("unknown operand tag: {:#04x}", err.number)); - match ty { - OperandType::Const => OperandData::Const(self.read_u32()), - OperandType::BigInt => OperandData::BigInt(self.read_i64()), - OperandType::Local => { - let layer = self.read_u8(); - let idx = self.read_u32(); - OperandData::Local { layer, idx } - } - OperandType::BuiltinConst => OperandData::BuiltinConst(self.read_string_id()), - OperandType::Builtins => OperandData::Builtins, - OperandType::ReplBinding => OperandData::ReplBinding(self.read_string_id()), - OperandType::ScopedImportBinding => { - let slot_id = self.read_u32(); - let name = self.read_string_id(); - OperandData::ScopedImportBinding { slot_id, name } - } - } + read_aliases! { + read_op -> Op, + read_u8 -> u8, + read_u16 -> u16, + read_u32 -> u32, + read_i32 -> i32, + read_i64 -> i64, + read_f64 -> f64, + read_string_id -> StringId, + read_operand_data -> OperandData, } pub fn pc(&self) -> usize { diff --git a/fix-compiler/Cargo.toml b/fix-compiler/Cargo.toml index 9c04e4f..6ee0448 100644 --- a/fix-compiler/Cargo.toml +++ b/fix-compiler/Cargo.toml @@ -17,3 +17,6 @@ fix-error = { path = "../fix-error" } fix-lang = { path = "../fix-lang" } fix-runtime = { path = "../fix-runtime" } tracing = "0.1" + +[lints] +workspace = true diff --git a/fix-compiler/src/context.rs b/fix-compiler/src/context.rs index 9da2deb..cf70155 100644 --- a/fix-compiler/src/context.rs +++ b/fix-compiler/src/context.rs @@ -114,8 +114,13 @@ impl<'a, R: VmRuntimeCtx> CompilerCtx<'a, R> { source, ); let ir = downgrade_ctx.downgrade_toplevel(expr)?; + // SAFETY: `ir` borrows from `bump`, which is moved into the returned + // `OwnedIr` alongside the reference. The `'static` lifetime is a + // storage token that is re-narrowed to the `OwnedIr`'s own borrow in + // `as_ref`, so no reference ever outlives the arena backing it. let ir = unsafe { std::mem::transmute::, RawIrRef<'static>>(ir) }; - Ok(OwnedIr { _bump: bump, ir }) + // SAFETY: `ir` borrows from `bump` + Ok(unsafe { OwnedIr::new(bump, ir) }) }) } } @@ -153,15 +158,7 @@ impl<'a, R: VmRuntimeCtx> BytecodeContext for CompilerCtx<'a, R> { Bool(x) => StaticValue::new(x), String(x) => StaticValue::new(x), Path(x) => StaticValue::new(fix_runtime::Path(x)), - PrimOp { - id, - arity, - dispatch_ip, - } => StaticValue::new(fix_runtime::PrimOp { - id, - arity, - dispatch_ip, - }), + PrimOp(id) => StaticValue::new(fix_runtime::PrimOp::from(id)), Null => StaticValue::default(), }; self.runtime.add_const(val) @@ -288,6 +285,10 @@ impl<'ctx: 'ir, 'id, 'ir, R: VmRuntimeCtx> DowngradeContext<'id, 'ir> self.runtime.resolve_string(id).into() } + #[expect( + clippy::unwrap_in_result, + reason = "exceeding u8::MAX thunk scope layers is a compiler invariant violation, not a user-facing error" + )] fn lookup( &mut self, sym: StringId, @@ -364,6 +365,10 @@ impl<'ctx: 'ir, 'id, 'ir, R: VmRuntimeCtx> DowngradeContext<'id, 'ir> self.source.clone() } + #[expect( + clippy::panic_in_result_fn, + reason = "assert_eq guards an internal invariant (bindings and values have equal length); a mismatch is a compiler bug, not a user error" + )] fn with_let_scope(&mut self, keys: &[StringId], f: F) -> Result where F: FnOnce( @@ -425,6 +430,10 @@ impl<'ctx: 'ir, 'id, 'ir, R: VmRuntimeCtx> DowngradeContext<'id, 'ir> ret } + #[expect( + clippy::panic, + reason = "exceeding u8::MAX thunk scope layers is a compiler invariant violation, not a user-facing error" + )] fn with_thunk_scope( &mut self, f: F, @@ -490,7 +499,6 @@ enum Scope<'ctx, 'id, 'ir> { Repl(&'ctx HashSet), ScopedImport { keys: HashSet, - #[allow(dead_code)] slot_id: u32, }, Let(HashMap>), @@ -534,13 +542,29 @@ impl<'a, 'ctx, 'id, 'ir, R: VmRuntimeCtx> ScopeGuard<'a, 'ctx, 'id, 'ir, R> { } } -struct OwnedIr { - _bump: Bump, - ir: RawIrRef<'static>, -} +mod sealed { + use super::*; -impl OwnedIr { - fn as_ref<'ir>(&'ir self) -> RawIrRef<'ir> { - unsafe { std::mem::transmute::, RawIrRef<'ir>>(self.ir) } + pub struct OwnedIr { + _bump: Bump, + ir: RawIrRef<'static>, + } + + impl OwnedIr { + /// # Safety + /// `ir` must borrows from `bump` + pub unsafe fn new(bump: Bump, ir: RawIrRef<'static>) -> Self { + Self { _bump: bump, ir } + } + + pub fn as_ref<'ir>(&'ir self) -> RawIrRef<'ir> { + // SAFETY: `self.ir`'s `'static` lifetime is a storage token; the IR is + // backed by `self._bump`, which lives as long as `self`. Narrowing to + // `'ir` (tied to `&self`) hands out a reference that cannot outlive the + // arena. + unsafe { std::mem::transmute::, RawIrRef<'ir>>(self.ir) } + } } } + +pub use sealed::OwnedIr; diff --git a/fix-compiler/src/ir.rs b/fix-compiler/src/ir.rs index ab00623..db58097 100644 --- a/fix-compiler/src/ir.rs +++ b/fix-compiler/src/ir.rs @@ -3,7 +3,7 @@ use std::marker::PhantomData; use bumpalo::Bump; use bumpalo::collections::Vec; -use fix_lang::{BUILTINS, BuiltinId, StringId}; +use fix_lang::{BuiltinId, StringId}; use ghost_cell::{GhostCell, GhostToken}; use rnix::{TextRange, ast}; use string_interner::DefaultStringInterner; @@ -222,7 +222,6 @@ pub enum Ir<'ir, R: RefExt<'ir> + ?Sized + 'ir> { pub struct ThunkId(pub usize); /// Represents a key in an attribute path. -#[allow(unused)] #[derive(Debug)] pub enum Attr { /// A dynamic attribute key, which is an expression that must evaluate to a string. @@ -291,9 +290,8 @@ pub fn new_global_env( let builtins_sym = StringId(strings.get_or_intern("builtins")); global_env.insert(builtins_sym, MaybeThunk::Builtins); - for (idx, &(name, _)) in BUILTINS.iter().enumerate() { - let id = BuiltinId::try_from(idx as u8).expect("infallible"); - let name = StringId(strings.get_or_intern(name)); + for id in BuiltinId::ALL { + let name = StringId(strings.get_or_intern(id.info().global_name)); global_env.insert(name, MaybeThunk::Builtin(id)); } diff --git a/fix-compiler/src/ir/downgrade.rs b/fix-compiler/src/ir/downgrade.rs index 36d11cc..6c378a6 100644 --- a/fix-compiler/src/ir/downgrade.rs +++ b/fix-compiler/src/ir/downgrade.rs @@ -156,6 +156,10 @@ impl<'id: 'ir, 'ir, Ctx: DowngradeContext<'id, 'ir>> Downgrade<'id, 'ir, Ctx> fo let path = { let temp = self.content().require(ctx, span)?; let text = temp.text(); + #[expect( + clippy::string_slice, + reason = "PathSearch text is <...> wrapped in ASCII angle brackets, so byte indices 1 and len-1 are char boundaries" + )] let id = ctx.intern_string(&text[1..text.len() - 1]); let expr = ctx.new_expr(Ir::Str(id)); ctx.maybe_thunk(expr) @@ -463,15 +467,13 @@ impl<'id: 'ir, 'ir, Ctx: DowngradeContext<'id, 'ir>> Downgrade<'id, 'ir, Ctx> fo } let (ret, thunks) = ctx.with_thunk_scope(|ctx| { - let param; - let body; - - match raw_param { + let (param, body) = match raw_param { ast::Param::IdentParam(id) => { let param_sym = ctx.intern_string(id.to_string()); - param = None; - - body = ctx.with_param_scope(param_sym, |ctx| body_ast.downgrade(ctx))?; + ( + None, + ctx.with_param_scope(param_sym, |ctx| body_ast.downgrade(ctx))?, + ) } ast::Param::Pattern(pattern) => { let alias = pattern @@ -493,15 +495,16 @@ impl<'id: 'ir, 'ir, Ctx: DowngradeContext<'id, 'ir>> Downgrade<'id, 'ir, Ctx> fo body_ast.clone().downgrade(ctx) })?; - param = Some(Param { - required, - optional, - ellipsis, - }); - - body = inner_body; + ( + Some(Param { + required, + optional, + ellipsis, + }), + inner_body, + ) } - } + }; Result::Ok(Ret { param, body }) }); @@ -550,6 +553,10 @@ impl<'id: 'ir, 'ir> PendingAttrSet<'ir> { } } + #[expect( + clippy::indexing_slicing, + reason = "path is non-empty here: path.first() was just unwrapped above, so path[1..] is in bounds" + )] fn insert( &mut self, path: &[ast::Attr], @@ -651,6 +658,10 @@ impl<'id: 'ir, 'ir> PendingAttrSet<'ir> { ) -> Result<()> { if !path.is_empty() { let mut nested = PendingAttrSet::new_in(ctx.bump()); + #[expect( + clippy::indexing_slicing, + reason = "path is non-empty in this branch, so path[0] and path[1..] are in bounds" + )] nested.insert_dynamic( path[0].clone(), path[0].syntax().text_range(), @@ -665,6 +676,10 @@ impl<'id: 'ir, 'ir> PendingAttrSet<'ir> { Ok(()) } + #[expect( + clippy::unreachable, + reason = "value was just reassigned to PendingValue::Set on the line above, so the re-match always hits the Set arm" + )] fn ensure_pending_set<'a>( value: &'a mut PendingValue<'ir>, ctx: &mut impl DowngradeContext<'id, 'ir>, diff --git a/fix-compiler/src/lib.rs b/fix-compiler/src/lib.rs index d3ecb99..3ec932b 100644 --- a/fix-compiler/src/lib.rs +++ b/fix-compiler/src/lib.rs @@ -1,5 +1,5 @@ -use fix_bytecode::{Const, Continuation, InstructionPtr, Op, OperandType}; -use fix_lang::{BUILTINS, StringId}; +use fix_bytecode::{Const, InstructionPtr, Op, OperandType}; +use fix_lang::StringId; use hashbrown::HashMap; use rnix::TextRange; use string_interner::Symbol as _; @@ -54,7 +54,6 @@ impl<'a, Ctx: BytecodeContext> BytecodeEmitter<'a, Ctx> { } } - #[must_use] fn inline_maybe_thunk(&self, val: &MaybeThunk) -> InlineOperand { use MaybeThunk::*; match *val { @@ -75,14 +74,7 @@ impl<'a, Ctx: BytecodeContext> BytecodeEmitter<'a, Ctx> { InlineOperand::Local { layer, local } } Arg { layer } => InlineOperand::Local { layer, local: 0 }, - Builtin(id) => { - let (_, arity) = BUILTINS[id as usize]; - InlineOperand::Const(Const::PrimOp { - id, - arity, - dispatch_ip: Continuation::entry_for_builtin(id).ip(), - }) - } + Builtin(id) => InlineOperand::Const(Const::PrimOp(id)), BuiltinConst(id) => InlineOperand::BuiltinConst(id), Builtins => InlineOperand::Builtins, ReplBinding(id) => InlineOperand::ReplBinding(id), @@ -186,6 +178,10 @@ impl<'a, Ctx: BytecodeContext> BytecodeEmitter<'a, Ctx> { offset } #[inline] + #[expect( + clippy::indexing_slicing, + reason = "offset addresses a 4-byte i32 placeholder this compiler previously emitted, so it is in bounds" + )] fn patch_i32(&mut self, offset: usize, val: i32) { self.ctx.get_code_mut()[offset..offset + 4].copy_from_slice(&val.to_le_bytes()); } @@ -214,6 +210,10 @@ impl<'a, Ctx: BytecodeContext> BytecodeEmitter<'a, Ctx> { self.scope_stack.last().map_or(0, |s| s.depth) } + #[expect( + clippy::panic, + reason = "a ThunkId must resolve in some enclosing scope; failure indicates a compiler bug" + )] fn resolve_thunk(&self, id: ThunkId) -> (u8, u32) { for scope in self.scope_stack.iter().rev() { if let Some(&local_idx) = scope.thunk_map.get(&id) { @@ -520,6 +520,10 @@ impl<'a, Ctx: BytecodeContext> BytecodeEmitter<'a, Ctx> { } } + #[expect( + clippy::unreachable, + reason = "the outer match only reaches this arm for the binary operator kinds enumerated above" + )] fn emit_binop(&mut self, lhs: RawIrRef<'_>, rhs: RawIrRef<'_>, kind: BinOpKind) { use BinOpKind::*; match kind { @@ -712,14 +716,25 @@ impl<'a, Ctx: BytecodeContext> BytecodeEmitter<'a, Ctx> { } if let Some(default) = default { - let before: i32 = self.ctx.get_code().len().try_into().unwrap(); + // FIXME: i32??? + let before: i32 = self + .ctx + .get_code() + .len() + .try_into() + .expect("emitted code length fits in i32"); for patch in dynamic_patches { self.patch_jump_target(patch); } self.emit_op(Op::JumpIfSelectSucceeded); let placeholder = self.emit_i32_placeholder(); self.emit_expr(default); - let after: i32 = self.ctx.get_code().len().try_into().unwrap(); + let after: i32 = self + .ctx + .get_code() + .len() + .try_into() + .expect("emitted code length fits in i32"); // Offset is relative to after the placeholder, so subtract the // size of JumpIfSelectSucceeded (1) + placeholder (4). self.patch_i32(placeholder, after - before - 5); @@ -730,6 +745,10 @@ impl<'a, Ctx: BytecodeContext> BytecodeEmitter<'a, Ctx> { } } + #[expect( + clippy::panic, + reason = "a hasAttr attrpath always has at least one attr by construction of the AST" + )] fn emit_has_attr(&mut self, lhs: RawIrRef<'_>, rhs: &[Attr>]) { self.emit_expr(lhs); diff --git a/fix-error/Cargo.toml b/fix-error/Cargo.toml index c9cf658..b1b5139 100644 --- a/fix-error/Cargo.toml +++ b/fix-error/Cargo.toml @@ -7,3 +7,6 @@ edition = "2024" miette = { version = "7.6", features = ["fancy"] } rnix = { workspace = true } thiserror = "2.0" + +[lints] +workspace = true diff --git a/fix-lang/Cargo.toml b/fix-lang/Cargo.toml index 30f2dbf..9d01024 100644 --- a/fix-lang/Cargo.toml +++ b/fix-lang/Cargo.toml @@ -8,3 +8,6 @@ ere = { workspace = true } gc-arena = { workspace = true } num_enum = { workspace = true } string-interner = { workspace = true } + +[lints] +workspace = true diff --git a/fix-lang/src/lib.rs b/fix-lang/src/lib.rs index 1aceaee..a24e3d5 100644 --- a/fix-lang/src/lib.rs +++ b/fix-lang/src/lib.rs @@ -8,7 +8,7 @@ use num_enum::TryFromPrimitive; macro_rules! define_builtins { ($(($name:literal, $variant:ident, $arity:expr)),* $(,)?) => { - pub const BUILTINS: &[(&str, u8)] = &[ + const BUILTINS: &[(&str, u8)] = &[ $(($name, $arity),)* ]; @@ -18,6 +18,10 @@ macro_rules! define_builtins { pub enum BuiltinId { $($variant,)* } + + impl BuiltinId { + pub const ALL: [Self; BUILTINS.len()] = [$(Self::$variant,)*]; + } }; } @@ -127,6 +131,36 @@ define_builtins! { ("__zipAttrsWith", ZipAttrsWith, 2), } +#[derive(Clone, Copy, Debug, PartialEq, Eq, Hash)] +pub struct BuiltinInfo { + pub name: &'static str, + pub global_name: &'static str, + pub global: bool, + pub arity: u8, +} + +impl BuiltinId { + pub const TOTAL: usize = BUILTINS.len(); + + #[expect( + clippy::indexing_slicing, + reason = "a `BuiltinId` discriminant is always a valid index into the `BUILTINS` table" + )] + #[inline(always)] + pub fn info(self) -> BuiltinInfo { + let (global_name, arity) = BUILTINS[self as usize]; + let (name, global) = global_name + .strip_prefix("__") + .map_or((global_name, true), |name| (name, false)); + BuiltinInfo { + name, + global_name, + global, + arity, + } + } +} + #[repr(transparent)] #[derive(Debug, Clone, Copy, PartialEq, Eq, Hash, PartialOrd, Ord, Collect)] #[collect(require_static)] @@ -427,6 +461,10 @@ fn fmt_nix_float(f: &mut Formatter<'_>, x: f64) -> FmtResult { let precision: i32 = 6; let exp = x.abs().log10().floor() as i32; + #[expect( + clippy::cast_sign_loss, + reason = "this branch runs only when exp < precision, so precision-1-exp and precision-1 are non-negative" + )] let formatted = if exp >= -4 && exp < precision { let decimal_places = (precision - 1 - exp) as usize; format!("{x:.decimal_places$}") @@ -451,11 +489,9 @@ fn fmt_nix_float(f: &mut Formatter<'_>, x: f64) -> FmtResult { }; if formatted.contains('.') { - if let Some(e_pos) = formatted.find('e') { - let trimmed = formatted[..e_pos] - .trim_end_matches('0') - .trim_end_matches('.'); - write!(f, "{}{}", trimmed, &formatted[e_pos..]) + if let Some((head, tail)) = formatted.split_once('e') { + let trimmed = head.trim_end_matches('0').trim_end_matches('.'); + write!(f, "{trimmed}e{tail}") } else { let trimmed = formatted.trim_end_matches('0').trim_end_matches('.'); write!(f, "{trimmed}") diff --git a/fix-macros/Cargo.toml b/fix-macros/Cargo.toml index 4ef8705..6aa125a 100644 --- a/fix-macros/Cargo.toml +++ b/fix-macros/Cargo.toml @@ -11,3 +11,6 @@ manyhow = "0.11" proc-macro2 = "1.0" quote = "1.0" syn = { version = "2.0", features = ["full", "visit"] } + +[lints] +workspace = true diff --git a/fix-runtime/Cargo.toml b/fix-runtime/Cargo.toml index 622d8ed..a40bdb6 100644 --- a/fix-runtime/Cargo.toml +++ b/fix-runtime/Cargo.toml @@ -14,3 +14,6 @@ fix-bytecode = { path = "../fix-bytecode" } fix-error = { path = "../fix-error" } fix-lang = { path = "../fix-lang" } fix-macros = { path = "../fix-macros" } + +[lints] +workspace = true diff --git a/fix-runtime/src/boxing.rs b/fix-runtime/src/boxing.rs index b0313b6..1e01956 100644 --- a/fix-runtime/src/boxing.rs +++ b/fix-runtime/src/boxing.rs @@ -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(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::

(); + // 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::

(); - 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(value: &Value) -> P { - #[cfg(target_pointer_width = "64")] - { - let val = (unsafe { value.whole() } as *const [u8; 8]).cast::

(); - 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::

(); + // 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 { 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 } } } diff --git a/fix-runtime/src/forced.rs b/fix-runtime/src/forced.rs index 10fe5ad..7b3cf1c 100644 --- a/fix-runtime/src/forced.rs +++ b/fix-runtime/src/forced.rs @@ -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::().is_none() { - let _: Step = m.finish_type_err(NixType::Float, v.ty()); - return Step::Break(Break::Done); + if !v.is::() { + m.finish_type_err(NixType::Float, v.ty()) + } else { + Step::Continue(()) } - Step::Continue(()) } #[inline(always)] diff --git a/fix-runtime/src/host.rs b/fix-runtime/src/host.rs index 30d13ea..16809df 100644 --- a/fix-runtime/src/host.rs +++ b/fix-runtime/src/host.rs @@ -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 ConvertValueWithSeen for T { Value::Thunk } } else if let Some(primop) = val.downcast::() { - 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::() { - 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 } diff --git a/fix-runtime/src/machine.rs b/fix-runtime/src/machine.rs index 5527da6..93dc3cf 100644 --- a/fix-runtime/src/machine.rs +++ b/fix-runtime/src/machine.rs @@ -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>; diff --git a/fix-runtime/src/resolve.rs b/fix-runtime/src/resolve.rs index a20d0b4..6308d44 100644 --- a/fix-runtime/src/resolve.rs +++ b/fix-runtime/src/resolve.rs @@ -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::() - .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::().expect("scope must be attrset"); - #[allow(clippy::unwrap_used)] attrs.lookup(name).expect("scoped binding not found") } } diff --git a/fix-runtime/src/state.rs b/fix-runtime/src/state.rs index 8a7483a..189bc4a 100644 --- a/fix-runtime/src/state.rs +++ b/fix-runtime/src/state.rs @@ -8,7 +8,6 @@ use hashbrown::HashSet; use crate::{GcEnv, Thunk}; -#[allow(dead_code)] pub enum VmError { Catchable(String), Uncatchable(Box), @@ -51,7 +50,6 @@ pub enum Break { pub type Step = ControlFlow; -#[allow(dead_code)] pub struct ErrorFrame { pub span_id: u32, pub message: Option, diff --git a/fix-runtime/src/string_context.rs b/fix-runtime/src/string_context.rs index 5edb984..613b63a 100644 --- a/fix-runtime/src/string_context.rs +++ b/fix-runtime/src/string_context.rs @@ -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(); diff --git a/fix-runtime/src/value.rs b/fix-runtime/src/value.rs index 5929951..2fee91c 100644 --- a/fix-runtime/src/value.rs +++ b/fix-runtime/src/value.rs @@ -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 { ::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>(&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 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(val: T) -> Self { Self { raw: val.to_raw_box(), @@ -230,7 +263,10 @@ impl<'gc> Value<'gc> { #[inline] pub fn downcast(self) -> Option> { - self.is::().then(|| unsafe { T::Ty::from_raw_box(self.raw) }) + self.is::() + // SAFETY: `is::()` 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::() { NixType::Float @@ -316,7 +356,10 @@ impl<'gc> From 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(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> { 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; 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>>) -> 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 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(); diff --git a/fix-vm/Cargo.toml b/fix-vm/Cargo.toml index ae23248..9958994 100644 --- a/fix-vm/Cargo.toml +++ b/fix-vm/Cargo.toml @@ -16,3 +16,6 @@ fix-runtime = { path = "../fix-runtime" } [features] tailcall = [] + +[lints] +workspace = true diff --git a/fix-vm/src/dispatch_tailcall.rs b/fix-vm/src/dispatch_tailcall.rs index bbcd739..1c6b969 100644 --- a/fix-vm/src/dispatch_tailcall.rs +++ b/fix-vm/src/dispatch_tailcall.rs @@ -22,6 +22,7 @@ pub(crate) type OpFn<'gc, C> = extern "rust-preserve-none" fn( pub(crate) struct DispatchTable<'gc, C: VmRuntimeCtx>(pub(crate) [OpFn<'gc, C>; 256]); +#[expect(clippy::panic, reason = "illegal opcode should panic")] extern "rust-preserve-none" fn op_illegal<'gc, C: VmRuntimeCtx>( _vm: &mut Vm<'gc>, _mc: &Mutation<'gc>, @@ -200,6 +201,7 @@ tail_fn!(op_load_scoped_binding, (ctx, reader, mc)); macro_rules! table { ($($variant:ident => $fn:ident),* $(,)?) => { impl<'gc, C: VmRuntimeCtx> DispatchTable<'gc, C> { + #[expect(clippy::indexing_slicing, reason = "Op is repr(u8)")] pub(crate) const NEW: Self = { let mut arr: [OpFn<'gc, C>; 256] = [op_illegal; 256]; $( arr[fix_bytecode::Op::$variant as usize] = $fn; )* @@ -209,7 +211,6 @@ macro_rules! table { // Exhaustiveness check: fails to compile if `fix_bytecode::Op` gains, // loses, or renames a variant that isn't wired up above. - #[allow(dead_code)] const _: fn(fix_bytecode::Op) = |op| match op { $( fix_bytecode::Op::$variant => (), )* }; @@ -291,6 +292,10 @@ table! { Illegal => op_illegal, } +#[expect( + clippy::indexing_slicing, + reason = "assume well-formed bytecode; Op is repr(u8)" +)] pub(crate) fn run_tailcall<'gc, C: VmRuntimeCtx>( vm: &mut Vm<'gc>, mc: &Mutation<'gc>, diff --git a/fix-vm/src/instructions/calls.rs b/fix-vm/src/instructions/calls.rs index 199cc3c..3a67345 100644 --- a/fix-vm/src/instructions/calls.rs +++ b/fix-vm/src/instructions/calls.rs @@ -9,6 +9,10 @@ use crate::{ }; #[inline(always)] +#[expect( + clippy::indexing_slicing, + reason = "app.args is a fixed [Value; 3]; indices are bounded by primop arity (<= 3) validated at PrimOpApp construction" +)] pub(crate) fn call<'gc, M: Machine<'gc>>( m: &mut M, reader: &mut BytecodeReader<'_>, diff --git a/fix-vm/src/instructions/collections.rs b/fix-vm/src/instructions/collections.rs index 726380d..264615e 100644 --- a/fix-vm/src/instructions/collections.rs +++ b/fix-vm/src/instructions/collections.rs @@ -117,6 +117,10 @@ pub(crate) fn op_select_dynamic<'gc, M: Machine<'gc>>( /// Only recognises Select opcodes and jumps; encountering any other /// opcode means we've reached the end of the select sequence and /// should report the missing-attribute error. +#[expect( + clippy::cast_sign_loss, + reason = "jump target = pc + offset is a non-negative bytecode address by codegen invariant" +)] fn select_skip<'gc, M: Machine<'gc>>( m: &mut M, key: StringId, @@ -126,6 +130,7 @@ fn select_skip<'gc, M: Machine<'gc>>( use fix_bytecode::Op::*; loop { match reader.read_op() { + // Skip rest of the attrpath SelectStatic => { reader.set_pc(reader.pc() + 4 + 4); } @@ -136,10 +141,14 @@ fn select_skip<'gc, M: Machine<'gc>>( reader.set_pc(reader.pc() + 4); break Step::Continue(()); } + + // Default (`a.b or c`) JumpIfSelectFailed => { let offset = reader.read_i32(); reader.set_pc(((reader.pc() as isize) + (offset as isize)) as usize); } + + // Report error _ => { let name = ctx.resolve_string(key); return m.finish_err(Error::eval_error(format!("attribute '{name}' missing"))); @@ -150,6 +159,14 @@ fn select_skip<'gc, M: Machine<'gc>>( /// Skip the rest of a **HasAttr** attrpath after an intermediate /// lookup failed. Only recognises HasAttr opcodes and jumps. +#[expect( + clippy::cast_sign_loss, + reason = "jump target = pc + offset is a non-negative bytecode address by codegen invariant" +)] +#[expect( + clippy::unreachable, + reason = "has_attr_skip only runs over a codegen-produced HasAttr attrpath sequence; any other opcode is a codegen bug" +)] fn has_attr_skip(reader: &mut BytecodeReader<'_>) -> Step { use fix_bytecode::Op::*; loop { @@ -245,6 +262,10 @@ pub(crate) fn op_jump_if_select_failed<'gc, M: Machine<'gc>>( } #[inline(always)] +#[expect( + clippy::cast_sign_loss, + reason = "jump target = pc + offset is a non-negative bytecode address by codegen invariant" +)] pub(crate) fn op_jump_if_select_succeeded<'gc, M: Machine<'gc>>( _m: &mut M, reader: &mut BytecodeReader<'_>, diff --git a/fix-vm/src/instructions/control.rs b/fix-vm/src/instructions/control.rs index f478e4a..43b034f 100644 --- a/fix-vm/src/instructions/control.rs +++ b/fix-vm/src/instructions/control.rs @@ -5,6 +5,10 @@ use gc_arena::Mutation; use crate::{BytecodeReader, Step, VmRuntimeCtx}; #[inline(always)] +#[expect( + clippy::cast_sign_loss, + reason = "jump target = pc + offset is a non-negative bytecode address by codegen invariant" +)] pub(crate) fn op_jump_if_false<'gc, M: Machine<'gc>>( m: &mut M, reader: &mut BytecodeReader<'_>, @@ -19,6 +23,10 @@ pub(crate) fn op_jump_if_false<'gc, M: Machine<'gc>>( } #[inline(always)] +#[expect( + clippy::cast_sign_loss, + reason = "jump target = pc + offset is a non-negative bytecode address by codegen invariant" +)] pub(crate) fn op_jump_if_true<'gc, M: Machine<'gc>>( m: &mut M, reader: &mut BytecodeReader<'_>, @@ -33,6 +41,10 @@ pub(crate) fn op_jump_if_true<'gc, M: Machine<'gc>>( } #[inline(always)] +#[expect( + clippy::cast_sign_loss, + reason = "jump target = pc + offset is a non-negative bytecode address by codegen invariant" +)] pub(crate) fn op_jump<'gc, M: Machine<'gc>>(_m: &mut M, reader: &mut BytecodeReader<'_>) -> Step { let offset = reader.read_i32(); reader.set_pc(((reader.pc() as isize) + (offset as isize)) as usize); diff --git a/fix-vm/src/instructions/misc.rs b/fix-vm/src/instructions/misc.rs index 8a662d3..4afb7f6 100644 --- a/fix-vm/src/instructions/misc.rs +++ b/fix-vm/src/instructions/misc.rs @@ -2,7 +2,7 @@ use std::path::PathBuf; use fix_bytecode::Continuation; use fix_error::Error; -use fix_lang::{BUILTINS, BuiltinId, StringId}; +use fix_lang::{BuiltinId, StringId}; use fix_runtime::{ AttrSet, Machine, MachineExt, NixString, Path, StrictValue, StringContext, canon_path_str, }; @@ -11,11 +11,15 @@ use crate::{BytecodeReader, PrimOp, Step, Value, VmRuntimeCtx, VmRuntimeCtxExt}; #[inline(always)] pub(crate) fn op_load_builtins<'gc, M: Machine<'gc>>(m: &mut M) -> Step { - m.push(m.builtins()); + m.push(m.builtins().into()); Step::Continue(()) } #[inline(always)] +#[expect( + clippy::panic, + reason = "codegen only emits LoadBuiltin with valid BuiltinId bytes; an unknown id is a codegen/bytecode-corruption bug" +)] pub(crate) fn op_load_builtin<'gc, M: Machine<'gc>>( m: &mut M, reader: &mut BytecodeReader<'_>, @@ -24,7 +28,7 @@ pub(crate) fn op_load_builtin<'gc, M: Machine<'gc>>( .map_err(|err| panic!("unknown builtin id: {}", err.number)); m.push(Value::new(PrimOp { id, - arity: BUILTINS[id as usize].1, + arity: id.info().arity, dispatch_ip: Continuation::entry_for_builtin(id).ip(), })); Step::Continue(()) diff --git a/fix-vm/src/instructions/variables.rs b/fix-vm/src/instructions/variables.rs index c9d9358..e33fd42 100644 --- a/fix-vm/src/instructions/variables.rs +++ b/fix-vm/src/instructions/variables.rs @@ -3,6 +3,10 @@ use fix_runtime::Machine; use crate::{BytecodeReader, Mutation, Step, Value}; #[inline(always)] +#[expect( + clippy::indexing_slicing, + reason = "local slot index is produced by codegen and bounded by the frame's AllocLocals count" +)] pub(crate) fn op_load_local<'gc, M: Machine<'gc>>( m: &mut M, reader: &mut BytecodeReader<'_>, @@ -13,6 +17,10 @@ pub(crate) fn op_load_local<'gc, M: Machine<'gc>>( } #[inline(always)] +#[expect( + clippy::indexing_slicing, + reason = "local slot index is produced by codegen and bounded by the target frame's AllocLocals count" +)] pub(crate) fn op_load_outer<'gc, M: Machine<'gc>>( m: &mut M, reader: &mut BytecodeReader<'_>, @@ -30,6 +38,10 @@ pub(crate) fn op_load_outer<'gc, M: Machine<'gc>>( } #[inline(always)] +#[expect( + clippy::indexing_slicing, + reason = "local slot index is produced by codegen and bounded by the frame's AllocLocals count" +)] pub(crate) fn op_store_local<'gc, M: Machine<'gc>>( m: &mut M, reader: &mut BytecodeReader<'_>, diff --git a/fix-vm/src/instructions/with_scope.rs b/fix-vm/src/instructions/with_scope.rs index 84fba61..510785d 100644 --- a/fix-vm/src/instructions/with_scope.rs +++ b/fix-vm/src/instructions/with_scope.rs @@ -6,14 +6,21 @@ use smallvec::SmallVec; use crate::{Break, BytecodeReader, CallFrame, Step, VmRuntimeCtx}; #[inline(always)] +#[expect( + clippy::indexing_slicing, + clippy::cast_sign_loss, + reason = "counter is a non-negative with-scope index in 0..n, staying within the namespaces vec of length n" +)] pub(crate) fn op_lookup_with<'gc, M: Machine<'gc>>( m: &mut M, ctx: &mut impl VmRuntimeCtx, reader: &mut BytecodeReader<'_>, mc: &gc_arena::Mutation<'gc>, ) -> Step { - #[allow(clippy::unwrap_used)] - let counter = m.peek_forced(0).downcast::().unwrap(); + let counter = m + .peek_forced(0) + .downcast::() + .expect("stack slot must be an integer"); let name = reader.read_string_id(); let n = reader.read_u8(); diff --git a/fix-vm/src/lib.rs b/fix-vm/src/lib.rs index 876a881..d105113 100644 --- a/fix-vm/src/lib.rs +++ b/fix-vm/src/lib.rs @@ -1,5 +1,7 @@ -#![warn(clippy::unwrap_used)] -#![cfg_attr(feature = "tailcall", expect(incomplete_features))] +#![cfg_attr( + feature = "tailcall", + expect(incomplete_features, reason = "for testing purpose only") +)] #![cfg_attr( feature = "tailcall", feature(explicit_tail_calls, rust_preserve_none_cc) @@ -9,7 +11,7 @@ use std::path::PathBuf; use fix_bytecode::{Continuation, InstructionPtr}; use fix_error::{Error, Result, Source}; -use fix_lang::{BUILTINS, BuiltinId, StringId}; +use fix_lang::{BuiltinId, StringId}; use gc_arena::metrics::Pacing; use gc_arena::{Arena, Collect, Gc, Mutation, RefLock, Rootable}; use hashbrown::HashMap; @@ -29,7 +31,10 @@ pub struct Vm<'gc> { stack: Vec>, call_stack: Vec>, call_depth: usize, - #[allow(dead_code)] + #[expect( + dead_code, + reason = "error_context is reserved for tryEval catch-frame tracking, not yet wired up" + )] #[collect(require_static)] error_context: Vec, @@ -38,7 +43,7 @@ pub struct Vm<'gc> { import_cache: HashMap>, scope_slots: Vec>, - builtins: Value<'gc>, + builtins: Gc<'gc, AttrSet<'gc>>, empty_list: Value<'gc>, empty_attrs: Value<'gc>, @@ -53,13 +58,12 @@ pub struct Vm<'gc> { functor_sym: StringId, } -fn init_builtins<'gc>(mc: &Mutation<'gc>, ctx: &mut impl VmRuntimeCtx) -> Value<'gc> { - let mut entries = SmallVec::with_capacity(BUILTINS.len()); +fn init_builtins<'gc>(mc: &Mutation<'gc>, ctx: &mut impl VmRuntimeCtx) -> Gc<'gc, AttrSet<'gc>> { + let mut entries = SmallVec::with_capacity(BuiltinId::TOTAL); - for (idx, &(name, arity)) in BUILTINS.iter().enumerate() { - let id = BuiltinId::try_from(idx as u8).expect("infallible"); - let name = name.strip_prefix("__").unwrap_or(name); - let name = ctx.intern_string(name); + for id in BuiltinId::ALL { + let arity = id.info().arity; + let name = ctx.intern_string(id.info().name); let dispatch_ip = Continuation::entry_for_builtin(id).ip(); entries.push(( name, @@ -101,7 +105,7 @@ fn init_builtins<'gc>(mc: &Mutation<'gc>, ctx: &mut impl VmRuntimeCtx) -> Value< let builtins_value = Value::new(builtins_set); *self_ref_thunk.borrow_mut(mc) = ThunkState::Evaluated(builtins_value.restrict().expect("builtins is not a thunk")); - builtins_value + builtins_set } impl<'gc> Vm<'gc> { @@ -298,7 +302,7 @@ impl<'gc> Machine<'gc> for Vm<'gc> { } #[inline(always)] - fn builtins(&self) -> Value<'gc> { + fn builtins(&self) -> Gc<'gc, AttrSet<'gc>> { self.builtins } @@ -456,6 +460,10 @@ impl<'gc> Vm<'gc> { #[inline(always)] #[cfg(not(feature = "tailcall"))] + #[expect( + clippy::unreachable, + reason = "Op::Illegal is a sentinel never emitted by codegen; reaching it is a codegen bug" + )] fn execute_batch( &mut self, bytecode: &[u8], diff --git a/fix-vm/src/primops/context.rs b/fix-vm/src/primops/context.rs index f80e2f0..821f3cc 100644 --- a/fix-vm/src/primops/context.rs +++ b/fix-vm/src/primops/context.rs @@ -200,16 +200,25 @@ pub fn append_context<'gc, M: Machine<'gc>>( Step::Continue(()) } +#[expect( + clippy::indexing_slicing, + clippy::cast_sign_loss, + reason = "idx is a non-negative loop counter guarded by `idx as usize >= attrs.entries.len()`, so it indexes attrs.entries in bounds" +)] pub fn append_context_loop<'gc, M: Machine<'gc>>( m: &mut M, ctx: &mut impl VmRuntimeCtx, reader: &mut BytecodeReader<'_>, mc: &Mutation<'gc>, ) -> Step { - #[allow(clippy::unwrap_used)] - let idx = m.peek(1).downcast::().unwrap(); - #[allow(clippy::unwrap_used)] - let attrs = m.peek_forced(2).downcast::().unwrap(); + let idx = m + .peek(1) + .downcast::() + .expect("stack slot must be an integer"); + let attrs = m + .peek_forced(2) + .downcast::() + .expect("stack slot must be an attrset"); if idx as usize >= attrs.entries.len() { return append_context_finalize(m, ctx, reader, mc); @@ -227,6 +236,11 @@ pub fn append_context_loop<'gc, M: Machine<'gc>>( Step::Continue(()) } +#[expect( + clippy::indexing_slicing, + clippy::cast_sign_loss, + reason = "idx is the same non-negative outer-loop counter, in range for outer.entries validated by append_context_loop" +)] pub fn append_context_entry_forced<'gc, M: Machine<'gc>>( m: &mut M, ctx: &mut impl VmRuntimeCtx, @@ -242,10 +256,14 @@ pub fn append_context_entry_forced<'gc, M: Machine<'gc>>( return m.finish_type_err(NixType::AttrSet, entry_val.ty()); }; - #[allow(clippy::unwrap_used)] - let idx = m.peek(2).downcast::().unwrap(); - #[allow(clippy::unwrap_used)] - let outer = m.peek_forced(3).downcast::().unwrap(); + let idx = m + .peek(2) + .downcast::() + .expect("stack slot must be an integer"); + let outer = m + .peek_forced(3) + .downcast::() + .expect("stack slot must be an attrset"); let path_key = outer.entries[idx as usize].0; let path_str_owned: Box = ctx.resolve_string(path_key).into(); if !path_str_owned.starts_with("/nix/store/") { @@ -262,8 +280,10 @@ pub fn append_context_entry_forced<'gc, M: Machine<'gc>>( let all_outputs_id = ctx.intern_string("allOutputs"); let outputs_id = ctx.intern_string("outputs"); - #[allow(clippy::unwrap_used)] - let acc_gc = m.peek(1).downcast::().unwrap(); + let acc_gc = m + .peek(1) + .downcast::() + .expect("stack slot must be a string"); let mut new_acc: StringContext = acc_gc.context().iter().cloned().collect(); if let Some(v) = entry_attrs.lookup(path_id) @@ -302,9 +322,11 @@ pub fn append_context_entry_forced<'gc, M: Machine<'gc>>( return Step::Continue(()); } - let _ = m.pop(); - #[allow(clippy::unwrap_used)] - let idx_back = m.peek(1).downcast::().unwrap(); + m.drop_n(1); + let idx_back = m + .peek(1) + .downcast::() + .expect("stack slot must be an integer"); m.replace(1, Value::new(idx_back + 1)); reader.set_pc(Continuation::PAppendContextLoop.ip() as usize); Step::Continue(()) @@ -323,9 +345,11 @@ pub fn append_context_outputs_forced<'gc, M: Machine<'gc>>( }; if list.inner.borrow().is_empty() { // Stack: [strVal, attrs, idx, acc, list] -> drop list, bump idx. - let _ = m.pop(); - #[allow(clippy::unwrap_used)] - let idx_back = m.peek(1).downcast::().unwrap(); + m.drop_n(1); + let idx_back = m + .peek(1) + .downcast::() + .expect("stack slot must be an integer"); m.replace(1, Value::new(idx_back + 1)); reader.set_pc(Continuation::PAppendContextLoop.ip() as usize); return Step::Continue(()); @@ -336,24 +360,34 @@ pub fn append_context_outputs_forced<'gc, M: Machine<'gc>>( Step::Continue(()) } +#[expect( + clippy::indexing_slicing, + clippy::cast_sign_loss, + reason = "oidx is a non-negative loop counter guarded by `oidx as usize >= len`, so it indexes the list in bounds" +)] pub fn append_context_output_element_loop<'gc, M: Machine<'gc>>( m: &mut M, _ctx: &mut impl VmRuntimeCtx, reader: &mut BytecodeReader<'_>, mc: &Mutation<'gc>, ) -> Step { - #[allow(clippy::unwrap_used)] - let oidx = m.peek(0).downcast::().unwrap(); - #[allow(clippy::unwrap_used)] - let list = m.peek_forced(1).downcast::().unwrap(); + let oidx = m + .peek(0) + .downcast::() + .expect("stack slot must be an integer"); + let list = m + .peek_forced(1) + .downcast::() + .expect("stack slot must be a list"); let len = list.inner.borrow().len(); if oidx as usize >= len { // Stack: [strVal, attrs, idx, acc, list, oidx] -> drop oidx & list, // bump idx in place. - let _ = m.pop(); - let _ = m.pop(); - #[allow(clippy::unwrap_used)] - let idx_back = m.peek(1).downcast::().unwrap(); + m.drop_n(2); + let idx_back = m + .peek(1) + .downcast::() + .expect("stack slot must be an integer"); m.replace(1, Value::new(idx_back + 1)); reader.set_pc(Continuation::PAppendContextLoop.ip() as usize); return Step::Continue(()); @@ -371,6 +405,11 @@ pub fn append_context_output_element_loop<'gc, M: Machine<'gc>>( Step::Continue(()) } +#[expect( + clippy::indexing_slicing, + clippy::cast_sign_loss, + reason = "idx is the non-negative outer-loop counter, in range for outer.entries validated by append_context_loop" +)] pub fn append_context_output_element_forced<'gc, M: Machine<'gc>>( m: &mut M, ctx: &mut impl VmRuntimeCtx, @@ -384,10 +423,14 @@ pub fn append_context_output_element_forced<'gc, M: Machine<'gc>>( }; let output_name: Box = output_name.into(); - #[allow(clippy::unwrap_used)] - let idx = m.peek(4).downcast::().unwrap(); - #[allow(clippy::unwrap_used)] - let outer = m.peek_forced(5).downcast::().unwrap(); + let idx = m + .peek(4) + .downcast::() + .expect("stack slot must be an integer"); + let outer = m + .peek_forced(5) + .downcast::() + .expect("stack slot must be an attrset"); let path_key = outer.entries[idx as usize].0; let path_str: Box = ctx.resolve_string(path_key).into(); if !path_str.ends_with(".drv") { @@ -396,8 +439,10 @@ pub fn append_context_output_element_forced<'gc, M: Machine<'gc>>( ))); } - #[allow(clippy::unwrap_used)] - let acc_gc = m.peek(3).downcast::().unwrap(); + let acc_gc = m + .peek(3) + .downcast::() + .expect("stack slot must be a string"); let mut new_acc: StringContext = acc_gc.context().iter().cloned().collect(); new_acc.insert(StringContextElem::Built { drv_path: path_str, @@ -408,14 +453,20 @@ pub fn append_context_output_element_forced<'gc, M: Machine<'gc>>( // Stack: [strVal, attrs, idx, acc, list, oidx, outElem] -> drop outElem, // bump oidx in place. - let _ = m.pop(); - #[allow(clippy::unwrap_used)] - let oidx = m.peek(0).downcast::().unwrap(); + m.drop_n(1); + let oidx = m + .peek(0) + .downcast::() + .expect("stack slot must be an integer"); m.replace(0, Value::new(oidx + 1)); reader.set_pc(Continuation::PAppendContextOutputElementLoop.ip() as usize); Step::Continue(()) } +#[expect( + clippy::panic, + reason = "strVal was forced to WHNF at appendContext entry, so restrict() can never observe a thunk here" +)] fn append_context_finalize<'gc, M: Machine<'gc>>( m: &mut M, ctx: &mut impl VmRuntimeCtx, @@ -423,10 +474,11 @@ fn append_context_finalize<'gc, M: Machine<'gc>>( mc: &Mutation<'gc>, ) -> Step { // Stack: [strVal, attrs, idx, acc] - #[allow(clippy::unwrap_used)] - let acc_gc = m.pop().downcast::().unwrap(); - let _ = m.pop(); // idx - let _ = m.pop(); // attrs + let acc_gc = m + .pop() + .downcast::() + .expect("stack slot must be a string"); + m.drop_n(2); let str_val_raw = m.pop(); // The strVal was already forced at entry; restrict() is infallible here. diff --git a/fix-vm/src/primops/control.rs b/fix-vm/src/primops/control.rs index c36672a..a18b269 100644 --- a/fix-vm/src/primops/control.rs +++ b/fix-vm/src/primops/control.rs @@ -15,7 +15,7 @@ pub fn seq<'gc, M: Machine<'gc>>( // stack: [e1, e2] - force e1, return e2 m.force_slot(1, reader, mc)?; let e2 = m.pop(); - let _ = m.pop(); + m.drop_n(1); m.return_from_primop(e2, reader) } @@ -64,7 +64,7 @@ pub fn deep_seq_force_top<'gc, M: Machine<'gc>>( if children.is_empty() { let e2 = m.pop(); - let _ = m.pop(); + m.drop_n(1); return m.return_from_primop(e2, reader); } @@ -73,7 +73,7 @@ pub fn deep_seq_force_top<'gc, M: Machine<'gc>>( let worklist = List::new(mc, children); let e2 = m.pop(); - let _ = m.pop(); + m.drop_n(1); m.push(e2); m.push(Value::new(seen)); m.push(Value::new(worklist)); @@ -88,20 +88,25 @@ pub fn deep_seq_push<'gc, M: Machine<'gc>>( mc: &Mutation<'gc>, ) -> Step { // stack: [e2, seen, worklist, counter] - #[allow(clippy::unwrap_used)] - let counter = m.peek(0).downcast::().unwrap(); + let counter = m + .peek(0) + .downcast::() + .expect("stack slot must be an integer"); if counter == 0 { - let _ = m.pop(); // counter - let _ = m.pop(); // worklist - let _ = m.pop(); // seen + m.drop_n(3); let val = m.pop(); return m.return_from_primop(val, reader); } - #[allow(clippy::unwrap_used)] - let worklist = m.peek_forced(1).downcast::().unwrap(); - #[allow(clippy::unwrap_used)] - let item = worklist.unlock(mc).borrow_mut().pop().unwrap(); + let worklist = m + .peek_forced(1) + .downcast::() + .expect("stack slot must be a list"); + let item = worklist + .unlock(mc) + .borrow_mut() + .pop() + .expect("worklist is non-empty while counter > 0"); m.replace(0, Value::new(counter - 1)); m.push(item); @@ -118,18 +123,24 @@ pub fn deep_seq_loop<'gc, M: Machine<'gc>>( ) -> Step { // stack after pop: [e2, seen, worklist, counter] let item = m.pop(); - #[allow(clippy::unwrap_used)] - let counter = m.peek(0).downcast::().unwrap(); + let counter = m + .peek(0) + .downcast::() + .expect("stack slot must be an integer"); let mut added: usize = 0; if let Some(attrs) = item.downcast::() { let attrs = &attrs.entries; - #[allow(clippy::unwrap_used)] - let seen = m.peek_forced(2).downcast::().unwrap(); + let seen = m + .peek_forced(2) + .downcast::() + .expect("stack slot must be a list"); if !is_value_in_seen(seen, item) { add_value_to_seen(seen, mc, item); - #[allow(clippy::unwrap_used)] - let worklist = m.peek_forced(1).downcast::().unwrap(); + let worklist = m + .peek_forced(1) + .downcast::() + .expect("stack slot must be a list"); { let mut wl = worklist.unlock(mc).borrow_mut(); for &(_, v) in attrs.iter() { @@ -139,12 +150,16 @@ pub fn deep_seq_loop<'gc, M: Machine<'gc>>( } } } else if let Some(list) = item.downcast::() { - #[allow(clippy::unwrap_used)] - let seen = m.peek_forced(2).downcast::().unwrap(); + let seen = m + .peek_forced(2) + .downcast::() + .expect("stack slot must be a list"); if !is_value_in_seen(seen, item) { add_value_to_seen(seen, mc, item); - #[allow(clippy::unwrap_used)] - let worklist = m.peek_forced(1).downcast::().unwrap(); + let worklist = m + .peek_forced(1) + .downcast::() + .expect("stack slot must be a list"); { let inner = list.inner.borrow(); let mut wl = worklist.unlock(mc).borrow_mut(); @@ -191,20 +206,27 @@ pub fn force_result_shallow<'gc, M: Machine<'gc>>( Step::Continue(()) } +#[expect( + clippy::cast_sign_loss, + reason = "idx is a non-negative loop counter (0..=len); used only with .get(), so the cast is always a valid index or safely out of range" +)] pub fn force_result_shallow_push<'gc, M: Machine<'gc>>( m: &mut M, ctx: &mut impl VmRuntimeCtx, reader: &mut BytecodeReader<'_>, mc: &Mutation<'gc>, ) -> Step { - #[allow(clippy::unwrap_used)] - let idx = m.peek(1).downcast::().unwrap(); - #[allow(clippy::unwrap_used)] - let len = m.peek(0).downcast::().unwrap(); + let idx = m + .peek(1) + .downcast::() + .expect("stack slot must be an integer"); + let len = m + .peek(0) + .downcast::() + .expect("stack slot must be an integer"); if idx == len { - let _ = m.pop(); // len - let _ = m.pop(); // idx + m.drop_n(2); let val = m.pop(); return m.finish_ok(ctx.convert_value(val)); } @@ -237,7 +259,7 @@ pub fn force_result_shallow_loop<'gc, M: Machine<'gc>>( reader: &mut BytecodeReader<'_>, _mc: &Mutation<'gc>, ) -> Step { - let _ = m.pop(); // forced child + m.drop_n(1); reader.set_pc(Continuation::ForceResultShallowPush.ip() as usize); Step::Continue(()) } @@ -308,6 +330,10 @@ pub fn call_functor_2<'gc, M: Machine<'gc>>( m.call(reader, mc, orig_arg, saved.pc) } +#[expect( + clippy::unreachable, + reason = "CallPattern is only dispatched for closures whose pattern is Some, established when the Call opcode routed here" +)] pub fn call_pattern<'gc, M: Machine<'gc>>( m: &mut M, ctx: &mut impl VmRuntimeCtx, diff --git a/fix-vm/src/primops/conv.rs b/fix-vm/src/primops/conv.rs index 6b36919..0a9db96 100644 --- a/fix-vm/src/primops/conv.rs +++ b/fix-vm/src/primops/conv.rs @@ -27,6 +27,10 @@ pub fn to_string<'gc, M: Machine<'gc>>( ))) } +#[expect( + clippy::unreachable, + reason = "val was forced to WHNF by force_and_retry, so its type is never Thunk here" +)] pub fn type_of<'gc, M: Machine<'gc>>( m: &mut M, ctx: &mut impl VmRuntimeCtx, diff --git a/fix-vm/src/primops/eq.rs b/fix-vm/src/primops/eq.rs index a19d50d..25ae70b 100644 --- a/fix-vm/src/primops/eq.rs +++ b/fix-vm/src/primops/eq.rs @@ -88,8 +88,7 @@ pub fn eq_force<'gc, M: Machine<'gc>>( } fn finalize<'gc, M: Machine<'gc>>(m: &mut M, reader: &mut BytecodeReader<'_>) -> Step { - let _ = m.pop(); - let _ = m.pop(); + m.drop_n(2); let result = m .pop() .downcast::() diff --git a/fix-vm/src/primops/io.rs b/fix-vm/src/primops/io.rs index 9171b44..40e58bc 100644 --- a/fix-vm/src/primops/io.rs +++ b/fix-vm/src/primops/io.rs @@ -55,6 +55,10 @@ pub fn import<'gc, M: Machine<'gc>>( Step::Break(Break::LoadFile) } +#[expect( + clippy::unreachable, + reason = "import always pushes the PImportFinalize call frame before this runs, so pop_call_frame is always Some" +)] pub fn import_finalize<'gc, M: Machine<'gc>>( m: &mut M, ctx: &mut impl VmRuntimeCtx, @@ -62,8 +66,10 @@ pub fn import_finalize<'gc, M: Machine<'gc>>( ) -> Step { // stack: [path_sid, return_value] let val = m.pop(); - #[allow(clippy::unwrap_used)] - let path_sid = m.pop().downcast::().unwrap(); + let path_sid = m + .pop() + .downcast::() + .expect("stack slot must be a string"); // The cache key is keyed by the absolute path string we interned in // `import`. Resolve it back to the host PathBuf. let path_str = ctx.resolve_string(path_sid).to_owned(); diff --git a/fix-vm/src/primops/list.rs b/fix-vm/src/primops/list.rs index c15687f..74e9cbf 100644 --- a/fix-vm/src/primops/list.rs +++ b/fix-vm/src/primops/list.rs @@ -24,6 +24,11 @@ pub fn filter_force_list<'gc, M: Machine<'gc>>( Step::Continue(()) } +#[expect( + clippy::indexing_slicing, + clippy::cast_sign_loss, + reason = "idx is a non-negative loop counter in 0..list.len(), so it indexes the list in bounds" +)] pub fn filter_call_pred<'gc, M: Machine<'gc>>( m: &mut M, reader: &mut BytecodeReader<'_>, @@ -31,36 +36,51 @@ pub fn filter_call_pred<'gc, M: Machine<'gc>>( ) -> Step { m.force_slot(3, reader, mc)?; let pred = m.peek_forced(3); - #[allow(clippy::unwrap_used)] - let idx = m.peek(1).downcast::().unwrap(); - #[allow(clippy::unwrap_used)] - let elem = m.peek_forced(2).downcast::().unwrap().inner.borrow()[idx as usize]; + let idx = m + .peek(1) + .downcast::() + .expect("stack slot must be an integer"); + let elem = m + .peek_forced(2) + .downcast::() + .expect("stack slot must be a list") + .inner + .borrow()[idx as usize]; m.push(pred.relax()); m.call(reader, mc, elem, Continuation::PFilterCheck.ip() as usize) } +#[expect( + clippy::indexing_slicing, + clippy::cast_sign_loss, + reason = "idx is a non-negative loop counter in 0..list.len(), so it indexes the list in bounds" +)] pub fn filter_check<'gc, M: Machine<'gc>>( m: &mut M, reader: &mut BytecodeReader<'_>, mc: &Mutation<'gc>, ) -> Step { let ret = m.force_and_retry::(reader, mc)?; - #[allow(clippy::unwrap_used)] - let idx = m.peek(1).downcast::().unwrap(); - #[allow(clippy::unwrap_used)] - let list = m.peek_forced(2).downcast::().unwrap(); + let idx = m + .peek(1) + .downcast::() + .expect("stack slot must be an integer"); + let list = m + .peek_forced(2) + .downcast::() + .expect("stack slot must be a list"); let list = list.inner.borrow(); - #[allow(clippy::unwrap_used)] - let acc = m.peek_forced(0).downcast::().unwrap(); + let acc = m + .peek_forced(0) + .downcast::() + .expect("stack slot must be a list"); if ret { let mut acc = acc.unlock(mc).borrow_mut(); acc.push(list[idx as usize]); } if idx as usize == list.len() - 1 { let acc = m.pop(); - let _ = m.pop(); // idx - let _ = m.pop(); // list - let _ = m.pop(); // pred + m.drop_n(3); return m.return_from_primop(acc, reader); } m.replace(1, Value::new(idx + 1)); @@ -87,7 +107,7 @@ pub fn foldl_strict_entry<'gc, M: Machine<'gc>>( return m.finish_type_err(NixType::List, list_val.ty()); }; if list.inner.borrow().is_empty() { - let _ = m.pop(); // list + m.drop_n(1); reader.set_pc(Continuation::PFoldlStrictEmpty.ip() as usize); return Step::Continue(()); } @@ -106,7 +126,7 @@ pub fn foldl_strict_empty<'gc, M: Machine<'gc>>( mc: &Mutation<'gc>, ) -> Step { let nul = m.force_and_retry::(reader, mc)?; - let _ = m.pop(); // op + m.drop_n(1); m.return_from_primop(nul.relax(), reader) } @@ -127,15 +147,24 @@ pub fn foldl_strict_call1<'gc, M: Machine<'gc>>( ) } +#[expect( + clippy::indexing_slicing, + clippy::cast_sign_loss, + reason = "idx is a non-negative loop counter in 0..list.len(), so it indexes the list in bounds" +)] pub fn foldl_strict_call2<'gc, M: Machine<'gc>>( m: &mut M, reader: &mut BytecodeReader<'_>, mc: &Mutation<'gc>, ) -> Step { - #[allow(clippy::unwrap_used)] - let idx = m.peek(2).downcast::().unwrap(); - #[allow(clippy::unwrap_used)] - let list = m.peek_forced(3).downcast::().unwrap(); + let idx = m + .peek(2) + .downcast::() + .expect("stack slot must be an integer"); + let list = m + .peek_forced(3) + .downcast::() + .expect("stack slot must be a list"); let elem = list.inner.borrow()[idx as usize]; m.call( reader, @@ -145,6 +174,10 @@ pub fn foldl_strict_call2<'gc, M: Machine<'gc>>( ) } +#[expect( + clippy::cast_sign_loss, + reason = "idx is a non-negative loop counter in 0..list.len()" +)] pub fn foldl_strict_update<'gc, M: Machine<'gc>>( m: &mut M, reader: &mut BytecodeReader<'_>, @@ -152,16 +185,18 @@ pub fn foldl_strict_update<'gc, M: Machine<'gc>>( ) -> Step { let result = m.pop(); m.replace(0, result); - #[allow(clippy::unwrap_used)] - let idx = m.peek(1).downcast::().unwrap(); - #[allow(clippy::unwrap_used)] - let list = m.peek_forced(2).downcast::().unwrap(); + let idx = m + .peek(1) + .downcast::() + .expect("stack slot must be an integer"); + let list = m + .peek_forced(2) + .downcast::() + .expect("stack slot must be a list"); let len = list.inner.borrow().len(); if (idx as usize) + 1 == len { let acc = m.pop(); - let _ = m.pop(); // idx - let _ = m.pop(); // list - let _ = m.pop(); // op + m.drop_n(3); return m.return_from_primop(acc, reader); } m.replace(1, Value::new(idx + 1)); @@ -192,35 +227,52 @@ pub fn all_entry<'gc, M: Machine<'gc>>( Step::Continue(()) } +#[expect( + clippy::indexing_slicing, + clippy::cast_sign_loss, + reason = "idx is a non-negative loop counter in 0..list.len(), so it indexes the list in bounds" +)] pub fn all_call_pred<'gc, M: Machine<'gc>>( m: &mut M, reader: &mut BytecodeReader<'_>, mc: &Mutation<'gc>, ) -> Step { let pred = m.peek_forced(2); - #[allow(clippy::unwrap_used)] - let idx = m.peek(0).downcast::().unwrap(); - #[allow(clippy::unwrap_used)] - let elem = m.peek_forced(1).downcast::().unwrap().inner.borrow()[idx as usize]; + let idx = m + .peek(0) + .downcast::() + .expect("stack slot must be an integer"); + let elem = m + .peek_forced(1) + .downcast::() + .expect("stack slot must be a list") + .inner + .borrow()[idx as usize]; m.push(pred.relax()); m.call(reader, mc, elem, Continuation::PAllCheck.ip() as usize) } +#[expect( + clippy::cast_sign_loss, + reason = "idx is a non-negative loop counter in 0..list.len()" +)] pub fn all_check<'gc, M: Machine<'gc>>( m: &mut M, reader: &mut BytecodeReader<'_>, mc: &Mutation<'gc>, ) -> Step { let ret = m.force_and_retry::(reader, mc)?; - #[allow(clippy::unwrap_used)] - let idx = m.peek(0).downcast::().unwrap(); - #[allow(clippy::unwrap_used)] - let list = m.peek_forced(1).downcast::().unwrap(); + let idx = m + .peek(0) + .downcast::() + .expect("stack slot must be an integer"); + let list = m + .peek_forced(1) + .downcast::() + .expect("stack slot must be a list"); let list = list.inner.borrow(); if idx as usize == list.len() - 1 || !ret { - let _ = m.pop(); // idx - let _ = m.pop(); // list - let _ = m.pop(); // pred + m.drop_n(3); return m.return_from_primop(Value::new(ret), reader); } m.replace(0, Value::new(idx + 1)); @@ -251,35 +303,52 @@ pub fn any_entry<'gc, M: Machine<'gc>>( Step::Continue(()) } +#[expect( + clippy::indexing_slicing, + clippy::cast_sign_loss, + reason = "idx is a non-negative loop counter in 0..list.len(), so it indexes the list in bounds" +)] pub fn any_call_pred<'gc, M: Machine<'gc>>( m: &mut M, reader: &mut BytecodeReader<'_>, mc: &Mutation<'gc>, ) -> Step { let pred = m.peek_forced(2); - #[allow(clippy::unwrap_used)] - let idx = m.peek(0).downcast::().unwrap(); - #[allow(clippy::unwrap_used)] - let elem = m.peek_forced(1).downcast::().unwrap().inner.borrow()[idx as usize]; + let idx = m + .peek(0) + .downcast::() + .expect("stack slot must be an integer"); + let elem = m + .peek_forced(1) + .downcast::() + .expect("stack slot must be a list") + .inner + .borrow()[idx as usize]; m.push(pred.relax()); m.call(reader, mc, elem, Continuation::PAnyCheck.ip() as usize) } +#[expect( + clippy::cast_sign_loss, + reason = "idx is a non-negative loop counter in 0..list.len()" +)] pub fn any_check<'gc, M: Machine<'gc>>( m: &mut M, reader: &mut BytecodeReader<'_>, mc: &Mutation<'gc>, ) -> Step { let ret = m.force_and_retry::(reader, mc)?; - #[allow(clippy::unwrap_used)] - let idx = m.peek(0).downcast::().unwrap(); - #[allow(clippy::unwrap_used)] - let list = m.peek_forced(1).downcast::().unwrap(); + let idx = m + .peek(0) + .downcast::() + .expect("stack slot must be an integer"); + let list = m + .peek_forced(1) + .downcast::() + .expect("stack slot must be a list"); let list = list.inner.borrow(); if idx as usize == list.len() - 1 || ret { - let _ = m.pop(); // idx - let _ = m.pop(); // list - let _ = m.pop(); // pred + m.drop_n(3); return m.return_from_primop(Value::new(ret), reader); } m.replace(0, Value::new(idx + 1)); diff --git a/fix-vm/src/primops/mod.rs b/fix-vm/src/primops/mod.rs index d17127b..7abdcbd 100644 --- a/fix-vm/src/primops/mod.rs +++ b/fix-vm/src/primops/mod.rs @@ -18,7 +18,6 @@ pub use io::*; pub use list::*; pub use path::*; -#[allow(clippy::too_many_lines)] pub fn dispatch_cont<'gc, M: Machine<'gc>>( m: &mut M, ctx: &mut impl VmRuntimeCtx, diff --git a/fix/Cargo.toml b/fix/Cargo.toml index d874f62..e135fc5 100644 --- a/fix/Cargo.toml +++ b/fix/Cargo.toml @@ -48,5 +48,9 @@ fix-vm = { path = "../fix-vm" } [dev-dependencies] criterion = { version = "0.8", features = ["html_reports"] } +serial_test = "3.5" tempfile = "3.24" test-log = { version = "0.2", features = ["trace"] } + +[lints] +workspace = true diff --git a/fix/benches/utils.rs b/fix/benches/utils.rs index 22348e9..bc87dc1 100644 --- a/fix/benches/utils.rs +++ b/fix/benches/utils.rs @@ -1,4 +1,5 @@ -#![allow(dead_code)] +#![allow(clippy::allow_attributes_without_reason)] +#![allow(dead_code, clippy::unwrap_used, clippy::unwrap_in_result)] use fix::Evaluator; use fix_error::{Result, Source}; diff --git a/fix/src/lib.rs b/fix/src/lib.rs index 75b4f20..6143247 100644 --- a/fix/src/lib.rs +++ b/fix/src/lib.rs @@ -1,6 +1,3 @@ -#![warn(clippy::unwrap_used)] -#![allow(dead_code)] - use fix_bytecode::InstructionPtr; use fix_bytecode::disassembler::{Disassembler, DisassemblerContext}; use fix_compiler::{CodeState, ExtraScope}; @@ -11,7 +8,6 @@ use fix_vm::Vm; use hashbrown::{HashMap, HashSet}; use string_interner::{DefaultStringInterner, Symbol as _}; -mod derivation; pub mod logging; #[global_allocator] @@ -101,12 +97,12 @@ impl VmRuntimeCtx for RuntimeState { StringId(self.strings.get_or_intern(s)) } fn resolve_string(&self, id: StringId) -> &str { - #[allow(clippy::unwrap_used)] - self.strings.resolve(id.0).unwrap() + self.strings + .resolve(id.0) + .expect("interned string id must resolve") } fn get_const(&self, id: u32) -> StaticValue { - #[allow(clippy::unwrap_used)] - self.constants.get(id).unwrap() + self.constants.get(id).expect("const id must be valid") } fn add_const(&mut self, val: StaticValue) -> u32 { self.constants.insert(val) @@ -145,9 +141,9 @@ impl DisassemblerContext for Evaluator { &self.code.bytecode } - #[allow(clippy::unwrap_used)] fn resolve_string(&self, id: u32) -> &str { - let id = string_interner::symbol::SymbolU32::try_from_usize(id as usize).unwrap(); - self.runtime.strings.resolve(id).unwrap() + let id = string_interner::symbol::SymbolU32::try_from_usize(id as usize) + .expect("invalid string id"); + self.runtime.strings.resolve(id).expect("invalid string id") } } diff --git a/fix/src/logging.rs b/fix/src/logging.rs index 20ea887..d64cd40 100644 --- a/fix/src/logging.rs +++ b/fix/src/logging.rs @@ -5,7 +5,7 @@ use tracing_subscriber::layer::SubscriberExt; use tracing_subscriber::util::SubscriberInitExt; use tracing_subscriber::{EnvFilter, Layer, fmt}; -pub fn init_logging() { +pub fn init_logging() -> Result<(), miette::InstallError> { let is_terminal = std::io::stderr().is_terminal(); let show_time = env::var("NIX_JS_LOG_TIME") .map(|v| v == "1" || v.to_lowercase() == "true") @@ -32,10 +32,10 @@ pub fn init_logging() { .with(fmt_layer) .init(); - init_miette_handler(); + init_miette_handler() } -fn init_miette_handler() { +fn init_miette_handler() -> Result<(), miette::InstallError> { let is_terminal = std::io::stderr().is_terminal(); miette::set_hook(Box::new(move |_| { Box::new( @@ -46,5 +46,4 @@ fn init_miette_handler() { .build(), ) })) - .ok(); } diff --git a/fix/src/main.rs b/fix/src/main.rs index f9e1750..4b7b345 100644 --- a/fix/src/main.rs +++ b/fix/src/main.rs @@ -20,33 +20,49 @@ struct Cli { enum Command { Compile { #[clap(flatten)] - source: ExprSource, + source: ExprSourceArgs, #[arg(long)] silent: bool, }, Eval { #[clap(flatten)] - source: ExprSource, + source: ExprSourceArgs, }, Repl, } #[derive(Args)] #[group(required = true, multiple = false)] -struct ExprSource { +struct ExprSourceArgs { #[clap(short, long)] expr: Option, #[clap(short, long)] file: Option, } +enum ExprSource { + Expr(String), + File(PathBuf), +} + +#[expect( + clippy::unreachable, + reason = "clap's arg group guarantees exactly one of --expr/--file is set" +)] +impl From for ExprSource { + fn from(args: ExprSourceArgs) -> Self { + match (args.expr, args.file) { + (Some(expr), None) => ExprSource::Expr(expr), + (None, Some(file)) => ExprSource::File(file), + _ => unreachable!(), + } + } +} + fn run_compile(eval: &mut Evaluator, src: ExprSource, silent: bool) -> Result<()> { - let src = if let Some(expr) = src.expr { - Source::new_eval(expr)? - } else if let Some(file) = src.file { - Source::new_file(file)? - } else { - unreachable!() + let src = match src { + ExprSource::Expr(expr) => Source::new_eval(expr)?, + ExprSource::File(file) => Source::new_file(file)?, }; match eval.compile_bytecode(src) { Ok(ip) => { @@ -63,12 +79,9 @@ fn run_compile(eval: &mut Evaluator, src: ExprSource, silent: bool) -> Result<() } fn run_eval(eval: &mut Evaluator, src: ExprSource) -> Result<()> { - let src = if let Some(expr) = src.expr { - Source::new_eval(expr)? - } else if let Some(file) = src.file { - Source::new_file(file)? - } else { - unreachable!() + let src = match src { + ExprSource::Expr(expr) => Source::new_eval(expr)?, + ExprSource::File(file) => Source::new_file(file)?, }; match eval.eval_deep(src) { Ok(value) => { @@ -93,7 +106,9 @@ fn run_repl(eval: &mut Evaluator) -> Result<()> { if line.trim().is_empty() { continue; } - let _ = rl.add_history_entry(line.as_str()); + if let Err(err) = rl.add_history_entry(line.as_str()) { + eprintln!("[WARN] Failed to add history entry: {err}"); + } if let Some([Some(_), Some(ident), Some(rest)]) = RE.exec(&line) { if let Some(expr) = rest.strip_prefix('=') { let expr = expr.trim_start(); @@ -137,15 +152,15 @@ fn run_repl(eval: &mut Evaluator) -> Result<()> { } fn main() -> Result<()> { - fix::logging::init_logging(); + fix::logging::init_logging()?; let cli = Cli::parse(); let mut eval = Evaluator::new(); match cli.command { - Command::Compile { source, silent } => run_compile(&mut eval, source, silent), - Command::Eval { source } => run_eval(&mut eval, source), + Command::Compile { source, silent } => run_compile(&mut eval, source.into(), silent), + Command::Eval { source } => run_eval(&mut eval, source.into()), Command::Repl => run_repl(&mut eval), } } diff --git a/fix/tests/tests/derivation.rs b/fix/tests/tests/derivation.rs index be37749..ac525c5 100644 --- a/fix/tests/tests/derivation.rs +++ b/fix/tests/tests/derivation.rs @@ -402,17 +402,15 @@ fn fixed_output_sha256_flat() { #[test_log::test] fn fixed_output_missing_hashalgo() { - assert!( - eval_deep_result( - r#"derivation { + eval_deep_result( + r#"derivation { name = "default"; builder = "/bin/sh"; system = "x86_64-linux"; outputHash = "0000000000000000000000000000000000000000000000000000000000000000"; }"#, - ) - .is_err() - ); + ) + .unwrap_err(); } #[test_log::test] diff --git a/fix/tests/tests/io_operations.rs b/fix/tests/tests/io_operations.rs index 2ea0002..52264c6 100644 --- a/fix/tests/tests/io_operations.rs +++ b/fix/tests/tests/io_operations.rs @@ -350,7 +350,7 @@ fn read_dir_nonexistent_fails() { let expr = r#"builtins.readDir "/nonexistent/directory""#; let result = eval_result(expr); - assert!(result.is_err()); + result.unwrap_err(); } #[test_log::test] diff --git a/fix/tests/tests/lang.rs b/fix/tests/tests/lang.rs index 977f4a7..a6b3d9d 100644 --- a/fix/tests/tests/lang.rs +++ b/fix/tests/tests/lang.rs @@ -5,6 +5,7 @@ use std::path::PathBuf; use fix::Evaluator; use fix_error::{Source, SourceType}; use fix_lang::Value; +use serial_test::serial; fn get_lang_dir() -> PathBuf { PathBuf::from(env!("CARGO_MANIFEST_DIR")).join("tests/tests/lang") @@ -160,9 +161,14 @@ mod okay { eval_okay_test!(getattrpos); eval_okay_test!(getattrpos_functionargs); eval_okay_test!(getattrpos_undefined); - eval_okay_test!(getenv, || { - unsafe { std::env::set_var("TEST_VAR", "foo") }; - }); + eval_okay_test!( + #[serial(env)] + getenv, + || { + // SAFETY: guarded with #[serial_test::serial] + unsafe { std::env::set_var("TEST_VAR", "foo") }; + } + ); eval_okay_test!(groupBy); eval_okay_test!(r#if); eval_okay_test!(ind_string); @@ -194,11 +200,16 @@ mod okay { eval_okay_test!(partition); eval_okay_test!(path); eval_okay_test!(pathexists); - eval_okay_test!(path_string_interpolation, || { - unsafe { - std::env::set_var("HOME", "/fake-home"); + eval_okay_test!( + #[serial(env)] + path_string_interpolation, + || { + // SAFETY: guarded with #[serial_test::serial] + unsafe { + std::env::set_var("HOME", "/fake-home"); + } } - }); + ); eval_okay_test!(patterns); eval_okay_test!(print); eval_okay_test!(readDir); diff --git a/fix/tests/tests/main.rs b/fix/tests/tests/main.rs index 65f11db..a82e202 100644 --- a/fix/tests/tests/main.rs +++ b/fix/tests/tests/main.rs @@ -1,3 +1,11 @@ +#![allow(clippy::allow_attributes_without_reason)] +#![allow( + dead_code, + clippy::unwrap_used, + clippy::unwrap_in_result, + clippy::panic +)] + mod derivation; mod findfile; mod io_operations; diff --git a/fix/tests/tests/string_context.rs b/fix/tests/tests/string_context.rs index 99b6465..419306d 100644 --- a/fix/tests/tests/string_context.rs +++ b/fix/tests/tests/string_context.rs @@ -348,7 +348,7 @@ fn substring_zero_length_empty_value() { } #[test_log::test] -#[allow(non_snake_case)] +#[expect(non_snake_case)] fn concatStringsSep_preserves_context() { let result = eval( r#" @@ -365,7 +365,7 @@ fn concatStringsSep_preserves_context() { } #[test_log::test] -#[allow(non_snake_case)] +#[expect(non_snake_case)] fn concatStringsSep_merges_contexts() { let result = eval( r#" @@ -383,7 +383,7 @@ fn concatStringsSep_merges_contexts() { } #[test_log::test] -#[allow(non_snake_case)] +#[expect(non_snake_case)] fn concatStringsSep_separator_has_context() { let result = eval( r#" @@ -398,7 +398,7 @@ fn concatStringsSep_separator_has_context() { } #[test_log::test] -#[allow(non_snake_case)] +#[expect(non_snake_case)] fn replaceStrings_input_context_preserved() { let result = eval( r#" @@ -413,7 +413,7 @@ fn replaceStrings_input_context_preserved() { } #[test_log::test] -#[allow(non_snake_case)] +#[expect(non_snake_case)] fn replaceStrings_replacement_context_collected() { let result = eval( r#" @@ -428,7 +428,7 @@ fn replaceStrings_replacement_context_collected() { } #[test_log::test] -#[allow(non_snake_case)] +#[expect(non_snake_case)] fn replaceStrings_merges_contexts() { let result = eval( r#" @@ -446,7 +446,7 @@ fn replaceStrings_merges_contexts() { } #[test_log::test] -#[allow(non_snake_case)] +#[expect(non_snake_case)] fn replaceStrings_lazy_evaluation_context() { let result = eval( r#" @@ -461,7 +461,7 @@ fn replaceStrings_lazy_evaluation_context() { } #[test_log::test] -#[allow(non_snake_case)] +#[expect(non_snake_case)] fn baseNameOf_preserves_context() { let result = eval( r#" diff --git a/fix/tests/tests/utils.rs b/fix/tests/tests/utils.rs index acbbda9..67be623 100644 --- a/fix/tests/tests/utils.rs +++ b/fix/tests/tests/utils.rs @@ -1,5 +1,3 @@ -#![allow(dead_code)] - use fix::Evaluator; use fix_error::{Result, Source}; use fix_lang::Value;