From 5627b9d0e98a7d033898968abdba3f57565e002c Mon Sep 17 00:00:00 2001 From: Jeremy Banks <_@jeremy.ca> Date: Thu, 11 Aug 2022 21:37:06 +0000 Subject: [PATCH 1/7] Mark `Rc`, `Arc`, and their `Weak`s as `#[repr(transparent)]`. --- library/alloc/src/rc.rs | 2 ++ library/alloc/src/sync.rs | 2 ++ 2 files changed, 4 insertions(+) diff --git a/library/alloc/src/rc.rs b/library/alloc/src/rc.rs index b89b03683baef..4fa9309ba1733 100644 --- a/library/alloc/src/rc.rs +++ b/library/alloc/src/rc.rs @@ -306,6 +306,7 @@ struct RcBox { #[cfg_attr(not(test), rustc_diagnostic_item = "Rc")] #[stable(feature = "rust1", since = "1.0.0")] #[rustc_insignificant_dtor] +#[repr(transparent)] pub struct Rc { ptr: NonNull>, phantom: PhantomData>, @@ -2143,6 +2144,7 @@ impl> ToRcSlice for I { /// /// [`upgrade`]: Weak::upgrade #[stable(feature = "rc_weak", since = "1.4.0")] +#[repr(transparent)] pub struct Weak { // This is a `NonNull` to allow optimizing the size of this type in enums, // but it is not necessarily a valid pointer. diff --git a/library/alloc/src/sync.rs b/library/alloc/src/sync.rs index 4c03cc3ed158a..b504f1c8c20dd 100644 --- a/library/alloc/src/sync.rs +++ b/library/alloc/src/sync.rs @@ -232,6 +232,7 @@ macro_rules! acquire { /// [rc_examples]: crate::rc#examples #[cfg_attr(not(test), rustc_diagnostic_item = "Arc")] #[stable(feature = "rust1", since = "1.0.0")] +#[repr(transparent)] pub struct Arc { ptr: NonNull>, phantom: PhantomData>, @@ -282,6 +283,7 @@ impl Arc { /// /// [`upgrade`]: Weak::upgrade #[stable(feature = "arc_weak", since = "1.4.0")] +#[repr(transparent)] pub struct Weak { // This is a `NonNull` to allow optimizing the size of this type in enums, // but it is not necessarily a valid pointer. From be2d57c312da36d6ff57b7fd858a2367f838a79d Mon Sep 17 00:00:00 2001 From: Jeremy Banks <_@jeremy.ca> Date: Thu, 11 Aug 2022 22:04:55 +0000 Subject: [PATCH 2/7] Add `Arc::as_weak` and `Rc::as_weak` behind new `#![feature("rc_as_weak")]`. --- library/alloc/src/rc.rs | 22 ++++++++++++++++++++++ library/alloc/src/sync.rs | 22 ++++++++++++++++++++++ 2 files changed, 44 insertions(+) diff --git a/library/alloc/src/rc.rs b/library/alloc/src/rc.rs index 4fa9309ba1733..56db31c08a4c4 100644 --- a/library/alloc/src/rc.rs +++ b/library/alloc/src/rc.rs @@ -934,6 +934,28 @@ impl Rc { Weak { ptr: this.ptr } } + /// Convert a reference to an [`Rc`] into a reference to a [`Weak`] of the same type. + /// + /// This is a type-only operation; it doesn't modify the inner reference counts. + /// + /// # Examples + /// + /// ``` + /// #![feature(rc_as_weak)] + /// + /// use std::rc::{Rc, Weak}; + /// + /// let five: &Rc = &Rc::new(5); + /// + /// let weak_five: &Weak = Rc::as_weak(five); + /// ``` + #[inline] + #[unstable(feature = "rc_as_weak", issue = "none")] + #[must_use] + pub const fn as_weak<'a>(this: &'a Self) -> &'a Weak { + unsafe { mem::transmute::<&'a Rc, &'a Weak>(this) } + } + /// Gets the number of [`Weak`] pointers to this allocation. /// /// # Examples diff --git a/library/alloc/src/sync.rs b/library/alloc/src/sync.rs index b504f1c8c20dd..dc145195c4826 100644 --- a/library/alloc/src/sync.rs +++ b/library/alloc/src/sync.rs @@ -961,6 +961,28 @@ impl Arc { } } + /// Convert a reference to an [`Arc`] into a reference to a [`Weak`] of the same type. + /// + /// This is a type-only operation; it doesn't modify the inner reference counts. + /// + /// # Examples + /// + /// ``` + /// #![feature(rc_as_weak)] + /// + /// use std::sync::{Arc, Weak}; + /// + /// let five: &Arc = &Arc::new(5); + /// + /// let weak_five: &Weak = Arc::as_weak(five); + /// ``` + #[inline] + #[unstable(feature = "rc_as_weak", issue = "none")] + #[must_use] + pub const fn as_weak<'a>(this: &'a Self) -> &'a Weak { + unsafe { mem::transmute::<&'a Arc, &'a Weak>(this) } + } + /// Gets the number of [`Weak`] pointers to this allocation. /// /// # Safety From 668c57a8000b2850bdec3da5bff21cb23fcec570 Mon Sep 17 00:00:00 2001 From: Jeremy Banks <_@jeremy.ca> Date: Sat, 13 Aug 2022 02:06:58 +0000 Subject: [PATCH 3/7] add issue number for #![feature(rc_as_weak)] --- library/alloc/src/rc.rs | 2 +- library/alloc/src/sync.rs | 2 +- 2 files changed, 2 insertions(+), 2 deletions(-) diff --git a/library/alloc/src/rc.rs b/library/alloc/src/rc.rs index 56db31c08a4c4..1e461abe537b1 100644 --- a/library/alloc/src/rc.rs +++ b/library/alloc/src/rc.rs @@ -950,7 +950,7 @@ impl Rc { /// let weak_five: &Weak = Rc::as_weak(five); /// ``` #[inline] - #[unstable(feature = "rc_as_weak", issue = "none")] + #[unstable(feature = "rc_as_weak", issue = "100472")] #[must_use] pub const fn as_weak<'a>(this: &'a Self) -> &'a Weak { unsafe { mem::transmute::<&'a Rc, &'a Weak>(this) } diff --git a/library/alloc/src/sync.rs b/library/alloc/src/sync.rs index dc145195c4826..d5408335fa255 100644 --- a/library/alloc/src/sync.rs +++ b/library/alloc/src/sync.rs @@ -977,7 +977,7 @@ impl Arc { /// let weak_five: &Weak = Arc::as_weak(five); /// ``` #[inline] - #[unstable(feature = "rc_as_weak", issue = "none")] + #[unstable(feature = "rc_as_weak", issue = "100472")] #[must_use] pub const fn as_weak<'a>(this: &'a Self) -> &'a Weak { unsafe { mem::transmute::<&'a Arc, &'a Weak>(this) } From 2885987d45ea8fa37644cf2b7021df7e709b804f Mon Sep 17 00:00:00 2001 From: Jeremy Banks <_@jeremy.ca> Date: Sat, 13 Aug 2022 02:35:51 +0000 Subject: [PATCH 4/7] Add test assertions that as_weak does not modify reference counts, and points to the right address. --- library/alloc/src/rc/tests.rs | 4 ++++ library/alloc/src/sync/tests.rs | 4 ++++ 2 files changed, 8 insertions(+) diff --git a/library/alloc/src/rc/tests.rs b/library/alloc/src/rc/tests.rs index 32433cfbdcff6..4ba6444109e9b 100644 --- a/library/alloc/src/rc/tests.rs +++ b/library/alloc/src/rc/tests.rs @@ -103,6 +103,10 @@ fn test_weak_count() { let w = Rc::downgrade(&a); assert!(Rc::strong_count(&a) == 1); assert!(Rc::weak_count(&a) == 1); + let r = Rc::as_weak(&a); + assert!(Rc::strong_count(&a) == 1); + assert!(Rc::weak_count(&a) == 1); + assert!(r.as_ptr() == Rc::as_ptr(&a)); drop(w); assert!(Rc::strong_count(&a) == 1); assert!(Rc::weak_count(&a) == 0); diff --git a/library/alloc/src/sync/tests.rs b/library/alloc/src/sync/tests.rs index 202d0e7f02057..9188ce165e7b9 100644 --- a/library/alloc/src/sync/tests.rs +++ b/library/alloc/src/sync/tests.rs @@ -314,6 +314,10 @@ fn test_weak_count() { let w = Arc::downgrade(&a); assert!(Arc::strong_count(&a) == 1); assert!(Arc::weak_count(&a) == 1); + let r = Arc::as_weak(&a); + assert!(Arc::strong_count(&a) == 1); + assert!(Arc::weak_count(&a) == 1); + assert!(r.as_ptr() == Arc::as_ptr(&a)); let x = w.clone(); assert!(Arc::weak_count(&a) == 2); drop(w); From b369af878f6c328022fe325cc593bda84509882c Mon Sep 17 00:00:00 2001 From: Jeremy Banks <_@jeremy.ca> Date: Mon, 15 Aug 2022 19:26:19 +0000 Subject: [PATCH 5/7] add type annotations to test for Arc::as_weak --- library/alloc/src/rc/tests.rs | 2 +- library/alloc/src/sync/tests.rs | 2 +- 2 files changed, 2 insertions(+), 2 deletions(-) diff --git a/library/alloc/src/rc/tests.rs b/library/alloc/src/rc/tests.rs index 4ba6444109e9b..dbec1b3d30f8a 100644 --- a/library/alloc/src/rc/tests.rs +++ b/library/alloc/src/rc/tests.rs @@ -103,7 +103,7 @@ fn test_weak_count() { let w = Rc::downgrade(&a); assert!(Rc::strong_count(&a) == 1); assert!(Rc::weak_count(&a) == 1); - let r = Rc::as_weak(&a); + let r: &Weak = Rc::as_weak(&a); assert!(Rc::strong_count(&a) == 1); assert!(Rc::weak_count(&a) == 1); assert!(r.as_ptr() == Rc::as_ptr(&a)); diff --git a/library/alloc/src/sync/tests.rs b/library/alloc/src/sync/tests.rs index 9188ce165e7b9..4eca3bb9f15ee 100644 --- a/library/alloc/src/sync/tests.rs +++ b/library/alloc/src/sync/tests.rs @@ -314,7 +314,7 @@ fn test_weak_count() { let w = Arc::downgrade(&a); assert!(Arc::strong_count(&a) == 1); assert!(Arc::weak_count(&a) == 1); - let r = Arc::as_weak(&a); + let r: &Weak = Arc::as_weak(&a); assert!(Arc::strong_count(&a) == 1); assert!(Arc::weak_count(&a) == 1); assert!(r.as_ptr() == Arc::as_ptr(&a)); From 876236ca796b4f19e8541bd88b62a7140b1e9d01 Mon Sep 17 00:00:00 2001 From: Jeremy Banks <_@jeremy.ca> Date: Mon, 15 Aug 2022 19:47:53 +0000 Subject: [PATCH 6/7] Replace use of `transmute` with pointer casts in `Arc`/`Rc::as_weak()`. per https://github.com/rust-lang/rust/pull/100472#discussion_r945222720 --- library/alloc/src/rc.rs | 5 +++-- library/alloc/src/sync.rs | 5 +++-- 2 files changed, 6 insertions(+), 4 deletions(-) diff --git a/library/alloc/src/rc.rs b/library/alloc/src/rc.rs index 1e461abe537b1..4f128f348e57c 100644 --- a/library/alloc/src/rc.rs +++ b/library/alloc/src/rc.rs @@ -952,8 +952,9 @@ impl Rc { #[inline] #[unstable(feature = "rc_as_weak", issue = "100472")] #[must_use] - pub const fn as_weak<'a>(this: &'a Self) -> &'a Weak { - unsafe { mem::transmute::<&'a Rc, &'a Weak>(this) } + pub const fn as_weak(this: &Self) -> &Weak { + let weak = this as *const Self as *const Weak; + unsafe { &*weak } } /// Gets the number of [`Weak`] pointers to this allocation. diff --git a/library/alloc/src/sync.rs b/library/alloc/src/sync.rs index d5408335fa255..a0f89344fee47 100644 --- a/library/alloc/src/sync.rs +++ b/library/alloc/src/sync.rs @@ -979,8 +979,9 @@ impl Arc { #[inline] #[unstable(feature = "rc_as_weak", issue = "100472")] #[must_use] - pub const fn as_weak<'a>(this: &'a Self) -> &'a Weak { - unsafe { mem::transmute::<&'a Arc, &'a Weak>(this) } + pub const fn as_weak(this: &Self) -> &Weak { + let weak = this as *const Self as *const Weak; + unsafe { &*weak } } /// Gets the number of [`Weak`] pointers to this allocation. From a9f43d90be083a55f2e1387d7fc0e7e0dffe649e Mon Sep 17 00:00:00 2001 From: Jeremy Banks <_@jeremy.ca> Date: Tue, 16 Aug 2022 05:09:04 +0000 Subject: [PATCH 7/7] as_weak: add a SAFETY comment and mention the potentially-weird weak_count() result in the documentation --- library/alloc/src/rc.rs | 12 ++++++++++++ library/alloc/src/sync.rs | 12 ++++++++++++ 2 files changed, 24 insertions(+) diff --git a/library/alloc/src/rc.rs b/library/alloc/src/rc.rs index 4f128f348e57c..7a795a73065d6 100644 --- a/library/alloc/src/rc.rs +++ b/library/alloc/src/rc.rs @@ -937,6 +937,9 @@ impl Rc { /// Convert a reference to an [`Rc`] into a reference to a [`Weak`] of the same type. /// /// This is a type-only operation; it doesn't modify the inner reference counts. + /// Therefore if you call [`Weak::weak_count()`] on the returned `&Weak`, + /// it is possible to get a result of `0`, which is otherwise only possible + /// when there are no remaining strong references and the value has been dropped. /// /// # Examples /// @@ -953,6 +956,15 @@ impl Rc { #[unstable(feature = "rc_as_weak", issue = "100472")] #[must_use] pub const fn as_weak(this: &Self) -> &Weak { + // SAFETY: `Rc` and `Weak` are guaranteed to have the same representation + // because both are `#[repr(transparent)]` with their only (non-ZST) field being + // being a `NonNull>`. The static guarantees carried by a `Weak` (that + // the pointer will point to a live `RcBox` allocation _unless_ it is the sentinel + // value of `usize::MAX`) are strictly weaker than those carried by an `Rc` (that + // the pointer will always point to a live `RcBox` allocation containing a live `T`). + // The different drop in their drop behaviour is not relevant because this function + // is only concerned with shared references, not owned values. Therefore no safety + // invariants are violated by interpreting an `&Rc` as a `&Weak`. let weak = this as *const Self as *const Weak; unsafe { &*weak } } diff --git a/library/alloc/src/sync.rs b/library/alloc/src/sync.rs index a0f89344fee47..40e80417e9954 100644 --- a/library/alloc/src/sync.rs +++ b/library/alloc/src/sync.rs @@ -964,6 +964,9 @@ impl Arc { /// Convert a reference to an [`Arc`] into a reference to a [`Weak`] of the same type. /// /// This is a type-only operation; it doesn't modify the inner reference counts. + /// Therefore if you call [`Weak::weak_count()`] on the returned `&Weak`, + /// it is possible to get a result of `0`, which is otherwise only possible + /// when there are no remaining strong references and the value has been dropped. /// /// # Examples /// @@ -980,6 +983,15 @@ impl Arc { #[unstable(feature = "rc_as_weak", issue = "100472")] #[must_use] pub const fn as_weak(this: &Self) -> &Weak { + // SAFETY: `Arc` and `Weak` are guaranteed to have the same representation + // because both are `#[repr(transparent)]` with their only (non-ZST) field being + // being a `NonNull>`. The static guarantees carried by a `Weak` (that + // the pointer will point to a live `ArcInner` allocation _unless_ it is the sentinel + // value of `usize::MAX`) are strictly weaker than those carried by an `Arc` (that + // the pointer will always point to a live `ArcInner` allocation containing a live `T`). + // The different drop in their drop behaviour is not relevant because this function + // is only concerned with shared references, not owned values. Therefore no safety + // invariants are violated by interpreting an `&Arc` as a `&Weak`. let weak = this as *const Self as *const Weak; unsafe { &*weak } }