On Fri, Aug 15, 2025 at 7:32 AM Jesung Yang <y.j3ms.n@gmail.com> wrote:
>
> This patch series introduces derive macros for `TryFrom` and `Into`
> traits.
I took a quick look -- it is a nice series!
Some notes:
- 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.
- 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.)
- 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 a lot!
Cheers,
Miguel