Rust for Linux List
 help / color / mirror / Atom feed
From: "Gary Guo" <gary@garyguo.net>
To: "Alexandre Courbot" <acourbot@nvidia.com>, "Gary Guo" <gary@garyguo.net>
Cc: "Danilo Krummrich" <dakr@kernel.org>,
	"Alice Ryhl" <aliceryhl@google.com>,
	"Daniel Almeida" <daniel.almeida@collabora.com>,
	"Miguel Ojeda" <ojeda@kernel.org>,
	"Boqun Feng" <boqun@kernel.org>,
	"Björn Roy Baron" <bjorn3_gh@protonmail.com>,
	"Benno Lossin" <lossin@kernel.org>,
	"Andreas Hindborg" <a.hindborg@kernel.org>,
	"Trevor Gross" <tmgross@umich.edu>,
	"Tamir Duberstein" <tamird@kernel.org>,
	"Onur Özkan" <work@onurozkan.dev>,
	"David Airlie" <airlied@gmail.com>,
	"Simona Vetter" <simona@ffwll.ch>,
	"Bjorn Helgaas" <bhelgaas@google.com>,
	"Krzysztof Wilczyński" <kwilczynski@kernel.org>,
	driver-core@lists.linux.dev, rust-for-linux@vger.kernel.org,
	linux-kernel@vger.kernel.org, nova-gpu@lists.linux.dev,
	dri-devel@lists.freedesktop.org, linux-pci@vger.kernel.org
Subject: Re: [PATCH v3 01/16] rust: io: register: reimplement as proc macro
Date: Thu, 27 Aug 2026 15:23:19 +0100	[thread overview]
Message-ID: <DKZSLL3OOPXR.S5FE63HNESUQ@garyguo.net> (raw)
In-Reply-To: <DKZI8IV8X6Q0.27QMUO34GOFQ2@nvidia.com>

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 <gary@garyguo.net>
>
> 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

  reply	other threads:[~2026-08-27 14:23 UTC|newest]

Thread overview: 41+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2026-08-19 11:09 [PATCH v3 00/16] rust: io: support register projections and remove relative registers Gary Guo
2026-08-19 11:09 ` [PATCH v3 01/16] rust: io: register: reimplement as proc macro Gary Guo
2026-08-27  6:16   ` Alexandre Courbot
2026-08-27 14:23     ` Gary Guo [this message]
2026-08-19 11:09 ` [PATCH v3 02/16] rust: mem: add `transmute` with deferred size check Gary Guo
2026-08-27  6:24   ` Alexandre Courbot
2026-08-27 14:26     ` Gary Guo
2026-08-19 11:09 ` [PATCH v3 03/16] rust: mem: add `AsRepr` and `AsReprMut` Gary Guo
2026-08-26 11:25   ` Alexandre Courbot
2026-08-26 11:50     ` Gary Guo
2026-08-26 13:30       ` Alexandre Courbot
2026-08-19 11:09 ` [PATCH v3 04/16] rust: io: perform conversions using `AsRepr` Gary Guo
2026-08-27  6:53   ` Alexandre Courbot
2026-08-27 14:21     ` Gary Guo
2026-08-27 14:39       ` Alexandre Courbot
2026-08-19 11:09 ` [PATCH v3 05/16] rust: io: support register projections Gary Guo
2026-08-27  7:01   ` Alexandre Courbot
2026-08-19 11:09 ` [PATCH v3 06/16] rust: io: register: allow explicit base type specification Gary Guo
2026-08-27  7:17   ` Alexandre Courbot
2026-08-19 11:09 ` [PATCH v3 07/16] gpu: nova-core: specify base type for registers Gary Guo
2026-08-27 11:20   ` Alexandre Courbot
2026-08-19 11:09 ` [PATCH v3 08/16] drm/tyr: " Gary Guo
2026-08-19 11:09 ` [PATCH v3 09/16] samples: rust: pci: " Gary Guo
2026-08-27 11:29   ` Alexandre Courbot
2026-08-19 11:09 ` [PATCH v3 10/16] rust: io: register: make register have a typed base Gary Guo
2026-08-27 13:42   ` Alexandre Courbot
2026-08-27 14:32     ` Gary Guo
2026-08-28  0:01       ` Alexandre Courbot
2026-08-19 11:09 ` [PATCH v3 11/16] rust: io: register: support fixed offset register without bitfield Gary Guo
2026-08-28  0:38   ` Alexandre Courbot
2026-08-19 11:09 ` [PATCH v3 12/16] gpu: nova-core: use projection for PFALCON and PFALCON2 registers Gary Guo
2026-08-28  1:04   ` Alexandre Courbot
2026-08-28  1:24     ` Alexandre Courbot
2026-08-19 11:09 ` [PATCH v3 13/16] gpu: nova-core: convert hshub0 from relative register to projection Gary Guo
2026-08-28  1:29   ` Alexandre Courbot
2026-08-19 11:09 ` [PATCH v3 14/16] rust: io: register: remove relative registers Gary Guo
2026-08-28  1:33   ` Alexandre Courbot
2026-08-19 11:09 ` [PATCH v3 15/16] rust: io: register: remove `Register` trait and cleanup macro Gary Guo
2026-08-28  2:07   ` Alexandre Courbot
2026-08-19 11:09 ` [PATCH v3 16/16] rust: io: register: unify handling of register with/without bitfields Gary Guo
2026-08-28  2:07   ` Alexandre Courbot

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=DKZSLL3OOPXR.S5FE63HNESUQ@garyguo.net \
    --to=gary@garyguo.net \
    --cc=a.hindborg@kernel.org \
    --cc=acourbot@nvidia.com \
    --cc=airlied@gmail.com \
    --cc=aliceryhl@google.com \
    --cc=bhelgaas@google.com \
    --cc=bjorn3_gh@protonmail.com \
    --cc=boqun@kernel.org \
    --cc=dakr@kernel.org \
    --cc=daniel.almeida@collabora.com \
    --cc=dri-devel@lists.freedesktop.org \
    --cc=driver-core@lists.linux.dev \
    --cc=kwilczynski@kernel.org \
    --cc=linux-kernel@vger.kernel.org \
    --cc=linux-pci@vger.kernel.org \
    --cc=lossin@kernel.org \
    --cc=nova-gpu@lists.linux.dev \
    --cc=ojeda@kernel.org \
    --cc=rust-for-linux@vger.kernel.org \
    --cc=simona@ffwll.ch \
    --cc=tamird@kernel.org \
    --cc=tmgross@umich.edu \
    --cc=work@onurozkan.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 a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox