Re: [PATCH 1/2] rust: add BitInt integer wrapping type

Alexandre Courbot
2025-11-02T05:46:10Z

Quick self-review as I've found a couple of points that can be improved.

On Fri Oct 31, 2025 at 10:39 PM JST, Alexandre Courbot wrote:
<snip>
> diff --git a/rust/kernel/num/bitint.rs b/rust/kernel/num/bitint.rs
> new file mode 100644
> index 000000000000..5af1447347a8
> --- /dev/null
> +++ b/rust/kernel/num/bitint.rs
> @@ -0,0 +1,896 @@
> +// SPDX-License-Identifier: GPL-2.0
> +
> +//! [`BitInt`], a primitive integer type with a limited set of bits usable to represent values.
> +
> +use core::ops::Deref;
> +
> +use kernel::num::{Integer, Signed, Unsigned};
> +use kernel::prelude::*;
> +
> +/// Evaluates to `true` if `$value` can be represented using at most `$num_bits` on `$type`.
> +///
> +/// Can be used in const context.
> +macro_rules! fits_within {
> + ($value:expr, $type:ty, $num_bits:expr) => {{
> + // Any attempt to create a `BitInt` with more bits used for representation than its backing
> + // type (i.e. create an invalid `BitInt`) must be aborted at build time.
> + build_assert!(
> + <$type>::BITS >= $num_bits,
> + "Number of bits requested for representation is larger than backing type."
> + );

All these similar invariant checks sprinkled around the code are very
error-prone. A better way to enforce them is to have a private
constructor that is called by any path that creates a new `BitInt`, and
have these invariants enforced in a const block:

const fn __new(value: T) -> Self {
// Enforce the type invariants.
const {
// `NUM_BITS` cannot be zero.
build_assert!(NUM_BITS != 0);
// The backing type is at least as large as `NUM_BITS`.
build_assert!(NUM_BITS <= T::BITS);
}

Self(value)
}

This means we do this in one place instead of 3 or 4, and since the
invariants are enforced in const context we get a decent error message
should they fail.

<snip>
> +mod atleast_impls {
> + use super::*;
> +
> + // Number of bits at least as large as `64`.
> + impl_size_rule!(AtLeast64Bits, 64);
> +
> + // Number of bits at least as large as `32`.
> + impl_size_rule!(AtLeast32Bits,
> + 32 33 34 35 36 37 38 39
> + 40 41 42 43 44 45 46 47
> + 48 49 50 51 52 53 54 55
> + 56 57 58 59 60 61 62 63
> + );
> + // Anything larger than `64` is also larger than `32`.
> + impl<T> AtLeast32Bits for T where T: AtLeast64Bits {}
> +
> + // Number of bits at least as large as `16`.
> + impl_size_rule!(AtLeast16Bits,
> + 16 17 18 19 20 21 22 23
> + 24 25 26 27 28 29 30 31
> + );
> + // Anything larger than `32` is also larger than `16`.
> + impl<T> AtLeast16Bits for T where T: AtLeast32Bits {}
> +
> + // Number of bits at least as large as a `8`.
> + impl_size_rule!(AtLeast8Bits, 8 9 10 11 12 13 14 15);
> + // Anything larger than `16` is also larger than `8`.
> + impl<T> AtLeast8Bits for T where T: AtLeast16Bits {}
> +
> + // Number of bits at least as large as `1`.
> + impl_size_rule!(AtLeast1Bit, 1 2 3 4 5 6 7);
> + // Anything larger than `8` is also larger than `1`.
> + impl<T> AtLeast1Bit for T where T: AtLeast8Bits {}
> +
> + /// Generates `From` implementations from a primitive type into a [`BitInt`] that is
> + /// guaranteed to being able to contain it.
> + macro_rules! impl_from_primitive {
> + ($($type:ty => $trait:ident),*) => {
> + $(
> + impl<T, const NUM_BITS: u32> From<$type> for BitInt<T, NUM_BITS>
> + where
> + Self: $trait,
> + T: From<$type> + Boundable,
> + {
> + fn from(value: $type) -> Self {
> + build_assert!(
> + T::BITS >= NUM_BITS,
> + "Number of bits requested for representation is larger than backing type."
> + );
> + // INVARIANT: the impl constraints guarantee that the source type is smaller
> + // than `NUM_BITS`, and the `build_assert` above that the backing type can
> + // contain `NUM_BITS`.
> + Self(T::from(value))
> + }
> + }
> + )*
> + }
> + }
> +
> + impl_from_primitive!(
> + bool => AtLeast1Bit,
> + u8 => AtLeast8Bits,
> + i8 => AtLeast8Bits,
> + u16 => AtLeast16Bits,
> + i16 => AtLeast16Bits,
> + u32 => AtLeast32Bits,
> + i32 => AtLeast32Bits,
> + u64 =>AtLeast64Bits,
> + i64 =>AtLeast64Bits
> + );

I dislike this part so much, especially since it could be done much more
cleanly if only we had `generic_const_exprs`. But for now we have to
work our way around with macros.

Still, this can be simplified significantly by replacing the
`AtLeast8Bits`, `AtLeast16Bits`, ... traits with a single
`AtLeastXBits<const N: usize>`.

By doing so, the cryptic

Self: $trait,

constraint can become

Self: AtLeastXBits<{ <$type as Integer>::BITS as usize }>,

which is much more explicit about its purpose, and we can remove the
`$trait` argument from the macro.

what more, we can even write a constraint like

Self: AtLeastXBits<{ <$type as Integer>::BITS as usize + 1 }>,

And support conversion from unsigned primitive types to signed `BitInt`s
with at least one more bit.

The same simplification can also be done for the `IntoXX` traits below.

I'll submit a v2 soonish with these improvements and some more minor
fixes.