rust/pin-init/src/macros.rs | 181 ++++++++++++++++++++++++++++-------- 1 file changed, 142 insertions(+), 39 deletions(-)
Assigning a field a value in an initializer macro can be marked with the
`#[bind]` attribute. Doing so creates a `let` binding with the same
name. This `let` binding has the type `Pin<&mut T>` if the field is
structurally pinned or `&mut T` otherwise (where `T` is the type of the
field).
Signed-off-by: Benno Lossin <lossin@kernel.org>
---
Changes from v1:
* require explicit annotation through `#[bind]` to create the let
binding
* this removes the need to patch existing uses of initializer macros
---
rust/pin-init/src/macros.rs | 181 ++++++++++++++++++++++++++++--------
1 file changed, 142 insertions(+), 39 deletions(-)
diff --git a/rust/pin-init/src/macros.rs b/rust/pin-init/src/macros.rs
index 9ced630737b8..21798a9d195f 100644
--- a/rust/pin-init/src/macros.rs
+++ b/rust/pin-init/src/macros.rs
@@ -988,38 +988,56 @@ fn drop(&mut self) {
@pinned($($(#[$($p_attr:tt)*])* $pvis:vis $p_field:ident : $p_type:ty),* $(,)?),
@not_pinned($($(#[$($attr:tt)*])* $fvis:vis $field:ident : $type:ty),* $(,)?),
) => {
- // For every field, we create a projection function according to its projection type. If a
- // field is structurally pinned, then it must be initialized via `PinInit`, if it is not
- // structurally pinned, then it can be initialized via `Init`.
- //
- // The functions are `unsafe` to prevent accidentally calling them.
- #[allow(dead_code)]
- #[expect(clippy::missing_safety_doc)]
- impl<$($impl_generics)*> $pin_data<$($ty_generics)*>
- where $($whr)*
- {
- $(
- $(#[$($p_attr)*])*
- $pvis unsafe fn $p_field<E>(
- self,
- slot: *mut $p_type,
- init: impl $crate::PinInit<$p_type, E>,
- ) -> ::core::result::Result<(), E> {
- // SAFETY: TODO.
- unsafe { $crate::PinInit::__pinned_init(init, slot) }
- }
- )*
- $(
- $(#[$($attr)*])*
- $fvis unsafe fn $field<E>(
- self,
- slot: *mut $type,
- init: impl $crate::Init<$type, E>,
- ) -> ::core::result::Result<(), E> {
- // SAFETY: TODO.
- unsafe { $crate::Init::__init(init, slot) }
- }
- )*
+ $crate::macros::paste! {
+ // For every field, we create a projection function according to its projection type. If a
+ // field is structurally pinned, then it must be initialized via `PinInit`, if it is not
+ // structurally pinned, then it can be initialized via `Init`.
+ //
+ // The functions are `unsafe` to prevent accidentally calling them.
+ #[allow(dead_code)]
+ #[expect(clippy::missing_safety_doc)]
+ impl<$($impl_generics)*> $pin_data<$($ty_generics)*>
+ where $($whr)*
+ {
+ $(
+ $(#[$($p_attr)*])*
+ $pvis unsafe fn $p_field<E>(
+ self,
+ slot: *mut $p_type,
+ init: impl $crate::PinInit<$p_type, E>,
+ ) -> ::core::result::Result<(), E> {
+ // SAFETY: TODO.
+ unsafe { $crate::PinInit::__pinned_init(init, slot) }
+ }
+
+ $(#[$($p_attr)*])*
+ $pvis unsafe fn [<__project_ $p_field>]<'__slot>(
+ self,
+ slot: &'__slot mut $p_type,
+ ) -> ::core::pin::Pin<&'__slot mut $p_type> {
+ ::core::pin::Pin::new_unchecked(slot)
+ }
+ )*
+ $(
+ $(#[$($attr)*])*
+ $fvis unsafe fn $field<E>(
+ self,
+ slot: *mut $type,
+ init: impl $crate::Init<$type, E>,
+ ) -> ::core::result::Result<(), E> {
+ // SAFETY: TODO.
+ unsafe { $crate::Init::__init(init, slot) }
+ }
+
+ $(#[$($attr)*])*
+ $fvis unsafe fn [<__project_ $field>]<'__slot>(
+ self,
+ slot: &'__slot mut $type,
+ ) -> &'__slot mut $type {
+ slot
+ }
+ )*
+ }
}
};
}
@@ -1207,7 +1225,7 @@ fn assert_zeroable<T: $crate::Zeroable>(_: *mut T) {}
@slot($slot:ident),
@guards($($guards:ident,)*),
// In-place initialization syntax.
- @munch_fields($field:ident <- $val:expr, $($rest:tt)*),
+ @munch_fields($(#[$bind:ident])? $field:ident <- $val:expr, $($rest:tt)*),
) => {
let init = $val;
// Call the initializer.
@@ -1216,6 +1234,11 @@ fn assert_zeroable<T: $crate::Zeroable>(_: *mut T) {}
// 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), init)? };
+ $crate::__init_internal!(bind($(#[$bind])?):
+ @field($field),
+ @slot($slot),
+ @data($data),
+ );
// Create the drop guard:
//
// We rely on macro hygiene to make it impossible for users to access this local variable.
@@ -1239,7 +1262,7 @@ fn assert_zeroable<T: $crate::Zeroable>(_: *mut T) {}
@slot($slot:ident),
@guards($($guards:ident,)*),
// In-place initialization syntax.
- @munch_fields($field:ident <- $val:expr, $($rest:tt)*),
+ @munch_fields($(#[$bind:ident])? $field:ident <- $val:expr, $($rest:tt)*),
) => {
let init = $val;
// Call the initializer.
@@ -1247,6 +1270,13 @@ fn assert_zeroable<T: $crate::Zeroable>(_: *mut T) {}
// SAFETY: `slot` is valid, because we are inside of an initializer closure, we
// return when an error/panic occurs.
unsafe { $crate::Init::__init(init, ::core::ptr::addr_of_mut!((*$slot).$field))? };
+
+ $crate::__init_internal!(bind($(#[$bind])?):
+ @field($field),
+ @slot($slot),
+ @data(),
+ );
+
// Create the drop guard:
//
// We rely on macro hygiene to make it impossible for users to access this local variable.
@@ -1265,12 +1295,51 @@ fn assert_zeroable<T: $crate::Zeroable>(_: *mut T) {}
);
}
};
- (init_slot($($use_data:ident)?):
+ (init_slot(): // No `use_data`, so all fields are not structurally pinned
+ @data($data:ident),
+ @slot($slot:ident),
+ @guards($($guards:ident,)*),
+ // Init by-value.
+ @munch_fields($(#[$bind:ident])? $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) };
+ }
+
+ $crate::__init_internal!(bind($(#[$bind])?):
+ @field($field),
+ @slot($slot),
+ @data(),
+ );
+
+ // Create the drop guard:
+ //
+ // We rely on macro hygiene to make it impossible for users to access this local variable.
+ // We use `paste!` to create new hygiene for `$field`.
+ $crate::macros::paste! {
+ // SAFETY: We forget the guard later when initialization has succeeded.
+ let [< __ $field _guard >] = unsafe {
+ $crate::__internal::DropGuard::new(::core::ptr::addr_of_mut!((*$slot).$field))
+ };
+
+ $crate::__init_internal!(init_slot():
+ @data($data),
+ @slot($slot),
+ @guards([< __ $field _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)*),
+ @munch_fields($(#[$bind:ident])? $field:ident $(: $val:expr)?, $($rest:tt)*),
) => {
{
$(let $field = $val;)?
@@ -1279,6 +1348,12 @@ fn assert_zeroable<T: $crate::Zeroable>(_: *mut T) {}
// SAFETY: The memory at `slot` is uninitialized.
unsafe { ::core::ptr::write(::core::ptr::addr_of_mut!((*$slot).$field), $field) };
}
+ $crate::__init_internal!(bind($(#[$bind])?):
+ @field($field),
+ @slot($slot),
+ @data($data),
+ );
+
// Create the drop guard:
//
// We rely on macro hygiene to make it impossible for users to access this local variable.
@@ -1289,7 +1364,7 @@ fn assert_zeroable<T: $crate::Zeroable>(_: *mut T) {}
$crate::__internal::DropGuard::new(::core::ptr::addr_of_mut!((*$slot).$field))
};
- $crate::__init_internal!(init_slot($($use_data)?):
+ $crate::__init_internal!(init_slot($use_data):
@data($data),
@slot($slot),
@guards([< __ $field _guard >], $($guards,)*),
@@ -1297,6 +1372,34 @@ fn assert_zeroable<T: $crate::Zeroable>(_: *mut T) {}
);
}
};
+ (bind(#[bind]):
+ @field($field:ident),
+ @slot($slot:ident),
+ @data($data:ident),
+ ) => {
+ // SAFETY:
+ // - the project function does the correct field projection,
+ // - the field has been initialized,
+ // - the reference is only valid until the end of the initializer.
+ let $field = $crate::macros::paste!(unsafe { $data.[< __project_ $field >](&mut (*$slot).$field) });
+ };
+ (bind(#[bind]):
+ @field($field:ident),
+ @slot($slot:ident),
+ @data(),
+ ) => {
+ // SAFETY:
+ // - the field is not structurally pinned, since no `use_data` was required to create this
+ // initializer,
+ // - the field has been initialized,
+ // - the reference is only valid until the end of the initializer.
+ let $field = unsafe { &mut (*$slot).$field };
+ };
+ (bind():
+ @field($field:ident),
+ @slot($slot:ident),
+ @data($($data:ident)?),
+ ) => {};
(make_initializer:
@slot($slot:ident),
@type_name($t:path),
@@ -1354,7 +1457,7 @@ fn assert_zeroable<T: $crate::Zeroable>(_: *mut T) {}
(make_initializer:
@slot($slot:ident),
@type_name($t:path),
- @munch_fields($field:ident <- $val:expr, $($rest:tt)*),
+ @munch_fields($(#[$bind:ident])? $field:ident <- $val:expr, $($rest:tt)*),
@acc($($acc:tt)*),
) => {
$crate::__init_internal!(make_initializer:
@@ -1367,7 +1470,7 @@ fn assert_zeroable<T: $crate::Zeroable>(_: *mut T) {}
(make_initializer:
@slot($slot:ident),
@type_name($t:path),
- @munch_fields($field:ident $(: $val:expr)?, $($rest:tt)*),
+ @munch_fields($(#[$bind:ident])? $field:ident $(: $val:expr)?, $($rest:tt)*),
@acc($($acc:tt)*),
) => {
$crate::__init_internal!(make_initializer:
base-commit: 8f5ae30d69d7543eee0d70083daf4de8fe15d585
--
2.50.1On Wed, Sep 10, 2025 at 12:07:53PM +0200, Benno Lossin wrote: > Assigning a field a value in an initializer macro can be marked with the > `#[bind]` attribute. Doing so creates a `let` binding with the same > name. This `let` binding has the type `Pin<&mut T>` if the field is > structurally pinned or `&mut T` otherwise (where `T` is the type of the > field). > > Signed-off-by: Benno Lossin <lossin@kernel.org> Is there a reason we can't apply this to all fields and avoid the attribute? Do we have a place that might be able to use this? Alice
On Wed Sep 10, 2025 at 12:17 PM CEST, Alice Ryhl wrote: > On Wed, Sep 10, 2025 at 12:07:53PM +0200, Benno Lossin wrote: >> Assigning a field a value in an initializer macro can be marked with the >> `#[bind]` attribute. Doing so creates a `let` binding with the same >> name. This `let` binding has the type `Pin<&mut T>` if the field is >> structurally pinned or `&mut T` otherwise (where `T` is the type of the >> field). >> >> Signed-off-by: Benno Lossin <lossin@kernel.org> > > Is there a reason we can't apply this to all fields and avoid the > attribute? Adding the attribute was due to Boqun's concern on v1 [1]. I think it might be surprising too, but I'm also happy with no attribute. [1]: https://lore.kernel.org/all/aLshd0_C-1rh3FAg@tardis-2.local > Do we have a place that might be able to use this? I didn't find one, but Danilo plans to base some changes on top this cycle that need this. --- Cheers, Benno
On Wed, Sep 10, 2025 at 12:36 PM Benno Lossin <lossin@kernel.org> wrote: > > On Wed Sep 10, 2025 at 12:17 PM CEST, Alice Ryhl wrote: > > On Wed, Sep 10, 2025 at 12:07:53PM +0200, Benno Lossin wrote: > >> Assigning a field a value in an initializer macro can be marked with the > >> `#[bind]` attribute. Doing so creates a `let` binding with the same > >> name. This `let` binding has the type `Pin<&mut T>` if the field is > >> structurally pinned or `&mut T` otherwise (where `T` is the type of the > >> field). > >> > >> Signed-off-by: Benno Lossin <lossin@kernel.org> > > > > Is there a reason we can't apply this to all fields and avoid the > > attribute? > > Adding the attribute was due to Boqun's concern on v1 [1]. I think it > might be surprising too, but I'm also happy with no attribute. > > [1]: https://lore.kernel.org/all/aLshd0_C-1rh3FAg@tardis-2.local IMO the ideal is if it works without an attribute. Perhaps trying that in the kernel is a reasonable experiment to find out whether that's reasonable to do for the general language feature? > > Do we have a place that might be able to use this? > > I didn't find one, but Danilo plans to base some changes on top this > cycle that need this. Danilo, what plans do you have? Alice
On Wed Sep 10, 2025 at 12:40 PM CEST, Alice Ryhl wrote: > On Wed, Sep 10, 2025 at 12:36 PM Benno Lossin <lossin@kernel.org> wrote: >> >> On Wed Sep 10, 2025 at 12:17 PM CEST, Alice Ryhl wrote: >> > On Wed, Sep 10, 2025 at 12:07:53PM +0200, Benno Lossin wrote: >> >> Assigning a field a value in an initializer macro can be marked with the >> >> `#[bind]` attribute. Doing so creates a `let` binding with the same >> >> name. This `let` binding has the type `Pin<&mut T>` if the field is >> >> structurally pinned or `&mut T` otherwise (where `T` is the type of the >> >> field). >> >> >> >> Signed-off-by: Benno Lossin <lossin@kernel.org> >> > >> > Is there a reason we can't apply this to all fields and avoid the >> > attribute? >> >> Adding the attribute was due to Boqun's concern on v1 [1]. I think it >> might be surprising too, but I'm also happy with no attribute. >> >> [1]: https://lore.kernel.org/all/aLshd0_C-1rh3FAg@tardis-2.local > > IMO the ideal is if it works without an attribute. Perhaps trying that > in the kernel is a reasonable experiment to find out whether that's > reasonable to do for the general language feature? @Boqun what is your opinion on this? I'm open to take v2 or v1, whatever you guys prefer. --- Cheers, Benno
On Wed, Sep 10, 2025 at 02:19:00PM +0200, Benno Lossin wrote: > On Wed Sep 10, 2025 at 12:40 PM CEST, Alice Ryhl wrote: > > On Wed, Sep 10, 2025 at 12:36 PM Benno Lossin <lossin@kernel.org> wrote: > >> > >> On Wed Sep 10, 2025 at 12:17 PM CEST, Alice Ryhl wrote: > >> > On Wed, Sep 10, 2025 at 12:07:53PM +0200, Benno Lossin wrote: > >> >> Assigning a field a value in an initializer macro can be marked with the > >> >> `#[bind]` attribute. Doing so creates a `let` binding with the same > >> >> name. This `let` binding has the type `Pin<&mut T>` if the field is > >> >> structurally pinned or `&mut T` otherwise (where `T` is the type of the > >> >> field). > >> >> > >> >> Signed-off-by: Benno Lossin <lossin@kernel.org> > >> > > >> > Is there a reason we can't apply this to all fields and avoid the > >> > attribute? > >> > >> Adding the attribute was due to Boqun's concern on v1 [1]. I think it > >> might be surprising too, but I'm also happy with no attribute. > >> > >> [1]: https://lore.kernel.org/all/aLshd0_C-1rh3FAg@tardis-2.local > > > > IMO the ideal is if it works without an attribute. Perhaps trying that > > in the kernel is a reasonable experiment to find out whether that's > > reasonable to do for the general language feature? > > @Boqun what is your opinion on this? > If we plan to make the in-place initializer language feature behave similar, as I asked here [1], then dropping `#[bind]` seems good to me. [1]: https://lore.kernel.org/rust-for-linux/aLshd0_C-1rh3FAg@tardis-2.local/ Thanks! Regards, Boqun > I'm open to take v2 or v1, whatever you guys prefer. > > --- > Cheers, > Benno
On Wed Sep 10, 2025 at 4:02 PM CEST, Boqun Feng wrote: > On Wed, Sep 10, 2025 at 02:19:00PM +0200, Benno Lossin wrote: >> On Wed Sep 10, 2025 at 12:40 PM CEST, Alice Ryhl wrote: >> > On Wed, Sep 10, 2025 at 12:36 PM Benno Lossin <lossin@kernel.org> wrote: >> >> >> >> On Wed Sep 10, 2025 at 12:17 PM CEST, Alice Ryhl wrote: >> >> > On Wed, Sep 10, 2025 at 12:07:53PM +0200, Benno Lossin wrote: >> >> >> Assigning a field a value in an initializer macro can be marked with the >> >> >> `#[bind]` attribute. Doing so creates a `let` binding with the same >> >> >> name. This `let` binding has the type `Pin<&mut T>` if the field is >> >> >> structurally pinned or `&mut T` otherwise (where `T` is the type of the >> >> >> field). >> >> >> >> >> >> Signed-off-by: Benno Lossin <lossin@kernel.org> >> >> > >> >> > Is there a reason we can't apply this to all fields and avoid the >> >> > attribute? >> >> >> >> Adding the attribute was due to Boqun's concern on v1 [1]. I think it >> >> might be surprising too, but I'm also happy with no attribute. >> >> >> >> [1]: https://lore.kernel.org/all/aLshd0_C-1rh3FAg@tardis-2.local >> > >> > IMO the ideal is if it works without an attribute. Perhaps trying that >> > in the kernel is a reasonable experiment to find out whether that's >> > reasonable to do for the general language feature? >> >> @Boqun what is your opinion on this? >> > > If we plan to make the in-place initializer language feature behave > similar, as I asked here [1], then dropping `#[bind]` seems good to me. I don't think we can promise how that language feature is going to look like. It will definitely support accessing already initialized fields, but in what form remains to be seen. --- Cheers, Benno > [1]: https://lore.kernel.org/rust-for-linux/aLshd0_C-1rh3FAg@tardis-2.local/ > > Thanks! > > Regards, > Boqun > >> I'm open to take v2 or v1, whatever you guys prefer. >> >> --- >> Cheers, >> Benno
On 9/10/25 4:59 PM, Benno Lossin wrote: > On Wed Sep 10, 2025 at 4:02 PM CEST, Boqun Feng wrote: >> On Wed, Sep 10, 2025 at 02:19:00PM +0200, Benno Lossin wrote: >>> On Wed Sep 10, 2025 at 12:40 PM CEST, Alice Ryhl wrote: >>>> On Wed, Sep 10, 2025 at 12:36 PM Benno Lossin <lossin@kernel.org> wrote: >>>>> >>>>> On Wed Sep 10, 2025 at 12:17 PM CEST, Alice Ryhl wrote: >>>>>> On Wed, Sep 10, 2025 at 12:07:53PM +0200, Benno Lossin wrote: >>>>>>> Assigning a field a value in an initializer macro can be marked with the >>>>>>> `#[bind]` attribute. Doing so creates a `let` binding with the same >>>>>>> name. This `let` binding has the type `Pin<&mut T>` if the field is >>>>>>> structurally pinned or `&mut T` otherwise (where `T` is the type of the >>>>>>> field). >>>>>>> >>>>>>> Signed-off-by: Benno Lossin <lossin@kernel.org> >>>>>> >>>>>> Is there a reason we can't apply this to all fields and avoid the >>>>>> attribute? >>>>> >>>>> Adding the attribute was due to Boqun's concern on v1 [1]. I think it >>>>> might be surprising too, but I'm also happy with no attribute. >>>>> >>>>> [1]: https://lore.kernel.org/all/aLshd0_C-1rh3FAg@tardis-2.local >>>> >>>> IMO the ideal is if it works without an attribute. Perhaps trying that >>>> in the kernel is a reasonable experiment to find out whether that's >>>> reasonable to do for the general language feature? >>> >>> @Boqun what is your opinion on this? >>> >> >> If we plan to make the in-place initializer language feature behave >> similar, as I asked here [1], then dropping `#[bind]` seems good to me. > > I don't think we can promise how that language feature is going to look > like. It will definitely support accessing already initialized fields, > but in what form remains to be seen. And as Alice said, getting some experience from real applications is not the worst thing. We can always change it if we see it causing too much pain.
On Wed, Sep 10, 2025 at 04:59:11PM +0200, Benno Lossin wrote: > On Wed Sep 10, 2025 at 4:02 PM CEST, Boqun Feng wrote: > > On Wed, Sep 10, 2025 at 02:19:00PM +0200, Benno Lossin wrote: > >> On Wed Sep 10, 2025 at 12:40 PM CEST, Alice Ryhl wrote: > >> > On Wed, Sep 10, 2025 at 12:36 PM Benno Lossin <lossin@kernel.org> wrote: > >> >> > >> >> On Wed Sep 10, 2025 at 12:17 PM CEST, Alice Ryhl wrote: > >> >> > On Wed, Sep 10, 2025 at 12:07:53PM +0200, Benno Lossin wrote: > >> >> >> Assigning a field a value in an initializer macro can be marked with the > >> >> >> `#[bind]` attribute. Doing so creates a `let` binding with the same > >> >> >> name. This `let` binding has the type `Pin<&mut T>` if the field is > >> >> >> structurally pinned or `&mut T` otherwise (where `T` is the type of the > >> >> >> field). > >> >> >> > >> >> >> Signed-off-by: Benno Lossin <lossin@kernel.org> > >> >> > > >> >> > Is there a reason we can't apply this to all fields and avoid the > >> >> > attribute? > >> >> > >> >> Adding the attribute was due to Boqun's concern on v1 [1]. I think it > >> >> might be surprising too, but I'm also happy with no attribute. > >> >> > >> >> [1]: https://lore.kernel.org/all/aLshd0_C-1rh3FAg@tardis-2.local > >> > > >> > IMO the ideal is if it works without an attribute. Perhaps trying that > >> > in the kernel is a reasonable experiment to find out whether that's > >> > reasonable to do for the general language feature? > >> > >> @Boqun what is your opinion on this? > >> > > > > If we plan to make the in-place initializer language feature behave > > similar, as I asked here [1], then dropping `#[bind]` seems good to me. > > I don't think we can promise how that language feature is going to look > like. It will definitely support accessing already initialized fields, > but in what form remains to be seen. > Sure, but in kernel we are able to stay the same as whatever the language feature will be like, right? In other words, as long as we propose the same thing to the language feature and keep kernel code and language feature synced (presumbly there could be some more discussion on the syntax when presented to Rust commmunity), then I'm think it's fine. Thanks! Regards, Boqun > --- > Cheers, > Benno > > > [1]: https://lore.kernel.org/rust-for-linux/aLshd0_C-1rh3FAg@tardis-2.local/ > > > > Thanks! > > > > Regards, > > Boqun > > > >> I'm open to take v2 or v1, whatever you guys prefer. > >> > >> --- > >> Cheers, > >> Benno >
On Wed Sep 10, 2025 at 5:06 PM CEST, Boqun Feng wrote: > On Wed, Sep 10, 2025 at 04:59:11PM +0200, Benno Lossin wrote: >> On Wed Sep 10, 2025 at 4:02 PM CEST, Boqun Feng wrote: >> > On Wed, Sep 10, 2025 at 02:19:00PM +0200, Benno Lossin wrote: >> >> On Wed Sep 10, 2025 at 12:40 PM CEST, Alice Ryhl wrote: >> >> > On Wed, Sep 10, 2025 at 12:36 PM Benno Lossin <lossin@kernel.org> wrote: >> >> >> >> >> >> On Wed Sep 10, 2025 at 12:17 PM CEST, Alice Ryhl wrote: >> >> >> > On Wed, Sep 10, 2025 at 12:07:53PM +0200, Benno Lossin wrote: >> >> >> >> Assigning a field a value in an initializer macro can be marked with the >> >> >> >> `#[bind]` attribute. Doing so creates a `let` binding with the same >> >> >> >> name. This `let` binding has the type `Pin<&mut T>` if the field is >> >> >> >> structurally pinned or `&mut T` otherwise (where `T` is the type of the >> >> >> >> field). >> >> >> >> >> >> >> >> Signed-off-by: Benno Lossin <lossin@kernel.org> >> >> >> > >> >> >> > Is there a reason we can't apply this to all fields and avoid the >> >> >> > attribute? >> >> >> >> >> >> Adding the attribute was due to Boqun's concern on v1 [1]. I think it >> >> >> might be surprising too, but I'm also happy with no attribute. >> >> >> >> >> >> [1]: https://lore.kernel.org/all/aLshd0_C-1rh3FAg@tardis-2.local >> >> > >> >> > IMO the ideal is if it works without an attribute. Perhaps trying that >> >> > in the kernel is a reasonable experiment to find out whether that's >> >> > reasonable to do for the general language feature? >> >> >> >> @Boqun what is your opinion on this? >> >> >> > >> > If we plan to make the in-place initializer language feature behave >> > similar, as I asked here [1], then dropping `#[bind]` seems good to me. >> >> I don't think we can promise how that language feature is going to look >> like. It will definitely support accessing already initialized fields, >> but in what form remains to be seen. >> > > Sure, but in kernel we are able to stay the same as whatever the > language feature will be like, right? Yes :) > In other words, as long as we propose the same thing to the language > feature and keep kernel code and language feature synced (presumbly > there could be some more discussion on the syntax when presented to Rust > commmunity), then I'm think it's fine. I'll take v1 (with the in-tree fixes) then. Do you mind giving your RB for that one? Thanks! --- Cheers, Benno
On 9/10/25 2:19 PM, Benno Lossin wrote: > On Wed Sep 10, 2025 at 12:40 PM CEST, Alice Ryhl wrote: >> On Wed, Sep 10, 2025 at 12:36 PM Benno Lossin <lossin@kernel.org> wrote: >>> >>> On Wed Sep 10, 2025 at 12:17 PM CEST, Alice Ryhl wrote: >>>> On Wed, Sep 10, 2025 at 12:07:53PM +0200, Benno Lossin wrote: >>>>> Assigning a field a value in an initializer macro can be marked with the >>>>> `#[bind]` attribute. Doing so creates a `let` binding with the same >>>>> name. This `let` binding has the type `Pin<&mut T>` if the field is >>>>> structurally pinned or `&mut T` otherwise (where `T` is the type of the >>>>> field). >>>>> >>>>> Signed-off-by: Benno Lossin <lossin@kernel.org> >>>> >>>> Is there a reason we can't apply this to all fields and avoid the >>>> attribute? >>> >>> Adding the attribute was due to Boqun's concern on v1 [1]. I think it >>> might be surprising too, but I'm also happy with no attribute. >>> >>> [1]: https://lore.kernel.org/all/aLshd0_C-1rh3FAg@tardis-2.local >> >> IMO the ideal is if it works without an attribute. Perhaps trying that >> in the kernel is a reasonable experiment to find out whether that's >> reasonable to do for the general language feature? > > @Boqun what is your opinion on this? > > I'm open to take v2 or v1, whatever you guys prefer. Same for me, feel free to add Reviewed-by: Danilo Krummrich <dakr@kernel.org> to either of them. This version is also Tested-by: Danilo Krummrich <dakr@kernel.org>
On Wed Sep 10, 2025 at 12:40 PM CEST, Alice Ryhl wrote:
> On Wed, Sep 10, 2025 at 12:36 PM Benno Lossin <lossin@kernel.org> wrote:
>>
>> On Wed Sep 10, 2025 at 12:17 PM CEST, Alice Ryhl wrote:
>> > On Wed, Sep 10, 2025 at 12:07:53PM +0200, Benno Lossin wrote:
>> >> Assigning a field a value in an initializer macro can be marked with the
>> >> `#[bind]` attribute. Doing so creates a `let` binding with the same
>> >> name. This `let` binding has the type `Pin<&mut T>` if the field is
>> >> structurally pinned or `&mut T` otherwise (where `T` is the type of the
>> >> field).
>> >>
>> >> Signed-off-by: Benno Lossin <lossin@kernel.org>
>> >
>> > Is there a reason we can't apply this to all fields and avoid the
>> > attribute?
>>
>> Adding the attribute was due to Boqun's concern on v1 [1]. I think it
>> might be surprising too, but I'm also happy with no attribute.
>>
>> [1]: https://lore.kernel.org/all/aLshd0_C-1rh3FAg@tardis-2.local
>
> IMO the ideal is if it works without an attribute. Perhaps trying that
> in the kernel is a reasonable experiment to find out whether that's
> reasonable to do for the general language feature?
>
>> > Do we have a place that might be able to use this?
>>
>> I didn't find one, but Danilo plans to base some changes on top this
>> cycle that need this.
We can use it in devres right away:
diff --git a/rust/kernel/devres.rs b/rust/kernel/devres.rs
index d04e3fcebafb..97c616a1733d 100644
--- a/rust/kernel/devres.rs
+++ b/rust/kernel/devres.rs
@@ -137,10 +137,11 @@ pub fn new<'a, E>(
{
let callback = Self::devres_callback;
- try_pin_init!(&this in Self {
+ try_pin_init!(Self {
dev: dev.into(),
callback,
// INVARIANT: `inner` is properly initialized.
+ #[bind]
inner <- Opaque::pin_init(try_pin_init!(Inner {
devm <- Completion::new(),
revoke <- Completion::new(),
@@ -150,8 +151,7 @@ pub fn new<'a, E>(
//
// [1] https://github.com/Rust-for-Linux/pin-init/pull/69
_add_action: {
- // SAFETY: `this` is a valid pointer to uninitialized memory.
- let inner = unsafe { &raw mut (*this.as_ptr()).inner };
+ let inner = core::ptr::from_ref(inner.into_ref().get_ref());
// SAFETY:
// - `dev.as_raw()` is a pointer to a valid bound device.
@@ -160,7 +160,7 @@ pub fn new<'a, E>(
// properly initialized, because we require `dev` (i.e. the *bound* device) to
// live at least as long as the returned `impl PinInit<Self, Error>`.
to_result(unsafe {
- bindings::devm_add_action(dev.as_raw(), Some(callback), inner.cast())
+ bindings::devm_add_action(dev.as_raw(), Some(callback), inner.cast_mut().cast())
}).inspect_err(|_| {
let inner = Opaque::cast_into(inner);
Together with the initializer code blocks this becomes quite nice. :)
> Danilo, what plans do you have?
Besides that, the plan is [1].
[1] https://lore.kernel.org/all/DCL32RUQ6Z56.1ERY7JBK6O1J6@kernel.org/
On Wed, Sep 10, 2025 at 1:15 PM Danilo Krummrich <dakr@kernel.org> wrote:
>
> On Wed Sep 10, 2025 at 12:40 PM CEST, Alice Ryhl wrote:
> > On Wed, Sep 10, 2025 at 12:36 PM Benno Lossin <lossin@kernel.org> wrote:
> >>
> >> On Wed Sep 10, 2025 at 12:17 PM CEST, Alice Ryhl wrote:
> >> > On Wed, Sep 10, 2025 at 12:07:53PM +0200, Benno Lossin wrote:
> >> >> Assigning a field a value in an initializer macro can be marked with the
> >> >> `#[bind]` attribute. Doing so creates a `let` binding with the same
> >> >> name. This `let` binding has the type `Pin<&mut T>` if the field is
> >> >> structurally pinned or `&mut T` otherwise (where `T` is the type of the
> >> >> field).
> >> >>
> >> >> Signed-off-by: Benno Lossin <lossin@kernel.org>
> >> >
> >> > Is there a reason we can't apply this to all fields and avoid the
> >> > attribute?
> >>
> >> Adding the attribute was due to Boqun's concern on v1 [1]. I think it
> >> might be surprising too, but I'm also happy with no attribute.
> >>
> >> [1]: https://lore.kernel.org/all/aLshd0_C-1rh3FAg@tardis-2.local
> >
> > IMO the ideal is if it works without an attribute. Perhaps trying that
> > in the kernel is a reasonable experiment to find out whether that's
> > reasonable to do for the general language feature?
> >
> >> > Do we have a place that might be able to use this?
> >>
> >> I didn't find one, but Danilo plans to base some changes on top this
> >> cycle that need this.
>
> We can use it in devres right away:
>
> diff --git a/rust/kernel/devres.rs b/rust/kernel/devres.rs
> index d04e3fcebafb..97c616a1733d 100644
> --- a/rust/kernel/devres.rs
> +++ b/rust/kernel/devres.rs
> @@ -137,10 +137,11 @@ pub fn new<'a, E>(
> {
> let callback = Self::devres_callback;
>
> - try_pin_init!(&this in Self {
> + try_pin_init!(Self {
> dev: dev.into(),
> callback,
> // INVARIANT: `inner` is properly initialized.
> + #[bind]
> inner <- Opaque::pin_init(try_pin_init!(Inner {
> devm <- Completion::new(),
> revoke <- Completion::new(),
> @@ -150,8 +151,7 @@ pub fn new<'a, E>(
> //
> // [1] https://github.com/Rust-for-Linux/pin-init/pull/69
> _add_action: {
> - // SAFETY: `this` is a valid pointer to uninitialized memory.
> - let inner = unsafe { &raw mut (*this.as_ptr()).inner };
> + let inner = core::ptr::from_ref(inner.into_ref().get_ref());
Overall looks good. Looks like you want Opaque::get here rather than
the cast cast cast operation.
> // SAFETY:
> // - `dev.as_raw()` is a pointer to a valid bound device.
> @@ -160,7 +160,7 @@ pub fn new<'a, E>(
> // properly initialized, because we require `dev` (i.e. the *bound* device) to
> // live at least as long as the returned `impl PinInit<Self, Error>`.
> to_result(unsafe {
> - bindings::devm_add_action(dev.as_raw(), Some(callback), inner.cast())
> + bindings::devm_add_action(dev.as_raw(), Some(callback), inner.cast_mut().cast())
> }).inspect_err(|_| {
> let inner = Opaque::cast_into(inner);
>
>
> Together with the initializer code blocks this becomes quite nice. :)
>
> > Danilo, what plans do you have?
>
> Besides that, the plan is [1].
>
> [1] https://lore.kernel.org/all/DCL32RUQ6Z56.1ERY7JBK6O1J6@kernel.org/
Looks nice :)
Alice
On 9/10/25 1:52 PM, Alice Ryhl wrote:
> On Wed, Sep 10, 2025 at 1:15 PM Danilo Krummrich <dakr@kernel.org> wrote:
>>
>> On Wed Sep 10, 2025 at 12:40 PM CEST, Alice Ryhl wrote:
>>> On Wed, Sep 10, 2025 at 12:36 PM Benno Lossin <lossin@kernel.org> wrote:
>>>>
>>>> On Wed Sep 10, 2025 at 12:17 PM CEST, Alice Ryhl wrote:
>>>>> On Wed, Sep 10, 2025 at 12:07:53PM +0200, Benno Lossin wrote:
>>>>>> Assigning a field a value in an initializer macro can be marked with the
>>>>>> `#[bind]` attribute. Doing so creates a `let` binding with the same
>>>>>> name. This `let` binding has the type `Pin<&mut T>` if the field is
>>>>>> structurally pinned or `&mut T` otherwise (where `T` is the type of the
>>>>>> field).
>>>>>>
>>>>>> Signed-off-by: Benno Lossin <lossin@kernel.org>
>>>>>
>>>>> Is there a reason we can't apply this to all fields and avoid the
>>>>> attribute?
>>>>
>>>> Adding the attribute was due to Boqun's concern on v1 [1]. I think it
>>>> might be surprising too, but I'm also happy with no attribute.
>>>>
>>>> [1]: https://lore.kernel.org/all/aLshd0_C-1rh3FAg@tardis-2.local
>>>
>>> IMO the ideal is if it works without an attribute. Perhaps trying that
>>> in the kernel is a reasonable experiment to find out whether that's
>>> reasonable to do for the general language feature?
>>>
>>>>> Do we have a place that might be able to use this?
>>>>
>>>> I didn't find one, but Danilo plans to base some changes on top this
>>>> cycle that need this.
>>
>> We can use it in devres right away:
>>
>> diff --git a/rust/kernel/devres.rs b/rust/kernel/devres.rs
>> index d04e3fcebafb..97c616a1733d 100644
>> --- a/rust/kernel/devres.rs
>> +++ b/rust/kernel/devres.rs
>> @@ -137,10 +137,11 @@ pub fn new<'a, E>(
>> {
>> let callback = Self::devres_callback;
>>
>> - try_pin_init!(&this in Self {
>> + try_pin_init!(Self {
>> dev: dev.into(),
>> callback,
>> // INVARIANT: `inner` is properly initialized.
>> + #[bind]
>> inner <- Opaque::pin_init(try_pin_init!(Inner {
>> devm <- Completion::new(),
>> revoke <- Completion::new(),
>> @@ -150,8 +151,7 @@ pub fn new<'a, E>(
>> //
>> // [1] https://github.com/Rust-for-Linux/pin-init/pull/69
>> _add_action: {
>> - // SAFETY: `this` is a valid pointer to uninitialized memory.
>> - let inner = unsafe { &raw mut (*this.as_ptr()).inner };
>> + let inner = core::ptr::from_ref(inner.into_ref().get_ref());
>
> Overall looks good. Looks like you want Opaque::get here rather than
> the cast cast cast operation.
Oh, indeed. I overlooked that. :)
© 2016 - 2025 Red Hat, Inc.