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: <20260108124137.22c32313.gary@garyguo.net>
Date: Thu, 8 Jan 2026 12:41:37 +0000
From: Gary Guo <gary@...yguo.net>
To: Tamir Duberstein <tamird@...il.com>
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 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.

> 
> > +    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);
> > +    }