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?