public inbox for rust-for-linux@vger.kernel.org
 help / color / mirror / Atom feed
From: "Gary Guo" <gary@garyguo.net>
To: "Benno Lossin" <lossin@kernel.org>, "Gary Guo" <gary@garyguo.net>,
	"Miguel Ojeda" <ojeda@kernel.org>,
	"Boqun Feng" <boqun.feng@gmail.com>,
	"Björn Roy Baron" <bjorn3_gh@protonmail.com>,
	"Andreas Hindborg" <a.hindborg@kernel.org>,
	"Alice Ryhl" <aliceryhl@google.com>,
	"Trevor Gross" <tmgross@umich.edu>,
	"Danilo Krummrich" <dakr@kernel.org>,
	"Fiona Behrens" <me@kloenk.dev>,
	"Tamir Duberstein" <tamird@gmail.com>,
	"Alban Kurti" <kurti@invicto.ai>
Cc: <rust-for-linux@vger.kernel.org>, <linux-kernel@vger.kernel.org>
Subject: Re: [PATCH 05/12] rust: pin-init: rewrite the `#[pinned_drop]` attribute macro using `syn`
Date: Fri, 09 Jan 2026 12:12:01 +0000	[thread overview]
Message-ID: <DFK1RR7HGFKN.XM2V8C9D0PR1@garyguo.net> (raw)
In-Reply-To: <20260108135127.3153925-6-lossin@kernel.org>

On Thu Jan 8, 2026 at 1:50 PM GMT, Benno Lossin wrote:
> Rewrite the attribute macro for implementing `PinnedDrop` using `syn`.
> Otherwise no functional changes intended aside from improved error
> messages on syntactic and semantical errors. For example:
>
> When missing the `drop` function in the implementation, the old error
> was:
>
>     error: no rules expected `)`
>      --> tests/ui/compile-fail/pinned_drop/no_fn.rs:6:1
>       |
>     6 | #[pinned_drop]
>       | ^^^^^^^^^^^^^^ no rules expected this token in macro call
>       |
>     note: while trying to match keyword `fn`
>      --> src/macros.rs
>       |
>       |             fn drop($($sig:tt)*) {
>       |             ^^
>       = note: this error originates in the attribute macro `pinned_drop` (in Nightly builds, run with -Z macro-backtrace for more info)
>
> And the new one is:
>
>     error[E0046]: not all trait items implemented, missing: `drop`
>      --> tests/ui/compile-fail/pinned_drop/no_fn.rs:7:1
>       |
>     7 | impl PinnedDrop for Foo {}
>       | ^^^^^^^^^^^^^^^^^^^^^^^ missing `drop` in implementation
>       |
>       = help: implement the missing item: `fn drop(self: Pin<&mut Self>, _: OnlyCallFromDrop) { todo!() }`
>
> Signed-off-by: Benno Lossin <lossin@kernel.org>
> ---
>  rust/pin-init/internal/src/lib.rs         |  6 +-
>  rust/pin-init/internal/src/pinned_drop.rs | 87 ++++++++++++-----------
>  rust/pin-init/src/macros.rs               | 28 --------
>  3 files changed, 52 insertions(+), 69 deletions(-)
>
> diff --git a/rust/pin-init/internal/src/lib.rs b/rust/pin-init/internal/src/lib.rs
> index ec593362c5ac..d0156b82b5d3 100644
> --- a/rust/pin-init/internal/src/lib.rs
> +++ b/rust/pin-init/internal/src/lib.rs
> @@ -25,7 +25,11 @@ pub fn pin_data(inner: TokenStream, item: TokenStream) -> TokenStream {
>  
>  #[proc_macro_attribute]
>  pub fn pinned_drop(args: TokenStream, input: TokenStream) -> TokenStream {
> -    pinned_drop::pinned_drop(args.into(), input.into()).into()
> +    pinned_drop::pinned_drop(
> +        parse_macro_input!(args as _),
> +        parse_macro_input!(input as _),
> +    )

`as _` can be removed.

> +    .into()
>  }
>  
>  #[proc_macro_derive(Zeroable)]
> diff --git a/rust/pin-init/internal/src/pinned_drop.rs b/rust/pin-init/internal/src/pinned_drop.rs
> index cf8cd1c42984..4df2cb9959fb 100644
> --- a/rust/pin-init/internal/src/pinned_drop.rs
> +++ b/rust/pin-init/internal/src/pinned_drop.rs
> @@ -1,49 +1,56 @@
>  // SPDX-License-Identifier: Apache-2.0 OR MIT
>  
> -use proc_macro2::{TokenStream, TokenTree};
> -use quote::quote;
> +use proc_macro2::TokenStream;
> +use quote::{quote, quote_spanned};
> +use syn::{parse::Nothing, parse_quote, spanned::Spanned, ImplItem, ItemImpl, Token};
>  
> -pub(crate) fn pinned_drop(_args: TokenStream, input: TokenStream) -> TokenStream {
> -    let mut toks = input.into_iter().collect::<Vec<_>>();
> -    assert!(!toks.is_empty());
> -    // Ensure that we have an `impl` item.
> -    assert!(matches!(&toks[0], TokenTree::Ident(i) if i == "impl"));
> -    // Ensure that we are implementing `PinnedDrop`.
> -    let mut nesting: usize = 0;
> -    let mut pinned_drop_idx = None;
> -    for (i, tt) in toks.iter().enumerate() {
> -        match tt {
> -            TokenTree::Punct(p) if p.as_char() == '<' => {
> -                nesting += 1;
> +pub(crate) fn pinned_drop(_args: Nothing, mut input: ItemImpl) -> TokenStream {
> +    let mut errors = vec![];

Any reason to not make this `Vec<Error>` and use `syn::Error::combine`?

> +    if let Some(unsafety) = input.unsafety {
> +        errors.push(quote_spanned! {unsafety.span=>
> +            ::core::compile_error!("implementing `PinnedDrop` is safe");
> +        });
> +    }
> +    input.unsafety = Some(Token![unsafe](input.impl_token.span));
> +    match &mut input.trait_ {
> +        Some((not, path, _for)) => {
> +            if let Some(not) = not {
> +                errors.push(quote_spanned! {not.span=>
> +                    ::core::compile_error!("cannot implement `!PinnedDrop`");
> +                });
>              }
> -            TokenTree::Punct(p) if p.as_char() == '>' => {
> -                nesting = nesting.checked_sub(1).unwrap();
> -                continue;
> +            for (seg, expected) in path
> +                .segments
> +                .iter()
> +                .rev()
> +                .zip(["PinnedDrop", "pin_init", ""])
> +            {
> +                if expected.is_empty() || seg.ident != expected {
> +                    errors.push(quote_spanned! {seg.span()=>
> +                        ::core::compile_error!("bad import path for `PinnedDrop`");
> +                    });
> +                }
> +                if !seg.arguments.is_none() {
> +                    errors.push(quote_spanned! {seg.arguments.span()=>
> +                        ::core::compile_error!("unexpected arguments for `PinnedDrop` path");
> +                    });
> +                }
>              }
> -            _ => {}
> -        }
> -        if i >= 1 && nesting == 0 {
> -            // Found the end of the generics, this should be `PinnedDrop`.
> -            assert!(
> -                matches!(tt, TokenTree::Ident(i) if i == "PinnedDrop"),
> -                "expected 'PinnedDrop', found: '{tt:?}'"
> -            );
> -            pinned_drop_idx = Some(i);
> -            break;
> +            *path = parse_quote!(::pin_init::PinnedDrop);
>          }
> +        None => errors.push(quote_spanned! {input.impl_token.span=>

nit: this can be `input.impl_token.span.join(input.self_ty.span())`.

Best,
Gary

> +            ::core::compile_error!("expected `impl ... PinnedDrop for ...`, got inherent impl");
> +        }),
>      }
> -    let idx = pinned_drop_idx
> -        .unwrap_or_else(|| panic!("Expected an `impl` block implementing `PinnedDrop`."));
> -    // Fully qualify the `PinnedDrop`, as to avoid any tampering.
> -    toks.splice(idx..idx, quote!(::pin_init::));
> -    // Take the `{}` body and call the declarative macro.
> -    if let Some(TokenTree::Group(last)) = toks.pop() {
> -        let last = last.stream();
> -        quote!(::pin_init::__pinned_drop! {
> -            @impl_sig(#(#toks)*),
> -            @impl_body(#last),
> -        })
> -    } else {
> -        TokenStream::from_iter(toks)
> +    for item in &mut input.items {
> +        if let ImplItem::Fn(fn_item) = item {
> +            if fn_item.sig.ident == "drop" {
> +                fn_item
> +                    .sig
> +                    .inputs
> +                    .push(parse_quote!(_: ::pin_init::__internal::OnlyCallFromDrop));
> +            }
> +        }
>      }
> +    quote!(#(#errors)* #input)
>  }


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

Thread overview: 40+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2026-01-08 13:50 [PATCH 00/13] `syn` rewrite of pin-init Benno Lossin
2026-01-08 13:50 ` [PATCH 01/12] rust: pin-init: remove `try_` versions of the initializer macros Benno Lossin
2026-01-08 13:50 ` [PATCH 02/12] rust: pin-init: allow the crate to refer to itself as `pin-init` in doc tests Benno Lossin
2026-01-08 13:50 ` [PATCH 03/12] rust: pin-init: add `syn` dependency and remove `proc-macro[2]` and `quote` workarounds Benno Lossin
2026-01-08 13:50 ` [PATCH 04/12] rust: pin-init: rewrite `derive(Zeroable)` and `derive(MaybeZeroable)` using `syn` Benno Lossin
2026-01-09 12:02   ` Gary Guo
2026-01-08 13:50 ` [PATCH 05/12] rust: pin-init: rewrite the `#[pinned_drop]` attribute macro " Benno Lossin
2026-01-09 12:12   ` Gary Guo [this message]
2026-01-09 15:34     ` Benno Lossin
2026-01-09 16:42       ` Gary Guo
2026-01-08 13:50 ` [PATCH 06/12] rust: pin-init: rewrite `#[pin_data]` " Benno Lossin
2026-01-09  7:45   ` kernel test robot
2026-01-09 12:47   ` Gary Guo
2026-01-09 16:39     ` Benno Lossin
2026-01-09 16:46       ` Gary Guo
2026-01-10 16:41         ` Benno Lossin
2026-01-10 19:18           ` Gary Guo
2026-01-08 13:50 ` [PATCH 07/12] rust: pin-init: add `?Sized` bounds to traits in `#[pin_data]` macro Benno Lossin
2026-01-08 13:50 ` [PATCH 08/12] rust: pin-init: rewrite the initializer macros using `syn` Benno Lossin
2026-01-09  8:44   ` kernel test robot
2026-01-09 13:45   ` Gary Guo
2026-01-09 17:24     ` Benno Lossin
2026-01-10 16:21       ` Benno Lossin
2026-01-10 18:14     ` Benno Lossin
2026-01-10 19:20       ` Gary Guo
2026-01-10 23:18         ` Benno Lossin
2026-01-11  1:10           ` Gary Guo
2026-01-11 10:04             ` Benno Lossin
2026-01-08 13:50 ` [PATCH 09/12] rust: pin-init: add `#[default_error(<type>)]` attribute to initializer macros Benno Lossin
2026-01-09 13:52   ` Gary Guo
2026-01-08 13:50 ` [PATCH 10/12] rust: init: use `#[default_error(err)]` for the " Benno Lossin
2026-01-08 13:50 ` [PATCH 11/12] rust: pin-init: internal: init: add support for attributes on initializer fields Benno Lossin
2026-01-09 13:55   ` Gary Guo
2026-01-09 18:02     ` Benno Lossin
2026-01-09 21:16       ` Gary Guo
2026-01-08 13:50 ` [PATCH 12/12] rust: pin-init: internal: init: add escape hatch for referencing initialized fields Benno Lossin
2026-01-09  9:44   ` kernel test robot
2026-01-09 13:58   ` Gary Guo
2026-01-09 18:04     ` Benno Lossin
2026-01-08 13:50 ` [PATCH 13/13] MAINTAINERS: add Gary Guo to pin-init Benno Lossin

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=DFK1RR7HGFKN.XM2V8C9D0PR1@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=kurti@invicto.ai \
    --cc=linux-kernel@vger.kernel.org \
    --cc=lossin@kernel.org \
    --cc=me@kloenk.dev \
    --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