Hello Alex, On 10/27/2025 8:54 AM, Alexandre Courbot wrote: > The core library's `From` implementations do not cover conversions > that are not portable or future-proof. For instance, even though it is > safe today, `From<usize>` is not implemented for `u64` because of the > possibility to support larger-than-64bit architectures in the future. > > However, the kernel supports a narrower set of architectures, and in the > case of Nova we only support 64-bit. This makes it helpful and desirable > to provide more infallible conversions, lest we need to rely on the `as` > keyword and carry the risk of silently losing data. > > Thus, introduce a new module `num` that provides safe const functions > performing more conversions allowed by the build target, as well as > `FromAs` and `IntoAs` traits that are just extensions of `From` and > `Into` to conversions that are known to be lossless.
Why not just implement `From` and `Into` for the types missing it then, with adequate comments about why such conversions are Ok for the kernel, instead of introducing a new trait? This is exactly what `From`/`Into` is for right? > Suggested-by: Danilo Krummrich <[email protected]> > Link: > https://lore.kernel.org/rust-for-linux/[email protected]/ > Acked-by: Danilo Krummrich <[email protected]> > Signed-off-by: Alexandre Courbot <[email protected]> > --- > drivers/gpu/nova-core/nova_core.rs | 1 + > drivers/gpu/nova-core/num.rs | 158 > +++++++++++++++++++++++++++++++++++++ > 2 files changed, 159 insertions(+) > > diff --git a/drivers/gpu/nova-core/nova_core.rs > b/drivers/gpu/nova-core/nova_core.rs > index e130166c1086..9180ec9c27ef 100644 > --- a/drivers/gpu/nova-core/nova_core.rs > +++ b/drivers/gpu/nova-core/nova_core.rs > @@ -13,6 +13,7 @@ > mod gfw; > mod gpu; > mod gsp; > +mod num; > mod regs; > mod vbios; > > diff --git a/drivers/gpu/nova-core/num.rs b/drivers/gpu/nova-core/num.rs > new file mode 100644 > index 000000000000..adb5a92f0d51 > --- /dev/null > +++ b/drivers/gpu/nova-core/num.rs > @@ -0,0 +1,158 @@ > +// SPDX-License-Identifier: GPL-2.0 > + > +//! Numerical helpers functions and traits. > +//! > +//! This is essentially a staging module for code to mature until it can be > moved to the `kernel` > +//! crate. > + > +use kernel::{build_error, macros::paste}; > + > +macro_rules! impl_lossless_as { > + ($from:ty as { $($into:ty),* }) => { > + $( > + paste! { > + #[doc = ::core::concat!( > + "Losslessly converts a [`", > + ::core::stringify!($from), > + "`] into a [`", > + ::core::stringify!($into), > + "`].")] > + /// > + /// This conversion is allowed as it is always lossless. Prefer > this over the `as` > + /// keyword to ensure no lossy conversions are performed. > + /// > + /// This is for use from a `const` context. For non `const` use, > prefer the [`FromAs`] > + /// and [`IntoAs`] traits. > + /// > + /// # Examples > + /// > + /// ``` > + /// use crate::num; > + /// > + #[doc = ::core::concat!( > + "assert_eq!(num::", > + ::core::stringify!($from), > + "_as_", > + ::core::stringify!($into), > + "(1", > + ::core::stringify!($from), > + "), 1", > + ::core::stringify!($into), > + ");")] > + /// ``` > + #[allow(unused)] > + pub(crate) const fn [<$from _as_ $into>](value: $from) -> $into { > + kernel::static_assert!(size_of::<$into>() >= > size_of::<$from>()); > + > + value as $into > + } > + } > + )* > + }; > +} > + > +impl_lossless_as!(u8 as { u16, u32, u64, usize }); > +impl_lossless_as!(u16 as { u32, u64, usize }); > +impl_lossless_as!(u32 as { u64, usize } ); I prefer consistency with the notation in num.rs in the rust standard library: impl_from!(u16 => usize) But I am Ok with your notation as well, which is concise. > +// `u64` and `usize` have the same size on 64-bit platforms. > +#[cfg(CONFIG_64BIT)] > +impl_lossless_as!(u64 as { usize } ); > + > +// A `usize` fits into a `u64` on 32 and 64-bit platforms. > +#[cfg(any(CONFIG_32BIT, CONFIG_64BIT))] > +impl_lossless_as!(usize as { u64 }); > + > +// A `usize` fits into a `u32` on 32-bit platforms. > +#[cfg(CONFIG_32BIT)] > +impl_lossless_as!(usize as { u32 }); > + > +/// Extension trait providing guaranteed lossless conversion to `Self` from > `T`. > +/// > +/// The standard library's `From` implementations do not cover conversions > that are not portable or > +/// future-proof. For instance, even though it is safe today, `From<usize>` > is not implemented for > +/// [`u64`] because of the possibility to support larger-than-64bit > architectures in the future. > +/// > +/// The workaround is to either deal with the error handling of [`TryFrom`] > for an operation that > +/// technically cannot fail, or to use the `as` keyword, which can silently > strip data if the > +/// destination type is smaller than the source. > +/// > +/// Both options are hardly acceptable for the kernel. It is also a much > more architecture > +/// dependent environment, supporting only 32 and 64 bit architectures, with > some modules > +/// explicitly depending on a specific bus width that could greatly benefit > from infallible > +/// conversion operations. > +/// > +/// Thus this extension trait that provides, for the architecture the kernel > is built for, safe > +/// conversion between types for which such conversion is lossless. > +/// > +/// In other words, this trait is implemented if, for the current build > target and with `t: T`, the > +/// `t as Self` operation is completely lossless. > +/// > +/// Prefer this over the `as` keyword to ensure no lossy conversions are > performed. > +/// > +/// If you need to perform a conversion in `const` context, use > [`u64_as_usize`], > +/// [`u32_as_usize`], [`usize_as_u64`], etc. > +/// > +/// # Examples > +/// > +/// ``` > +/// use crate::num::FromAs; > +/// > +/// assert_eq!(usize::from_as(0xf00u32), 0xf00u32 as usize); This `from_as()` syntax will be very confusing for users IMO, honestly we should just keep it as `from()`, otherwise there is confusion and ambiguity around whether someone should use `::from()` or `::from_as()`. Please let us just keep all infallible conversions to use `from()`/`into()` and all infallible ones to use `try_from()`/`try_into()`. No need to additional `_as()` prefix, as it creates confusion. In the end of the day, the fact the conversion is lossless is not relevant, all conversions that don't use the `as` keyword should be expected to be lossless right? thanks, - Joel
