Merge pull request #115 from bluss/merge-0.4

Merge 0.4 branch to master

.travis.yml

@@ -19,13 +19,16 @@ matrix:
     - rust: nightly
       env:
         - NODEFAULT=1
+        - ARRAYVECTEST_ENSURE_UNION=1
     - rust: nightly
       env:
       - NODROP_FEATURES='use_needs_drop'
+      - ARRAYVECTEST_ENSURE_UNION=1
     - rust: nightly
       env:
       - FEATURES='serde use_union'
       - NODROP_FEATURES='use_union'
+      - ARRAYVECTEST_ENSURE_UNION=1
 branches:
   only:
     - master

Cargo.toml

@@ -1,6 +1,6 @@
 [package]
 name = "arrayvec"
-version = "0.4.8"
+version = "0.4.9"
 authors = ["bluss"]
 license = "MIT/Apache-2.0"
 
@@ -11,6 +11,8 @@ repository = "https://github.com/bluss/arrayvec"
 keywords = ["stack", "vector", "array", "data-structure", "no_std"]
 categories = ["data-structures", "no-std"]
 
+[build-dependencies]
+
 [dependencies]
 nodrop = { version = "0.1.12", path = "nodrop", default-features = false }
 
@@ -37,12 +39,14 @@ harness = false
 [features]
 default = ["std"]
 std = []
-use_union = []
 serde-1 = ["serde"]
 
 array-sizes-33-128 = []
 array-sizes-129-255 = []
 
+# has no effect
+use_union = []
+
 [package.metadata.docs.rs]
 features = ["serde-1"]
 

README.rst

@@ -22,6 +22,20 @@ __ https://docs.rs/arrayvec
 Recent Changes (arrayvec)
 -------------------------
 
+- 0.4.9
+
+  - Use ``union`` in the implementation on when this is detected to be supported
+    (nightly only for now). This is a better solution for treating uninitialized
+    regions correctly, and we'll use it in stable Rust as soon as we are able.
+    When this is enabled, the ``ArrayVec`` has no space overhead in its memory
+    layout, although the size of the vec should not be relied upon. (See `#114`_)
+  - ``ArrayString`` updated to not use uninitialized memory, it instead zeros its
+    backing array. This will be refined in the next version, since we
+    need to make changes to the user visible API.
+  - The ``use_union`` feature now does nothing (like its documentation foretold).
+
+.. _`#114`: https://github.com/bluss/arrayvec/pull/114
+
 - 0.4.8
 
   - Implement Clone and Debug for ``IntoIter`` by @clarcharr

build.rs

@@ -0,0 +1,79 @@
+
+use std::env;
+use std::io::Write;
+use std::process::{Command, Stdio};
+
+fn main() {
+    // we need to output *some* file to opt out of the default
+    println!("cargo:rerun-if-changed=build.rs");
+
+    detect_maybe_uninit();
+}
+
+fn detect_maybe_uninit() {
+    let has_unstable_union_with_md = probe(&maybe_uninit_code(true));
+    if has_unstable_union_with_md {
+        println!("cargo:rustc-cfg=has_manually_drop_in_union");
+        println!("cargo:rustc-cfg=has_union_feature");
+        return;
+    }
+
+    let has_stable_union_with_md = probe(&maybe_uninit_code(false));
+    if has_stable_union_with_md {
+        println!("cargo:rustc-cfg=has_manually_drop_in_union");
+    }
+}
+
+// To guard against changes in this currently unstable feature, use
+// a detection tests instead of a Rustc version and/or date test.
+fn maybe_uninit_code(use_feature: bool) -> String {
+    let feature = if use_feature { "#![feature(untagged_unions)]" } else { "" };
+
+    let code = "
+    #![allow(warnings)]
+    use std::mem::ManuallyDrop;
+
+    #[derive(Copy)]
+    pub union MaybeUninit<T> {
+        empty: (),
+        value: ManuallyDrop<T>,
+    }
+
+    impl<T> Clone for MaybeUninit<T> where T: Copy
+    {
+        fn clone(&self) -> Self { *self }
+    }
+
+    fn main() {
+        let value1 = MaybeUninit::<[i32; 3]> { empty: () };
+        let value2 = MaybeUninit { value: ManuallyDrop::new([1, 2, 3]) };
+    }
+    ";
+
+
+    [feature, code].concat()
+}
+
+/// Test if a code snippet can be compiled
+fn probe(code: &str) -> bool {
+    let rustc = env::var_os("RUSTC").unwrap_or_else(|| "rustc".into());
+    let out_dir = env::var_os("OUT_DIR").expect("environment variable OUT_DIR");
+
+    let mut child = Command::new(rustc)
+        .arg("--out-dir")
+        .arg(out_dir)
+        .arg("--emit=obj")
+        .arg("-")
+        .stdin(Stdio::piped())
+        .spawn()
+        .expect("rustc probe");
+
+    child
+        .stdin
+        .as_mut()
+        .expect("rustc stdin")
+        .write_all(code.as_bytes())
+        .expect("write rustc stdin");
+
+    child.wait().expect("rustc probe").success()
+}

src/array_string.rs

@@ -2,6 +2,7 @@ use std::borrow::Borrow;
 use std::cmp;
 use std::fmt;
 use std::hash::{Hash, Hasher};
+use std::mem;
 use std::ptr;
 use std::ops::{Deref, DerefMut};
 use std::str;
@@ -25,6 +26,7 @@ use serde::{Serialize, Deserialize, Serializer, Deserializer};
 /// if needed.
 #[derive(Copy)]
 pub struct ArrayString<A: Array<Item=u8>> {
+    // FIXME: Use Copyable union for xs when we can
     xs: A,
     len: A::Index,
 }
@@ -52,7 +54,8 @@ impl<A: Array<Item=u8>> ArrayString<A> {
     pub fn new() -> ArrayString<A> {
         unsafe {
             ArrayString {
-                xs: ::new_array(),
+                // FIXME: Use Copyable union for xs when we can
+                xs: mem::zeroed(),
                 len: Index::from(0),
             }
         }

src/lib.rs

@@ -7,14 +7,6 @@
 //!   - Optional, enabled by default
 //!   - Use libstd; disable to use `no_std` instead.
 //!
-//! - `use_union`
-//!   - Optional
-//!   - Requires Rust nightly channel
-//!   - Experimental: This flag uses nightly so it *may break* unexpectedly
-//!     at some point; since it doesn't change API this flag may also change
-//!     to do nothing in the future.
-//!   - Use the unstable feature untagged unions for the internal implementation,
-//!     which may have reduced space overhead
 //! - `serde-1`
 //!   - Optional
 //!   - Enable serialization for ArrayVec and ArrayString using serde 1.0
@@ -28,13 +20,17 @@
 //!
 #![doc(html_root_url="https://docs.rs/arrayvec/0.4/")]
 #![cfg_attr(not(feature="std"), no_std)]
-extern crate nodrop;
+#![cfg_attr(has_union_feature, feature(untagged_unions))]
+
 #[cfg(feature="serde-1")]
 extern crate serde;
 
 #[cfg(not(feature="std"))]
 extern crate core as std;
 
+#[cfg(not(has_manually_drop_in_union))]
+extern crate nodrop;
+
 use std::cmp;
 use std::iter;
 use std::mem;
@@ -53,11 +49,14 @@ use std::fmt;
 #[cfg(feature="std")]
 use std::io;
 
-#[cfg(not(feature="use_union"))]
-use nodrop::NoDrop;
 
-#[cfg(feature="use_union")]
-use std::mem::ManuallyDrop as NoDrop;
+#[cfg(has_manually_drop_in_union)]
+mod maybe_uninit;
+#[cfg(not(has_manually_drop_in_union))]
+#[path="maybe_uninit_nodrop.rs"]
+mod maybe_uninit;
+
+use maybe_uninit::MaybeUninit;
 
 #[cfg(feature="serde-1")]
 use serde::{Serialize, Deserialize, Serializer, Deserializer};
@@ -75,14 +74,6 @@ pub use array_string::ArrayString;
 pub use errors::CapacityError;
 
 
-unsafe fn new_array<A: Array>() -> A {
-    // Note: Returning an uninitialized value here only works
-    // if we can be sure the data is never used. The nullable pointer
-    // inside enum optimization conflicts with this this for example,
-    // so we need to be extra careful. See `NoDrop` enum.
-    mem::uninitialized()
-}
-
 /// A vector with a fixed capacity.
 ///
 /// The `ArrayVec` is a vector backed by a fixed size array. It keeps track of
@@ -96,7 +87,7 @@ unsafe fn new_array<A: Array>() -> A {
 ///
 /// ArrayVec can be converted into a by value iterator.
 pub struct ArrayVec<A: Array> {
-    xs: NoDrop<A>,
+    xs: MaybeUninit<A>,
     len: A::Index,
 }
 
@@ -133,7 +124,7 @@ impl<A: Array> ArrayVec<A> {
     /// ```
     pub fn new() -> ArrayVec<A> {
         unsafe {
-            ArrayVec { xs: NoDrop::new(new_array()), len: Index::from(0) }
+            ArrayVec { xs: MaybeUninit::uninitialized(), len: Index::from(0) }
         }
     }
 
@@ -565,7 +556,7 @@ impl<A: Array> ArrayVec<A> {
         let other_len = other.len();
 
         unsafe {
-            let dst = self.xs.as_mut_ptr().offset(self_len as isize);
+            let dst = self.xs.ptr_mut().offset(self_len as isize);
             ptr::copy_nonoverlapping(other.as_ptr(), dst, other_len);
             self.set_len(self_len + other_len);
         }
@@ -631,7 +622,7 @@ impl<A: Array> ArrayVec<A> {
             Err(self)
         } else {
             unsafe {
-                let array = ptr::read(&*self.xs);
+                let array = ptr::read(self.xs.ptr() as *const A);
                 mem::forget(self);
                 Ok(array)
             }
@@ -660,7 +651,7 @@ impl<A: Array> Deref for ArrayVec<A> {
     #[inline]
     fn deref(&self) -> &[A::Item] {
         unsafe {
-            slice::from_raw_parts(self.xs.as_ptr(), self.len())
+            slice::from_raw_parts(self.xs.ptr(), self.len())
         }
     }
 }
@@ -670,7 +661,7 @@ impl<A: Array> DerefMut for ArrayVec<A> {
     fn deref_mut(&mut self) -> &mut [A::Item] {
         let len = self.len();
         unsafe {
-            slice::from_raw_parts_mut(self.xs.as_mut_ptr(), len)
+            slice::from_raw_parts_mut(self.xs.ptr_mut(), len)
         }
     }
 }
@@ -686,7 +677,7 @@ impl<A: Array> DerefMut for ArrayVec<A> {
 /// ```
 impl<A: Array> From<A> for ArrayVec<A> {
     fn from(array: A) -> Self {
-        ArrayVec { xs: NoDrop::new(array), len: Index::from(A::capacity()) }
+        ArrayVec { xs: MaybeUninit::from(array), len: Index::from(A::capacity()) }
     }
 }
 

src/maybe_uninit.rs

@@ -0,0 +1,51 @@
+
+
+use array::Array;
+use std::mem::ManuallyDrop;
+
+/// A combination of ManuallyDrop and “maybe uninitialized”;
+/// this wraps a value that can be wholly or partially uninitialized;
+/// it also has no drop regardless of the type of T.
+#[derive(Copy)]
+pub union MaybeUninit<T> {
+    empty: (),
+    value: ManuallyDrop<T>,
+}
+// Why we don't use std's MaybeUninit on nightly? See the ptr method
+
+impl<T> Clone for MaybeUninit<T> where T: Copy
+{
+    fn clone(&self) -> Self { *self }
+}
+
+impl<T> MaybeUninit<T> {
+    /// Create a new MaybeUninit with uninitialized interior
+    pub unsafe fn uninitialized() -> Self {
+        MaybeUninit { empty: () }
+    }
+
+    /// Create a new MaybeUninit from the value `v`.
+    pub fn from(v: T) -> Self {
+        MaybeUninit { value: ManuallyDrop::new(v) }
+    }
+
+    // Raw pointer casts written so that we don't reference or access the
+    // uninitialized interior value
+
+    /// Return a raw pointer to the start of the interior array
+    pub fn ptr(&self) -> *const T::Item
+        where T: Array
+    {
+        // std MaybeUninit creates a &self.value reference here which is
+        // not guaranteed to be sound in our case - we will partially
+        // initialize the value, not always wholly.
+        self as *const _ as *const T::Item
+    }
+
+    /// Return a mut raw pointer to the start of the interior array
+    pub fn ptr_mut(&mut self) -> *mut T::Item
+        where T: Array
+    {
+        self as *mut _ as *mut T::Item
+    }
+}

src/maybe_uninit_nodrop.rs

@@ -0,0 +1,41 @@
+
+use array::Array;
+use nodrop::NoDrop;
+use std::mem::uninitialized;
+
+/// A combination of NoDrop and “maybe uninitialized”;
+/// this wraps a value that can be wholly or partially uninitialized.
+///
+/// NOTE: This is known to not be a good solution, but it's the one we have kept
+/// working on stable Rust. Stable improvements are encouraged, in any form,
+/// but of course we are waiting for a real, stable, MaybeUninit.
+pub struct MaybeUninit<T>(NoDrop<T>);
+// why don't we use ManuallyDrop here: It doesn't inhibit
+// enum layout optimizations that depend on T, and we support older Rust.
+
+impl<T> MaybeUninit<T> {
+    /// Create a new MaybeUninit with uninitialized interior
+    pub unsafe fn uninitialized() -> Self {
+        Self::from(uninitialized())
+    }
+
+    /// Create a new MaybeUninit from the value `v`.
+    pub fn from(v: T) -> Self {
+        MaybeUninit(NoDrop::new(v))
+    }
+
+    /// Return a raw pointer to the start of the interior array
+    pub fn ptr(&self) -> *const T::Item
+        where T: Array
+    {
+        &*self.0 as *const T as *const _
+    }
+
+    /// Return a mut raw pointer to the start of the interior array
+    pub fn ptr_mut(&mut self) -> *mut T::Item
+        where T: Array
+    {
+        &mut *self.0 as *mut T as *mut _
+    }
+}
+

tests/tests.rs

@@ -281,6 +281,14 @@ fn test_compact_size() {
     assert!(mem::size_of::<QuadArray>() <= 24);
 }
 
+#[test]
+fn test_still_works_with_option_arrayvec() {
+    type RefArray = ArrayVec<[&'static i32; 2]>;
+    let array = Some(RefArray::new());
+    assert!(array.is_some());
+    println!("{:?}", array);
+}
+
 #[test]
 fn test_drain() {
     let mut v = ArrayVec::from([0; 8]);
@@ -617,3 +625,12 @@ fn test_sizes_129_255() {
     ArrayVec::from([0u8; 255]);
 }
 
+
+#[test]
+fn test_nightly_uses_maybe_uninit() {
+    if option_env!("ARRAYVECTEST_ENSURE_UNION").map(|s| !s.is_empty()).unwrap_or(false) {
+        assert!(cfg!(has_manually_drop_in_union));
+        type ByteArray = ArrayVec<[u8; 4]>;
+        assert!(mem::size_of::<ByteArray>() == 5);
+    }
+}