mirror of https://lore.kernel.org/lkml/
 help / color / mirror / Atom feed
From: Gary Guo <gary@garyguo.net>
To: Benno Lossin <benno.lossin@proton.me>
Cc: "Miguel Ojeda" <ojeda@kernel.org>,
	"Alex Gaynor" <alex.gaynor@gmail.com>,
	"Wedson Almeida Filho" <wedsonaf@gmail.com>,
	"Boqun Feng" <boqun.feng@gmail.com>,
	"Björn Roy Baron" <bjorn3_gh@protonmail.com>,
	"Andreas Hindborg" <a.hindborg@samsung.com>,
	"Alice Ryhl" <aliceryhl@google.com>,
	"Sumera Priyadarsini" <sylphrenadin@gmail.com>,
	"Vincenzo Palazzo" <vincenzopalazzodev@gmail.com>,
	"Asahi Lina" <lina@asahilina.net>,
	"Martin Rodriguez Reboredo" <yakoyoku@gmail.com>,
	rust-for-linux@vger.kernel.org, linux-kernel@vger.kernel.org
Subject: Re: [PATCH v2 1/3] rust: macros: add `decl_generics` to `parse_generics()`
Date: Sat, 23 Dec 2023 14:41:10 +0000	[thread overview]
Message-ID: <20231223144110.346766ed.gary@garyguo.net> (raw)
In-Reply-To: <20231213220447.3613500-1-benno.lossin@proton.me>

The logic looks good to me, some nits:

> +/// Parsed generics.
> +///
> +/// See the field documentation for an explanation what each of the fields represents.
> +///
> +/// # Examples
> +///
> +/// ```rust,ignore
> +/// # let input = todo!();
> +/// let (Generics { decl_generics, impl_generics, ty_generics }, rest) = parse_generics(input);
> +/// quote! {
> +///     struct Foo<$($decl_generics)*> {
> +///         // ...
> +///     }
> +///
> +///     impl<$impl_generics> Foo<$ty_generics> {
> +///         fn foo() {
> +///             // ...
> +///         }
> +///     }
> +/// }
> +/// ```
>  pub(crate) struct Generics {
> +    /// The generics with bounds and default values (e.g. `T: Clone, const N: usize = 0`).
> +    ///
> +    /// Use this on type definitions e.g. `struct Foo<$decl_generics> ...` (or `union`/`enum`).
> +    pub(crate) decl_generics: Vec<TokenTree>,
> +    /// The generics with bounds (e.g. `T: Clone, const N: usize`).
> +    ///
> +    /// Use this on `impl` blocks e.g. `impl<$impl_generics> Trait for ...`.
>      pub(crate) impl_generics: Vec<TokenTree>,
> +    /// The generics without bounds and without default values (e.g. `T, N`).
> +    ///
> +    /// Use this when you use the type that is declared with these generics e.g.
> +    /// `Foo<$ty_generics>`.
>      pub(crate) ty_generics: Vec<TokenTree>,
>  }
>  
> @@ -81,6 +113,8 @@ pub(crate) struct Generics {
>  pub(crate) fn parse_generics(input: TokenStream) -> (Generics, Vec<TokenTree>) {
>      // `impl_generics`, the declared generics with their bounds.
>      let mut impl_generics = vec![];
> +    // The generics with bounds and default values.
> +    let mut decl_generics = vec![];

It'll be great if the order if the same as the declaration in struct
above.

>      // Only the names of the generics, without any bounds.
>      let mut ty_generics = vec![];
>      // Tokens not related to the generics e.g. the `where` token and definition.
> @@ -90,10 +124,17 @@ pub(crate) fn parse_generics(input: TokenStream) -> (Generics, Vec<TokenTree>) {
>      let mut toks = input.into_iter();
>      // If we are at the beginning of a generic parameter.
>      let mut at_start = true;
> -    for tt in &mut toks {
> +    let mut skip_until_comma = false;
> +    while let Some(tt) = toks.next() {
> +        if nesting == 1 && matches!(&tt, TokenTree::Punct(p) if p.as_char() == '>') {
> +            // Found the end of the generics.
> +            break;
> +        } else if nesting >= 1 {
> +            decl_generics.push(tt.clone());
> +        }
>          match tt.clone() {
>              TokenTree::Punct(p) if p.as_char() == '<' => {
> -                if nesting >= 1 {
> +                if nesting >= 1 && !skip_until_comma {
>                      // This is inside of the generics and part of some bound.
>                      impl_generics.push(tt);
>                  }
> @@ -105,49 +146,70 @@ pub(crate) fn parse_generics(input: TokenStream) -> (Generics, Vec<TokenTree>) {
>                      break;
>                  } else {
>                      nesting -= 1;
> -                    if nesting >= 1 {
> +                    if nesting >= 1 && !skip_until_comma {
>                          // We are still inside of the generics and part of some bound.
>                          impl_generics.push(tt);
>                      }
> -                    if nesting == 0 {
> -                        break;
> -                    }
>                  }
>              }
> -            tt => {
> +            TokenTree::Punct(p) if skip_until_comma && p.as_char() == ',' => {
>                  if nesting == 1 {
> -                    // Here depending on the token, it might be a generic variable name.
> -                    match &tt {
> -                        // Ignore const.
> -                        TokenTree::Ident(i) if i.to_string() == "const" => {}
> -                        TokenTree::Ident(_) if at_start => {
> -                            ty_generics.push(tt.clone());
> -                            // We also already push the `,` token, this makes it easier to append
> -                            // generics.
> -                            ty_generics.push(TokenTree::Punct(Punct::new(',', Spacing::Alone)));
> -                            at_start = false;
> -                        }
> -                        TokenTree::Punct(p) if p.as_char() == ',' => at_start = true,
> -                        // Lifetimes begin with `'`.
> -                        TokenTree::Punct(p) if p.as_char() == '\'' && at_start => {
> -                            ty_generics.push(tt.clone());
> -                        }
> -                        _ => {}
> -                    }
> +                    impl_generics.push(TokenTree::Punct(p.clone()));
> +                    ty_generics.push(TokenTree::Punct(p));

This could just be

	impl_generics.push(tt.clone());
	ty_generics.push(tt);

?

> +                    skip_until_comma = false;
>                  }
> -                if nesting >= 1 {
> -                    impl_generics.push(tt);
> -                } else if nesting == 0 {
> +            }
> +            tt if !skip_until_comma => {
> +                match nesting {
>                      // If we haven't entered the generics yet, we still want to keep these tokens.
> -                    rest.push(tt);
> +                    0 => rest.push(tt),
> +                    1 => {
> +                        // Here depending on the token, it might be a generic variable name.
> +                        match tt {
> +                            TokenTree::Ident(i) if at_start && i.to_string() == "const" => {
> +                                let Some(name) = toks.next() else {
> +                                    // Parsing error.
> +                                    break;
> +                                };
> +                                impl_generics.push(TokenTree::Ident(i));

Similar, this could just be push tt.

> +                                impl_generics.push(name.clone());
> +                                ty_generics.push(name.clone());
> +                                decl_generics.push(name);
> +                                at_start = false;
> +                            }
> +                            tt @ TokenTree::Ident(_) if at_start => {
> +                                impl_generics.push(tt.clone());
> +                                ty_generics.push(tt);
> +                                at_start = false;
> +                            }
> +                            TokenTree::Punct(p) if p.as_char() == ',' => {
> +                                impl_generics.push(TokenTree::Punct(p.clone()));
> +                                ty_generics.push(TokenTree::Punct(p));
> +                                at_start = true;
> +                            }

I am not sure why the ident above uses tt, but this spells thing all
out.

> +                            // Lifetimes begin with `'`.
> +                            TokenTree::Punct(p) if p.as_char() == '\'' && at_start => {
> +                                ty_generics.push(TokenTree::Punct(p.clone()));
> +                                impl_generics.push(TokenTree::Punct(p));
> +                            }

ditto

> +                            // Generics can have default values, we skip these.
> +                            TokenTree::Punct(p) if p.as_char() == '=' => {
> +                                skip_until_comma = true;
> +                            }
> +                            tt => impl_generics.push(tt),
> +                        }
> +                    }
> +                    _ => impl_generics.push(tt),
>                  }
>              }
> +            _ => {}
>          }
>      }
>      rest.extend(toks);
>      (
>          Generics {
>              impl_generics,
> +            decl_generics,
>              ty_generics,
>          },
>          rest,
> diff --git a/rust/macros/pin_data.rs b/rust/macros/pin_data.rs
> index 6d58cfda9872..022e68e9720d 100644
> --- a/rust/macros/pin_data.rs
> +++ b/rust/macros/pin_data.rs
> @@ -10,6 +10,7 @@ pub(crate) fn pin_data(args: TokenStream, input: TokenStream) -> TokenStream {
>      let (
>          Generics {
>              impl_generics,
> +            decl_generics: _,
>              ty_generics,
>          },
>          rest,
> diff --git a/rust/macros/zeroable.rs b/rust/macros/zeroable.rs
> index 0d605c46ab3b..cfee2cec18d5 100644
> --- a/rust/macros/zeroable.rs
> +++ b/rust/macros/zeroable.rs
> @@ -7,6 +7,7 @@ pub(crate) fn derive(input: TokenStream) -> TokenStream {
>      let (
>          Generics {
>              impl_generics,
> +            decl_generics: _,
>              ty_generics,
>          },
>          mut rest,
> 
> base-commit: d9857c16cfc6bce7764e1b79956c6a028f97f4d0


  parent reply	other threads:[~2023-12-23 14:41 UTC|newest]

Thread overview: 14+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2023-12-13 22:08 Benno Lossin
2023-12-13 22:08 ` [PATCH v2 2/3] rust: macros: allow generic parameter default values in `#[pin_data]` Benno Lossin
2023-12-14  3:11   ` Martin Rodriguez Reboredo
2023-12-23 14:42   ` Gary Guo
2024-01-04 15:18   ` Alice Ryhl
2023-12-13 22:09 ` [PATCH v2 3/3] rust: workqueue: add `#[pin_data]` to `Work` Benno Lossin
2023-12-14  3:13   ` Martin Rodriguez Reboredo
2023-12-18 17:47     ` Benno Lossin
2023-12-18 20:49       ` Martin Rodriguez Reboredo
2023-12-23 14:43   ` Gary Guo
2024-01-04 15:19   ` Alice Ryhl
2023-12-14  2:57 ` [PATCH v2 1/3] rust: macros: add `decl_generics` to `parse_generics()` Martin Rodriguez Reboredo
2023-12-23 14:41 ` Gary Guo [this message]
2024-01-04 15:17 ` Alice Ryhl

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=20231223144110.346766ed.gary@garyguo.net \
    --to=gary@garyguo.net \
    --cc=a.hindborg@samsung.com \
    --cc=alex.gaynor@gmail.com \
    --cc=aliceryhl@google.com \
    --cc=benno.lossin@proton.me \
    --cc=bjorn3_gh@protonmail.com \
    --cc=boqun.feng@gmail.com \
    --cc=lina@asahilina.net \
    --cc=linux-kernel@vger.kernel.org \
    --cc=ojeda@kernel.org \
    --cc=rust-for-linux@vger.kernel.org \
    --cc=sylphrenadin@gmail.com \
    --cc=vincenzopalazzodev@gmail.com \
    --cc=wedsonaf@gmail.com \
    --cc=yakoyoku@gmail.com \
    /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

all inboxes | Powered by JetHome®