Received-SPF: pass (google.com: domain of linux-kernel-owner@vger.kernel.org designates 2620:137:e000::1:20 as permitted sender) client-ip=2620:137:e000::1:20;
Date:   Sun, 25 Jun 2023 22:01:17 +0100
From:   Gary Guo <gary@garyguo.net>
To:     Benno Lossin <benno.lossin@proton.me>
Cc:     Miguel Ojeda <ojeda@kernel.org>,
        Wedson Almeida Filho <wedsonaf@gmail.com>,
        Alex Gaynor <alex.gaynor@gmail.com>,
        Boqun Feng <boqun.feng@gmail.com>,
        =?UTF-8?B?QmrDtnJu?= Roy Baron <bjorn3_gh@protonmail.com>,
        Alice Ryhl <aliceryhl@google.com>,
        Andreas Hindborg <nmi@metaspace.dk>,
        rust-for-linux@vger.kernel.org, linux-kernel@vger.kernel.org,
        patches@lists.linux.dev, Asahi Lina <lina@asahilina.net>
Subject: Re: [PATCH 7/7] rust: init: add support for arbitrary paths in init
 macros
Message-ID: <20230625220117.667db9ea.gary@garyguo.net>
In-Reply-To: <20230624092330.157338-7-benno.lossin@proton.me>
References: <20230624092330.157338-1-benno.lossin@proton.me>
        <20230624092330.157338-7-benno.lossin@proton.me>
Content-Type: text/plain; charset=US-ASCII
Content-Transfer-Encoding: 7bit
MIME-Version: 1.0
X-MS-Exchange-AntiSpam-MessageData-ChunkCount: 1
X-MS-Exchange-AntiSpam-MessageData-0: =?us-ascii?Q?l43f+wo/Q9l5LZ4pfvc8qIY/LjKcPhxMZuSaN+sX26uccUrp0hSCl4qTBe+T?=
 =?us-ascii?Q?f6bohOgIkFxbxGnpt3Yt3ilq7jXCz4p4vchkD3cxIqKL/UAKii25wOl74EK0?=
 =?us-ascii?Q?u/z43/g+QEe4GzheQPxL0Fp/30XriImT5L7qSbsFx71ZlQfuchgBoL0hvGWF?=
 =?us-ascii?Q?eo7dN6dAhE3sFNz8LY1pR8CogUVtHsqPxcRRc6Bh/HxRkldrLfcVZdr8pAIl?=
 =?us-ascii?Q?yzndYKpi86QtPi2Ldy/eSXOdvQE1TnlWtpPJTKPK/BXBOT86NXErhaPHJk45?=
 =?us-ascii?Q?9KtNzKo3pNRIZVCB7Z1RuAwMTSNn4YIhhmY5CpFjLdP2Om2cWC+ADq6K8BU4?=
 =?us-ascii?Q?C63XN9dci8FmFwh0cmUwWEhlFNXelN2BybQELCipVdQIjSsX23OqEzwpTMuo?=
 =?us-ascii?Q?x4qQAmj7F9Uh2SwZS6ibPgKdRpeQv5fE9ujaquF3Ljwr/mY/8sH4G8n1iQoe?=
 =?us-ascii?Q?M9O70ktJHEJysUhL8rOfo8N2JBfqg+MxuNTEYmTmLCiNoKtnlN6+I2e/T3y0?=
 =?us-ascii?Q?vLIDgPK9tuS6ZyQ1n6VPB7HU8fuq4wd1rusQPLd33kDw6m6NMdVI2vKEvoNE?=
 =?us-ascii?Q?8q1e60pMrbsSuzcLm2Ehn9PO4YsVJFS+fZrijixWHAb8O3LicFZ4YBnue2VY?=
 =?us-ascii?Q?G8hucVZt/SCn3cI8tUSFtr+T4BIvRdjTDKLYktSLzrTwKkPnuIwg5Yfsuwd6?=
 =?us-ascii?Q?EJRqrP4g8xaf/0sWyRzI0lM+a8pGY6DRIWMDqHkuKmOy6gqEUXn9x6g4emo/?=
 =?us-ascii?Q?Va3iMvu0ae676F/mFS7uPRTyu+LJ7DlvqoEeBcraV+2O8g6fQ04N6DE9Ux2U?=
 =?us-ascii?Q?3FBCWgW8qOKObmyzPP6P5RzMguuAWqzlEEyu5JFmjXuRLNS8/juHWYvpI5mI?=
 =?us-ascii?Q?5pEFleezKun2B1WDEN3bxWBdka9Hh1AwRbW1psQlDGJaMTn1gMfvr+6Upbaq?=
 =?us-ascii?Q?uz1zkkSStAYYDGypbrTQi7xxplpTmiF+vSEMbOtIXM7YaoeN90eSVCi9EURn?=
 =?us-ascii?Q?WhLWV7TRF8x55SBys3IV+qG0dudckdbxjRspaJAYk5dX6UKtaHqLle/9ZrdS?=
 =?us-ascii?Q?Jq21IarBTCwpODcukwvBlOcJSfrq9z97RMIBLv1Jt8q9N08RddpHk/Mc5WrN?=
 =?us-ascii?Q?6H7SBe2TmZR5AIZKWjAHP1Cee5rlOxiKvwGgvnnFCKSYGE3XGvrYrvXfyzT2?=
 =?us-ascii?Q?Z4WIBZN4k+pnWEE03n7bVI4KrILj8s+jWAgVSNhbA8zQUw16jjTitHPW3/gI?=
 =?us-ascii?Q?HtVntZjYL8xLYhwREeQKUqZrFDBV0cO0shE5OEB+VrPWBpmVqNjCmMTDQKMk?=
 =?us-ascii?Q?AMhkWhnHSSH2rT6UgRi0k9XeChfrr1taOry1CknrKz8mPOerzmrg5DZtih6K?=
 =?us-ascii?Q?EVlKXN4a/UJAfpjJEDVDDGHo+lQ106NMz4Zxm3QrUGUVOaG7r5vZZC9zb3C0?=
 =?us-ascii?Q?v5LOcDUM7CW8Q6YtdF97CVUdCBg/zrs5r2n9MBRMhuoDAK/zeu6GXw3qXgEc?=
 =?us-ascii?Q?HgOZ86T0WGAYP656bYHfE/1VVHoBAbjIM8fUVPapk4XEZrApEdxcHLthhf9l?=
 =?us-ascii?Q?W5e1yDdEB4QecFPXOAxEXIFpXHsiW36PUag/gfpZ?=
X-OriginatorOrg: garyguo.net
X-MS-Exchange-CrossTenant-Network-Message-Id: 8f6953b7-09a8-4e9a-e57b-08db75bf532d
X-MS-Exchange-CrossTenant-AuthSource: LO2P265MB5183.GBRP265.PROD.OUTLOOK.COM
X-MS-Exchange-CrossTenant-AuthAs: Internal
X-MS-Exchange-CrossTenant-OriginalArrivalTime: 25 Jun 2023 21:01:19.9233
 (UTC)
X-MS-Exchange-CrossTenant-FromEntityHeader: Hosted
X-MS-Exchange-CrossTenant-Id: bbc898ad-b10f-4e10-8552-d9377b823d45
X-MS-Exchange-CrossTenant-MailboxType: HOSTED
X-MS-Exchange-CrossTenant-UserPrincipalName: eXbssnFxZbBMFlT9FNPw+ysdGclsUcnIt0S1StsxQFEgr8WjO6sqZzzAFbYlWmTV+Kt5fcPJbyOjoR0d7jYoRA==
X-MS-Exchange-Transport-CrossTenantHeadersStamped: LO4P265MB7369
X-Spam-Status: No, score=-2.1 required=5.0 tests=BAYES_00,DKIM_SIGNED,
        DKIM_VALID,DKIM_VALID_AU,DKIM_VALID_EF,RCVD_IN_DNSWL_NONE,
        RCVD_IN_MSPIKE_H2,SPF_HELO_PASS,SPF_PASS,T_SCC_BODY_TEXT_LINE
        autolearn=ham autolearn_force=no version=3.4.6
X-Spam-Checker-Version: SpamAssassin 3.4.6 (2021-04-09) on
        lindbergh.monkeyblade.net
Precedence: bulk
List-ID: <linux-kernel.vger.kernel.org>
X-Mailing-List: linux-kernel@vger.kernel.org

On Sat, 24 Jun 2023 09:25:39 +0000
Benno Lossin <benno.lossin@proton.me> wrote:

> Previously only `ident` and generic types were supported in the
> `{try_}{pin_}init!` macros. This patch allows arbitrary path fragments,
> so for example `Foo::Bar` but also very complex paths such as
> `<Foo as Baz>::Bar::<0, i32>`.
> 
> Internally this is accomplished by using `path` fragments. Due to some
> peculiar declarative macro limitations, we have to "forget" certain
> additional parsing information in the token trees. This is achieved by
> the new `retokenize` proc macro. It does not modify the input, but just
> strips this information. For example, if a declarative macro takes
> `$t:path` as its input, it cannot sensibly propagate this to a macro that
> takes `$($p:tt)*` as its input, since the `$t` token will only be
> considered one `tt` token for the second macro. If we first pipe the
> tokens through `retokenize`, then it parses as expected.

I think this "retokenize" macro could also be functionally replaced by
`paste`. Would you mind to apply my paste patch (referenced in a
previous email) and see if it works?

Best,
Gary

> 
> Suggested-by: Asahi Lina <lina@asahilina.net>
> Signed-off-by: Benno Lossin <benno.lossin@proton.me>
> ---
>  rust/kernel/init/__internal.rs |  2 ++
>  rust/kernel/init/macros.rs     | 42 +++++++++++++++++++---------------
>  rust/macros/lib.rs             | 17 +++++++++++++-
>  3 files changed, 41 insertions(+), 20 deletions(-)
> 
> diff --git a/rust/kernel/init/__internal.rs b/rust/kernel/init/__internal.rs
> index 7abd1fb65e41..e36a706a4a1b 100644
> --- a/rust/kernel/init/__internal.rs
> +++ b/rust/kernel/init/__internal.rs
> @@ -9,6 +9,8 @@
> 
>  use super::*;
> 
> +pub use ::macros::retokenize;
> +
>  /// See the [nomicon] for what subtyping is. See also [this table].
>  ///
>  /// [nomicon]: https://doc.rust-lang.org/nomicon/subtyping.html
> diff --git a/rust/kernel/init/macros.rs b/rust/kernel/init/macros.rs
> index 5dcb2e513f26..6a82be675808 100644
> --- a/rust/kernel/init/macros.rs
> +++ b/rust/kernel/init/macros.rs
> @@ -998,7 +998,7 @@ impl<$($impl_generics)*> $pin_data<$($ty_generics)*>
>  macro_rules! __init_internal {
>      (
>          @this($($this:ident)?),
> -        @typ($t:ident $(::<$($generics:ty),*>)?),
> +        @typ($t:path),
>          @fields($($fields:tt)*),
>          @error($err:ty),
>          // Either `PinData` or `InitData`, `$use_data` should only be present in the `PinData`
> @@ -1012,7 +1012,7 @@ macro_rules! __init_internal {
>      ) => {
>          $crate::__init_internal!(with_update_parsed:
>              @this($($this)?),
> -            @typ($t $(::<$($generics),*>)? ),
> +            @typ($t),
>              @fields($($fields)*),
>              @error($err),
>              @data($data, $($use_data)?),
> @@ -1023,7 +1023,7 @@ macro_rules! __init_internal {
>      };
>      (
>          @this($($this:ident)?),
> -        @typ($t:ident $(::<$($generics:ty),*>)?),
> +        @typ($t:path),
>          @fields($($fields:tt)*),
>          @error($err:ty),
>          // Either `PinData` or `InitData`, `$use_data` should only be present in the `PinData`
> @@ -1037,7 +1037,7 @@ macro_rules! __init_internal {
>      ) => {
>          $crate::__init_internal!(with_update_parsed:
>              @this($($this)?),
> -            @typ($t $(::<$($generics),*>)? ),
> +            @typ($t),
>              @fields($($fields)*),
>              @error($err),
>              @data($data, $($use_data)?),
> @@ -1048,7 +1048,7 @@ macro_rules! __init_internal {
>      };
>      (
>          @this($($this:ident)?),
> -        @typ($t:ident $(::<$($generics:ty),*>)?),
> +        @typ($t:path),
>          @fields($($fields:tt)*),
>          @error($err:ty),
>          // Either `PinData` or `InitData`, `$use_data` should only be present in the `PinData`
> @@ -1062,7 +1062,7 @@ macro_rules! __init_internal {
>      ) => {
>          $crate::__init_internal!(
>              @this($($this)?),
> -            @typ($t $(::<$($generics),*>)? ),
> +            @typ($t),
>              @fields($($fields)*),
>              @error($err),
>              @data($data, $($use_data)?),
> @@ -1073,7 +1073,7 @@ macro_rules! __init_internal {
>      };
>      (with_update_parsed:
>          @this($($this:ident)?),
> -        @typ($t:ident $(::<$($generics:ty),*>)?),
> +        @typ($t:path),
>          @fields($($fields:tt)*),
>          @error($err:ty),
>          // Either `PinData` or `InitData`, `$use_data` should only be present in the `PinData`
> @@ -1092,7 +1092,7 @@ macro_rules! __init_internal {
>          // Get the data about fields from the supplied type.
>          let data = unsafe {
>              use $crate::init::__internal::$has_data;
> -            $t$(::<$($generics),*>)?::$get_data()
> +            $crate::init::__internal::retokenize!($t::$get_data())
>          };
>          // Ensure that `data` really is of type `$data` and help with type inference:
>          let init = $crate::init::__internal::$data::make_closure::<_, __InitOk, $err>(
> @@ -1247,7 +1247,7 @@ fn is_zeroable<T: Zeroable>(ptr: *mut T) {}
>      };
>      (make_initializer:
>          @slot($slot:ident),
> -        @type_name($t:ident),
> +        @type_name($t:path),
>          @munch_fields(..Zeroable::zeroed() $(,)?),
>          @acc($($acc:tt)*),
>      ) => {
> @@ -1263,15 +1263,17 @@ fn is_zeroable<T: Zeroable>(ptr: *mut T) {}
>              // not get executed, so it has no effect.
>              ::core::ptr::write($slot, zeroed);
>              zeroed = ::core::mem::zeroed();
> -            ::core::ptr::write($slot, $t {
> -                $($acc)*
> -                ..zeroed
> -            });
> +            $crate::init::__internal::retokenize!(
> +                ::core::ptr::write($slot, $t {
> +                    $($acc)*
> +                    ..zeroed
> +                });
> +            );
>          }
>      };
>      (make_initializer:
>          @slot($slot:ident),
> -        @type_name($t:ident),
> +        @type_name($t:path),
>          @munch_fields($(,)?),
>          @acc($($acc:tt)*),
>      ) => {
> @@ -1279,14 +1281,16 @@ fn is_zeroable<T: Zeroable>(ptr: *mut T) {}
>          // Since we are in the `if false` branch, this will never get executed. We abuse `slot` to
>          // get the correct type inference here:
>          unsafe {
> -            ::core::ptr::write($slot, $t {
> -                $($acc)*
> -            });
> +            $crate::init::__internal::retokenize!(
> +                ::core::ptr::write($slot, $t {
> +                    $($acc)*
> +                });
> +            );
>          }
>      };
>      (make_initializer:
>          @slot($slot:ident),
> -        @type_name($t:ident),
> +        @type_name($t:path),
>          @munch_fields($field:ident <- $val:expr, $($rest:tt)*),
>          @acc($($acc:tt)*),
>      ) => {
> @@ -1299,7 +1303,7 @@ fn is_zeroable<T: Zeroable>(ptr: *mut T) {}
>      };
>      (make_initializer:
>          @slot($slot:ident),
> -        @type_name($t:ident),
> +        @type_name($t:path),
>          @munch_fields($field:ident $(: $val:expr)?, $($rest:tt)*),
>          @acc($($acc:tt)*),
>      ) => {
> diff --git a/rust/macros/lib.rs b/rust/macros/lib.rs
> index 9f056a5c780a..d329ab622fd4 100644
> --- a/rust/macros/lib.rs
> +++ b/rust/macros/lib.rs
> @@ -12,7 +12,7 @@
>  mod vtable;
>  mod zeroable;
> 
> -use proc_macro::TokenStream;
> +use proc_macro::{Group, TokenStream, TokenTree};
> 
>  /// Declares a kernel module.
>  ///
> @@ -266,3 +266,18 @@ pub fn pinned_drop(args: TokenStream, input: TokenStream) -> TokenStream {
>  pub fn derive_zeroable(input: TokenStream) -> TokenStream {
>      zeroable::derive(input)
>  }
> +
> +/// Does not modify the given TokenStream, but removes any declarative macro information.
> +#[proc_macro]
> +pub fn retokenize(input: TokenStream) -> TokenStream {
> +    fn id(tt: TokenTree) -> TokenTree {
> +        match tt {
> +            TokenTree::Group(g) => TokenTree::Group(Group::new(
> +                g.delimiter(),
> +                g.stream().into_iter().map(id).collect(),
> +            )),
> +            x => x,
> +        }
> +    }
> +    input.into_iter().map(id).collect()
> +}
> --
> 2.41.0
> 
>