From 70ae0faa8638366e35b04e2be8c80faa1907ff97 Mon Sep 17 00:00:00 2001 From: Glenn Griffin Date: Sat, 12 Dec 2020 15:39:51 -0800 Subject: [PATCH 1/7] Expose ArcInner as public. The goal of this change is to provide more flexibility over how an `Arc` is allocated. An example of when this could be useful is when the user does not want to panic on allocation failure and instead return a Result. The changes here provide a way of constructing an `Arc` by first creating an `ArcInner` wrapping that in a `Box>`, and finally creating an `Arc` from the `Box>`. The last step does no allocation of it's own and merely takes control of the `ArcInner` from the allocation within the Box. This does not make any of the fields within the `ArcInner` visible so it is *not* stabilizing the exact layout of the `ArcInner`. The only operations that can be done with `ArcInner` is initializing one from a `T` and subsequently creating an `Arc` from a `Box>`. --- library/alloc/src/sync.rs | 49 ++++++++++++++++++++++++++++----- library/alloc/src/sync/tests.rs | 21 ++++++++++++++ 2 files changed, 63 insertions(+), 7 deletions(-) diff --git a/library/alloc/src/sync.rs b/library/alloc/src/sync.rs index 9d478a302e96c..689641192becc 100644 --- a/library/alloc/src/sync.rs +++ b/library/alloc/src/sync.rs @@ -294,11 +294,15 @@ impl fmt::Debug for Weak { } } +/// TODO: doc comment // This is repr(C) to future-proof against possible field-reordering, which // would interfere with otherwise safe [into|from]_raw() of transmutable // inner types. +// TODO: Consider whether there is a better name for this type if it's going to +// be publicly accessible. Maybe `ArcRepr`? +#[unstable(feature = "rc_stable_repr", issue = "none")] #[repr(C)] -struct ArcInner { +pub struct ArcInner { strong: atomic::AtomicUsize, // the value usize::MAX acts as a sentinel for temporarily "locking" the @@ -309,7 +313,36 @@ struct ArcInner { data: T, } +#[unstable(feature = "rc_stable_repr", issue = "none")] +impl fmt::Display for ArcInner { + fn fmt(&self, f: &mut fmt::Formatter<'_>) -> fmt::Result { + fmt::Display::fmt(&self.data, f) + } +} + +#[unstable(feature = "rc_stable_repr", issue = "none")] +impl fmt::Debug for ArcInner { + fn fmt(&self, f: &mut fmt::Formatter<'_>) -> fmt::Result { + fmt::Debug::fmt(&self.data, f) + } +} + +impl ArcInner { + // TODO + #[inline] + #[unstable(feature = "rc_stable_repr", issue = "none")] + pub fn new(data: T) -> ArcInner { + ArcInner{ + strong: atomic::AtomicUsize::new(1), + weak: atomic::AtomicUsize::new(1), + data, + } + } +} + +#[stable(feature = "rust1", since = "1.0.0")] unsafe impl Send for ArcInner {} +#[stable(feature = "rust1", since = "1.0.0")] unsafe impl Sync for ArcInner {} impl Arc { @@ -327,12 +360,8 @@ impl Arc { pub fn new(data: T) -> Arc { // Start the weak pointer count as 1 which is the weak pointer that's // held by all the strong pointers (kinda), see std/rc.rs for more info - let x: Box<_> = box ArcInner { - strong: atomic::AtomicUsize::new(1), - weak: atomic::AtomicUsize::new(1), - data, - }; - Self::from_inner(Box::leak(x).into()) + let x = box ArcInner::new(data); + Self::from_repr(x) } /// Constructs a new `Arc` using a weak reference to itself. Attempting @@ -976,6 +1005,12 @@ impl Arc { pub fn ptr_eq(this: &Self, other: &Self) -> bool { this.ptr.as_ptr() == other.ptr.as_ptr() } + + /// TODO: doc comment + #[unstable(feature = "rc_stable_repr", issue = "none")] + pub fn from_repr(repr: Box>) -> Self { + Self::from_inner(Box::leak(repr).into()) + } } impl Arc { diff --git a/library/alloc/src/sync/tests.rs b/library/alloc/src/sync/tests.rs index 77f328d48f94d..d85480e196605 100644 --- a/library/alloc/src/sync/tests.rs +++ b/library/alloc/src/sync/tests.rs @@ -601,3 +601,24 @@ fn test_arc_cyclic_two_refs() { assert_eq!(Arc::strong_count(&two_refs), 3); assert_eq!(Arc::weak_count(&two_refs), 2); } + +#[test] +fn test_arc_from_repr() { + let repr = Box::new(ArcInner::new(42usize)); + let mut x= Arc::from_repr(repr); + + assert_eq!(Arc::strong_count(&x), 1); + assert_eq!(Arc::weak_count(&x), 0); + assert_eq!(Arc::get_mut(&mut x), Some(&mut 42)); +} + +#[test] +fn test_arc_from_repr_unsized() { + let slice_repr: Box> = Box::new(ArcInner::new([1, 2, 3])); + let slice: Arc<[i32]> = Arc::from_repr(slice_repr); + assert_eq!(format!("{:?}", slice), "[1, 2, 3]"); + + let debug_repr = Box::new(ArcInner::new(3)); + let debug: Arc = Arc::from_repr(debug_repr); + assert_eq!(format!("{:?}", debug), "3"); +} From d30e9d6fbdcaa9e1263e6f92c46b3e85dbbb574b Mon Sep 17 00:00:00 2001 From: Glenn Griffin Date: Sat, 12 Dec 2020 15:47:25 -0800 Subject: [PATCH 2/7] Rename `ArcInner` to `ArcRepr`. `ArcInner` does not seem like the most descriptive name for a type that's now publicly accessible. Rename it to `ArcRepr` since it's the underyling representation of an `Arc`. --- library/alloc/src/sync.rs | 70 ++++++++++++++++----------------- library/alloc/src/sync/tests.rs | 6 +-- 2 files changed, 38 insertions(+), 38 deletions(-) diff --git a/library/alloc/src/sync.rs b/library/alloc/src/sync.rs index 689641192becc..a3ad1cd6e2ff6 100644 --- a/library/alloc/src/sync.rs +++ b/library/alloc/src/sync.rs @@ -221,8 +221,8 @@ macro_rules! acquire { #[cfg_attr(not(test), rustc_diagnostic_item = "Arc")] #[stable(feature = "rust1", since = "1.0.0")] pub struct Arc { - ptr: NonNull>, - phantom: PhantomData>, + ptr: NonNull>, + phantom: PhantomData>, } #[stable(feature = "rust1", since = "1.0.0")] @@ -237,11 +237,11 @@ impl, U: ?Sized> CoerceUnsized> for Arc {} impl, U: ?Sized> DispatchFromDyn> for Arc {} impl Arc { - fn from_inner(ptr: NonNull>) -> Self { + fn from_inner(ptr: NonNull>) -> Self { Self { ptr, phantom: PhantomData } } - unsafe fn from_ptr(ptr: *mut ArcInner) -> Self { + unsafe fn from_ptr(ptr: *mut ArcRepr) -> Self { unsafe { Self::from_inner(NonNull::new_unchecked(ptr)) } } } @@ -274,7 +274,7 @@ pub struct Weak { // to allocate space on the heap. That's not a value a real pointer // will ever have because RcBox has alignment at least 2. // This is only possible when `T: Sized`; unsized `T` never dangle. - ptr: NonNull>, + ptr: NonNull>, } #[stable(feature = "arc_weak", since = "1.4.0")] @@ -302,7 +302,7 @@ impl fmt::Debug for Weak { // be publicly accessible. Maybe `ArcRepr`? #[unstable(feature = "rc_stable_repr", issue = "none")] #[repr(C)] -pub struct ArcInner { +pub struct ArcRepr { strong: atomic::AtomicUsize, // the value usize::MAX acts as a sentinel for temporarily "locking" the @@ -314,25 +314,25 @@ pub struct ArcInner { } #[unstable(feature = "rc_stable_repr", issue = "none")] -impl fmt::Display for ArcInner { +impl fmt::Display for ArcRepr { fn fmt(&self, f: &mut fmt::Formatter<'_>) -> fmt::Result { fmt::Display::fmt(&self.data, f) } } #[unstable(feature = "rc_stable_repr", issue = "none")] -impl fmt::Debug for ArcInner { +impl fmt::Debug for ArcRepr { fn fmt(&self, f: &mut fmt::Formatter<'_>) -> fmt::Result { fmt::Debug::fmt(&self.data, f) } } -impl ArcInner { +impl ArcRepr { // TODO #[inline] #[unstable(feature = "rc_stable_repr", issue = "none")] - pub fn new(data: T) -> ArcInner { - ArcInner{ + pub fn new(data: T) -> ArcRepr { + ArcRepr{ strong: atomic::AtomicUsize::new(1), weak: atomic::AtomicUsize::new(1), data, @@ -341,9 +341,9 @@ impl ArcInner { } #[stable(feature = "rust1", since = "1.0.0")] -unsafe impl Send for ArcInner {} +unsafe impl Send for ArcRepr {} #[stable(feature = "rust1", since = "1.0.0")] -unsafe impl Sync for ArcInner {} +unsafe impl Sync for ArcRepr {} impl Arc { /// Constructs a new `Arc`. @@ -360,7 +360,7 @@ impl Arc { pub fn new(data: T) -> Arc { // Start the weak pointer count as 1 which is the weak pointer that's // held by all the strong pointers (kinda), see std/rc.rs for more info - let x = box ArcInner::new(data); + let x = box ArcRepr::new(data); Self::from_repr(x) } @@ -389,13 +389,13 @@ impl Arc { pub fn new_cyclic(data_fn: impl FnOnce(&Weak) -> T) -> Arc { // Construct the inner in the "uninitialized" state with a single // weak reference. - let uninit_ptr: NonNull<_> = Box::leak(box ArcInner { + let uninit_ptr: NonNull<_> = Box::leak(box ArcRepr { strong: atomic::AtomicUsize::new(0), weak: atomic::AtomicUsize::new(1), data: mem::MaybeUninit::::uninit(), }) .into(); - let init_ptr: NonNull> = uninit_ptr.cast(); + let init_ptr: NonNull> = uninit_ptr.cast(); let weak = Weak { ptr: init_ptr }; @@ -464,7 +464,7 @@ impl Arc { Arc::from_ptr(Arc::allocate_for_layout( Layout::new::(), |layout| Global.allocate(layout), - |mem| mem as *mut ArcInner>, + |mem| mem as *mut ArcRepr>, )) } } @@ -495,7 +495,7 @@ impl Arc { Arc::from_ptr(Arc::allocate_for_layout( Layout::new::(), |layout| Global.allocate_zeroed(layout), - |mem| mem as *mut ArcInner>, + |mem| mem as *mut ArcRepr>, )) } } @@ -604,7 +604,7 @@ impl Arc<[T]> { |layout| Global.allocate_zeroed(layout), |mem| { ptr::slice_from_raw_parts_mut(mem as *mut T, len) - as *mut ArcInner<[mem::MaybeUninit]> + as *mut ArcRepr<[mem::MaybeUninit]> }, )) } @@ -731,7 +731,7 @@ impl Arc { /// ``` #[stable(feature = "rc_as_ptr", since = "1.45.0")] pub fn as_ptr(this: &Self) -> *const T { - let ptr: *mut ArcInner = NonNull::as_ptr(this.ptr); + let ptr: *mut ArcRepr = NonNull::as_ptr(this.ptr); // SAFETY: This cannot go through Deref::deref or RcBoxPtr::inner because // this is required to retain raw/mut provenance such that e.g. `get_mut` can @@ -782,7 +782,7 @@ impl Arc { let offset = data_offset(ptr); // Reverse the offset to find the original ArcInner. - let fake_ptr = ptr as *mut ArcInner; + let fake_ptr = ptr as *mut ArcRepr; let arc_ptr = set_data_ptr(fake_ptr, (ptr as *mut u8).offset(-offset)); Self::from_ptr(arc_ptr) @@ -963,7 +963,7 @@ impl Arc { } #[inline] - fn inner(&self) -> &ArcInner { + fn inner(&self) -> &ArcRepr { // This unsafety is ok because while this arc is alive we're guaranteed // that the inner pointer is valid. Furthermore, we know that the // `ArcInner` structure itself is `Sync` because the inner data is @@ -1008,7 +1008,7 @@ impl Arc { /// TODO: doc comment #[unstable(feature = "rc_stable_repr", issue = "none")] - pub fn from_repr(repr: Box>) -> Self { + pub fn from_repr(repr: Box>) -> Self { Self::from_inner(Box::leak(repr).into()) } } @@ -1022,13 +1022,13 @@ impl Arc { unsafe fn allocate_for_layout( value_layout: Layout, allocate: impl FnOnce(Layout) -> Result, AllocError>, - mem_to_arcinner: impl FnOnce(*mut u8) -> *mut ArcInner, - ) -> *mut ArcInner { + mem_to_arcinner: impl FnOnce(*mut u8) -> *mut ArcRepr, + ) -> *mut ArcRepr { // Calculate layout using the given value layout. // Previously, layout was calculated on the expression // `&*(ptr as *const ArcInner)`, but this created a misaligned // reference (see #54908). - let layout = Layout::new::>().extend(value_layout).unwrap().0.pad_to_align(); + let layout = Layout::new::>().extend(value_layout).unwrap().0.pad_to_align(); let ptr = allocate(layout).unwrap_or_else(|_| handle_alloc_error(layout)); @@ -1045,13 +1045,13 @@ impl Arc { } /// Allocates an `ArcInner` with sufficient space for an unsized inner value. - unsafe fn allocate_for_ptr(ptr: *const T) -> *mut ArcInner { + unsafe fn allocate_for_ptr(ptr: *const T) -> *mut ArcRepr { // Allocate for the `ArcInner` using the given value. unsafe { Self::allocate_for_layout( Layout::for_value(&*ptr), |layout| Global.allocate(layout), - |mem| set_data_ptr(ptr as *mut T, mem) as *mut ArcInner, + |mem| set_data_ptr(ptr as *mut T, mem) as *mut ArcRepr, ) } } @@ -1081,12 +1081,12 @@ impl Arc { impl Arc<[T]> { /// Allocates an `ArcInner<[T]>` with the given length. - unsafe fn allocate_for_slice(len: usize) -> *mut ArcInner<[T]> { + unsafe fn allocate_for_slice(len: usize) -> *mut ArcRepr<[T]> { unsafe { Self::allocate_for_layout( Layout::array::(len).unwrap(), |layout| Global.allocate(layout), - |mem| ptr::slice_from_raw_parts_mut(mem as *mut T, len) as *mut ArcInner<[T]>, + |mem| ptr::slice_from_raw_parts_mut(mem as *mut T, len) as *mut ArcRepr<[T]>, ) } } @@ -1534,7 +1534,7 @@ impl Arc { T: Any + Send + Sync + 'static, { if (*self).is::() { - let ptr = self.ptr.cast::>(); + let ptr = self.ptr.cast::>(); mem::forget(self); Ok(Arc::from_inner(ptr)) } else { @@ -1559,7 +1559,7 @@ impl Weak { /// ``` #[stable(feature = "downgraded_weak", since = "1.10.0")] pub fn new() -> Weak { - Weak { ptr: NonNull::new(usize::MAX as *mut ArcInner).expect("MAX is not 0") } + Weak { ptr: NonNull::new(usize::MAX as *mut ArcRepr).expect("MAX is not 0") } } } @@ -1598,7 +1598,7 @@ impl Weak { /// [`null`]: core::ptr::null #[stable(feature = "weak_into_raw", since = "1.45.0")] pub fn as_ptr(&self) -> *const T { - let ptr: *mut ArcInner = NonNull::as_ptr(self.ptr); + let ptr: *mut ArcRepr = NonNull::as_ptr(self.ptr); // SAFETY: we must offset the pointer manually, and said pointer may be // a dangling weak (usize::MAX) if T is sized. data_offset is safe to call, @@ -1697,7 +1697,7 @@ impl Weak { // 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, (ptr as *mut u8).wrapping_offset(-offset)) + set_data_ptr(ptr as *mut ArcRepr, (ptr as *mut u8).wrapping_offset(-offset)) }; // SAFETY: we now have recovered the original Weak pointer, so can create the Weak. @@ -2404,6 +2404,6 @@ unsafe fn data_offset(ptr: *const T) -> isize { #[inline] fn data_offset_align(align: usize) -> isize { - let layout = Layout::new::>(); + let layout = Layout::new::>(); (layout.size() + layout.padding_needed_for(align)) as isize } diff --git a/library/alloc/src/sync/tests.rs b/library/alloc/src/sync/tests.rs index d85480e196605..74c1ca12bb137 100644 --- a/library/alloc/src/sync/tests.rs +++ b/library/alloc/src/sync/tests.rs @@ -604,7 +604,7 @@ fn test_arc_cyclic_two_refs() { #[test] fn test_arc_from_repr() { - let repr = Box::new(ArcInner::new(42usize)); + let repr = Box::new(ArcRepr::new(42usize)); let mut x= Arc::from_repr(repr); assert_eq!(Arc::strong_count(&x), 1); @@ -614,11 +614,11 @@ fn test_arc_from_repr() { #[test] fn test_arc_from_repr_unsized() { - let slice_repr: Box> = Box::new(ArcInner::new([1, 2, 3])); + let slice_repr: Box> = Box::new(ArcRepr::new([1, 2, 3])); let slice: Arc<[i32]> = Arc::from_repr(slice_repr); assert_eq!(format!("{:?}", slice), "[1, 2, 3]"); - let debug_repr = Box::new(ArcInner::new(3)); + let debug_repr = Box::new(ArcRepr::new(3)); let debug: Arc = Arc::from_repr(debug_repr); assert_eq!(format!("{:?}", debug), "3"); } From b2081bf4e3d966e175abb769638206bf02af922f Mon Sep 17 00:00:00 2001 From: Glenn Griffin Date: Sat, 12 Dec 2020 16:17:05 -0800 Subject: [PATCH 3/7] Expose RcBox as public. The goal of this change is to provide more flexibility over how an `Rc` is allocated. An example of when this could be useful is when the user does not want to panic on allocation failure and instead return a Result. The changes here provide a way of constructing an `Rc` by first creating an `RcBox` wrapping that in a `Box>`, and finally creating an `Rc` from the `Box>`. The last step does no allocation of it's own and merely takes control of the `RcBox` from the allocation within the Box. This does not make any of the fields within the `RcBox` visible so it is *not* stabilizing the exact layout of the `RcBox`. The only operations that can be done with `RcBox` is initializing one from a `T` and subsequently creating an `Rc` from a `Box>`. --- library/alloc/src/rc.rs | 37 +++++++++++++++++++++++++++++++---- library/alloc/src/rc/tests.rs | 18 +++++++++++++++++ 2 files changed, 51 insertions(+), 4 deletions(-) diff --git a/library/alloc/src/rc.rs b/library/alloc/src/rc.rs index a96be57143d38..b30b5a9f21133 100644 --- a/library/alloc/src/rc.rs +++ b/library/alloc/src/rc.rs @@ -270,16 +270,41 @@ use crate::vec::Vec; #[cfg(test)] mod tests; +/// TODO: doc comments // This is repr(C) to future-proof against possible field-reordering, which // would interfere with otherwise safe [into|from]_raw() of transmutable // inner types. +#[unstable(feature = "rc_stable_repr", issue = "none")] #[repr(C)] -struct RcBox { +pub struct RcBox { strong: Cell, weak: Cell, value: T, } +impl RcBox { + /// TODO: doc comments + #[unstable(feature = "rc_stable_repr", issue = "none")] + pub fn new(value: T) -> Self { + RcBox { strong: Cell::new(1), weak: Cell::new(1), value } + } +} + +#[unstable(feature = "rc_stable_repr", issue = "none")] +impl fmt::Display for RcBox { + fn fmt(&self, f: &mut fmt::Formatter<'_>) -> fmt::Result { + fmt::Display::fmt(&self.value, f) + } +} + +#[unstable(feature = "rc_stable_repr", issue = "none")] +impl fmt::Debug for RcBox { + fn fmt(&self, f: &mut fmt::Formatter<'_>) -> fmt::Result { + fmt::Debug::fmt(&self.value, f) + } +} + + /// A single-threaded reference-counting pointer. 'Rc' stands for 'Reference /// Counted'. /// @@ -341,9 +366,7 @@ impl Rc { // pointers, which ensures that the weak destructor never frees // the allocation while the strong destructor is running, even // if the weak pointer is stored inside the strong one. - Self::from_inner( - Box::leak(box RcBox { strong: Cell::new(1), weak: Cell::new(1), value }).into(), - ) + Self::from_repr(box RcBox::new(value)) } /// Constructs a new `Rc` using a weak reference to itself. Attempting @@ -892,6 +915,12 @@ impl Rc { pub fn ptr_eq(this: &Self, other: &Self) -> bool { this.ptr.as_ptr() == other.ptr.as_ptr() } + + /// TODO: doc comments + #[unstable(feature = "rc_stable_repr", issue = "none")] + pub fn from_repr(repr: Box>) -> Self { + Self::from_inner(Box::leak(repr).into()) + } } impl Rc { diff --git a/library/alloc/src/rc/tests.rs b/library/alloc/src/rc/tests.rs index bb5c3f4f90433..ac22d4db97758 100644 --- a/library/alloc/src/rc/tests.rs +++ b/library/alloc/src/rc/tests.rs @@ -542,3 +542,21 @@ fn test_rc_cyclic_with_two_ref() { assert_eq!(Rc::strong_count(&two_refs), 3); assert_eq!(Rc::weak_count(&two_refs), 2); } + +#[test] +fn test_rc_from_repr() { + let rc_repr = Box::new(RcBox::new(5)); + let x = Rc::from_repr(rc_repr); + assert_eq!(*x, 5); +} + +#[test] +fn test_rc_from_repr_unsized() { + let slice_repr = Box::new(RcBox::new([1, 2, 3])); + let slice: Rc<[i32]> = Rc::from_repr(slice_repr); + assert_eq!(format!("{:?}", slice), "[1, 2, 3]"); + + let debug_repr = Box::new(RcBox::new(3)); + let debug: Rc = Rc::from_repr(debug_repr); + assert_eq!(format!("{:?}", debug), "3"); +} \ No newline at end of file From 907e441e06c8dd26f3a43aca8e357086646b81d4 Mon Sep 17 00:00:00 2001 From: Glenn Griffin Date: Sat, 12 Dec 2020 16:19:14 -0800 Subject: [PATCH 4/7] Rename `RcBox` to `RcRepr`. `RcBox` does not seem like the most descriptive name for a type that's now publicly accessible. Rename it to `RcRepr` since it's the underyling representation of an `Rc`. --- library/alloc/src/rc.rs | 66 +++++++++++++++++------------------ library/alloc/src/rc/tests.rs | 6 ++-- 2 files changed, 36 insertions(+), 36 deletions(-) diff --git a/library/alloc/src/rc.rs b/library/alloc/src/rc.rs index b30b5a9f21133..c6b4e3c140cbf 100644 --- a/library/alloc/src/rc.rs +++ b/library/alloc/src/rc.rs @@ -276,29 +276,29 @@ mod tests; // inner types. #[unstable(feature = "rc_stable_repr", issue = "none")] #[repr(C)] -pub struct RcBox { +pub struct RcRepr { strong: Cell, weak: Cell, value: T, } -impl RcBox { +impl RcRepr { /// TODO: doc comments #[unstable(feature = "rc_stable_repr", issue = "none")] pub fn new(value: T) -> Self { - RcBox { strong: Cell::new(1), weak: Cell::new(1), value } + RcRepr { strong: Cell::new(1), weak: Cell::new(1), value } } } #[unstable(feature = "rc_stable_repr", issue = "none")] -impl fmt::Display for RcBox { +impl fmt::Display for RcRepr { fn fmt(&self, f: &mut fmt::Formatter<'_>) -> fmt::Result { fmt::Display::fmt(&self.value, f) } } #[unstable(feature = "rc_stable_repr", issue = "none")] -impl fmt::Debug for RcBox { +impl fmt::Debug for RcRepr { fn fmt(&self, f: &mut fmt::Formatter<'_>) -> fmt::Result { fmt::Debug::fmt(&self.value, f) } @@ -318,8 +318,8 @@ impl fmt::Debug for RcBox { #[cfg_attr(not(test), rustc_diagnostic_item = "Rc")] #[stable(feature = "rust1", since = "1.0.0")] pub struct Rc { - ptr: NonNull>, - phantom: PhantomData>, + ptr: NonNull>, + phantom: PhantomData>, } #[stable(feature = "rust1", since = "1.0.0")] @@ -335,17 +335,17 @@ impl, U: ?Sized> DispatchFromDyn> for Rc {} impl Rc { #[inline(always)] - fn inner(&self) -> &RcBox { + fn inner(&self) -> &RcRepr { // This unsafety is ok because while this Rc is alive we're guaranteed // that the inner pointer is valid. unsafe { self.ptr.as_ref() } } - fn from_inner(ptr: NonNull>) -> Self { + fn from_inner(ptr: NonNull>) -> Self { Self { ptr, phantom: PhantomData } } - unsafe fn from_ptr(ptr: *mut RcBox) -> Self { + unsafe fn from_ptr(ptr: *mut RcRepr) -> Self { Self::from_inner(unsafe { NonNull::new_unchecked(ptr) }) } } @@ -366,7 +366,7 @@ impl Rc { // pointers, which ensures that the weak destructor never frees // the allocation while the strong destructor is running, even // if the weak pointer is stored inside the strong one. - Self::from_repr(box RcBox::new(value)) + Self::from_repr(box RcRepr::new(value)) } /// Constructs a new `Rc` using a weak reference to itself. Attempting @@ -377,14 +377,14 @@ impl Rc { pub fn new_cyclic(data_fn: impl FnOnce(&Weak) -> T) -> Rc { // Construct the inner in the "uninitialized" state with a single // weak reference. - let uninit_ptr: NonNull<_> = Box::leak(box RcBox { + let uninit_ptr: NonNull<_> = Box::leak(box RcRepr { strong: Cell::new(0), weak: Cell::new(1), value: mem::MaybeUninit::::uninit(), }) .into(); - let init_ptr: NonNull> = uninit_ptr.cast(); + let init_ptr: NonNull> = uninit_ptr.cast(); let weak = Weak { ptr: init_ptr }; @@ -440,7 +440,7 @@ impl Rc { Rc::from_ptr(Rc::allocate_for_layout( Layout::new::(), |layout| Global.allocate(layout), - |mem| mem as *mut RcBox>, + |mem| mem as *mut RcRepr>, )) } } @@ -471,7 +471,7 @@ impl Rc { Rc::from_ptr(Rc::allocate_for_layout( Layout::new::(), |layout| Global.allocate_zeroed(layout), - |mem| mem as *mut RcBox>, + |mem| mem as *mut RcRepr>, )) } } @@ -581,7 +581,7 @@ impl Rc<[T]> { |layout| Global.allocate_zeroed(layout), |mem| { ptr::slice_from_raw_parts_mut(mem as *mut T, len) - as *mut RcBox<[mem::MaybeUninit]> + as *mut RcRepr<[mem::MaybeUninit]> }, )) } @@ -710,7 +710,7 @@ impl Rc { /// ``` #[stable(feature = "weak_into_raw", since = "1.45.0")] pub fn as_ptr(this: &Self) -> *const T { - let ptr: *mut RcBox = NonNull::as_ptr(this.ptr); + let ptr: *mut RcRepr = NonNull::as_ptr(this.ptr); // SAFETY: This cannot go through Deref::deref or Rc::inner because // this is required to retain raw/mut provenance such that e.g. `get_mut` can @@ -760,7 +760,7 @@ impl Rc { let offset = unsafe { data_offset(ptr) }; // Reverse the offset to find the original RcBox. - let fake_ptr = ptr as *mut RcBox; + let fake_ptr = ptr as *mut RcRepr; let rc_ptr = unsafe { set_data_ptr(fake_ptr, (ptr as *mut u8).offset(-offset)) }; unsafe { Self::from_ptr(rc_ptr) } @@ -918,7 +918,7 @@ impl Rc { /// TODO: doc comments #[unstable(feature = "rc_stable_repr", issue = "none")] - pub fn from_repr(repr: Box>) -> Self { + pub fn from_repr(repr: Box>) -> Self { Self::from_inner(Box::leak(repr).into()) } } @@ -1022,7 +1022,7 @@ impl Rc { /// ``` pub fn downcast(self) -> Result, Rc> { if (*self).is::() { - let ptr = self.ptr.cast::>(); + let ptr = self.ptr.cast::>(); forget(self); Ok(Rc::from_inner(ptr)) } else { @@ -1040,13 +1040,13 @@ impl Rc { unsafe fn allocate_for_layout( value_layout: Layout, allocate: impl FnOnce(Layout) -> Result, AllocError>, - mem_to_rcbox: impl FnOnce(*mut u8) -> *mut RcBox, - ) -> *mut RcBox { + mem_to_rcbox: impl FnOnce(*mut u8) -> *mut RcRepr, + ) -> *mut RcRepr { // Calculate layout using the given value layout. // Previously, layout was calculated on the expression // `&*(ptr as *const RcBox)`, but this created a misaligned // reference (see #54908). - let layout = Layout::new::>().extend(value_layout).unwrap().0.pad_to_align(); + let layout = Layout::new::>().extend(value_layout).unwrap().0.pad_to_align(); // Allocate for the layout. let ptr = allocate(layout).unwrap_or_else(|_| handle_alloc_error(layout)); @@ -1064,13 +1064,13 @@ impl Rc { } /// Allocates an `RcBox` with sufficient space for an unsized inner value - unsafe fn allocate_for_ptr(ptr: *const T) -> *mut RcBox { + unsafe fn allocate_for_ptr(ptr: *const T) -> *mut RcRepr { // Allocate for the `RcBox` using the given value. unsafe { Self::allocate_for_layout( Layout::for_value(&*ptr), |layout| Global.allocate(layout), - |mem| set_data_ptr(ptr as *mut T, mem) as *mut RcBox, + |mem| set_data_ptr(ptr as *mut T, mem) as *mut RcRepr, ) } } @@ -1100,12 +1100,12 @@ impl Rc { impl Rc<[T]> { /// Allocates an `RcBox<[T]>` with the given length. - unsafe fn allocate_for_slice(len: usize) -> *mut RcBox<[T]> { + unsafe fn allocate_for_slice(len: usize) -> *mut RcRepr<[T]> { unsafe { Self::allocate_for_layout( Layout::array::(len).unwrap(), |layout| Global.allocate(layout), - |mem| ptr::slice_from_raw_parts_mut(mem as *mut T, len) as *mut RcBox<[T]>, + |mem| ptr::slice_from_raw_parts_mut(mem as *mut T, len) as *mut RcRepr<[T]>, ) } } @@ -1732,7 +1732,7 @@ pub struct Weak { // to allocate space on the heap. That's not a value a real pointer // will ever have because RcBox has alignment at least 2. // This is only possible when `T: Sized`; unsized `T` never dangle. - ptr: NonNull>, + ptr: NonNull>, } #[stable(feature = "rc_weak", since = "1.4.0")] @@ -1762,7 +1762,7 @@ impl Weak { /// ``` #[stable(feature = "downgraded_weak", since = "1.10.0")] pub fn new() -> Weak { - Weak { ptr: NonNull::new(usize::MAX as *mut RcBox).expect("MAX is not 0") } + Weak { ptr: NonNull::new(usize::MAX as *mut RcRepr).expect("MAX is not 0") } } } @@ -1806,7 +1806,7 @@ impl Weak { /// [`null`]: core::ptr::null #[stable(feature = "rc_as_ptr", since = "1.45.0")] pub fn as_ptr(&self) -> *const T { - let ptr: *mut RcBox = NonNull::as_ptr(self.ptr); + let ptr: *mut RcRepr = NonNull::as_ptr(self.ptr); // SAFETY: we must offset the pointer manually, and said pointer may be // a dangling weak (usize::MAX) if T is sized. data_offset is safe to call, @@ -1905,7 +1905,7 @@ impl Weak { // 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, (ptr as *mut u8).wrapping_offset(-offset)) + set_data_ptr(ptr as *mut RcRepr, (ptr as *mut u8).wrapping_offset(-offset)) }; // SAFETY: we now have recovered the original Weak pointer, so can create the Weak. @@ -2188,7 +2188,7 @@ trait RcInnerPtr { } } -impl RcInnerPtr for RcBox { +impl RcInnerPtr for RcRepr { #[inline(always)] fn weak_ref(&self) -> &Cell { &self.weak @@ -2249,6 +2249,6 @@ unsafe fn data_offset(ptr: *const T) -> isize { #[inline] fn data_offset_align(align: usize) -> isize { - let layout = Layout::new::>(); + let layout = Layout::new::>(); (layout.size() + layout.padding_needed_for(align)) as isize } diff --git a/library/alloc/src/rc/tests.rs b/library/alloc/src/rc/tests.rs index ac22d4db97758..57be2aa1a1291 100644 --- a/library/alloc/src/rc/tests.rs +++ b/library/alloc/src/rc/tests.rs @@ -545,18 +545,18 @@ fn test_rc_cyclic_with_two_ref() { #[test] fn test_rc_from_repr() { - let rc_repr = Box::new(RcBox::new(5)); + let rc_repr = Box::new(RcRepr::new(5)); let x = Rc::from_repr(rc_repr); assert_eq!(*x, 5); } #[test] fn test_rc_from_repr_unsized() { - let slice_repr = Box::new(RcBox::new([1, 2, 3])); + let slice_repr = Box::new(RcRepr::new([1, 2, 3])); let slice: Rc<[i32]> = Rc::from_repr(slice_repr); assert_eq!(format!("{:?}", slice), "[1, 2, 3]"); - let debug_repr = Box::new(RcBox::new(3)); + let debug_repr = Box::new(RcRepr::new(3)); let debug: Rc = Rc::from_repr(debug_repr); assert_eq!(format!("{:?}", debug), "3"); } \ No newline at end of file From ea14e2257201d1d74bd3c04c85f57d5c6a21a956 Mon Sep 17 00:00:00 2001 From: Glenn Griffin Date: Sat, 12 Dec 2020 21:25:47 -0800 Subject: [PATCH 5/7] Add documentation comments for ArcRepr and RcRepr. --- library/alloc/src/rc.rs | 34 +++++++++++++++++++++++++++++++--- library/alloc/src/sync.rs | 34 +++++++++++++++++++++++++++++++--- 2 files changed, 62 insertions(+), 6 deletions(-) diff --git a/library/alloc/src/rc.rs b/library/alloc/src/rc.rs index c6b4e3c140cbf..723e27babc291 100644 --- a/library/alloc/src/rc.rs +++ b/library/alloc/src/rc.rs @@ -270,7 +270,16 @@ use crate::vec::Vec; #[cfg(test)] mod tests; -/// TODO: doc comments +/// The underlying representation of an `Rc`. +/// +/// It's possible to create an [`Rc`][rc] from a `Box>` using +/// [`Rc::from_repr(boxed_rc_repr)`][from_repr]. This is only necessary when +/// needing to have more control over how the `Rc` is allocated. Most users +/// should use [`Rc::new`][rc_new]. +/// +/// [rc]: Rc +/// [from_repr]: Rc::from_repr +/// [rc_new]: Rc::new // This is repr(C) to future-proof against possible field-reordering, which // would interfere with otherwise safe [into|from]_raw() of transmutable // inner types. @@ -283,7 +292,15 @@ pub struct RcRepr { } impl RcRepr { - /// TODO: doc comments + /// Constructs a new RcRepr. + /// + /// # Examples + /// + /// ``` + /// use std::rc::RcRepr; + /// + /// let five = RcRepr::new(5); + /// ``` #[unstable(feature = "rc_stable_repr", issue = "none")] pub fn new(value: T) -> Self { RcRepr { strong: Cell::new(1), weak: Cell::new(1), value } @@ -916,7 +933,18 @@ impl Rc { this.ptr.as_ptr() == other.ptr.as_ptr() } - /// TODO: doc comments + /// Constructs a new Rc from a Box>. + /// + /// The new Rc consumes the allocation from the Box. This method does not + /// allocate. + /// + /// # Examples + /// + /// ``` + /// use std::rc::{Rc, RcRepr}; + /// + /// let five = Rc::from_repr(Box::new(RcRepr::new(5))); + /// ``` #[unstable(feature = "rc_stable_repr", issue = "none")] pub fn from_repr(repr: Box>) -> Self { Self::from_inner(Box::leak(repr).into()) diff --git a/library/alloc/src/sync.rs b/library/alloc/src/sync.rs index a3ad1cd6e2ff6..aae9e24f1f24a 100644 --- a/library/alloc/src/sync.rs +++ b/library/alloc/src/sync.rs @@ -294,7 +294,16 @@ impl fmt::Debug for Weak { } } -/// TODO: doc comment +/// The underlying representation of an `Arc`. +/// +/// It's possible to create an [`Arc`][arc] from a `Box>` using +/// [`Arc::from_repr(boxed_arc_repr)`][from_repr]. This is only necessary when +/// needing to have more control over how the `Arc` is allocated. Most users +/// should use [`Arc::new`][arc_new]. +/// +/// [arc]: ARc +/// [from_repr]: Arc::from_repr +/// [arc_new]: Arc::new // This is repr(C) to future-proof against possible field-reordering, which // would interfere with otherwise safe [into|from]_raw() of transmutable // inner types. @@ -328,7 +337,15 @@ impl fmt::Debug for ArcRepr { } impl ArcRepr { - // TODO + /// Constructs a new ArcRepr. + /// + /// # Examples + /// + /// ``` + /// use std::sync::ArcRepr; + /// + /// let five = ArcRepr::new(5); + /// ``` #[inline] #[unstable(feature = "rc_stable_repr", issue = "none")] pub fn new(data: T) -> ArcRepr { @@ -1006,7 +1023,18 @@ impl Arc { this.ptr.as_ptr() == other.ptr.as_ptr() } - /// TODO: doc comment + /// Constructs a new Arc from a Box>. + /// + /// The new Arc consumes the allocation from the Box. This method does not + /// allocate. + /// + /// # Examples + /// + /// ``` + /// use std::sync::{Arc, ArcRepr}; + /// + /// let five = Arc::from_repr(Box::new(ArcRepr::new(5))); + /// ``` #[unstable(feature = "rc_stable_repr", issue = "none")] pub fn from_repr(repr: Box>) -> Self { Self::from_inner(Box::leak(repr).into()) From dcdda7d183067ce26d4b141b0924d5d2da1c7be4 Mon Sep 17 00:00:00 2001 From: Glenn Griffin Date: Mon, 14 Dec 2020 08:43:29 -0800 Subject: [PATCH 6/7] Some tidy fixes. --- library/alloc/src/rc/tests.rs | 2 +- library/alloc/src/sync.rs | 1 - 2 files changed, 1 insertion(+), 2 deletions(-) diff --git a/library/alloc/src/rc/tests.rs b/library/alloc/src/rc/tests.rs index 57be2aa1a1291..5414499ca189f 100644 --- a/library/alloc/src/rc/tests.rs +++ b/library/alloc/src/rc/tests.rs @@ -559,4 +559,4 @@ fn test_rc_from_repr_unsized() { let debug_repr = Box::new(RcRepr::new(3)); let debug: Rc = Rc::from_repr(debug_repr); assert_eq!(format!("{:?}", debug), "3"); -} \ No newline at end of file +} diff --git a/library/alloc/src/sync.rs b/library/alloc/src/sync.rs index aae9e24f1f24a..9d7b7c3df865f 100644 --- a/library/alloc/src/sync.rs +++ b/library/alloc/src/sync.rs @@ -307,7 +307,6 @@ impl fmt::Debug for Weak { // This is repr(C) to future-proof against possible field-reordering, which // would interfere with otherwise safe [into|from]_raw() of transmutable // inner types. -// TODO: Consider whether there is a better name for this type if it's going to // be publicly accessible. Maybe `ArcRepr`? #[unstable(feature = "rc_stable_repr", issue = "none")] #[repr(C)] From f3b554d17407b186f8c89fe95f180504b88e31ee Mon Sep 17 00:00:00 2001 From: Glenn Griffin Date: Mon, 14 Dec 2020 09:10:07 -0800 Subject: [PATCH 7/7] ./x.py fmt --- library/alloc/src/rc.rs | 1 - library/alloc/src/sync.rs | 6 +----- library/alloc/src/sync/tests.rs | 2 +- 3 files changed, 2 insertions(+), 7 deletions(-) diff --git a/library/alloc/src/rc.rs b/library/alloc/src/rc.rs index 723e27babc291..d3daf22c42fe1 100644 --- a/library/alloc/src/rc.rs +++ b/library/alloc/src/rc.rs @@ -321,7 +321,6 @@ impl fmt::Debug for RcRepr { } } - /// A single-threaded reference-counting pointer. 'Rc' stands for 'Reference /// Counted'. /// diff --git a/library/alloc/src/sync.rs b/library/alloc/src/sync.rs index 9d7b7c3df865f..d145312f4badb 100644 --- a/library/alloc/src/sync.rs +++ b/library/alloc/src/sync.rs @@ -348,11 +348,7 @@ impl ArcRepr { #[inline] #[unstable(feature = "rc_stable_repr", issue = "none")] pub fn new(data: T) -> ArcRepr { - ArcRepr{ - strong: atomic::AtomicUsize::new(1), - weak: atomic::AtomicUsize::new(1), - data, - } + ArcRepr { strong: atomic::AtomicUsize::new(1), weak: atomic::AtomicUsize::new(1), data } } } diff --git a/library/alloc/src/sync/tests.rs b/library/alloc/src/sync/tests.rs index 74c1ca12bb137..417504e911f27 100644 --- a/library/alloc/src/sync/tests.rs +++ b/library/alloc/src/sync/tests.rs @@ -605,7 +605,7 @@ fn test_arc_cyclic_two_refs() { #[test] fn test_arc_from_repr() { let repr = Box::new(ArcRepr::new(42usize)); - let mut x= Arc::from_repr(repr); + let mut x = Arc::from_repr(repr); assert_eq!(Arc::strong_count(&x), 1); assert_eq!(Arc::weak_count(&x), 0);