Auto merge of #74160 - CAD97:weak-as-unsized-ptr, r=RalfJung

Allow Weak::as_ptr and friends for unsized T

Relaxes `impl<T> Weak<T>` to `impl<T: ?Sized> Weak<T>` for the methods `rc::Weak::as_ptr`, `into_raw`, and `from_raw`.

Follow-up to #73845, which did most of the impl work to make these functions work for `T: ?Sized`.

We still have to adjust the implementation of `Weak::from_raw` here, however, because I missed a use of `ptr.is_null()` previously. This check was necessary when `into`/`from_raw` were first implemented, as `into_raw` returned `ptr::null()` for dangling weak. However, we now just (wrapping) offset dangling weaks' pointers the same as nondangling weak, so the null check is no longer necessary (or even hit). (I can submit just 17a928f as a separate PR if desired.)

As a nice side effect, moves the `fn is_dangling` definition closer to `Weak::new`, which creates the dangling weak.

This technically stabilizes that "something like `align_of_val_raw`" is possible to do. However, I believe the part of the functionality required by these methods here -- specifically, getting the alignment of a pointee from a pointer where it may be dangling iff the pointee is `Sized` -- is uncontroversial enough to stabilize these methods without a way to implement them on stable Rust.

r? `@RalfJung,` who reviewed #73845.

ATTN: This changes (relaxes) the (input) generic bounds on stable fn!

Author: bors

library/alloc/src/rc.rs

@@ -1721,7 +1721,21 @@ impl<T> Weak<T> {
     pub fn new() -> Weak<T> {
         Weak { ptr: NonNull::new(usize::MAX as *mut RcBox<T>).expect("MAX is not 0") }
     }
+}
 
+pub(crate) fn is_dangling<T: ?Sized>(ptr: NonNull<T>) -> bool {
+    let address = ptr.as_ptr() as *mut () as usize;
+    address == usize::MAX
+}
+
+/// Helper type to allow accessing the reference counts without
+/// making any assertions about the data field.
+struct WeakInner<'a> {
+    weak: &'a Cell<usize>,
+    strong: &'a Cell<usize>,
+}
+
+impl<T: ?Sized> Weak<T> {
     /// Returns a raw pointer to the object `T` pointed to by this `Weak<T>`.
     ///
     /// The pointer is valid only if there are some strong references. The pointer may be dangling,
@@ -1841,33 +1855,20 @@ impl<T> Weak<T> {
     /// [`new`]: Weak::new
     #[stable(feature = "weak_into_raw", since = "1.45.0")]
     pub unsafe fn from_raw(ptr: *const T) -> Self {
-        if ptr.is_null() {
-            Self::new()
-        } else {
-            // See Rc::from_raw for details
-            unsafe {
-                let offset = data_offset(ptr);
-                let fake_ptr = ptr as *mut RcBox<T>;
-                let ptr = set_data_ptr(fake_ptr, (ptr as *mut u8).offset(-offset));
-                Weak { ptr: NonNull::new(ptr).expect("Invalid pointer passed to from_raw") }
-            }
-        }
-    }
-}
+        // SAFETY: data_offset is safe to call, because this pointer originates from a Weak.
+        // See Weak::as_ptr for context on how the input pointer is derived.
+        let offset = unsafe { data_offset(ptr) };
 
-pub(crate) fn is_dangling<T: ?Sized>(ptr: NonNull<T>) -> bool {
-    let address = ptr.as_ptr() as *mut () as usize;
-    address == usize::MAX
-}
+        // Reverse the offset to find the original RcBox.
+        // SAFETY: we use wrapping_offset here because the pointer may be dangling (but only if T: Sized).
+        let ptr = unsafe {
+            set_data_ptr(ptr as *mut RcBox<T>, (ptr as *mut u8).wrapping_offset(-offset))
+        };
 
-/// Helper type to allow accessing the reference counts without
-/// making any assertions about the data field.
-struct WeakInner<'a> {
-    weak: &'a Cell<usize>,
-    strong: &'a Cell<usize>,
-}
+        // SAFETY: we now have recovered the original Weak pointer, so can create the Weak.
+        Weak { ptr: unsafe { NonNull::new_unchecked(ptr) } }
+    }
 
-impl<T: ?Sized> Weak<T> {
     /// Attempts to upgrade the `Weak` pointer to an [`Rc`], delaying
     /// dropping of the inner value if successful.
     ///

library/alloc/src/rc/tests.rs

@@ -190,6 +190,48 @@ fn test_into_from_raw_unsized() {
     assert_eq!(rc2.to_string(), "123");
 }
 
+#[test]
+fn into_from_weak_raw() {
+    let x = Rc::new(box "hello");
+    let y = Rc::downgrade(&x);
+
+    let y_ptr = Weak::into_raw(y);
+    unsafe {
+        assert_eq!(**y_ptr, "hello");
+
+        let y = Weak::from_raw(y_ptr);
+        let y_up = Weak::upgrade(&y).unwrap();
+        assert_eq!(**y_up, "hello");
+        drop(y_up);
+
+        assert_eq!(Rc::try_unwrap(x).map(|x| *x), Ok("hello"));
+    }
+}
+
+#[test]
+fn test_into_from_weak_raw_unsized() {
+    use std::fmt::Display;
+    use std::string::ToString;
+
+    let arc: Rc<str> = Rc::from("foo");
+    let weak: Weak<str> = Rc::downgrade(&arc);
+
+    let ptr = Weak::into_raw(weak.clone());
+    let weak2 = unsafe { Weak::from_raw(ptr) };
+
+    assert_eq!(unsafe { &*ptr }, "foo");
+    assert!(weak.ptr_eq(&weak2));
+
+    let arc: Rc<dyn Display> = Rc::new(123);
+    let weak: Weak<dyn Display> = Rc::downgrade(&arc);
+
+    let ptr = Weak::into_raw(weak.clone());
+    let weak2 = unsafe { Weak::from_raw(ptr) };
+
+    assert_eq!(unsafe { &*ptr }.to_string(), "123");
+    assert!(weak.ptr_eq(&weak2));
+}
+
 #[test]
 fn get_mut() {
     let mut x = Rc::new(3);

library/alloc/src/sync.rs

@@ -1509,7 +1509,16 @@ impl<T> Weak<T> {
     pub fn new() -> Weak<T> {
         Weak { ptr: NonNull::new(usize::MAX as *mut ArcInner<T>).expect("MAX is not 0") }
     }
+}
+
+/// Helper type to allow accessing the reference counts without
+/// making any assertions about the data field.
+struct WeakInner<'a> {
+    weak: &'a atomic::AtomicUsize,
+    strong: &'a atomic::AtomicUsize,
+}
 
+impl<T: ?Sized> Weak<T> {
     /// Returns a raw pointer to the object `T` pointed to by this `Weak<T>`.
     ///
     /// The pointer is valid only if there are some strong references. The pointer may be dangling,
@@ -1629,28 +1638,20 @@ impl<T> Weak<T> {
     /// [`forget`]: std::mem::forget
     #[stable(feature = "weak_into_raw", since = "1.45.0")]
     pub unsafe fn from_raw(ptr: *const T) -> Self {
-        if ptr.is_null() {
-            Self::new()
-        } else {
-            // See Arc::from_raw for details
-            unsafe {
-                let offset = data_offset(ptr);
-                let fake_ptr = ptr as *mut ArcInner<T>;
-                let ptr = set_data_ptr(fake_ptr, (ptr as *mut u8).offset(-offset));
-                Weak { ptr: NonNull::new(ptr).expect("Invalid pointer passed to from_raw") }
-            }
-        }
-    }
-}
+        // SAFETY: data_offset is safe to call, because this pointer originates from a Weak.
+        // See Weak::as_ptr for context on how the input pointer is derived.
+        let offset = unsafe { data_offset(ptr) };
+
+        // Reverse the offset to find the original ArcInner.
+        // SAFETY: we use wrapping_offset here because the pointer may be dangling (but only if T: Sized)
+        let ptr = unsafe {
+            set_data_ptr(ptr as *mut ArcInner<T>, (ptr as *mut u8).wrapping_offset(-offset))
+        };
 
-/// Helper type to allow accessing the reference counts without
-/// making any assertions about the data field.
-struct WeakInner<'a> {
-    weak: &'a atomic::AtomicUsize,
-    strong: &'a atomic::AtomicUsize,
-}
+        // SAFETY: we now have recovered the original Weak pointer, so can create the Weak.
+        unsafe { Weak { ptr: NonNull::new_unchecked(ptr) } }
+    }
 
-impl<T: ?Sized> Weak<T> {
     /// Attempts to upgrade the `Weak` pointer to an [`Arc`], delaying
     /// dropping of the inner value if successful.
     ///

library/alloc/src/sync/tests.rs

@@ -140,6 +140,48 @@ fn test_into_from_raw_unsized() {
     assert_eq!(arc2.to_string(), "123");
 }
 
+#[test]
+fn into_from_weak_raw() {
+    let x = Arc::new(box "hello");
+    let y = Arc::downgrade(&x);
+
+    let y_ptr = Weak::into_raw(y);
+    unsafe {
+        assert_eq!(**y_ptr, "hello");
+
+        let y = Weak::from_raw(y_ptr);
+        let y_up = Weak::upgrade(&y).unwrap();
+        assert_eq!(**y_up, "hello");
+        drop(y_up);
+
+        assert_eq!(Arc::try_unwrap(x).map(|x| *x), Ok("hello"));
+    }
+}
+
+#[test]
+fn test_into_from_weak_raw_unsized() {
+    use std::fmt::Display;
+    use std::string::ToString;
+
+    let arc: Arc<str> = Arc::from("foo");
+    let weak: Weak<str> = Arc::downgrade(&arc);
+
+    let ptr = Weak::into_raw(weak.clone());
+    let weak2 = unsafe { Weak::from_raw(ptr) };
+
+    assert_eq!(unsafe { &*ptr }, "foo");
+    assert!(weak.ptr_eq(&weak2));
+
+    let arc: Arc<dyn Display> = Arc::new(123);
+    let weak: Weak<dyn Display> = Arc::downgrade(&arc);
+
+    let ptr = Weak::into_raw(weak.clone());
+    let weak2 = unsafe { Weak::from_raw(ptr) };
+
+    assert_eq!(unsafe { &*ptr }.to_string(), "123");
+    assert!(weak.ptr_eq(&weak2));
+}
+
 #[test]
 fn test_cowarc_clone_make_mut() {
     let mut cow0 = Arc::new(75);