From 2b718e8d9ca6efdccf4d72647cb3d3101e15b7fa Mon Sep 17 00:00:00 2001 From: Trevor Spiteri Date: Sat, 4 Apr 2020 14:24:26 +0200 Subject: [PATCH] use ManuallyDrop instead of forget inside collections This commit changes some usage of mem::forget into mem::ManuallyDrop in some Vec, VecDeque, BTreeMap and Box methods. Before the commit, the generated IR for some of the methods was longer, and even after optimization, some unwinding artifacts were still present. --- src/liballoc/boxed.rs | 2 +- src/liballoc/collections/btree/map.rs | 16 ++++++++-------- src/liballoc/collections/vec_deque.rs | 11 +++++------ src/liballoc/raw_vec.rs | 16 +++++++--------- src/liballoc/vec.rs | 27 +++++++++++++-------------- 5 files changed, 34 insertions(+), 38 deletions(-) diff --git a/src/liballoc/boxed.rs b/src/liballoc/boxed.rs index 5406956a5288..db7420954a09 100644 --- a/src/liballoc/boxed.rs +++ b/src/liballoc/boxed.rs @@ -469,8 +469,8 @@ impl Box { #[inline] #[doc(hidden)] pub fn into_unique(b: Box) -> Unique { + let b = mem::ManuallyDrop::new(b); let mut unique = b.0; - mem::forget(b); // 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 diff --git a/src/liballoc/collections/btree/map.rs b/src/liballoc/collections/btree/map.rs index bbeced1751d1..36770ee416cb 100644 --- a/src/liballoc/collections/btree/map.rs +++ b/src/liballoc/collections/btree/map.rs @@ -4,9 +4,10 @@ use core::fmt::Debug; use core::hash::{Hash, Hasher}; use core::iter::{FromIterator, FusedIterator, Peekable}; use core::marker::PhantomData; +use core::mem::{self, ManuallyDrop}; use core::ops::Bound::{Excluded, Included, Unbounded}; use core::ops::{Index, RangeBounds}; -use core::{fmt, mem, ptr}; +use core::{fmt, ptr}; use super::node::{self, marker, ForceResult::*, Handle, InsertResult::*, NodeRef}; use super::search::{self, SearchResult::*}; @@ -190,9 +191,9 @@ impl Clone for BTreeMap { // We can't destructure subtree directly // because BTreeMap implements Drop let (subroot, sublength) = unsafe { + let subtree = ManuallyDrop::new(subtree); let root = ptr::read(&subtree.root); let length = subtree.length; - mem::forget(subtree); (root, length) }; @@ -1515,15 +1516,14 @@ impl IntoIterator for BTreeMap { type IntoIter = IntoIter; fn into_iter(self) -> IntoIter { - if self.root.is_none() { - mem::forget(self); + let me = ManuallyDrop::new(self); + if me.root.is_none() { return IntoIter { front: None, back: None, length: 0 }; } - let root1 = unsafe { unwrap_unchecked(ptr::read(&self.root)).into_ref() }; - let root2 = unsafe { unwrap_unchecked(ptr::read(&self.root)).into_ref() }; - let len = self.length; - mem::forget(self); + let root1 = unsafe { unwrap_unchecked(ptr::read(&me.root)).into_ref() }; + let root2 = unsafe { unwrap_unchecked(ptr::read(&me.root)).into_ref() }; + let len = me.length; IntoIter { front: Some(root1.first_leaf_edge()), diff --git a/src/liballoc/collections/vec_deque.rs b/src/liballoc/collections/vec_deque.rs index 94532521a906..c17a37c7bae1 100644 --- a/src/liballoc/collections/vec_deque.rs +++ b/src/liballoc/collections/vec_deque.rs @@ -12,7 +12,7 @@ use core::cmp::{self, Ordering}; use core::fmt; use core::hash::{Hash, Hasher}; use core::iter::{once, repeat_with, FromIterator, FusedIterator}; -use core::mem::{self, replace}; +use core::mem::{self, replace, ManuallyDrop}; use core::ops::Bound::{Excluded, Included, Unbounded}; use core::ops::{Index, IndexMut, RangeBounds, Try}; use core::ptr::{self, NonNull}; @@ -2898,12 +2898,12 @@ impl From> for VecDeque { /// This avoids reallocating where possible, but the conditions for that are /// strict, and subject to change, and so shouldn't be relied upon unless the /// `Vec` came from `From>` and hasn't been reallocated. - fn from(mut other: Vec) -> Self { + fn from(other: Vec) -> Self { unsafe { + let mut other = ManuallyDrop::new(other); let other_buf = other.as_mut_ptr(); let mut buf = RawVec::from_raw_parts(other_buf, other.capacity()); let len = other.len(); - mem::forget(other); // We need to extend the buf if it's not a power of two, too small // or doesn't have at least one free space @@ -2955,6 +2955,7 @@ impl From> for Vec { other.make_contiguous(); unsafe { + let other = ManuallyDrop::new(other); let buf = other.buf.ptr(); let len = other.len(); let cap = other.cap(); @@ -2962,9 +2963,7 @@ impl From> for Vec { if other.head != 0 { ptr::copy(buf.add(other.tail), buf, len); } - let out = Vec::from_raw_parts(buf, len, cap); - mem::forget(other); - out + Vec::from_raw_parts(buf, len, cap) } } } diff --git a/src/liballoc/raw_vec.rs b/src/liballoc/raw_vec.rs index 2bf40490e781..12e32fd9d352 100644 --- a/src/liballoc/raw_vec.rs +++ b/src/liballoc/raw_vec.rs @@ -3,7 +3,7 @@ use core::alloc::MemoryBlock; use core::cmp; -use core::mem::{self, MaybeUninit}; +use core::mem::{self, ManuallyDrop, MaybeUninit}; use core::ops::Drop; use core::ptr::{NonNull, Unique}; use core::slice; @@ -112,11 +112,10 @@ impl RawVec { } /// Converts a `Box<[T]>` into a `RawVec`. - pub fn from_box(mut slice: Box<[T]>) -> Self { + pub fn from_box(slice: Box<[T]>) -> Self { unsafe { - let result = RawVec::from_raw_parts(slice.as_mut_ptr(), slice.len()); - mem::forget(slice); - result + let mut slice = ManuallyDrop::new(slice); + RawVec::from_raw_parts(slice.as_mut_ptr(), slice.len()) } } } @@ -579,11 +578,10 @@ impl RawVec { "`len` must be smaller than or equal to `self.capacity()`" ); + let me = ManuallyDrop::new(self); // NOTE: not calling `capacity()` here; actually using the real `cap` field! - let slice = slice::from_raw_parts_mut(self.ptr() as *mut MaybeUninit, len); - let output = Box::from_raw(slice); - mem::forget(self); - output + let slice = slice::from_raw_parts_mut(me.ptr() as *mut MaybeUninit, len); + Box::from_raw(slice) } } diff --git a/src/liballoc/vec.rs b/src/liballoc/vec.rs index 96a6399d0518..80574efe0627 100644 --- a/src/liballoc/vec.rs +++ b/src/liballoc/vec.rs @@ -66,7 +66,7 @@ use core::hash::{self, Hash}; use core::intrinsics::{arith_offset, assume}; use core::iter::{FromIterator, FusedIterator, TrustedLen}; use core::marker::PhantomData; -use core::mem; +use core::mem::{self, ManuallyDrop}; use core::ops::Bound::{Excluded, Included, Unbounded}; use core::ops::{self, Index, IndexMut, RangeBounds}; use core::ptr::{self, NonNull}; @@ -392,7 +392,7 @@ impl Vec { /// ``` #[unstable(feature = "vec_into_raw_parts", reason = "new API", issue = "65816")] pub fn into_raw_parts(self) -> (*mut T, usize, usize) { - let mut me = mem::ManuallyDrop::new(self); + let mut me = ManuallyDrop::new(self); (me.as_mut_ptr(), me.len(), me.capacity()) } @@ -678,9 +678,9 @@ impl Vec { pub fn into_boxed_slice(mut self) -> Box<[T]> { unsafe { self.shrink_to_fit(); - let buf = ptr::read(&self.buf); - let len = self.len(); - mem::forget(self); + let me = ManuallyDrop::new(self); + let buf = ptr::read(&me.buf); + let len = me.len(); buf.into_box(len).assume_init() } } @@ -1949,16 +1949,16 @@ impl IntoIterator for Vec { /// } /// ``` #[inline] - fn into_iter(mut self) -> IntoIter { + fn into_iter(self) -> IntoIter { unsafe { - let begin = self.as_mut_ptr(); + let mut me = ManuallyDrop::new(self); + let begin = me.as_mut_ptr(); let end = if mem::size_of::() == 0 { - arith_offset(begin as *const i8, self.len() as isize) as *const T + arith_offset(begin as *const i8, me.len() as isize) as *const T } else { - begin.add(self.len()) as *const T + begin.add(me.len()) as *const T }; - let cap = self.buf.capacity(); - mem::forget(self); + let cap = me.buf.capacity(); IntoIter { buf: NonNull::new_unchecked(begin), phantom: PhantomData, @@ -2081,9 +2081,8 @@ impl SpecExtend> for Vec { // has not been advanced at all. if iterator.buf.as_ptr() as *const _ == iterator.ptr { unsafe { - let vec = Vec::from_raw_parts(iterator.buf.as_ptr(), iterator.len(), iterator.cap); - mem::forget(iterator); - vec + let it = ManuallyDrop::new(iterator); + Vec::from_raw_parts(it.buf.as_ptr(), it.len(), it.cap) } } else { let mut vector = Vec::new();