diff mbox series

[v2,2/3] rust: macros: allow generic parameter default values in `#[pin_data]`

Message ID 20231213220447.3613500-2-benno.lossin@proton.me
State New
Headers
Series [v2,1/3] rust: macros: add `decl_generics` to `parse_generics()` |

Commit Message

Benno Lossin Dec. 13, 2023, 10:08 p.m. UTC 
  Add support for generic parameters defaults in `#[pin_data]` by using
the newly introduced `decl_generics` instead of the `impl_generics`.

Before this would not compile:

    #[pin_data]
    struct Foo<const N: usize = 0> {
        // ...
    }

because it would be expanded to this:

    struct Foo<const N: usize = 0> {
        // ...
    }

    const _: () = {
        struct __ThePinData<const N: usize = 0> {
            __phantom: ::core::marker::PhantomData<fn(Foo<N>) -> Foo<N>>,
        }
        impl<const N: usize = 0> ::core::clone::Clone for __ThePinData<N> {
            fn clone(&self) -> Self {
                *self
            }
        }

        // [...] rest of expansion omitted
    };

The problem is with the `impl<const N: usize = 0>`, since that is
invalid Rust syntax. It should not mention the default value at all,
since default values only make sense on type definitions.

The new `impl_generics` do not contain the default values, thus
generating correct Rust code.

This is used by the next commit that puts `#[pin_data]` on
`kernel::workqueue::Work`.

Signed-off-by: Benno Lossin <benno.lossin@proton.me>
---
v1 -> v2:
- clarify the change in the commit message
- add motivation to the commit message

 rust/kernel/init/macros.rs | 19 ++++++++++++++++++-
 rust/macros/pin_data.rs    |  3 ++-
 2 files changed, 20 insertions(+), 2 deletions(-)

Comments

Martin Rodriguez Reboredo Dec. 14, 2023, 3:11 a.m. UTC | #1 
On 12/13/23 19:08, Benno Lossin wrote:
> Add support for generic parameters defaults in `#[pin_data]` by using
> the newly introduced `decl_generics` instead of the `impl_generics`.
> 
> Before this would not compile:
> 
>      #[pin_data]
>      struct Foo<const N: usize = 0> {
>          // ...
>      }
> 
> because it would be expanded to this:
> 
>      struct Foo<const N: usize = 0> {
>          // ...
>      }
> 
>      const _: () = {
>          struct __ThePinData<const N: usize = 0> {
>              __phantom: ::core::marker::PhantomData<fn(Foo<N>) -> Foo<N>>,
>          }
>          impl<const N: usize = 0> ::core::clone::Clone for __ThePinData<N> {
>              fn clone(&self) -> Self {
>                  *self
>              }
>          }
> 
>          // [...] rest of expansion omitted
>      };
> 
> The problem is with the `impl<const N: usize = 0>`, since that is
> invalid Rust syntax. It should not mention the default value at all,
> since default values only make sense on type definitions.
> 
> The new `impl_generics` do not contain the default values, thus
> generating correct Rust code.
> 
> This is used by the next commit that puts `#[pin_data]` on
> `kernel::workqueue::Work`.
> 
> Signed-off-by: Benno Lossin <benno.lossin@proton.me>
> ---
> [...]
Reviewed-by: Martin Rodriguez Reboredo <yakoyoku@gmail.com>
Gary Guo Dec. 23, 2023, 2:42 p.m. UTC | #2 
On Wed, 13 Dec 2023 22:08:53 +0000
Benno Lossin <benno.lossin@proton.me> wrote:

> Add support for generic parameters defaults in `#[pin_data]` by using
> the newly introduced `decl_generics` instead of the `impl_generics`.
> 
> Before this would not compile:
> 
>     #[pin_data]
>     struct Foo<const N: usize = 0> {
>         // ...
>     }
> 
> because it would be expanded to this:
> 
>     struct Foo<const N: usize = 0> {
>         // ...
>     }
> 
>     const _: () = {
>         struct __ThePinData<const N: usize = 0> {
>             __phantom: ::core::marker::PhantomData<fn(Foo<N>) -> Foo<N>>,
>         }
>         impl<const N: usize = 0> ::core::clone::Clone for __ThePinData<N> {
>             fn clone(&self) -> Self {
>                 *self
>             }
>         }
> 
>         // [...] rest of expansion omitted
>     };
> 
> The problem is with the `impl<const N: usize = 0>`, since that is
> invalid Rust syntax. It should not mention the default value at all,
> since default values only make sense on type definitions.
> 
> The new `impl_generics` do not contain the default values, thus
> generating correct Rust code.
> 
> This is used by the next commit that puts `#[pin_data]` on
> `kernel::workqueue::Work`.
> 
> Signed-off-by: Benno Lossin <benno.lossin@proton.me>

Reviewed-by: Gary Guo <gary@garyguo.net>

> ---
> v1 -> v2:
> - clarify the change in the commit message
> - add motivation to the commit message
> 
>  rust/kernel/init/macros.rs | 19 ++++++++++++++++++-
>  rust/macros/pin_data.rs    |  3 ++-
>  2 files changed, 20 insertions(+), 2 deletions(-)
Alice Ryhl Jan. 4, 2024, 3:18 p.m. UTC | #3 
On Wed, Dec 13, 2023 at 11:09 PM Benno Lossin <benno.lossin@proton.me> wrote:
>
> Add support for generic parameters defaults in `#[pin_data]` by using
> the newly introduced `decl_generics` instead of the `impl_generics`.
>
> Before this would not compile:
>
>     #[pin_data]
>     struct Foo<const N: usize = 0> {
>         // ...
>     }
>
> because it would be expanded to this:
>
>     struct Foo<const N: usize = 0> {
>         // ...
>     }
>
>     const _: () = {
>         struct __ThePinData<const N: usize = 0> {
>             __phantom: ::core::marker::PhantomData<fn(Foo<N>) -> Foo<N>>,
>         }
>         impl<const N: usize = 0> ::core::clone::Clone for __ThePinData<N> {
>             fn clone(&self) -> Self {
>                 *self
>             }
>         }
>
>         // [...] rest of expansion omitted
>     };
>
> The problem is with the `impl<const N: usize = 0>`, since that is
> invalid Rust syntax. It should not mention the default value at all,
> since default values only make sense on type definitions.
>
> The new `impl_generics` do not contain the default values, thus
> generating correct Rust code.
>
> This is used by the next commit that puts `#[pin_data]` on
> `kernel::workqueue::Work`.
>
> Signed-off-by: Benno Lossin <benno.lossin@proton.me>

Tested-by: Alice Ryhl <aliceryhl@google.com>
Reviewed-by: Alice Ryhl <aliceryhl@google.com>
diff mbox series

Patch

diff --git a/rust/kernel/init/macros.rs b/rust/kernel/init/macros.rs
index cb6e61b6c50b..624e9108e3b4 100644
--- a/rust/kernel/init/macros.rs
+++ b/rust/kernel/init/macros.rs
@@ -538,6 +538,7 @@  macro_rules! __pin_data {
         ),
         @impl_generics($($impl_generics:tt)*),
         @ty_generics($($ty_generics:tt)*),
+        @decl_generics($($decl_generics:tt)*),
         @body({ $($fields:tt)* }),
     ) => {
         // We now use token munching to iterate through all of the fields. While doing this we
@@ -560,6 +561,9 @@  macro_rules! __pin_data {
             @impl_generics($($impl_generics)*),
             // The 'ty generics', the generics that will need to be specified on the impl blocks.
             @ty_generics($($ty_generics)*),
+            // The 'decl generics', the generics that need to be specified on the struct
+            // definition.
+            @decl_generics($($decl_generics)*),
             // The where clause of any impl block and the declaration.
             @where($($($whr)*)?),
             // The remaining fields tokens that need to be processed.
@@ -585,6 +589,7 @@  macro_rules! __pin_data {
         @name($name:ident),
         @impl_generics($($impl_generics:tt)*),
         @ty_generics($($ty_generics:tt)*),
+        @decl_generics($($decl_generics:tt)*),
         @where($($whr:tt)*),
         // We found a PhantomPinned field, this should generally be pinned!
         @fields_munch($field:ident : $($($(::)?core::)?marker::)?PhantomPinned, $($rest:tt)*),
@@ -607,6 +612,7 @@  macro_rules! __pin_data {
             @name($name),
             @impl_generics($($impl_generics)*),
             @ty_generics($($ty_generics)*),
+            @decl_generics($($decl_generics)*),
             @where($($whr)*),
             @fields_munch($($rest)*),
             @pinned($($pinned)* $($accum)* $field: ::core::marker::PhantomPinned,),
@@ -623,6 +629,7 @@  macro_rules! __pin_data {
         @name($name:ident),
         @impl_generics($($impl_generics:tt)*),
         @ty_generics($($ty_generics:tt)*),
+        @decl_generics($($decl_generics:tt)*),
         @where($($whr:tt)*),
         // We reached the field declaration.
         @fields_munch($field:ident : $type:ty, $($rest:tt)*),
@@ -640,6 +647,7 @@  macro_rules! __pin_data {
             @name($name),
             @impl_generics($($impl_generics)*),
             @ty_generics($($ty_generics)*),
+            @decl_generics($($decl_generics)*),
             @where($($whr)*),
             @fields_munch($($rest)*),
             @pinned($($pinned)* $($accum)* $field: $type,),
@@ -656,6 +664,7 @@  macro_rules! __pin_data {
         @name($name:ident),
         @impl_generics($($impl_generics:tt)*),
         @ty_generics($($ty_generics:tt)*),
+        @decl_generics($($decl_generics:tt)*),
         @where($($whr:tt)*),
         // We reached the field declaration.
         @fields_munch($field:ident : $type:ty, $($rest:tt)*),
@@ -673,6 +682,7 @@  macro_rules! __pin_data {
             @name($name),
             @impl_generics($($impl_generics)*),
             @ty_generics($($ty_generics)*),
+            @decl_generics($($decl_generics)*),
             @where($($whr)*),
             @fields_munch($($rest)*),
             @pinned($($pinned)*),
@@ -689,6 +699,7 @@  macro_rules! __pin_data {
         @name($name:ident),
         @impl_generics($($impl_generics:tt)*),
         @ty_generics($($ty_generics:tt)*),
+        @decl_generics($($decl_generics:tt)*),
         @where($($whr:tt)*),
         // We found the `#[pin]` attr.
         @fields_munch(#[pin] $($rest:tt)*),
@@ -705,6 +716,7 @@  macro_rules! __pin_data {
             @name($name),
             @impl_generics($($impl_generics)*),
             @ty_generics($($ty_generics)*),
+            @decl_generics($($decl_generics)*),
             @where($($whr)*),
             @fields_munch($($rest)*),
             // We do not include `#[pin]` in the list of attributes, since it is not actually an
@@ -724,6 +736,7 @@  macro_rules! __pin_data {
         @name($name:ident),
         @impl_generics($($impl_generics:tt)*),
         @ty_generics($($ty_generics:tt)*),
+        @decl_generics($($decl_generics:tt)*),
         @where($($whr:tt)*),
         // We reached the field declaration with visibility, for simplicity we only munch the
         // visibility and put it into `$accum`.
@@ -741,6 +754,7 @@  macro_rules! __pin_data {
             @name($name),
             @impl_generics($($impl_generics)*),
             @ty_generics($($ty_generics)*),
+            @decl_generics($($decl_generics)*),
             @where($($whr)*),
             @fields_munch($field $($rest)*),
             @pinned($($pinned)*),
@@ -757,6 +771,7 @@  macro_rules! __pin_data {
         @name($name:ident),
         @impl_generics($($impl_generics:tt)*),
         @ty_generics($($ty_generics:tt)*),
+        @decl_generics($($decl_generics:tt)*),
         @where($($whr:tt)*),
         // Some other attribute, just put it into `$accum`.
         @fields_munch(#[$($attr:tt)*] $($rest:tt)*),
@@ -773,6 +788,7 @@  macro_rules! __pin_data {
             @name($name),
             @impl_generics($($impl_generics)*),
             @ty_generics($($ty_generics)*),
+            @decl_generics($($decl_generics)*),
             @where($($whr)*),
             @fields_munch($($rest)*),
             @pinned($($pinned)*),
@@ -789,6 +805,7 @@  macro_rules! __pin_data {
         @name($name:ident),
         @impl_generics($($impl_generics:tt)*),
         @ty_generics($($ty_generics:tt)*),
+        @decl_generics($($decl_generics:tt)*),
         @where($($whr:tt)*),
         // We reached the end of the fields, plus an optional additional comma, since we added one
         // before and the user is also allowed to put a trailing comma.
@@ -802,7 +819,7 @@  macro_rules! __pin_data {
     ) => {
         // Declare the struct with all fields in the correct order.
         $($struct_attrs)*
-        $vis struct $name <$($impl_generics)*>
+        $vis struct $name <$($decl_generics)*>
         where $($whr)*
         {
             $($fields)*
diff --git a/rust/macros/pin_data.rs b/rust/macros/pin_data.rs
index 022e68e9720d..1d4a3547c684 100644
--- a/rust/macros/pin_data.rs
+++ b/rust/macros/pin_data.rs
@@ -10,7 +10,7 @@  pub(crate) fn pin_data(args: TokenStream, input: TokenStream) -> TokenStream {
     let (
         Generics {
             impl_generics,
-            decl_generics: _,
+            decl_generics,
             ty_generics,
         },
         rest,
@@ -77,6 +77,7 @@  pub(crate) fn pin_data(args: TokenStream, input: TokenStream) -> TokenStream {
         @sig(#(#rest)*),
         @impl_generics(#(#impl_generics)*),
         @ty_generics(#(#ty_generics)*),
+        @decl_generics(#(#decl_generics)*),
         @body(#last),
     });
     quoted.extend(errs);