Skip to content

Commit

Permalink
Rollup merge of rust-lang#129449 - coolreader18:pin-as_deref_mut-sign…
Browse files Browse the repository at this point in the history 
…ature, r=dtolnay

Put Pin::as_deref_mut in impl Pin<Ptr> / rearrange Pin methods

Tracking issue: rust-lang#86918

Based on the suggestion in rust-lang#86918 (comment)

> Some advantages:
>
>     * Synergy with the existing `as_ref` and `as_mut` signatures (stable since Rust 1.33)
>
>     * Lifetime elision reduces noise in the signature
>
>     * Turbofish less verbose: `Pin::<&mut T>::as_deref_mut` vs `Pin::<&mut Pin<&mut T>>::as_deref_mut`

The comment seemed to imply that `Pin::as_ref` and `Pin::as_mut` already share an impl block, which they don't. So, I rearranged it so that they do, and we can see which looks better in the docs.

<details><summary><b>Docs screenshots</b></summary>

Current nightly:
![image](https://github.com/user-attachments/assets/b432cb82-8f4b-48ae-bafc-2fe49d0ad48c)

`Pin::as_deref_mut` moved into the same block as `as_mut`:
![image](https://github.com/user-attachments/assets/f9b35722-6a88-4465-ad1c-28d8e91902ac)

`Pin::as_ref`, `as_mut`, and `as_deref_mut` all in the same block:
![image](https://github.com/user-attachments/assets/9a1b2bf0-70a6-4751-b13f-390f1d575244)

</details>

I think I like the last one the most; obviously I'm biased since I'm the one who rearranged it, but it doesn't make sense to me to have `as_ref` methods split up by an `into_inner` method.

r? dtolnay
  • Loading branch information
@workingjubilee
workingjubilee authored Aug 24, 2024
2 parents 25a3c88 + c65ef3d commit 81707e5
Showing 1 changed file with 73 additions and 70 deletions.
143 changes: 73 additions & 70 deletions library/core/src/pin.rs
View file
Original file line number Diff line number Diff line change
Expand Up @@ -1291,8 +1291,8 @@ impl<Ptr: Deref> Pin<Ptr> {
/// // Now, if `x` was the only reference, we have a mutable reference to
/// // data that we pinned above, which we could use to move it as we have
/// // seen in the previous example. We have violated the pinning API contract.
/// }
/// ```
/// }
/// ```
///
/// ## Pinning of closure captures
///
Expand Down Expand Up @@ -1370,33 +1370,6 @@ impl<Ptr: Deref> Pin<Ptr> {
unsafe { Pin::new_unchecked(&*self.__pointer) }
}

/// Unwraps this `Pin<Ptr>`, returning the underlying `Ptr`.
///
/// # Safety
///
/// This function is unsafe. You must guarantee that you will continue to
/// treat the pointer `Ptr` as pinned after you call this function, so that
/// the invariants on the `Pin` type can be upheld. If the code using the
/// resulting `Ptr` does not continue to maintain the pinning invariants that
/// is a violation of the API contract and may lead to undefined behavior in
/// later (safe) operations.
///
/// Note that you must be able to guarantee that the data pointed to by `Ptr`
/// will be treated as pinned all the way until its `drop` handler is complete!
///
/// *For more information, see the [`pin` module docs][self]*
///
/// If the underlying data is [`Unpin`], [`Pin::into_inner`] should be used
/// instead.
#[inline(always)]
#[rustc_const_unstable(feature = "const_pin", issue = "76654")]
#[stable(feature = "pin_into_inner", since = "1.39.0")]
pub const unsafe fn into_inner_unchecked(pin: Pin<Ptr>) -> Ptr {
pin.__pointer
}
}

impl<Ptr: DerefMut> Pin<Ptr> {
/// Gets a mutable reference to the pinned value this `Pin<Ptr>` points to.
///
/// This is a generic method to go from `&mut Pin<Pointer<T>>` to `Pin<&mut T>`.
Expand Down Expand Up @@ -1428,11 +1401,55 @@ impl<Ptr: DerefMut> Pin<Ptr> {
/// ```
#[stable(feature = "pin", since = "1.33.0")]
#[inline(always)]
pub fn as_mut(&mut self) -> Pin<&mut Ptr::Target> {
pub fn as_mut(&mut self) -> Pin<&mut Ptr::Target>
where
Ptr: DerefMut,
{
// SAFETY: see documentation on this function
unsafe { Pin::new_unchecked(&mut *self.__pointer) }
}

/// Gets `Pin<&mut T>` to the underlying pinned value from this nested `Pin`-pointer.
///
/// This is a generic method to go from `Pin<&mut Pin<Pointer<T>>>` to `Pin<&mut T>`. It is
/// safe because the existence of a `Pin<Pointer<T>>` ensures that the pointee, `T`, cannot
/// move in the future, and this method does not enable the pointee to move. "Malicious"
/// implementations of `Ptr::DerefMut` are likewise ruled out by the contract of
/// `Pin::new_unchecked`.
#[unstable(feature = "pin_deref_mut", issue = "86918")]
#[must_use = "`self` will be dropped if the result is not used"]
#[inline(always)]
pub fn as_deref_mut(self: Pin<&mut Pin<Ptr>>) -> Pin<&mut Ptr::Target>
where
Ptr: DerefMut,
{
// SAFETY: What we're asserting here is that going from
//
// Pin<&mut Pin<Ptr>>
//
// to
//
// Pin<&mut Ptr::Target>
//
// is safe.
//
// We need to ensure that two things hold for that to be the case:
//
// 1) Once we give out a `Pin<&mut Ptr::Target>`, a `&mut Ptr::Target` will not be given out.
// 2) By giving out a `Pin<&mut Ptr::Target>`, we do not risk violating
// `Pin<&mut Pin<Ptr>>`
//
// The existence of `Pin<Ptr>` is sufficient to guarantee #1: since we already have a
// `Pin<Ptr>`, it must already uphold the pinning guarantees, which must mean that
// `Pin<&mut Ptr::Target>` does as well, since `Pin::as_mut` is safe. We do not have to rely
// on the fact that `Ptr` is _also_ pinned.
//
// For #2, we need to ensure that code given a `Pin<&mut Ptr::Target>` cannot cause the
// `Pin<Ptr>` to move? That is not possible, since `Pin<&mut Ptr::Target>` no longer retains
// any access to the `Ptr` itself, much less the `Pin<Ptr>`.
unsafe { self.get_unchecked_mut() }.as_mut()
}

/// Assigns a new value to the memory location pointed to by the `Pin<Ptr>`.
///
/// This overwrites pinned data, but that is okay: the original pinned value's destructor gets
Expand All @@ -1457,10 +1474,36 @@ impl<Ptr: DerefMut> Pin<Ptr> {
#[inline(always)]
pub fn set(&mut self, value: Ptr::Target)
where
Ptr: DerefMut,
Ptr::Target: Sized,
{
*(self.__pointer) = value;
}

/// Unwraps this `Pin<Ptr>`, returning the underlying `Ptr`.
///
/// # Safety
///
/// This function is unsafe. You must guarantee that you will continue to
/// treat the pointer `Ptr` as pinned after you call this function, so that
/// the invariants on the `Pin` type can be upheld. If the code using the
/// resulting `Ptr` does not continue to maintain the pinning invariants that
/// is a violation of the API contract and may lead to undefined behavior in
/// later (safe) operations.
///
/// Note that you must be able to guarantee that the data pointed to by `Ptr`
/// will be treated as pinned all the way until its `drop` handler is complete!
///
/// *For more information, see the [`pin` module docs][self]*
///
/// If the underlying data is [`Unpin`], [`Pin::into_inner`] should be used
/// instead.
#[inline(always)]
#[rustc_const_unstable(feature = "const_pin", issue = "76654")]
#[stable(feature = "pin_into_inner", since = "1.39.0")]
pub const unsafe fn into_inner_unchecked(pin: Pin<Ptr>) -> Ptr {
pin.__pointer
}
}

impl<'a, T: ?Sized> Pin<&'a T> {
Expand Down Expand Up @@ -1613,46 +1656,6 @@ impl<T: ?Sized> Pin<&'static T> {
}
}

impl<'a, Ptr: DerefMut> Pin<&'a mut Pin<Ptr>> {
/// Gets `Pin<&mut T>` to the underlying pinned value from this nested `Pin`-pointer.
///
/// This is a generic method to go from `Pin<&mut Pin<Pointer<T>>>` to `Pin<&mut T>`. It is
/// safe because the existence of a `Pin<Pointer<T>>` ensures that the pointee, `T`, cannot
/// move in the future, and this method does not enable the pointee to move. "Malicious"
/// implementations of `Ptr::DerefMut` are likewise ruled out by the contract of
/// `Pin::new_unchecked`.
#[unstable(feature = "pin_deref_mut", issue = "86918")]
#[must_use = "`self` will be dropped if the result is not used"]
#[inline(always)]
pub fn as_deref_mut(self) -> Pin<&'a mut Ptr::Target> {
// SAFETY: What we're asserting here is that going from
//
// Pin<&mut Pin<Ptr>>
//
// to
//
// Pin<&mut Ptr::Target>
//
// is safe.
//
// We need to ensure that two things hold for that to be the case:
//
// 1) Once we give out a `Pin<&mut Ptr::Target>`, a `&mut Ptr::Target` will not be given out.
// 2) By giving out a `Pin<&mut Ptr::Target>`, we do not risk violating
// `Pin<&mut Pin<Ptr>>`
//
// The existence of `Pin<Ptr>` is sufficient to guarantee #1: since we already have a
// `Pin<Ptr>`, it must already uphold the pinning guarantees, which must mean that
// `Pin<&mut Ptr::Target>` does as well, since `Pin::as_mut` is safe. We do not have to rely
// on the fact that `Ptr` is _also_ pinned.
//
// For #2, we need to ensure that code given a `Pin<&mut Ptr::Target>` cannot cause the
// `Pin<Ptr>` to move? That is not possible, since `Pin<&mut Ptr::Target>` no longer retains
// any access to the `Ptr` itself, much less the `Pin<Ptr>`.
unsafe { self.get_unchecked_mut() }.as_mut()
}
}

impl<T: ?Sized> Pin<&'static mut T> {
/// Gets a pinning mutable reference from a static mutable reference.
///
Expand Down

0 comments on commit 81707e5

Please sign in to comment.