Re: [PATCH] rust: pin-init: add references to previously initialized fields

["Benno Lossin" <lossin@kernel.org>]

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