Message-Id: <DDHU4LL4GGIY.16OJMIL7ZK58P@nvidia.com>
Date: Tue, 14 Oct 2025 15:35:22 +0900
From: "Alexandre Courbot" <acourbot@...dia.com>
To: "Jesung Yang" <y.j3ms.n@...il.com>, "Miguel Ojeda"
 <miguel.ojeda.sandonis@...il.com>
Cc: "Miguel Ojeda" <ojeda@...nel.org>, "Alex Gaynor"
 <alex.gaynor@...il.com>, "Boqun Feng" <boqun.feng@...il.com>, "Gary Guo"
 <gary@...yguo.net>, Björn Roy Baron
 <bjorn3_gh@...tonmail.com>, "Benno Lossin" <lossin@...nel.org>, "Andreas
 Hindborg" <a.hindborg@...nel.org>, "Alice Ryhl" <aliceryhl@...gle.com>,
 "Trevor Gross" <tmgross@...ch.edu>, "Danilo Krummrich" <dakr@...nel.org>,
 "Alexandre Courbot" <acourbot@...dia.com>, <linux-kernel@...r.kernel.org>,
 <rust-for-linux@...r.kernel.org>, <nouveau@...ts.freedesktop.org>
Subject: Re: [PATCH v2 0/5] rust: add `TryFrom` and `Into` derive macros

On Sat Oct 11, 2025 at 11:53 PM JST, Jesung Yang wrote:
> Hi,
>
> On Fri, Oct 10, 2025 at 10:04 PM Miguel Ojeda
> <miguel.ojeda.sandonis@...il.com> wrote:
> [...]
>> But what case do you think we cannot assert? We can always take the
>> discriminant and reject whatever inputs (e.g. ranges) we decide, no?
>> And we know what type we are going into, so we can always decide what
>> the values to check will be, i.e. we could in principle even support
>> infallible conversions of the discriminant to other types like, say,
>> the bounded integers or powers of two.
>>
>> Maybe the issue is in what you say at "the discriminant value
>> interpreted as the target type" -- I am not sure what you mean by
>> "interpreted", i.e. I would think of this as accepting only some bit
>> patterns, i.e. working with in the discriminant space, not the target
>> type one.
>>
>> I may be missing something, but in any case, at the end of the day,
>> within a proc macro "everything" should be possible one way or another
>> -- even if we had to inspect manually the literals :) And it seems
>> worth to remove the pitfall.
>>
>> If really needed, we can always drop support for certain combinations.
>> We already do, in the sense that we don't cover every single other
>> type out there, like the ones I mention above, e.g. `Alignment`. But,
>> just in case, I assume with that approach you mean skipping some
>> combinations early (the ones that cannot be checked) and then still
>> asserting the discriminants, right? Otherwise the caveat is still
>> there.
>
> Sorry about the confusion -- the rough sketch I shared earlier had
> several mistakes.
>
> My actual intention was to emit a compile-time error using
> `compile_error!()` whenever a conversion could overflow. With this
> approach, the caveat wouldn't exist, since proc macro users wouldn't be
> able to generate `TryFrom` or `Into` (`From`) implementations that
> could potentially cause overflow issues. For example:
>
>     // This emits a compile-time error because not all `i128` values
>     // can be converted to `u128`, even though 0 and 1 are valid `u128`
>     // values.
>     #[derive(TryFrom, Into)]
>     #[try_from(u128)]
>     #[into(u128)]
>     #[repr(i128)]
>     enum MyEnum {
>         A = 0,
>         B = 1,
>     }
>
> To make this idea work as intended, I should have revised the earlier
> sketch as follows:
>
> - const U128_ALLOWED: [&str; 9] =
> -     ["u8", "i8", "u16", "i16", "i32", "u32", "u64", "i64", "u128"];
> - const I128_ALLOWED: [&str; 9] =
> -     ["u8", "i8", "u16", "i16", "i32", "u32", "u64", "i64", "i128"];
> + // Allowed helper inputs when `repr(u128)` is used.
> + const U128_ALLOWED: [&str; 1] = ["u128"];
> + // Allowed helper inputs when `repr(i128)` is used.
> + const I128_ALLOWED: [&str; 1] = ["i128"];
>
> The downside of this approach is that, as you can see, it is overly
> restrictive for large target types such as `u128` and `i128`, because
> the remaining numeric types cannot accommodate their full range. As a
> result, the macros could reject some valid use cases, for example when
> the actual discriminants can be converted without overflow, since the
> check operates at the type level rather than on specific discriminants.
>
> Considering this, and as you suggested, I think the right direction is
> to introduce a compile-time check for each discriminant. I initially
> thought it would be difficult, but after some exploration, it seems
> doable.
>
> Thanks a lot for your feedback, I really appreciate it!

Note that if we adopt the bounded integer types [1], this problem might
get exacerbated.

The initial motivation for these macros was automatically generate the
conversions between register fields and their Rust enum. This was
working fine as long as the fields were represented using primitive
integers, but we will likely switch to more restrictive types where only
a given set of bits is valid. So it is highly desirable to support
conversion from/to these bounded types as well.

This might not be too difficult to shoehorn as there is an
`is_in_bounds!` macro (which we can turn into a const method if that's
more suitable) that your proc macro could leverage, but I can't help but
thing that maybe there is a better, more general solution than
special-casing this?

[1] https://lore.kernel.org/rust-for-linux/20251009-bounded_ints-v2-0-ff3d7fee3ffd@nvidia.com/

Powered by blists - more mailing lists