Nouveau - Oct 2025 - [PATCH v2 0/5] rust: add `TryFrom` and `Into` derive macros

Hi,

On Wed, Oct 1, 2025 at 5:13?AM Miguel Ojeda
<miguel.ojeda.sandonis at gmail.com> wrote:
[...]
>   - My biggest concern is the overflow caveat, which is a fairly big
> one if one, especially if one is dealing with runtime values.
>
>     Can we do better? Accessing the discriminant via `as` is available
> in const context, and you already have every variant easily available,
> so can we check that every variant fits in the relevant target types?
>
>     For instance, by creating some item-level const blocks
> (`static_assert!`s) -- dummy example for an unsigned-to-unsigned case:
>
>         const _: () = { assert! (E::A as u128 <= u8::MAX as u128); };
>         const _: () = { assert! (E::B as u128 <= u8::MAX as u128); };
>         ...
>
>     and so on. There may be better ways to do it -- I just quickly
> brute forced it that unsigned case with what you already had for
> handling variants:
>
>         variants.iter().map(|variant| {
>             quote! {
>                 const _: () = { assert!(#enum_ident::#variant as u128
> <= #ty::MAX as u128); };
>             }
>         });
>
>     Maybe this was already discussed elsewhere and there is a reason
> not to do something like this, but it seems to me that we should try
> to avoid that risk.
Thanks, I see your point, and I agree that compile-time checking for
potential overflows is a better and safer approach.

That said, it becomes a bit trickier when dealing with conversions
between signed and unsigned types, particularly when `u128` and `i128`
are involved. For example:

    #[derive(TryFrom, Into)]
    #[try_from(u128)]
    #[into(u128)]
    #[repr(i128)]
    enum MyEnum {
        A = 0xffffffffffffffff0, // larger than u64::MAX
        B = -1
    }

In this case, since there's no numeric type that can encompass both
`u128` and `i128`, I don't think we can express a compile-time
assertion like the one you suggested. While such edge cases involving
128-bit numeric types are unlikely in practice, the broader challenge
is that, in signed-to-unsigned conversions, I think it's difficult to
detect overflows using only the `repr` type, the target type, and the
discriminant value interpreted as the target type (please correct me if
I've misunderstood something here).

I'm considering an alternative approach: performing these checks while
parsing the macro inputs, to handle all combinations of `repr` and
target types (such as `u128` in the above example) in a unified way. I
believe this makes the behavior easier to reason about and better
covers edge cases like conversions between `i128` and `u128`. For
example:

    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"];
    ...

    // Use this function after parsing `#[repr(...)]`, `#[into(u128)]`
    // and `#[try_from(...)]`.
    fn check_overflow(
        discriminant_repr: &str,
        helper_inputs: Vec<&str>
    ) -> Vec<&str> {
        let mut violations = Vec::new();
        if discriminant_repr == "u128" {
            for ty in helper_inputs.iter() {
                if !U128_ALLOWED.contains(&ty) {
                    violations.push(ty);
                }
            }
        } else if discriminant_repr == "i128" {
            for ty in helper_inputs.iter() {
                if !I128_ALLOWED.contains(&ty) {
                    violations.push(ty);
                }
            }
        }
        ...
        violations
    }

This is a rough sketch, but it gives a consistent way to reject
obviously invalid combinations early during parsing. I'd appreciate
your thoughts -- does this approach seem reasonable to you as well?
>   - On other news, I will post in the coming days the `syn` patches,
> and my plan is to merge them for next cycle, so when those are out,
> Benno thought you could give them a go (we can still merge this with
> your current approach and then convert, but still, more `syn` users
> and converting the existing macros would be nice :).
>
>     (By the way, the linked patches about converting the existing
> macros to `syn` are an RFC in the sense that they cannot be applied,
> but having `syn` itself is something we already agreed on a long time
> ago.)
Sounds good -- I'd be happy to give `syn` a try. It should simplify
the parsing logic quite a bit, and I believe it'll also make things
easier for reviewers.
>   - Couple nits: typo arise -> arises, and I would do `repr-rust`
> instead of `repr-rs` since that is the anchor in the reference that
> you are linking.
Thanks, I'll fix them in v3.

Best Regards,
Jesung
>
> Thanks a lot!
>
> Cheers,
> Miguel

On Fri, Oct 10, 2025 at 12:08?PM Jesung Yang <y.j3ms.n at gmail.com>
wrote:
>
> That said, it becomes a bit trickier when dealing with conversions
> between signed and unsigned types, particularly when `u128` and `i128`
> are involved. For example:
Yeah, it is why I said it was a dummy unsigned case -- I didn't mean
that single comparison would work for all cases.

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.

Thanks!

Cheers,
Miguel

Hi,

On Fri, Oct 10, 2025 at 10:04?PM Miguel Ojeda
<miguel.ojeda.sandonis at gmail.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!

Best Regards,
Jesung
> Thanks!
>
> Cheers,
> Miguel