Benno Lossin
2025-Jun-22 08:11 UTC
[PATCH 1/3] rust: add `num` module with `PowerOfTwo` type
On Fri Jun 20, 2025 at 3:14 PM CEST, Alexandre Courbot wrote:> +/// An unsigned integer which is guaranteed to be a power of 2. > +/// > +/// # Invariants > +/// > +/// The stored value is guaranteed to be a power of two. > +#[derive(Debug, Clone, Copy, PartialEq, Eq, PartialOrd, Ord, Hash)] > +#[repr(transparent)] > +pub struct PowerOfTwo<T>(T); > + > +macro_rules! power_of_two_impl { > + ($($t:ty),+) => { > + $( > + impl PowerOfTwo<$t> {I tried to use this type in a doctest like this: use kernel::num::PowerOfTwo; fn new(x: usize) -> PowerOfTwo<usize> { PowerOfTwo::new(1 << x) } And it doesn't compile :( error[E0034]: multiple applicable items in scope --> rust/doctests_kernel_generated.rs:4930:17 | 4930 | PowerOfTwo::new(1 << x) | ^^^ multiple `new` found | = note: candidate #1 is defined in an impl for the type `PowerOfTwo<u128>` = note: candidate #2 is defined in an impl for the type `PowerOfTwo<u16>` = note: candidate #3 is defined in an impl for the type `PowerOfTwo<u32>` = note: candidate #4 is defined in an impl for the type `PowerOfTwo<u64>` = note: and 2 others error: aborting due to 1 previous error The problem is that the function `new` exists 6 times for each of the integer types. You can write `PowerOfTwo::<usize>::new()` instead, but that's annoying... We probably need an `Integer` trait and then do impl<I: Integer> PowerOfTwo<I> { pub const fn new(value: I) -> Self; }> + /// Validates that `v` is a power of two at build-time, and returns it wrapped into > + /// [`PowerOfTwo`]. > + /// > + /// A build error is triggered if `v` cannot be asserted to be a power of two. > + /// > + /// # Examples > + /// > + /// ``` > + /// use kernel::num::PowerOfTwo; > + /// > + #[doc = concat!("let v = PowerOfTwo::<", stringify!($t), ">::new(16);")] > + /// assert_eq!(v.value(), 16); > + /// ``` > + #[inline(always)] > + pub const fn new(v: $t) -> Self { > + build_assert!(v.count_ones() == 1);Why not `v.is_power_of_two()`?> + Self(v)Missing `// INVARIANT` comment.> + } > + > + /// Validates that `v` is a power of two at runtime, and returns it wrapped into > + /// [`PowerOfTwo`]. > + /// > + /// [`None`] is returned if `v` was not a power of two. > + /// > + /// # Examples > + /// > + /// ``` > + /// use kernel::num::PowerOfTwo; > + /// > + #[doc = concat!( > + "assert_eq!(PowerOfTwo::<", > + stringify!($t), > + ">::try_new(16), Some(PowerOfTwo::<", > + stringify!($t), > + ">::new(16)));" > + )] > + #[doc = concat!( > + "assert_eq!(PowerOfTwo::<", > + stringify!($t), > + ">::try_new(15), None);" > + )] > + /// ``` > + #[inline(always)] > + pub const fn try_new(v: $t) -> Option<Self> {Maybe `new_checked` is a better name, since it doesn't return a result?> + match v.count_ones() {Why not `is_power_of_two()`?> + 1 => Some(Self(v)),Missing `// INVARIANT` comment.> + _ => None, > + } > + } > + > + /// Returns the value of this instance. > + /// > + /// It is guaranteed to be a power of two. > + /// > + /// # Examples > + /// > + /// ``` > + /// use kernel::num::PowerOfTwo; > + /// > + #[doc = concat!("let v = PowerOfTwo::<", stringify!($t), ">::new(16);")] > + /// assert_eq!(v.value(), 16); > + /// ``` > + #[inline(always)] > + pub const fn value(self) -> $t { > + self.0Let's add: if !self.0.is_power_of_two() { core::hint::unreachable_unchecked() } self.0> + } > + > + /// Returns the mask corresponding to `self.value() - 1`. > + /// > + /// # Examples > + /// > + /// ``` > + /// use kernel::num::PowerOfTwo; > + /// > + #[doc = concat!("let v = PowerOfTwo::<", stringify!($t), ">::new(0x10);")] > + /// assert_eq!(v.mask(), 0xf); > + /// ``` > + #[inline(always)] > + pub const fn mask(self) -> $t { > + self.0.wrapping_sub(1)Then use `self.value().wrapping_sub(1)` here instead to also propagate the information. --- Cheers, Benno> + }
Alexandre Courbot
2025-Jul-25 03:38 UTC
[PATCH 1/3] rust: add `num` module with `PowerOfTwo` type
Hi Benno, Sorry, took some time to come back to this! On Sun Jun 22, 2025 at 5:11 PM JST, Benno Lossin wrote:> On Fri Jun 20, 2025 at 3:14 PM CEST, Alexandre Courbot wrote: >> +/// An unsigned integer which is guaranteed to be a power of 2. >> +/// >> +/// # Invariants >> +/// >> +/// The stored value is guaranteed to be a power of two. >> +#[derive(Debug, Clone, Copy, PartialEq, Eq, PartialOrd, Ord, Hash)] >> +#[repr(transparent)] >> +pub struct PowerOfTwo<T>(T); >> + >> +macro_rules! power_of_two_impl { >> + ($($t:ty),+) => { >> + $( >> + impl PowerOfTwo<$t> { > > I tried to use this type in a doctest like this: > > use kernel::num::PowerOfTwo; > > fn new(x: usize) -> PowerOfTwo<usize> { > PowerOfTwo::new(1 << x) > } > > And it doesn't compile :( > > error[E0034]: multiple applicable items in scope > --> rust/doctests_kernel_generated.rs:4930:17 > | > 4930 | PowerOfTwo::new(1 << x) > | ^^^ multiple `new` found > | > = note: candidate #1 is defined in an impl for the type `PowerOfTwo<u128>` > = note: candidate #2 is defined in an impl for the type `PowerOfTwo<u16>` > = note: candidate #3 is defined in an impl for the type `PowerOfTwo<u32>` > = note: candidate #4 is defined in an impl for the type `PowerOfTwo<u64>` > = note: and 2 others > > error: aborting due to 1 previous error > > The problem is that the function `new` exists 6 times for each of the > integer types. You can write `PowerOfTwo::<usize>::new()` instead, but > that's annoying...This should go away as we switch to the non-generic `Alignment` type thankfully.> > We probably need an `Integer` trait and then do > > impl<I: Integer> PowerOfTwo<I> { > pub const fn new(value: I) -> Self; > } > >> + /// Validates that `v` is a power of two at build-time, and returns it wrapped into >> + /// [`PowerOfTwo`]. >> + /// >> + /// A build error is triggered if `v` cannot be asserted to be a power of two. >> + /// >> + /// # Examples >> + /// >> + /// ``` >> + /// use kernel::num::PowerOfTwo; >> + /// >> + #[doc = concat!("let v = PowerOfTwo::<", stringify!($t), ">::new(16);")] >> + /// assert_eq!(v.value(), 16); >> + /// ``` >> + #[inline(always)] >> + pub const fn new(v: $t) -> Self { >> + build_assert!(v.count_ones() == 1); > > Why not `v.is_power_of_two()`?Why not indeed. :) Fixed.> >> + Self(v) > > Missing `// INVARIANT` comment.Added (and in other places as well).> >> + } >> + >> + /// Validates that `v` is a power of two at runtime, and returns it wrapped into >> + /// [`PowerOfTwo`]. >> + /// >> + /// [`None`] is returned if `v` was not a power of two. >> + /// >> + /// # Examples >> + /// >> + /// ``` >> + /// use kernel::num::PowerOfTwo; >> + /// >> + #[doc = concat!( >> + "assert_eq!(PowerOfTwo::<", >> + stringify!($t), >> + ">::try_new(16), Some(PowerOfTwo::<", >> + stringify!($t), >> + ">::new(16)));" >> + )] >> + #[doc = concat!( >> + "assert_eq!(PowerOfTwo::<", >> + stringify!($t), >> + ">::try_new(15), None);" >> + )] >> + /// ``` >> + #[inline(always)] >> + pub const fn try_new(v: $t) -> Option<Self> { > > Maybe `new_checked` is a better name, since it doesn't return a result?Definitely.> >> + match v.count_ones() { > > Why not `is_power_of_two()`?Fixed, thanks.> >> + 1 => Some(Self(v)), > > Missing `// INVARIANT` comment. > >> + _ => None, >> + } >> + } >> + >> + /// Returns the value of this instance. >> + /// >> + /// It is guaranteed to be a power of two. >> + /// >> + /// # Examples >> + /// >> + /// ``` >> + /// use kernel::num::PowerOfTwo; >> + /// >> + #[doc = concat!("let v = PowerOfTwo::<", stringify!($t), ">::new(16);")] >> + /// assert_eq!(v.value(), 16); >> + /// ``` >> + #[inline(always)] >> + pub const fn value(self) -> $t { >> + self.0 > > Let's add: > > if !self.0.is_power_of_two() { > core::hint::unreachable_unchecked() > } > self.0Sure. Is it to enable compiler optimizations by making assumptions about the returned value?> >> + } >> + >> + /// Returns the mask corresponding to `self.value() - 1`. >> + /// >> + /// # Examples >> + /// >> + /// ``` >> + /// use kernel::num::PowerOfTwo; >> + /// >> + #[doc = concat!("let v = PowerOfTwo::<", stringify!($t), ">::new(0x10);")] >> + /// assert_eq!(v.mask(), 0xf); >> + /// ``` >> + #[inline(always)] >> + pub const fn mask(self) -> $t { >> + self.0.wrapping_sub(1) > > Then use `self.value().wrapping_sub(1)` here instead to also propagate > the information.Ack.