public inbox for rust-for-linux@vger.kernel.org
 help / color / mirror / Atom feed
From: Gary Guo <gary@garyguo.net>
To: Tamir Duberstein <tamird@gmail.com>
Cc: "Miguel Ojeda" <ojeda@kernel.org>,
	"Boqun Feng" <boqun.feng@gmail.com>,
	"Björn Roy Baron" <bjorn3_gh@protonmail.com>,
	"Benno Lossin" <lossin@kernel.org>,
	"Andreas Hindborg" <a.hindborg@kernel.org>,
	"Alice Ryhl" <aliceryhl@google.com>,
	"Trevor Gross" <tmgross@umich.edu>,
	"Danilo Krummrich" <dakr@kernel.org>,
	"Igor Korotin" <igor.korotin.linux@gmail.com>,
	"José Expósito" <jose.exposito89@gmail.com>,
	rust-for-linux@vger.kernel.org, linux-kernel@vger.kernel.org
Subject: Re: [PATCH v2 03/11] rust: macros: convert `#[vtable]` macro to use `syn`
Date: Thu, 8 Jan 2026 12:41:37 +0000	[thread overview]
Message-ID: <20260108124137.22c32313.gary@garyguo.net> (raw)
In-Reply-To: <CAJ-ks9nbPTQojM-QcHPKv+NdUHwaRbBRVqzJ3CwgQTJTuD=bRQ@mail.gmail.com>

On Wed, 7 Jan 2026 11:48:28 -0500
Tamir Duberstein <tamird@gmail.com> wrote:

> On Wed, Jan 7, 2026 at 11:30 AM Gary Guo <gary@kernel.org> wrote:
> >
> > From: Gary Guo <gary@garyguo.net>
> >
> > `#[vtable]` is converted to use syn. This is more robust than the
> > previous heuristic-based searching of defined methods and functions.
> >
> > When doing so, the trait and impl are split into two code paths as the
> > types are distinct when parsed by `syn`.
> >
> > Signed-off-by: Gary Guo <gary@garyguo.net>  
> 
> Reviewed-by: Tamir Duberstein <tamird@gmail.com>
> 
> > ---
> >  rust/macros/lib.rs    |   9 ++-
> >  rust/macros/vtable.rs | 163 ++++++++++++++++++++++--------------------
> >  2 files changed, 93 insertions(+), 79 deletions(-)
> >
> > diff --git a/rust/macros/vtable.rs b/rust/macros/vtable.rs
> > index a67d1cc81a2d3..a39bedb703973 100644
> > --- a/rust/macros/vtable.rs
> > +++ b/rust/macros/vtable.rs
> > @@ -1,97 +1,106 @@
> >  // SPDX-License-Identifier: GPL-2.0
> >
> >  use std::collections::HashSet;
> > -use std::fmt::Write;
> >
> > -use proc_macro2::{Delimiter, Group, TokenStream, TokenTree};
> > +use proc_macro2::{
> > +    Ident,
> > +    TokenStream, //
> > +};
> > +use quote::ToTokens;
> > +use syn::{
> > +    parse_quote,
> > +    Error,
> > +    ImplItem,
> > +    Item,
> > +    ItemImpl,
> > +    ItemTrait,
> > +    Result,
> > +    TraitItem, //
> > +};
> >
> > -pub(crate) fn vtable(_attr: TokenStream, ts: TokenStream) -> TokenStream {
> > -    let mut tokens: Vec<_> = ts.into_iter().collect();
> > +fn handle_trait(mut item: ItemTrait) -> Result<ItemTrait> {
> > +    let mut functions = Vec::new();  
> 
> Would be easier to propagate `#[cfg]`(as in your FIXME below) if you
> collect the generated constants into this vector instead -- then you
> can do all the code generation in the `for item in &item.items` loop
> where you have all the attributes.

This is a very good suggestion. I tried it out and the code is much
cleaner. After making the change the `#[cfg]` change is just a few lines
too -- I'll include the cfg change in a new commit in the next version.

> 
> > +    for item in &item.items {
> > +        if let TraitItem::Fn(fn_item) = item {
> > +            functions.push(fn_item.sig.ident.clone());
> > +        }
> > +    }
> > +
> > +    item.items.push(parse_quote! {
> > +         /// A marker to prevent implementors from forgetting to use [`#[vtable]`](vtable)
> > +         /// attribute when implementing this trait.
> > +         const USE_VTABLE_ATTR: ();
> > +    });
> >
> > -    // Scan for the `trait` or `impl` keyword.
> > -    let is_trait = tokens
> > -        .iter()
> > -        .find_map(|token| match token {
> > -            TokenTree::Ident(ident) => match ident.to_string().as_str() {
> > -                "trait" => Some(true),
> > -                "impl" => Some(false),
> > -                _ => None,
> > -            },
> > -            _ => None,
> > -        })
> > -        .expect("#[vtable] attribute should only be applied to trait or impl block");
> > +    let mut consts = HashSet::new();
> > +    for name in functions {
> > +        let gen_const_name = Ident::new(
> > +            &format!("HAS_{}", name.to_string().to_uppercase()),
> > +            name.span(),
> > +        );
> > +        // Skip if it's declared already -- this can happen if `#[cfg]` is used to selectively
> > +        // define functions.
> > +        // FIXME: `#[cfg]` should be copied and propagated to the generated consts.
> > +        if consts.contains(&gen_const_name) {
> > +            continue;
> > +        }
> > +        // We don't know on the implementation-site whether a method is required or provided
> > +        // so we have to generate a const for all methods.
> > +        let comment = format!("Indicates if the `{name}` method is overridden by the implementor.");  
> 
> We're already quasi-quoting below, does putting the comment there work?

The comment has `{name}` interpolation and you cannot just use `quote!`
for it.

You could do

	#[doc = concat!("...", stringify!(...), "...")]

But I think it's cleaner to just use `format!`.

> 
> > +        item.items.push(parse_quote! {
> > +            #[doc = #comment]
> > +            const #gen_const_name: bool = false;
> > +        });
> > +        consts.insert(gen_const_name);
> > +    }

  reply	other threads:[~2026-01-08 12:41 UTC|newest]

Thread overview: 23+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2026-01-07 16:15 [PATCH v2 00/11] refactor Rust proc macros with `syn` Gary Guo
2026-01-07 16:15 ` [PATCH v2 01/11] rust: pin-init: internal: remove proc-macro[2] and quote workarounds Gary Guo
2026-01-07 16:40   ` Tamir Duberstein
2026-01-07 16:15 ` [PATCH v2 02/11] rust: macros: use `quote!` from vendored crate Gary Guo
2026-01-07 16:15 ` [PATCH v2 03/11] rust: macros: convert `#[vtable]` macro to use `syn` Gary Guo
2026-01-07 16:48   ` Tamir Duberstein
2026-01-08 12:41     ` Gary Guo [this message]
2026-01-08 15:10       ` Tamir Duberstein
2026-01-08 15:24         ` Gary Guo
2026-01-11 17:03   ` Benno Lossin
2026-01-11 21:25     ` Gary Guo
2026-01-07 16:15 ` [PATCH v2 04/11] rust: macros: use `syn` to parse `module!` macro Gary Guo
2026-01-07 17:11   ` Tamir Duberstein
2026-01-07 16:15 ` [PATCH v2 05/11] rust: macros: use `quote!` for " Gary Guo
2026-01-07 17:19   ` Tamir Duberstein
2026-01-07 17:54     ` Gary Guo
2026-01-07 16:15 ` [PATCH v2 06/11] rust: macros: convert `#[export]` to use `syn` Gary Guo
2026-01-07 16:15 ` [PATCH v2 07/11] rust: macros: convert `concat_idents!` " Gary Guo
2026-01-07 16:15 ` [PATCH v2 08/11] rust: macros: convert `#[kunit_tests]` macro " Gary Guo
2026-01-07 17:22   ` Tamir Duberstein
2026-01-07 16:15 ` [PATCH v2 09/11] rust: macros: allow arbitrary types to be used in `module!` macro Gary Guo
2026-01-07 16:15 ` [PATCH v2 10/11] rust: macros: rearrange `#[doc(hidden)]` " Gary Guo
2026-01-07 16:15 ` [PATCH v2 11/11] rust: kunit: use `pin_init::zeroed` instead of custom null value 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=20260108124137.22c32313.gary@garyguo.net \
    --to=gary@garyguo.net \
    --cc=a.hindborg@kernel.org \
    --cc=aliceryhl@google.com \
    --cc=bjorn3_gh@protonmail.com \
    --cc=boqun.feng@gmail.com \
    --cc=dakr@kernel.org \
    --cc=igor.korotin.linux@gmail.com \
    --cc=jose.exposito89@gmail.com \
    --cc=linux-kernel@vger.kernel.org \
    --cc=lossin@kernel.org \
    --cc=ojeda@kernel.org \
    --cc=rust-for-linux@vger.kernel.org \
    --cc=tamird@gmail.com \
    --cc=tmgross@umich.edu \
    /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