rust-for-linux.vger.kernel.org archive mirror
 help / color / mirror / Atom feed
From: Boqun Feng <boqun.feng@gmail.com>
To: Benno Lossin <benno.lossin@proton.me>
Cc: "Miguel Ojeda" <ojeda@kernel.org>,
	"Alex Gaynor" <alex.gaynor@gmail.com>,
	"Gary Guo" <gary@garyguo.net>,
	"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>,
	"Christian Schrefl" <chrisi.schrefl@gmail.com>,
	"Alban Kurti" <kurti@invicto.ai>,
	"Michael Vetter" <jubalh@iodoru.org>,
	rust-for-linux@vger.kernel.org, linux-kernel@vger.kernel.org
Subject: Re: [PATCH 8/8] rust: pin-init: add `MaybeZeroable` derive macro
Date: Mon, 21 Apr 2025 21:54:11 -0700	[thread overview]
Message-ID: <aAcg86QfvlA0zAh4@Mac.home> (raw)
In-Reply-To: <20250421221728.528089-9-benno.lossin@proton.me>

On Mon, Apr 21, 2025 at 10:18:52PM +0000, Benno Lossin wrote:
> This derive macro implements `Zeroable` for structs & unions precisely
> if all fields also implement `Zeroable` and does nothing otherwise. The
> plain `Zeroable` derive macro instead errors when it cannot derive
> `Zeroable` safely. The `MaybeZeroable` derive macro is useful in cases
> where manual checking is infeasible such as with the bindings crate.
> 

Hmm... seems we need a customized auto trait? How hard would that be?

Regards,
Boqun

> Move the zeroable generics parsing into a standalone function in order
> to avoid code duplication between the two derive macros.
> 
> Link: https://github.com/Rust-for-Linux/pin-init/pull/42/commits/1165cdad1a391b923efaf30cf76bc61e38da022e
> Signed-off-by: Benno Lossin <benno.lossin@proton.me>
> ---
>  rust/pin-init/internal/src/lib.rs      |  5 +++
>  rust/pin-init/internal/src/zeroable.rs | 27 +++++++++++-
>  rust/pin-init/src/lib.rs               | 30 +++++++++++++
>  rust/pin-init/src/macros.rs            | 59 ++++++++++++++++++++++++++
>  4 files changed, 120 insertions(+), 1 deletion(-)
> 
> diff --git a/rust/pin-init/internal/src/lib.rs b/rust/pin-init/internal/src/lib.rs
> index 56aa9ecc1e1a..297b0129a5bf 100644
> --- a/rust/pin-init/internal/src/lib.rs
> +++ b/rust/pin-init/internal/src/lib.rs
> @@ -47,3 +47,8 @@ pub fn pinned_drop(args: TokenStream, input: TokenStream) -> TokenStream {
>  pub fn derive_zeroable(input: TokenStream) -> TokenStream {
>      zeroable::derive(input.into()).into()
>  }
> +
> +#[proc_macro_derive(MaybeZeroable)]
> +pub fn maybe_derive_zeroable(input: TokenStream) -> TokenStream {
> +    zeroable::maybe_derive(input.into()).into()
> +}
> diff --git a/rust/pin-init/internal/src/zeroable.rs b/rust/pin-init/internal/src/zeroable.rs
> index acc94008c152..e0ed3998445c 100644
> --- a/rust/pin-init/internal/src/zeroable.rs
> +++ b/rust/pin-init/internal/src/zeroable.rs
> @@ -6,7 +6,14 @@
>  use crate::helpers::{parse_generics, Generics};
>  use proc_macro::{TokenStream, TokenTree};
>  
> -pub(crate) fn derive(input: TokenStream) -> TokenStream {
> +pub(crate) fn parse_zeroable_derive_input(
> +    input: TokenStream,
> +) -> (
> +    Vec<TokenTree>,
> +    Vec<TokenTree>,
> +    Vec<TokenTree>,
> +    Option<TokenTree>,
> +) {
>      let (
>          Generics {
>              impl_generics,
> @@ -64,6 +71,11 @@ pub(crate) fn derive(input: TokenStream) -> TokenStream {
>      if in_generic && !inserted {
>          new_impl_generics.extend(quote! { : ::pin_init::Zeroable });
>      }
> +    (rest, new_impl_generics, ty_generics, last)
> +}
> +
> +pub(crate) fn derive(input: TokenStream) -> TokenStream {
> +    let (rest, new_impl_generics, ty_generics, last) = parse_zeroable_derive_input(input);
>      quote! {
>          ::pin_init::__derive_zeroable!(
>              parse_input:
> @@ -74,3 +86,16 @@ pub(crate) fn derive(input: TokenStream) -> TokenStream {
>          );
>      }
>  }
> +
> +pub(crate) fn maybe_derive(input: TokenStream) -> TokenStream {
> +    let (rest, new_impl_generics, ty_generics, last) = parse_zeroable_derive_input(input);
> +    quote! {
> +        ::pin_init::__maybe_derive_zeroable!(
> +            parse_input:
> +                @sig(#(#rest)*),
> +                @impl_generics(#(#new_impl_generics)*),
> +                @ty_generics(#(#ty_generics)*),
> +                @body(#last),
> +        );
> +    }
> +}
> diff --git a/rust/pin-init/src/lib.rs b/rust/pin-init/src/lib.rs
> index 774f8ca033bc..05a0cd6ad8f4 100644
> --- a/rust/pin-init/src/lib.rs
> +++ b/rust/pin-init/src/lib.rs
> @@ -413,6 +413,36 @@
>  /// ```
>  pub use ::pin_init_internal::Zeroable;
>  
> +/// Derives the [`Zeroable`] trait for the given struct if all fields implement [`Zeroable`].
> +///
> +/// Contrary to the derive macro named [`macro@Zeroable`], this one silently fails when a field
> +/// doesn't implement [`Zeroable`].
> +///
> +/// # Examples
> +///
> +/// ```
> +/// use pin_init::MaybeZeroable;
> +///
> +/// // implmements `Zeroable`
> +/// #[derive(MaybeZeroable)]
> +/// pub struct DriverData {
> +///     id: i64,
> +///     buf_ptr: *mut u8,
> +///     len: usize,
> +/// }
> +///
> +/// // does not implmement `Zeroable`
> +/// #[derive(MaybeZeroable)]
> +/// pub struct DriverData2 {
> +///     id: i64,
> +///     buf_ptr: *mut u8,
> +///     len: usize,
> +///     // this field doesn't implement `Zeroable`
> +///     other_data: &'static i32,
> +/// }
> +/// ```
> +pub use ::pin_init_internal::MaybeZeroable;
> +
>  /// Initialize and pin a type directly on the stack.
>  ///
>  /// # Examples
> diff --git a/rust/pin-init/src/macros.rs b/rust/pin-init/src/macros.rs
> index 332d7e08925b..935d77745d1d 100644
> --- a/rust/pin-init/src/macros.rs
> +++ b/rust/pin-init/src/macros.rs
> @@ -1443,3 +1443,62 @@ fn ensure_zeroable<$($impl_generics)*>()
>          };
>      };
>  }
> +
> +#[doc(hidden)]
> +#[macro_export]
> +macro_rules! __maybe_derive_zeroable {
> +    (parse_input:
> +        @sig(
> +            $(#[$($struct_attr:tt)*])*
> +            $vis:vis struct $name:ident
> +            $(where $($whr:tt)*)?
> +        ),
> +        @impl_generics($($impl_generics:tt)*),
> +        @ty_generics($($ty_generics:tt)*),
> +        @body({
> +            $(
> +                $(#[$($field_attr:tt)*])*
> +                $field_vis:vis $field:ident : $field_ty:ty
> +            ),* $(,)?
> +        }),
> +    ) => {
> +        // SAFETY: Every field type implements `Zeroable` and padding bytes may be zero.
> +        #[automatically_derived]
> +        unsafe impl<$($impl_generics)*> $crate::Zeroable for $name<$($ty_generics)*>
> +        where
> +            $(
> +                // the `for<'__dummy>` HRTB makes this not error without the `trivial_bounds`
> +                // feature <https://github.com/rust-lang/rust/issues/48214#issuecomment-2557829956>.
> +                $field_ty: for<'__dummy> $crate::Zeroable,
> +            )*
> +            $($($whr)*)?
> +        {}
> +    };
> +    (parse_input:
> +        @sig(
> +            $(#[$($struct_attr:tt)*])*
> +            $vis:vis union $name:ident
> +            $(where $($whr:tt)*)?
> +        ),
> +        @impl_generics($($impl_generics:tt)*),
> +        @ty_generics($($ty_generics:tt)*),
> +        @body({
> +            $(
> +                $(#[$($field_attr:tt)*])*
> +                $field_vis:vis $field:ident : $field_ty:ty
> +            ),* $(,)?
> +        }),
> +    ) => {
> +        // SAFETY: Every field type implements `Zeroable` and padding bytes may be zero.
> +        #[automatically_derived]
> +        unsafe impl<$($impl_generics)*> $crate::Zeroable for $name<$($ty_generics)*>
> +        where
> +            $(
> +                // the `for<'__dummy>` HRTB makes this not error without the `trivial_bounds`
> +                // feature <https://github.com/rust-lang/rust/issues/48214#issuecomment-2557829956>.
> +                $field_ty: for<'__dummy> $crate::Zeroable,
> +            )*
> +            $($($whr)*)?
> +        {}
> +    };
> +}
> -- 
> 2.48.1
> 
> 

  reply	other threads:[~2025-04-22  4:54 UTC|newest]

Thread overview: 23+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2025-04-21 22:17 [PATCH 0/8] pin-init sync for v6.16 Benno Lossin
2025-04-21 22:17 ` [PATCH 1/8] rust: pin-init: add `cast_[pin_]init` functions to change the initialized type Benno Lossin
2025-04-22  6:56   ` Christian Schrefl
2025-04-21 22:17 ` [PATCH 2/8] rust: pin-init: Add the `Wrapper` trait Benno Lossin
2025-04-22  4:44   ` Boqun Feng
2025-04-21 22:18 ` [PATCH 3/8] rust: pin-init: Implement `Wrapper` for `UnsafePinned` behind feature flag Benno Lossin
2025-04-22  9:42   ` Christian Schrefl
2025-04-22 11:21     ` Benno Lossin
2025-04-22 14:17       ` Christian Schrefl
2025-04-21 22:18 ` [PATCH 4/8] rust: pin-init: Update Changelog and Readme Benno Lossin
2025-04-21 22:18 ` [PATCH 5/8] rust: pin-init: Update the structural pinning link in readme Benno Lossin
2025-04-21 22:18 ` [PATCH 6/8] rust: pin-init: allow `pub` fields in `derive(Zeroable)` Benno Lossin
2025-04-22  4:55   ` Boqun Feng
2025-04-22  8:30     ` Benno Lossin
2025-04-22 14:14       ` Boqun Feng
2025-04-22 14:45         ` Benno Lossin
2025-04-22 21:11           ` Boqun Feng
2025-04-22 21:56             ` Benno Lossin
2025-04-21 22:18 ` [PATCH 7/8] rust: pin-init: allow `Zeroable` derive macro to also be applied to unions Benno Lossin
2025-04-21 22:18 ` [PATCH 8/8] rust: pin-init: add `MaybeZeroable` derive macro Benno Lossin
2025-04-22  4:54   ` Boqun Feng [this message]
2025-04-22  7:56     ` Benno Lossin
2025-05-01 16:38 ` [PATCH 0/8] pin-init sync for v6.16 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=aAcg86QfvlA0zAh4@Mac.home \
    --to=boqun.feng@gmail.com \
    --cc=a.hindborg@kernel.org \
    --cc=alex.gaynor@gmail.com \
    --cc=aliceryhl@google.com \
    --cc=benno.lossin@proton.me \
    --cc=bjorn3_gh@protonmail.com \
    --cc=chrisi.schrefl@gmail.com \
    --cc=dakr@kernel.org \
    --cc=gary@garyguo.net \
    --cc=jubalh@iodoru.org \
    --cc=kurti@invicto.ai \
    --cc=linux-kernel@vger.kernel.org \
    --cc=me@kloenk.dev \
    --cc=ojeda@kernel.org \
    --cc=rust-for-linux@vger.kernel.org \
    --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;
as well as URLs for NNTP newsgroup(s).