Received-SPF: pass (google.com: domain of linux-kernel-owner@vger.kernel.org designates 2620:137:e000::1:20 as permitted sender) client-ip=2620:137:e000::1:20;
Feedback-ID: i56684263:Fastmail
Message-ID: <1c2338c5-adab-f85c-645e-44e361ef0677@ryhl.io>
Date:   Thu, 30 Mar 2023 22:24:53 +0200
MIME-Version: 1.0
User-Agent: Mozilla/5.0 (X11; Linux x86_64; rv:102.0) Gecko/20100101
 Thunderbird/102.9.0
Subject: Re: [PATCH v3 06/13] rust: init/sync: add `InPlaceInit` trait to
 pin-initialize smart pointers
Content-Language: en-US-large
To:     Benno Lossin <y86-dev@protonmail.com>
Cc:     rust-for-linux@vger.kernel.org, linux-kernel@vger.kernel.org,
        patches@lists.linux.dev, Miguel Ojeda <ojeda@kernel.org>,
        Alex Gaynor <alex.gaynor@gmail.com>,
        Wedson Almeida Filho <wedsonaf@gmail.com>,
        Boqun Feng <boqun.feng@gmail.com>, Gary Guo <gary@garyguo.net>,
        =?UTF-8?Q?Bj=c3=b6rn_Roy_Baron?= <bjorn3_gh@protonmail.com>
References: <20230329223239.138757-1-y86-dev@protonmail.com>
 <20230329223239.138757-7-y86-dev@protonmail.com>
 <1f93a045-5bd8-e07f-cf1b-7b1196c8ab54@ryhl.io>
 <3ea5cd8d-84ca-59bd-de50-fef185233a50@protonmail.com>
From:   Alice Ryhl <alice@ryhl.io>
In-Reply-To: <3ea5cd8d-84ca-59bd-de50-fef185233a50@protonmail.com>
Content-Type: text/plain; charset=UTF-8; format=flowed
Content-Transfer-Encoding: 7bit
Precedence: bulk

On 3/30/23 17:28, Benno Lossin wrote:
> On 30.03.23 16:37, Alice Ryhl wrote:
>> On 3/30/23 00:33, y86-dev@protonmail.com wrote:
>>> From: Benno Lossin <y86-dev@protonmail.com>
>>>
>>> The `InPlaceInit` trait that provides two functions, for initializing
>>> using `PinInit<T, E>` and `Init<T>`. It is implemented by `Arc<T>`,
>>> `UniqueArc<T>` and `Box<T>`.
>>>
>>> Signed-off-by: Benno Lossin <y86-dev@protonmail.com>
>>> ---
>>>
>>> +/// Smart pointer that can initialize memory in-place.
>>> +pub trait InPlaceInit<T>: Sized {
>>> +    /// Use the given initializer to in-place initialize a `T`.
>>> +    ///
>>> +    /// If `T: !Unpin` it will not be able to move afterwards.
>>> +    fn pin_init<E>(init: impl PinInit<T, E>) -> error::Result<Pin<Self>>
>>> +    where
>>> +        Error: From<E>;
>>> +
>>> +    /// Use the given initializer to in-place initialize a `T`.
>>> +    fn init<E>(init: impl Init<T, E>) -> error::Result<Self>
>>> +    where
>>> +        Error: From<E>;
>>> +}
>>
>> This definition is potentially rather limiting, because it can only be
>> used with error types that can be converted into a `kernel::Error`. What
>> do you think of this alternative?
>>
>> pub trait InPlaceInit<T>: Sized {
>>        fn pin_init<E>(init: impl PinInit<T, E>) -> Result<Pin<Self>, E>
>>        where
>>            E: From<AllocError>;
>>
>>        fn init<E>(init: impl Init<T, E>) -> Result<Self, E>
>>        where
>>            E: From<AllocError>;
>> }
> 
> I initially implemented it like this, but it required almost always that
> `E` is specified, I will try and see if the situation is any different now,
> but I do not think so. In the user-space version of this API (see [1]) I
> have four functions, normal variants that return an `AllocError` and `try`
> variants that look exactly like what you suggested. In the kernel, we could
> make the normal variants as they are now and add the `try` variants as you
> described.
> 
> [1]: https://docs.rs/pinned-init/0.0.5/pinned_init/trait.InPlaceInit.html
> 
> --
> Cheers,
> Benno

Ok, if my alternative causes type inference problems, then let us keep 
the version you proposed here. We can add more variants later. (Or if 
you want to add more now, that's also fine.)

Reviewed-by: Alice Ryhl <aliceryhl@google.com>