linux-kernel - Re: [PATCH v2 03/11] rust: macros: convert `#[vtable]` macro to use `syn`

lists.openwall.net		lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC
Open Source and information security mailing list archives
Hash Suite: Windows password security audit tool. GUI, reports in PDF.
[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]
Message-ID: <CAJ-ks9=N+1YU6jBn6KcGWXHhggimuWRKDb9rM=0TwtST6s8dUA@mail.gmail.com>
Date: Thu, 8 Jan 2026 10:10:23 -0500
From: Tamir Duberstein <tamird@...il.com>
To: Gary Guo <gary@...yguo.net>
Cc: Miguel Ojeda <ojeda@...nel.org>, Boqun Feng <boqun.feng@...il.com>, 
	Björn Roy Baron <bjorn3_gh@...tonmail.com>, 
	Benno Lossin <lossin@...nel.org>, Andreas Hindborg <a.hindborg@...nel.org>, 
	Alice Ryhl <aliceryhl@...gle.com>, Trevor Gross <tmgross@...ch.edu>, 
	Danilo Krummrich <dakr@...nel.org>, Igor Korotin <igor.korotin.linux@...il.com>, 
	José Expósito <jose.exposito89@...il.com>, 
	rust-for-linux@...r.kernel.org, linux-kernel@...r.kernel.org
Subject: Re: [PATCH v2 03/11] rust: macros: convert `#[vtable]` macro to use `syn`

On Thu, Jan 8, 2026 at 7:41 AM Gary Guo <gary@...yguo.net> wrote:
>
> On Wed, 7 Jan 2026 11:48:28 -0500
> Tamir Duberstein <tamird@...il.com> wrote:
>
> > On Wed, Jan 7, 2026 at 11:30 AM Gary Guo <gary@...nel.org> wrote:
> > >
> > > From: Gary Guo <gary@...yguo.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@...yguo.net>
> >
> > Reviewed-by: Tamir Duberstein <tamird@...il.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.

Great.

> >
> > > +    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!`.

Ack, though I think it would be:

#[doc = concat!("Indicates if the `", #name, "` method is overridden
by the implementor.")]

i.e. no need for `stringify`.

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