Thank you for your contribution! Sashiko AI review found 2 potential issue(s) to consider: - [Low] rust: macros: unused_comparisons warning on 128-bit boundary checks - [Low] rust: macros: dead_code and unused_variables on empty enums --
commit 2f4a5e53f9efd805dce115afdbd130dde8b72a62 Author: Jesung Yang <[email protected]> rust: macros: add derive macro for `TryFrom` Introduce a procedural macro TryFrom to automatically implement the TryFrom trait for unit-only enums, reducing boilerplate when interpreting numeric values as enum variants. > diff --git a/rust/macros/convert.rs b/rust/macros/convert.rs > index 6702c2e48c1fe..c64e35447859c 100644 > --- a/rust/macros/convert.rs > +++ b/rust/macros/convert.rs [ ... ] > @@ -245,6 +253,56 @@ fn from(#param: #enum_ident) -> #input_ty { > } > } > > + 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()); [Severity: Low] Does this trigger an unused_variables warning if the user derives TryFrom on a variant-less (empty) enum? The generated try_from function never uses the value parameter in that case, which could result in a build failure under CONFIG_WERROR. > + > + let overflow_assertion = emit_overflow_assert(enum_ident, variants, > repr_ty, input_ty); [Severity: Low] Can this generated overflow assertion cause a dead_code warning for empty enums? When an enum is empty, no variant assertions are generated to invoke the internal fits() helper function, leaving it unused. Also, when deriving TryFrom for a 128-bit target type, does the generated fits() function trigger the unused_comparisons lint? For example, the bounds checks in emit_overflow_assert() generate comparisons like this: (d as ::core::primitive::u128) <= (dst_max as ::core::primitive::u128) If dst_max is the maximum possible value for a u128, the comparison is trivially true and triggers the unused_comparisons lint. This can cause build failures under CONFIG_WERROR. Would it make sense to apply #[allow(unused_comparisons)] to the entire fits() function scope rather than just the signedness boolean variables? > + let emit_cast = |variant| { > + let qualified_variant = ::quote::quote! { #enum_ident::#variant > }; > + match input_ty { -- Sashiko AI review ยท https://sashiko.dev/#/patchset/[email protected]?part=2
