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.

Sign up for GitHub

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

Use MaybeUninit for storage of inline items #162

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
2 commits
Select commit Hold shift + click to select a range
a83ccec
Use MaybeUninit for storage of inline items.
Sep 8, 2019
28c09eb
Run miri in CI, and bump MSRV
Sep 8, 2019
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
5 changes: 3 additions & 2 deletions .travis.yml
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -1,6 +1,6 @@
language: rust
rust:
- 1.20.0
- 1.36.0
- nightly
- beta
- stable
Expand All @@ -11,4 +11,5 @@ script: |
([ $TRAVIS_RUST_VERSION != nightly ] || cargo check --verbose --no-default-features) &&
([ $TRAVIS_RUST_VERSION != nightly ] || cargo test --verbose --features union) &&
([ $TRAVIS_RUST_VERSION != nightly ] || cargo test --verbose --all-features) &&
([ $TRAVIS_RUST_VERSION != nightly ] || cargo bench --verbose bench)
([ $TRAVIS_RUST_VERSION != nightly ] || cargo bench --verbose bench) &&
([ $TRAVIS_RUST_VERSION != nightly ] || bash ./scripts/run_miri.sh)
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This is awesome -- I had adding Miri here on my TODO list but didn't get around to actually do that yet. :)

101 changes: 49 additions & 52 deletions lib.rs
View file
Open in desktop
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
21 changes: 21 additions & 0 deletions scripts/run_miri.sh
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -0,0 +1,21 @@
#!/usr/bin/bash

set -ex

# Clean out our target dir, which may have artifacts compiled by a version of
# rust different from the one we're about to download.
cargo clean

# Install and run the latest version of nightly where miri built successfully.
# Taken from: https://github.com/rust-lang/miri#running-miri-on-ci

MIRI_NIGHTLY=nightly-$(curl -s https://rust-lang.github.io/rustup-components-history/x86_64-unknown-linux-gnu/miri)
echo "Installing latest nightly with Miri: $MIRI_NIGHTLY"
rustup default "$MIRI_NIGHTLY"

rustup component add miri
cargo miri setup

cargo miri test --verbose -- -- -Zunstable-options --exclude-should-panic
cargo miri test --verbose --features union -- -- -Zunstable-options --exclude-should-panic
cargo miri test --verbose --all-features -- -- -Zunstable-options --exclude-should-panic