[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]
Message-Id: <DCMVHB8P7Z2G.PCOWPQXBSBT6@kernel.org>
Date: Sun, 07 Sep 2025 23:06:21 +0200
From: "Benno Lossin" <lossin@...nel.org>
To: "Danilo Krummrich" <dakr@...nel.org>
Cc: "Boqun Feng" <boqun.feng@...il.com>, "Miguel Ojeda" <ojeda@...nel.org>,
"Alex Gaynor" <alex.gaynor@...il.com>, "Gary Guo" <gary@...yguo.net>,
Björn Roy Baron <bjorn3_gh@...tonmail.com>, "Andreas
Hindborg" <a.hindborg@...nel.org>, "Alice Ryhl" <aliceryhl@...gle.com>,
"Trevor Gross" <tmgross@...ch.edu>, "Fiona Behrens" <me@...enk.dev>, "Alban
Kurti" <kurti@...icto.ai>, "Greg Kroah-Hartman"
<gregkh@...uxfoundation.org>, "Rafael J. Wysocki" <rafael@...nel.org>,
"Bjorn Helgaas" <bhelgaas@...gle.com>,
Krzysztof Wilczy´nski <kwilczynski@...nel.org>,
<rust-for-linux@...r.kernel.org>, <linux-kernel@...r.kernel.org>
Subject: Re: [PATCH] rust: pin-init: add references to previously
initialized fields
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
Powered by blists - more mailing lists