Remove Pointer::with_ref in favour implementing it on tagged pointers directly

Author: WaffleLapkin

compiler/rustc_data_structures/src/tagged_ptr.rs

@@ -13,7 +13,6 @@
 //! The tag must implement the `Tag` trait. We assert that the tag and `Pointer`
 //! are compatible at compile time.
 
-use std::mem::ManuallyDrop;
 use std::ops::Deref;
 use std::ptr::NonNull;
 use std::rc::Rc;
@@ -81,16 +80,6 @@ pub unsafe trait Pointer: Deref {
     /// This acts as `ptr::read` semantically, it should not be called more than
     /// once on non-`Copy` `Pointer`s.
     unsafe fn from_ptr(ptr: NonNull<Self::Target>) -> Self;
-
-    /// This provides a reference to the `Pointer` itself, rather than the
-    /// `Deref::Target`. It is used for cases where we want to call methods that
-    /// may be implement differently for the Pointer than the Pointee (e.g.,
-    /// `Rc::clone` vs cloning the inner value).
-    ///
-    /// # Safety
-    ///
-    /// The passed `ptr` must be returned from `into_usize`.
-    unsafe fn with_ref<R, F: FnOnce(&Self) -> R>(ptr: NonNull<Self::Target>, f: F) -> R;
 }
 
 /// This describes tags that the `TaggedPtr` struct can hold.
@@ -124,11 +113,6 @@ unsafe impl<T: ?Sized + Aligned> Pointer for Box<T> {
     unsafe fn from_ptr(ptr: NonNull<T>) -> Self {
         Box::from_raw(ptr.as_ptr())
     }
-
-    unsafe fn with_ref<R, F: FnOnce(&Self) -> R>(ptr: NonNull<T>, f: F) -> R {
-        let raw = ManuallyDrop::new(Self::from_ptr(ptr));
-        f(&raw)
-    }
 }
 
 unsafe impl<T: ?Sized + Aligned> Pointer for Rc<T> {
@@ -143,11 +127,6 @@ unsafe impl<T: ?Sized + Aligned> Pointer for Rc<T> {
     unsafe fn from_ptr(ptr: NonNull<T>) -> Self {
         Rc::from_raw(ptr.as_ptr())
     }
-
-    unsafe fn with_ref<R, F: FnOnce(&Self) -> R>(ptr: NonNull<T>, f: F) -> R {
-        let raw = ManuallyDrop::new(Self::from_ptr(ptr));
-        f(&raw)
-    }
 }
 
 unsafe impl<T: ?Sized + Aligned> Pointer for Arc<T> {
@@ -162,11 +141,6 @@ unsafe impl<T: ?Sized + Aligned> Pointer for Arc<T> {
     unsafe fn from_ptr(ptr: NonNull<T>) -> Self {
         Arc::from_raw(ptr.as_ptr())
     }
-
-    unsafe fn with_ref<R, F: FnOnce(&Self) -> R>(ptr: NonNull<T>, f: F) -> R {
-        let raw = ManuallyDrop::new(Self::from_ptr(ptr));
-        f(&raw)
-    }
 }
 
 unsafe impl<'a, T: 'a + ?Sized + Aligned> Pointer for &'a T {
@@ -181,10 +155,6 @@ unsafe impl<'a, T: 'a + ?Sized + Aligned> Pointer for &'a T {
     unsafe fn from_ptr(ptr: NonNull<T>) -> Self {
         ptr.as_ref()
     }
-
-    unsafe fn with_ref<R, F: FnOnce(&Self) -> R>(ptr: NonNull<T>, f: F) -> R {
-        f(&ptr.as_ref())
-    }
 }
 
 unsafe impl<'a, T: 'a + ?Sized + Aligned> Pointer for &'a mut T {
@@ -199,10 +169,6 @@ unsafe impl<'a, T: 'a + ?Sized + Aligned> Pointer for &'a mut T {
     unsafe fn from_ptr(mut ptr: NonNull<T>) -> Self {
         ptr.as_mut()
     }
-
-    unsafe fn with_ref<R, F: FnOnce(&Self) -> R>(mut ptr: NonNull<T>, f: F) -> R {
-        f(&ptr.as_mut())
-    }
 }
 
 /// Returns the number of bits available for use for tags in a pointer to `T`

compiler/rustc_data_structures/src/tagged_ptr/copy.rs

@@ -2,6 +2,7 @@ use super::{Pointer, Tag};
 use crate::stable_hasher::{HashStable, StableHasher};
 use std::fmt;
 use std::marker::PhantomData;
+use std::mem::ManuallyDrop;
 use std::num::NonZeroUsize;
 use std::ops::{Deref, DerefMut};
 use std::ptr::NonNull;
@@ -85,6 +86,24 @@ where
         self.packed.map_addr(|addr| unsafe { NonZeroUsize::new_unchecked(addr.get() << T::BITS) })
     }
 
+    /// This provides a reference to the `P` pointer itself, rather than the
+    /// `Deref::Target`. It is used for cases where we want to call methods
+    /// that may be implement differently for the Pointer than the Pointee
+    /// (e.g., `Rc::clone` vs cloning the inner value).
+    pub(super) fn with_pointer_ref<R>(&self, f: impl FnOnce(&P) -> R) -> R {
+        // Safety:
+        // - `self.raw.pointer_raw()` is originally returned from `P::into_ptr`
+        //   and as such is valid for `P::from_ptr`.
+        //   - This also allows us to not care whatever `f` panics or not.
+        // - Even though we create a copy of the pointer, we store it inside
+        //   `ManuallyDrop` and only access it by-ref, so we don't double-drop.
+        //
+        // Semantically this is just `f(&self.pointer)` (where `self.pointer`
+        // is non-packed original pointer).
+        let ptr = unsafe { ManuallyDrop::new(P::from_ptr(self.pointer_raw())) };
+        f(&ptr)
+    }
+
     pub fn pointer(self) -> P
     where
         P: Copy,
@@ -189,9 +208,7 @@ where
     T: Tag + HashStable<HCX>,
 {
     fn hash_stable(&self, hcx: &mut HCX, hasher: &mut StableHasher) {
-        unsafe {
-            Pointer::with_ref(self.pointer_raw(), |p: &P| p.hash_stable(hcx, hasher));
-        }
+        self.with_pointer_ref(|ptr| ptr.hash_stable(hcx, hasher));
         self.tag().hash_stable(hcx, hasher);
     }
 }

compiler/rustc_data_structures/src/tagged_ptr/drop.rs

@@ -1,8 +1,8 @@
-use super::{Pointer, Tag};
-use crate::stable_hasher::{HashStable, StableHasher};
 use std::fmt;
 
 use super::CopyTaggedPtr;
+use super::{Pointer, Tag};
+use crate::stable_hasher::{HashStable, StableHasher};
 
 /// A TaggedPtr implementing `Drop`.
 ///
@@ -23,7 +23,9 @@ where
     T: Tag,
 {
     fn clone(&self) -> Self {
-        unsafe { Self::new(P::with_ref(self.raw.pointer_raw(), |p| p.clone()), self.raw.tag()) }
+        let ptr = self.raw.with_pointer_ref(P::clone);
+
+        Self::new(ptr, self.tag())
     }
 }