Re: [PATCH v3 01/16] rust: io: register: reimplement as proc macro

"Alexandre Courbot" <[email protected]>
Newsgroups dev.linux.lists.nova-gpu,dev.linux.lists.driver-core,org.freedesktop.lists.dri-devel,org.kernel.vger.linux-kernel,org.kernel.vger.linux-pci,org.kernel.vger.rust-for-linux
Message-ID <[email protected]>
On Wed Aug 19, 2026 at 8:09 PM JST, Gary Guo wrote:
> The existing `register!` macro is implemented as a declarative macro.
> Reimplement it as proc macro instead, with no functional changes intended.
>
> The old implementation produces unhelpful diagnostics when things go wrong.
> For example, for code like
>
>     register! {
>         pub(crate) TESTREG(u32) {
>             31:0    data;
>         }
>     }
>
> which misses out the "@ offset" part of the specification, and the
> following error is produced:
>
> error: no rules expected `{`
>    --> test.rs:42:5
>     |
>  42 | /     register! {
>  43 | |         pub(crate) TESTREG(u32) {
>  44 | |             31:0    data;
> ...   |
> 100 | |     }
>     | |_____^ no rules expected this token in macro call
>
> which isn't very helpful. With the proc macro implementation, the following
> error is produced:
>
> error: expected `@` or `=>`
>   --> tests.rs:43:33
>    |
> 43 |         pub(crate) TESTREG(u32) {
>    |                                 ^
>
> which is much more helpful. Apart from diagnostics, proc macro also has a
> benefit of not having follow-set restrictions, which makes syntax like
>
>     register!(name: ty @ offset);
>
> possible; declarative macro will reject this as `@` is not in the
> follow-set of "ty" metavariable kind.
>
> Signed-off-by: Gary Guo <[email protected]>

Well that patch was a great way for me to finally learn how to write
proc macros! :)

The parser and generator are both simple enough, but the new macro
module would read a bit better with some light high-level doc (i.e.
module-level and most important types). But even for a beginner like me
this is almost self-explanatory, so no need to go too deep.

<...>
> diff --git a/rust/macros/io/mod.rs b/rust/macros/io/mod.rs
> new file mode 100644
> index 000000000000..39fa5bc302ba
> --- /dev/null
> +++ b/rust/macros/io/mod.rs
> @@ -0,0 +1,3 @@
> +// SPDX-License-Identifier: Apache-2.0 OR MIT

Other files use GPL-2.0 as license, is this on purpose?

<...>
> +pub(crate) fn register(def: RegDef) -> Result<TokenStream> {
> +    let mut outputs = TokenStream::new();
> +
> +    for reg in def.regs {
> +        let Reg {
> +            attrs,
> +            vis,
> +            name,
> +            storage,
> +            array,
> +            relative_base,
> +            offset,
> +            bitfield_args,
> +        } = reg;
> +
> +        // Use register name's span for generated code, so error messages (if any) can point to it
> +        // instead of the entire register allocation.
> +        let span = name.span().resolved_at(Span::mixed_site());
> +
> +        let offset = match offset {
> +            RegOffset::Fixed { offset } => quote!(#offset),
> +            RegOffset::Alias { alias } => {
> +                quote_spanned!(alias.span().resolved_at(span) =>
> +                    <#alias as ::kernel::io::register::Register>::OFFSET
> +                )
> +            }
> +            RegOffset::ElementAlias { alias, idx } => {
> +                outputs.extend(quote_spanned!(idx.span().resolved_at(span) =>
> +                    ::kernel::build_assert::static_assert!(
> +                        #idx < <#alias as ::kernel::io::register::RegisterArray>::SIZE
> +                    );
> +                ));
> +                quote_spanned!(alias.span().resolved_at(span) =>
> +                    <#alias as ::kernel::io::register::Register>::OFFSET
> +                        + #idx * <#alias as ::kernel::io::register::RegisterArray>::STRIDE

Sashiko raised this but `#idx` should be within parentheses (this code
is replaced later down the series but let's make it correct
nonetheless).

<...>
> diff --git a/rust/macros/lib.rs b/rust/macros/lib.rs
> index 24f96feaeb34..5807dee84747 100644
> --- a/rust/macros/lib.rs
> +++ b/rust/macros/lib.rs
> @@ -19,6 +19,7 @@
>  mod fmt;
>  mod for_lt;
>  mod helpers;
> +mod io;
>  mod kunit;
>  mod module;
>  mod paste;
> @@ -481,6 +482,15 @@ pub fn paste(input: TokenStream) -> TokenStream {
>          .into()
>  }
>  
> +#[doc(hidden)] // Documented in `kernel` crate.
> +#[proc_macro]
> +#[allow(non_snake_case)]

Is `non_snake_case` needed here?
lmpx.com only provides a reader for public news (NNTP) servers. It is not affiliated with the servers or forums shown here and is not responsible for the content of articles, which is written by their respective authors.