From b23e2326c4d9228a8ebcab0b4d3ce1e25066e9c2 Mon Sep 17 00:00:00 2001 From: a dinosaur Date: Sun, 2 Nov 2025 18:37:44 +1100 Subject: [PATCH] Clean up ordered bitset (implementation & usages) and write tests --- src/argparse.rs | 6 ++-- src/options.rs | 8 ++--- src/ordered_bitset.rs | 74 +++++++++++++++++++++++++++++++++++++++---- 3 files changed, 73 insertions(+), 15 deletions(-) diff --git a/src/argparse.rs b/src/argparse.rs index 55c35e8..b001e4f 100644 --- a/src/argparse.rs +++ b/src/argparse.rs @@ -94,8 +94,6 @@ impl From for ParseError<'_> { impl core::error::Error for ParseError<'_> {} -type RequiredParamsBitSet = ordered_bitset::OrderedBitSet; - /// Internal state tracked by the parser struct ParserState { positional_index: usize, @@ -195,7 +193,7 @@ impl Opts { // Match a suitable option by name (ignoring the first flag character & skipping positional arguments) let (name, option) = self.options.iter() - .filter(|opt| matches!(opt.r#type, OptType::Flag | OptType::Value)) .find_map(|opt| { + .filter(|opt| matches!(opt.r#type, OptType::Flag | OptType::Value)).find_map(|opt| { if let Some(name) = opt.match_name(option_str, 1) { Some((name, opt)) } else { @@ -204,7 +202,7 @@ impl Opts { } None } - }) .ok_or(ParseError::UnknownOption(option_str))?; + }).ok_or(ParseError::UnknownOption(option_str))?; // Mark required option as visited if option.is_required() { diff --git a/src/options.rs b/src/options.rs index 960d780..e737fab 100644 --- a/src/options.rs +++ b/src/options.rs @@ -13,10 +13,10 @@ pub struct Opts { description: Option<&'static str>, } -type BitSetType = u32; -const BITSET_SLOTS: usize = 4; +type RequiredParamsBitSet = ordered_bitset::OrderedBitSet; + /// The maximum amount of allowed required non-positional options. -pub const MAX_REQUIRED_OPTIONS: usize = BitSetType::BITS as usize * BITSET_SLOTS; +pub const MAX_REQUIRED_OPTIONS: usize = RequiredParamsBitSet::CAPACITY; impl Opts { /// Build argument parser options with the default flag character of '-' @@ -30,7 +30,7 @@ impl Opts { } opt_idx += 1; } - assert!(num_required_parameters <= MAX_REQUIRED_OPTIONS, + assert!(num_required_parameters <= RequiredParamsBitSet::CAPACITY, "More than 128 non-positional required option entries is not supported at this time"); Self { diff --git a/src/ordered_bitset.rs b/src/ordered_bitset.rs index 9784327..3a000e7 100644 --- a/src/ordered_bitset.rs +++ b/src/ordered_bitset.rs @@ -15,12 +15,15 @@ impl Default for OrderedBitSet { // TODO: Obvious target for improvement when const traits land impl OrderedBitSet { + /// Number of slots in the bit set. + pub(crate) const CAPACITY: usize = T::BITS as usize * S; + + /// Creates a new, empty bit set. pub(crate) const fn new() -> Self { Self([T::ZERO; S]) } + /// Sets the slot at `index` to a binary value. pub(crate) fn insert(&mut self, index: usize, value: bool) { - let array_idx = index >> T::SHIFT; - debug_assert!(array_idx < S, "Index out of range"); - let bit_idx = index & T::MASK; + let (array_idx, bit_idx) = self.internal_index(index); let bit_mask = T::from_usize(0b1) << T::from_usize(bit_idx); if value { self.0[array_idx] |= bit_mask; @@ -29,21 +32,30 @@ impl OrderedBitSet { } } + /// Gets the binary value at slot `index`. pub(crate) fn get(&self, index: usize) -> bool { - let array_idx = index >> T::SHIFT; - debug_assert!(array_idx < S, "Index out of range"); - let bit_idx = index & T::MASK; + let (array_idx, bit_idx) = self.internal_index(index); let bit_mask = T::from_usize(0b1) << T::from_usize(bit_idx); (self.0[array_idx] & bit_mask) != T::from_usize(0) } + + #[inline] + const fn internal_index(&self, index: usize) -> (usize, usize) { + debug_assert!(index < Self::CAPACITY, "Index out of range"); + let array_idx = index >> T::SHIFT; + let bit_idx = index & T::MASK; + (array_idx, bit_idx) + } } -trait OrderedBitSetStorage: Default + Copy + Clone + Eq + PartialEq +trait OrderedBitSetStorage: core::fmt::Debug + + Default + Copy + Clone + Eq + PartialEq + BitAnd + Shl + Not + BitAndAssign + BitOrAssign { const ZERO: Self; const SHIFT: u32; const MASK: usize; + const BITS: u32; fn from_usize(value: usize) -> Self; } @@ -53,6 +65,8 @@ macro_rules! impl_bitset_storage { const ZERO: $t = 0; const SHIFT: u32 = $b.ilog2(); const MASK: usize = $b as usize - 1; + const BITS: u32 = $b; + #[inline(always)] fn from_usize(value: usize) -> $t { value as $t } } }; @@ -63,3 +77,49 @@ impl_bitset_storage!(u16, u16::BITS); impl_bitset_storage!(u32, u32::BITS); impl_bitset_storage!(u64, u64::BITS); impl_bitset_storage!(u128, u128::BITS); + + +#[cfg(test)] +mod tests { + use super::*; + + #[test] + fn test_bitset_storage() { + fn harness(bits_expect: u32, shift_expect: u32) { + assert_eq!(T::ZERO, T::from_usize(0)); + assert_eq!(T::SHIFT, shift_expect); + assert_eq!(T::MASK, bits_expect as usize - 1); + assert_eq!(T::BITS, bits_expect); + } + + harness::(8, 3); + harness::(16, 4); + harness::(32, 5); + harness::(64, 6); + harness::(128, 7); + } + + #[test] + fn test_ordered_bitset() { + fn harness(indices: &[usize]) { + let mut bitset = OrderedBitSet::::new(); + for &index in indices { + bitset.insert(index, true); + } + for slot in 0..OrderedBitSet::::CAPACITY { + assert_eq!(bitset.get(slot), indices.contains(&slot)); + } + for &index in indices { + bitset.insert(index, false); + assert!(!bitset.get(index)); + } + } + + let indices = [1, 32, 33, 127, 44, 47, 49]; + harness::(&indices); + harness::(&indices); + harness::(&indices); + harness::(&indices); + harness::(&indices); + } +}