Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Use MaybeUninit for storage of inline items #162

Merged
merged 2 commits into from
Oct 19, 2019
Merged
Changes from 1 commit
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
Next Next commit
Use MaybeUninit for storage of inline items.
This includes two breaking changes, in addition to the fact that it will
require a MSRV bump:

1. The functions on the `Array` trait `ptr` and `ptr_mut` have been
   removed. Because these took a `&self`/`&mut self` argument, there's
   no way for us to call them when we only have a `MaybeUninit<A>`. Now,
   we just use the memory of the object directly.

   This limits the flexibility of custom implementations of `Array`,
   (they can no longer return pointers to values other than themselves)
   but I imagine this is very rare and was probably broken somehow to
   begin with. Anybody who does this will get a compile error.

2. `from_buf_and_len_unchecked` now takes a MaybeUninit<A>, so that
   callers have the option of only partially initializing the array.
  • Loading branch information
Thom Chiovoloni committed Oct 18, 2019
commit a83ccecf515b5273e4dd0306c1b9f6dcb7daf963
101 changes: 49 additions & 52 deletions lib.rs
Original file line number Diff line number Diff line change
Expand Up @@ -63,7 +63,7 @@ use std::iter::{repeat, FromIterator, IntoIterator};
#[cfg(feature = "serde")]
use std::marker::PhantomData;
use std::mem;
use std::mem::ManuallyDrop;
use std::mem::MaybeUninit;
use std::ops;
use std::ptr;
use std::slice;
Expand Down Expand Up @@ -280,29 +280,27 @@ impl<'a, T: 'a> Drop for Drain<'a, T> {

#[cfg(feature = "union")]
union SmallVecData<A: Array> {
inline: ManuallyDrop<A>,
inline: MaybeUninit<A>,
heap: (*mut A::Item, usize),
}

#[cfg(feature = "union")]
impl<A: Array> SmallVecData<A> {
#[inline]
unsafe fn inline(&self) -> &A {
&self.inline
unsafe fn inline(&self) -> *const A::Item {
self.inline.as_ptr() as *const A::Item
}
#[inline]
unsafe fn inline_mut(&mut self) -> &mut A {
&mut self.inline
unsafe fn inline_mut(&mut self) -> *mut A::Item {
self.inline.as_mut_ptr() as *mut A::Item
}
#[inline]
fn from_inline(inline: A) -> SmallVecData<A> {
SmallVecData {
inline: ManuallyDrop::new(inline),
}
fn from_inline(inline: MaybeUninit<A>) -> SmallVecData<A> {
SmallVecData { inline }
}
#[inline]
unsafe fn into_inline(self) -> A {
ManuallyDrop::into_inner(self.inline)
unsafe fn into_inline(self) -> MaybeUninit<A> {
self.inline
}
#[inline]
unsafe fn heap(&self) -> (*mut A::Item, usize) {
Expand All @@ -320,34 +318,34 @@ impl<A: Array> SmallVecData<A> {

#[cfg(not(feature = "union"))]
enum SmallVecData<A: Array> {
Inline(ManuallyDrop<A>),
Inline(MaybeUninit<A>),
Heap((*mut A::Item, usize)),
}

#[cfg(not(feature = "union"))]
impl<A: Array> SmallVecData<A> {
#[inline]
unsafe fn inline(&self) -> &A {
unsafe fn inline(&self) -> *const A::Item {
match *self {
SmallVecData::Inline(ref a) => a,
SmallVecData::Inline(ref a) => a.as_ptr() as *const A::Item,
_ => debug_unreachable!(),
}
}
#[inline]
unsafe fn inline_mut(&mut self) -> &mut A {
unsafe fn inline_mut(&mut self) -> *mut A::Item {
match *self {
SmallVecData::Inline(ref mut a) => a,
SmallVecData::Inline(ref mut a) => a.as_mut_ptr() as *mut A::Item,
_ => debug_unreachable!(),
}
}
#[inline]
fn from_inline(inline: A) -> SmallVecData<A> {
SmallVecData::Inline(ManuallyDrop::new(inline))
fn from_inline(inline: MaybeUninit<A>) -> SmallVecData<A> {
SmallVecData::Inline(inline)
}
#[inline]
unsafe fn into_inline(self) -> A {
unsafe fn into_inline(self) -> MaybeUninit<A> {
match self {
SmallVecData::Inline(a) => ManuallyDrop::into_inner(a),
SmallVecData::Inline(a) => a,
_ => debug_unreachable!(),
}
}
Expand Down Expand Up @@ -412,11 +410,15 @@ impl<A: Array> SmallVec<A> {
/// Construct an empty vector
#[inline]
pub fn new() -> SmallVec<A> {
unsafe {
SmallVec {
capacity: 0,
data: SmallVecData::from_inline(mem::uninitialized()),
}
// Try to detect invalid custom implementations of `Array`. Hopefuly,
// this check should be optimized away entirely for valid ones.
assert!(
mem::size_of::<A>() == A::size() * mem::size_of::<A::Item>()
&& mem::align_of::<A>() >= mem::align_of::<A::Item>()
);
SmallVec {
capacity: 0,
data: SmallVecData::from_inline(MaybeUninit::uninit()),
}
}

Expand Down Expand Up @@ -456,10 +458,10 @@ impl<A: Array> SmallVec<A> {
pub fn from_vec(mut vec: Vec<A::Item>) -> SmallVec<A> {
if vec.capacity() <= A::size() {
unsafe {
let mut data = SmallVecData::<A>::from_inline(mem::uninitialized());
let mut data = SmallVecData::<A>::from_inline(MaybeUninit::uninit());
let len = vec.len();
vec.set_len(0);
ptr::copy_nonoverlapping(vec.as_ptr(), data.inline_mut().ptr_mut(), len);
ptr::copy_nonoverlapping(vec.as_ptr(), data.inline_mut(), len);

SmallVec {
capacity: len,
Expand Down Expand Up @@ -492,7 +494,7 @@ impl<A: Array> SmallVec<A> {
pub fn from_buf(buf: A) -> SmallVec<A> {
SmallVec {
capacity: A::size(),
data: SmallVecData::from_inline(buf),
data: SmallVecData::from_inline(MaybeUninit::new(buf)),
}
}

Expand All @@ -511,7 +513,7 @@ impl<A: Array> SmallVec<A> {
#[inline]
pub fn from_buf_and_len(buf: A, len: usize) -> SmallVec<A> {
assert!(len <= A::size());
unsafe { SmallVec::from_buf_and_len_unchecked(buf, len) }
unsafe { SmallVec::from_buf_and_len_unchecked(MaybeUninit::new(buf), len) }
}

/// Constructs a new `SmallVec` on the stack from an `A` without
Expand All @@ -520,16 +522,17 @@ impl<A: Array> SmallVec<A> {
///
/// ```rust
/// use smallvec::SmallVec;
/// use std::mem::MaybeUninit;
///
/// let buf = [1, 2, 3, 4, 5, 0, 0, 0];
/// let small_vec: SmallVec<_> = unsafe {
/// SmallVec::from_buf_and_len_unchecked(buf, 5)
/// SmallVec::from_buf_and_len_unchecked(MaybeUninit::new(buf), 5)
/// };
///
/// assert_eq!(&*small_vec, &[1, 2, 3, 4, 5]);
/// ```
#[inline]
pub unsafe fn from_buf_and_len_unchecked(buf: A, len: usize) -> SmallVec<A> {
pub unsafe fn from_buf_and_len_unchecked(buf: MaybeUninit<A>, len: usize) -> SmallVec<A> {
SmallVec {
capacity: len,
data: SmallVecData::from_inline(buf),
Expand Down Expand Up @@ -579,7 +582,7 @@ impl<A: Array> SmallVec<A> {
let (ptr, len) = self.data.heap();
(ptr, len, self.capacity)
} else {
(self.data.inline().ptr(), self.capacity, A::size())
(self.data.inline(), self.capacity, A::size())
}
}
}
Expand All @@ -592,11 +595,7 @@ impl<A: Array> SmallVec<A> {
let &mut (ptr, ref mut len_ptr) = self.data.heap_mut();
(ptr, len_ptr, self.capacity)
} else {
(
self.data.inline_mut().ptr_mut(),
&mut self.capacity,
A::size(),
)
(self.data.inline_mut(), &mut self.capacity, A::size())
}
}
}
Expand Down Expand Up @@ -663,8 +662,8 @@ impl<A: Array> SmallVec<A> {
if unspilled {
return;
}
self.data = SmallVecData::from_inline(mem::uninitialized());
ptr::copy_nonoverlapping(ptr, self.data.inline_mut().ptr_mut(), len);
self.data = SmallVecData::from_inline(MaybeUninit::uninit());
ptr::copy_nonoverlapping(ptr, self.data.inline_mut(), len);
self.capacity = len;
} else if new_cap != cap {
let mut vec = Vec::with_capacity(new_cap);
Expand Down Expand Up @@ -730,8 +729,8 @@ impl<A: Array> SmallVec<A> {
if self.inline_size() >= len {
unsafe {
let (ptr, len) = self.data.heap();
self.data = SmallVecData::from_inline(mem::uninitialized());
ptr::copy_nonoverlapping(ptr, self.data.inline_mut().ptr_mut(), len);
self.data = SmallVecData::from_inline(MaybeUninit::uninit());
ptr::copy_nonoverlapping(ptr, self.data.inline_mut(), len);
deallocate(ptr, self.capacity);
self.capacity = len;
}
Expand Down Expand Up @@ -900,7 +899,7 @@ impl<A: Array> SmallVec<A> {
unsafe {
let data = ptr::read(&self.data);
mem::forget(self);
Ok(data.into_inline())
Ok(data.into_inline().assume_init())
}
}
}
Expand Down Expand Up @@ -1062,8 +1061,12 @@ where
SmallVec {
capacity: len,
data: SmallVecData::from_inline(unsafe {
let mut data: A = mem::uninitialized();
ptr::copy_nonoverlapping(slice.as_ptr(), data.ptr_mut(), len);
let mut data: MaybeUninit<A> = MaybeUninit::uninit();
ptr::copy_nonoverlapping(
slice.as_ptr(),
data.as_mut_ptr() as *mut A::Item,
len,
);
data
}),
}
Expand Down Expand Up @@ -1603,10 +1606,6 @@ pub unsafe trait Array {
type Item;
/// Returns the number of items the array can hold.
fn size() -> usize;
/// Returns a pointer to the first element of the array.
fn ptr(&self) -> *const Self::Item;
/// Returns a mutable pointer to the first element of the array.
fn ptr_mut(&mut self) -> *mut Self::Item;
}

/// Set the length of the vec when the `SetLenOnDrop` value goes out of scope.
Expand Down Expand Up @@ -1650,8 +1649,6 @@ macro_rules! impl_array(
unsafe impl<T> Array for [T; $size] {
type Item = T;
fn size() -> usize { $size }
fn ptr(&self) -> *const T { self.as_ptr() }
fn ptr_mut(&mut self) -> *mut T { self.as_mut_ptr() }
}
)+
}
Expand Down Expand Up @@ -1985,7 +1982,7 @@ mod tests {
);
}

#[cfg(feature = "std")]
#[cfg(all(feature = "std", not(miri)))] // Miri currently does not support unwinding
#[test]
// https://github.com/servo/rust-smallvec/issues/96
fn test_insert_many_panic() {
Expand Down