All of lore.kernel.org
 help / color / mirror / Atom feed
From: sashiko-bot@kernel.org
To: "Alexandre Courbot" <acourbot@nvidia.com>
Cc: "Danilo Krummrich" <dakr@kernel.org>,
	dri-devel@lists.freedesktop.org, ojeda@kernel.org,
	"Alexandre Courbot" <acourbot@nvidia.com>
Subject: Re: [PATCH 6/8] rust: io: register: use new relative base syntax in doc and examples
Date: Tue, 21 Jul 2026 11:40:59 +0000	[thread overview]
Message-ID: <20260721114059.F17941F000E9@smtp.kernel.org> (raw)
In-Reply-To: <20260721-registers_fix-v1-6-3711b2317e1f@nvidia.com>

Thank you for your contribution! Sashiko AI review found 1 potential issue(s) to consider:
- [Medium] The `register!` macro fails to compile when using the deprecated `+` syntax for relative register aliases.
--

--- Patch [6]: [PATCH 6/8] rust: io: register: use new relative base syntax in doc and examples ---
commit ad4a7f900a81289c91c14aec0581812ddbb526e4
Author: Alexandre Courbot <acourbot@nvidia.com>

rust: io: register: use new relative base syntax in doc and examples

This commit (along with earlier patches in the series) converts the
documentation and examples to use the new Base: syntax to specify
relative registers bases. It also updates the macro's internal rules
to support this new syntax while keeping temporary support for the
old syntax.

> diff --git a/rust/kernel/io/register.rs b/rust/kernel/io/register.rs
> index 80e638a892d74..eeb7f859f8b9d 100644
> --- a/rust/kernel/io/register.rs
> +++ b/rust/kernel/io/register.rs
> @@ -801,6 +801,26 @@ fn into_io_op(self) -> (FixedRegisterLoc<T>, T) {
>  /// ```
>  #[macro_export]
>  macro_rules! register {
> +    // Deprecated `+` syntax for relative registers and their aliases.
> +    (
> +        $(
> +            $(#[$attr:meta])* $vis:vis $name:ident ($storage:ty)
> +                $([ $size:expr $(, stride = $stride:expr)? ])?
> +                $(@ $($base:ident +)? $offset:literal)?
> +                $(=> $alias:ident $(+ $alias_offset:ty)? $([$alias_idx:expr])? )?
> +            { $($fields:tt)* }
> +        )*
> +    ) => {
> +        $(
> +        $crate::register!(
> +            @reg $(#[$attr])* $vis $name ($storage) $([$size $(, stride = $stride)?])?
> +                $(@ $($base :)? $offset)?
> +                $(=> $alias $(: $alias_offset)? $([$alias_idx])? )?
                  ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^

[Severity: Medium]
Could this cause a regression where the macro fails to compile when using the
deprecated + syntax for relative register aliases?

In the deprecated branch above, $alias_offset is matched as a :ty fragment.
It is then forwarded here to an internal @reg rule:

[ ... ]

>      // Creates an alias register of relative offset register `alias` with its own fields.
>      (
> -        @reg $(#[$attr:meta])* $vis:vis $name:ident ($storage:ty) => $base:ident + $alias:ident
> +        @reg $(#[$attr:meta])* $vis:vis $name:ident ($storage:ty) => $base:path : $alias:path
>              { $($fields:tt)* }
>      ) => {

However, this internal @reg rule expects a :path fragment. Rust macros do
not allow an abstract syntax tree node parsed as a ty fragment to match a
path specifier, which will result in a "no rules expected the token"
compilation error.

Since it was intended that the old form continues to be accepted temporarily,
continuous integration might miss this conditional compilation error if no
dependent driver using the old syntax is built in standard configs. Could
$alias_offset be matched as a :path or :ident instead of :ty in the
deprecated branch to avoid this?

-- 
Sashiko AI review · https://sashiko.dev/#/patchset/20260721-registers_fix-v1-0-3711b2317e1f@nvidia.com?part=6

  reply	other threads:[~2026-07-21 11:41 UTC|newest]

Thread overview: 18+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2026-07-21 11:00 [PATCH 0/8] rust: io: register: allow paths for relative register bases Alexandre Courbot
2026-07-21 11:00 ` [PATCH 1/8] rust: io: register: dispatch fixed array shortcut internally Alexandre Courbot
2026-07-21 14:17   ` Gary Guo
2026-07-21 14:24   ` Gary Guo
2026-07-21 11:00 ` [PATCH 2/8] rust: io: register: remove unused rule arguments Alexandre Courbot
2026-07-21 11:00 ` [PATCH 3/8] rust: io: register: use path fragment for alias destination Alexandre Courbot
2026-07-21 11:07   ` sashiko-bot
2026-07-21 11:00 ` [PATCH 4/8] rust: io: register: use path fragment in relative internal rules Alexandre Courbot
2026-07-21 11:41   ` Alexandre Courbot
2026-07-21 11:00 ` [PATCH 5/8] rust: io: register: allow paths for relative register bases Alexandre Courbot
2026-07-21 11:14   ` sashiko-bot
2026-07-21 11:00 ` [PATCH 6/8] rust: io: register: use new relative base syntax in doc and examples Alexandre Courbot
2026-07-21 11:40   ` sashiko-bot [this message]
2026-07-21 11:00 ` [PATCH 7/8] gpu: nova-core: convert relative registers to new syntax Alexandre Courbot
2026-07-21 11:00 ` [PATCH 8/8] rust: io: register: remove deprecated relative register rule Alexandre Courbot
2026-07-21 14:34 ` [PATCH 0/8] rust: io: register: allow paths for relative register bases Gary Guo
2026-07-21 15:02   ` Alexandre Courbot
2026-07-21 15:06     ` Gary 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=20260721114059.F17941F000E9@smtp.kernel.org \
    --to=sashiko-bot@kernel.org \
    --cc=acourbot@nvidia.com \
    --cc=dakr@kernel.org \
    --cc=dri-devel@lists.freedesktop.org \
    --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.