Auto merge of #162 - thomcc:maybe-uninit, r=emilio

Use MaybeUninit for storage of inline items

Fixes #126

I didn't see a PR open for this, and was poking around so I figured I'd do it. Sorry if I stepped on anyone's toes.

This should remove the use of mem::uninitialized, and ensure that all values used directly or via references are initialized. (e.g. T and &T are always initialized, but `*const T` and `*mut T` may not be, which is fine).

With these changes, `miri` passes. I also changed the CI to run `miri`, hopefully.

That said, this includes three breaking changes, so it will require a min version 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`, as they can no longer return pointers to values other than themselves, but hopefully nobody does that (IMO there's a pretty good case for sealing `Array`...)

2. `SmallVec::from_buf_and_len_unchecked` now takes a MaybeUninit<A> and not an A. This means that callers have the option of only partially initializing said array without risking nasal demons. I can undo this change if desired, as it's

3. Rust 1.36.0 is required to use MaybeUninit, so this bumps the MSRV from 1.20.0 to 1.36.0. It seems tricky to make the use of MaybeUninit optional here, at least without a bunch or work (ignoring the fact that doing so would require adding a build.rs which parses `rustc -Vv` to tell us the version).

<!-- Reviewable:start -->
---
This change is [<img src="https://reviewable.io/review_button.svg" height="34" align="absmiddle" alt="Reviewable"/>](https://reviewable.io/reviews/servo/rust-smallvec/162)
<!-- Reviewable:end -->

Author: bors-servo

.travis.yml

@@ -1,6 +1,6 @@
 language: rust
 rust:
-  - 1.20.0
+  - 1.36.0
   - nightly
   - beta
   - stable
@@ -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)

lib.rs

@@ -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;
@@ -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) {
@@ -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!(),
         }
     }
@@ -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()),
         }
     }
 
@@ -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,
@@ -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)),
         }
     }
 
@@ -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
@@ -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),
@@ -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())
             }
         }
     }
@@ -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())
             }
         }
     }
@@ -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);
@@ -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;
             }
@@ -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())
             }
         }
     }
@@ -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
                 }),
             }
@@ -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.
@@ -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() }
             }
         )+
     }
@@ -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() {

scripts/run_miri.sh

@@ -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