From: sashiko-bot@kernel.org
To: "Kaiqi Guo" <chaoji_xinren@163.com>
Cc: ojeda@kernel.org, dri-devel@lists.freedesktop.org,
"Danilo Krummrich" <dakr@kernel.org>,
gary@garyguo.net, "Alexandre Courbot" <acourbot@nvidia.com>
Subject: Re: [PATCH v6 01/10] rust: macros: add derive macro for `Into`
Date: Tue, 29 Sep 2026 14:10:59 +0000 [thread overview]
Message-ID: <20260929141100.1452F1F000FF@smtp.kernel.org> (raw)
In-Reply-To: <20260929135855.36775-2-chaoji_xinren@163.com>
Thank you for your contribution! Sashiko AI review found 2 potential issue(s) to consider:
- [Low] rust: macros: Into derive fails on repr with nested arguments
- [Low] rust: macros: Bounded validation incorrectly accepts bool
--
commit f3bc2f572cd1e2c7a2b7b3fee89fc39e25420448
Author: Jesung Yang <y.j3ms.n@gmail.com>
rust: macros: add derive macro for Into
This commit introduces a procedural macro Into to automatically derive the Into
trait for unit-only enums. It generates From implementations based on the into
or repr attributes, supporting primitive types and the Bounded type. It also
generates compile-time assertions to ensure variant discriminants fit within
the destination type.
Link: https://lore.kernel.org/rust-for-linux/20250624132337.2242-1-dakr@kernel.org/ [1]
> diff --git a/rust/macros/convert.rs b/rust/macros/convert.rs
> new file mode 100644
> index 0000000000000..6702c2e48c1fe
> --- /dev/null
> +++ b/rust/macros/convert.rs
> @@ -0,0 +1,527 @@
[ ... ]
> +fn parse_attrs(
> + target: DeriveTarget,
> + attrs: &[Attribute],
> +) -> syn::Result<(Vec<Type>, bool, Option<Ident>)> {
> + let helper = target.get_helper_name();
> +
> + let mut is_repr_c = false;
> + let mut repr_ty = None;
> + let mut helper_tys = Vec::new();
> + for attr in attrs {
> + if attr.path().is_ident("repr") {
> + attr.parse_nested_meta(|meta| {
> + let ident = meta.path.get_ident();
> + if let Some(i) = ident {
> + if is_valid_primitive(i) {
> + repr_ty = ident.cloned();
> + } else if i == "C" {
> + is_repr_c = true;
> + }
> + }
> + // Delegate `repr` attribute validation to rustc.
> + Ok(())
[Severity: Low]
Does this closure returning Ok(()) without consuming the inner tokens cause
parsing to fail?
For example, if a developer applies #[derive(Into)] to an enum with a
#[repr(align(4))] attribute, the inner token 4 is not consumed. Will this
cause the syn crate to return a parsing error and break the build?
> + })?;
> + } else if attr.path().is_ident(helper) {
> + let args = attr.parse_args_with(Punctuated::<Type, Token![,]>::parse_terminated)?;
> + helper_tys.extend(args);
> + }
> + }
> +
> + Ok((helper_tys, is_repr_c, repr_ty))
> +}
[ ... ]
> +fn validate_bounded(path_segment: &PathSegment) -> syn::Result<ValidTy> {
> + let PathSegment { ident, arguments } = path_segment;
> + return match arguments {
> + PathArguments::AngleBracketed(inner) if ident == Bounded::NAME => {
> + let AngleBracketedGenericArguments {
> + colon2_token, args, ..
> + } = inner;
[ ... ]
> + let Some(base_ty_ident) = base_ty_lowered.path.get_ident() else {
> + return Err(make_base_ty_err(base_ty));
> + };
> +
> + if !is_valid_primitive(base_ty_ident) {
[Severity: Low]
Does this validation step incorrectly accept bool as a base type for Bounded?
Because is_valid_primitive() returns true for bool, specifying
Bounded<bool, 1> passes this check.
Later in the code, emit_min() attempts to emit MIN and BITS properties:
rust/macros/convert.rs:emit_min() {
...
::quote::quote! { #base_ty::MIN >> (#base_ty::BITS - #bits) }
...
}
Since bool::MIN and bool::BITS do not exist, will this generate invalid code
that causes the build to fail?
> + return Err(make_base_ty_err(base_ty));
> + }
--
Sashiko AI review · https://sashiko.dev/#/patchset/20260929135855.36775-1-chaoji_xinren@163.com?part=1
next prev parent reply other threads:[~2026-09-29 14:11 UTC|newest]
Thread overview: 15+ messages / expand[flat|nested] mbox.gz Atom feed top
2026-09-29 13:58 [PATCH v6 00/10] rust: add conversion derives and exhaustive From support Kaiqi Guo
2026-09-29 13:58 ` [PATCH v6 01/10] rust: macros: add derive macro for `Into` Kaiqi Guo
2026-09-29 14:10 ` sashiko-bot [this message]
2026-09-29 13:58 ` [PATCH v6 02/10] rust: macros: add derive macro for `TryFrom` Kaiqi Guo
2026-09-29 14:10 ` sashiko-bot
2026-09-29 13:58 ` [PATCH v6 03/10] rust: macros: add `convert` helper attribute Kaiqi Guo
2026-09-29 13:58 ` [PATCH v6 04/10] rust: macros: add private doctests for `Into` derive macro Kaiqi Guo
2026-09-29 13:58 ` [PATCH v6 05/10] rust: macros: add private doctests for `TryFrom` " Kaiqi Guo
2026-09-29 13:58 ` [PATCH v6 06/10] rust: macros: exercise the intended conversion doctest failures Kaiqi Guo
2026-09-29 18:47 ` Miguel Ojeda
2026-09-29 13:58 ` [PATCH v6 07/10] rust: macros: derive From for exhaustive enum conversions Kaiqi Guo
2026-09-29 13:58 ` [PATCH v6 08/10] rust: macros: test exhaustive conversion derives Kaiqi Guo
2026-09-29 14:07 ` sashiko-bot
2026-09-29 13:58 ` [PATCH v6 09/10] rust: macros: validate and qualify conversion helper types Kaiqi Guo
2026-09-29 13:58 ` [PATCH v6 10/10] gpu: nova-core: use conversion derives for two register enums Kaiqi Guo
Reply instructions:
You may reply publicly to this message via plain-text email
using any one of the following methods:
* Save the following mbox file, import it into your mail client,
and reply-to-all from there: mbox
Avoid top-posting and favor interleaved quoting:
https://en.wikipedia.org/wiki/Posting_style#Interleaved_style
* Reply using the --to, --cc, and --in-reply-to
switches of git-send-email(1):
git send-email \
--in-reply-to=20260929141100.1452F1F000FF@smtp.kernel.org \
--to=sashiko-bot@kernel.org \
--cc=acourbot@nvidia.com \
--cc=chaoji_xinren@163.com \
--cc=dakr@kernel.org \
--cc=dri-devel@lists.freedesktop.org \
--cc=gary@garyguo.net \
--cc=ojeda@kernel.org \
--cc=sashiko-reviews@lists.linux.dev \
/path/to/YOUR_REPLY
https://kernel.org/pub/software/scm/git/docs/git-send-email.html
* If your mail client supports setting the In-Reply-To header
via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line
before the message body.
This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.