Messages in this thread | | | Date | Wed, 03 Apr 2024 15:51:42 +0000 | From | Benno Lossin <> | Subject | Re: [PATCH 1/9] rust: list: add ListArc |
| |
On 02.04.24 14:16, Alice Ryhl wrote: > +impl<T: ListArcSafe<ID>, const ID: u64> ListArc<T, ID> { > + /// Constructs a new reference counted instance of `T`. > + pub fn try_new(contents: T) -> Result<Self, AllocError> { > + Ok(Self::from_unique(UniqueArc::try_new(contents)?)) > + } > + > + /// Use the given initializer to in-place initialize a `T`. > + /// > + /// If `T: !Unpin` it will not be able to move afterwards. > + pub fn pin_init<E>(init: impl PinInit<T, E>) -> error::Result<Self> > + where > + Error: From<E>, > + { > + Ok(Self::from_pin_unique(UniqueArc::pin_init(init)?)) > + }
pin-init has a general trait for this: InPlaceInit. I don't know if the other functions that it provides would help you.
> +} > + > +impl<T, const ID: u64> ListArc<T, ID> > +where > + T: ListArcSafe<ID> + ?Sized, > +{ > + /// Convert a [`UniqueArc`] into a [`ListArc`]. > + pub fn from_unique(mut unique: UniqueArc<T>) -> Self { > + // SAFETY: We have a `UniqueArc`, so there is no `ListArc`. > + unsafe { T::on_create_list_arc_from_unique(&mut unique) }; > + let arc = Arc::from(unique); > + // SAFETY: We just called `on_create_list_arc_from_unique` on an arc without a `ListArc`, > + // so we can create a `ListArc`. > + unsafe { Self::transmute_from_arc(arc) } > + } > + > + /// Convert a pinned [`UniqueArc`] into a [`ListArc`]. > + pub fn from_pin_unique(unique: Pin<UniqueArc<T>>) -> Self { > + // SAFETY: We continue to treat this pointer as pinned after this call, since `ListArc` > + // implicitly pins its value.
This is not sufficient, since you also rely on `Self::from_unique` to handle the parameter as if it were pinned, which it does not. Since it calls `T::on_create_list_arc_from_unique` which just gets a `&mut self` as a parameter and it could move stuff out.
> + Self::from_unique(unsafe { Pin::into_inner_unchecked(unique) }) > + } > + > + /// Like [`from_unique`], but creates two `ListArcs`. > + /// > + /// The two ids must be different. > + /// > + /// [`from_unique`]: ListArc::from_unique > + pub fn pair_from_unique<const ID2: u64>(mut unique: UniqueArc<T>) -> (Self, ListArc<T, ID2>) > + where > + T: ListArcSafe<ID2>, > + { > + assert_ne!(ID, ID2);
Can this be a `build_assert!`?
> + > + // SAFETY: We have a `UniqueArc`, so we can call this method.
I liked the comment from above better:
// SAFETY: We have a `UniqueArc`, so there is no `ListArc`.
Maybe use the variation "so there are no `ListArc`s for any ID.".
> + unsafe { <T as ListArcSafe<ID>>::on_create_list_arc_from_unique(&mut unique) }; > + // SAFETY: We have a `UniqueArc`, so we can call this method. The two ids are not equal. > + unsafe { <T as ListArcSafe<ID2>>::on_create_list_arc_from_unique(&mut unique) }; > + > + let arc1 = Arc::from(unique); > + let arc2 = Arc::clone(&arc1); > + > + // SAFETY: We just called `on_create_list_arc_from_unique` on an arc without a `ListArc`, > + // so we can create a `ListArc`.
I would mention the two different IDs again.
> + unsafe { > + ( > + Self::transmute_from_arc(arc1), > + ListArc::transmute_from_arc(arc2), > + ) > + } > + } > + > + /// Like [`pair_from_unique`], but uses a pinned arc. > + /// > + /// The two ids must be different. > + /// > + /// [`pair_from_unique`]: ListArc::pair_from_unique > + pub fn pair_from_pin_unique<const ID2: u64>( > + unique: Pin<UniqueArc<T>>, > + ) -> (Self, ListArc<T, ID2>) > + where > + T: ListArcSafe<ID2>, > + { > + // SAFETY: We continue to treat this pointer as pinned after this call, since `ListArc` > + // implicitly pins its value. > + Self::pair_from_unique(unsafe { Pin::into_inner_unchecked(unique) }) > + } > + > + /// Transmutes an [`Arc`] into a `ListArc` without updating the tracking inside `T`. > + /// > + /// # Safety > + /// > + /// * The value must not already have a `ListArc` reference. > + /// * The tracking inside `T` must think that there is a `ListArc` reference. > + #[inline] > + unsafe fn transmute_from_arc(me: Arc<T>) -> Self { > + // INVARIANT: By the safety requirements, the invariants on `ListArc` are satisfied. > + // SAFETY: ListArc is repr(transparent). > + unsafe { core::mem::transmute(me) }
Why do you need a transmute here? Can't you just construct the struct?
Self { arc: me }
-- Cheers, Benno
> + } > +
| |