From 6c099e148f3b51e791159e1dc38c29712bf93181 Mon Sep 17 00:00:00 2001 From: bluss Date: Sat, 1 Dec 2018 11:58:01 +0100 Subject: [PATCH 1/7] FIX: Add associated constant for Array's capacity This isn't of much use at the moment, but future Rust features could mean we can use it. --- src/array.rs | 5 ++++- 1 file changed, 4 insertions(+), 1 deletion(-) diff --git a/src/array.rs b/src/array.rs index 8d043e4c..b6ccd017 100644 --- a/src/array.rs +++ b/src/array.rs @@ -15,9 +15,11 @@ pub unsafe trait Array { /// The array’s element type type Item; + /// The smallest type that can index and tell the length of the array. #[doc(hidden)] - /// The smallest index type that indexes the array. type Index: Index; + /// The array's element capacity + const CAPACITY: usize; #[doc(hidden)] fn as_ptr(&self) -> *const Self::Item; #[doc(hidden)] @@ -91,6 +93,7 @@ macro_rules! fix_array_impl { unsafe impl Array for [T; $len] { type Item = T; type Index = $index_type; + const CAPACITY: usize = $len; #[doc(hidden)] #[inline(always)] fn as_ptr(&self) -> *const T { self as *const _ as *const _ } From f5290c1eeac18a6c5837b9219e8e4ad4be25a91d Mon Sep 17 00:00:00 2001 From: bluss Date: Sat, 1 Dec 2018 12:01:09 +0100 Subject: [PATCH 2/7] FIX: Remove unused .as_mut_ptr() on the Array trait Raw pointer taking should go through the MaybeUninit wrappers around the arrays anyway, when it is partially uninitialized, which it often is. The remaining .as_ptr() and .as_slice() methods on Array is only used on a fully initialized array in ArrayString::from_byte_string --- src/array.rs | 5 ----- 1 file changed, 5 deletions(-) diff --git a/src/array.rs b/src/array.rs index b6ccd017..ad18c795 100644 --- a/src/array.rs +++ b/src/array.rs @@ -23,8 +23,6 @@ pub unsafe trait Array { #[doc(hidden)] fn as_ptr(&self) -> *const Self::Item; #[doc(hidden)] - fn as_mut_ptr(&mut self) -> *mut Self::Item; - #[doc(hidden)] fn capacity() -> usize; } @@ -99,9 +97,6 @@ macro_rules! fix_array_impl { fn as_ptr(&self) -> *const T { self as *const _ as *const _ } #[doc(hidden)] #[inline(always)] - fn as_mut_ptr(&mut self) -> *mut T { self as *mut _ as *mut _} - #[doc(hidden)] - #[inline(always)] fn capacity() -> usize { $len } } ) From 784ccc97ca3fcb1892a97ec53eb30ed588a8e26a Mon Sep 17 00:00:00 2001 From: bluss Date: Sat, 1 Dec 2018 12:13:37 +0100 Subject: [PATCH 3/7] DOC: Remove warning on ArrayVec's into_inner method --- src/lib.rs | 3 --- 1 file changed, 3 deletions(-) diff --git a/src/lib.rs b/src/lib.rs index 54c9b4c1..b7ed449c 100644 --- a/src/lib.rs +++ b/src/lib.rs @@ -614,9 +614,6 @@ impl ArrayVec { /// /// Return an `Ok` value with the array if length equals capacity, /// return an `Err` with self otherwise. - /// - /// `Note:` This function may incur unproportionally large overhead - /// to move the array out, its performance is not optimal. pub fn into_inner(self) -> Result { if self.len() < self.capacity() { Err(self) From c4cd63209fc9053d20236e363c63c847431a0b86 Mon Sep 17 00:00:00 2001 From: bluss Date: Sun, 25 Nov 2018 12:18:59 +0100 Subject: [PATCH 4/7] FEAT: Use a separate union MaybeUninitCopy for ArrayString This is the "real" union solution, and ArrayString can use it since its backing array is Copy. Unfortunately, we'll have to use the Copy bound on the type, making it "viral" and visible in the user API. --- src/array_string.rs | 122 ++++++++++++++++++++++++++------------- src/lib.rs | 3 + src/maybe_uninit_copy.rs | 42 ++++++++++++++ 3 files changed, 128 insertions(+), 39 deletions(-) create mode 100644 src/maybe_uninit_copy.rs diff --git a/src/array_string.rs b/src/array_string.rs index 05b33842..a52fb58c 100644 --- a/src/array_string.rs +++ b/src/array_string.rs @@ -2,7 +2,6 @@ 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; @@ -17,6 +16,8 @@ use char::encode_utf8; #[cfg(feature="serde-1")] use serde::{Serialize, Deserialize, Serializer, Deserializer}; +use super::MaybeUninitCopy; + /// A string with a fixed capacity. /// /// The `ArrayString` is a string backed by a fixed size array. It keeps track @@ -25,20 +26,25 @@ use serde::{Serialize, Deserialize, Serializer, Deserializer}; /// The string is a contiguous value that you can store directly on the stack /// if needed. #[derive(Copy)] -pub struct ArrayString> { - // FIXME: Use Copyable union for xs when we can - xs: A, +pub struct ArrayString + where A: Array + Copy +{ + xs: MaybeUninitCopy, len: A::Index, } -impl> Default for ArrayString { +impl Default for ArrayString + where A: Array + Copy +{ /// Return an empty `ArrayString` fn default() -> ArrayString { ArrayString::new() } } -impl> ArrayString { +impl ArrayString + where A: Array + Copy +{ /// Create a new empty `ArrayString`. /// /// Capacity is inferred from the type parameter. @@ -54,8 +60,7 @@ impl> ArrayString { pub fn new() -> ArrayString { unsafe { ArrayString { - // FIXME: Use Copyable union for xs when we can - xs: mem::zeroed(), + xs: MaybeUninitCopy::uninitialized(), len: Index::from(0), } } @@ -91,11 +96,12 @@ impl> ArrayString { /// let string = ArrayString::from_byte_string(b"hello world").unwrap(); /// ``` pub fn from_byte_string(b: &A) -> Result { - let mut arraystr = Self::new(); - let s = try!(str::from_utf8(b.as_slice())); - let _result = arraystr.try_push_str(s); - debug_assert!(_result.is_ok()); - Ok(arraystr) + let len = str::from_utf8(b.as_slice())?.len(); + debug_assert_eq!(len, A::capacity()); + Ok(ArrayString { + xs: MaybeUninitCopy::from(*b), + len: Index::from(A::capacity()), + }) } /// Return the capacity of the `ArrayString`. @@ -213,7 +219,7 @@ impl> ArrayString { return Err(CapacityError::new(s)); } unsafe { - let dst = self.xs.as_mut_ptr().offset(self.len() as isize); + let dst = self.xs.ptr_mut().offset(self.len() as isize); let src = s.as_ptr(); ptr::copy_nonoverlapping(src, dst, s.len()); let newl = self.len() + s.len(); @@ -307,8 +313,8 @@ impl> ArrayString { let next = idx + ch.len_utf8(); let len = self.len(); unsafe { - ptr::copy(self.xs.as_ptr().offset(next as isize), - self.xs.as_mut_ptr().offset(idx as isize), + ptr::copy(self.xs.ptr().offset(next as isize), + self.xs.ptr_mut().offset(idx as isize), len - next); self.set_len(len - (next - idx)); } @@ -342,75 +348,99 @@ impl> ArrayString { /// Return a mutable slice of the whole string’s buffer unsafe fn raw_mut_bytes(&mut self) -> &mut [u8] { - slice::from_raw_parts_mut(self.xs.as_mut_ptr(), self.capacity()) + slice::from_raw_parts_mut(self.xs.ptr_mut(), self.capacity()) } } -impl> Deref for ArrayString { +impl Deref for ArrayString + where A: Array + Copy +{ type Target = str; #[inline] fn deref(&self) -> &str { unsafe { - let sl = slice::from_raw_parts(self.xs.as_ptr(), self.len.to_usize()); + let sl = slice::from_raw_parts(self.xs.ptr(), self.len.to_usize()); str::from_utf8_unchecked(sl) } } } -impl> DerefMut for ArrayString { +impl DerefMut for ArrayString + where A: Array + Copy +{ #[inline] fn deref_mut(&mut self) -> &mut str { unsafe { - let sl = slice::from_raw_parts_mut(self.xs.as_mut_ptr(), self.len.to_usize()); + let sl = slice::from_raw_parts_mut(self.xs.ptr_mut(), self.len.to_usize()); str::from_utf8_unchecked_mut(sl) } } } -impl> PartialEq for ArrayString { +impl PartialEq for ArrayString + where A: Array + Copy +{ fn eq(&self, rhs: &Self) -> bool { **self == **rhs } } -impl> PartialEq for ArrayString { +impl PartialEq for ArrayString + where A: Array + Copy +{ fn eq(&self, rhs: &str) -> bool { &**self == rhs } } -impl> PartialEq> for str { +impl PartialEq> for str + where A: Array + Copy +{ fn eq(&self, rhs: &ArrayString) -> bool { self == &**rhs } } -impl> Eq for ArrayString { } +impl Eq for ArrayString + where A: Array + Copy +{ } -impl> Hash for ArrayString { +impl Hash for ArrayString + where A: Array + Copy +{ fn hash(&self, h: &mut H) { (**self).hash(h) } } -impl> Borrow for ArrayString { +impl Borrow for ArrayString + where A: Array + Copy +{ fn borrow(&self) -> &str { self } } -impl> AsRef for ArrayString { +impl AsRef for ArrayString + where A: Array + Copy +{ fn as_ref(&self) -> &str { self } } -impl> fmt::Debug for ArrayString { +impl fmt::Debug for ArrayString + where A: Array + Copy +{ fn fmt(&self, f: &mut fmt::Formatter) -> fmt::Result { (**self).fmt(f) } } -impl> fmt::Display for ArrayString { +impl fmt::Display for ArrayString + where A: Array + Copy +{ fn fmt(&self, f: &mut fmt::Formatter) -> fmt::Result { (**self).fmt(f) } } /// `Write` appends written data to the end of the string. -impl> fmt::Write for ArrayString { +impl fmt::Write for ArrayString + where A: Array + Copy +{ fn write_char(&mut self, c: char) -> fmt::Result { self.try_push(c).map_err(|_| fmt::Error) } @@ -420,7 +450,9 @@ impl> fmt::Write for ArrayString { } } -impl + Copy> Clone for ArrayString { +impl Clone for ArrayString + where A: Array + Copy +{ fn clone(&self) -> ArrayString { *self } @@ -431,7 +463,9 @@ impl + Copy> Clone for ArrayString { } } -impl> PartialOrd for ArrayString { +impl PartialOrd for ArrayString + where A: Array + Copy +{ fn partial_cmp(&self, rhs: &Self) -> Option { (**self).partial_cmp(&**rhs) } @@ -441,7 +475,9 @@ impl> PartialOrd for ArrayString { fn ge(&self, rhs: &Self) -> bool { **self >= **rhs } } -impl> PartialOrd for ArrayString { +impl PartialOrd for ArrayString + where A: Array + Copy +{ fn partial_cmp(&self, rhs: &str) -> Option { (**self).partial_cmp(rhs) } @@ -451,7 +487,9 @@ impl> PartialOrd for ArrayString { fn ge(&self, rhs: &str) -> bool { &**self >= rhs } } -impl> PartialOrd> for str { +impl PartialOrd> for str + where A: Array + Copy +{ fn partial_cmp(&self, rhs: &ArrayString) -> Option { self.partial_cmp(&**rhs) } @@ -461,7 +499,9 @@ impl> PartialOrd> for str { fn ge(&self, rhs: &ArrayString) -> bool { self >= &**rhs } } -impl> Ord for ArrayString { +impl Ord for ArrayString + where A: Array + Copy +{ fn cmp(&self, rhs: &Self) -> cmp::Ordering { (**self).cmp(&**rhs) } @@ -469,7 +509,9 @@ impl> Ord for ArrayString { #[cfg(feature="serde-1")] /// Requires crate feature `"serde-1"` -impl> Serialize for ArrayString { +impl Serialize for ArrayString + where A: Array + Copy +{ fn serialize(&self, serializer: S) -> Result where S: Serializer { @@ -479,7 +521,9 @@ impl> Serialize for ArrayString { #[cfg(feature="serde-1")] /// Requires crate feature `"serde-1"` -impl<'de, A: Array> Deserialize<'de> for ArrayString { +impl<'de, A> Deserialize<'de> for ArrayString + where A: Array + Copy +{ fn deserialize(deserializer: D) -> Result where D: Deserializer<'de> { @@ -488,7 +532,7 @@ impl<'de, A: Array> Deserialize<'de> for ArrayString { struct ArrayStringVisitor>(PhantomData); - impl<'de, A: Array> Visitor<'de> for ArrayStringVisitor { + impl<'de, A: Copy + Array> Visitor<'de> for ArrayStringVisitor { type Value = ArrayString; fn expecting(&self, formatter: &mut fmt::Formatter) -> fmt::Result { diff --git a/src/lib.rs b/src/lib.rs index b7ed449c..a2a5547f 100644 --- a/src/lib.rs +++ b/src/lib.rs @@ -56,7 +56,10 @@ mod maybe_uninit; #[path="maybe_uninit_nodrop.rs"] mod maybe_uninit; +mod maybe_uninit_copy; + use maybe_uninit::MaybeUninit; +use maybe_uninit_copy::MaybeUninitCopy; #[cfg(feature="serde-1")] use serde::{Serialize, Deserialize, Serializer, Deserializer}; diff --git a/src/maybe_uninit_copy.rs b/src/maybe_uninit_copy.rs new file mode 100644 index 00000000..dd08e572 --- /dev/null +++ b/src/maybe_uninit_copy.rs @@ -0,0 +1,42 @@ + +use array::Array; + +#[derive(Copy, Clone)] +pub union MaybeUninitCopy + where T: Copy +{ + empty: (), + value: T, +} + +impl MaybeUninitCopy + where T: Copy +{ + /// Create a new MaybeUninit with uninitialized interior + pub unsafe fn uninitialized() -> Self { + Self { empty: () } + } + + /// Create a new MaybeUninit from the value `v`. + pub fn from(value: T) -> Self { + Self { value } + } + + // 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 + { + 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 + } +} + From c4b95279e96f9f4cfc9dc55f2e924bd1d38abf71 Mon Sep 17 00:00:00 2001 From: bluss Date: Sat, 15 Dec 2018 14:23:12 +0100 Subject: [PATCH 5/7] MAINT: Remove feature with no effect ("use_union") --- Cargo.toml | 3 --- 1 file changed, 3 deletions(-) diff --git a/Cargo.toml b/Cargo.toml index 61a61ce5..6a45270a 100644 --- a/Cargo.toml +++ b/Cargo.toml @@ -44,9 +44,6 @@ serde-1 = ["serde"] array-sizes-33-128 = [] array-sizes-129-255 = [] -# has no effect -use_union = [] - [package.metadata.docs.rs] features = ["serde-1"] From f218e094d4f76d6f202a9674c49742ffedd0d1be Mon Sep 17 00:00:00 2001 From: bluss Date: Sat, 1 Dec 2018 12:29:46 +0100 Subject: [PATCH 6/7] MAINT: Edit travis setup for changed crate features --- .travis.yml | 33 ++++++++++++++++----------------- 1 file changed, 16 insertions(+), 17 deletions(-) diff --git a/.travis.yml b/.travis.yml index e261a0d2..62725b17 100644 --- a/.travis.yml +++ b/.travis.yml @@ -4,11 +4,10 @@ env: - FEATURES='serde-1' matrix: include: - - rust: 1.20.0 + - rust: stable - rust: stable env: - - NODEFAULT=1 - - NODROP_FEATURES='use_needs_drop' + - FEATURES='serde-1' - rust: 1.22.1 env: - FEATURES='array-sizes-33-128 array-sizes-129-255' @@ -18,29 +17,29 @@ matrix: - rust: beta - rust: nightly env: - - NODEFAULT=1 - - ARRAYVECTEST_ENSURE_UNION=1 + - ARRAYVECTEST_ENSURE_UNION=1 - rust: nightly env: - - NODROP_FEATURES='use_needs_drop' + - FEATURES='serde' - ARRAYVECTEST_ENSURE_UNION=1 - rust: nightly env: - - FEATURES='serde use_union' - - NODROP_FEATURES='use_union' + - FEATURES='serde-1' - ARRAYVECTEST_ENSURE_UNION=1 + - rust: nightly + env: + - FEATURES='array-sizes-33-128 array-sizes-129-255' branches: only: - master - 0.4 script: - | - ([ ! -z "$NODROP_FEATURES" ] || cargo build --verbose --features "$FEATURES") && - ([ "$NODEFAULT" != 1 ] || cargo build --verbose --no-default-features) && - ([ ! -z "$NODROP_FEATURES" ] || cargo test --verbose --features "$FEATURES") && - ([ ! -z "$NODROP_FEATURES" ] || cargo test --release --verbose --features "$FEATURES") && - ([ ! -z "$NODROP_FEATURES" ] || cargo bench --verbose --features "$FEATURES" -- --test) && - ([ ! -z "$NODROP_FEATURES" ] || cargo doc --verbose --features "$FEATURES") && - ([ "$NODEFAULT" != 1 ] || cargo build --verbose --manifest-path=nodrop/Cargo.toml --no-default-features) && - cargo test --verbose --manifest-path=nodrop/Cargo.toml --features "$NODROP_FEATURES" && - cargo bench --verbose --manifest-path=nodrop/Cargo.toml --features "$NODROP_FEATURES" -- --test + cargo build -v --no-default-features && + cargo build -v --features "$FEATURES" && + cargo test -v --features "$FEATURES" && + cargo test -v --release --features "$FEATURES" && + cargo bench -v --features "$FEATURES" --no-run && + cargo doc -v --features "$FEATURES" && + cargo build -v --manifest-path=nodrop/Cargo.toml && + cargo test -v --manifest-path=nodrop/Cargo.toml From 74745a9756df22e7f87695d0617790f21556b901 Mon Sep 17 00:00:00 2001 From: bluss Date: Sun, 16 Dec 2018 18:32:34 +0100 Subject: [PATCH 7/7] DOC: Update minimum rust requirement to Rust 1.24 Rust 1.24 is on debian stable. It is compatible with crossbeam (Rust 1.26), the rdep with most downloads. --- .travis.yml | 6 +++--- src/lib.rs | 2 +- 2 files changed, 4 insertions(+), 4 deletions(-) diff --git a/.travis.yml b/.travis.yml index 62725b17..018c729e 100644 --- a/.travis.yml +++ b/.travis.yml @@ -4,13 +4,13 @@ env: - FEATURES='serde-1' matrix: include: + - rust: 1.24.1 + env: + - FEATURES='array-sizes-33-128 array-sizes-129-255' - rust: stable - rust: stable env: - FEATURES='serde-1' - - rust: 1.22.1 - env: - - FEATURES='array-sizes-33-128 array-sizes-129-255' - rust: stable env: - FEATURES='array-sizes-33-128 array-sizes-129-255' diff --git a/src/lib.rs b/src/lib.rs index a2a5547f..727cc7bb 100644 --- a/src/lib.rs +++ b/src/lib.rs @@ -16,7 +16,7 @@ //! //! ## Rust Version //! -//! This version of arrayvec requires Rust 1.13 or later. +//! This version of arrayvec requires Rust 1.24 or later. //! #![doc(html_root_url="https://docs.rs/arrayvec/0.4/")] #![cfg_attr(not(feature="std"), no_std)]