Message-ID: <d5e8e635-08c4-4d3d-99a8-27f3b3a2153b@gmail.com>
Date: Fri, 2 May 2025 02:08:13 +0200
From: Christian Schrefl <chrisi.schrefl@...il.com>
To: Benno Lossin <lossin@...nel.org>, Sky <sky@...9.dev>,
 Miguel Ojeda <ojeda@...nel.org>, Alex Gaynor <alex.gaynor@...il.com>,
 Boqun Feng <boqun.feng@...il.com>, Gary Guo <gary@...yguo.net>,
 Björn Roy Baron <bjorn3_gh@...tonmail.com>,
 Benno Lossin <benno.lossin@...ton.me>,
 Andreas Hindborg <a.hindborg@...nel.org>, Alice Ryhl <aliceryhl@...gle.com>,
 Trevor Gross <tmgross@...ch.edu>, Danilo Krummrich <dakr@...nel.org>,
 Gerald Wisböck <gerald.wisboeck@...ther.ink>,
 Ralf Jung <post@...fj.de>
Cc: linux-kernel@...r.kernel.org, rust-for-linux@...r.kernel.org
Subject: Re: [PATCH v2 1/3] rust: add UnsafePinned type

[cc Ralf]

On 02.05.25 12:51 AM, Benno Lossin wrote:
> On Thu May 1, 2025 at 9:11 PM CEST, Christian Schrefl wrote:
>> On 01.05.25 8:51 PM, Benno Lossin wrote:
>>> On Wed Apr 30, 2025 at 7:30 PM CEST, Christian Schrefl wrote:
>>>> On 30.04.25 11:45 AM, Benno Lossin wrote:
>>>>> On Wed Apr 30, 2025 at 10:36 AM CEST, Christian Schrefl wrote:
>>>>>> +/// This implementation works because of the "`!Unpin` hack" in rustc, which allows (some kinds of)
>>>>>> +/// mutual aliasing of `!Unpin` types. This hack might be removed at some point, after which only
>>>>>> +/// the `core::pin::UnsafePinned` type will allow this behavior. In order to simplify the migration
>>>>>> +/// to future rust versions only this polyfill of this type should be used when this behavior is
>>>>>> +/// required.
>>>>>> +///
>>>>>> +/// In order to disable niche optimizations this implementation uses [`UnsafeCell`] internally,
>>>>>> +/// the upstream version however will not. So the fact that [`UnsafePinned`] contains an
>>>>>> +/// [`UnsafeCell`] must not be relied on (Other than the niche blocking).
>>>>>
>>>>> I would make this last paragraph a normal comment, I don't think we
>>>>> should expose it in the docs.
>>>>
>>>> I added this as docs since I wanted it to be a bit more visible,
>>>> but I can replace the comment text (about `UnsafeCell`) with this paragraph
>>>> and drop it from the docs if you want.
>>>
>>> I think we shouldn't talk about these implementation details in the
>>> docs.
>>
>> Alright, what do you think of:
>>
>> // As opposed to the upstream Rust type this contains a `PhantomPinned`` and `UnsafeCell<T>`
> 
> There are two '`' after PhantomPinned.
> 
>> // - `PhantomPinned` to avoid needing a `impl<T> !Unpin for UnsafePinned<T>`
> 
> s/ a / an /
> 
> I find the phrasing 'avoid needing <negative impl>' a bit weird, I'd
> just say "`PhantomPinned` to ensure the struct always is `!Unpin` and
> thus enables the `!Unpin` hack".

Thanks I'll use that.

> 
> If you have a link to somewhere that explains that hack, then I'd also
> put it there. I forgot if it's written down somewhere.

I haven't found anything that describes the hack in detail.
>From what I understand its a combination of disabling `noalias`
[0] (this PR enables it for `Unpin` types) and disabling 
`dereferencable` [1] on `&mut !Unpin` types.
Related rust issue about this [2].

Maybe Alice, Ralf or someone else form the rust side can provide
a better reference?

[0]: https://github.com/rust-lang/rust/pull/82834
[1]: https://github.com/rust-lang/rust/pull/106180
[2]: https://github.com/rust-lang/rust/issues/63818

Cheers
Christian

>> //   Required to use the `!Unpin hack`.
>> // - In order to disable niche optimizations this implementation uses `UnsafeCell` internally,
>> //   the upstream version however currently does not. This will most likely change in the future
>> //   but for now we don't expose this in the documentation, since adding the guarantee is simpler
>> //   than removing it. Meaning that for now the fact that `UnsafePinned` contains an `UnsafeCell`
>> //   must not be relied on (Other than the niche blocking).

Powered by blists - more mailing lists