Merges the implementations of `try_init!` and `try_pin_init!`. These two
macros are very similar, but use different traits. The new macro
`__init_internal!` that is now the implementation for both takes these
traits as parameters.
This change does not affect any users, as no public API has been
changed, but it should simplify maintaining the init macros.
Signed-off-by: Benno Lossin <[email protected]>
---
rust/kernel/init.rs | 388 +++----------------------------------
rust/kernel/init/macros.rs | 237 +++++++++++++++++++++-
2 files changed, 259 insertions(+), 366 deletions(-)
diff --git a/rust/kernel/init.rs b/rust/kernel/init.rs
index b4332a4ec1f4..d9a91950cba2 100644
--- a/rust/kernel/init.rs
+++ b/rust/kernel/init.rs
@@ -540,11 +540,14 @@ macro_rules! pin_init {
($(&$this:ident in)? $t:ident $(::<$($generics:ty),* $(,)?>)? {
$($fields:tt)*
}) => {
- $crate::try_pin_init!(
+ $crate::__init_internal!(
@this($($this)?),
@typ($t $(::<$($generics),*>)?),
@fields($($fields)*),
@error(::core::convert::Infallible),
+ @data(PinData, use_data),
+ @has_data(HasPinData, __pin_data),
+ @construct_closure(pin_init_from_closure),
)
};
}
@@ -593,205 +596,29 @@ macro_rules! try_pin_init {
($(&$this:ident in)? $t:ident $(::<$($generics:ty),* $(,)?>)? {
$($fields:tt)*
}) => {
- $crate::try_pin_init!(
+ $crate::__init_internal!(
@this($($this)?),
@typ($t $(::<$($generics),*>)? ),
@fields($($fields)*),
@error($crate::error::Error),
+ @data(PinData, use_data),
+ @has_data(HasPinData, __pin_data),
+ @construct_closure(pin_init_from_closure),
)
};
($(&$this:ident in)? $t:ident $(::<$($generics:ty),* $(,)?>)? {
$($fields:tt)*
}? $err:ty) => {
- $crate::try_pin_init!(
+ $crate::__init_internal!(
@this($($this)?),
@typ($t $(::<$($generics),*>)? ),
@fields($($fields)*),
@error($err),
+ @data(PinData, use_data),
+ @has_data(HasPinData, __pin_data),
+ @construct_closure(pin_init_from_closure),
)
};
- (
- @this($($this:ident)?),
- @typ($t:ident $(::<$($generics:ty),*>)?),
- @fields($($fields:tt)*),
- @error($err:ty),
- ) => {{
- // We do not want to allow arbitrary returns, so we declare this type as the `Ok` return
- // type and shadow it later when we insert the arbitrary user code. That way there will be
- // no possibility of returning without `unsafe`.
- struct __InitOk;
- // Get the pin data from the supplied type.
- let data = unsafe {
- use $crate::init::__internal::HasPinData;
- $t$(::<$($generics),*>)?::__pin_data()
- };
- // Ensure that `data` really is of type `PinData` and help with type inference:
- let init = $crate::init::__internal::PinData::make_closure::<_, __InitOk, $err>(
- data,
- move |slot| {
- {
- // Shadow the structure so it cannot be used to return early.
- struct __InitOk;
- // Create the `this` so it can be referenced by the user inside of the
- // expressions creating the individual fields.
- $(let $this = unsafe { ::core::ptr::NonNull::new_unchecked(slot) };)?
- // Initialize every field.
- $crate::try_pin_init!(init_slot:
- @data(data),
- @slot(slot),
- @munch_fields($($fields)*,),
- );
- // We use unreachable code to ensure that all fields have been mentioned exactly
- // once, this struct initializer will still be type-checked and complain with a
- // very natural error message if a field is forgotten/mentioned more than once.
- #[allow(unreachable_code, clippy::diverging_sub_expression)]
- if false {
- $crate::try_pin_init!(make_initializer:
- @slot(slot),
- @type_name($t),
- @munch_fields($($fields)*,),
- @acc(),
- );
- }
- // Forget all guards, since initialization was a success.
- $crate::try_pin_init!(forget_guards:
- @munch_fields($($fields)*,),
- );
- }
- Ok(__InitOk)
- }
- );
- let init = move |slot| -> ::core::result::Result<(), $err> {
- init(slot).map(|__InitOk| ())
- };
- let init = unsafe { $crate::init::pin_init_from_closure::<_, $err>(init) };
- init
- }};
- (init_slot:
- @data($data:ident),
- @slot($slot:ident),
- @munch_fields($(,)?),
- ) => {
- // Endpoint of munching, no fields are left.
- };
- (init_slot:
- @data($data:ident),
- @slot($slot:ident),
- // In-place initialization syntax.
- @munch_fields($field:ident <- $val:expr, $($rest:tt)*),
- ) => {
- let $field = $val;
- // Call the initializer.
- //
- // SAFETY: `slot` is valid, because we are inside of an initializer closure, we
- // return when an error/panic occurs.
- // We also use the `data` to require the correct trait (`Init` or `PinInit`) for `$field`.
- unsafe { $data.$field(::core::ptr::addr_of_mut!((*$slot).$field), $field)? };
- // Create the drop guard.
- //
- // We only give access to `&DropGuard`, so it cannot be forgotten via safe code.
- //
- // SAFETY: We forget the guard later when initialization has succeeded.
- let $field = &unsafe {
- $crate::init::__internal::DropGuard::new(::core::ptr::addr_of_mut!((*$slot).$field))
- };
-
- $crate::try_pin_init!(init_slot:
- @data($data),
- @slot($slot),
- @munch_fields($($rest)*),
- );
- };
- (init_slot:
- @data($data:ident),
- @slot($slot:ident),
- // Direct value init, this is safe for every field.
- @munch_fields($field:ident $(: $val:expr)?, $($rest:tt)*),
- ) => {
- $(let $field = $val;)?
- // Initialize the field.
- //
- // SAFETY: The memory at `slot` is uninitialized.
- unsafe { ::core::ptr::write(::core::ptr::addr_of_mut!((*$slot).$field), $field) };
- // Create the drop guard:
- //
- // We only give access to `&DropGuard`, so it cannot be accidentally forgotten.
- //
- // SAFETY: We forget the guard later when initialization has succeeded.
- let $field = &unsafe {
- $crate::init::__internal::DropGuard::new(::core::ptr::addr_of_mut!((*$slot).$field))
- };
-
- $crate::try_pin_init!(init_slot:
- @data($data),
- @slot($slot),
- @munch_fields($($rest)*),
- );
- };
- (make_initializer:
- @slot($slot:ident),
- @type_name($t:ident),
- @munch_fields($(,)?),
- @acc($($acc:tt)*),
- ) => {
- // Endpoint, nothing more to munch, create the initializer.
- // 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)*
- });
- }
- };
- (make_initializer:
- @slot($slot:ident),
- @type_name($t:ident),
- @munch_fields($field:ident <- $val:expr, $($rest:tt)*),
- @acc($($acc:tt)*),
- ) => {
- $crate::try_pin_init!(make_initializer:
- @slot($slot),
- @type_name($t),
- @munch_fields($($rest)*),
- @acc($($acc)* $field: ::core::panic!(),),
- );
- };
- (make_initializer:
- @slot($slot:ident),
- @type_name($t:ident),
- @munch_fields($field:ident $(: $val:expr)?, $($rest:tt)*),
- @acc($($acc:tt)*),
- ) => {
- $crate::try_pin_init!(make_initializer:
- @slot($slot),
- @type_name($t),
- @munch_fields($($rest)*),
- @acc($($acc)* $field: ::core::panic!(),),
- );
- };
- (forget_guards:
- @munch_fields($(,)?),
- ) => {
- // Munching finished.
- };
- (forget_guards:
- @munch_fields($field:ident <- $val:expr, $($rest:tt)*),
- ) => {
- unsafe { $crate::init::__internal::DropGuard::forget($field) };
-
- $crate::try_pin_init!(forget_guards:
- @munch_fields($($rest)*),
- );
- };
- (forget_guards:
- @munch_fields($field:ident $(: $val:expr)?, $($rest:tt)*),
- ) => {
- unsafe { $crate::init::__internal::DropGuard::forget($field) };
-
- $crate::try_pin_init!(forget_guards:
- @munch_fields($($rest)*),
- );
- };
}
/// Construct an in-place initializer for `struct`s.
@@ -816,11 +643,14 @@ macro_rules! init {
($(&$this:ident in)? $t:ident $(::<$($generics:ty),* $(,)?>)? {
$($fields:tt)*
}) => {
- $crate::try_init!(
+ $crate::__init_internal!(
@this($($this)?),
@typ($t $(::<$($generics),*>)?),
@fields($($fields)*),
@error(::core::convert::Infallible),
+ @data(InitData, /*no use_data*/),
+ @has_data(HasInitData, __init_data),
+ @construct_closure(init_from_closure),
)
}
}
@@ -863,199 +693,29 @@ macro_rules! try_init {
($(&$this:ident in)? $t:ident $(::<$($generics:ty),* $(,)?>)? {
$($fields:tt)*
}) => {
- $crate::try_init!(
+ $crate::__init_internal!(
@this($($this)?),
@typ($t $(::<$($generics),*>)?),
@fields($($fields)*),
@error($crate::error::Error),
+ @data(InitData, /*no use_data*/),
+ @has_data(HasInitData, __init_data),
+ @construct_closure(init_from_closure),
)
};
($(&$this:ident in)? $t:ident $(::<$($generics:ty),* $(,)?>)? {
$($fields:tt)*
}? $err:ty) => {
- $crate::try_init!(
+ $crate::__init_internal!(
@this($($this)?),
@typ($t $(::<$($generics),*>)?),
@fields($($fields)*),
@error($err),
+ @data(InitData, /*no use_data*/),
+ @has_data(HasInitData, __init_data),
+ @construct_closure(init_from_closure),
)
};
- (
- @this($($this:ident)?),
- @typ($t:ident $(::<$($generics:ty),*>)?),
- @fields($($fields:tt)*),
- @error($err:ty),
- ) => {{
- // We do not want to allow arbitrary returns, so we declare this type as the `Ok` return
- // type and shadow it later when we insert the arbitrary user code. That way there will be
- // no possibility of returning without `unsafe`.
- struct __InitOk;
- // Get the init data from the supplied type.
- let data = unsafe {
- use $crate::init::__internal::HasInitData;
- $t$(::<$($generics),*>)?::__init_data()
- };
- // Ensure that `data` really is of type `InitData` and help with type inference:
- let init = $crate::init::__internal::InitData::make_closure::<_, __InitOk, $err>(
- data,
- move |slot| {
- {
- // Shadow the structure so it cannot be used to return early.
- struct __InitOk;
- // Create the `this` so it can be referenced by the user inside of the
- // expressions creating the individual fields.
- $(let $this = unsafe { ::core::ptr::NonNull::new_unchecked(slot) };)?
- // Initialize every field.
- $crate::try_init!(init_slot:
- @slot(slot),
- @munch_fields($($fields)*,),
- );
- // We use unreachable code to ensure that all fields have been mentioned exactly
- // once, this struct initializer will still be type-checked and complain with a
- // very natural error message if a field is forgotten/mentioned more than once.
- #[allow(unreachable_code, clippy::diverging_sub_expression)]
- if false {
- $crate::try_init!(make_initializer:
- @slot(slot),
- @type_name($t),
- @munch_fields($($fields)*,),
- @acc(),
- );
- }
- // Forget all guards, since initialization was a success.
- $crate::try_init!(forget_guards:
- @munch_fields($($fields)*,),
- );
- }
- Ok(__InitOk)
- }
- );
- let init = move |slot| -> ::core::result::Result<(), $err> {
- init(slot).map(|__InitOk| ())
- };
- let init = unsafe { $crate::init::init_from_closure::<_, $err>(init) };
- init
- }};
- (init_slot:
- @slot($slot:ident),
- @munch_fields( $(,)?),
- ) => {
- // Endpoint of munching, no fields are left.
- };
- (init_slot:
- @slot($slot:ident),
- @munch_fields($field:ident <- $val:expr, $($rest:tt)*),
- ) => {
- let $field = $val;
- // Call the initializer.
- //
- // SAFETY: `slot` is valid, because we are inside of an initializer closure, we
- // return when an error/panic occurs.
- unsafe {
- $crate::init::Init::__init($field, ::core::ptr::addr_of_mut!((*$slot).$field))?;
- }
- // Create the drop guard.
- //
- // We only give access to `&DropGuard`, so it cannot be accidentally forgotten.
- //
- // SAFETY: We forget the guard later when initialization has succeeded.
- let $field = &unsafe {
- $crate::init::__internal::DropGuard::new(::core::ptr::addr_of_mut!((*$slot).$field))
- };
-
- $crate::try_init!(init_slot:
- @slot($slot),
- @munch_fields($($rest)*),
- );
- };
- (init_slot:
- @slot($slot:ident),
- // Direct value init.
- @munch_fields($field:ident $(: $val:expr)?, $($rest:tt)*),
- ) => {
- $(let $field = $val;)?
- // Call the initializer.
- //
- // SAFETY: The memory at `slot` is uninitialized.
- unsafe { ::core::ptr::write(::core::ptr::addr_of_mut!((*$slot).$field), $field) };
- // Create the drop guard.
- //
- // We only give access to `&DropGuard`, so it cannot be accidentally forgotten.
- //
- // SAFETY: We forget the guard later when initialization has succeeded.
- let $field = &unsafe {
- $crate::init::__internal::DropGuard::new(::core::ptr::addr_of_mut!((*$slot).$field))
- };
-
- $crate::try_init!(init_slot:
- @slot($slot),
- @munch_fields($($rest)*),
- );
- };
- (make_initializer:
- @slot($slot:ident),
- @type_name($t:ident),
- @munch_fields( $(,)?),
- @acc($($acc:tt)*),
- ) => {
- // Endpoint, nothing more to munch, create the initializer.
- // 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)*
- });
- }
- };
- (make_initializer:
- @slot($slot:ident),
- @type_name($t:ident),
- @munch_fields($field:ident <- $val:expr, $($rest:tt)*),
- @acc($($acc:tt)*),
- ) => {
- $crate::try_init!(make_initializer:
- @slot($slot),
- @type_name($t),
- @munch_fields($($rest)*),
- @acc($($acc)*$field: ::core::panic!(),),
- );
- };
- (make_initializer:
- @slot($slot:ident),
- @type_name($t:ident),
- @munch_fields($field:ident $(: $val:expr)?, $($rest:tt)*),
- @acc($($acc:tt)*),
- ) => {
- $crate::try_init!(make_initializer:
- @slot($slot),
- @type_name($t),
- @munch_fields($($rest)*),
- @acc($($acc)*$field: ::core::panic!(),),
- );
- };
- (forget_guards:
- @munch_fields($(,)?),
- ) => {
- // Munching finished.
- };
- (forget_guards:
- @munch_fields($field:ident <- $val:expr, $($rest:tt)*),
- ) => {
- unsafe { $crate::init::__internal::DropGuard::forget($field) };
-
- $crate::try_init!(forget_guards:
- @munch_fields($($rest)*),
- );
- };
- (forget_guards:
- @munch_fields($field:ident $(: $val:expr)?, $($rest:tt)*),
- ) => {
- unsafe { $crate::init::__internal::DropGuard::forget($field) };
-
- $crate::try_init!(forget_guards:
- @munch_fields($($rest)*),
- );
- };
}
/// A pin-initializer for the type `T`.
diff --git a/rust/kernel/init/macros.rs b/rust/kernel/init/macros.rs
index 00aa4e956c0a..fbaebd34f218 100644
--- a/rust/kernel/init/macros.rs
+++ b/rust/kernel/init/macros.rs
@@ -1,10 +1,12 @@
// SPDX-License-Identifier: Apache-2.0 OR MIT
//! This module provides the macros that actually implement the proc-macros `pin_data` and
-//! `pinned_drop`.
+//! `pinned_drop`. It also contains `__init_internal` the implementation of the `{try_}{pin_}init!`
+//! macros.
//!
//! These macros should never be called directly, since they expect their input to be
-//! in a certain format which is internal. Use the proc-macros instead.
+//! in a certain format which is internal. If used incorrectly, these macros can lead to UB even in
+//! safe code! Use the public facing macros instead.
//!
//! This architecture has been chosen because the kernel does not yet have access to `syn` which
//! would make matters a lot easier for implementing these as proc-macros.
@@ -980,3 +982,234 @@ impl<$($impl_generics)*> $pin_data<$($ty_generics)*>
}
};
}
+
+/// The internal init macro. Do not call manually!
+///
+/// This is called by the `{try_}{pin_}init!` macros with various inputs.
+///
+/// This macro has multiple internal call configurations, these are always the very first ident:
+/// - nothing: this is the base case and called by the `{try_}{pin_}init!` macros.
+/// - `init_slot`: recursively creates the code that initializes all fields in `slot`.
+/// - `make_initializer`: recursively create the struct initializer that guarantees that every
+/// field has been initialized exactly once.
+/// - `forget_guards`: recursively forget the drop guards for every field.
+#[doc(hidden)]
+#[macro_export]
+macro_rules! __init_internal {
+ (
+ @this($($this:ident)?),
+ @typ($t:ident $(::<$($generics:ty),*>)?),
+ @fields($($fields:tt)*),
+ @error($err:ty),
+ // Either `PinData` or `InitData`, `$use_data` should only be present in the `PinData`
+ // case.
+ @data($data:ident, $($use_data:ident)?),
+ // `HasPinData` or `HasInitData`.
+ @has_data($has_data:ident, $get_data:ident),
+ // `pin_init_from_closure` or `init_from_closure`.
+ @construct_closure($construct_closure:ident),
+ ) => {{
+ // We do not want to allow arbitrary returns, so we declare this type as the `Ok` return
+ // type and shadow it later when we insert the arbitrary user code. That way there will be
+ // no possibility of returning without `unsafe`.
+ struct __InitOk;
+ // Get the data about fields from the supplied type.
+ let data = unsafe {
+ use $crate::init::__internal::$has_data;
+ $t$(::<$($generics),*>)?::$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>(
+ data,
+ move |slot| {
+ {
+ // Shadow the structure so it cannot be used to return early.
+ struct __InitOk;
+ // Create the `this` so it can be referenced by the user inside of the
+ // expressions creating the individual fields.
+ $(let $this = unsafe { ::core::ptr::NonNull::new_unchecked(slot) };)?
+ // Initialize every field.
+ $crate::__init_internal!(init_slot($($use_data)?):
+ @data(data),
+ @slot(slot),
+ @munch_fields($($fields)*,),
+ );
+ // We use unreachable code to ensure that all fields have been mentioned exactly
+ // once, this struct initializer will still be type-checked and complain with a
+ // very natural error message if a field is forgotten/mentioned more than once.
+ #[allow(unreachable_code, clippy::diverging_sub_expression)]
+ if false {
+ $crate::__init_internal!(make_initializer:
+ @slot(slot),
+ @type_name($t),
+ @munch_fields($($fields)*,),
+ @acc(),
+ );
+ }
+ // Forget all guards, since initialization was a success.
+ $crate::__init_internal!(forget_guards:
+ @munch_fields($($fields)*,),
+ );
+ }
+ Ok(__InitOk)
+ }
+ );
+ let init = move |slot| -> ::core::result::Result<(), $err> {
+ init(slot).map(|__InitOk| ())
+ };
+ let init = unsafe { $crate::init::$construct_closure::<_, $err>(init) };
+ init
+ }};
+ (init_slot($($use_data:ident)?):
+ @data($data:ident),
+ @slot($slot:ident),
+ @munch_fields($(,)?),
+ ) => {
+ // Endpoint of munching, no fields are left.
+ };
+ (init_slot($use_data:ident): // use_data is present, so we use the `data` to init fields.
+ @data($data:ident),
+ @slot($slot:ident),
+ // In-place initialization syntax.
+ @munch_fields($field:ident <- $val:expr, $($rest:tt)*),
+ ) => {
+ let $field = $val;
+ // Call the initializer.
+ //
+ // SAFETY: `slot` is valid, because we are inside of an initializer closure, we
+ // return when an error/panic occurs.
+ // We also use the `data` to require the correct trait (`Init` or `PinInit`) for `$field`.
+ unsafe { $data.$field(::core::ptr::addr_of_mut!((*$slot).$field), $field)? };
+ // Create the drop guard.
+ //
+ // We only give access to `&DropGuard`, so it cannot be forgotten via safe code.
+ //
+ // SAFETY: We forget the guard later when initialization has succeeded.
+ let $field = &unsafe {
+ $crate::init::__internal::DropGuard::new(::core::ptr::addr_of_mut!((*$slot).$field))
+ };
+
+ $crate::__init_internal!(init_slot($use_data):
+ @data($data),
+ @slot($slot),
+ @munch_fields($($rest)*),
+ );
+ };
+ (init_slot(): // no use_data, so we use `Init::__init` directly.
+ @data($data:ident),
+ @slot($slot:ident),
+ // In-place initialization syntax.
+ @munch_fields($field:ident <- $val:expr, $($rest:tt)*),
+ ) => {
+ let $field = $val;
+ // Call the initializer.
+ //
+ // SAFETY: `slot` is valid, because we are inside of an initializer closure, we
+ // return when an error/panic occurs.
+ unsafe { $crate::init::Init::__init($field, ::core::ptr::addr_of_mut!((*$slot).$field))? };
+ // Create the drop guard.
+ //
+ // We only give access to `&DropGuard`, so it cannot be forgotten via safe code.
+ //
+ // SAFETY: We forget the guard later when initialization has succeeded.
+ let $field = &unsafe {
+ $crate::init::__internal::DropGuard::new(::core::ptr::addr_of_mut!((*$slot).$field))
+ };
+
+ $crate::__init_internal!(init_slot():
+ @data($data),
+ @slot($slot),
+ @munch_fields($($rest)*),
+ );
+ };
+ (init_slot($($use_data:ident)?):
+ @data($data:ident),
+ @slot($slot:ident),
+ // Init by-value.
+ @munch_fields($field:ident $(: $val:expr)?, $($rest:tt)*),
+ ) => {
+ $(let $field = $val;)?
+ // Initialize the field.
+ //
+ // SAFETY: The memory at `slot` is uninitialized.
+ unsafe { ::core::ptr::write(::core::ptr::addr_of_mut!((*$slot).$field), $field) };
+ // Create the drop guard:
+ //
+ // We only give access to `&DropGuard`, so it cannot be accidentally forgotten.
+ //
+ // SAFETY: We forget the guard later when initialization has succeeded.
+ let $field = &unsafe {
+ $crate::init::__internal::DropGuard::new(::core::ptr::addr_of_mut!((*$slot).$field))
+ };
+
+ $crate::__init_internal!(init_slot($($use_data)?):
+ @data($data),
+ @slot($slot),
+ @munch_fields($($rest)*),
+ );
+ };
+ (make_initializer:
+ @slot($slot:ident),
+ @type_name($t:ident),
+ @munch_fields($(,)?),
+ @acc($($acc:tt)*),
+ ) => {
+ // Endpoint, nothing more to munch, create the initializer.
+ // 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)*
+ });
+ }
+ };
+ (make_initializer:
+ @slot($slot:ident),
+ @type_name($t:ident),
+ @munch_fields($field:ident <- $val:expr, $($rest:tt)*),
+ @acc($($acc:tt)*),
+ ) => {
+ $crate::__init_internal!(make_initializer:
+ @slot($slot),
+ @type_name($t),
+ @munch_fields($($rest)*),
+ @acc($($acc)* $field: ::core::panic!(),),
+ );
+ };
+ (make_initializer:
+ @slot($slot:ident),
+ @type_name($t:ident),
+ @munch_fields($field:ident $(: $val:expr)?, $($rest:tt)*),
+ @acc($($acc:tt)*),
+ ) => {
+ $crate::__init_internal!(make_initializer:
+ @slot($slot),
+ @type_name($t),
+ @munch_fields($($rest)*),
+ @acc($($acc)* $field: ::core::panic!(),),
+ );
+ };
+ (forget_guards:
+ @munch_fields($(,)?),
+ ) => {
+ // Munching finished.
+ };
+ (forget_guards:
+ @munch_fields($field:ident <- $val:expr, $($rest:tt)*),
+ ) => {
+ unsafe { $crate::init::__internal::DropGuard::forget($field) };
+
+ $crate::__init_internal!(forget_guards:
+ @munch_fields($($rest)*),
+ );
+ };
+ (forget_guards:
+ @munch_fields($field:ident $(: $val:expr)?, $($rest:tt)*),
+ ) => {
+ unsafe { $crate::init::__internal::DropGuard::forget($field) };
+
+ $crate::__init_internal!(forget_guards:
+ @munch_fields($($rest)*),
+ );
+ };
+}
base-commit: d2e3115d717197cb2bc020dd1f06b06538474ac3
--
2.41.0
Add a derive proc-macro for the `Zeroable` trait. The macro supports
structs where every field implements the `Zeroable` trait. This way
`unsafe` implementations can be avoided.
The macro is split into two parts:
- a proc-macro to parse generics into impl and ty generics,
- a declarative macro that expands to the impl block.
Suggested-by: Asahi Lina <[email protected]>
Signed-off-by: Benno Lossin <[email protected]>
---
rust/kernel/init/macros.rs | 28 ++++++++++++++++++++++++++++
rust/kernel/prelude.rs | 2 +-
rust/macros/lib.rs | 20 ++++++++++++++++++++
rust/macros/quote.rs | 6 ++++++
rust/macros/zeroable.rs | 25 +++++++++++++++++++++++++
5 files changed, 80 insertions(+), 1 deletion(-)
create mode 100644 rust/macros/zeroable.rs
diff --git a/rust/kernel/init/macros.rs b/rust/kernel/init/macros.rs
index fbaebd34f218..e8165ff53a94 100644
--- a/rust/kernel/init/macros.rs
+++ b/rust/kernel/init/macros.rs
@@ -1213,3 +1213,31 @@ macro_rules! __init_internal {
);
};
}
+
+#[doc(hidden)]
+#[macro_export]
+macro_rules! __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: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
+ $($field_ty: $crate::Zeroable,)*
+ $($($whr)*)?
+ {}
+ };
+}
diff --git a/rust/kernel/prelude.rs b/rust/kernel/prelude.rs
index c28587d68ebc..ae21600970b3 100644
--- a/rust/kernel/prelude.rs
+++ b/rust/kernel/prelude.rs
@@ -18,7 +18,7 @@
pub use alloc::{boxed::Box, vec::Vec};
#[doc(no_inline)]
-pub use macros::{module, pin_data, pinned_drop, vtable};
+pub use macros::{module, pin_data, pinned_drop, vtable, Zeroable};
pub use super::build_assert;
diff --git a/rust/macros/lib.rs b/rust/macros/lib.rs
index 3fc74cb4ea19..9f056a5c780a 100644
--- a/rust/macros/lib.rs
+++ b/rust/macros/lib.rs
@@ -10,6 +10,7 @@
mod pin_data;
mod pinned_drop;
mod vtable;
+mod zeroable;
use proc_macro::TokenStream;
@@ -246,3 +247,22 @@ pub fn pin_data(inner: TokenStream, item: TokenStream) -> TokenStream {
pub fn pinned_drop(args: TokenStream, input: TokenStream) -> TokenStream {
pinned_drop::pinned_drop(args, input)
}
+
+/// Derives the [`Zeroable`] trait for the given struct.
+///
+/// This can only be used for structs where every field implements the [`Zeroable`] trait.
+///
+/// # Examples
+///
+/// ```rust
+/// #[derive(Zeroable)]
+/// pub struct DriverData {
+/// id: i64,
+/// buf_ptr: *mut u8,
+/// len: usize,
+/// }
+/// ```
+#[proc_macro_derive(Zeroable)]
+pub fn derive_zeroable(input: TokenStream) -> TokenStream {
+ zeroable::derive(input)
+}
diff --git a/rust/macros/quote.rs b/rust/macros/quote.rs
index dddbb4e6f4cb..b76c198a4ed5 100644
--- a/rust/macros/quote.rs
+++ b/rust/macros/quote.rs
@@ -124,6 +124,12 @@ macro_rules! quote_spanned {
));
quote_spanned!(@proc $v $span $($tt)*);
};
+ (@proc $v:ident $span:ident ; $($tt:tt)*) => {
+ $v.push(::proc_macro::TokenTree::Punct(
+ ::proc_macro::Punct::new(';', ::proc_macro::Spacing::Alone)
+ ));
+ quote_spanned!(@proc $v $span $($tt)*);
+ };
(@proc $v:ident $span:ident $id:ident $($tt:tt)*) => {
$v.push(::proc_macro::TokenTree::Ident(::proc_macro::Ident::new(stringify!($id), $span)));
quote_spanned!(@proc $v $span $($tt)*);
diff --git a/rust/macros/zeroable.rs b/rust/macros/zeroable.rs
new file mode 100644
index 000000000000..cddb866c44ef
--- /dev/null
+++ b/rust/macros/zeroable.rs
@@ -0,0 +1,25 @@
+// SPDX-License-Identifier: GPL-2.0
+
+use crate::helpers::{parse_generics, Generics};
+use proc_macro::TokenStream;
+
+pub(crate) fn derive(input: TokenStream) -> TokenStream {
+ let (
+ Generics {
+ impl_generics,
+ ty_generics,
+ },
+ mut rest,
+ ) = parse_generics(input);
+ // This should be the body of the struct `{...}`.
+ let last = rest.pop();
+ quote! {
+ ::kernel::__derive_zeroable!(
+ parse_input:
+ @sig(#(#rest)*),
+ @impl_generics(#(#impl_generics)*),
+ @ty_generics(#(#ty_generics)*),
+ @body(#last),
+ );
+ }
+}
--
2.41.0
Add the struct update syntax to the init macros, but only for
`..Zeroable::zeroed()`. Adding this at the end of the struct initializer
allows one to omit fields from the initializer, these fields will be
initialized with 0x00 set to every byte. Only types that implement the
`Zeroable` trait can utilize this.
Suggested-by: Asahi Lina <[email protected]>
Signed-off-by: Benno Lossin <[email protected]>
---
rust/kernel/init.rs | 16 +++++-
rust/kernel/init/macros.rs | 114 ++++++++++++++++++++++++++++++++++++-
2 files changed, 128 insertions(+), 2 deletions(-)
diff --git a/rust/kernel/init.rs b/rust/kernel/init.rs
index ecf6a4bd0ce4..44bc3e77419a 100644
--- a/rust/kernel/init.rs
+++ b/rust/kernel/init.rs
@@ -508,14 +508,18 @@ macro_rules! stack_try_pin_init {
/// - Fields that you want to initialize in-place have to use `<-` instead of `:`.
/// - In front of the initializer you can write `&this in` to have access to a [`NonNull<Self>`]
/// pointer named `this` inside of the initializer.
+/// - Using struct update syntax one can place `..Zeroable::zeroed()` at the very end of the
+/// struct, this initializes every field with 0 and then runs all initializers specified in the
+/// body. This can only be done if [`Zeroable`] is implemented for the struct.
///
/// For instance:
///
/// ```rust
/// # use kernel::pin_init;
-/// # use macros::pin_data;
+/// # use macros::{pin_data, Zeroable};
/// # use core::{ptr::addr_of_mut, marker::PhantomPinned};
/// #[pin_data]
+/// #[derive(Zeroable)]
/// struct Buf {
/// // `ptr` points into `buf`.
/// ptr: *mut u8,
@@ -528,6 +532,10 @@ macro_rules! stack_try_pin_init {
/// ptr: unsafe { addr_of_mut!((*this.as_ptr()).buf).cast() },
/// pin: PhantomPinned,
/// });
+/// pin_init!(Buf {
+/// buf: [1; 64],
+/// ..Zeroable::zeroed(),
+/// });
/// ```
///
/// [`try_pin_init!`]: kernel::try_pin_init
@@ -547,6 +555,7 @@ macro_rules! pin_init {
@data(PinData, use_data),
@has_data(HasPinData, __pin_data),
@construct_closure(pin_init_from_closure),
+ @munch_fields($($fields)*),
)
};
}
@@ -603,6 +612,7 @@ macro_rules! try_pin_init {
@data(PinData, use_data),
@has_data(HasPinData, __pin_data),
@construct_closure(pin_init_from_closure),
+ @munch_fields($($fields)*),
)
};
($(&$this:ident in)? $t:ident $(::<$($generics:ty),* $(,)?>)? {
@@ -616,6 +626,7 @@ macro_rules! try_pin_init {
@data(PinData, use_data),
@has_data(HasPinData, __pin_data),
@construct_closure(pin_init_from_closure),
+ @munch_fields($($fields)*),
)
};
}
@@ -650,6 +661,7 @@ macro_rules! init {
@data(InitData, /*no use_data*/),
@has_data(HasInitData, __init_data),
@construct_closure(init_from_closure),
+ @munch_fields($($fields)*),
)
}
}
@@ -700,6 +712,7 @@ macro_rules! try_init {
@data(InitData, /*no use_data*/),
@has_data(HasInitData, __init_data),
@construct_closure(init_from_closure),
+ @munch_fields($($fields)*),
)
};
($(&$this:ident in)? $t:ident $(::<$($generics:ty),* $(,)?>)? {
@@ -713,6 +726,7 @@ macro_rules! try_init {
@data(InitData, /*no use_data*/),
@has_data(HasInitData, __init_data),
@construct_closure(init_from_closure),
+ @munch_fields($($fields)*),
)
};
}
diff --git a/rust/kernel/init/macros.rs b/rust/kernel/init/macros.rs
index 1e0c4aca055a..5dcb2e513f26 100644
--- a/rust/kernel/init/macros.rs
+++ b/rust/kernel/init/macros.rs
@@ -989,6 +989,7 @@ impl<$($impl_generics)*> $pin_data<$($ty_generics)*>
///
/// This macro has multiple internal call configurations, these are always the very first ident:
/// - nothing: this is the base case and called by the `{try_}{pin_}init!` macros.
+/// - `with_update_parsed`: when the `..Zeroable::zeroed()` syntax has been handled.
/// - `init_slot`: recursively creates the code that initializes all fields in `slot`.
/// - `make_initializer`: recursively create the struct initializer that guarantees that every
/// field has been initialized exactly once.
@@ -1007,6 +1008,82 @@ macro_rules! __init_internal {
@has_data($has_data:ident, $get_data:ident),
// `pin_init_from_closure` or `init_from_closure`.
@construct_closure($construct_closure:ident),
+ @munch_fields(),
+ ) => {
+ $crate::__init_internal!(with_update_parsed:
+ @this($($this)?),
+ @typ($t $(::<$($generics),*>)? ),
+ @fields($($fields)*),
+ @error($err),
+ @data($data, $($use_data)?),
+ @has_data($has_data, $get_data),
+ @construct_closure($construct_closure),
+ @zeroed(), // nothing means default behavior.
+ )
+ };
+ (
+ @this($($this:ident)?),
+ @typ($t:ident $(::<$($generics:ty),*>)?),
+ @fields($($fields:tt)*),
+ @error($err:ty),
+ // Either `PinData` or `InitData`, `$use_data` should only be present in the `PinData`
+ // case.
+ @data($data:ident, $($use_data:ident)?),
+ // `HasPinData` or `HasInitData`.
+ @has_data($has_data:ident, $get_data:ident),
+ // `pin_init_from_closure` or `init_from_closure`.
+ @construct_closure($construct_closure:ident),
+ @munch_fields(..Zeroable::zeroed()),
+ ) => {
+ $crate::__init_internal!(with_update_parsed:
+ @this($($this)?),
+ @typ($t $(::<$($generics),*>)? ),
+ @fields($($fields)*),
+ @error($err),
+ @data($data, $($use_data)?),
+ @has_data($has_data, $get_data),
+ @construct_closure($construct_closure),
+ @zeroed(()), // `()` means zero all fields not mentioned.
+ )
+ };
+ (
+ @this($($this:ident)?),
+ @typ($t:ident $(::<$($generics:ty),*>)?),
+ @fields($($fields:tt)*),
+ @error($err:ty),
+ // Either `PinData` or `InitData`, `$use_data` should only be present in the `PinData`
+ // case.
+ @data($data:ident, $($use_data:ident)?),
+ // `HasPinData` or `HasInitData`.
+ @has_data($has_data:ident, $get_data:ident),
+ // `pin_init_from_closure` or `init_from_closure`.
+ @construct_closure($construct_closure:ident),
+ @munch_fields($ignore:tt $($rest:tt)*),
+ ) => {
+ $crate::__init_internal!(
+ @this($($this)?),
+ @typ($t $(::<$($generics),*>)? ),
+ @fields($($fields)*),
+ @error($err),
+ @data($data, $($use_data)?),
+ @has_data($has_data, $get_data),
+ @construct_closure($construct_closure),
+ @munch_fields($($rest)*),
+ )
+ };
+ (with_update_parsed:
+ @this($($this:ident)?),
+ @typ($t:ident $(::<$($generics:ty),*>)?),
+ @fields($($fields:tt)*),
+ @error($err:ty),
+ // Either `PinData` or `InitData`, `$use_data` should only be present in the `PinData`
+ // case.
+ @data($data:ident, $($use_data:ident)?),
+ // `HasPinData` or `HasInitData`.
+ @has_data($has_data:ident, $get_data:ident),
+ // `pin_init_from_closure` or `init_from_closure`.
+ @construct_closure($construct_closure:ident),
+ @zeroed($($init_zeroed:expr)?),
) => {{
// We do not want to allow arbitrary returns, so we declare this type as the `Ok` return
// type and shadow it later when we insert the arbitrary user code. That way there will be
@@ -1024,6 +1101,17 @@ macro_rules! __init_internal {
{
// Shadow the structure so it cannot be used to return early.
struct __InitOk;
+ // If `$init_zeroed` is present we should zero the slot now and not emit an
+ // error when fields are missing (since they will be zeroed). We also have to
+ // check that the type actually implements `Zeroable`.
+ $(
+ fn is_zeroable<T: Zeroable>(ptr: *mut T) {}
+ // Ensure that the struct is indeed `Zeroable`.
+ is_zeroable(slot);
+ // SAFETY: The type implements `Zeroable` by the check above.
+ unsafe { ::core::ptr::write_bytes(slot, 0, 1) };
+ $init_zeroed // this will be `()` if set.
+ )?
// Create the `this` so it can be referenced by the user inside of the
// expressions creating the individual fields.
$(let $this = unsafe { ::core::ptr::NonNull::new_unchecked(slot) };)?
@@ -1064,7 +1152,7 @@ macro_rules! __init_internal {
@data($data:ident),
@slot($slot:ident),
@guards($($guards:ident,)*),
- @munch_fields($(,)?),
+ @munch_fields($(..Zeroable::zeroed())? $(,)?),
) => {
// Endpoint of munching, no fields are left. If execution reaches this point, all fields
// have been initialized. Therefore we can now dismiss the guards by forgetting them.
@@ -1157,6 +1245,30 @@ macro_rules! __init_internal {
@munch_fields($($rest)*),
);
};
+ (make_initializer:
+ @slot($slot:ident),
+ @type_name($t:ident),
+ @munch_fields(..Zeroable::zeroed() $(,)?),
+ @acc($($acc:tt)*),
+ ) => {
+ // Endpoint, nothing more to munch, create the initializer. Since the users specified
+ // `..Zeroable::zeroed()`, the slot will already have been zeroed and all field that have
+ // not been overwritten are thus zero and initialized. We still check that all fields are
+ // actually accessible by using the struct update syntax ourselves.
+ // Since we are in the `if false` branch, this will never get executed. We abuse `slot` to
+ // get the correct type inference here:
+ unsafe {
+ let mut zeroed = ::core::mem::zeroed();
+ // We have to use type inference her to make zeroed have the correct type. This does
+ // not get executed, so it has no effect.
+ ::core::ptr::write($slot, zeroed);
+ zeroed = ::core::mem::zeroed();
+ ::core::ptr::write($slot, $t {
+ $($acc)*
+ ..zeroed
+ });
+ }
+ };
(make_initializer:
@slot($slot:ident),
@type_name($t:ident),
--
2.41.0
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.
Suggested-by: Asahi Lina <[email protected]>
Signed-off-by: Benno Lossin <[email protected]>
---
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
Add two functions `pin_init_array_from_fn` and `init_array_from_fn` that
take a function that generates initializers for `T` from usize, the added
functions then return an initializer for `[T; N]` where every element is
initialized by an element returned from the generator function.
Suggested-by: Asahi Lina <[email protected]>
Signed-off-by: Benno Lossin <[email protected]>
---
rust/kernel/init.rs | 90 +++++++++++++++++++++++++++++++++++++++++++++
1 file changed, 90 insertions(+)
diff --git a/rust/kernel/init.rs b/rust/kernel/init.rs
index 44bc3e77419a..c9ea4bf71987 100644
--- a/rust/kernel/init.rs
+++ b/rust/kernel/init.rs
@@ -867,6 +867,96 @@ pub fn uninit<T, E>() -> impl Init<MaybeUninit<T>, E> {
unsafe { init_from_closure(|_| Ok(())) }
}
+/// Initializes an array by initializing each element via the provided initializer.
+///
+/// # Examples
+///
+/// ```rust
+/// let array: Box<[usize; 1000_000_000]>= Box::init(init_array_from_fn(|i| i)).unwrap();
+/// println!("{array:?}");
+/// ```
+pub fn init_array_from_fn<I, const N: usize, T, E>(
+ mut make_init: impl FnMut(usize) -> I,
+) -> impl Init<[T; N], E>
+where
+ I: Init<T, E>,
+{
+ let init = move |slot: *mut [T; N]| {
+ let slot = slot.cast::<T>();
+ for i in 0..N {
+ let init = make_init(i);
+ // SAFETY: since 0 <= `i` < N, it is still in bounds of `[T; N]`.
+ let ptr = unsafe { slot.add(i) };
+ // SAFETY: The pointer is derived from `slot` and thus satisfies the `__init`
+ // requirements.
+ match unsafe { init.__init(ptr) } {
+ Ok(()) => {}
+ Err(e) => {
+ // We now free every element that has been initialized before:
+ for j in 0..i {
+ let ptr = unsafe { slot.add(j) };
+ // SAFETY: The value was initialized in a previous iteration of the loop
+ // and since we return `Err` below, the caller will consider the memory at
+ // `slot` as uninitialized.
+ unsafe { ptr::drop_in_place(ptr) };
+ }
+ return Err(e);
+ }
+ }
+ }
+ Ok(())
+ };
+ // SAFETY: The initializer above initializes every element of the array. On failure it drops
+ // any initialized elements and returns `Err`.
+ unsafe { init_from_closure(init) }
+}
+
+/// Initializes an array by initializing each element via the provided initializer.
+///
+/// # Examples
+///
+/// ```rust
+/// let array: Arc<[Mutex<usize>; 1000_000_000]>=
+/// Arc::pin_init(init_array_from_fn(|i| new_mutex!(i))).unwrap();
+/// println!("{array:?}");
+/// ```
+pub fn pin_init_array_from_fn<I, const N: usize, T, E>(
+ mut make_init: impl FnMut(usize) -> I,
+) -> impl PinInit<[T; N], E>
+where
+ I: PinInit<T, E>,
+{
+ let init = move |slot: *mut [T; N]| {
+ let slot = slot.cast::<T>();
+ for i in 0..N {
+ let init = make_init(i);
+ // SAFETY: since 0 <= `i` < N, it is still in bounds of `[T; N]`.
+ let ptr = unsafe { slot.add(i) };
+ // SAFETY: The pointer is derived from `slot` and thus satisfies the `__pinned_init`
+ // requirements.
+ match unsafe { init.__pinned_init(ptr) } {
+ Ok(()) => {}
+ Err(e) => {
+ // We now have to free every element that has been initialized before, since we
+ // have to abide by the drop guarantee.
+ for j in 0..i {
+ let ptr = unsafe { slot.add(j) };
+ // SAFETY: The value was initialized in a previous iteration of the loop
+ // and since we return `Err` below, the caller will consider the memory at
+ // `slot` as uninitialized.
+ unsafe { ptr::drop_in_place(ptr) };
+ }
+ return Err(e);
+ }
+ }
+ }
+ Ok(())
+ };
+ // SAFETY: The initializer above initializes every element of the array. On failure it drops
+ // any initialized elements and returns `Err`.
+ unsafe { pin_init_from_closure(init) }
+}
+
// SAFETY: Every type can be initialized by-value.
unsafe impl<T, E> Init<T, E> for T {
unsafe fn __init(self, slot: *mut T) -> Result<(), E> {
--
2.41.0
Use hygienic identifiers for the guards instead of the field names. This
makes the init macros feel more like normal struct initializers, since
assigning identifiers with the name of a field does not create
conflicts.
Also change the internals of the guards, no need to make the `forget`
function `unsafe`, since users cannot access the guards anyways. Now the
guards are carried directly on the stack and have no extra `Cell<bool>`
field that marks if they have been forgotten or not, instead they are
just forgotten via `mem::forget`.
Suggested-by: Asahi Lina <[email protected]>
Signed-off-by: Benno Lossin <[email protected]>
---
rust/kernel/init.rs | 1 -
rust/kernel/init/__internal.rs | 25 +++------------
rust/kernel/init/macros.rs | 56 ++++++++++++----------------------
3 files changed, 23 insertions(+), 59 deletions(-)
diff --git a/rust/kernel/init.rs b/rust/kernel/init.rs
index d9a91950cba2..ecf6a4bd0ce4 100644
--- a/rust/kernel/init.rs
+++ b/rust/kernel/init.rs
@@ -206,7 +206,6 @@
use alloc::boxed::Box;
use core::{
alloc::AllocError,
- cell::Cell,
convert::Infallible,
marker::PhantomData,
mem::MaybeUninit,
diff --git a/rust/kernel/init/__internal.rs b/rust/kernel/init/__internal.rs
index 44751fb62b51..7abd1fb65e41 100644
--- a/rust/kernel/init/__internal.rs
+++ b/rust/kernel/init/__internal.rs
@@ -174,7 +174,6 @@ pub fn init<E>(self: Pin<&mut Self>, init: impl PinInit<T, E>) -> Result<Pin<&mu
/// Can be forgotten to prevent the drop.
pub struct DropGuard<T: ?Sized> {
ptr: *mut T,
- do_drop: Cell<bool>,
}
impl<T: ?Sized> DropGuard<T> {
@@ -190,32 +189,16 @@ impl<T: ?Sized> DropGuard<T> {
/// - will not be dropped by any other means.
#[inline]
pub unsafe fn new(ptr: *mut T) -> Self {
- Self {
- ptr,
- do_drop: Cell::new(true),
- }
- }
-
- /// Prevents this guard from dropping the supplied pointer.
- ///
- /// # Safety
- ///
- /// This function is unsafe in order to prevent safe code from forgetting this guard. It should
- /// only be called by the macros in this module.
- #[inline]
- pub unsafe fn forget(&self) {
- self.do_drop.set(false);
+ Self { ptr }
}
}
impl<T: ?Sized> Drop for DropGuard<T> {
#[inline]
fn drop(&mut self) {
- if self.do_drop.get() {
- // SAFETY: A `DropGuard` can only be constructed using the unsafe `new` function
- // ensuring that this operation is safe.
- unsafe { ptr::drop_in_place(self.ptr) }
- }
+ // SAFETY: A `DropGuard` can only be constructed using the unsafe `new` function
+ // ensuring that this operation is safe.
+ unsafe { ptr::drop_in_place(self.ptr) }
}
}
diff --git a/rust/kernel/init/macros.rs b/rust/kernel/init/macros.rs
index e8165ff53a94..df4281743175 100644
--- a/rust/kernel/init/macros.rs
+++ b/rust/kernel/init/macros.rs
@@ -992,7 +992,6 @@ impl<$($impl_generics)*> $pin_data<$($ty_generics)*>
/// - `init_slot`: recursively creates the code that initializes all fields in `slot`.
/// - `make_initializer`: recursively create the struct initializer that guarantees that every
/// field has been initialized exactly once.
-/// - `forget_guards`: recursively forget the drop guards for every field.
#[doc(hidden)]
#[macro_export]
macro_rules! __init_internal {
@@ -1032,6 +1031,7 @@ macro_rules! __init_internal {
$crate::__init_internal!(init_slot($($use_data)?):
@data(data),
@slot(slot),
+ @guards(),
@munch_fields($($fields)*,),
);
// We use unreachable code to ensure that all fields have been mentioned exactly
@@ -1046,10 +1046,6 @@ macro_rules! __init_internal {
@acc(),
);
}
- // Forget all guards, since initialization was a success.
- $crate::__init_internal!(forget_guards:
- @munch_fields($($fields)*,),
- );
}
Ok(__InitOk)
}
@@ -1063,13 +1059,17 @@ macro_rules! __init_internal {
(init_slot($($use_data:ident)?):
@data($data:ident),
@slot($slot:ident),
+ @guards($($guards:ident,)*),
@munch_fields($(,)?),
) => {
- // Endpoint of munching, no fields are left.
+ // Endpoint of munching, no fields are left. If execution reaches this point, all fields
+ // have been initialized. Therefore we can now dismiss the guards by forgetting them.
+ $(::core::mem::forget($guards);)*
};
(init_slot($use_data:ident): // use_data is present, so we use the `data` to init fields.
@data($data:ident),
@slot($slot:ident),
+ @guards($($guards:ident,)*),
// In-place initialization syntax.
@munch_fields($field:ident <- $val:expr, $($rest:tt)*),
) => {
@@ -1080,24 +1080,26 @@ macro_rules! __init_internal {
// return when an error/panic occurs.
// We also use the `data` to require the correct trait (`Init` or `PinInit`) for `$field`.
unsafe { $data.$field(::core::ptr::addr_of_mut!((*$slot).$field), $field)? };
- // Create the drop guard.
+ // Create the drop guard:
//
- // We only give access to `&DropGuard`, so it cannot be forgotten via safe code.
+ // We rely on macro hygiene to make it impossible for users to access this local variable.
//
// SAFETY: We forget the guard later when initialization has succeeded.
- let $field = &unsafe {
+ let guard = unsafe {
$crate::init::__internal::DropGuard::new(::core::ptr::addr_of_mut!((*$slot).$field))
};
$crate::__init_internal!(init_slot($use_data):
@data($data),
@slot($slot),
+ @guards(guard, $($guards,)*),
@munch_fields($($rest)*),
);
};
(init_slot(): // no use_data, so we use `Init::__init` directly.
@data($data:ident),
@slot($slot:ident),
+ @guards($($guards:ident,)*),
// In-place initialization syntax.
@munch_fields($field:ident <- $val:expr, $($rest:tt)*),
) => {
@@ -1107,24 +1109,26 @@ macro_rules! __init_internal {
// SAFETY: `slot` is valid, because we are inside of an initializer closure, we
// return when an error/panic occurs.
unsafe { $crate::init::Init::__init($field, ::core::ptr::addr_of_mut!((*$slot).$field))? };
- // Create the drop guard.
+ // Create the drop guard:
//
- // We only give access to `&DropGuard`, so it cannot be forgotten via safe code.
+ // We rely on macro hygiene to make it impossible for users to access this local variable.
//
// SAFETY: We forget the guard later when initialization has succeeded.
- let $field = &unsafe {
+ let guard = unsafe {
$crate::init::__internal::DropGuard::new(::core::ptr::addr_of_mut!((*$slot).$field))
};
$crate::__init_internal!(init_slot():
@data($data),
@slot($slot),
+ @guards(guard, $($guards,)*),
@munch_fields($($rest)*),
);
};
(init_slot($($use_data:ident)?):
@data($data:ident),
@slot($slot:ident),
+ @guards($($guards:ident,)*),
// Init by-value.
@munch_fields($field:ident $(: $val:expr)?, $($rest:tt)*),
) => {
@@ -1135,16 +1139,17 @@ macro_rules! __init_internal {
unsafe { ::core::ptr::write(::core::ptr::addr_of_mut!((*$slot).$field), $field) };
// Create the drop guard:
//
- // We only give access to `&DropGuard`, so it cannot be accidentally forgotten.
+ // We rely on macro hygiene to make it impossible for users to access this local variable.
//
// SAFETY: We forget the guard later when initialization has succeeded.
- let $field = &unsafe {
+ let guard = unsafe {
$crate::init::__internal::DropGuard::new(::core::ptr::addr_of_mut!((*$slot).$field))
};
$crate::__init_internal!(init_slot($($use_data)?):
@data($data),
@slot($slot),
+ @guards(guard, $($guards,)*),
@munch_fields($($rest)*),
);
};
@@ -1189,29 +1194,6 @@ macro_rules! __init_internal {
@acc($($acc)* $field: ::core::panic!(),),
);
};
- (forget_guards:
- @munch_fields($(,)?),
- ) => {
- // Munching finished.
- };
- (forget_guards:
- @munch_fields($field:ident <- $val:expr, $($rest:tt)*),
- ) => {
- unsafe { $crate::init::__internal::DropGuard::forget($field) };
-
- $crate::__init_internal!(forget_guards:
- @munch_fields($($rest)*),
- );
- };
- (forget_guards:
- @munch_fields($field:ident $(: $val:expr)?, $($rest:tt)*),
- ) => {
- unsafe { $crate::init::__internal::DropGuard::forget($field) };
-
- $crate::__init_internal!(forget_guards:
- @munch_fields($($rest)*),
- );
- };
}
#[doc(hidden)]
--
2.41.0
In the implementation of the init macros there is a `if false` statement
that type checks the initializer to ensure every field is initialized.
Since the next patch has a stack variable to store the struct, the
function might allocate too much memory on debug builds. Putting the
struct into a closure ensures that even in debug builds no stack
overflow error is caused. In release builds this was not a problem since
the code was optimized away due to the `if false`.
Signed-off-by: Benno Lossin <[email protected]>
---
rust/kernel/init/macros.rs | 18 +++++++++++-------
1 file changed, 11 insertions(+), 7 deletions(-)
diff --git a/rust/kernel/init/macros.rs b/rust/kernel/init/macros.rs
index df4281743175..1e0c4aca055a 100644
--- a/rust/kernel/init/macros.rs
+++ b/rust/kernel/init/macros.rs
@@ -1037,14 +1037,18 @@ macro_rules! __init_internal {
// We use unreachable code to ensure that all fields have been mentioned exactly
// once, this struct initializer will still be type-checked and complain with a
// very natural error message if a field is forgotten/mentioned more than once.
- #[allow(unreachable_code, clippy::diverging_sub_expression)]
+ #[allow(unreachable_code,
+ clippy::diverging_sub_expression,
+ clippy::redundant_closure_call)]
if false {
- $crate::__init_internal!(make_initializer:
- @slot(slot),
- @type_name($t),
- @munch_fields($($fields)*,),
- @acc(),
- );
+ (|| {
+ $crate::__init_internal!(make_initializer:
+ @slot(slot),
+ @type_name($t),
+ @munch_fields($($fields)*,),
+ @acc(),
+ );
+ })();
}
}
Ok(__InitOk)
--
2.41.0
On Saturday, June 24th, 2023 at 11:25, Benno Lossin <[email protected]> wrote:
> Add a derive proc-macro for the `Zeroable` trait. The macro supports
> structs where every field implements the `Zeroable` trait. This way
> `unsafe` implementations can be avoided.
>
> The macro is split into two parts:
> - a proc-macro to parse generics into impl and ty generics,
> - a declarative macro that expands to the impl block.
>
> Suggested-by: Asahi Lina <[email protected]>
> Signed-off-by: Benno Lossin <[email protected]>
Reviewed-by: Björn Roy Baron <[email protected]>
> ---
> rust/kernel/init/macros.rs | 28 ++++++++++++++++++++++++++++
> rust/kernel/prelude.rs | 2 +-
> rust/macros/lib.rs | 20 ++++++++++++++++++++
> rust/macros/quote.rs | 6 ++++++
> rust/macros/zeroable.rs | 25 +++++++++++++++++++++++++
> 5 files changed, 80 insertions(+), 1 deletion(-)
> create mode 100644 rust/macros/zeroable.rs
>
> diff --git a/rust/kernel/init/macros.rs b/rust/kernel/init/macros.rs
> index fbaebd34f218..e8165ff53a94 100644
> --- a/rust/kernel/init/macros.rs
> +++ b/rust/kernel/init/macros.rs
> @@ -1213,3 +1213,31 @@ macro_rules! __init_internal {
> );
> };
> }
> +
> +#[doc(hidden)]
> +#[macro_export]
> +macro_rules! __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: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
> + $($field_ty: $crate::Zeroable,)*
> + $($($whr)*)?
> + {}
> + };
> +}
> diff --git a/rust/kernel/prelude.rs b/rust/kernel/prelude.rs
> index c28587d68ebc..ae21600970b3 100644
> --- a/rust/kernel/prelude.rs
> +++ b/rust/kernel/prelude.rs
> @@ -18,7 +18,7 @@
> pub use alloc::{boxed::Box, vec::Vec};
>
> #[doc(no_inline)]
> -pub use macros::{module, pin_data, pinned_drop, vtable};
> +pub use macros::{module, pin_data, pinned_drop, vtable, Zeroable};
>
> pub use super::build_assert;
>
> diff --git a/rust/macros/lib.rs b/rust/macros/lib.rs
> index 3fc74cb4ea19..9f056a5c780a 100644
> --- a/rust/macros/lib.rs
> +++ b/rust/macros/lib.rs
> @@ -10,6 +10,7 @@
> mod pin_data;
> mod pinned_drop;
> mod vtable;
> +mod zeroable;
>
> use proc_macro::TokenStream;
>
> @@ -246,3 +247,22 @@ pub fn pin_data(inner: TokenStream, item: TokenStream) -> TokenStream {
> pub fn pinned_drop(args: TokenStream, input: TokenStream) -> TokenStream {
> pinned_drop::pinned_drop(args, input)
> }
> +
> +/// Derives the [`Zeroable`] trait for the given struct.
> +///
> +/// This can only be used for structs where every field implements the [`Zeroable`] trait.
> +///
> +/// # Examples
> +///
> +/// ```rust
> +/// #[derive(Zeroable)]
> +/// pub struct DriverData {
> +/// id: i64,
> +/// buf_ptr: *mut u8,
> +/// len: usize,
> +/// }
> +/// ```
> +#[proc_macro_derive(Zeroable)]
> +pub fn derive_zeroable(input: TokenStream) -> TokenStream {
> + zeroable::derive(input)
> +}
> diff --git a/rust/macros/quote.rs b/rust/macros/quote.rs
> index dddbb4e6f4cb..b76c198a4ed5 100644
> --- a/rust/macros/quote.rs
> +++ b/rust/macros/quote.rs
> @@ -124,6 +124,12 @@ macro_rules! quote_spanned {
> ));
> quote_spanned!(@proc $v $span $($tt)*);
> };
> + (@proc $v:ident $span:ident ; $($tt:tt)*) => {
> + $v.push(::proc_macro::TokenTree::Punct(
> + ::proc_macro::Punct::new(';', ::proc_macro::Spacing::Alone)
> + ));
> + quote_spanned!(@proc $v $span $($tt)*);
> + };
> (@proc $v:ident $span:ident $id:ident $($tt:tt)*) => {
> $v.push(::proc_macro::TokenTree::Ident(::proc_macro::Ident::new(stringify!($id), $span)));
> quote_spanned!(@proc $v $span $($tt)*);
> diff --git a/rust/macros/zeroable.rs b/rust/macros/zeroable.rs
> new file mode 100644
> index 000000000000..cddb866c44ef
> --- /dev/null
> +++ b/rust/macros/zeroable.rs
> @@ -0,0 +1,25 @@
> +// SPDX-License-Identifier: GPL-2.0
> +
> +use crate::helpers::{parse_generics, Generics};
> +use proc_macro::TokenStream;
> +
> +pub(crate) fn derive(input: TokenStream) -> TokenStream {
> + let (
> + Generics {
> + impl_generics,
> + ty_generics,
> + },
> + mut rest,
> + ) = parse_generics(input);
> + // This should be the body of the struct `{...}`.
> + let last = rest.pop();
> + quote! {
> + ::kernel::__derive_zeroable!(
> + parse_input:
> + @sig(#(#rest)*),
> + @impl_generics(#(#impl_generics)*),
> + @ty_generics(#(#ty_generics)*),
> + @body(#last),
> + );
> + }
> +}
> --
> 2.41.0
On Saturday, June 24th, 2023 at 11:25, Benno Lossin <[email protected]> wrote:
> In the implementation of the init macros there is a `if false` statement
> that type checks the initializer to ensure every field is initialized.
> Since the next patch has a stack variable to store the struct, the
> function might allocate too much memory on debug builds. Putting the
> struct into a closure ensures that even in debug builds no stack
> overflow error is caused. In release builds this was not a problem since
> the code was optimized away due to the `if false`.
>
> Signed-off-by: Benno Lossin <[email protected]>
> ---
> rust/kernel/init/macros.rs | 18 +++++++++++-------
> 1 file changed, 11 insertions(+), 7 deletions(-)
>
> diff --git a/rust/kernel/init/macros.rs b/rust/kernel/init/macros.rs
> index df4281743175..1e0c4aca055a 100644
> --- a/rust/kernel/init/macros.rs
> +++ b/rust/kernel/init/macros.rs
> @@ -1037,14 +1037,18 @@ macro_rules! __init_internal {
> // We use unreachable code to ensure that all fields have been mentioned exactly
> // once, this struct initializer will still be type-checked and complain with a
> // very natural error message if a field is forgotten/mentioned more than once.
> - #[allow(unreachable_code, clippy::diverging_sub_expression)]
> + #[allow(unreachable_code,
> + clippy::diverging_sub_expression,
> + clippy::redundant_closure_call)]
> if false {
> - $crate::__init_internal!(make_initializer:
> - @slot(slot),
> - @type_name($t),
> - @munch_fields($($fields)*,),
> - @acc(),
> - );
> + (|| {
> + $crate::__init_internal!(make_initializer:
> + @slot(slot),
> + @type_name($t),
> + @munch_fields($($fields)*,),
> + @acc(),
> + );
> + })();
Is it necessary to call this closure? Typechecking of the closure should happen even without calling it.
> }
> }
> Ok(__InitOk)
> --
> 2.41.0
Cheers,
Björn
On Saturday, June 24th, 2023 at 11:25, Benno Lossin <[email protected]> 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.
>
> Suggested-by: Asahi Lina <[email protected]>
> Signed-off-by: Benno Lossin <[email protected]>
Reviewed-by: Björn Roy Baron <[email protected]>
> ---
> 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
On Saturday, June 24th, 2023 at 11:24, Benno Lossin <[email protected]> wrote:
> Merges the implementations of `try_init!` and `try_pin_init!`. These two
> macros are very similar, but use different traits. The new macro
> `__init_internal!` that is now the implementation for both takes these
> traits as parameters.
>
> This change does not affect any users, as no public API has been
> changed, but it should simplify maintaining the init macros.
>
> Signed-off-by: Benno Lossin <[email protected]>
A bit hard to review due to the large blocks of code that were moved, but git show --color-moved didn't show anything weird. Nice to see less code duplication in any case.
Reviewed-by: Björn Roy Baron <[email protected]>
> ---
> rust/kernel/init.rs | 388 +++----------------------------------
> rust/kernel/init/macros.rs | 237 +++++++++++++++++++++-
> 2 files changed, 259 insertions(+), 366 deletions(-)
>
> diff --git a/rust/kernel/init.rs b/rust/kernel/init.rs
> index b4332a4ec1f4..d9a91950cba2 100644
> --- a/rust/kernel/init.rs
> +++ b/rust/kernel/init.rs
> @@ -540,11 +540,14 @@ macro_rules! pin_init {
> ($(&$this:ident in)? $t:ident $(::<$($generics:ty),* $(,)?>)? {
> $($fields:tt)*
> }) => {
> - $crate::try_pin_init!(
> + $crate::__init_internal!(
> @this($($this)?),
> @typ($t $(::<$($generics),*>)?),
> @fields($($fields)*),
> @error(::core::convert::Infallible),
> + @data(PinData, use_data),
> + @has_data(HasPinData, __pin_data),
> + @construct_closure(pin_init_from_closure),
> )
> };
> }
> @@ -593,205 +596,29 @@ macro_rules! try_pin_init {
> ($(&$this:ident in)? $t:ident $(::<$($generics:ty),* $(,)?>)? {
> $($fields:tt)*
> }) => {
> - $crate::try_pin_init!(
> + $crate::__init_internal!(
> @this($($this)?),
> @typ($t $(::<$($generics),*>)? ),
> @fields($($fields)*),
> @error($crate::error::Error),
> + @data(PinData, use_data),
> + @has_data(HasPinData, __pin_data),
> + @construct_closure(pin_init_from_closure),
> )
> };
> ($(&$this:ident in)? $t:ident $(::<$($generics:ty),* $(,)?>)? {
> $($fields:tt)*
> }? $err:ty) => {
> - $crate::try_pin_init!(
> + $crate::__init_internal!(
> @this($($this)?),
> @typ($t $(::<$($generics),*>)? ),
> @fields($($fields)*),
> @error($err),
> + @data(PinData, use_data),
> + @has_data(HasPinData, __pin_data),
> + @construct_closure(pin_init_from_closure),
> )
> };
> - (
> - @this($($this:ident)?),
> - @typ($t:ident $(::<$($generics:ty),*>)?),
> - @fields($($fields:tt)*),
> - @error($err:ty),
> - ) => {{
> - // We do not want to allow arbitrary returns, so we declare this type as the `Ok` return
> - // type and shadow it later when we insert the arbitrary user code. That way there will be
> - // no possibility of returning without `unsafe`.
> - struct __InitOk;
> - // Get the pin data from the supplied type.
> - let data = unsafe {
> - use $crate::init::__internal::HasPinData;
> - $t$(::<$($generics),*>)?::__pin_data()
> - };
> - // Ensure that `data` really is of type `PinData` and help with type inference:
> - let init = $crate::init::__internal::PinData::make_closure::<_, __InitOk, $err>(
> - data,
> - move |slot| {
> - {
> - // Shadow the structure so it cannot be used to return early.
> - struct __InitOk;
> - // Create the `this` so it can be referenced by the user inside of the
> - // expressions creating the individual fields.
> - $(let $this = unsafe { ::core::ptr::NonNull::new_unchecked(slot) };)?
> - // Initialize every field.
> - $crate::try_pin_init!(init_slot:
> - @data(data),
> - @slot(slot),
> - @munch_fields($($fields)*,),
> - );
> - // We use unreachable code to ensure that all fields have been mentioned exactly
> - // once, this struct initializer will still be type-checked and complain with a
> - // very natural error message if a field is forgotten/mentioned more than once.
> - #[allow(unreachable_code, clippy::diverging_sub_expression)]
> - if false {
> - $crate::try_pin_init!(make_initializer:
> - @slot(slot),
> - @type_name($t),
> - @munch_fields($($fields)*,),
> - @acc(),
> - );
> - }
> - // Forget all guards, since initialization was a success.
> - $crate::try_pin_init!(forget_guards:
> - @munch_fields($($fields)*,),
> - );
> - }
> - Ok(__InitOk)
> - }
> - );
> - let init = move |slot| -> ::core::result::Result<(), $err> {
> - init(slot).map(|__InitOk| ())
> - };
> - let init = unsafe { $crate::init::pin_init_from_closure::<_, $err>(init) };
> - init
> - }};
> - (init_slot:
> - @data($data:ident),
> - @slot($slot:ident),
> - @munch_fields($(,)?),
> - ) => {
> - // Endpoint of munching, no fields are left.
> - };
> - (init_slot:
> - @data($data:ident),
> - @slot($slot:ident),
> - // In-place initialization syntax.
> - @munch_fields($field:ident <- $val:expr, $($rest:tt)*),
> - ) => {
> - let $field = $val;
> - // Call the initializer.
> - //
> - // SAFETY: `slot` is valid, because we are inside of an initializer closure, we
> - // return when an error/panic occurs.
> - // We also use the `data` to require the correct trait (`Init` or `PinInit`) for `$field`.
> - unsafe { $data.$field(::core::ptr::addr_of_mut!((*$slot).$field), $field)? };
> - // Create the drop guard.
> - //
> - // We only give access to `&DropGuard`, so it cannot be forgotten via safe code.
> - //
> - // SAFETY: We forget the guard later when initialization has succeeded.
> - let $field = &unsafe {
> - $crate::init::__internal::DropGuard::new(::core::ptr::addr_of_mut!((*$slot).$field))
> - };
> -
> - $crate::try_pin_init!(init_slot:
> - @data($data),
> - @slot($slot),
> - @munch_fields($($rest)*),
> - );
> - };
> - (init_slot:
> - @data($data:ident),
> - @slot($slot:ident),
> - // Direct value init, this is safe for every field.
> - @munch_fields($field:ident $(: $val:expr)?, $($rest:tt)*),
> - ) => {
> - $(let $field = $val;)?
> - // Initialize the field.
> - //
> - // SAFETY: The memory at `slot` is uninitialized.
> - unsafe { ::core::ptr::write(::core::ptr::addr_of_mut!((*$slot).$field), $field) };
> - // Create the drop guard:
> - //
> - // We only give access to `&DropGuard`, so it cannot be accidentally forgotten.
> - //
> - // SAFETY: We forget the guard later when initialization has succeeded.
> - let $field = &unsafe {
> - $crate::init::__internal::DropGuard::new(::core::ptr::addr_of_mut!((*$slot).$field))
> - };
> -
> - $crate::try_pin_init!(init_slot:
> - @data($data),
> - @slot($slot),
> - @munch_fields($($rest)*),
> - );
> - };
> - (make_initializer:
> - @slot($slot:ident),
> - @type_name($t:ident),
> - @munch_fields($(,)?),
> - @acc($($acc:tt)*),
> - ) => {
> - // Endpoint, nothing more to munch, create the initializer.
> - // 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)*
> - });
> - }
> - };
> - (make_initializer:
> - @slot($slot:ident),
> - @type_name($t:ident),
> - @munch_fields($field:ident <- $val:expr, $($rest:tt)*),
> - @acc($($acc:tt)*),
> - ) => {
> - $crate::try_pin_init!(make_initializer:
> - @slot($slot),
> - @type_name($t),
> - @munch_fields($($rest)*),
> - @acc($($acc)* $field: ::core::panic!(),),
> - );
> - };
> - (make_initializer:
> - @slot($slot:ident),
> - @type_name($t:ident),
> - @munch_fields($field:ident $(: $val:expr)?, $($rest:tt)*),
> - @acc($($acc:tt)*),
> - ) => {
> - $crate::try_pin_init!(make_initializer:
> - @slot($slot),
> - @type_name($t),
> - @munch_fields($($rest)*),
> - @acc($($acc)* $field: ::core::panic!(),),
> - );
> - };
> - (forget_guards:
> - @munch_fields($(,)?),
> - ) => {
> - // Munching finished.
> - };
> - (forget_guards:
> - @munch_fields($field:ident <- $val:expr, $($rest:tt)*),
> - ) => {
> - unsafe { $crate::init::__internal::DropGuard::forget($field) };
> -
> - $crate::try_pin_init!(forget_guards:
> - @munch_fields($($rest)*),
> - );
> - };
> - (forget_guards:
> - @munch_fields($field:ident $(: $val:expr)?, $($rest:tt)*),
> - ) => {
> - unsafe { $crate::init::__internal::DropGuard::forget($field) };
> -
> - $crate::try_pin_init!(forget_guards:
> - @munch_fields($($rest)*),
> - );
> - };
> }
>
> /// Construct an in-place initializer for `struct`s.
> @@ -816,11 +643,14 @@ macro_rules! init {
> ($(&$this:ident in)? $t:ident $(::<$($generics:ty),* $(,)?>)? {
> $($fields:tt)*
> }) => {
> - $crate::try_init!(
> + $crate::__init_internal!(
> @this($($this)?),
> @typ($t $(::<$($generics),*>)?),
> @fields($($fields)*),
> @error(::core::convert::Infallible),
> + @data(InitData, /*no use_data*/),
> + @has_data(HasInitData, __init_data),
> + @construct_closure(init_from_closure),
> )
> }
> }
> @@ -863,199 +693,29 @@ macro_rules! try_init {
> ($(&$this:ident in)? $t:ident $(::<$($generics:ty),* $(,)?>)? {
> $($fields:tt)*
> }) => {
> - $crate::try_init!(
> + $crate::__init_internal!(
> @this($($this)?),
> @typ($t $(::<$($generics),*>)?),
> @fields($($fields)*),
> @error($crate::error::Error),
> + @data(InitData, /*no use_data*/),
> + @has_data(HasInitData, __init_data),
> + @construct_closure(init_from_closure),
> )
> };
> ($(&$this:ident in)? $t:ident $(::<$($generics:ty),* $(,)?>)? {
> $($fields:tt)*
> }? $err:ty) => {
> - $crate::try_init!(
> + $crate::__init_internal!(
> @this($($this)?),
> @typ($t $(::<$($generics),*>)?),
> @fields($($fields)*),
> @error($err),
> + @data(InitData, /*no use_data*/),
> + @has_data(HasInitData, __init_data),
> + @construct_closure(init_from_closure),
> )
> };
> - (
> - @this($($this:ident)?),
> - @typ($t:ident $(::<$($generics:ty),*>)?),
> - @fields($($fields:tt)*),
> - @error($err:ty),
> - ) => {{
> - // We do not want to allow arbitrary returns, so we declare this type as the `Ok` return
> - // type and shadow it later when we insert the arbitrary user code. That way there will be
> - // no possibility of returning without `unsafe`.
> - struct __InitOk;
> - // Get the init data from the supplied type.
> - let data = unsafe {
> - use $crate::init::__internal::HasInitData;
> - $t$(::<$($generics),*>)?::__init_data()
> - };
> - // Ensure that `data` really is of type `InitData` and help with type inference:
> - let init = $crate::init::__internal::InitData::make_closure::<_, __InitOk, $err>(
> - data,
> - move |slot| {
> - {
> - // Shadow the structure so it cannot be used to return early.
> - struct __InitOk;
> - // Create the `this` so it can be referenced by the user inside of the
> - // expressions creating the individual fields.
> - $(let $this = unsafe { ::core::ptr::NonNull::new_unchecked(slot) };)?
> - // Initialize every field.
> - $crate::try_init!(init_slot:
> - @slot(slot),
> - @munch_fields($($fields)*,),
> - );
> - // We use unreachable code to ensure that all fields have been mentioned exactly
> - // once, this struct initializer will still be type-checked and complain with a
> - // very natural error message if a field is forgotten/mentioned more than once.
> - #[allow(unreachable_code, clippy::diverging_sub_expression)]
> - if false {
> - $crate::try_init!(make_initializer:
> - @slot(slot),
> - @type_name($t),
> - @munch_fields($($fields)*,),
> - @acc(),
> - );
> - }
> - // Forget all guards, since initialization was a success.
> - $crate::try_init!(forget_guards:
> - @munch_fields($($fields)*,),
> - );
> - }
> - Ok(__InitOk)
> - }
> - );
> - let init = move |slot| -> ::core::result::Result<(), $err> {
> - init(slot).map(|__InitOk| ())
> - };
> - let init = unsafe { $crate::init::init_from_closure::<_, $err>(init) };
> - init
> - }};
> - (init_slot:
> - @slot($slot:ident),
> - @munch_fields( $(,)?),
> - ) => {
> - // Endpoint of munching, no fields are left.
> - };
> - (init_slot:
> - @slot($slot:ident),
> - @munch_fields($field:ident <- $val:expr, $($rest:tt)*),
> - ) => {
> - let $field = $val;
> - // Call the initializer.
> - //
> - // SAFETY: `slot` is valid, because we are inside of an initializer closure, we
> - // return when an error/panic occurs.
> - unsafe {
> - $crate::init::Init::__init($field, ::core::ptr::addr_of_mut!((*$slot).$field))?;
> - }
> - // Create the drop guard.
> - //
> - // We only give access to `&DropGuard`, so it cannot be accidentally forgotten.
> - //
> - // SAFETY: We forget the guard later when initialization has succeeded.
> - let $field = &unsafe {
> - $crate::init::__internal::DropGuard::new(::core::ptr::addr_of_mut!((*$slot).$field))
> - };
> -
> - $crate::try_init!(init_slot:
> - @slot($slot),
> - @munch_fields($($rest)*),
> - );
> - };
> - (init_slot:
> - @slot($slot:ident),
> - // Direct value init.
> - @munch_fields($field:ident $(: $val:expr)?, $($rest:tt)*),
> - ) => {
> - $(let $field = $val;)?
> - // Call the initializer.
> - //
> - // SAFETY: The memory at `slot` is uninitialized.
> - unsafe { ::core::ptr::write(::core::ptr::addr_of_mut!((*$slot).$field), $field) };
> - // Create the drop guard.
> - //
> - // We only give access to `&DropGuard`, so it cannot be accidentally forgotten.
> - //
> - // SAFETY: We forget the guard later when initialization has succeeded.
> - let $field = &unsafe {
> - $crate::init::__internal::DropGuard::new(::core::ptr::addr_of_mut!((*$slot).$field))
> - };
> -
> - $crate::try_init!(init_slot:
> - @slot($slot),
> - @munch_fields($($rest)*),
> - );
> - };
> - (make_initializer:
> - @slot($slot:ident),
> - @type_name($t:ident),
> - @munch_fields( $(,)?),
> - @acc($($acc:tt)*),
> - ) => {
> - // Endpoint, nothing more to munch, create the initializer.
> - // 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)*
> - });
> - }
> - };
> - (make_initializer:
> - @slot($slot:ident),
> - @type_name($t:ident),
> - @munch_fields($field:ident <- $val:expr, $($rest:tt)*),
> - @acc($($acc:tt)*),
> - ) => {
> - $crate::try_init!(make_initializer:
> - @slot($slot),
> - @type_name($t),
> - @munch_fields($($rest)*),
> - @acc($($acc)*$field: ::core::panic!(),),
> - );
> - };
> - (make_initializer:
> - @slot($slot:ident),
> - @type_name($t:ident),
> - @munch_fields($field:ident $(: $val:expr)?, $($rest:tt)*),
> - @acc($($acc:tt)*),
> - ) => {
> - $crate::try_init!(make_initializer:
> - @slot($slot),
> - @type_name($t),
> - @munch_fields($($rest)*),
> - @acc($($acc)*$field: ::core::panic!(),),
> - );
> - };
> - (forget_guards:
> - @munch_fields($(,)?),
> - ) => {
> - // Munching finished.
> - };
> - (forget_guards:
> - @munch_fields($field:ident <- $val:expr, $($rest:tt)*),
> - ) => {
> - unsafe { $crate::init::__internal::DropGuard::forget($field) };
> -
> - $crate::try_init!(forget_guards:
> - @munch_fields($($rest)*),
> - );
> - };
> - (forget_guards:
> - @munch_fields($field:ident $(: $val:expr)?, $($rest:tt)*),
> - ) => {
> - unsafe { $crate::init::__internal::DropGuard::forget($field) };
> -
> - $crate::try_init!(forget_guards:
> - @munch_fields($($rest)*),
> - );
> - };
> }
>
> /// A pin-initializer for the type `T`.
> diff --git a/rust/kernel/init/macros.rs b/rust/kernel/init/macros.rs
> index 00aa4e956c0a..fbaebd34f218 100644
> --- a/rust/kernel/init/macros.rs
> +++ b/rust/kernel/init/macros.rs
> @@ -1,10 +1,12 @@
> // SPDX-License-Identifier: Apache-2.0 OR MIT
>
> //! This module provides the macros that actually implement the proc-macros `pin_data` and
> -//! `pinned_drop`.
> +//! `pinned_drop`. It also contains `__init_internal` the implementation of the `{try_}{pin_}init!`
> +//! macros.
> //!
> //! These macros should never be called directly, since they expect their input to be
> -//! in a certain format which is internal. Use the proc-macros instead.
> +//! in a certain format which is internal. If used incorrectly, these macros can lead to UB even in
> +//! safe code! Use the public facing macros instead.
> //!
> //! This architecture has been chosen because the kernel does not yet have access to `syn` which
> //! would make matters a lot easier for implementing these as proc-macros.
> @@ -980,3 +982,234 @@ impl<$($impl_generics)*> $pin_data<$($ty_generics)*>
> }
> };
> }
> +
> +/// The internal init macro. Do not call manually!
> +///
> +/// This is called by the `{try_}{pin_}init!` macros with various inputs.
> +///
> +/// This macro has multiple internal call configurations, these are always the very first ident:
> +/// - nothing: this is the base case and called by the `{try_}{pin_}init!` macros.
> +/// - `init_slot`: recursively creates the code that initializes all fields in `slot`.
> +/// - `make_initializer`: recursively create the struct initializer that guarantees that every
> +/// field has been initialized exactly once.
> +/// - `forget_guards`: recursively forget the drop guards for every field.
> +#[doc(hidden)]
> +#[macro_export]
> +macro_rules! __init_internal {
> + (
> + @this($($this:ident)?),
> + @typ($t:ident $(::<$($generics:ty),*>)?),
> + @fields($($fields:tt)*),
> + @error($err:ty),
> + // Either `PinData` or `InitData`, `$use_data` should only be present in the `PinData`
> + // case.
> + @data($data:ident, $($use_data:ident)?),
> + // `HasPinData` or `HasInitData`.
> + @has_data($has_data:ident, $get_data:ident),
> + // `pin_init_from_closure` or `init_from_closure`.
> + @construct_closure($construct_closure:ident),
> + ) => {{
> + // We do not want to allow arbitrary returns, so we declare this type as the `Ok` return
> + // type and shadow it later when we insert the arbitrary user code. That way there will be
> + // no possibility of returning without `unsafe`.
> + struct __InitOk;
> + // Get the data about fields from the supplied type.
> + let data = unsafe {
> + use $crate::init::__internal::$has_data;
> + $t$(::<$($generics),*>)?::$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>(
> + data,
> + move |slot| {
> + {
> + // Shadow the structure so it cannot be used to return early.
> + struct __InitOk;
> + // Create the `this` so it can be referenced by the user inside of the
> + // expressions creating the individual fields.
> + $(let $this = unsafe { ::core::ptr::NonNull::new_unchecked(slot) };)?
> + // Initialize every field.
> + $crate::__init_internal!(init_slot($($use_data)?):
> + @data(data),
> + @slot(slot),
> + @munch_fields($($fields)*,),
> + );
> + // We use unreachable code to ensure that all fields have been mentioned exactly
> + // once, this struct initializer will still be type-checked and complain with a
> + // very natural error message if a field is forgotten/mentioned more than once.
> + #[allow(unreachable_code, clippy::diverging_sub_expression)]
> + if false {
> + $crate::__init_internal!(make_initializer:
> + @slot(slot),
> + @type_name($t),
> + @munch_fields($($fields)*,),
> + @acc(),
> + );
> + }
> + // Forget all guards, since initialization was a success.
> + $crate::__init_internal!(forget_guards:
> + @munch_fields($($fields)*,),
> + );
> + }
> + Ok(__InitOk)
> + }
> + );
> + let init = move |slot| -> ::core::result::Result<(), $err> {
> + init(slot).map(|__InitOk| ())
> + };
> + let init = unsafe { $crate::init::$construct_closure::<_, $err>(init) };
> + init
> + }};
> + (init_slot($($use_data:ident)?):
> + @data($data:ident),
> + @slot($slot:ident),
> + @munch_fields($(,)?),
> + ) => {
> + // Endpoint of munching, no fields are left.
> + };
> + (init_slot($use_data:ident): // use_data is present, so we use the `data` to init fields.
> + @data($data:ident),
> + @slot($slot:ident),
> + // In-place initialization syntax.
> + @munch_fields($field:ident <- $val:expr, $($rest:tt)*),
> + ) => {
> + let $field = $val;
> + // Call the initializer.
> + //
> + // SAFETY: `slot` is valid, because we are inside of an initializer closure, we
> + // return when an error/panic occurs.
> + // We also use the `data` to require the correct trait (`Init` or `PinInit`) for `$field`.
> + unsafe { $data.$field(::core::ptr::addr_of_mut!((*$slot).$field), $field)? };
> + // Create the drop guard.
> + //
> + // We only give access to `&DropGuard`, so it cannot be forgotten via safe code.
> + //
> + // SAFETY: We forget the guard later when initialization has succeeded.
> + let $field = &unsafe {
> + $crate::init::__internal::DropGuard::new(::core::ptr::addr_of_mut!((*$slot).$field))
> + };
> +
> + $crate::__init_internal!(init_slot($use_data):
> + @data($data),
> + @slot($slot),
> + @munch_fields($($rest)*),
> + );
> + };
> + (init_slot(): // no use_data, so we use `Init::__init` directly.
> + @data($data:ident),
> + @slot($slot:ident),
> + // In-place initialization syntax.
> + @munch_fields($field:ident <- $val:expr, $($rest:tt)*),
> + ) => {
> + let $field = $val;
> + // Call the initializer.
> + //
> + // SAFETY: `slot` is valid, because we are inside of an initializer closure, we
> + // return when an error/panic occurs.
> + unsafe { $crate::init::Init::__init($field, ::core::ptr::addr_of_mut!((*$slot).$field))? };
> + // Create the drop guard.
> + //
> + // We only give access to `&DropGuard`, so it cannot be forgotten via safe code.
> + //
> + // SAFETY: We forget the guard later when initialization has succeeded.
> + let $field = &unsafe {
> + $crate::init::__internal::DropGuard::new(::core::ptr::addr_of_mut!((*$slot).$field))
> + };
> +
> + $crate::__init_internal!(init_slot():
> + @data($data),
> + @slot($slot),
> + @munch_fields($($rest)*),
> + );
> + };
> + (init_slot($($use_data:ident)?):
> + @data($data:ident),
> + @slot($slot:ident),
> + // Init by-value.
> + @munch_fields($field:ident $(: $val:expr)?, $($rest:tt)*),
> + ) => {
> + $(let $field = $val;)?
> + // Initialize the field.
> + //
> + // SAFETY: The memory at `slot` is uninitialized.
> + unsafe { ::core::ptr::write(::core::ptr::addr_of_mut!((*$slot).$field), $field) };
> + // Create the drop guard:
> + //
> + // We only give access to `&DropGuard`, so it cannot be accidentally forgotten.
> + //
> + // SAFETY: We forget the guard later when initialization has succeeded.
> + let $field = &unsafe {
> + $crate::init::__internal::DropGuard::new(::core::ptr::addr_of_mut!((*$slot).$field))
> + };
> +
> + $crate::__init_internal!(init_slot($($use_data)?):
> + @data($data),
> + @slot($slot),
> + @munch_fields($($rest)*),
> + );
> + };
> + (make_initializer:
> + @slot($slot:ident),
> + @type_name($t:ident),
> + @munch_fields($(,)?),
> + @acc($($acc:tt)*),
> + ) => {
> + // Endpoint, nothing more to munch, create the initializer.
> + // 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)*
> + });
> + }
> + };
> + (make_initializer:
> + @slot($slot:ident),
> + @type_name($t:ident),
> + @munch_fields($field:ident <- $val:expr, $($rest:tt)*),
> + @acc($($acc:tt)*),
> + ) => {
> + $crate::__init_internal!(make_initializer:
> + @slot($slot),
> + @type_name($t),
> + @munch_fields($($rest)*),
> + @acc($($acc)* $field: ::core::panic!(),),
> + );
> + };
> + (make_initializer:
> + @slot($slot:ident),
> + @type_name($t:ident),
> + @munch_fields($field:ident $(: $val:expr)?, $($rest:tt)*),
> + @acc($($acc:tt)*),
> + ) => {
> + $crate::__init_internal!(make_initializer:
> + @slot($slot),
> + @type_name($t),
> + @munch_fields($($rest)*),
> + @acc($($acc)* $field: ::core::panic!(),),
> + );
> + };
> + (forget_guards:
> + @munch_fields($(,)?),
> + ) => {
> + // Munching finished.
> + };
> + (forget_guards:
> + @munch_fields($field:ident <- $val:expr, $($rest:tt)*),
> + ) => {
> + unsafe { $crate::init::__internal::DropGuard::forget($field) };
> +
> + $crate::__init_internal!(forget_guards:
> + @munch_fields($($rest)*),
> + );
> + };
> + (forget_guards:
> + @munch_fields($field:ident $(: $val:expr)?, $($rest:tt)*),
> + ) => {
> + unsafe { $crate::init::__internal::DropGuard::forget($field) };
> +
> + $crate::__init_internal!(forget_guards:
> + @munch_fields($($rest)*),
> + );
> + };
> +}
>
> base-commit: d2e3115d717197cb2bc020dd1f06b06538474ac3
> --
> 2.41.0
On Saturday, June 24th, 2023 at 11:25, Benno Lossin <[email protected]> wrote:
> Use hygienic identifiers for the guards instead of the field names. This
> makes the init macros feel more like normal struct initializers, since
> assigning identifiers with the name of a field does not create
> conflicts.
> Also change the internals of the guards, no need to make the `forget`
> function `unsafe`, since users cannot access the guards anyways. Now the
> guards are carried directly on the stack and have no extra `Cell<bool>`
> field that marks if they have been forgotten or not, instead they are
> just forgotten via `mem::forget`.
>
> Suggested-by: Asahi Lina <[email protected]>
> Signed-off-by: Benno Lossin <[email protected]>
Reviewed-by: Björn Roy Baron <[email protected]>
> ---
> rust/kernel/init.rs | 1 -
> rust/kernel/init/__internal.rs | 25 +++------------
> rust/kernel/init/macros.rs | 56 ++++++++++++----------------------
> 3 files changed, 23 insertions(+), 59 deletions(-)
>
> diff --git a/rust/kernel/init.rs b/rust/kernel/init.rs
> index d9a91950cba2..ecf6a4bd0ce4 100644
> --- a/rust/kernel/init.rs
> +++ b/rust/kernel/init.rs
> @@ -206,7 +206,6 @@
> use alloc::boxed::Box;
> use core::{
> alloc::AllocError,
> - cell::Cell,
> convert::Infallible,
> marker::PhantomData,
> mem::MaybeUninit,
> diff --git a/rust/kernel/init/__internal.rs b/rust/kernel/init/__internal.rs
> index 44751fb62b51..7abd1fb65e41 100644
> --- a/rust/kernel/init/__internal.rs
> +++ b/rust/kernel/init/__internal.rs
> @@ -174,7 +174,6 @@ pub fn init<E>(self: Pin<&mut Self>, init: impl PinInit<T, E>) -> Result<Pin<&mu
> /// Can be forgotten to prevent the drop.
> pub struct DropGuard<T: ?Sized> {
> ptr: *mut T,
> - do_drop: Cell<bool>,
> }
>
> impl<T: ?Sized> DropGuard<T> {
> @@ -190,32 +189,16 @@ impl<T: ?Sized> DropGuard<T> {
> /// - will not be dropped by any other means.
> #[inline]
> pub unsafe fn new(ptr: *mut T) -> Self {
> - Self {
> - ptr,
> - do_drop: Cell::new(true),
> - }
> - }
> -
> - /// Prevents this guard from dropping the supplied pointer.
> - ///
> - /// # Safety
> - ///
> - /// This function is unsafe in order to prevent safe code from forgetting this guard. It should
> - /// only be called by the macros in this module.
> - #[inline]
> - pub unsafe fn forget(&self) {
> - self.do_drop.set(false);
> + Self { ptr }
> }
> }
>
> impl<T: ?Sized> Drop for DropGuard<T> {
> #[inline]
> fn drop(&mut self) {
> - if self.do_drop.get() {
> - // SAFETY: A `DropGuard` can only be constructed using the unsafe `new` function
> - // ensuring that this operation is safe.
> - unsafe { ptr::drop_in_place(self.ptr) }
> - }
> + // SAFETY: A `DropGuard` can only be constructed using the unsafe `new` function
> + // ensuring that this operation is safe.
> + unsafe { ptr::drop_in_place(self.ptr) }
> }
> }
>
> diff --git a/rust/kernel/init/macros.rs b/rust/kernel/init/macros.rs
> index e8165ff53a94..df4281743175 100644
> --- a/rust/kernel/init/macros.rs
> +++ b/rust/kernel/init/macros.rs
> @@ -992,7 +992,6 @@ impl<$($impl_generics)*> $pin_data<$($ty_generics)*>
> /// - `init_slot`: recursively creates the code that initializes all fields in `slot`.
> /// - `make_initializer`: recursively create the struct initializer that guarantees that every
> /// field has been initialized exactly once.
> -/// - `forget_guards`: recursively forget the drop guards for every field.
> #[doc(hidden)]
> #[macro_export]
> macro_rules! __init_internal {
> @@ -1032,6 +1031,7 @@ macro_rules! __init_internal {
> $crate::__init_internal!(init_slot($($use_data)?):
> @data(data),
> @slot(slot),
> + @guards(),
> @munch_fields($($fields)*,),
> );
> // We use unreachable code to ensure that all fields have been mentioned exactly
> @@ -1046,10 +1046,6 @@ macro_rules! __init_internal {
> @acc(),
> );
> }
> - // Forget all guards, since initialization was a success.
> - $crate::__init_internal!(forget_guards:
> - @munch_fields($($fields)*,),
> - );
> }
> Ok(__InitOk)
> }
> @@ -1063,13 +1059,17 @@ macro_rules! __init_internal {
> (init_slot($($use_data:ident)?):
> @data($data:ident),
> @slot($slot:ident),
> + @guards($($guards:ident,)*),
> @munch_fields($(,)?),
> ) => {
> - // Endpoint of munching, no fields are left.
> + // Endpoint of munching, no fields are left. If execution reaches this point, all fields
> + // have been initialized. Therefore we can now dismiss the guards by forgetting them.
> + $(::core::mem::forget($guards);)*
> };
> (init_slot($use_data:ident): // use_data is present, so we use the `data` to init fields.
> @data($data:ident),
> @slot($slot:ident),
> + @guards($($guards:ident,)*),
> // In-place initialization syntax.
> @munch_fields($field:ident <- $val:expr, $($rest:tt)*),
> ) => {
> @@ -1080,24 +1080,26 @@ macro_rules! __init_internal {
> // return when an error/panic occurs.
> // We also use the `data` to require the correct trait (`Init` or `PinInit`) for `$field`.
> unsafe { $data.$field(::core::ptr::addr_of_mut!((*$slot).$field), $field)? };
> - // Create the drop guard.
> + // Create the drop guard:
> //
> - // We only give access to `&DropGuard`, so it cannot be forgotten via safe code.
> + // We rely on macro hygiene to make it impossible for users to access this local variable.
> //
> // SAFETY: We forget the guard later when initialization has succeeded.
> - let $field = &unsafe {
> + let guard = unsafe {
> $crate::init::__internal::DropGuard::new(::core::ptr::addr_of_mut!((*$slot).$field))
> };
>
> $crate::__init_internal!(init_slot($use_data):
> @data($data),
> @slot($slot),
> + @guards(guard, $($guards,)*),
> @munch_fields($($rest)*),
> );
> };
> (init_slot(): // no use_data, so we use `Init::__init` directly.
> @data($data:ident),
> @slot($slot:ident),
> + @guards($($guards:ident,)*),
> // In-place initialization syntax.
> @munch_fields($field:ident <- $val:expr, $($rest:tt)*),
> ) => {
> @@ -1107,24 +1109,26 @@ macro_rules! __init_internal {
> // SAFETY: `slot` is valid, because we are inside of an initializer closure, we
> // return when an error/panic occurs.
> unsafe { $crate::init::Init::__init($field, ::core::ptr::addr_of_mut!((*$slot).$field))? };
> - // Create the drop guard.
> + // Create the drop guard:
> //
> - // We only give access to `&DropGuard`, so it cannot be forgotten via safe code.
> + // We rely on macro hygiene to make it impossible for users to access this local variable.
> //
> // SAFETY: We forget the guard later when initialization has succeeded.
> - let $field = &unsafe {
> + let guard = unsafe {
> $crate::init::__internal::DropGuard::new(::core::ptr::addr_of_mut!((*$slot).$field))
> };
>
> $crate::__init_internal!(init_slot():
> @data($data),
> @slot($slot),
> + @guards(guard, $($guards,)*),
> @munch_fields($($rest)*),
> );
> };
> (init_slot($($use_data:ident)?):
> @data($data:ident),
> @slot($slot:ident),
> + @guards($($guards:ident,)*),
> // Init by-value.
> @munch_fields($field:ident $(: $val:expr)?, $($rest:tt)*),
> ) => {
> @@ -1135,16 +1139,17 @@ macro_rules! __init_internal {
> unsafe { ::core::ptr::write(::core::ptr::addr_of_mut!((*$slot).$field), $field) };
> // Create the drop guard:
> //
> - // We only give access to `&DropGuard`, so it cannot be accidentally forgotten.
> + // We rely on macro hygiene to make it impossible for users to access this local variable.
> //
> // SAFETY: We forget the guard later when initialization has succeeded.
> - let $field = &unsafe {
> + let guard = unsafe {
> $crate::init::__internal::DropGuard::new(::core::ptr::addr_of_mut!((*$slot).$field))
> };
>
> $crate::__init_internal!(init_slot($($use_data)?):
> @data($data),
> @slot($slot),
> + @guards(guard, $($guards,)*),
> @munch_fields($($rest)*),
> );
> };
> @@ -1189,29 +1194,6 @@ macro_rules! __init_internal {
> @acc($($acc)* $field: ::core::panic!(),),
> );
> };
> - (forget_guards:
> - @munch_fields($(,)?),
> - ) => {
> - // Munching finished.
> - };
> - (forget_guards:
> - @munch_fields($field:ident <- $val:expr, $($rest:tt)*),
> - ) => {
> - unsafe { $crate::init::__internal::DropGuard::forget($field) };
> -
> - $crate::__init_internal!(forget_guards:
> - @munch_fields($($rest)*),
> - );
> - };
> - (forget_guards:
> - @munch_fields($field:ident $(: $val:expr)?, $($rest:tt)*),
> - ) => {
> - unsafe { $crate::init::__internal::DropGuard::forget($field) };
> -
> - $crate::__init_internal!(forget_guards:
> - @munch_fields($($rest)*),
> - );
> - };
> }
>
> #[doc(hidden)]
> --
> 2.41.0
On Saturday, June 24th, 2023 at 11:25, Benno Lossin <[email protected]> wrote:
> Add the struct update syntax to the init macros, but only for
> `..Zeroable::zeroed()`. Adding this at the end of the struct initializer
> allows one to omit fields from the initializer, these fields will be
> initialized with 0x00 set to every byte. Only types that implement the
> `Zeroable` trait can utilize this.
>
> Suggested-by: Asahi Lina <[email protected]>
> Signed-off-by: Benno Lossin <[email protected]>
> ---
> rust/kernel/init.rs | 16 +++++-
> rust/kernel/init/macros.rs | 114 ++++++++++++++++++++++++++++++++++++-
> 2 files changed, 128 insertions(+), 2 deletions(-)
>
> diff --git a/rust/kernel/init.rs b/rust/kernel/init.rs
> index ecf6a4bd0ce4..44bc3e77419a 100644
> --- a/rust/kernel/init.rs
> +++ b/rust/kernel/init.rs
> @@ -508,14 +508,18 @@ macro_rules! stack_try_pin_init {
> /// - Fields that you want to initialize in-place have to use `<-` instead of `:`.
> /// - In front of the initializer you can write `&this in` to have access to a [`NonNull<Self>`]
> /// pointer named `this` inside of the initializer.
> +/// - Using struct update syntax one can place `..Zeroable::zeroed()` at the very end of the
> +/// struct, this initializes every field with 0 and then runs all initializers specified in the
> +/// body. This can only be done if [`Zeroable`] is implemented for the struct.
> ///
> /// For instance:
> ///
> /// ```rust
> /// # use kernel::pin_init;
> -/// # use macros::pin_data;
> +/// # use macros::{pin_data, Zeroable};
> /// # use core::{ptr::addr_of_mut, marker::PhantomPinned};
> /// #[pin_data]
> +/// #[derive(Zeroable)]
> /// struct Buf {
> /// // `ptr` points into `buf`.
> /// ptr: *mut u8,
> @@ -528,6 +532,10 @@ macro_rules! stack_try_pin_init {
> /// ptr: unsafe { addr_of_mut!((*this.as_ptr()).buf).cast() },
> /// pin: PhantomPinned,
> /// });
> +/// pin_init!(Buf {
> +/// buf: [1; 64],
> +/// ..Zeroable::zeroed(),
> +/// });
> /// ```
> ///
> /// [`try_pin_init!`]: kernel::try_pin_init
> @@ -547,6 +555,7 @@ macro_rules! pin_init {
> @data(PinData, use_data),
> @has_data(HasPinData, __pin_data),
> @construct_closure(pin_init_from_closure),
> + @munch_fields($($fields)*),
> )
> };
> }
> @@ -603,6 +612,7 @@ macro_rules! try_pin_init {
> @data(PinData, use_data),
> @has_data(HasPinData, __pin_data),
> @construct_closure(pin_init_from_closure),
> + @munch_fields($($fields)*),
> )
> };
> ($(&$this:ident in)? $t:ident $(::<$($generics:ty),* $(,)?>)? {
> @@ -616,6 +626,7 @@ macro_rules! try_pin_init {
> @data(PinData, use_data),
> @has_data(HasPinData, __pin_data),
> @construct_closure(pin_init_from_closure),
> + @munch_fields($($fields)*),
> )
> };
> }
> @@ -650,6 +661,7 @@ macro_rules! init {
> @data(InitData, /*no use_data*/),
> @has_data(HasInitData, __init_data),
> @construct_closure(init_from_closure),
> + @munch_fields($($fields)*),
> )
> }
> }
> @@ -700,6 +712,7 @@ macro_rules! try_init {
> @data(InitData, /*no use_data*/),
> @has_data(HasInitData, __init_data),
> @construct_closure(init_from_closure),
> + @munch_fields($($fields)*),
> )
> };
> ($(&$this:ident in)? $t:ident $(::<$($generics:ty),* $(,)?>)? {
> @@ -713,6 +726,7 @@ macro_rules! try_init {
> @data(InitData, /*no use_data*/),
> @has_data(HasInitData, __init_data),
> @construct_closure(init_from_closure),
> + @munch_fields($($fields)*),
> )
> };
> }
> diff --git a/rust/kernel/init/macros.rs b/rust/kernel/init/macros.rs
> index 1e0c4aca055a..5dcb2e513f26 100644
> --- a/rust/kernel/init/macros.rs
> +++ b/rust/kernel/init/macros.rs
> @@ -989,6 +989,7 @@ impl<$($impl_generics)*> $pin_data<$($ty_generics)*>
> ///
> /// This macro has multiple internal call configurations, these are always the very first ident:
> /// - nothing: this is the base case and called by the `{try_}{pin_}init!` macros.
> +/// - `with_update_parsed`: when the `..Zeroable::zeroed()` syntax has been handled.
> /// - `init_slot`: recursively creates the code that initializes all fields in `slot`.
> /// - `make_initializer`: recursively create the struct initializer that guarantees that every
> /// field has been initialized exactly once.
> @@ -1007,6 +1008,82 @@ macro_rules! __init_internal {
> @has_data($has_data:ident, $get_data:ident),
> // `pin_init_from_closure` or `init_from_closure`.
> @construct_closure($construct_closure:ident),
> + @munch_fields(),
> + ) => {
> + $crate::__init_internal!(with_update_parsed:
> + @this($($this)?),
> + @typ($t $(::<$($generics),*>)? ),
> + @fields($($fields)*),
> + @error($err),
> + @data($data, $($use_data)?),
> + @has_data($has_data, $get_data),
> + @construct_closure($construct_closure),
> + @zeroed(), // nothing means default behavior.
> + )
> + };
> + (
> + @this($($this:ident)?),
> + @typ($t:ident $(::<$($generics:ty),*>)?),
> + @fields($($fields:tt)*),
> + @error($err:ty),
> + // Either `PinData` or `InitData`, `$use_data` should only be present in the `PinData`
> + // case.
> + @data($data:ident, $($use_data:ident)?),
> + // `HasPinData` or `HasInitData`.
> + @has_data($has_data:ident, $get_data:ident),
> + // `pin_init_from_closure` or `init_from_closure`.
> + @construct_closure($construct_closure:ident),
> + @munch_fields(..Zeroable::zeroed()),
> + ) => {
> + $crate::__init_internal!(with_update_parsed:
> + @this($($this)?),
> + @typ($t $(::<$($generics),*>)? ),
> + @fields($($fields)*),
> + @error($err),
> + @data($data, $($use_data)?),
> + @has_data($has_data, $get_data),
> + @construct_closure($construct_closure),
> + @zeroed(()), // `()` means zero all fields not mentioned.
> + )
> + };
> + (
> + @this($($this:ident)?),
> + @typ($t:ident $(::<$($generics:ty),*>)?),
> + @fields($($fields:tt)*),
> + @error($err:ty),
> + // Either `PinData` or `InitData`, `$use_data` should only be present in the `PinData`
> + // case.
> + @data($data:ident, $($use_data:ident)?),
> + // `HasPinData` or `HasInitData`.
> + @has_data($has_data:ident, $get_data:ident),
> + // `pin_init_from_closure` or `init_from_closure`.
> + @construct_closure($construct_closure:ident),
> + @munch_fields($ignore:tt $($rest:tt)*),
> + ) => {
> + $crate::__init_internal!(
> + @this($($this)?),
> + @typ($t $(::<$($generics),*>)? ),
> + @fields($($fields)*),
> + @error($err),
> + @data($data, $($use_data)?),
> + @has_data($has_data, $get_data),
> + @construct_closure($construct_closure),
> + @munch_fields($($rest)*),
> + )
> + };
> + (with_update_parsed:
> + @this($($this:ident)?),
> + @typ($t:ident $(::<$($generics:ty),*>)?),
> + @fields($($fields:tt)*),
> + @error($err:ty),
> + // Either `PinData` or `InitData`, `$use_data` should only be present in the `PinData`
> + // case.
> + @data($data:ident, $($use_data:ident)?),
> + // `HasPinData` or `HasInitData`.
> + @has_data($has_data:ident, $get_data:ident),
> + // `pin_init_from_closure` or `init_from_closure`.
> + @construct_closure($construct_closure:ident),
> + @zeroed($($init_zeroed:expr)?),
> ) => {{
> // We do not want to allow arbitrary returns, so we declare this type as the `Ok` return
> // type and shadow it later when we insert the arbitrary user code. That way there will be
> @@ -1024,6 +1101,17 @@ macro_rules! __init_internal {
> {
> // Shadow the structure so it cannot be used to return early.
> struct __InitOk;
> + // If `$init_zeroed` is present we should zero the slot now and not emit an
> + // error when fields are missing (since they will be zeroed). We also have to
> + // check that the type actually implements `Zeroable`.
> + $(
> + fn is_zeroable<T: Zeroable>(ptr: *mut T) {}
Maybe call this assert_zeroable?
> + // Ensure that the struct is indeed `Zeroable`.
> + is_zeroable(slot);
> + // SAFETY: The type implements `Zeroable` by the check above.
> + unsafe { ::core::ptr::write_bytes(slot, 0, 1) };
> + $init_zeroed // this will be `()` if set.
How does this work? Shouldn't there be a ; after $init_zeroed to consume the () value?
> + )?
> // Create the `this` so it can be referenced by the user inside of the
> // expressions creating the individual fields.
> $(let $this = unsafe { ::core::ptr::NonNull::new_unchecked(slot) };)?
> @@ -1064,7 +1152,7 @@ macro_rules! __init_internal {
> @data($data:ident),
> @slot($slot:ident),
> @guards($($guards:ident,)*),
> - @munch_fields($(,)?),
> + @munch_fields($(..Zeroable::zeroed())? $(,)?),
> ) => {
> // Endpoint of munching, no fields are left. If execution reaches this point, all fields
> // have been initialized. Therefore we can now dismiss the guards by forgetting them.
> @@ -1157,6 +1245,30 @@ macro_rules! __init_internal {
> @munch_fields($($rest)*),
> );
> };
> + (make_initializer:
> + @slot($slot:ident),
> + @type_name($t:ident),
> + @munch_fields(..Zeroable::zeroed() $(,)?),
> + @acc($($acc:tt)*),
> + ) => {
> + // Endpoint, nothing more to munch, create the initializer. Since the users specified
> + // `..Zeroable::zeroed()`, the slot will already have been zeroed and all field that have
> + // not been overwritten are thus zero and initialized. We still check that all fields are
> + // actually accessible by using the struct update syntax ourselves.
> + // Since we are in the `if false` branch, this will never get executed. We abuse `slot` to
> + // get the correct type inference here:
> + unsafe {
> + let mut zeroed = ::core::mem::zeroed();
> + // We have to use type inference her to make zeroed have the correct type. This does
*here
> + // not get executed, so it has no effect.
> + ::core::ptr::write($slot, zeroed);
> + zeroed = ::core::mem::zeroed();
> + ::core::ptr::write($slot, $t {
> + $($acc)*
> + ..zeroed
> + });
> + }
> + };
> (make_initializer:
> @slot($slot:ident),
> @type_name($t:ident),
> --
> 2.41.0
Cheers,
Björn
On Saturday, June 24th, 2023 at 11:25, Benno Lossin <[email protected]> wrote:
> Add two functions `pin_init_array_from_fn` and `init_array_from_fn` that
> take a function that generates initializers for `T` from usize, the added
> functions then return an initializer for `[T; N]` where every element is
> initialized by an element returned from the generator function.
>
> Suggested-by: Asahi Lina <[email protected]>
> Signed-off-by: Benno Lossin <[email protected]>
Reviewed-by: Björn Roy Baron <[email protected]>
> ---
> rust/kernel/init.rs | 90 +++++++++++++++++++++++++++++++++++++++++++++
> 1 file changed, 90 insertions(+)
>
> diff --git a/rust/kernel/init.rs b/rust/kernel/init.rs
> index 44bc3e77419a..c9ea4bf71987 100644
> --- a/rust/kernel/init.rs
> +++ b/rust/kernel/init.rs
> @@ -867,6 +867,96 @@ pub fn uninit<T, E>() -> impl Init<MaybeUninit<T>, E> {
> unsafe { init_from_closure(|_| Ok(())) }
> }
>
> +/// Initializes an array by initializing each element via the provided initializer.
> +///
> +/// # Examples
> +///
> +/// ```rust
> +/// let array: Box<[usize; 1000_000_000]>= Box::init(init_array_from_fn(|i| i)).unwrap();
> +/// println!("{array:?}");
> +/// ```
> +pub fn init_array_from_fn<I, const N: usize, T, E>(
> + mut make_init: impl FnMut(usize) -> I,
> +) -> impl Init<[T; N], E>
> +where
> + I: Init<T, E>,
> +{
> + let init = move |slot: *mut [T; N]| {
> + let slot = slot.cast::<T>();
> + for i in 0..N {
> + let init = make_init(i);
> + // SAFETY: since 0 <= `i` < N, it is still in bounds of `[T; N]`.
> + let ptr = unsafe { slot.add(i) };
> + // SAFETY: The pointer is derived from `slot` and thus satisfies the `__init`
> + // requirements.
> + match unsafe { init.__init(ptr) } {
> + Ok(()) => {}
> + Err(e) => {
> + // We now free every element that has been initialized before:
> + for j in 0..i {
> + let ptr = unsafe { slot.add(j) };
> + // SAFETY: The value was initialized in a previous iteration of the loop
> + // and since we return `Err` below, the caller will consider the memory at
> + // `slot` as uninitialized.
> + unsafe { ptr::drop_in_place(ptr) };
> + }
> + return Err(e);
> + }
> + }
> + }
> + Ok(())
> + };
> + // SAFETY: The initializer above initializes every element of the array. On failure it drops
> + // any initialized elements and returns `Err`.
> + unsafe { init_from_closure(init) }
> +}
> +
> +/// Initializes an array by initializing each element via the provided initializer.
> +///
> +/// # Examples
> +///
> +/// ```rust
> +/// let array: Arc<[Mutex<usize>; 1000_000_000]>=
> +/// Arc::pin_init(init_array_from_fn(|i| new_mutex!(i))).unwrap();
> +/// println!("{array:?}");
> +/// ```
> +pub fn pin_init_array_from_fn<I, const N: usize, T, E>(
> + mut make_init: impl FnMut(usize) -> I,
> +) -> impl PinInit<[T; N], E>
> +where
> + I: PinInit<T, E>,
> +{
> + let init = move |slot: *mut [T; N]| {
> + let slot = slot.cast::<T>();
> + for i in 0..N {
> + let init = make_init(i);
> + // SAFETY: since 0 <= `i` < N, it is still in bounds of `[T; N]`.
> + let ptr = unsafe { slot.add(i) };
> + // SAFETY: The pointer is derived from `slot` and thus satisfies the `__pinned_init`
> + // requirements.
> + match unsafe { init.__pinned_init(ptr) } {
> + Ok(()) => {}
> + Err(e) => {
> + // We now have to free every element that has been initialized before, since we
> + // have to abide by the drop guarantee.
> + for j in 0..i {
> + let ptr = unsafe { slot.add(j) };
> + // SAFETY: The value was initialized in a previous iteration of the loop
> + // and since we return `Err` below, the caller will consider the memory at
> + // `slot` as uninitialized.
> + unsafe { ptr::drop_in_place(ptr) };
> + }
> + return Err(e);
> + }
> + }
> + }
> + Ok(())
> + };
> + // SAFETY: The initializer above initializes every element of the array. On failure it drops
> + // any initialized elements and returns `Err`.
> + unsafe { pin_init_from_closure(init) }
> +}
> +
> // SAFETY: Every type can be initialized by-value.
> unsafe impl<T, E> Init<T, E> for T {
> unsafe fn __init(self, slot: *mut T) -> Result<(), E> {
> --
> 2.41.0
On 6/24/23 17:03, Björn Roy Baron wrote:
> On Saturday, June 24th, 2023 at 11:25, Benno Lossin <[email protected]> wrote:
>
>> In the implementation of the init macros there is a `if false` statement
>> that type checks the initializer to ensure every field is initialized.
>> Since the next patch has a stack variable to store the struct, the
>> function might allocate too much memory on debug builds. Putting the
>> struct into a closure ensures that even in debug builds no stack
>> overflow error is caused. In release builds this was not a problem since
>> the code was optimized away due to the `if false`.
>>
>> Signed-off-by: Benno Lossin <[email protected]>
>> ---
>> rust/kernel/init/macros.rs | 18 +++++++++++-------
>> 1 file changed, 11 insertions(+), 7 deletions(-)
>>
>> diff --git a/rust/kernel/init/macros.rs b/rust/kernel/init/macros.rs
>> index df4281743175..1e0c4aca055a 100644
>> --- a/rust/kernel/init/macros.rs
>> +++ b/rust/kernel/init/macros.rs
>> @@ -1037,14 +1037,18 @@ macro_rules! __init_internal {
>> // We use unreachable code to ensure that all fields have been mentioned exactly
>> // once, this struct initializer will still be type-checked and complain with a
>> // very natural error message if a field is forgotten/mentioned more than once.
>> - #[allow(unreachable_code, clippy::diverging_sub_expression)]
>> + #[allow(unreachable_code,
>> + clippy::diverging_sub_expression,
>> + clippy::redundant_closure_call)]
>> if false {
>> - $crate::__init_internal!(make_initializer:
>> - @slot(slot),
>> - @type_name($t),
>> - @munch_fields($($fields)*,),
>> - @acc(),
>> - );
>> + (|| {
>> + $crate::__init_internal!(make_initializer:
>> + @slot(slot),
>> + @type_name($t),
>> + @munch_fields($($fields)*,),
>> + @acc(),
>> + );
>> + })();
>
> Is it necessary to call this closure? Typechecking of the closure should happen even without calling it.
You are right, I do not need to call it. Will fix.
--
Cheers,
Benno
>
>> }
>> }
>> Ok(__InitOk)
>> --
>> 2.41.0
>
> Cheers,
> Björn
On 6/24/23 17:11, Björn Roy Baron wrote:
> On Saturday, June 24th, 2023 at 11:25, Benno Lossin <[email protected]> wrote:
>
>> Add the struct update syntax to the init macros, but only for
>> `..Zeroable::zeroed()`. Adding this at the end of the struct initializer
>> allows one to omit fields from the initializer, these fields will be
>> initialized with 0x00 set to every byte. Only types that implement the
>> `Zeroable` trait can utilize this.
>>
>> Suggested-by: Asahi Lina <[email protected]>
>> Signed-off-by: Benno Lossin <[email protected]>
>> ---
>> rust/kernel/init.rs | 16 +++++-
>> rust/kernel/init/macros.rs | 114 ++++++++++++++++++++++++++++++++++++-
>> 2 files changed, 128 insertions(+), 2 deletions(-)
>>
>> diff --git a/rust/kernel/init.rs b/rust/kernel/init.rs
>> index ecf6a4bd0ce4..44bc3e77419a 100644
>> --- a/rust/kernel/init.rs
>> +++ b/rust/kernel/init.rs
>> @@ -508,14 +508,18 @@ macro_rules! stack_try_pin_init {
>> /// - Fields that you want to initialize in-place have to use `<-` instead of `:`.
>> /// - In front of the initializer you can write `&this in` to have access to a [`NonNull<Self>`]
>> /// pointer named `this` inside of the initializer.
>> +/// - Using struct update syntax one can place `..Zeroable::zeroed()` at the very end of the
>> +/// struct, this initializes every field with 0 and then runs all initializers specified in the
>> +/// body. This can only be done if [`Zeroable`] is implemented for the struct.
>> ///
>> /// For instance:
>> ///
>> /// ```rust
>> /// # use kernel::pin_init;
>> -/// # use macros::pin_data;
>> +/// # use macros::{pin_data, Zeroable};
>> /// # use core::{ptr::addr_of_mut, marker::PhantomPinned};
>> /// #[pin_data]
>> +/// #[derive(Zeroable)]
>> /// struct Buf {
>> /// // `ptr` points into `buf`.
>> /// ptr: *mut u8,
>> @@ -528,6 +532,10 @@ macro_rules! stack_try_pin_init {
>> /// ptr: unsafe { addr_of_mut!((*this.as_ptr()).buf).cast() },
>> /// pin: PhantomPinned,
>> /// });
>> +/// pin_init!(Buf {
>> +/// buf: [1; 64],
>> +/// ..Zeroable::zeroed(),
>> +/// });
>> /// ```
>> ///
>> /// [`try_pin_init!`]: kernel::try_pin_init
>> @@ -547,6 +555,7 @@ macro_rules! pin_init {
>> @data(PinData, use_data),
>> @has_data(HasPinData, __pin_data),
>> @construct_closure(pin_init_from_closure),
>> + @munch_fields($($fields)*),
>> )
>> };
>> }
>> @@ -603,6 +612,7 @@ macro_rules! try_pin_init {
>> @data(PinData, use_data),
>> @has_data(HasPinData, __pin_data),
>> @construct_closure(pin_init_from_closure),
>> + @munch_fields($($fields)*),
>> )
>> };
>> ($(&$this:ident in)? $t:ident $(::<$($generics:ty),* $(,)?>)? {
>> @@ -616,6 +626,7 @@ macro_rules! try_pin_init {
>> @data(PinData, use_data),
>> @has_data(HasPinData, __pin_data),
>> @construct_closure(pin_init_from_closure),
>> + @munch_fields($($fields)*),
>> )
>> };
>> }
>> @@ -650,6 +661,7 @@ macro_rules! init {
>> @data(InitData, /*no use_data*/),
>> @has_data(HasInitData, __init_data),
>> @construct_closure(init_from_closure),
>> + @munch_fields($($fields)*),
>> )
>> }
>> }
>> @@ -700,6 +712,7 @@ macro_rules! try_init {
>> @data(InitData, /*no use_data*/),
>> @has_data(HasInitData, __init_data),
>> @construct_closure(init_from_closure),
>> + @munch_fields($($fields)*),
>> )
>> };
>> ($(&$this:ident in)? $t:ident $(::<$($generics:ty),* $(,)?>)? {
>> @@ -713,6 +726,7 @@ macro_rules! try_init {
>> @data(InitData, /*no use_data*/),
>> @has_data(HasInitData, __init_data),
>> @construct_closure(init_from_closure),
>> + @munch_fields($($fields)*),
>> )
>> };
>> }
>> diff --git a/rust/kernel/init/macros.rs b/rust/kernel/init/macros.rs
>> index 1e0c4aca055a..5dcb2e513f26 100644
>> --- a/rust/kernel/init/macros.rs
>> +++ b/rust/kernel/init/macros.rs
>> @@ -989,6 +989,7 @@ impl<$($impl_generics)*> $pin_data<$($ty_generics)*>
>> ///
>> /// This macro has multiple internal call configurations, these are always the very first ident:
>> /// - nothing: this is the base case and called by the `{try_}{pin_}init!` macros.
>> +/// - `with_update_parsed`: when the `..Zeroable::zeroed()` syntax has been handled.
>> /// - `init_slot`: recursively creates the code that initializes all fields in `slot`.
>> /// - `make_initializer`: recursively create the struct initializer that guarantees that every
>> /// field has been initialized exactly once.
>> @@ -1007,6 +1008,82 @@ macro_rules! __init_internal {
>> @has_data($has_data:ident, $get_data:ident),
>> // `pin_init_from_closure` or `init_from_closure`.
>> @construct_closure($construct_closure:ident),
>> + @munch_fields(),
>> + ) => {
>> + $crate::__init_internal!(with_update_parsed:
>> + @this($($this)?),
>> + @typ($t $(::<$($generics),*>)? ),
>> + @fields($($fields)*),
>> + @error($err),
>> + @data($data, $($use_data)?),
>> + @has_data($has_data, $get_data),
>> + @construct_closure($construct_closure),
>> + @zeroed(), // nothing means default behavior.
>> + )
>> + };
>> + (
>> + @this($($this:ident)?),
>> + @typ($t:ident $(::<$($generics:ty),*>)?),
>> + @fields($($fields:tt)*),
>> + @error($err:ty),
>> + // Either `PinData` or `InitData`, `$use_data` should only be present in the `PinData`
>> + // case.
>> + @data($data:ident, $($use_data:ident)?),
>> + // `HasPinData` or `HasInitData`.
>> + @has_data($has_data:ident, $get_data:ident),
>> + // `pin_init_from_closure` or `init_from_closure`.
>> + @construct_closure($construct_closure:ident),
>> + @munch_fields(..Zeroable::zeroed()),
>> + ) => {
>> + $crate::__init_internal!(with_update_parsed:
>> + @this($($this)?),
>> + @typ($t $(::<$($generics),*>)? ),
>> + @fields($($fields)*),
>> + @error($err),
>> + @data($data, $($use_data)?),
>> + @has_data($has_data, $get_data),
>> + @construct_closure($construct_closure),
>> + @zeroed(()), // `()` means zero all fields not mentioned.
>> + )
>> + };
>> + (
>> + @this($($this:ident)?),
>> + @typ($t:ident $(::<$($generics:ty),*>)?),
>> + @fields($($fields:tt)*),
>> + @error($err:ty),
>> + // Either `PinData` or `InitData`, `$use_data` should only be present in the `PinData`
>> + // case.
>> + @data($data:ident, $($use_data:ident)?),
>> + // `HasPinData` or `HasInitData`.
>> + @has_data($has_data:ident, $get_data:ident),
>> + // `pin_init_from_closure` or `init_from_closure`.
>> + @construct_closure($construct_closure:ident),
>> + @munch_fields($ignore:tt $($rest:tt)*),
>> + ) => {
>> + $crate::__init_internal!(
>> + @this($($this)?),
>> + @typ($t $(::<$($generics),*>)? ),
>> + @fields($($fields)*),
>> + @error($err),
>> + @data($data, $($use_data)?),
>> + @has_data($has_data, $get_data),
>> + @construct_closure($construct_closure),
>> + @munch_fields($($rest)*),
>> + )
>> + };
>> + (with_update_parsed:
>> + @this($($this:ident)?),
>> + @typ($t:ident $(::<$($generics:ty),*>)?),
>> + @fields($($fields:tt)*),
>> + @error($err:ty),
>> + // Either `PinData` or `InitData`, `$use_data` should only be present in the `PinData`
>> + // case.
>> + @data($data:ident, $($use_data:ident)?),
>> + // `HasPinData` or `HasInitData`.
>> + @has_data($has_data:ident, $get_data:ident),
>> + // `pin_init_from_closure` or `init_from_closure`.
>> + @construct_closure($construct_closure:ident),
>> + @zeroed($($init_zeroed:expr)?),
>> ) => {{
>> // We do not want to allow arbitrary returns, so we declare this type as the `Ok` return
>> // type and shadow it later when we insert the arbitrary user code. That way there will be
>> @@ -1024,6 +1101,17 @@ macro_rules! __init_internal {
>> {
>> // Shadow the structure so it cannot be used to return early.
>> struct __InitOk;
>> + // If `$init_zeroed` is present we should zero the slot now and not emit an
>> + // error when fields are missing (since they will be zeroed). We also have to
>> + // check that the type actually implements `Zeroable`.
>> + $(
>> + fn is_zeroable<T: Zeroable>(ptr: *mut T) {}
>
> Maybe call this assert_zeroable?
Sure.
>
>> + // Ensure that the struct is indeed `Zeroable`.
>> + is_zeroable(slot);
>> + // SAFETY: The type implements `Zeroable` by the check above.
>> + unsafe { ::core::ptr::write_bytes(slot, 0, 1) };
>> + $init_zeroed // this will be `()` if set.
>
> How does this work? Shouldn't there be a ; after $init_zeroed to consume the () value?
It is the last expression of a block and since it is `()` it is ok
(adding a ; would also be ok, but it is not necessary).
>
>> + )?
>> // Create the `this` so it can be referenced by the user inside of the
>> // expressions creating the individual fields.
>> $(let $this = unsafe { ::core::ptr::NonNull::new_unchecked(slot) };)?
>> @@ -1064,7 +1152,7 @@ macro_rules! __init_internal {
>> @data($data:ident),
>> @slot($slot:ident),
>> @guards($($guards:ident,)*),
>> - @munch_fields($(,)?),
>> + @munch_fields($(..Zeroable::zeroed())? $(,)?),
>> ) => {
>> // Endpoint of munching, no fields are left. If execution reaches this point, all fields
>> // have been initialized. Therefore we can now dismiss the guards by forgetting them.
>> @@ -1157,6 +1245,30 @@ macro_rules! __init_internal {
>> @munch_fields($($rest)*),
>> );
>> };
>> + (make_initializer:
>> + @slot($slot:ident),
>> + @type_name($t:ident),
>> + @munch_fields(..Zeroable::zeroed() $(,)?),
>> + @acc($($acc:tt)*),
>> + ) => {
>> + // Endpoint, nothing more to munch, create the initializer. Since the users specified
>> + // `..Zeroable::zeroed()`, the slot will already have been zeroed and all field that have
>> + // not been overwritten are thus zero and initialized. We still check that all fields are
>> + // actually accessible by using the struct update syntax ourselves.
>> + // Since we are in the `if false` branch, this will never get executed. We abuse `slot` to
>> + // get the correct type inference here:
>> + unsafe {
>> + let mut zeroed = ::core::mem::zeroed();
>> + // We have to use type inference her to make zeroed have the correct type. This does
>
> *here
Will fix.
--
Cheers,
Benno
>
>> + // not get executed, so it has no effect.
>> + ::core::ptr::write($slot, zeroed);
>> + zeroed = ::core::mem::zeroed();
>> + ::core::ptr::write($slot, $t {
>> + $($acc)*
>> + ..zeroed
>> + });
>> + }
>> + };
>> (make_initializer:
>> @slot($slot:ident),
>> @type_name($t:ident),
>> --
>> 2.41.0
>
> Cheers,
> Björn
On 25.06.23 14:56, Björn Roy Baron wrote:
> On Saturday, June 24th, 2023 at 23:14, Benno Lossin <[email protected]> wrote:
>>>> + // Ensure that the struct is indeed `Zeroable`.
>>>> + is_zeroable(slot);
>>>> + // SAFETY: The type implements `Zeroable` by the check above.
>>>> + unsafe { ::core::ptr::write_bytes(slot, 0, 1) };
>>>> + $init_zeroed // this will be `()` if set.
>>>
>>> How does this work? Shouldn't there be a ; after $init_zeroed to consume the () value?
>>
>> It is the last expression of a block and since it is `()` it is ok
>> (adding a ; would also be ok, but it is not necessary).
>
> I'm surprised it is considered the last expression of a block. Unlike with {} using $()? will still
> allow variables defined inside this as if they were outside of it. Also I can't reproduce this
> behavior with:
>
> macro_rules! foo {
> ($($a:expr)?) => {
> $($a)?
> bar();
> }
> }
>
> fn main() {
> foo!(());
> }
>
> Is there something I'm missing?
>
> Cheers,
> Björn
Not sure what you mean with "allow variables defined inside this
as if they were outside of it". But note that in the macro `$init_zeroed`
is the last expression of a block. Here is a small example:
```
macro_rules! foo {
($($a:expr)?) => {{
$(
bar();
$a
)?
}};
}
fn bar() {}
fn main() {
foo!(());
foo!();
}
```
it expands to this:
```
fn main() {
{
bar();
()
};
{};
}
```
--
Cheers,
Benno
On Saturday, June 24th, 2023 at 23:14, Benno Lossin <[email protected]> wrote:
> On 6/24/23 17:11, Björn Roy Baron wrote:
> > On Saturday, June 24th, 2023 at 11:25, Benno Lossin <[email protected]> wrote:
> >
> >> Add the struct update syntax to the init macros, but only for
> >> `..Zeroable::zeroed()`. Adding this at the end of the struct initializer
> >> allows one to omit fields from the initializer, these fields will be
> >> initialized with 0x00 set to every byte. Only types that implement the
> >> `Zeroable` trait can utilize this.
> >>
> >> Suggested-by: Asahi Lina <[email protected]>
> >> Signed-off-by: Benno Lossin <[email protected]>
> >> ---
> >> rust/kernel/init.rs | 16 +++++-
> >> rust/kernel/init/macros.rs | 114 ++++++++++++++++++++++++++++++++++++-
> >> 2 files changed, 128 insertions(+), 2 deletions(-)
> >>
> >> diff --git a/rust/kernel/init.rs b/rust/kernel/init.rs
> >> index ecf6a4bd0ce4..44bc3e77419a 100644
> >> --- a/rust/kernel/init.rs
> >> +++ b/rust/kernel/init.rs
> >> @@ -508,14 +508,18 @@ macro_rules! stack_try_pin_init {
> >> /// - Fields that you want to initialize in-place have to use `<-` instead of `:`.
> >> /// - In front of the initializer you can write `&this in` to have access to a [`NonNull<Self>`]
> >> /// pointer named `this` inside of the initializer.
> >> +/// - Using struct update syntax one can place `..Zeroable::zeroed()` at the very end of the
> >> +/// struct, this initializes every field with 0 and then runs all initializers specified in the
> >> +/// body. This can only be done if [`Zeroable`] is implemented for the struct.
> >> ///
> >> /// For instance:
> >> ///
> >> /// ```rust
> >> /// # use kernel::pin_init;
> >> -/// # use macros::pin_data;
> >> +/// # use macros::{pin_data, Zeroable};
> >> /// # use core::{ptr::addr_of_mut, marker::PhantomPinned};
> >> /// #[pin_data]
> >> +/// #[derive(Zeroable)]
> >> /// struct Buf {
> >> /// // `ptr` points into `buf`.
> >> /// ptr: *mut u8,
> >> @@ -528,6 +532,10 @@ macro_rules! stack_try_pin_init {
> >> /// ptr: unsafe { addr_of_mut!((*this.as_ptr()).buf).cast() },
> >> /// pin: PhantomPinned,
> >> /// });
> >> +/// pin_init!(Buf {
> >> +/// buf: [1; 64],
> >> +/// ..Zeroable::zeroed(),
> >> +/// });
> >> /// ```
> >> ///
> >> /// [`try_pin_init!`]: kernel::try_pin_init
> >> @@ -547,6 +555,7 @@ macro_rules! pin_init {
> >> @data(PinData, use_data),
> >> @has_data(HasPinData, __pin_data),
> >> @construct_closure(pin_init_from_closure),
> >> + @munch_fields($($fields)*),
> >> )
> >> };
> >> }
> >> @@ -603,6 +612,7 @@ macro_rules! try_pin_init {
> >> @data(PinData, use_data),
> >> @has_data(HasPinData, __pin_data),
> >> @construct_closure(pin_init_from_closure),
> >> + @munch_fields($($fields)*),
> >> )
> >> };
> >> ($(&$this:ident in)? $t:ident $(::<$($generics:ty),* $(,)?>)? {
> >> @@ -616,6 +626,7 @@ macro_rules! try_pin_init {
> >> @data(PinData, use_data),
> >> @has_data(HasPinData, __pin_data),
> >> @construct_closure(pin_init_from_closure),
> >> + @munch_fields($($fields)*),
> >> )
> >> };
> >> }
> >> @@ -650,6 +661,7 @@ macro_rules! init {
> >> @data(InitData, /*no use_data*/),
> >> @has_data(HasInitData, __init_data),
> >> @construct_closure(init_from_closure),
> >> + @munch_fields($($fields)*),
> >> )
> >> }
> >> }
> >> @@ -700,6 +712,7 @@ macro_rules! try_init {
> >> @data(InitData, /*no use_data*/),
> >> @has_data(HasInitData, __init_data),
> >> @construct_closure(init_from_closure),
> >> + @munch_fields($($fields)*),
> >> )
> >> };
> >> ($(&$this:ident in)? $t:ident $(::<$($generics:ty),* $(,)?>)? {
> >> @@ -713,6 +726,7 @@ macro_rules! try_init {
> >> @data(InitData, /*no use_data*/),
> >> @has_data(HasInitData, __init_data),
> >> @construct_closure(init_from_closure),
> >> + @munch_fields($($fields)*),
> >> )
> >> };
> >> }
> >> diff --git a/rust/kernel/init/macros.rs b/rust/kernel/init/macros.rs
> >> index 1e0c4aca055a..5dcb2e513f26 100644
> >> --- a/rust/kernel/init/macros.rs
> >> +++ b/rust/kernel/init/macros.rs
> >> @@ -989,6 +989,7 @@ impl<$($impl_generics)*> $pin_data<$($ty_generics)*>
> >> ///
> >> /// This macro has multiple internal call configurations, these are always the very first ident:
> >> /// - nothing: this is the base case and called by the `{try_}{pin_}init!` macros.
> >> +/// - `with_update_parsed`: when the `..Zeroable::zeroed()` syntax has been handled.
> >> /// - `init_slot`: recursively creates the code that initializes all fields in `slot`.
> >> /// - `make_initializer`: recursively create the struct initializer that guarantees that every
> >> /// field has been initialized exactly once.
> >> @@ -1007,6 +1008,82 @@ macro_rules! __init_internal {
> >> @has_data($has_data:ident, $get_data:ident),
> >> // `pin_init_from_closure` or `init_from_closure`.
> >> @construct_closure($construct_closure:ident),
> >> + @munch_fields(),
> >> + ) => {
> >> + $crate::__init_internal!(with_update_parsed:
> >> + @this($($this)?),
> >> + @typ($t $(::<$($generics),*>)? ),
> >> + @fields($($fields)*),
> >> + @error($err),
> >> + @data($data, $($use_data)?),
> >> + @has_data($has_data, $get_data),
> >> + @construct_closure($construct_closure),
> >> + @zeroed(), // nothing means default behavior.
> >> + )
> >> + };
> >> + (
> >> + @this($($this:ident)?),
> >> + @typ($t:ident $(::<$($generics:ty),*>)?),
> >> + @fields($($fields:tt)*),
> >> + @error($err:ty),
> >> + // Either `PinData` or `InitData`, `$use_data` should only be present in the `PinData`
> >> + // case.
> >> + @data($data:ident, $($use_data:ident)?),
> >> + // `HasPinData` or `HasInitData`.
> >> + @has_data($has_data:ident, $get_data:ident),
> >> + // `pin_init_from_closure` or `init_from_closure`.
> >> + @construct_closure($construct_closure:ident),
> >> + @munch_fields(..Zeroable::zeroed()),
> >> + ) => {
> >> + $crate::__init_internal!(with_update_parsed:
> >> + @this($($this)?),
> >> + @typ($t $(::<$($generics),*>)? ),
> >> + @fields($($fields)*),
> >> + @error($err),
> >> + @data($data, $($use_data)?),
> >> + @has_data($has_data, $get_data),
> >> + @construct_closure($construct_closure),
> >> + @zeroed(()), // `()` means zero all fields not mentioned.
> >> + )
> >> + };
> >> + (
> >> + @this($($this:ident)?),
> >> + @typ($t:ident $(::<$($generics:ty),*>)?),
> >> + @fields($($fields:tt)*),
> >> + @error($err:ty),
> >> + // Either `PinData` or `InitData`, `$use_data` should only be present in the `PinData`
> >> + // case.
> >> + @data($data:ident, $($use_data:ident)?),
> >> + // `HasPinData` or `HasInitData`.
> >> + @has_data($has_data:ident, $get_data:ident),
> >> + // `pin_init_from_closure` or `init_from_closure`.
> >> + @construct_closure($construct_closure:ident),
> >> + @munch_fields($ignore:tt $($rest:tt)*),
> >> + ) => {
> >> + $crate::__init_internal!(
> >> + @this($($this)?),
> >> + @typ($t $(::<$($generics),*>)? ),
> >> + @fields($($fields)*),
> >> + @error($err),
> >> + @data($data, $($use_data)?),
> >> + @has_data($has_data, $get_data),
> >> + @construct_closure($construct_closure),
> >> + @munch_fields($($rest)*),
> >> + )
> >> + };
> >> + (with_update_parsed:
> >> + @this($($this:ident)?),
> >> + @typ($t:ident $(::<$($generics:ty),*>)?),
> >> + @fields($($fields:tt)*),
> >> + @error($err:ty),
> >> + // Either `PinData` or `InitData`, `$use_data` should only be present in the `PinData`
> >> + // case.
> >> + @data($data:ident, $($use_data:ident)?),
> >> + // `HasPinData` or `HasInitData`.
> >> + @has_data($has_data:ident, $get_data:ident),
> >> + // `pin_init_from_closure` or `init_from_closure`.
> >> + @construct_closure($construct_closure:ident),
> >> + @zeroed($($init_zeroed:expr)?),
> >> ) => {{
> >> // We do not want to allow arbitrary returns, so we declare this type as the `Ok` return
> >> // type and shadow it later when we insert the arbitrary user code. That way there will be
> >> @@ -1024,6 +1101,17 @@ macro_rules! __init_internal {
> >> {
> >> // Shadow the structure so it cannot be used to return early.
> >> struct __InitOk;
> >> + // If `$init_zeroed` is present we should zero the slot now and not emit an
> >> + // error when fields are missing (since they will be zeroed). We also have to
> >> + // check that the type actually implements `Zeroable`.
> >> + $(
> >> + fn is_zeroable<T: Zeroable>(ptr: *mut T) {}
> >
> > Maybe call this assert_zeroable?
>
> Sure.
>
> >
> >> + // Ensure that the struct is indeed `Zeroable`.
> >> + is_zeroable(slot);
> >> + // SAFETY: The type implements `Zeroable` by the check above.
> >> + unsafe { ::core::ptr::write_bytes(slot, 0, 1) };
> >> + $init_zeroed // this will be `()` if set.
> >
> > How does this work? Shouldn't there be a ; after $init_zeroed to consume the () value?
>
> It is the last expression of a block and since it is `()` it is ok
> (adding a ; would also be ok, but it is not necessary).
I'm surprised it is considered the last expression of a block. Unlike with {} using $()? will still
allow variables defined inside this as if they were outside of it. Also I can't reproduce this
behavior with:
macro_rules! foo {
($($a:expr)?) => {
$($a)?
bar();
}
}
fn main() {
foo!(());
}
Is there something I'm missing?
Cheers,
Björn
>
> >
> >> + )?
> >> // Create the `this` so it can be referenced by the user inside of the
> >> // expressions creating the individual fields.
> >> $(let $this = unsafe { ::core::ptr::NonNull::new_unchecked(slot) };)?
> >> @@ -1064,7 +1152,7 @@ macro_rules! __init_internal {
> >> @data($data:ident),
> >> @slot($slot:ident),
> >> @guards($($guards:ident,)*),
> >> - @munch_fields($(,)?),
> >> + @munch_fields($(..Zeroable::zeroed())? $(,)?),
> >> ) => {
> >> // Endpoint of munching, no fields are left. If execution reaches this point, all fields
> >> // have been initialized. Therefore we can now dismiss the guards by forgetting them.
> >> @@ -1157,6 +1245,30 @@ macro_rules! __init_internal {
> >> @munch_fields($($rest)*),
> >> );
> >> };
> >> + (make_initializer:
> >> + @slot($slot:ident),
> >> + @type_name($t:ident),
> >> + @munch_fields(..Zeroable::zeroed() $(,)?),
> >> + @acc($($acc:tt)*),
> >> + ) => {
> >> + // Endpoint, nothing more to munch, create the initializer. Since the users specified
> >> + // `..Zeroable::zeroed()`, the slot will already have been zeroed and all field that have
> >> + // not been overwritten are thus zero and initialized. We still check that all fields are
> >> + // actually accessible by using the struct update syntax ourselves.
> >> + // Since we are in the `if false` branch, this will never get executed. We abuse `slot` to
> >> + // get the correct type inference here:
> >> + unsafe {
> >> + let mut zeroed = ::core::mem::zeroed();
> >> + // We have to use type inference her to make zeroed have the correct type. This does
> >
> > *here
>
> Will fix.
>
> --
> Cheers,
> Benno
>
> >
> >> + // not get executed, so it has no effect.
> >> + ::core::ptr::write($slot, zeroed);
> >> + zeroed = ::core::mem::zeroed();
> >> + ::core::ptr::write($slot, $t {
> >> + $($acc)*
> >> + ..zeroed
> >> + });
> >> + }
> >> + };
> >> (make_initializer:
> >> @slot($slot:ident),
> >> @type_name($t:ident),
> >> --
> >> 2.41.0
> >
> > Cheers,
> > Björn
On Sunday, June 25th, 2023 at 15:07, Benno Lossin <[email protected]> wrote:
> On 25.06.23 14:56, Björn Roy Baron wrote:
> > On Saturday, June 24th, 2023 at 23:14, Benno Lossin <[email protected]> wrote:
>
> >>>> + // Ensure that the struct is indeed `Zeroable`.
> >>>> + is_zeroable(slot);
> >>>> + // SAFETY: The type implements `Zeroable` by the check above.
> >>>> + unsafe { ::core::ptr::write_bytes(slot, 0, 1) };
> >>>> + $init_zeroed // this will be `()` if set.
> >>>
> >>> How does this work? Shouldn't there be a ; after $init_zeroed to consume the () value?
> >>
> >> It is the last expression of a block and since it is `()` it is ok
> >> (adding a ; would also be ok, but it is not necessary).
> >
> > I'm surprised it is considered the last expression of a block. Unlike with {} using $()? will still
> > allow variables defined inside this as if they were outside of it. Also I can't reproduce this
> > behavior with:
> >
> > macro_rules! foo {
> > ($($a:expr)?) => {
> > $($a)?
> > bar();
> > }
> > }
> >
> > fn main() {
> > foo!(());
> > }
> >
> > Is there something I'm missing?
> >
> > Cheers,
> > Björn
>
> Not sure what you mean with "allow variables defined inside this
> as if they were outside of it". But note that in the macro `$init_zeroed`
> is the last expression of a block. Here is a small example:
$(let $this = unsafe { ::core::ptr::NonNull::new_unchecked(slot) };)? comes after
this code in the same block that contains struct __InitOk;. And after that another
$crate::__init_internal!() invocation. That is why I don't get that this is allowed
at all.
>
> ```
> macro_rules! foo {
> ($($a:expr)?) => {{
> $(
> bar();
> $a
> )?
> }};
> }
>
> fn bar() {}
>
> fn main() {
> foo!(());
> foo!();
> }
> ```
>
> it expands to this:
> ```
> fn main() {
> {
> bar();
> ()
> };
> {};
> }
> ```
>
> --
> Cheers,
> Benno
>
On 6/25/23 16:17, Björn Roy Baron wrote:
> On Sunday, June 25th, 2023 at 15:07, Benno Lossin <[email protected]> wrote:
>> On 25.06.23 14:56, Björn Roy Baron wrote:
>>> On Saturday, June 24th, 2023 at 23:14, Benno Lossin <[email protected]> wrote:
>>
>>>>>> + // Ensure that the struct is indeed `Zeroable`.
>>>>>> + is_zeroable(slot);
>>>>>> + // SAFETY: The type implements `Zeroable` by the check above.
>>>>>> + unsafe { ::core::ptr::write_bytes(slot, 0, 1) };
>>>>>> + $init_zeroed // this will be `()` if set.
>>>>>
>>>>> How does this work? Shouldn't there be a ; after $init_zeroed to consume the () value?
>>>>
>>>> It is the last expression of a block and since it is `()` it is ok
>>>> (adding a ; would also be ok, but it is not necessary).
>>>
>>> I'm surprised it is considered the last expression of a block. Unlike with {} using $()? will still
>>> allow variables defined inside this as if they were outside of it. Also I can't reproduce this
>>> behavior with:
>>>
>>> macro_rules! foo {
>>> ($($a:expr)?) => {
>>> $($a)?
>>> bar();
>>> }
>>> }
>>>
>>> fn main() {
>>> foo!(());
>>> }
>>>
>>> Is there something I'm missing?
>>>
>>> Cheers,
>>> Björn
>>
>> Not sure what you mean with "allow variables defined inside this
>> as if they were outside of it". But note that in the macro `$init_zeroed`
>> is the last expression of a block. Here is a small example:
>
> $(let $this = unsafe { ::core::ptr::NonNull::new_unchecked(slot) };)? comes after
> this code in the same block that contains struct __InitOk;. And after that another
> $crate::__init_internal!() invocation. That is why I don't get that this is allowed
> at all.
>
Oh I see the issue now, I thought I wrote
```
$({
fn assert_zeroable<T: Zeroable>(ptr: *mut T) {}
// Ensure that the struct is indeed `Zeroable`.
assert_zeroable(slot);
// SAFETY: The type implements `Zeroable` by the check above.
unsafe { ::core::ptr::write_bytes(slot, 0, 1) };
$init_zeroed // this will be `()` if set.
})?
```
But I forgot the inner `{}`. Good catch!
--
Cheers,
Benno
>>
>> ```
>> macro_rules! foo {
>> ($($a:expr)?) => {{
>> $(
>> bar();
>> $a
>> )?
>> }};
>> }
>>
>> fn bar() {}
>>
>> fn main() {
>> foo!(());
>> foo!();
>> }
>> ```
>>
>> it expands to this:
>> ```
>> fn main() {
>> {
>> bar();
>> ()
>> };
>> {};
>> }
>> ```
>>
>> --
>> Cheers,
>> Benno
>>
On Sat, 24 Jun 2023 09:25:03 +0000
Benno Lossin <[email protected]> wrote:
> Add a derive proc-macro for the `Zeroable` trait. The macro supports
> structs where every field implements the `Zeroable` trait. This way
> `unsafe` implementations can be avoided.
>
> The macro is split into two parts:
> - a proc-macro to parse generics into impl and ty generics,
> - a declarative macro that expands to the impl block.
>
> Suggested-by: Asahi Lina <[email protected]>
> Signed-off-by: Benno Lossin <[email protected]>
Reviewed-by: Gary Guo <[email protected]>
> ---
> rust/kernel/init/macros.rs | 28 ++++++++++++++++++++++++++++
> rust/kernel/prelude.rs | 2 +-
> rust/macros/lib.rs | 20 ++++++++++++++++++++
> rust/macros/quote.rs | 6 ++++++
> rust/macros/zeroable.rs | 25 +++++++++++++++++++++++++
> 5 files changed, 80 insertions(+), 1 deletion(-)
> create mode 100644 rust/macros/zeroable.rs
>
> diff --git a/rust/kernel/init/macros.rs b/rust/kernel/init/macros.rs
> index fbaebd34f218..e8165ff53a94 100644
> --- a/rust/kernel/init/macros.rs
> +++ b/rust/kernel/init/macros.rs
> @@ -1213,3 +1213,31 @@ macro_rules! __init_internal {
> );
> };
> }
> +
> +#[doc(hidden)]
> +#[macro_export]
> +macro_rules! __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: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
> + $($field_ty: $crate::Zeroable,)*
> + $($($whr)*)?
> + {}
> + };
> +}
> diff --git a/rust/kernel/prelude.rs b/rust/kernel/prelude.rs
> index c28587d68ebc..ae21600970b3 100644
> --- a/rust/kernel/prelude.rs
> +++ b/rust/kernel/prelude.rs
> @@ -18,7 +18,7 @@
> pub use alloc::{boxed::Box, vec::Vec};
>
> #[doc(no_inline)]
> -pub use macros::{module, pin_data, pinned_drop, vtable};
> +pub use macros::{module, pin_data, pinned_drop, vtable, Zeroable};
>
> pub use super::build_assert;
>
> diff --git a/rust/macros/lib.rs b/rust/macros/lib.rs
> index 3fc74cb4ea19..9f056a5c780a 100644
> --- a/rust/macros/lib.rs
> +++ b/rust/macros/lib.rs
> @@ -10,6 +10,7 @@
> mod pin_data;
> mod pinned_drop;
> mod vtable;
> +mod zeroable;
>
> use proc_macro::TokenStream;
>
> @@ -246,3 +247,22 @@ pub fn pin_data(inner: TokenStream, item: TokenStream) -> TokenStream {
> pub fn pinned_drop(args: TokenStream, input: TokenStream) -> TokenStream {
> pinned_drop::pinned_drop(args, input)
> }
> +
> +/// Derives the [`Zeroable`] trait for the given struct.
> +///
> +/// This can only be used for structs where every field implements the [`Zeroable`] trait.
> +///
> +/// # Examples
> +///
> +/// ```rust
> +/// #[derive(Zeroable)]
> +/// pub struct DriverData {
> +/// id: i64,
> +/// buf_ptr: *mut u8,
> +/// len: usize,
> +/// }
> +/// ```
> +#[proc_macro_derive(Zeroable)]
> +pub fn derive_zeroable(input: TokenStream) -> TokenStream {
> + zeroable::derive(input)
> +}
> diff --git a/rust/macros/quote.rs b/rust/macros/quote.rs
> index dddbb4e6f4cb..b76c198a4ed5 100644
> --- a/rust/macros/quote.rs
> +++ b/rust/macros/quote.rs
> @@ -124,6 +124,12 @@ macro_rules! quote_spanned {
> ));
> quote_spanned!(@proc $v $span $($tt)*);
> };
> + (@proc $v:ident $span:ident ; $($tt:tt)*) => {
> + $v.push(::proc_macro::TokenTree::Punct(
> + ::proc_macro::Punct::new(';', ::proc_macro::Spacing::Alone)
> + ));
> + quote_spanned!(@proc $v $span $($tt)*);
> + };
> (@proc $v:ident $span:ident $id:ident $($tt:tt)*) => {
> $v.push(::proc_macro::TokenTree::Ident(::proc_macro::Ident::new(stringify!($id), $span)));
> quote_spanned!(@proc $v $span $($tt)*);
> diff --git a/rust/macros/zeroable.rs b/rust/macros/zeroable.rs
> new file mode 100644
> index 000000000000..cddb866c44ef
> --- /dev/null
> +++ b/rust/macros/zeroable.rs
> @@ -0,0 +1,25 @@
> +// SPDX-License-Identifier: GPL-2.0
> +
> +use crate::helpers::{parse_generics, Generics};
> +use proc_macro::TokenStream;
> +
> +pub(crate) fn derive(input: TokenStream) -> TokenStream {
> + let (
> + Generics {
> + impl_generics,
> + ty_generics,
> + },
> + mut rest,
> + ) = parse_generics(input);
> + // This should be the body of the struct `{...}`.
> + let last = rest.pop();
> + quote! {
> + ::kernel::__derive_zeroable!(
> + parse_input:
> + @sig(#(#rest)*),
> + @impl_generics(#(#impl_generics)*),
> + @ty_generics(#(#ty_generics)*),
> + @body(#last),
> + );
> + }
> +}
> --
> 2.41.0
>
>
On Sat, 24 Jun 2023 09:25:10 +0000
Benno Lossin <[email protected]> wrote:
> Use hygienic identifiers for the guards instead of the field names. This
> makes the init macros feel more like normal struct initializers, since
> assigning identifiers with the name of a field does not create
> conflicts.
> Also change the internals of the guards, no need to make the `forget`
> function `unsafe`, since users cannot access the guards anyways. Now the
> guards are carried directly on the stack and have no extra `Cell<bool>`
> field that marks if they have been forgotten or not, instead they are
> just forgotten via `mem::forget`.
The code LGTM, so:
Reviewed-by: Gary Guo <[email protected]>
Although this will cause the new expansion we have to be no longer
compatible with a totally-proc-macro impl, if we want to do everything
in proc macro in the future.
If we have the paste macro upstream (
https://github.com/nbdd0121/linux/commit/fff00461b0be7fd3ec218dcc428f25886b5ec04a
) then we can replace the `guard` with `paste!([<$field>])` and keep
the expansion identical.
>
> Suggested-by: Asahi Lina <[email protected]>
> Signed-off-by: Benno Lossin <[email protected]>
> ---
> rust/kernel/init.rs | 1 -
> rust/kernel/init/__internal.rs | 25 +++------------
> rust/kernel/init/macros.rs | 56 ++++++++++++----------------------
> 3 files changed, 23 insertions(+), 59 deletions(-)
>
> diff --git a/rust/kernel/init.rs b/rust/kernel/init.rs
> index d9a91950cba2..ecf6a4bd0ce4 100644
> --- a/rust/kernel/init.rs
> +++ b/rust/kernel/init.rs
> @@ -206,7 +206,6 @@
> use alloc::boxed::Box;
> use core::{
> alloc::AllocError,
> - cell::Cell,
> convert::Infallible,
> marker::PhantomData,
> mem::MaybeUninit,
> diff --git a/rust/kernel/init/__internal.rs b/rust/kernel/init/__internal.rs
> index 44751fb62b51..7abd1fb65e41 100644
> --- a/rust/kernel/init/__internal.rs
> +++ b/rust/kernel/init/__internal.rs
> @@ -174,7 +174,6 @@ pub fn init<E>(self: Pin<&mut Self>, init: impl PinInit<T, E>) -> Result<Pin<&mu
> /// Can be forgotten to prevent the drop.
> pub struct DropGuard<T: ?Sized> {
> ptr: *mut T,
> - do_drop: Cell<bool>,
> }
>
> impl<T: ?Sized> DropGuard<T> {
> @@ -190,32 +189,16 @@ impl<T: ?Sized> DropGuard<T> {
> /// - will not be dropped by any other means.
> #[inline]
> pub unsafe fn new(ptr: *mut T) -> Self {
> - Self {
> - ptr,
> - do_drop: Cell::new(true),
> - }
> - }
> -
> - /// Prevents this guard from dropping the supplied pointer.
> - ///
> - /// # Safety
> - ///
> - /// This function is unsafe in order to prevent safe code from forgetting this guard. It should
> - /// only be called by the macros in this module.
> - #[inline]
> - pub unsafe fn forget(&self) {
> - self.do_drop.set(false);
> + Self { ptr }
> }
> }
>
> impl<T: ?Sized> Drop for DropGuard<T> {
> #[inline]
> fn drop(&mut self) {
> - if self.do_drop.get() {
> - // SAFETY: A `DropGuard` can only be constructed using the unsafe `new` function
> - // ensuring that this operation is safe.
> - unsafe { ptr::drop_in_place(self.ptr) }
> - }
> + // SAFETY: A `DropGuard` can only be constructed using the unsafe `new` function
> + // ensuring that this operation is safe.
> + unsafe { ptr::drop_in_place(self.ptr) }
> }
> }
>
> diff --git a/rust/kernel/init/macros.rs b/rust/kernel/init/macros.rs
> index e8165ff53a94..df4281743175 100644
> --- a/rust/kernel/init/macros.rs
> +++ b/rust/kernel/init/macros.rs
> @@ -992,7 +992,6 @@ impl<$($impl_generics)*> $pin_data<$($ty_generics)*>
> /// - `init_slot`: recursively creates the code that initializes all fields in `slot`.
> /// - `make_initializer`: recursively create the struct initializer that guarantees that every
> /// field has been initialized exactly once.
> -/// - `forget_guards`: recursively forget the drop guards for every field.
> #[doc(hidden)]
> #[macro_export]
> macro_rules! __init_internal {
> @@ -1032,6 +1031,7 @@ macro_rules! __init_internal {
> $crate::__init_internal!(init_slot($($use_data)?):
> @data(data),
> @slot(slot),
> + @guards(),
> @munch_fields($($fields)*,),
> );
> // We use unreachable code to ensure that all fields have been mentioned exactly
> @@ -1046,10 +1046,6 @@ macro_rules! __init_internal {
> @acc(),
> );
> }
> - // Forget all guards, since initialization was a success.
> - $crate::__init_internal!(forget_guards:
> - @munch_fields($($fields)*,),
> - );
> }
> Ok(__InitOk)
> }
> @@ -1063,13 +1059,17 @@ macro_rules! __init_internal {
> (init_slot($($use_data:ident)?):
> @data($data:ident),
> @slot($slot:ident),
> + @guards($($guards:ident,)*),
> @munch_fields($(,)?),
> ) => {
> - // Endpoint of munching, no fields are left.
> + // Endpoint of munching, no fields are left. If execution reaches this point, all fields
> + // have been initialized. Therefore we can now dismiss the guards by forgetting them.
> + $(::core::mem::forget($guards);)*
> };
> (init_slot($use_data:ident): // use_data is present, so we use the `data` to init fields.
> @data($data:ident),
> @slot($slot:ident),
> + @guards($($guards:ident,)*),
> // In-place initialization syntax.
> @munch_fields($field:ident <- $val:expr, $($rest:tt)*),
> ) => {
> @@ -1080,24 +1080,26 @@ macro_rules! __init_internal {
> // return when an error/panic occurs.
> // We also use the `data` to require the correct trait (`Init` or `PinInit`) for `$field`.
> unsafe { $data.$field(::core::ptr::addr_of_mut!((*$slot).$field), $field)? };
> - // Create the drop guard.
> + // Create the drop guard:
> //
> - // We only give access to `&DropGuard`, so it cannot be forgotten via safe code.
> + // We rely on macro hygiene to make it impossible for users to access this local variable.
> //
> // SAFETY: We forget the guard later when initialization has succeeded.
> - let $field = &unsafe {
> + let guard = unsafe {
> $crate::init::__internal::DropGuard::new(::core::ptr::addr_of_mut!((*$slot).$field))
> };
>
> $crate::__init_internal!(init_slot($use_data):
> @data($data),
> @slot($slot),
> + @guards(guard, $($guards,)*),
> @munch_fields($($rest)*),
> );
> };
> (init_slot(): // no use_data, so we use `Init::__init` directly.
> @data($data:ident),
> @slot($slot:ident),
> + @guards($($guards:ident,)*),
> // In-place initialization syntax.
> @munch_fields($field:ident <- $val:expr, $($rest:tt)*),
> ) => {
> @@ -1107,24 +1109,26 @@ macro_rules! __init_internal {
> // SAFETY: `slot` is valid, because we are inside of an initializer closure, we
> // return when an error/panic occurs.
> unsafe { $crate::init::Init::__init($field, ::core::ptr::addr_of_mut!((*$slot).$field))? };
> - // Create the drop guard.
> + // Create the drop guard:
> //
> - // We only give access to `&DropGuard`, so it cannot be forgotten via safe code.
> + // We rely on macro hygiene to make it impossible for users to access this local variable.
> //
> // SAFETY: We forget the guard later when initialization has succeeded.
> - let $field = &unsafe {
> + let guard = unsafe {
> $crate::init::__internal::DropGuard::new(::core::ptr::addr_of_mut!((*$slot).$field))
> };
>
> $crate::__init_internal!(init_slot():
> @data($data),
> @slot($slot),
> + @guards(guard, $($guards,)*),
> @munch_fields($($rest)*),
> );
> };
> (init_slot($($use_data:ident)?):
> @data($data:ident),
> @slot($slot:ident),
> + @guards($($guards:ident,)*),
> // Init by-value.
> @munch_fields($field:ident $(: $val:expr)?, $($rest:tt)*),
> ) => {
> @@ -1135,16 +1139,17 @@ macro_rules! __init_internal {
> unsafe { ::core::ptr::write(::core::ptr::addr_of_mut!((*$slot).$field), $field) };
> // Create the drop guard:
> //
> - // We only give access to `&DropGuard`, so it cannot be accidentally forgotten.
> + // We rely on macro hygiene to make it impossible for users to access this local variable.
> //
> // SAFETY: We forget the guard later when initialization has succeeded.
> - let $field = &unsafe {
> + let guard = unsafe {
> $crate::init::__internal::DropGuard::new(::core::ptr::addr_of_mut!((*$slot).$field))
> };
>
> $crate::__init_internal!(init_slot($($use_data)?):
> @data($data),
> @slot($slot),
> + @guards(guard, $($guards,)*),
> @munch_fields($($rest)*),
> );
> };
> @@ -1189,29 +1194,6 @@ macro_rules! __init_internal {
> @acc($($acc)* $field: ::core::panic!(),),
> );
> };
> - (forget_guards:
> - @munch_fields($(,)?),
> - ) => {
> - // Munching finished.
> - };
> - (forget_guards:
> - @munch_fields($field:ident <- $val:expr, $($rest:tt)*),
> - ) => {
> - unsafe { $crate::init::__internal::DropGuard::forget($field) };
> -
> - $crate::__init_internal!(forget_guards:
> - @munch_fields($($rest)*),
> - );
> - };
> - (forget_guards:
> - @munch_fields($field:ident $(: $val:expr)?, $($rest:tt)*),
> - ) => {
> - unsafe { $crate::init::__internal::DropGuard::forget($field) };
> -
> - $crate::__init_internal!(forget_guards:
> - @munch_fields($($rest)*),
> - );
> - };
> }
>
> #[doc(hidden)]
> --
> 2.41.0
>
>
On Sat, 24 Jun 2023 09:25:39 +0000
Benno Lossin <[email protected]> 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 <[email protected]>
> Signed-off-by: Benno Lossin <[email protected]>
> ---
> 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
>
>
On 25.06.23 23:01, Gary Guo wrote:
> On Sat, 24 Jun 2023 09:25:39 +0000
> Benno Lossin <[email protected]> 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?
I tried your patch and it seems to work. I also executed all of the test
in the userspace library and they passed.
The `paste!` code also looks good to me. One thing that I thought was this:
do we want to accept `paste!( [<= foo bar>])`? Because the `<` token has
spacing `Joint`, maybe add a check for that? No idea if that would be a problem.
--
Cheers,
Benno
>
> Best,
> Gary
>
On 25.06.23 22:54, Gary Guo wrote:
> On Sat, 24 Jun 2023 09:25:10 +0000
> Benno Lossin <[email protected]> wrote:
>
>> Use hygienic identifiers for the guards instead of the field names. This
>> makes the init macros feel more like normal struct initializers, since
>> assigning identifiers with the name of a field does not create
>> conflicts.
>> Also change the internals of the guards, no need to make the `forget`
>> function `unsafe`, since users cannot access the guards anyways. Now the
>> guards are carried directly on the stack and have no extra `Cell<bool>`
>> field that marks if they have been forgotten or not, instead they are
>> just forgotten via `mem::forget`.
>
> The code LGTM, so:
>
> Reviewed-by: Gary Guo <[email protected]>
>
> Although this will cause the new expansion we have to be no longer
> compatible with a totally-proc-macro impl, if we want to do everything
> in proc macro in the future.
>
> If we have the paste macro upstream (
> https://github.com/nbdd0121/linux/commit/fff00461b0be7fd3ec218dcc428f25886b5ec04a
> ) then we can replace the `guard` with `paste!([<$field>])` and keep
> the expansion identical.
>
I tried it and it seems to work, but I am not sure why the hygiene is
set correctly. Could you maybe explain why this works?
```
$crate::__internal::paste!{
let [<$field>] = unsafe {
$crate::__internal::DropGuard::new(::core::ptr::addr_of_mut!((*$slot).$field))
};
$crate::__init_internal!(init_slot($use_data):
@data($data),
@slot($slot),
@guards([<$field>], $($guards,)*),
@munch_fields($($rest)*),
);
}
```
i.e. why can't a user access the guard? I think it is because the hygiene of the `[<>]`
is used, but not sure why that works.
--
Cheers,
Benno
On Wed, 28 Jun 2023 11:41:59 +0000
Benno Lossin <[email protected]> wrote:
> On 25.06.23 22:54, Gary Guo wrote:
> > On Sat, 24 Jun 2023 09:25:10 +0000
> > Benno Lossin <[email protected]> wrote:
> >
> >> Use hygienic identifiers for the guards instead of the field names. This
> >> makes the init macros feel more like normal struct initializers, since
> >> assigning identifiers with the name of a field does not create
> >> conflicts.
> >> Also change the internals of the guards, no need to make the `forget`
> >> function `unsafe`, since users cannot access the guards anyways. Now the
> >> guards are carried directly on the stack and have no extra `Cell<bool>`
> >> field that marks if they have been forgotten or not, instead they are
> >> just forgotten via `mem::forget`.
> >
> > The code LGTM, so:
> >
> > Reviewed-by: Gary Guo <[email protected]>
> >
> > Although this will cause the new expansion we have to be no longer
> > compatible with a totally-proc-macro impl, if we want to do everything
> > in proc macro in the future.
> >
> > If we have the paste macro upstream (
> > https://github.com/nbdd0121/linux/commit/fff00461b0be7fd3ec218dcc428f25886b5ec04a
> > ) then we can replace the `guard` with `paste!([<$field>])` and keep
> > the expansion identical.
> >
>
> I tried it and it seems to work, but I am not sure why the hygiene is
> set correctly. Could you maybe explain why this works?
> ```
> $crate::__internal::paste!{
> let [<$field>] = unsafe {
> $crate::__internal::DropGuard::new(::core::ptr::addr_of_mut!((*$slot).$field))
> };
> $crate::__init_internal!(init_slot($use_data):
> @data($data),
> @slot($slot),
> @guards([<$field>], $($guards,)*),
> @munch_fields($($rest)*),
> );
> }
> ```
>
> i.e. why can't a user access the guard? I think it is because the hygiene of the `[<>]`
> is used, but not sure why that works.
Yes, by default the hygiene of pasted macro is that of the group,
unless explicitly overriden.
Best,
Gary
On Wed, 28 Jun 2023 11:26:54 +0000
Benno Lossin <[email protected]> wrote:
> On 25.06.23 23:01, Gary Guo wrote:
> > On Sat, 24 Jun 2023 09:25:39 +0000
> > Benno Lossin <[email protected]> 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?
>
> I tried your patch and it seems to work. I also executed all of the test
> in the userspace library and they passed.
> The `paste!` code also looks good to me. One thing that I thought was this:
> do we want to accept `paste!( [<= foo bar>])`? Because the `<` token has
> spacing `Joint`, maybe add a check for that? No idea if that would be a problem.
>
I don't think checking that is necessary, since that wouldn't be
valid Rust anyway?
Best,
Gary
Benno Lossin <[email protected]> writes:
> Add a derive proc-macro for the `Zeroable` trait. The macro supports
> structs where every field implements the `Zeroable` trait. This way
> `unsafe` implementations can be avoided.
>
> The macro is split into two parts:
> - a proc-macro to parse generics into impl and ty generics,
> - a declarative macro that expands to the impl block.
>
> Suggested-by: Asahi Lina <[email protected]>
> Signed-off-by: Benno Lossin <[email protected]>
Reviewed-by: Alice Ryhl <[email protected]>
Benno Lossin <[email protected]> writes:
> Add the struct update syntax to the init macros, but only for
> `..Zeroable::zeroed()`. Adding this at the end of the struct initializer
> allows one to omit fields from the initializer, these fields will be
> initialized with 0x00 set to every byte. Only types that implement the
> `Zeroable` trait can utilize this.
>
> Suggested-by: Asahi Lina <[email protected]>
> Signed-off-by: Benno Lossin <[email protected]>
I'm a bit surprised by how large this change is, but it looks ok.
Reviewed-by: Alice Ryhl <[email protected]>
Benno Lossin <[email protected]> writes:
> Add two functions `pin_init_array_from_fn` and `init_array_from_fn` that
> take a function that generates initializers for `T` from usize, the added
> functions then return an initializer for `[T; N]` where every element is
> initialized by an element returned from the generator function.
>
> Suggested-by: Asahi Lina <[email protected]>
> Signed-off-by: Benno Lossin <[email protected]>
> ---
> +/// Initializes an array by initializing each element via the provided initializer.
> +///
> +/// # Examples
> +///
> +/// ```rust
> +/// let array: Box<[usize; 1000_000_000]>= Box::init(init_array_from_fn(|i| i)).unwrap();
> +/// println!("{array:?}");
> +/// ```
> +pub fn init_array_from_fn<I, const N: usize, T, E>(
> + mut make_init: impl FnMut(usize) -> I,
> +) -> impl Init<[T; N], E>
> +where
> + I: Init<T, E>,
> +{
> + let init = move |slot: *mut [T; N]| {
> + let slot = slot.cast::<T>();
> + for i in 0..N {
> + let init = make_init(i);
> + // SAFETY: since 0 <= `i` < N, it is still in bounds of `[T; N]`.
> + let ptr = unsafe { slot.add(i) };
> + // SAFETY: The pointer is derived from `slot` and thus satisfies the `__init`
> + // requirements.
> + match unsafe { init.__init(ptr) } {
> + Ok(()) => {}
> + Err(e) => {
> + // We now free every element that has been initialized before:
> + for j in 0..i {
> + let ptr = unsafe { slot.add(j) };
> + // SAFETY: The value was initialized in a previous iteration of the loop
> + // and since we return `Err` below, the caller will consider the memory at
> + // `slot` as uninitialized.
> + unsafe { ptr::drop_in_place(ptr) };
> + }
The loop can be simplified like this:
ptr::drop_in_place(ptr::slice_from_raw_parts(slot, i));
Yes, this actually works and will run the destructor of each value.
Alice
On Sat, Jun 24, 2023 at 09:25:19AM +0000, Benno Lossin wrote:
[...]
(this is `init_slot`)
> @@ -1064,7 +1152,7 @@ macro_rules! __init_internal {
> @data($data:ident),
> @slot($slot:ident),
> @guards($($guards:ident,)*),
> - @munch_fields($(,)?),
> + @munch_fields($(..Zeroable::zeroed())? $(,)?),
since you append an unconditional comma ',' to init_slot and
make_initializer when "calling" them in with_update_parsed, shouldn't
this be:
+ @munch_fields($(..Zeroable::zeroed(),)? $(,)?),
, and..
> ) => {
> // Endpoint of munching, no fields are left. If execution reaches this point, all fields
> // have been initialized. Therefore we can now dismiss the guards by forgetting them.
> @@ -1157,6 +1245,30 @@ macro_rules! __init_internal {
> @munch_fields($($rest)*),
> );
> };
> + (make_initializer:
> + @slot($slot:ident),
> + @type_name($t:ident),
> + @munch_fields(..Zeroable::zeroed() $(,)?),
this should be:
+ @munch_fields(..Zeroable::zeroed() , $(,)?),
Otherwise the example before `pin_init!()` wouldn't compile:
/// pin_init!(Buf {
/// buf: [1; 64],
/// ..Zeroable::zeroed(),
/// });
Regards,
Boqun
> + @acc($($acc:tt)*),
> + ) => {
> + // Endpoint, nothing more to munch, create the initializer. Since the users specified
> + // `..Zeroable::zeroed()`, the slot will already have been zeroed and all field that have
> + // not been overwritten are thus zero and initialized. We still check that all fields are
> + // actually accessible by using the struct update syntax ourselves.
> + // Since we are in the `if false` branch, this will never get executed. We abuse `slot` to
> + // get the correct type inference here:
> + unsafe {
> + let mut zeroed = ::core::mem::zeroed();
> + // We have to use type inference her to make zeroed have the correct type. This does
> + // not get executed, so it has no effect.
> + ::core::ptr::write($slot, zeroed);
> + zeroed = ::core::mem::zeroed();
> + ::core::ptr::write($slot, $t {
> + $($acc)*
> + ..zeroed
> + });
> + }
> + };
> (make_initializer:
> @slot($slot:ident),
> @type_name($t:ident),
> --
> 2.41.0
>
>
On Mon, 3 Jul 2023 11:15:55 -0700
Boqun Feng <[email protected]> wrote:
> On Sat, Jun 24, 2023 at 09:25:19AM +0000, Benno Lossin wrote:
> [...]
> (this is `init_slot`)
> > @@ -1064,7 +1152,7 @@ macro_rules! __init_internal {
> > @data($data:ident),
> > @slot($slot:ident),
> > @guards($($guards:ident,)*),
> > - @munch_fields($(,)?),
> > + @munch_fields($(..Zeroable::zeroed())? $(,)?),
>
> since you append an unconditional comma ',' to init_slot and
> make_initializer when "calling" them in with_update_parsed, shouldn't
> this be:
>
> + @munch_fields($(..Zeroable::zeroed(),)? $(,)?),
>
> , and..
>
> > ) => {
> > // Endpoint of munching, no fields are left. If execution reaches this point, all fields
> > // have been initialized. Therefore we can now dismiss the guards by forgetting them.
> > @@ -1157,6 +1245,30 @@ macro_rules! __init_internal {
> > @munch_fields($($rest)*),
> > );
> > };
> > + (make_initializer:
> > + @slot($slot:ident),
> > + @type_name($t:ident),
> > + @munch_fields(..Zeroable::zeroed() $(,)?),
>
> this should be:
>
> + @munch_fields(..Zeroable::zeroed() , $(,)?),
>
> Otherwise the example before `pin_init!()` wouldn't compile:
>
> /// pin_init!(Buf {
> /// buf: [1; 64],
> /// ..Zeroable::zeroed(),
> /// });
Comma is not allowed after base struct.
>
> Regards,
> Boqun
>
> > + @acc($($acc:tt)*),
> > + ) => {
> > + // Endpoint, nothing more to munch, create the initializer. Since the users specified
> > + // `..Zeroable::zeroed()`, the slot will already have been zeroed and all field that have
> > + // not been overwritten are thus zero and initialized. We still check that all fields are
> > + // actually accessible by using the struct update syntax ourselves.
> > + // Since we are in the `if false` branch, this will never get executed. We abuse `slot` to
> > + // get the correct type inference here:
> > + unsafe {
> > + let mut zeroed = ::core::mem::zeroed();
> > + // We have to use type inference her to make zeroed have the correct type. This does
> > + // not get executed, so it has no effect.
> > + ::core::ptr::write($slot, zeroed);
> > + zeroed = ::core::mem::zeroed();
> > + ::core::ptr::write($slot, $t {
> > + $($acc)*
> > + ..zeroed
> > + });
> > + }
> > + };
> > (make_initializer:
> > @slot($slot:ident),
> > @type_name($t:ident),
> > --
> > 2.41.0
> >
> >
On 05.07.23 19:48, Gary Guo wrote:
> On Mon, 3 Jul 2023 11:15:55 -0700
> Boqun Feng <[email protected]> wrote:
>
>> On Sat, Jun 24, 2023 at 09:25:19AM +0000, Benno Lossin wrote:
>> [...]
>> (this is `init_slot`)
>>> @@ -1064,7 +1152,7 @@ macro_rules! __init_internal {
>>> @data($data:ident),
>>> @slot($slot:ident),
>>> @guards($($guards:ident,)*),
>>> - @munch_fields($(,)?),
>>> + @munch_fields($(..Zeroable::zeroed())? $(,)?),
>>
>> since you append an unconditional comma ',' to init_slot and
>> make_initializer when "calling" them in with_update_parsed, shouldn't
>> this be:
>>
>> + @munch_fields($(..Zeroable::zeroed(),)? $(,)?),
>>
>> , and..
>>
>>> ) => {
>>> // Endpoint of munching, no fields are left. If execution reaches this point, all fields
>>> // have been initialized. Therefore we can now dismiss the guards by forgetting them.
>>> @@ -1157,6 +1245,30 @@ macro_rules! __init_internal {
>>> @munch_fields($($rest)*),
>>> );
>>> };
>>> + (make_initializer:
>>> + @slot($slot:ident),
>>> + @type_name($t:ident),
>>> + @munch_fields(..Zeroable::zeroed() $(,)?),
>>
>> this should be:
>>
>> + @munch_fields(..Zeroable::zeroed() , $(,)?),
>>
>> Otherwise the example before `pin_init!()` wouldn't compile:
>>
>> /// pin_init!(Buf {
>> /// buf: [1; 64],
>> /// ..Zeroable::zeroed(),
>> /// });
>
> Comma is not allowed after base struct.
Yes this is a mistake in the example, will fix.
--
Cheers,
Benno
>
>>
>> Regards,
>> Boqun
>>
>>> + @acc($($acc:tt)*),
>>> + ) => {
>>> + // Endpoint, nothing more to munch, create the initializer. Since the users specified
>>> + // `..Zeroable::zeroed()`, the slot will already have been zeroed and all field that have
>>> + // not been overwritten are thus zero and initialized. We still check that all fields are
>>> + // actually accessible by using the struct update syntax ourselves.
>>> + // Since we are in the `if false` branch, this will never get executed. We abuse `slot` to
>>> + // get the correct type inference here:
>>> + unsafe {
>>> + let mut zeroed = ::core::mem::zeroed();
>>> + // We have to use type inference her to make zeroed have the correct type. This does
>>> + // not get executed, so it has no effect.
>>> + ::core::ptr::write($slot, zeroed);
>>> + zeroed = ::core::mem::zeroed();
>>> + ::core::ptr::write($slot, $t {
>>> + $($acc)*
>>> + ..zeroed
>>> + });
>>> + }
>>> + };
>>> (make_initializer:
>>> @slot($slot:ident),
>>> @type_name($t:ident),
>>> --
>>> 2.41.0
>>>
>>>
>