* [PATCH] rust: pin-init: add references to previously initialized fields
@ 2025-09-05 14:00 Benno Lossin
2025-09-05 17:18 ` Benno Lossin
` (3 more replies)
0 siblings, 4 replies; 22+ messages in thread
From: Benno Lossin @ 2025-09-05 14:00 UTC (permalink / raw)
To: Benno Lossin, Miguel Ojeda, Alex Gaynor, Boqun Feng, Gary Guo,
Björn Roy Baron, Andreas Hindborg, Alice Ryhl, Trevor Gross,
Danilo Krummrich, Fiona Behrens, Alban Kurti
Cc: rust-for-linux, linux-kernel
After initializing a field in an initializer macro, create a variable
holding a reference that points at that field. The type is either
`Pin<&mut T>` or `&mut T` depending on the field's structural pinning
kind.
Link: https://github.com/Rust-for-Linux/pin-init/pull/83/commits/0f658594c39398f58cd5cb99a8141e370e225e74
Signed-off-by: Benno Lossin <lossin@kernel.org>
---
rust/pin-init/src/macros.rs | 149 ++++++++++++++++++++++++++++--------
1 file changed, 115 insertions(+), 34 deletions(-)
diff --git a/rust/pin-init/src/macros.rs b/rust/pin-init/src/macros.rs
index 9ced630737b8..1100c5a0b3de 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
+ }
+ )*
+ }
}
};
}
@@ -1216,6 +1234,13 @@ 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)? };
+ // 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.
+ #[allow(unused_variables)]
+ let $field = $crate::macros::paste!(unsafe { $data.[< __project_ $field >](&mut (*$slot).$field) });
+
// Create the drop guard:
//
// We rely on macro hygiene to make it impossible for users to access this local variable.
@@ -1247,6 +1272,14 @@ 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))? };
+
+ // SAFETY:
+ // - the field is not structurally pinned, since the line above must compile,
+ // - the field has been initialized,
+ // - the reference is only valid until the end of the initializer.
+ #[allow(unused_variables)]
+ let $field = unsafe { &mut (*$slot).$field };
+
// Create the drop guard:
//
// We rely on macro hygiene to make it impossible for users to access this local variable.
@@ -1265,7 +1298,48 @@ 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($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) };
+ }
+
+ #[allow(unused_variables)]
+ // 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 };
+
+ // 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,)*),
@@ -1279,6 +1353,13 @@ 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) };
}
+ // 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.
+ #[allow(unused_variables)]
+ let $field = $crate::macros::paste!(unsafe { $data.[< __project_ $field >](&mut (*$slot).$field) });
+
// Create the drop guard:
//
// We rely on macro hygiene to make it impossible for users to access this local variable.
@@ -1289,7 +1370,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,)*),
base-commit: 8f5ae30d69d7543eee0d70083daf4de8fe15d585
--
2.50.1
^ permalink raw reply related [flat|nested] 22+ messages in thread
* Re: [PATCH] rust: pin-init: add references to previously initialized fields
2025-09-05 14:00 [PATCH] rust: pin-init: add references to previously initialized fields Benno Lossin
@ 2025-09-05 17:18 ` Benno Lossin
2025-09-05 17:44 ` Boqun Feng
2025-09-05 17:21 ` Boqun Feng
` (2 subsequent siblings)
3 siblings, 1 reply; 22+ messages in thread
From: Benno Lossin @ 2025-09-05 17:18 UTC (permalink / raw)
To: Benno Lossin, Miguel Ojeda, Alex Gaynor, Boqun Feng, Gary Guo,
Björn Roy Baron, Andreas Hindborg, Alice Ryhl, Trevor Gross,
Danilo Krummrich, Fiona Behrens, Alban Kurti, Greg Kroah-Hartman,
Rafael J. Wysocki, Bjorn Helgaas, Krzysztof Wilczyński
Cc: rust-for-linux, linux-kernel
On Fri Sep 5, 2025 at 4:00 PM CEST, Benno Lossin wrote:
> After initializing a field in an initializer macro, create a variable
> holding a reference that points at that field. The type is either
> `Pin<&mut T>` or `&mut T` depending on the field's structural pinning
> kind.
>
> Link: https://github.com/Rust-for-Linux/pin-init/pull/83/commits/0f658594c39398f58cd5cb99a8141e370e225e74
> Signed-off-by: Benno Lossin <lossin@kernel.org>
I forgot to test with the right configuration and found some errors with
existing code. Here are their fixes. If I don't need to re-send, I will
add them on apply (if you want a v2, let me know).
---
Cheers,
Benno
diff --git a/rust/kernel/devres.rs b/rust/kernel/devres.rs
index da18091143a6..91dbf3f4b166 100644
--- a/rust/kernel/devres.rs
+++ b/rust/kernel/devres.rs
@@ -134,11 +134,9 @@ pub fn new<'a, E>(
T: 'a,
Error: From<E>,
{
- let callback = Self::devres_callback;
-
try_pin_init!(&this in Self {
dev: dev.into(),
- callback,
+ callback: Self::devres_callback,
// INVARIANT: `inner` is properly initialized.
inner <- {
// SAFETY: `this` is a valid pointer to uninitialized memory.
@@ -151,7 +149,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())
})?;
Opaque::pin_init(try_pin_init!(Inner {
diff --git a/samples/rust/rust_driver_pci.rs b/samples/rust/rust_driver_pci.rs
index 606946ff4d7f..1ac0b06fa3b3 100644
--- a/samples/rust/rust_driver_pci.rs
+++ b/samples/rust/rust_driver_pci.rs
@@ -78,8 +78,8 @@ fn probe(pdev: &pci::Device<Core>, info: &Self::IdInfo) -> Result<Pin<KBox<Self>
let drvdata = KBox::pin_init(
try_pin_init!(Self {
- pdev: pdev.into(),
bar <- pdev.iomap_region_sized::<{ Regs::END }>(0, c_str!("rust_driver_pci")),
+ pdev: pdev.into(),
index: *info,
}),
GFP_KERNEL,
^ permalink raw reply related [flat|nested] 22+ messages in thread
* Re: [PATCH] rust: pin-init: add references to previously initialized fields
2025-09-05 17:18 ` Benno Lossin
@ 2025-09-05 17:44 ` Boqun Feng
2025-09-06 10:52 ` Danilo Krummrich
0 siblings, 1 reply; 22+ messages in thread
From: Boqun Feng @ 2025-09-05 17:44 UTC (permalink / raw)
To: Benno Lossin
Cc: Miguel Ojeda, Alex Gaynor, Gary Guo, Björn Roy Baron,
Andreas Hindborg, Alice Ryhl, Trevor Gross, Danilo Krummrich,
Fiona Behrens, Alban Kurti, Greg Kroah-Hartman, Rafael J. Wysocki,
Bjorn Helgaas, Krzysztof Wilczy´nski, rust-for-linux,
linux-kernel
On Fri, Sep 05, 2025 at 07:18:25PM +0200, Benno Lossin wrote:
[...]
> index 606946ff4d7f..1ac0b06fa3b3 100644
> --- a/samples/rust/rust_driver_pci.rs
> +++ b/samples/rust/rust_driver_pci.rs
> @@ -78,8 +78,8 @@ fn probe(pdev: &pci::Device<Core>, info: &Self::IdInfo) -> Result<Pin<KBox<Self>
>
> let drvdata = KBox::pin_init(
> try_pin_init!(Self {
> - pdev: pdev.into(),
> bar <- pdev.iomap_region_sized::<{ Regs::END }>(0, c_str!("rust_driver_pci")),
> + pdev: pdev.into(),
Ok, this example is good enough for me to express the concern here: the
variable shadowing behavior seems not straightforward (maybe because in
normal Rust initalization expression, no binding is created for
previous variables, neither do we have a `let` here).
Would the future inplace initialization have the similar behavior? I
asked because a natural resolution is adding a special syntax like:
let a = ..;
try_pin_init!(Self {
b: a,
let a = a.into(); // create the new binding here.
c: a, // <- use the previous initalized `a`.
}
(Since I lost tracking of the discussion a bit, maybe there is a
previous discussion I've missed here?)
Regards,
Boqun
> index: *info,
> }),
> GFP_KERNEL,
^ permalink raw reply [flat|nested] 22+ messages in thread
* Re: [PATCH] rust: pin-init: add references to previously initialized fields
2025-09-05 17:44 ` Boqun Feng
@ 2025-09-06 10:52 ` Danilo Krummrich
2025-09-07 1:57 ` Boqun Feng
0 siblings, 1 reply; 22+ messages in thread
From: Danilo Krummrich @ 2025-09-06 10:52 UTC (permalink / raw)
To: Boqun Feng
Cc: Benno Lossin, Miguel Ojeda, Alex Gaynor, Gary Guo,
Björn Roy Baron, Andreas Hindborg, Alice Ryhl, Trevor Gross,
Fiona Behrens, Alban Kurti, Greg Kroah-Hartman, Rafael J. Wysocki,
Bjorn Helgaas, Krzysztof Wilczy´nski, rust-for-linux,
linux-kernel
On Fri Sep 5, 2025 at 7:44 PM CEST, Boqun Feng wrote:
> On Fri, Sep 05, 2025 at 07:18:25PM +0200, Benno Lossin wrote:
> [...]
>> index 606946ff4d7f..1ac0b06fa3b3 100644
>> --- a/samples/rust/rust_driver_pci.rs
>> +++ b/samples/rust/rust_driver_pci.rs
>> @@ -78,8 +78,8 @@ fn probe(pdev: &pci::Device<Core>, info: &Self::IdInfo) -> Result<Pin<KBox<Self>
>>
>> let drvdata = KBox::pin_init(
>> try_pin_init!(Self {
>> - pdev: pdev.into(),
>> bar <- pdev.iomap_region_sized::<{ Regs::END }>(0, c_str!("rust_driver_pci")),
>> + pdev: pdev.into(),
>
> Ok, this example is good enough for me to express the concern here: the
> variable shadowing behavior seems not straightforward (maybe because in
> normal Rust initalization expression, no binding is created for
> previous variables, neither do we have a `let` here).
>
> Would the future inplace initialization have the similar behavior? I
> asked because a natural resolution is adding a special syntax like:
>
> let a = ..;
>
> try_pin_init!(Self {
> b: a,
> let a = a.into(); // create the new binding here.
> c: a, // <- use the previous initalized `a`.
> }
Can you please clarify the example? I'm a bit confused that this is not a field
of Self, so currently this can just be written as:
try_pin_init!(Self {
b: a,
c: a.into,
})
Of course assuming that a is Clone, as the code above does as well.
So, if we are concerned by the variable shadowing, which I'm less concerned
about, maybe we can do this:
// The "original" `a` and `b`.
let a: A = ...;
let b: B = ...;
try_pin_init!(Self {
a, // Initialize the field only.
let b <- b, // Initialize the field and create a `&B` named `b`.
c: a.into(), // That's the "original" `a`.
d <- D::new(b), // Not the original `b`, but the pin-init one.
})
^ permalink raw reply [flat|nested] 22+ messages in thread
* Re: [PATCH] rust: pin-init: add references to previously initialized fields
2025-09-06 10:52 ` Danilo Krummrich
@ 2025-09-07 1:57 ` Boqun Feng
2025-09-07 2:07 ` Boqun Feng
0 siblings, 1 reply; 22+ messages in thread
From: Boqun Feng @ 2025-09-07 1:57 UTC (permalink / raw)
To: Danilo Krummrich
Cc: Benno Lossin, Miguel Ojeda, Alex Gaynor, Gary Guo,
Björn Roy Baron, Andreas Hindborg, Alice Ryhl, Trevor Gross,
Fiona Behrens, Alban Kurti, Greg Kroah-Hartman, Rafael J. Wysocki,
Bjorn Helgaas, Krzysztof Wilczy´nski, rust-for-linux,
linux-kernel
On Sat, Sep 06, 2025 at 12:52:22PM +0200, Danilo Krummrich wrote:
> On Fri Sep 5, 2025 at 7:44 PM CEST, Boqun Feng wrote:
> > On Fri, Sep 05, 2025 at 07:18:25PM +0200, Benno Lossin wrote:
> > [...]
> >> index 606946ff4d7f..1ac0b06fa3b3 100644
> >> --- a/samples/rust/rust_driver_pci.rs
> >> +++ b/samples/rust/rust_driver_pci.rs
> >> @@ -78,8 +78,8 @@ fn probe(pdev: &pci::Device<Core>, info: &Self::IdInfo) -> Result<Pin<KBox<Self>
> >>
> >> let drvdata = KBox::pin_init(
> >> try_pin_init!(Self {
> >> - pdev: pdev.into(),
> >> bar <- pdev.iomap_region_sized::<{ Regs::END }>(0, c_str!("rust_driver_pci")),
> >> + pdev: pdev.into(),
> >
> > Ok, this example is good enough for me to express the concern here: the
> > variable shadowing behavior seems not straightforward (maybe because in
> > normal Rust initalization expression, no binding is created for
> > previous variables, neither do we have a `let` here).
> >
> > Would the future inplace initialization have the similar behavior? I
> > asked because a natural resolution is adding a special syntax like:
> >
> > let a = ..;
> >
> > try_pin_init!(Self {
> > b: a,
> > let a = a.into(); // create the new binding here.
> > c: a, // <- use the previous initalized `a`.
> > }
>
> Can you please clarify the example? I'm a bit confused that this is not a field
> of Self, so currently this can just be written as:
>
Oh, I could have been more clear: `a` is a field of `Self`, and the
`let` part initalizes it.
> try_pin_init!(Self {
> b: a,
> c: a.into,
> })
>
> Of course assuming that a is Clone, as the code above does as well.
>
> So, if we are concerned by the variable shadowing, which I'm less concerned
> about, maybe we can do this:
I'm not that concerned to block this, but it does look to me like we are
inventing a new way (and even a different syntax because normal Rust
initialization doesn't create new bindings) to create binding, so I
think I should bring it up.
>
> // The "original" `a` and `b`.
> let a: A = ...;
> let b: B = ...;
>
> try_pin_init!(Self {
> a, // Initialize the field only.
> let b <- b, // Initialize the field and create a `&B` named `b`.
> c: a.into(), // That's the "original" `a`.
> d <- D::new(b), // Not the original `b`, but the pin-init one.
> })
This looks good to me as well. But I'm curious what the syntax would be
like in the in-place placement language feature in the future.
Regards,
Boqun
^ permalink raw reply [flat|nested] 22+ messages in thread
* Re: [PATCH] rust: pin-init: add references to previously initialized fields
2025-09-07 1:57 ` Boqun Feng
@ 2025-09-07 2:07 ` Boqun Feng
2025-09-07 8:41 ` Benno Lossin
0 siblings, 1 reply; 22+ messages in thread
From: Boqun Feng @ 2025-09-07 2:07 UTC (permalink / raw)
To: Danilo Krummrich
Cc: Benno Lossin, Miguel Ojeda, Alex Gaynor, Gary Guo,
Björn Roy Baron, Andreas Hindborg, Alice Ryhl, Trevor Gross,
Fiona Behrens, Alban Kurti, Greg Kroah-Hartman, Rafael J. Wysocki,
Bjorn Helgaas, Krzysztof Wilczy´nski, rust-for-linux,
linux-kernel
On Sat, Sep 06, 2025 at 06:57:04PM -0700, Boqun Feng wrote:
> On Sat, Sep 06, 2025 at 12:52:22PM +0200, Danilo Krummrich wrote:
> > On Fri Sep 5, 2025 at 7:44 PM CEST, Boqun Feng wrote:
> > > On Fri, Sep 05, 2025 at 07:18:25PM +0200, Benno Lossin wrote:
> > > [...]
> > >> index 606946ff4d7f..1ac0b06fa3b3 100644
> > >> --- a/samples/rust/rust_driver_pci.rs
> > >> +++ b/samples/rust/rust_driver_pci.rs
> > >> @@ -78,8 +78,8 @@ fn probe(pdev: &pci::Device<Core>, info: &Self::IdInfo) -> Result<Pin<KBox<Self>
> > >>
> > >> let drvdata = KBox::pin_init(
> > >> try_pin_init!(Self {
> > >> - pdev: pdev.into(),
> > >> bar <- pdev.iomap_region_sized::<{ Regs::END }>(0, c_str!("rust_driver_pci")),
> > >> + pdev: pdev.into(),
> > >
> > > Ok, this example is good enough for me to express the concern here: the
> > > variable shadowing behavior seems not straightforward (maybe because in
> > > normal Rust initalization expression, no binding is created for
> > > previous variables, neither do we have a `let` here).
> > >
> > > Would the future inplace initialization have the similar behavior? I
> > > asked because a natural resolution is adding a special syntax like:
> > >
> > > let a = ..;
> > >
> > > try_pin_init!(Self {
> > > b: a,
> > > let a = a.into(); // create the new binding here.
> > > c: a, // <- use the previous initalized `a`.
> > > }
> >
> > Can you please clarify the example? I'm a bit confused that this is not a field
> > of Self, so currently this can just be written as:
> >
>
> Oh, I could have been more clear: `a` is a field of `Self`, and the
> `let` part initalizes it.
>
> > try_pin_init!(Self {
> > b: a,
> > c: a.into,
> > })
> >
> > Of course assuming that a is Clone, as the code above does as well.
> >
> > So, if we are concerned by the variable shadowing, which I'm less concerned
> > about, maybe we can do this:
>
> I'm not that concerned to block this, but it does look to me like we are
> inventing a new way (and even a different syntax because normal Rust
> initialization doesn't create new bindings) to create binding, so I
> think I should bring it up.
>
> >
> > // The "original" `a` and `b`.
> > let a: A = ...;
> > let b: B = ...;
> >
> > try_pin_init!(Self {
> > a, // Initialize the field only.
> > let b <- b, // Initialize the field and create a `&B` named `b`.
> > c: a.into(), // That's the "original" `a`.
> > d <- D::new(b), // Not the original `b`, but the pin-init one.
> > })
Another idea is using `&this`:
try_pin_init!(&this in Self {
a, // Initialize the field only.
b <- b, // Initialize the field only.
c: a.into(), // That's the "original" `a`.
d <- D::new(this->b), // Not the original `b`, but the pin-init one.
})
, like a special field projection during initialization.
Regards,
Boqun
>
> This looks good to me as well. But I'm curious what the syntax would be
> like in the in-place placement language feature in the future.
>
> Regards,
> Boqun
^ permalink raw reply [flat|nested] 22+ messages in thread
* Re: [PATCH] rust: pin-init: add references to previously initialized fields
2025-09-07 2:07 ` Boqun Feng
@ 2025-09-07 8:41 ` Benno Lossin
2025-09-07 17:29 ` Danilo Krummrich
0 siblings, 1 reply; 22+ messages in thread
From: Benno Lossin @ 2025-09-07 8:41 UTC (permalink / raw)
To: Boqun Feng, Danilo Krummrich
Cc: Miguel Ojeda, Alex Gaynor, Gary Guo, Björn Roy Baron,
Andreas Hindborg, Alice Ryhl, Trevor Gross, Fiona Behrens,
Alban Kurti, Greg Kroah-Hartman, Rafael J. Wysocki, Bjorn Helgaas,
Krzysztof Wilczy´nski, rust-for-linux, linux-kernel
On Sun Sep 7, 2025 at 4:07 AM CEST, Boqun Feng wrote:
> On Sat, Sep 06, 2025 at 06:57:04PM -0700, Boqun Feng wrote:
>> On Sat, Sep 06, 2025 at 12:52:22PM +0200, Danilo Krummrich wrote:
>> > On Fri Sep 5, 2025 at 7:44 PM CEST, Boqun Feng wrote:
>> > > On Fri, Sep 05, 2025 at 07:18:25PM +0200, Benno Lossin wrote:
>> > > [...]
>> > >> index 606946ff4d7f..1ac0b06fa3b3 100644
>> > >> --- a/samples/rust/rust_driver_pci.rs
>> > >> +++ b/samples/rust/rust_driver_pci.rs
>> > >> @@ -78,8 +78,8 @@ fn probe(pdev: &pci::Device<Core>, info: &Self::IdInfo) -> Result<Pin<KBox<Self>
>> > >>
>> > >> let drvdata = KBox::pin_init(
>> > >> try_pin_init!(Self {
>> > >> - pdev: pdev.into(),
>> > >> bar <- pdev.iomap_region_sized::<{ Regs::END }>(0, c_str!("rust_driver_pci")),
>> > >> + pdev: pdev.into(),
>> > >
>> > > Ok, this example is good enough for me to express the concern here: the
>> > > variable shadowing behavior seems not straightforward (maybe because in
>> > > normal Rust initalization expression, no binding is created for
>> > > previous variables, neither do we have a `let` here).
>> > >
>> > > Would the future inplace initialization have the similar behavior? I
>> > > asked because a natural resolution is adding a special syntax like:
>> > >
>> > > let a = ..;
>> > >
>> > > try_pin_init!(Self {
>> > > b: a,
>> > > let a = a.into(); // create the new binding here.
>> > > c: a, // <- use the previous initalized `a`.
>> > > }
>> >
>> > Can you please clarify the example? I'm a bit confused that this is not a field
>> > of Self, so currently this can just be written as:
>> >
>>
>> Oh, I could have been more clear: `a` is a field of `Self`, and the
>> `let` part initalizes it.
>>
>> > try_pin_init!(Self {
>> > b: a,
>> > c: a.into,
>> > })
>> >
>> > Of course assuming that a is Clone, as the code above does as well.
>> >
>> > So, if we are concerned by the variable shadowing, which I'm less concerned
>> > about, maybe we can do this:
>>
>> I'm not that concerned to block this, but it does look to me like we are
>> inventing a new way (and even a different syntax because normal Rust
>> initialization doesn't create new bindings) to create binding, so I
>> think I should bring it up.
>>
>> >
>> > // The "original" `a` and `b`.
>> > let a: A = ...;
>> > let b: B = ...;
>> >
>> > try_pin_init!(Self {
>> > a, // Initialize the field only.
>> > let b <- b, // Initialize the field and create a `&B` named `b`.
>> > c: a.into(), // That's the "original" `a`.
>> > d <- D::new(b), // Not the original `b`, but the pin-init one.
>> > })
>
> Another idea is using `&this`:
>
> try_pin_init!(&this in Self {
> a, // Initialize the field only.
> b <- b, // Initialize the field only.
> c: a.into(), // That's the "original" `a`.
> d <- D::new(this->b), // Not the original `b`, but the pin-init one.
> })
>
> , like a special field projection during initialization.
The main issue with new syntax is the difficulty of implementing it. The
let one is fine, but it's pretty jarring & doesn't get formatted by
rustfmt (which I want to eventually have). Using `this` does look better
IMO, but it's near impossible to implement using declarative macros
(even using `syn` it seems difficult to me). So either we find some way
to express it in existing rust syntax (maybe use an attribute?), or we
just keep it this way.
Maybe Gary has some ideas on how to implement it.
---
Cheers,
Benno
^ permalink raw reply [flat|nested] 22+ messages in thread
* Re: [PATCH] rust: pin-init: add references to previously initialized fields
2025-09-07 8:41 ` Benno Lossin
@ 2025-09-07 17:29 ` Danilo Krummrich
2025-09-07 21:06 ` Benno Lossin
0 siblings, 1 reply; 22+ messages in thread
From: Danilo Krummrich @ 2025-09-07 17:29 UTC (permalink / raw)
To: Benno Lossin
Cc: Boqun Feng, Miguel Ojeda, Alex Gaynor, Gary Guo,
Björn Roy Baron, Andreas Hindborg, Alice Ryhl, Trevor Gross,
Fiona Behrens, Alban Kurti, Greg Kroah-Hartman, Rafael J. Wysocki,
Bjorn Helgaas, Krzysztof Wilczy´nski, rust-for-linux,
linux-kernel
On Sun Sep 7, 2025 at 10:41 AM CEST, Benno Lossin wrote:
> On Sun Sep 7, 2025 at 4:07 AM CEST, Boqun Feng wrote:
>> On Sat, Sep 06, 2025 at 06:57:04PM -0700, Boqun Feng wrote:
>>> On Sat, Sep 06, 2025 at 12:52:22PM +0200, Danilo Krummrich wrote:
>>> > On Fri Sep 5, 2025 at 7:44 PM CEST, Boqun Feng wrote:
>>> > > On Fri, Sep 05, 2025 at 07:18:25PM +0200, Benno Lossin wrote:
>>> > > [...]
>>> > >> index 606946ff4d7f..1ac0b06fa3b3 100644
>>> > >> --- a/samples/rust/rust_driver_pci.rs
>>> > >> +++ b/samples/rust/rust_driver_pci.rs
>>> > >> @@ -78,8 +78,8 @@ fn probe(pdev: &pci::Device<Core>, info: &Self::IdInfo) -> Result<Pin<KBox<Self>
>>> > >>
>>> > >> let drvdata = KBox::pin_init(
>>> > >> try_pin_init!(Self {
>>> > >> - pdev: pdev.into(),
>>> > >> bar <- pdev.iomap_region_sized::<{ Regs::END }>(0, c_str!("rust_driver_pci")),
>>> > >> + pdev: pdev.into(),
>>> > >
>>> > > Ok, this example is good enough for me to express the concern here: the
>>> > > variable shadowing behavior seems not straightforward (maybe because in
>>> > > normal Rust initalization expression, no binding is created for
>>> > > previous variables, neither do we have a `let` here).
>>> > >
>>> > > Would the future inplace initialization have the similar behavior? I
>>> > > asked because a natural resolution is adding a special syntax like:
>>> > >
>>> > > let a = ..;
>>> > >
>>> > > try_pin_init!(Self {
>>> > > b: a,
>>> > > let a = a.into(); // create the new binding here.
>>> > > c: a, // <- use the previous initalized `a`.
>>> > > }
>>> >
>>> > Can you please clarify the example? I'm a bit confused that this is not a field
>>> > of Self, so currently this can just be written as:
>>> >
>>>
>>> Oh, I could have been more clear: `a` is a field of `Self`, and the
>>> `let` part initalizes it.
>>>
>>> > try_pin_init!(Self {
>>> > b: a,
>>> > c: a.into,
>>> > })
>>> >
>>> > Of course assuming that a is Clone, as the code above does as well.
>>> >
>>> > So, if we are concerned by the variable shadowing, which I'm less concerned
>>> > about, maybe we can do this:
>>>
>>> I'm not that concerned to block this, but it does look to me like we are
>>> inventing a new way (and even a different syntax because normal Rust
>>> initialization doesn't create new bindings) to create binding, so I
>>> think I should bring it up.
>>>
>>> >
>>> > // The "original" `a` and `b`.
>>> > let a: A = ...;
>>> > let b: B = ...;
>>> >
>>> > try_pin_init!(Self {
>>> > a, // Initialize the field only.
>>> > let b <- b, // Initialize the field and create a `&B` named `b`.
>>> > c: a.into(), // That's the "original" `a`.
>>> > d <- D::new(b), // Not the original `b`, but the pin-init one.
>>> > })
>>
>> Another idea is using `&this`:
>>
>> try_pin_init!(&this in Self {
>> a, // Initialize the field only.
>> b <- b, // Initialize the field only.
>> c: a.into(), // That's the "original" `a`.
>> d <- D::new(this->b), // Not the original `b`, but the pin-init one.
>> })
>>
>> , like a special field projection during initialization.
>
> The main issue with new syntax is the difficulty of implementing it. The
> let one is fine, but it's pretty jarring & doesn't get formatted by
> rustfmt (which I want to eventually have). Using `this` does look better
> IMO, but it's near impossible to implement using declarative macros
> (even using `syn` it seems difficult to me). So either we find some way
> to express it in existing rust syntax (maybe use an attribute?), or we
> just keep it this way.
>
> Maybe Gary has some ideas on how to implement it.
I also thought about reusing `this`, but I think we should not reuse it. We
still need it to get pointers to uninitialized fields.
Surely, we could say that we provide this.as_ptr() to get the NonNull `this`
is currently defined to be and otherwise make it expose only the initialized
fields for a certain scope.
But as you say, that sounds tricky to implement and is probably not very
intuitive either. I'd rather say keep it as it is, if we don't want something
like the `let b <- b` syntax I proposed for formatting reasons.
^ permalink raw reply [flat|nested] 22+ messages in thread
* Re: [PATCH] rust: pin-init: add references to previously initialized fields
2025-09-07 17:29 ` Danilo Krummrich
@ 2025-09-07 21:06 ` Benno Lossin
2025-09-07 21:39 ` Danilo Krummrich
0 siblings, 1 reply; 22+ messages in thread
From: Benno Lossin @ 2025-09-07 21:06 UTC (permalink / raw)
To: Danilo Krummrich
Cc: Boqun Feng, Miguel Ojeda, Alex Gaynor, Gary Guo,
Björn Roy Baron, Andreas Hindborg, Alice Ryhl, Trevor Gross,
Fiona Behrens, Alban Kurti, Greg Kroah-Hartman, Rafael J. Wysocki,
Bjorn Helgaas, Krzysztof Wilczy´nski, rust-for-linux,
linux-kernel
On Sun Sep 7, 2025 at 7:29 PM CEST, Danilo Krummrich wrote:
> On Sun Sep 7, 2025 at 10:41 AM CEST, Benno Lossin wrote:
>> On Sun Sep 7, 2025 at 4:07 AM CEST, Boqun Feng wrote:
>>> On Sat, Sep 06, 2025 at 06:57:04PM -0700, Boqun Feng wrote:
>>>> On Sat, Sep 06, 2025 at 12:52:22PM +0200, Danilo Krummrich wrote:
>>>> > On Fri Sep 5, 2025 at 7:44 PM CEST, Boqun Feng wrote:
>>>> > > On Fri, Sep 05, 2025 at 07:18:25PM +0200, Benno Lossin wrote:
>>>> > > [...]
>>>> > >> index 606946ff4d7f..1ac0b06fa3b3 100644
>>>> > >> --- a/samples/rust/rust_driver_pci.rs
>>>> > >> +++ b/samples/rust/rust_driver_pci.rs
>>>> > >> @@ -78,8 +78,8 @@ fn probe(pdev: &pci::Device<Core>, info: &Self::IdInfo) -> Result<Pin<KBox<Self>
>>>> > >>
>>>> > >> let drvdata = KBox::pin_init(
>>>> > >> try_pin_init!(Self {
>>>> > >> - pdev: pdev.into(),
>>>> > >> bar <- pdev.iomap_region_sized::<{ Regs::END }>(0, c_str!("rust_driver_pci")),
>>>> > >> + pdev: pdev.into(),
>>>> > >
>>>> > > Ok, this example is good enough for me to express the concern here: the
>>>> > > variable shadowing behavior seems not straightforward (maybe because in
>>>> > > normal Rust initalization expression, no binding is created for
>>>> > > previous variables, neither do we have a `let` here).
>>>> > >
>>>> > > Would the future inplace initialization have the similar behavior? I
>>>> > > asked because a natural resolution is adding a special syntax like:
>>>> > >
>>>> > > let a = ..;
>>>> > >
>>>> > > try_pin_init!(Self {
>>>> > > b: a,
>>>> > > let a = a.into(); // create the new binding here.
>>>> > > c: a, // <- use the previous initalized `a`.
>>>> > > }
>>>> >
>>>> > Can you please clarify the example? I'm a bit confused that this is not a field
>>>> > of Self, so currently this can just be written as:
>>>> >
>>>>
>>>> Oh, I could have been more clear: `a` is a field of `Self`, and the
>>>> `let` part initalizes it.
>>>>
>>>> > try_pin_init!(Self {
>>>> > b: a,
>>>> > c: a.into,
>>>> > })
>>>> >
>>>> > Of course assuming that a is Clone, as the code above does as well.
>>>> >
>>>> > So, if we are concerned by the variable shadowing, which I'm less concerned
>>>> > about, maybe we can do this:
>>>>
>>>> I'm not that concerned to block this, but it does look to me like we are
>>>> inventing a new way (and even a different syntax because normal Rust
>>>> initialization doesn't create new bindings) to create binding, so I
>>>> think I should bring it up.
>>>>
>>>> >
>>>> > // The "original" `a` and `b`.
>>>> > let a: A = ...;
>>>> > let b: B = ...;
>>>> >
>>>> > try_pin_init!(Self {
>>>> > a, // Initialize the field only.
>>>> > let b <- b, // Initialize the field and create a `&B` named `b`.
>>>> > c: a.into(), // That's the "original" `a`.
>>>> > d <- D::new(b), // Not the original `b`, but the pin-init one.
>>>> > })
>>>
>>> Another idea is using `&this`:
>>>
>>> try_pin_init!(&this in Self {
>>> a, // Initialize the field only.
>>> b <- b, // Initialize the field only.
>>> c: a.into(), // That's the "original" `a`.
>>> d <- D::new(this->b), // Not the original `b`, but the pin-init one.
>>> })
>>>
>>> , like a special field projection during initialization.
>>
>> The main issue with new syntax is the difficulty of implementing it. The
>> let one is fine, but it's pretty jarring & doesn't get formatted by
>> rustfmt (which I want to eventually have). Using `this` does look better
>> IMO, but it's near impossible to implement using declarative macros
>> (even using `syn` it seems difficult to me). So either we find some way
>> to express it in existing rust syntax (maybe use an attribute?), or we
>> just keep it this way.
>>
>> Maybe Gary has some ideas on how to implement it.
>
> I also thought about reusing `this`, but I think we should not reuse it. We
> still need it to get pointers to uninitialized fields.
>
> Surely, we could say that we provide this.as_ptr() to get the NonNull `this`
> is currently defined to be and otherwise make it expose only the initialized
> fields for a certain scope.
I have some ideas of changing the syntax to be more closure-esque:
init!(|this| -> Result<MyStruct, Error> {
let x = 42;
MyStruct {
x,
}
})
There we could add another parameter, that would then serve this
purpose. We should also probably rename `this` to `slot` & then use
`this` for the initialized version.
But as I said before, implementing the `this` thing from a macro
perspective is rather difficult (I have two ideas on how to do it and
both are bad...).
> But as you say, that sounds tricky to implement and is probably not very
> intuitive either. I'd rather say keep it as it is, if we don't want something
> like the `let b <- b` syntax I proposed for formatting reasons.
I don't feel like that's conveying the correct thing, it looks as if you
are only declaring a local variable.
---
Cheers,
Benno
^ permalink raw reply [flat|nested] 22+ messages in thread
* Re: [PATCH] rust: pin-init: add references to previously initialized fields
2025-09-07 21:06 ` Benno Lossin
@ 2025-09-07 21:39 ` Danilo Krummrich
2025-09-07 22:51 ` Benno Lossin
0 siblings, 1 reply; 22+ messages in thread
From: Danilo Krummrich @ 2025-09-07 21:39 UTC (permalink / raw)
To: Benno Lossin
Cc: Boqun Feng, Miguel Ojeda, Alex Gaynor, Gary Guo,
Björn Roy Baron, Andreas Hindborg, Alice Ryhl, Trevor Gross,
Fiona Behrens, Alban Kurti, Greg Kroah-Hartman, Rafael J. Wysocki,
Bjorn Helgaas, Krzysztof Wilczy´nski, rust-for-linux,
linux-kernel
On Sun Sep 7, 2025 at 11:06 PM CEST, Benno Lossin wrote:
> On Sun Sep 7, 2025 at 7:29 PM CEST, Danilo Krummrich wrote:
> I have some ideas of changing the syntax to be more closure-esque:
>
> init!(|this| -> Result<MyStruct, Error> {
> let x = 42;
> MyStruct {
> x,
> }
> })
>
> There we could add another parameter, that would then serve this
> purpose. We should also probably rename `this` to `slot` & then use
> `this` for the initialized version.
I think that's a pretty good idea, but the part that I think is a little bit
confusing remains: `this` will need to have different fields depending on where
it's accessed.
> But as I said before, implementing the `this` thing from a macro
> perspective is rather difficult (I have two ideas on how to do it and
> both are bad...).
>
>> But as you say, that sounds tricky to implement and is probably not very
>> intuitive either. I'd rather say keep it as it is, if we don't want something
>> like the `let b <- b` syntax I proposed for formatting reasons.
>
> I don't feel like that's conveying the correct thing, it looks as if you
> are only declaring a local variable.
Yeah, it's not great, but given that it's a custom syntax it also does not
create wrong expectations I'd say.
Anyways, I'm fine with either. For now we probably want to land the version as
it is and revisit once you settle on the syntax rework you mentioned above.
^ permalink raw reply [flat|nested] 22+ messages in thread
* Re: [PATCH] rust: pin-init: add references to previously initialized fields
2025-09-07 21:39 ` Danilo Krummrich
@ 2025-09-07 22:51 ` Benno Lossin
2025-09-07 23:33 ` Danilo Krummrich
0 siblings, 1 reply; 22+ messages in thread
From: Benno Lossin @ 2025-09-07 22:51 UTC (permalink / raw)
To: Danilo Krummrich
Cc: Boqun Feng, Miguel Ojeda, Alex Gaynor, Gary Guo,
Björn Roy Baron, Andreas Hindborg, Alice Ryhl, Trevor Gross,
Fiona Behrens, Alban Kurti, Greg Kroah-Hartman, Rafael J. Wysocki,
Bjorn Helgaas, Krzysztof Wilczy´nski, rust-for-linux,
linux-kernel
On Sun Sep 7, 2025 at 11:39 PM CEST, Danilo Krummrich wrote:
> On Sun Sep 7, 2025 at 11:06 PM CEST, Benno Lossin wrote:
>> On Sun Sep 7, 2025 at 7:29 PM CEST, Danilo Krummrich wrote:
>> I have some ideas of changing the syntax to be more closure-esque:
>>
>> init!(|this| -> Result<MyStruct, Error> {
>> let x = 42;
>> MyStruct {
>> x,
>> }
>> })
>>
>> There we could add another parameter, that would then serve this
>> purpose. We should also probably rename `this` to `slot` & then use
>> `this` for the initialized version.
>
> I think that's a pretty good idea, but the part that I think is a little bit
> confusing remains: `this` will need to have different fields depending on where
> it's accessed.
Yeah (that's also the main issue with the macro implementation).
>> But as I said before, implementing the `this` thing from a macro
>> perspective is rather difficult (I have two ideas on how to do it and
>> both are bad...).
>>
>>> But as you say, that sounds tricky to implement and is probably not very
>>> intuitive either. I'd rather say keep it as it is, if we don't want something
>>> like the `let b <- b` syntax I proposed for formatting reasons.
>>
>> I don't feel like that's conveying the correct thing, it looks as if you
>> are only declaring a local variable.
>
> Yeah, it's not great, but given that it's a custom syntax it also does not
> create wrong expectations I'd say.
I'd say it looks like combining the `<-` operation already used by the
`init!` macro & a `let` binding. Thus introducing a local variable
that's (pin) initialized in-place. Not a field of the current struct.
> Anyways, I'm fine with either. For now we probably want to land the version as
> it is and revisit once you settle on the syntax rework you mentioned above.
I actually came up with a third option that looks best IMO:
init!(MyStruct {
x: 42,
#[with_binding]
y: 24,
z: *y,
})
The `#[with_binding]` attribute makes the macro generate a variable `y`.
`x` & `z` don't give access to their value. (we of course should come up
with a better name).
Any thoughts?
---
Cheers,
Benno
^ permalink raw reply [flat|nested] 22+ messages in thread
* Re: [PATCH] rust: pin-init: add references to previously initialized fields
2025-09-07 22:51 ` Benno Lossin
@ 2025-09-07 23:33 ` Danilo Krummrich
2025-09-08 2:08 ` Boqun Feng
0 siblings, 1 reply; 22+ messages in thread
From: Danilo Krummrich @ 2025-09-07 23:33 UTC (permalink / raw)
To: Benno Lossin
Cc: Boqun Feng, Miguel Ojeda, Alex Gaynor, Gary Guo,
Björn Roy Baron, Andreas Hindborg, Alice Ryhl, Trevor Gross,
Fiona Behrens, Alban Kurti, Greg Kroah-Hartman, Rafael J. Wysocki,
Bjorn Helgaas, Krzysztof Wilczy´nski, rust-for-linux,
linux-kernel
On Mon Sep 8, 2025 at 12:51 AM CEST, Benno Lossin wrote:
> I actually came up with a third option that looks best IMO:
>
> init!(MyStruct {
> x: 42,
> #[with_binding]
> y: 24,
> z: *y,
> })
>
> The `#[with_binding]` attribute makes the macro generate a variable `y`.
> `x` & `z` don't give access to their value. (we of course should come up
> with a better name).
>
> Any thoughts?
It may be a bit verbose is some cases, but it makes things pretty obvious, so
LGTM.
How about just #[bind] or #[access]?
^ permalink raw reply [flat|nested] 22+ messages in thread
* Re: [PATCH] rust: pin-init: add references to previously initialized fields
2025-09-07 23:33 ` Danilo Krummrich
@ 2025-09-08 2:08 ` Boqun Feng
2025-09-08 8:27 ` Benno Lossin
0 siblings, 1 reply; 22+ messages in thread
From: Boqun Feng @ 2025-09-08 2:08 UTC (permalink / raw)
To: Danilo Krummrich
Cc: Benno Lossin, Miguel Ojeda, Alex Gaynor, Gary Guo,
Björn Roy Baron, Andreas Hindborg, Alice Ryhl, Trevor Gross,
Fiona Behrens, Alban Kurti, Greg Kroah-Hartman, Rafael J. Wysocki,
Bjorn Helgaas, Krzysztof Wilczy´nski, rust-for-linux,
linux-kernel
On Mon, Sep 08, 2025 at 01:33:26AM +0200, Danilo Krummrich wrote:
> On Mon Sep 8, 2025 at 12:51 AM CEST, Benno Lossin wrote:
> > I actually came up with a third option that looks best IMO:
> >
> > init!(MyStruct {
> > x: 42,
> > #[with_binding]
> > y: 24,
> > z: *y,
> > })
> >
> > The `#[with_binding]` attribute makes the macro generate a variable `y`.
> > `x` & `z` don't give access to their value. (we of course should come up
> > with a better name).
> >
> > Any thoughts?
>
> It may be a bit verbose is some cases, but it makes things pretty obvious, so
> LGTM.
>
> How about just #[bind] or #[access]?
#[shadow] or #[maybe_rebind] ? Or #[pin_ref], the last one is clear
about the purpose.
Regards,
Boqun
^ permalink raw reply [flat|nested] 22+ messages in thread
* Re: [PATCH] rust: pin-init: add references to previously initialized fields
2025-09-08 2:08 ` Boqun Feng
@ 2025-09-08 8:27 ` Benno Lossin
2025-09-08 8:57 ` Danilo Krummrich
0 siblings, 1 reply; 22+ messages in thread
From: Benno Lossin @ 2025-09-08 8:27 UTC (permalink / raw)
To: Boqun Feng, Danilo Krummrich
Cc: Miguel Ojeda, Alex Gaynor, Gary Guo, Björn Roy Baron,
Andreas Hindborg, Alice Ryhl, Trevor Gross, Fiona Behrens,
Alban Kurti, Greg Kroah-Hartman, Rafael J. Wysocki, Bjorn Helgaas,
Krzysztof Wilczy´nski, rust-for-linux, linux-kernel
On Mon Sep 8, 2025 at 4:08 AM CEST, Boqun Feng wrote:
> On Mon, Sep 08, 2025 at 01:33:26AM +0200, Danilo Krummrich wrote:
>> On Mon Sep 8, 2025 at 12:51 AM CEST, Benno Lossin wrote:
>> > I actually came up with a third option that looks best IMO:
>> >
>> > init!(MyStruct {
>> > x: 42,
>> > #[with_binding]
>> > y: 24,
>> > z: *y,
>> > })
>> >
>> > The `#[with_binding]` attribute makes the macro generate a variable `y`.
>> > `x` & `z` don't give access to their value. (we of course should come up
>> > with a better name).
>> >
>> > Any thoughts?
>>
>> It may be a bit verbose is some cases, but it makes things pretty obvious, so
>> LGTM.
>>
>> How about just #[bind] or #[access]?
I like `#[bind]`.
> #[shadow] or #[maybe_rebind] ? Or #[pin_ref], the last one is clear
> about the purpose.
Hmm in `init!` it's never pinned.
---
Cheers,
Benno
^ permalink raw reply [flat|nested] 22+ messages in thread
* Re: [PATCH] rust: pin-init: add references to previously initialized fields
2025-09-08 8:27 ` Benno Lossin
@ 2025-09-08 8:57 ` Danilo Krummrich
2025-09-08 19:38 ` Boqun Feng
0 siblings, 1 reply; 22+ messages in thread
From: Danilo Krummrich @ 2025-09-08 8:57 UTC (permalink / raw)
To: Benno Lossin
Cc: Boqun Feng, Miguel Ojeda, Alex Gaynor, Gary Guo,
Björn Roy Baron, Andreas Hindborg, Alice Ryhl, Trevor Gross,
Fiona Behrens, Alban Kurti, Greg Kroah-Hartman, Rafael J. Wysocki,
Bjorn Helgaas, Krzysztof Wilczy´nski, rust-for-linux,
linux-kernel
On Mon Sep 8, 2025 at 10:27 AM CEST, Benno Lossin wrote:
> On Mon Sep 8, 2025 at 4:08 AM CEST, Boqun Feng wrote:
>> On Mon, Sep 08, 2025 at 01:33:26AM +0200, Danilo Krummrich wrote:
>>> On Mon Sep 8, 2025 at 12:51 AM CEST, Benno Lossin wrote:
>>> > I actually came up with a third option that looks best IMO:
>>> >
>>> > init!(MyStruct {
>>> > x: 42,
>>> > #[with_binding]
>>> > y: 24,
>>> > z: *y,
>>> > })
>>> >
>>> > The `#[with_binding]` attribute makes the macro generate a variable `y`.
>>> > `x` & `z` don't give access to their value. (we of course should come up
>>> > with a better name).
>>> >
>>> > Any thoughts?
>>>
>>> It may be a bit verbose is some cases, but it makes things pretty obvious, so
>>> LGTM.
>>>
>>> How about just #[bind] or #[access]?
>
> I like `#[bind]`.
>
>> #[shadow] or #[maybe_rebind] ? Or #[pin_ref], the last one is clear
>> about the purpose.
>
> Hmm in `init!` it's never pinned.
I thought about #[shadow] as well, but it is not really accurate I think, as we
might not shadow anything. #[maybe_rebind] sounds a bit like it conditionally
rebinds, as in "it may not do anything", but it always binds.
So, I think it should one clear instruction, i.e. #[bind], #[access], #[ref],
#[use], #[let], etc.
^ permalink raw reply [flat|nested] 22+ messages in thread
* Re: [PATCH] rust: pin-init: add references to previously initialized fields
2025-09-08 8:57 ` Danilo Krummrich
@ 2025-09-08 19:38 ` Boqun Feng
2025-09-08 20:31 ` Danilo Krummrich
2025-09-10 10:12 ` Benno Lossin
0 siblings, 2 replies; 22+ messages in thread
From: Boqun Feng @ 2025-09-08 19:38 UTC (permalink / raw)
To: Danilo Krummrich
Cc: Benno Lossin, Miguel Ojeda, Alex Gaynor, Gary Guo,
Björn Roy Baron, Andreas Hindborg, Alice Ryhl, Trevor Gross,
Fiona Behrens, Alban Kurti, Greg Kroah-Hartman, Rafael J. Wysocki,
Bjorn Helgaas, Krzysztof Wilczy´nski, rust-for-linux,
linux-kernel
On Mon, Sep 08, 2025 at 10:57:36AM +0200, Danilo Krummrich wrote:
> On Mon Sep 8, 2025 at 10:27 AM CEST, Benno Lossin wrote:
> > On Mon Sep 8, 2025 at 4:08 AM CEST, Boqun Feng wrote:
> >> On Mon, Sep 08, 2025 at 01:33:26AM +0200, Danilo Krummrich wrote:
> >>> On Mon Sep 8, 2025 at 12:51 AM CEST, Benno Lossin wrote:
> >>> > I actually came up with a third option that looks best IMO:
> >>> >
> >>> > init!(MyStruct {
> >>> > x: 42,
> >>> > #[with_binding]
> >>> > y: 24,
> >>> > z: *y,
> >>> > })
> >>> >
> >>> > The `#[with_binding]` attribute makes the macro generate a variable `y`.
> >>> > `x` & `z` don't give access to their value. (we of course should come up
> >>> > with a better name).
> >>> >
> >>> > Any thoughts?
> >>>
> >>> It may be a bit verbose is some cases, but it makes things pretty obvious, so
> >>> LGTM.
> >>>
> >>> How about just #[bind] or #[access]?
> >
> > I like `#[bind]`.
> >
> >> #[shadow] or #[maybe_rebind] ? Or #[pin_ref], the last one is clear
> >> about the purpose.
> >
> > Hmm in `init!` it's never pinned.
>
> I thought about #[shadow] as well, but it is not really accurate I think, as we
> might not shadow anything. #[maybe_rebind] sounds a bit like it conditionally
> rebinds, as in "it may not do anything", but it always binds.
>
> So, I think it should one clear instruction, i.e. #[bind], #[access], #[ref],
> #[use], #[let], etc.
In that sense I think `#[let]` is best? Because it indicates this field
initialization works as a `let`-statement (in term of creating a new
binding), of course I don't have strong ojections against other options.
Regards,
Boqun
^ permalink raw reply [flat|nested] 22+ messages in thread
* Re: [PATCH] rust: pin-init: add references to previously initialized fields
2025-09-08 19:38 ` Boqun Feng
@ 2025-09-08 20:31 ` Danilo Krummrich
2025-09-10 10:12 ` Benno Lossin
1 sibling, 0 replies; 22+ messages in thread
From: Danilo Krummrich @ 2025-09-08 20:31 UTC (permalink / raw)
To: Boqun Feng
Cc: Benno Lossin, Miguel Ojeda, Alex Gaynor, Gary Guo,
Björn Roy Baron, Andreas Hindborg, Alice Ryhl, Trevor Gross,
Fiona Behrens, Alban Kurti, Greg Kroah-Hartman, Rafael J. Wysocki,
Bjorn Helgaas, Krzysztof Wilczy´nski, rust-for-linux,
linux-kernel
On 9/8/25 9:38 PM, Boqun Feng wrote:
> On Mon, Sep 08, 2025 at 10:57:36AM +0200, Danilo Krummrich wrote:
>> So, I think it should one clear instruction, i.e. #[bind], #[access], #[ref],
>> #[use], #[let], etc.
>
> In that sense I think `#[let]` is best? Because it indicates this field
> initialization works as a `let`-statement (in term of creating a new
> binding), of course I don't have strong ojections against other options.
Same for me, I'm fine with any of this kind. :)
^ permalink raw reply [flat|nested] 22+ messages in thread
* Re: [PATCH] rust: pin-init: add references to previously initialized fields
2025-09-08 19:38 ` Boqun Feng
2025-09-08 20:31 ` Danilo Krummrich
@ 2025-09-10 10:12 ` Benno Lossin
1 sibling, 0 replies; 22+ messages in thread
From: Benno Lossin @ 2025-09-10 10:12 UTC (permalink / raw)
To: Boqun Feng, Danilo Krummrich
Cc: Miguel Ojeda, Alex Gaynor, Gary Guo, Björn Roy Baron,
Andreas Hindborg, Alice Ryhl, Trevor Gross, Fiona Behrens,
Alban Kurti, Greg Kroah-Hartman, Rafael J. Wysocki, Bjorn Helgaas,
Krzysztof Wilczy´nski, rust-for-linux, linux-kernel
On Mon Sep 8, 2025 at 9:38 PM CEST, Boqun Feng wrote:
> On Mon, Sep 08, 2025 at 10:57:36AM +0200, Danilo Krummrich wrote:
>> On Mon Sep 8, 2025 at 10:27 AM CEST, Benno Lossin wrote:
>> > On Mon Sep 8, 2025 at 4:08 AM CEST, Boqun Feng wrote:
>> >> On Mon, Sep 08, 2025 at 01:33:26AM +0200, Danilo Krummrich wrote:
>> >>> On Mon Sep 8, 2025 at 12:51 AM CEST, Benno Lossin wrote:
>> >>> > I actually came up with a third option that looks best IMO:
>> >>> >
>> >>> > init!(MyStruct {
>> >>> > x: 42,
>> >>> > #[with_binding]
>> >>> > y: 24,
>> >>> > z: *y,
>> >>> > })
>> >>> >
>> >>> > The `#[with_binding]` attribute makes the macro generate a variable `y`.
>> >>> > `x` & `z` don't give access to their value. (we of course should come up
>> >>> > with a better name).
>> >>> >
>> >>> > Any thoughts?
>> >>>
>> >>> It may be a bit verbose is some cases, but it makes things pretty obvious, so
>> >>> LGTM.
>> >>>
>> >>> How about just #[bind] or #[access]?
>> >
>> > I like `#[bind]`.
>> >
>> >> #[shadow] or #[maybe_rebind] ? Or #[pin_ref], the last one is clear
>> >> about the purpose.
>> >
>> > Hmm in `init!` it's never pinned.
>>
>> I thought about #[shadow] as well, but it is not really accurate I think, as we
>> might not shadow anything. #[maybe_rebind] sounds a bit like it conditionally
>> rebinds, as in "it may not do anything", but it always binds.
>>
>> So, I think it should one clear instruction, i.e. #[bind], #[access], #[ref],
>> #[use], #[let], etc.
>
> In that sense I think `#[let]` is best? Because it indicates this field
> initialization works as a `let`-statement (in term of creating a new
> binding), of course I don't have strong ojections against other options.
Ultimately I decided to go with `#[bind]`, since I felt like `#[let]`
might be confused with just having a let statement (ie replacing the
assignment with a let binding).
`#[bind]` also might be confused with some device binding I guess, but
we can rename it's too bad.
---
Cheers,
Benno
^ permalink raw reply [flat|nested] 22+ messages in thread
* Re: [PATCH] rust: pin-init: add references to previously initialized fields
2025-09-05 14:00 [PATCH] rust: pin-init: add references to previously initialized fields Benno Lossin
2025-09-05 17:18 ` Benno Lossin
@ 2025-09-05 17:21 ` Boqun Feng
2025-09-05 18:38 ` Danilo Krummrich
2025-09-06 14:23 ` kernel test robot
2025-09-11 21:35 ` Benno Lossin
3 siblings, 1 reply; 22+ messages in thread
From: Boqun Feng @ 2025-09-05 17:21 UTC (permalink / raw)
To: Benno Lossin
Cc: Miguel Ojeda, Alex Gaynor, Gary Guo, Björn Roy Baron,
Andreas Hindborg, Alice Ryhl, Trevor Gross, Danilo Krummrich,
Fiona Behrens, Alban Kurti, rust-for-linux, linux-kernel
On Fri, Sep 05, 2025 at 04:00:46PM +0200, Benno Lossin wrote:
> After initializing a field in an initializer macro, create a variable
> holding a reference that points at that field. The type is either
> `Pin<&mut T>` or `&mut T` depending on the field's structural pinning
> kind.
>
It's hard to review because of lack of examples. Could you or Danilo
share some sample usages? Thanks!
Regards,
Boqun
> Link: https://github.com/Rust-for-Linux/pin-init/pull/83/commits/0f658594c39398f58cd5cb99a8141e370e225e74
> Signed-off-by: Benno Lossin <lossin@kernel.org>
> ---
[...]
^ permalink raw reply [flat|nested] 22+ messages in thread
* Re: [PATCH] rust: pin-init: add references to previously initialized fields
2025-09-05 17:21 ` Boqun Feng
@ 2025-09-05 18:38 ` Danilo Krummrich
0 siblings, 0 replies; 22+ messages in thread
From: Danilo Krummrich @ 2025-09-05 18:38 UTC (permalink / raw)
To: Boqun Feng
Cc: Benno Lossin, Miguel Ojeda, Alex Gaynor, Gary Guo,
Björn Roy Baron, Andreas Hindborg, Alice Ryhl, Trevor Gross,
Fiona Behrens, Alban Kurti, Alexandre Courbot, rust-for-linux,
linux-kernel
(Cc: Alex)
On Fri Sep 5, 2025 at 7:21 PM CEST, Boqun Feng wrote:
> On Fri, Sep 05, 2025 at 04:00:46PM +0200, Benno Lossin wrote:
>> After initializing a field in an initializer macro, create a variable
>> holding a reference that points at that field. The type is either
>> `Pin<&mut T>` or `&mut T` depending on the field's structural pinning
>> kind.
>>
>
> It's hard to review because of lack of examples. Could you or Danilo
> share some sample usages? Thanks!
Sure, here's an example. Eventually, it's going to be a bit more complicated,
but basically that's it.
#[pin_data(PinnedDrop)]
pub(crate) struct Gpu {
spec: Spec,
bar: Arc<Devres<Bar0>>,
sysmem_flush: SysmemFlush,
gsp_falcon: Falcon<Gsp>,
sec2_falcon: Falcon<Sec2>,
#[pin]
gsp: Gsp,
}
impl Gpu {
pub(crate) fn new(
dev: &Device<Bound>,
bar: Arc<Devres<Bar0>>,
) -> impl PinInit<Self, Error> + '_ {
try_pin_init(Self {
bar,
spec: Spec::new(bar.access(dev)?)?,
gsp_falcon: Falcon::<Gsp>::new(dev, spec.chipset)?,
sec2_falcon: Falcon::<Sec2>::new(dev, spec.chipset)?,
sysmem_flush: SysmemFlush::register(dev, bar.access(dev)?, spec.chipset)?
gsp <- Gsp::new(gsp_falcon, sec2_falcon, sysmem_flush)?,
})
}
}
Imagine how much unsafe pointer mess this needs without this patch. :)
^ permalink raw reply [flat|nested] 22+ messages in thread
* Re: [PATCH] rust: pin-init: add references to previously initialized fields
2025-09-05 14:00 [PATCH] rust: pin-init: add references to previously initialized fields Benno Lossin
2025-09-05 17:18 ` Benno Lossin
2025-09-05 17:21 ` Boqun Feng
@ 2025-09-06 14:23 ` kernel test robot
2025-09-11 21:35 ` Benno Lossin
3 siblings, 0 replies; 22+ messages in thread
From: kernel test robot @ 2025-09-06 14:23 UTC (permalink / raw)
To: Benno Lossin, Miguel Ojeda, Alex Gaynor, Boqun Feng, Gary Guo,
Björn Roy Baron, Andreas Hindborg, Alice Ryhl, Trevor Gross,
Danilo Krummrich, Fiona Behrens, Alban Kurti
Cc: llvm, oe-kbuild-all, rust-for-linux, linux-kernel
Hi Benno,
kernel test robot noticed the following build errors:
[auto build test ERROR on 8f5ae30d69d7543eee0d70083daf4de8fe15d585]
url: https://github.com/intel-lab-lkp/linux/commits/Benno-Lossin/rust-pin-init-add-references-to-previously-initialized-fields/20250905-220242
base: 8f5ae30d69d7543eee0d70083daf4de8fe15d585
patch link: https://lore.kernel.org/r/20250905140047.3325945-1-lossin%40kernel.org
patch subject: [PATCH] rust: pin-init: add references to previously initialized fields
config: x86_64-rhel-9.4-rust (https://download.01.org/0day-ci/archive/20250906/202509062218.Mo9Wsmcd-lkp@intel.com/config)
compiler: clang version 20.1.8 (https://github.com/llvm/llvm-project 87f0227cb60147a26a1eeb4fb06e3b505e9c7261)
rustc: rustc 1.88.0 (6b00bc388 2025-06-23)
reproduce (this is a W=1 build): (https://download.01.org/0day-ci/archive/20250906/202509062218.Mo9Wsmcd-lkp@intel.com/reproduce)
If you fix the issue in a separate patch/commit (i.e. not just a new version of
the same patch/commit), kindly add following tags
| Reported-by: kernel test robot <lkp@intel.com>
| Closes: https://lore.kernel.org/oe-kbuild-all/202509062218.Mo9Wsmcd-lkp@intel.com/
All errors (new ones prefixed by >>):
>> error[E0308]: mismatched types
--> rust/kernel/devres.rs:154:66
|
154 | bindings::devm_add_action(dev.as_raw(), Some(callback), inner.cast())
| ---- ^^^^^^^^ expected fn pointer, found `&mut unsafe extern "C" fn(*mut c_void)`
| |
| arguments to this enum variant are incorrect
|
= note: expected fn pointer `unsafe extern "C" fn(_)`
found mutable reference `&mut unsafe extern "C" fn(_)`
help: the type constructed contains `&mut unsafe extern "C" fn(*mut ffi::c_void)` due to the type of the argument passed
--> rust/kernel/devres.rs:154:61
|
154 | bindings::devm_add_action(dev.as_raw(), Some(callback), inner.cast())
| ^^^^^--------^
| |
| this argument influences the type of `Some`
note: tuple variant defined here
--> /opt/cross/rustc-1.88.0-bindgen-0.72.0/rustup/toolchains/1.88.0-x86_64-unknown-linux-gnu/lib/rustlib/src/rust/library/core/src/option.rs:597:5
|
597 | Some(#[stable(feature = "rust1", since = "1.0.0")] T),
| ^^^^
help: consider dereferencing the borrow
|
154 | bindings::devm_add_action(dev.as_raw(), Some(*callback), inner.cast())
| +
--
0-DAY CI Kernel Test Service
https://github.com/intel/lkp-tests/wiki
^ permalink raw reply [flat|nested] 22+ messages in thread
* Re: [PATCH] rust: pin-init: add references to previously initialized fields
2025-09-05 14:00 [PATCH] rust: pin-init: add references to previously initialized fields Benno Lossin
` (2 preceding siblings ...)
2025-09-06 14:23 ` kernel test robot
@ 2025-09-11 21:35 ` Benno Lossin
3 siblings, 0 replies; 22+ messages in thread
From: Benno Lossin @ 2025-09-11 21:35 UTC (permalink / raw)
To: Benno Lossin, Miguel Ojeda, Alex Gaynor, Boqun Feng, Gary Guo,
Björn Roy Baron, Andreas Hindborg, Alice Ryhl, Trevor Gross,
Danilo Krummrich, Fiona Behrens, Alban Kurti
Cc: rust-for-linux, linux-kernel
On Fri Sep 5, 2025 at 4:00 PM CEST, Benno Lossin wrote:
> After initializing a field in an initializer macro, create a variable
> holding a reference that points at that field. The type is either
> `Pin<&mut T>` or `&mut T` depending on the field's structural pinning
> kind.
>
> Link: https://github.com/Rust-for-Linux/pin-init/pull/83/commits/0f658594c39398f58cd5cb99a8141e370e225e74
> Signed-off-by: Benno Lossin <lossin@kernel.org>
Applied to pin-init-next, thanks everyone!
Included the fixes to devres & rust_driver_pci.
---
Cheers,
Benno
^ permalink raw reply [flat|nested] 22+ messages in thread
end of thread, other threads:[~2025-09-11 21:36 UTC | newest]
Thread overview: 22+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2025-09-05 14:00 [PATCH] rust: pin-init: add references to previously initialized fields Benno Lossin
2025-09-05 17:18 ` Benno Lossin
2025-09-05 17:44 ` Boqun Feng
2025-09-06 10:52 ` Danilo Krummrich
2025-09-07 1:57 ` Boqun Feng
2025-09-07 2:07 ` Boqun Feng
2025-09-07 8:41 ` Benno Lossin
2025-09-07 17:29 ` Danilo Krummrich
2025-09-07 21:06 ` Benno Lossin
2025-09-07 21:39 ` Danilo Krummrich
2025-09-07 22:51 ` Benno Lossin
2025-09-07 23:33 ` Danilo Krummrich
2025-09-08 2:08 ` Boqun Feng
2025-09-08 8:27 ` Benno Lossin
2025-09-08 8:57 ` Danilo Krummrich
2025-09-08 19:38 ` Boqun Feng
2025-09-08 20:31 ` Danilo Krummrich
2025-09-10 10:12 ` Benno Lossin
2025-09-05 17:21 ` Boqun Feng
2025-09-05 18:38 ` Danilo Krummrich
2025-09-06 14:23 ` kernel test robot
2025-09-11 21:35 ` Benno Lossin
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).