From cdb6bef4fbaf114e3d8546bf0bb213471a8d0f7c Mon Sep 17 00:00:00 2001 From: Simon Sapin Date: Wed, 15 Apr 2020 15:37:46 +0200 Subject: [PATCH 1/4] Deprecate `Box::into_raw_non_null` Per https://github.com/rust-lang/rust/issues/47336#issuecomment-586589016 --- src/liballoc/boxed.rs | 33 ++++++++++++++++--------- src/liballoc/collections/linked_list.rs | 16 ++++++------ src/liballoc/lib.rs | 1 - src/liballoc/rc.rs | 8 +++--- src/liballoc/sync.rs | 2 +- 5 files changed, 32 insertions(+), 28 deletions(-) diff --git a/src/liballoc/boxed.rs b/src/liballoc/boxed.rs index db7420954a091..567fd625582b9 100644 --- a/src/liballoc/boxed.rs +++ b/src/liballoc/boxed.rs @@ -428,7 +428,15 @@ impl Box { #[stable(feature = "box_raw", since = "1.4.0")] #[inline] pub fn into_raw(b: Box) -> *mut T { - Box::into_raw_non_null(b).as_ptr() + let b = mem::ManuallyDrop::new(b); + let mut unique = b.0; + // Box is kind-of a library type, but recognized as a "unique pointer" by + // Stacked Borrows. This function here corresponds to "reborrowing to + // a raw pointer", but there is no actual reborrow here -- so + // without some care, the pointer we are returning here still carries + // the tag of `b`, with `Unique` permission. + // We round-trip through a mutable reference to avoid that. + unsafe { unique.as_mut() as *mut T } } /// Consumes the `Box`, returning the wrapped pointer as `NonNull`. @@ -451,6 +459,7 @@ impl Box { /// /// ``` /// #![feature(box_into_raw_non_null)] + /// #![allow(deprecated)] /// /// let x = Box::new(5); /// let ptr = Box::into_raw_non_null(x); @@ -460,24 +469,24 @@ impl Box { /// let x = unsafe { Box::from_raw(ptr.as_ptr()) }; /// ``` #[unstable(feature = "box_into_raw_non_null", issue = "47336")] + #[rustc_deprecated( + since = "1.44.0", + reason = "use `Box::leak(b).into()` or `NonNull::from(Box::leak(b))` instead" + )] #[inline] pub fn into_raw_non_null(b: Box) -> NonNull { - Box::into_unique(b).into() + Box::leak(b).into() } - #[unstable(feature = "ptr_internals", issue = "none", reason = "use into_raw_non_null instead")] + #[unstable( + feature = "ptr_internals", + issue = "none", + reason = "use `Box::leak(b).into()` or `NonNull::from(Box::leak(b))` instead" + )] #[inline] #[doc(hidden)] pub fn into_unique(b: Box) -> Unique { - let b = mem::ManuallyDrop::new(b); - let mut unique = b.0; - // Box is kind-of a library type, but recognized as a "unique pointer" by - // Stacked Borrows. This function here corresponds to "reborrowing to - // a raw pointer", but there is no actual reborrow here -- so - // without some care, the pointer we are returning here still carries - // the tag of `b`, with `Unique` permission. - // We round-trip through a mutable reference to avoid that. - unsafe { Unique::new_unchecked(unique.as_mut() as *mut T) } + Box::leak(b).into() } /// Consumes and leaks the `Box`, returning a mutable reference, diff --git a/src/liballoc/collections/linked_list.rs b/src/liballoc/collections/linked_list.rs index 53d4f7239b76e..623aa592b604a 100644 --- a/src/liballoc/collections/linked_list.rs +++ b/src/liballoc/collections/linked_list.rs @@ -143,7 +143,7 @@ impl LinkedList { unsafe { node.next = self.head; node.prev = None; - let node = Some(Box::into_raw_non_null(node)); + let node = Some(Box::leak(node).into()); match self.head { None => self.tail = node, @@ -184,7 +184,7 @@ impl LinkedList { unsafe { node.next = None; node.prev = self.tail; - let node = Some(Box::into_raw_non_null(node)); + let node = Some(Box::leak(node).into()); match self.tail { None => self.head = node, @@ -1133,11 +1133,9 @@ impl IterMut<'_, T> { Some(prev) => prev, }; - let node = Some(Box::into_raw_non_null(box Node { - next: Some(head), - prev: Some(prev), - element, - })); + let node = Some( + Box::leak(box Node { next: Some(head), prev: Some(prev), element }).into(), + ); // Not creating references to entire nodes to not invalidate the // reference to `element` we handed to the user. @@ -1442,7 +1440,7 @@ impl<'a, T> CursorMut<'a, T> { #[unstable(feature = "linked_list_cursors", issue = "58533")] pub fn insert_after(&mut self, item: T) { unsafe { - let spliced_node = Box::into_raw_non_null(Box::new(Node::new(item))); + let spliced_node = Box::leak(Box::new(Node::new(item))).into(); let node_next = match self.current { None => self.list.head, Some(node) => node.as_ref().next, @@ -1462,7 +1460,7 @@ impl<'a, T> CursorMut<'a, T> { #[unstable(feature = "linked_list_cursors", issue = "58533")] pub fn insert_before(&mut self, item: T) { unsafe { - let spliced_node = Box::into_raw_non_null(Box::new(Node::new(item))); + let spliced_node = Box::leak(Box::new(Node::new(item))).into(); let node_prev = match self.current { None => self.list.tail, Some(node) => node.as_ref().prev, diff --git a/src/liballoc/lib.rs b/src/liballoc/lib.rs index 121c1cde548cb..26eee09e6fa23 100644 --- a/src/liballoc/lib.rs +++ b/src/liballoc/lib.rs @@ -77,7 +77,6 @@ #![feature(allocator_api)] #![feature(allow_internal_unstable)] #![feature(arbitrary_self_types)] -#![feature(box_into_raw_non_null)] #![feature(box_patterns)] #![feature(box_syntax)] #![feature(cfg_sanitize)] diff --git a/src/liballoc/rc.rs b/src/liballoc/rc.rs index abc4056cf5695..653b4573e2a45 100644 --- a/src/liballoc/rc.rs +++ b/src/liballoc/rc.rs @@ -324,11 +324,9 @@ 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::into_raw_non_null(box RcBox { - strong: Cell::new(1), - weak: Cell::new(1), - value, - })) + Self::from_inner( + Box::leak(box RcBox { strong: Cell::new(1), weak: Cell::new(1), value }).into(), + ) } /// Constructs a new `Rc` with uninitialized contents. diff --git a/src/liballoc/sync.rs b/src/liballoc/sync.rs index b1b22e46a7c2f..59bc8686cf4f9 100644 --- a/src/liballoc/sync.rs +++ b/src/liballoc/sync.rs @@ -325,7 +325,7 @@ impl Arc { weak: atomic::AtomicUsize::new(1), data, }; - Self::from_inner(Box::into_raw_non_null(x)) + Self::from_inner(Box::leak(x).into()) } /// Constructs a new `Arc` with uninitialized contents. From b359fe1eea3ce987f8d77e4418b834db4b1e5353 Mon Sep 17 00:00:00 2001 From: Simon Sapin Date: Wed, 15 Apr 2020 15:56:07 +0200 Subject: [PATCH 2/4] Deprecate `Rc::into_raw_non_null` and `Arc::into_raw_non_null` --- src/liballoc/rc.rs | 2 ++ src/liballoc/sync.rs | 2 ++ 2 files changed, 4 insertions(+) diff --git a/src/liballoc/rc.rs b/src/liballoc/rc.rs index 653b4573e2a45..602f3b803f5fe 100644 --- a/src/liballoc/rc.rs +++ b/src/liballoc/rc.rs @@ -636,6 +636,7 @@ impl Rc { /// /// ``` /// #![feature(rc_into_raw_non_null)] + /// #![allow(deprecated)] /// /// use std::rc::Rc; /// @@ -645,6 +646,7 @@ impl Rc { /// assert_eq!(deref, "hello"); /// ``` #[unstable(feature = "rc_into_raw_non_null", issue = "47336")] + #[rustc_deprecated(since = "1.44.0", reason = "use `Rc::into_raw` instead")] #[inline] pub fn into_raw_non_null(this: Self) -> NonNull { // safe because Rc guarantees its pointer is non-null diff --git a/src/liballoc/sync.rs b/src/liballoc/sync.rs index 59bc8686cf4f9..34372b18254ac 100644 --- a/src/liballoc/sync.rs +++ b/src/liballoc/sync.rs @@ -635,6 +635,7 @@ impl Arc { /// /// ``` /// #![feature(rc_into_raw_non_null)] + /// #![allow(deprecated)] /// /// use std::sync::Arc; /// @@ -644,6 +645,7 @@ impl Arc { /// assert_eq!(deref, "hello"); /// ``` #[unstable(feature = "rc_into_raw_non_null", issue = "47336")] + #[rustc_deprecated(since = "1.44.0", reason = "use `Rc::into_raw` instead")] #[inline] pub fn into_raw_non_null(this: Self) -> NonNull { // safe because Arc guarantees its pointer is non-null From 9a1c7dba32e13184f4c5c3ad914d536f9799c524 Mon Sep 17 00:00:00 2001 From: Simon Sapin Date: Wed, 15 Apr 2020 18:32:56 +0200 Subject: [PATCH 3/4] Apply suggestions from code review Co-Authored-By: Ralf Jung --- src/liballoc/boxed.rs | 2 +- src/liballoc/sync.rs | 2 +- 2 files changed, 2 insertions(+), 2 deletions(-) diff --git a/src/liballoc/boxed.rs b/src/liballoc/boxed.rs index 567fd625582b9..1195805e6ea46 100644 --- a/src/liballoc/boxed.rs +++ b/src/liballoc/boxed.rs @@ -481,7 +481,7 @@ impl Box { #[unstable( feature = "ptr_internals", issue = "none", - reason = "use `Box::leak(b).into()` or `NonNull::from(Box::leak(b))` instead" + reason = "use `Box::leak(b).into()` or `Unique::from(Box::leak(b))` instead" )] #[inline] #[doc(hidden)] diff --git a/src/liballoc/sync.rs b/src/liballoc/sync.rs index 34372b18254ac..c05af4c7e5179 100644 --- a/src/liballoc/sync.rs +++ b/src/liballoc/sync.rs @@ -645,7 +645,7 @@ impl Arc { /// assert_eq!(deref, "hello"); /// ``` #[unstable(feature = "rc_into_raw_non_null", issue = "47336")] - #[rustc_deprecated(since = "1.44.0", reason = "use `Rc::into_raw` instead")] + #[rustc_deprecated(since = "1.44.0", reason = "use `Arc::into_raw` instead")] #[inline] pub fn into_raw_non_null(this: Self) -> NonNull { // safe because Arc guarantees its pointer is non-null From 7709d205ddcea48294f7916a3d7eb6d843bb6dbc Mon Sep 17 00:00:00 2001 From: Simon Sapin Date: Wed, 15 Apr 2020 18:38:22 +0200 Subject: [PATCH 4/4] Implement `Box::into_raw` based on `Box::leak` MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit … instead of the other way around. --- src/liballoc/boxed.rs | 27 +++++++++++++++++---------- 1 file changed, 17 insertions(+), 10 deletions(-) diff --git a/src/liballoc/boxed.rs b/src/liballoc/boxed.rs index 1195805e6ea46..3d657396a9feb 100644 --- a/src/liballoc/boxed.rs +++ b/src/liballoc/boxed.rs @@ -428,15 +428,12 @@ impl Box { #[stable(feature = "box_raw", since = "1.4.0")] #[inline] pub fn into_raw(b: Box) -> *mut T { - let b = mem::ManuallyDrop::new(b); - let mut unique = b.0; - // Box is kind-of a library type, but recognized as a "unique pointer" by - // Stacked Borrows. This function here corresponds to "reborrowing to - // a raw pointer", but there is no actual reborrow here -- so - // without some care, the pointer we are returning here still carries - // the tag of `b`, with `Unique` permission. - // We round-trip through a mutable reference to avoid that. - unsafe { unique.as_mut() as *mut T } + // Box is recognized as a "unique pointer" by Stacked Borrows, but internally it is a + // raw pointer for the type system. Turning it directly into a raw pointer would not be + // recognized as "releasing" the unique pointer to permit aliased raw accesses, + // so all raw pointer methods go through `leak` which creates a (unique) + // mutable reference. Turning *that* to a raw pointer behaves correctly. + Box::leak(b) as *mut T } /// Consumes the `Box`, returning the wrapped pointer as `NonNull`. @@ -475,6 +472,11 @@ impl Box { )] #[inline] pub fn into_raw_non_null(b: Box) -> NonNull { + // Box is recognized as a "unique pointer" by Stacked Borrows, but internally it is a + // raw pointer for the type system. Turning it directly into a raw pointer would not be + // recognized as "releasing" the unique pointer to permit aliased raw accesses, + // so all raw pointer methods go through `leak` which creates a (unique) + // mutable reference. Turning *that* to a raw pointer behaves correctly. Box::leak(b).into() } @@ -486,6 +488,11 @@ impl Box { #[inline] #[doc(hidden)] pub fn into_unique(b: Box) -> Unique { + // Box is recognized as a "unique pointer" by Stacked Borrows, but internally it is a + // raw pointer for the type system. Turning it directly into a raw pointer would not be + // recognized as "releasing" the unique pointer to permit aliased raw accesses, + // so all raw pointer methods go through `leak` which creates a (unique) + // mutable reference. Turning *that* to a raw pointer behaves correctly. Box::leak(b).into() } @@ -532,7 +539,7 @@ impl Box { where T: 'a, // Technically not needed, but kept to be explicit. { - unsafe { &mut *Box::into_raw(b) } + unsafe { &mut *mem::ManuallyDrop::new(b).0.as_ptr() } } /// Converts a `Box` into a `Pin>`