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

"Gary Guo" <[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 Thu Aug 27, 2026 at 7:16 AM BST, Alexandre Courbot wrote:
> 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).

Ack

>
> <...>
>> 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?

You probably already guessed it but both this and the Apache license is from
copy-pasta :) Will fix in next version.

Best,
Gary
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.