[PATCH 0/4] rust: add `FromPrimitive` support

Jesung Yang posted 4 patches 3 months, 2 weeks ago
rust/kernel/convert.rs | 154 +++++++++++++++++++++++++++++
rust/kernel/lib.rs     |   1 +
rust/macros/convert.rs | 217 +++++++++++++++++++++++++++++++++++++++++
rust/macros/lib.rs     |  71 ++++++++++++++
rust/macros/quote.rs   |  46 ++++++++-
5 files changed, 487 insertions(+), 2 deletions(-)
create mode 100644 rust/kernel/convert.rs
create mode 100644 rust/macros/convert.rs
[PATCH 0/4] rust: add `FromPrimitive` support
Posted by Jesung Yang 3 months, 2 weeks ago
This patch series introduces a new `FromPrimitive` trait along with its
corresponding derive macro.

A few enhancements were made to the custom `quote!` macro to write the
derive macro. These include support for additional punctuation tokens
and a fix for an unused variable warning when quoting simple forms.
Detailed information about these enhancements is provided in the
relevant patches.

While cleaning up the implementations, I came across an alternative
form of the `FromPrimitive` trait that might better suit the current
use case. Since types that implement this trait may often rely on just
one `from_*` method, the following design could be a simpler fit:

    trait FromPrimitive: Sized {
        type Primitive;

        fn from_bool(b: bool) -> Option<Self>
        where
            <Self as FromPrimitive>::Primitive: From<bool>,
        {
            Self::from_primitive(b.into())
        }

        fn from_primitive(n: Self::Primitive) -> Option<Self>;
    }

This is just a thought and not something I feel strongly about, but I
wanted to share it in case others find the idea useful. Feedback or
suggestions are very welcome.

The original discussion of FromPrimitive can be found on Zulip [1].

[1] https://rust-for-linux.zulipchat.com/#narrow/channel/288089/topic/x/near/524427350

Jesung Yang (4):
  rust: introduce `FromPrimitive` trait
  rust: macros: extend custom `quote!` macro
  rust: macros: prefix variable `span` with underscore
  rust: macros: add derive macro for `FromPrimitive`

 rust/kernel/convert.rs | 154 +++++++++++++++++++++++++++++
 rust/kernel/lib.rs     |   1 +
 rust/macros/convert.rs | 217 +++++++++++++++++++++++++++++++++++++++++
 rust/macros/lib.rs     |  71 ++++++++++++++
 rust/macros/quote.rs   |  46 ++++++++-
 5 files changed, 487 insertions(+), 2 deletions(-)
 create mode 100644 rust/kernel/convert.rs
 create mode 100644 rust/macros/convert.rs


base-commit: dc35ddcf97e99b18559d0855071030e664aae44d
-- 
2.39.5
Re: [PATCH 0/4] rust: add `FromPrimitive` support
Posted by Alexandre Courbot 3 months, 2 weeks ago
On Tue Jun 24, 2025 at 12:14 AM JST, Jesung Yang wrote:
> This patch series introduces a new `FromPrimitive` trait along with its
> corresponding derive macro.
>
> A few enhancements were made to the custom `quote!` macro to write the
> derive macro. These include support for additional punctuation tokens
> and a fix for an unused variable warning when quoting simple forms.
> Detailed information about these enhancements is provided in the
> relevant patches.

Thanks for crafting this! I have been able to sucessfully use it to
provide the implementations needed for Nova's `register!()` macro.

>
> While cleaning up the implementations, I came across an alternative
> form of the `FromPrimitive` trait that might better suit the current
> use case. Since types that implement this trait may often rely on just
> one `from_*` method, the following design could be a simpler fit:
>
>     trait FromPrimitive: Sized {
>         type Primitive;
>
>         fn from_bool(b: bool) -> Option<Self>
>         where
>             <Self as FromPrimitive>::Primitive: From<bool>,
>         {
>             Self::from_primitive(b.into())
>         }
>
>         fn from_primitive(n: Self::Primitive) -> Option<Self>;
>     }

This alternative form looks like it could be more suitable for us
indeed.

The userspace `num::FromPrimitive` is a bit too exhaustive to my taste,
as it makes methods available to build from primitives that we don't
really want. For instance, if I have an enum type that should only be
built from a `u16` or larger (because it has variants with values bigger
than 256), then it will still have a `from_u8` method, which looks
terribly wrong to me as the very fact that you are trying to build from
a `u8` indicates that you have mistakenly truncated the value at some
point, and thus you will possibly obtain a different variant from the
one you would get if you hadn't!

So from this perspective, having an associated type to indicate the
valid primitive type like you suggest sounds like an excellent idea. I
probably also wouldn't mind if we only supported that specific type
either, as callers can make the required conversion themselves and doing
so actually forces them to be conscious of the types they are passing
and be warned of potential mismatches.

But I guess that if we do so we can just introduce a derive macro that
implements `TryFrom` for us, without needing to introduce a new trait. I
might be too focused on my own use-case and would like to hear other
usage perspectives for this work.

If you add an associated type, I guess this means the derive macro
should have a helper attribute to specify it?

Another important aspect discussed on Zulip is the counterpart to
`FromPrimitive`, `ToPrimitive`. Here I feel more strongly that we should
*not* follow that the userspace `num` crate does, i.e. having all
operations return an `Option` - that would result in a lot of unneeded
error-checking code in the kernel. No, here it is pretty clear that we
should only provide infallible methods for the types that can store all
the possible values. Which is basically... one or several `Into`
implementations?

So indeed I'd like to understand first whether we actually need a new
trait, or whether our needs can be met by derive macros that provide the
right `TryFrom` and `Into` implementations. For nova-core, I think the
latter would be ok, but maybe I am missing the larger picture.
Re: [PATCH 0/4] rust: add `FromPrimitive` support
Posted by Jesung Yang 3 months, 2 weeks ago
On Wed, Jun 25, 2025 at 11:07 PM Alexandre Courbot <acourbot@nvidia.com> wrote:
>
> If you add an associated type, I guess this means the derive macro
> should have a helper attribute to specify it?

The current implementation tries to detect the enum's representation
(e.g., `#[repr(u8)]`) and falls back to `isize` if nothing is provided,
since it's the default [1]. However, when `#[repr(C)]` is used, the
internal representation is not controlled on the Rust side. To quote
the reference [2]:

For field-less enums, the C representation has the size and
alignment of the default enum size and alignment for the target
platform's C ABI.

Given this, it would definitely help with determinism if users provided
an explicit attribute. Being explicit also often improves clarity and
makes the intent more obvious.

[1]: https://doc.rust-lang.org/reference/items/enumerations.html#r-items.enum.discriminant.repr-rust
[2]: https://doc.rust-lang.org/reference/type-layout.html?highlight=repr#r-layout.repr.c.enum

Best regards,
Jesung