Re: [PATCH v3] rust: num: document why Integer is sealed

[Alexandre Courbot]

On Thu Sep 17, 2026 at 12:10 AM JST, Younes Akhouayri via B4 Relay wrote:
> From: Younes Akhouayri <git@xxxxxxxxx>
>
> Bounded relies on Integer implementations to provide primitive integer
> semantics when justifying unchecked operations.
>
> Explain why Integer is sealed next to its private supertrait. Document
> the dependency inside fits_within, so callers can rely on its result
> without repeating that explanation. Mention sealing in safety comments
> that directly rely on integer metadata, shifts or conversions.
>
> Suggested-by: Miguel Ojeda <ojeda@xxxxxxxxxx>
> Suggested-by: Gary Guo <gary@xxxxxxxxxxx>
> Suggested-by: Alexandre Courbot <acourbot@xxxxxxxxxx>
> Link: https://lore.kernel.org/all/CANiq72m8kycbfQ1teyne-OtB5d5TsUcX_WW0FCkWB3Ayyg-qWw@xxxxxxxxxxxxxx/
> Link: https://lore.kernel.org/all/DL88SQWYU15W.2CVZB5NVSSJGK@xxxxxxxxxxx/
> Link: https://lore.kernel.org/all/CANiq72kx-YPPEruOFdu-Dp7GX+8=Et6svG+sQtqEmmF7kpnVyQ@xxxxxxxxxxxxxx/
> Link: https://lore.kernel.org/all/DLDSZ09SM8HI.3PHPYGLV2YZBX@xxxxxxxxxx/
> Signed-off-by: Younes Akhouayri <git@xxxxxxxxx>
> ---
> Changes in v3:
> - Document the sealing dependency inside fits_within and restore its
>   callers' original safety comments, following Alexandre's feedback.
> - Restore extend's invariant-based comment and remove the repeated
>   sealing explanation from the second cast safety comment.
> - Link to v2: https://patch.msgid.link/20260908-docs-rust-num-integer-sealing-safety-v2-1-e8c65234db82@xxxxxxxxx
>
> Changes in v2:
> - Shorten the comment explaining why `Integer` is sealed.
> - Mention the seal in the `SAFETY` comments that rely on it.
> - Link to v1: https://patch.msgid.link/20260906-docs-rust-num-integer-sealing-safety-v1-1-78057391302c@xxxxxxxxx
>
> To: Alexandre Courbot <acourbot@xxxxxxxxxx>
> To: Yury Norov <yury.norov@xxxxxxxxx>
> To: Miguel Ojeda <ojeda@xxxxxxxxxx>
> To: Boqun Feng <boqun@xxxxxxxxxx>
> To: Gary Guo <gary@xxxxxxxxxxx>
> To: Björn Roy Baron <bjorn3_gh@xxxxxxxxxxxxxx>
> To: Benno Lossin <lossin@xxxxxxxxxx>
> To: Andreas Hindborg <a.hindborg@xxxxxxxxxx>
> To: Alice Ryhl <aliceryhl@xxxxxxxxxx>
> To: Trevor Gross <tmgross@xxxxxxxxx>
> To: Danilo Krummrich <dakr@xxxxxxxxxx>
> To: Daniel Almeida <daniel.almeida@xxxxxxxxxxxxx>
> To: Tamir Duberstein <tamird@xxxxxxxxxx>
> To: Onur Özkan <work@xxxxxxxxxxxxx>
> Cc: rust-for-linux@xxxxxxxxxxxxxxx
> Cc: linux-kernel@xxxxxxxxxxxxxxx
> ---
>  rust/kernel/num.rs         |  1 +
>  rust/kernel/num/bounded.rs | 32 +++++++++++++++++++-------------
>  2 files changed, 20 insertions(+), 13 deletions(-)
>
> diff --git a/rust/kernel/num.rs b/rust/kernel/num.rs
> index de589792a77a..0449e84a384a 100644
> --- a/rust/kernel/num.rs
> +++ b/rust/kernel/num.rs
> @@ -21,6 +21,7 @@ pub trait Sealed {}
>  
>  /// Describes core properties of integer types.
>  pub trait Integer:
> +    // Sealed so that unsafe code can rely on the correctness of its implementations.
>      private::Sealed
>      + Sized
>      + Copy
> diff --git a/rust/kernel/num/bounded.rs b/rust/kernel/num/bounded.rs
> index 2a2b0a4bca5e..aff35e2eb616 100644
> --- a/rust/kernel/num/bounded.rs
> +++ b/rust/kernel/num/bounded.rs
> @@ -40,6 +40,8 @@ macro_rules! fits_within {
>  /// Returns `true` if `value` can be represented with at most `N` bits in a `T`.
>  #[inline(always)]
>  fn fits_within<T: Integer>(value: T, num_bits: u32) -> bool {
> +    // `Integer` is sealed, so the bit width, shifts and equality have primitive integer semantics.
> +    // Unsafe code relies on this function correctly checking whether `value` fits.
>      fits_within!(value, T, num_bits)
>  }

So while I think these two hunks are indeed required...

>  
> @@ -472,8 +474,9 @@ pub fn cast<U>(self) -> Bounded<U, N>
>          T: Integer,
>          U: Integer<Signedness = T::Signedness>,
>      {
> -        // SAFETY: The converted value is represented using `N` bits, `U` can contain `N` bits, and
> -        // `U` and `T` have the same sign, hence this conversion cannot fail.
> +        // SAFETY: `Integer` is sealed, so the bit widths and signedness are correct. The converted
> +        // value is represented using `N` bits, `U` can contain `N` bits, and `U` and `T` have the
> +        // same sign, hence this conversion cannot fail.
>          let value = unsafe { U::try_from(self.get()).unwrap_unchecked() };
>  
>          // SAFETY: Although the backing type has changed, the value is still represented within
> @@ -498,8 +501,9 @@ pub fn shr<const SHIFT: u32, const RES: u32>(self) -> Bounded<T, RES> {
>          const_assert!(SHIFT < T::BITS);
>          const_assert!(RES + SHIFT >= N);
>  
> -        // SAFETY: We shift the value right by `SHIFT`, reducing the number of bits needed to
> -        // represent the shifted value by as much, and just asserted that `RES >= N - SHIFT`.
> +        // SAFETY: `Integer` is sealed, so the shift has primitive integer semantics. We reduce the
> +        // number of bits needed to represent the shifted value by `SHIFT`, and just asserted that
> +        // `RES >= N - SHIFT`.
>          unsafe { Bounded::__new(self.0 >> SHIFT) }
>      }
>  
> @@ -550,8 +554,9 @@ pub fn shr_exact<const SHIFT: u32, const RES: u32>(self) -> Option<Bounded<T, RE
>      pub fn shl<const SHIFT: u32, const RES: u32>(self) -> Bounded<T, RES> {
>          const_assert!(RES >= N + SHIFT);
>  
> -        // SAFETY: We shift the value left by `SHIFT`, augmenting the number of bits needed to
> -        // represent the shifted value by as much, and just asserted that `RES >= N + SHIFT`.
> +        // SAFETY: `Integer` is sealed, so the shift has primitive integer semantics. We augment
> +        // the number of bits needed to represent the shifted value by `SHIFT`, and just asserted
> +        // that `RES >= N + SHIFT`.
>          unsafe { Bounded::__new(self.0 << SHIFT) }
>      }
>  }
> @@ -1028,8 +1033,9 @@ impl<T, const N: u32> From<$type> for Bounded<T, N>
>              Self: AtLeastXBits<{ <$type as Integer>::BITS as usize }>,
>          {
>              fn from(value: $type) -> Self {
> -                // SAFETY: The trait bound on `Self` guarantees that `N` bits is
> -                // enough to hold any value of the source type.
> +                // SAFETY: `Integer` is sealed, so the bit widths and signedness are correct. The
> +                // trait bound on `Self` guarantees that `N` bits is enough to hold any value of
> +                // the source type.
>                  unsafe { Self::__new(T::from(value)) }
>              }
>          }
> @@ -1104,9 +1110,9 @@ impl<T, const N: u32> From<Bounded<T, N>> for $type
>              Bounded<T, N>: FitsInXBits<{ <$type as Integer>::BITS as usize }>,
>          {
>              fn from(value: Bounded<T, N>) -> $type {
> -                // SAFETY: The trait bound on `Bounded` ensures that any value it holds (which
> -                // is constrained to `N` bits) can fit into the destination type, so this
> -                // conversion cannot fail.
> +                // SAFETY: `Integer` is sealed, so the bit widths and signedness are correct. The
> +                // trait bound on `Bounded` ensures that any value it holds (which is constrained
> +                // to `N` bits) can fit into the destination type, so this conversion cannot fail.
>                  unsafe { <$type>::try_from(value.get()).unwrap_unchecked() }
>              }
>          }
> @@ -1137,8 +1143,8 @@ impl<T, const N: u32> From<bool> for Bounded<T, N>
>      T: Integer<Signedness = Unsigned> + From<bool>,
>  {
>      fn from(value: bool) -> Self {
> -        // SAFETY: A boolean is represented by `0` or `1`, so it fits within any valid unsigned
> -        // `Bounded` width.
> +        // SAFETY: `Integer` is sealed, so `T` is a primitive unsigned integer. A boolean is
> +        // represented by `0` or `1`, so it fits within any valid unsigned `Bounded` width.

... here I am not sure the remaining mentions give us much. The seal is
a property of `Integer` itself, and is documented there. So an `Integer`
bound already tells the reader that primitive semantics are guaranteed,
just as we don't justify `u32::BITS == 32` in a SAFETY comment.
Repeating that `Integer` is sealed at every site does not add new
information, it just makes the comments more complex.

The comment on `fits_within` is legit though, since it is safe code
whose correctness the unsafe code depends on.

That being said, I know Miguel asked for these mentions on v1, and I
don't want to send you back and forth between two reviewers. Miguel,
would you be OK with keeping this to the trait comment and
`fits_within`?