From 33b6bf4bc1456b670e134b7581d2e26b8c2cff6c Mon Sep 17 00:00:00 2001 From: Huon Wilson Date: Mon, 16 Dec 2013 23:53:57 +1100 Subject: [PATCH 1/6] std::vec: remove aliasing &mut [] and &[] from shift_opt. Also, dramatically simplify it with some tasteful raw pointers, rather than treating everything as a nail with `transmute`. --- src/libstd/vec.rs | 76 +++++++++++++++++++---------------------------- 1 file changed, 30 insertions(+), 46 deletions(-) diff --git a/src/libstd/vec.rs b/src/libstd/vec.rs index be9adc91e03..404cc66a2d2 100644 --- a/src/libstd/vec.rs +++ b/src/libstd/vec.rs @@ -1621,54 +1621,38 @@ impl OwnedVector for ~[T] { } fn shift_opt(&mut self) -> Option { - unsafe { - let ln = match self.len() { - 0 => return None, - 1 => return self.pop_opt(), - 2 => { - let last = self.pop(); - let first = self.pop_opt(); - self.push(last); - return first; + match self.len() { + 0 => None, + 1 => self.pop_opt(), + 2 => { + let last = self.pop(); + let first = self.pop_opt(); + self.push(last); + first + } + len => { + unsafe { + let next_len = len - 1; + + let ptr = self.as_ptr(); + + // copy out the head element, for the moment it exists + // unsafely on the stack and as the first element of the + // vector. + let head = ptr::read_ptr(ptr); + + // Memcpy everything to the left one element (leaving the + // last element unsafely in two consecutive memory + // locations) + ptr::copy_memory(self.as_mut_ptr(), ptr.offset(1), next_len); + + // set the new length, which means the second instance of + // the last element is forgotten. + self.set_len(next_len); + + Some(head) } - x => x - }; - - let next_ln = self.len() - 1; - - // Save the last element. We're going to overwrite its position - let work_elt = self.pop(); - // We still should have room to work where what last element was - assert!(self.capacity() >= ln); - // Pretend like we have the original length so we can use - // the vector copy_memory to overwrite the hole we just made - self.set_len(ln); - - // Memcopy the head element (the one we want) to the location we just - // popped. For the moment it unsafely exists at both the head and last - // positions - { - let first_slice = self.slice(0, 1); - let last_slice = self.slice(next_ln, ln); - raw::copy_memory(cast::transmute(last_slice), first_slice); } - - // Memcopy everything to the left one element - { - let init_slice = self.slice(0, next_ln); - let tail_slice = self.slice(1, ln); - raw::copy_memory(cast::transmute(init_slice), - tail_slice); - } - - // Set the new length. Now the vector is back to normal - self.set_len(next_ln); - - // Swap out the element we want from the end - let vp = self.as_mut_ptr(); - let vp = ptr::mut_offset(vp, (next_ln - 1) as int); - - Some(ptr::replace_ptr(vp, work_elt)) } } From ad20a78c548819b6671ee11eed7501e61429575a Mon Sep 17 00:00:00 2001 From: Huon Wilson Date: Mon, 16 Dec 2013 23:30:56 +1100 Subject: [PATCH 2/6] std::vec::raw: convert init_elem to a method. --- src/libstd/vec.rs | 32 ++++++++++++++++---------------- 1 file changed, 16 insertions(+), 16 deletions(-) diff --git a/src/libstd/vec.rs b/src/libstd/vec.rs index 404cc66a2d2..3a2b7c68955 100644 --- a/src/libstd/vec.rs +++ b/src/libstd/vec.rs @@ -2053,6 +2053,13 @@ pub trait MutableVector<'a, T> { /// Unsafely sets the element in index to the value unsafe fn unsafe_set(self, index: uint, val: T); + /** + * Unchecked vector index assignment. Does not drop the + * old value and hence is only suitable when the vector + * is newly allocated. + */ + unsafe fn init_elem(self, i: uint, val: T); + /// Similar to `as_imm_buf` but passing a `*mut T` fn as_mut_buf(self, f: |*mut T, uint| -> U) -> U; } @@ -2181,6 +2188,15 @@ impl<'a,T> MutableVector<'a, T> for &'a mut [T] { *self.unsafe_mut_ref(index) = val; } + #[inline] + unsafe fn init_elem(self, i: uint, val: T) { + let mut alloc = Some(val); + self.as_mut_buf(|p, _len| { + intrinsics::move_val_init(&mut(*ptr::mut_offset(p, i as int)), + alloc.take_unwrap()); + }) + } + #[inline] fn as_mut_buf(self, f: |*mut T, uint| -> U) -> U { let Slice{ data, len } = self.repr(); @@ -2221,9 +2237,7 @@ pub unsafe fn from_buf(ptr: *T, elts: uint) -> ~[T] { /// Unsafe operations pub mod raw { use cast; - use option::Some; use ptr; - use unstable::intrinsics; use vec::{with_capacity, ImmutableVector, MutableVector}; use unstable::raw::Slice; @@ -2257,20 +2271,6 @@ pub mod raw { })) } - /** - * Unchecked vector index assignment. Does not drop the - * old value and hence is only suitable when the vector - * is newly allocated. - */ - #[inline] - pub unsafe fn init_elem(v: &mut [T], i: uint, val: T) { - let mut alloc = Some(val); - v.as_mut_buf(|p, _len| { - intrinsics::move_val_init(&mut(*ptr::mut_offset(p, i as int)), - alloc.take_unwrap()); - }) - } - /** * Constructs a vector from an unsafe pointer to a buffer * From d0ae820765dbdbb941ee7f3a08aec895616b4db5 Mon Sep 17 00:00:00 2001 From: Huon Wilson Date: Mon, 16 Dec 2013 23:35:02 +1100 Subject: [PATCH 3/6] std::vec::raw: convert copy_memory to a method. --- src/libextra/uuid.rs | 4 +--- src/libstd/vec.rs | 41 +++++++++++++++++++++-------------------- 2 files changed, 22 insertions(+), 23 deletions(-) diff --git a/src/libextra/uuid.rs b/src/libextra/uuid.rs index 10c4b8a3e2f..38a1394d339 100644 --- a/src/libextra/uuid.rs +++ b/src/libextra/uuid.rs @@ -219,9 +219,7 @@ impl Uuid { } let mut uuid = Uuid{ bytes: [0, .. 16] }; - unsafe { - vec::raw::copy_memory(uuid.bytes, b); - } + vec::bytes::copy_memory(uuid.bytes, b); Some(uuid) } diff --git a/src/libstd/vec.rs b/src/libstd/vec.rs index 3a2b7c68955..ed58185e3b0 100644 --- a/src/libstd/vec.rs +++ b/src/libstd/vec.rs @@ -2060,6 +2060,12 @@ pub trait MutableVector<'a, T> { */ unsafe fn init_elem(self, i: uint, val: T); + /// Copies data from `src` to `self` + /// + /// `self` and `src` must not overlap. Fails if `self` is + /// shorter than `src`. + unsafe fn copy_memory(self, src: &[T]); + /// Similar to `as_imm_buf` but passing a `*mut T` fn as_mut_buf(self, f: |*mut T, uint| -> U) -> U; } @@ -2197,6 +2203,16 @@ impl<'a,T> MutableVector<'a, T> for &'a mut [T] { }) } + #[inline] + unsafe fn copy_memory(self, src: &[T]) { + self.as_mut_buf(|p_dst, len_dst| { + src.as_imm_buf(|p_src, len_src| { + assert!(len_dst >= len_src) + ptr::copy_memory(p_dst, p_src, len_src) + }) + }) + } + #[inline] fn as_mut_buf(self, f: |*mut T, uint| -> U) -> U { let Slice{ data, len } = self.repr(); @@ -2238,7 +2254,7 @@ pub unsafe fn from_buf(ptr: *T, elts: uint) -> ~[T] { pub mod raw { use cast; use ptr; - use vec::{with_capacity, ImmutableVector, MutableVector}; + use vec::{with_capacity, MutableVector}; use unstable::raw::Slice; /** @@ -2288,21 +2304,6 @@ pub mod raw { dst } - /** - * Copies data from one vector to another. - * - * Copies `src` to `dst`. The source and destination may overlap. - */ - #[inline] - pub unsafe fn copy_memory(dst: &mut [T], src: &[T]) { - dst.as_mut_buf(|p_dst, len_dst| { - src.as_imm_buf(|p_src, len_src| { - assert!(len_dst >= len_src) - ptr::copy_memory(p_dst, p_src, len_src) - }) - }) - } - /** * Returns a pointer to first element in slice and adjusts * slice so it no longer contains that element. Fails if @@ -2331,7 +2332,7 @@ pub mod raw { /// Operations on `[u8]`. pub mod bytes { - use vec::raw; + use vec::MutableVector; use ptr; /// A trait for operations on mutable `[u8]`s. @@ -2358,8 +2359,8 @@ pub mod bytes { */ #[inline] pub fn copy_memory(dst: &mut [u8], src: &[u8]) { - // Bound checks are done at vec::raw::copy_memory. - unsafe { raw::copy_memory(dst, src) } + // Bound checks are done at .copy_memory. + unsafe { dst.copy_memory(src) } } /** @@ -3585,7 +3586,7 @@ mod tests { unsafe { let mut a = [1, 2, 3, 4]; let b = [1, 2, 3, 4, 5]; - raw::copy_memory(a, b); + a.copy_memory(b); } } From 5c147cc40847da033674663cedabaf515eda8b20 Mon Sep 17 00:00:00 2001 From: Huon Wilson Date: Tue, 17 Dec 2013 00:06:13 +1100 Subject: [PATCH 4/6] std::vec::bytes: remove the reference to overlapping src and dest in docs for copy_memory. &mut [u8] and &[u8] really shouldn't be overlapping at all (part of the uniqueness/aliasing guarantee of &mut), so no point in encouraging it. --- src/libstd/vec.rs | 11 ++++------- 1 file changed, 4 insertions(+), 7 deletions(-) diff --git a/src/libstd/vec.rs b/src/libstd/vec.rs index ed58185e3b0..a0c94e6810a 100644 --- a/src/libstd/vec.rs +++ b/src/libstd/vec.rs @@ -2350,13 +2350,10 @@ pub mod bytes { } } - /** - * Copies data from one vector to another. - * - * Copies `src` to `dst`. The source and destination may - * overlap. Fails if the length of `dst` is less than the length - * of `src`. - */ + /// Copies data from one vector to another. + /// + /// Copies `src` to `dst`. Fails if the length of `dst` is less + /// than the length of `src`. #[inline] pub fn copy_memory(dst: &mut [u8], src: &[u8]) { // Bound checks are done at .copy_memory. From 8a5a5922c61cecd5f305a0cd54671dbf2f2a6519 Mon Sep 17 00:00:00 2001 From: Huon Wilson Date: Tue, 17 Dec 2013 00:31:59 +1100 Subject: [PATCH 5/6] std::vec: convert .copy_memory to use copy_nonoverlapping_memory. It is required that &mut[]s are disjoint from all other &(mut)[]s, so this assumption is ok. --- src/libstd/vec.rs | 10 +++++----- 1 file changed, 5 insertions(+), 5 deletions(-) diff --git a/src/libstd/vec.rs b/src/libstd/vec.rs index a0c94e6810a..2a0f575cdde 100644 --- a/src/libstd/vec.rs +++ b/src/libstd/vec.rs @@ -2060,7 +2060,7 @@ pub trait MutableVector<'a, T> { */ unsafe fn init_elem(self, i: uint, val: T); - /// Copies data from `src` to `self` + /// Copies data from `src` to `self`. /// /// `self` and `src` must not overlap. Fails if `self` is /// shorter than `src`. @@ -2208,7 +2208,7 @@ impl<'a,T> MutableVector<'a, T> for &'a mut [T] { self.as_mut_buf(|p_dst, len_dst| { src.as_imm_buf(|p_src, len_src| { assert!(len_dst >= len_src) - ptr::copy_memory(p_dst, p_src, len_src) + ptr::copy_nonoverlapping_memory(p_dst, p_src, len_src) }) }) } @@ -2350,10 +2350,10 @@ pub mod bytes { } } - /// Copies data from one vector to another. + /// Copies data from `src` to `dst` /// - /// Copies `src` to `dst`. Fails if the length of `dst` is less - /// than the length of `src`. + /// `src` and `dst` must not overlap. Fails if the length of `dst` + /// is less than the length of `src`. #[inline] pub fn copy_memory(dst: &mut [u8], src: &[u8]) { // Bound checks are done at .copy_memory. From dd355700cf4f1fa1744cdeb165b68898fa30a9d1 Mon Sep 17 00:00:00 2001 From: Huon Wilson Date: Tue, 17 Dec 2013 08:33:50 +1100 Subject: [PATCH 6/6] std::vec: make init_elem nicer by doing fewer moves. --- src/libstd/vec.rs | 6 +----- 1 file changed, 1 insertion(+), 5 deletions(-) diff --git a/src/libstd/vec.rs b/src/libstd/vec.rs index 2a0f575cdde..8722109736c 100644 --- a/src/libstd/vec.rs +++ b/src/libstd/vec.rs @@ -2196,11 +2196,7 @@ impl<'a,T> MutableVector<'a, T> for &'a mut [T] { #[inline] unsafe fn init_elem(self, i: uint, val: T) { - let mut alloc = Some(val); - self.as_mut_buf(|p, _len| { - intrinsics::move_val_init(&mut(*ptr::mut_offset(p, i as int)), - alloc.take_unwrap()); - }) + intrinsics::move_val_init(&mut (*self.as_mut_ptr().offset(i as int)), val); } #[inline]