[<prev] [next>] [<thread-prev] [day] [month] [year] [list]
Message-Id: <DG7C1LYQ4SOQ.1HBJ8WH923JHW@gmail.com>
Date: Fri, 06 Feb 2026 06:06:24 +0900
From: "Jesung Yang" <y.j3ms.n@...il.com>
To: "Charalampos Mitrodimas" <charmitro@...teo.net>, <y.j3ms.n@...il.com>
Cc: "Miguel Ojeda" <ojeda@...nel.org>, "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 v5 2/4] rust: macros: add derive macro for `TryFrom`
On Wed Feb 4, 2026 at 10:39 AM KST, Charalampos Mitrodimas wrote:
> Jesung Yang via B4 Relay <devnull+y.j3ms.n.gmail.com@...nel.org> writes:
[...]
>> + fn impl_try_from(
>> + enum_ident: &Ident,
>> + variants: &[Ident],
>> + repr_ty: &syn::Path,
>> + input_ty: &ValidTy,
>> + ) -> TokenStream {
>> + let param = Ident::new("value", Span::call_site());
>> +
>> + let overflow_assertion = emit_overflow_assert(enum_ident, variants, repr_ty, input_ty);
>> + let emit_cast = |variant| {
>> + let variant = ::quote::quote! { #enum_ident::#variant };
>> + match input_ty {
>> + ValidTy::Bounded(inner) => {
>> + let base_ty = inner.emit_qualified_base_ty();
>> + let expr = parse_quote! { #variant as #base_ty };
>> + inner.emit_new(&expr)
>> + }
>> + ValidTy::Primitive(ident) if ident == "bool" => {
>> + ::quote::quote! { ((#variant as #repr_ty) == 1) }
>> + }
>> + qualified @ ValidTy::Primitive(_) => ::quote::quote! { #variant as #qualified },
>> + }
>> + };
>> +
>> + let clauses = variants.iter().map(|variant| {
>> + let cast = emit_cast(variant);
>> + ::quote::quote! {
>> + if #param == #cast {
>> + ::core::result::Result::Ok(#enum_ident::#variant)
>> + } else
>> + }
>> + });
>> +
>> + ::quote::quote! {
>> + #[automatically_derived]
>> + impl ::core::convert::TryFrom<#input_ty> for #enum_ident {
>> + type Error = ::kernel::prelude::Error;
>> + fn try_from(#param: #input_ty) -> Result<#enum_ident, Self::Error> {
>> + #overflow_assertion
>> +
>> + #(#clauses)* {
>> + ::core::result::Result::Err(::kernel::prelude::EINVAL)
>
> What happens if we need a different error type here? For example, a
> quick look around in nova-core's "Chipset" enum, an unrecognized chipset
> ID warrants ENODEV rather than EINVAL, since it's about device
> identification.
>
> Not sure if it fits the design, just wondering if this flexibility would
> be useful, but would something like an optional
>
> error = <ERROR>
>
> in the
>
> #[try_from(...)]
>
> attribute make sense? e.g.
>
> #[try_from(u32, error = ENODEV)]
>
> defaulting ofcourse to EINVAL if unspecified.
I believe this is indeed a desired change.
Back in September, an RFC [1] using the same API (i.e., without error
customization) was sent; I took a quick look at the time and felt
everything was OK, but in hindsight, the need for this flexibility is
clear.
Your proposed API looks good to me. Unless there are objections, I'll
move forward with this approach.
Thanks!
[1] https://lore.kernel.org/rust-for-linux/20250930-nova-tryfrom-v1-1-0cc1bb507047@nvidia.com/
Best regards,
Jesung
Powered by blists - more mailing lists