Re: [PATCH v2 1/3] rust: add UnsafePinned type

From: Benno Lossin
Date: Wed Mar 26 2025 - 16:26:57 EST


On Fri Jan 31, 2025 at 4:08 PM CET, Christian Schrefl wrote:
> @@ -573,3 +576,57 @@ pub enum Either<L, R> {
> /// [`NotThreadSafe`]: type@NotThreadSafe
> #[allow(non_upper_case_globals)]
> pub const NotThreadSafe: NotThreadSafe = PhantomData;
> +
> +/// Stores a value that may be used from multiple mutable pointers.
> +///
> +/// `UnsafePinned` gets rid of some of the usual assumptions that Rust has for a value:
> +/// - The value is allowed to be mutated, when a `&UnsafePinned<T>` exists on the Rust side.
> +/// - No uniqueness for mutable references: it is fine to have multiple `&mut UnsafePinned<T>`
> +/// point to the same value.

We have another patch series [1] in transit that changes the wording on
the `Opaque<T>` type. I think we should mirror the same wording here.

[1]: https://lore.kernel.org/rust-for-linux/20250305053438.1532397-2-dirk.behme@xxxxxxxxxxxx/

> +///
> +/// To avoid the ability to use [`core::mem::swap`] this still needs to be used through a

IIRC typing out the whole path makes it also appear in the docs. I think
we should just use `mem::swap` and omit `core` of course you need to
add this to make it work:

/// [`mem::swap`]: core::mem::swap

> +/// [`core::pin::Pin`] reference.

Here, I would link to `Pin`, but say "[pinned](core::pin::Pin) pointer."
instead, since eg `Pin<Box<T>>` also is okay to use.

> +///
> +/// This is useful for cases where a value might be shared with C code
> +/// but not interpreted by it or in cases where it can not always be guaranteed that the
> +/// references are unique.
> +///
> +/// This is similar to [`Opaque<T>`] but is guaranteed to always contain valid data and will
> +/// call the [`Drop`] implementation of `T` when dropped.
> +#[repr(transparent)]
> +pub struct UnsafePinned<T> {
> + value: UnsafeCell<T>,
> + _pin: PhantomPinned,
> +}
> +
> +impl<T> UnsafePinned<T> {
> + /// Creates a new [`UnsafePinned`] value.
> + pub const fn new(value: T) -> Self {
> + Self {
> + value: UnsafeCell::new(value),
> + _pin: PhantomPinned,
> + }
> + }
> +
> + /// Create an [`UnsafePinned`] pin-initializer from the given pin-initializer.
> + pub fn try_pin_init<E>(value: impl PinInit<T, E>) -> impl PinInit<Self, E> {
> + // SAFETY:
> + // - In case of an error in `value` the error is returned, otherwise `slot` is fully
> + // initialized, since `self.value` is initialized and `_pin` is a zero sized type.
> + // - The `Pin` invariants of `self.value` are upheld, since no moving occurs.
> + unsafe { init::pin_init_from_closure(move |slot| value.__pinned_init(Self::raw_get(slot))) }

Ah this is a bit suboptimal, but I guess there currently isn't a better
way to do this. I'll add it to my list of things to improve with
pin-init.

> + }
> +
> + /// Returns a raw pointer to the contained data.
> + pub const fn get(&self) -> *mut T {
> + UnsafeCell::get(&self.value).cast::<T>()
> + }
> +
> + /// Gets the value behind `this`.
> + ///
> + /// This function is useful to get access to the value without creating intermediate
> + /// references.
> + pub const fn raw_get(this: *const Self) -> *mut T {
> + UnsafeCell::raw_get(this.cast::<UnsafeCell<MaybeUninit<T>>>()).cast::<T>()

Why the cast to `MaybeUninit<T>`? I think this can just be:

UnsafeCell::raw_get(&raw const this.value)

---
Cheers,
Benno

> + }
> +}