From 4184396f28d5612520f7b30718df9fff6918d239 Mon Sep 17 00:00:00 2001 From: Dan Schatzberg Date: Thu, 4 Sep 2014 15:25:23 -0400 Subject: [PATCH 1/4] Add lifetime bounds on Items and MutItems. This also requires a fix for Vec's MoveItems. This resolves issue #16941 --- src/libcollections/vec.rs | 56 +++++++++++++++++++++++++++++---------- src/libcore/slice.rs | 4 +-- 2 files changed, 44 insertions(+), 16 deletions(-) diff --git a/src/libcollections/vec.rs b/src/libcollections/vec.rs index 9dc122cfc7dc2..6fdf2fce0a244 100644 --- a/src/libcollections/vec.rs +++ b/src/libcollections/vec.rs @@ -845,11 +845,12 @@ impl Vec { #[inline] pub fn into_iter(self) -> MoveItems { unsafe { - let iter = mem::transmute(self.as_slice().iter()); let ptr = self.ptr; let cap = self.cap; + let begin = self.ptr as *const T; + let end = (self.ptr as uint + self.len()) as *const T; mem::forget(self); - MoveItems { allocation: ptr, cap: cap, iter: iter } + MoveItems { allocation: ptr, cap: cap, ptr: begin, end: end } } } @@ -1773,7 +1774,8 @@ impl MutableSeq for Vec { pub struct MoveItems { allocation: *mut T, // the block of memory allocated for the vector cap: uint, // the capacity of the vector - iter: Items<'static, T> + ptr: *const T, + end: *const T } impl MoveItems { @@ -1793,17 +1795,33 @@ impl Iterator for MoveItems { #[inline] fn next<'a>(&'a mut self) -> Option { unsafe { - // Unsafely transmute from Items<'static, T> to Items<'a, - // T> because otherwise the type checker requires that T - // be bounded by 'static. - let iter: &mut Items<'a, T> = mem::transmute(&mut self.iter); - iter.next().map(|x| ptr::read(x)) + if self.ptr == self.end { + None + } else { + if mem::size_of::() == 0 { + // purposefully don't use 'ptr.offset' because for + // vectors with 0-size elements this would return the + // same pointer. + self.ptr = mem::transmute(self.ptr as uint + 1); + + // Use a non-null pointer value + Some(ptr::read(mem::transmute(1u))) + } else { + let old = self.ptr; + self.ptr = self.ptr.offset(1); + + Some(ptr::read(old)) + } + } } } #[inline] fn size_hint(&self) -> (uint, Option) { - self.iter.size_hint() + let diff = (self.end as uint) - (self.ptr as uint); + let size = mem::size_of::(); + let exact = diff / (if size == 0 {1} else {size}); + (exact, Some(exact)) } } @@ -1811,11 +1829,21 @@ impl DoubleEndedIterator for MoveItems { #[inline] fn next_back<'a>(&'a mut self) -> Option { unsafe { - // Unsafely transmute from Items<'static, T> to Items<'a, - // T> because otherwise the type checker requires that T - // be bounded by 'static. - let iter: &mut Items<'a, T> = mem::transmute(&mut self.iter); - iter.next_back().map(|x| ptr::read(x)) + if self.end == self.ptr { + None + } else { + if mem::size_of::() == 0 { + // See above for why 'ptr.offset' isn't used + self.end = mem::transmute(self.end as uint - 1); + + // Use a non-null pointer value + Some(ptr::read(mem::transmute(1u))) + } else { + self.end = self.end.offset(-1); + + Some(ptr::read(mem::transmute(self.end))) + } + } } } } diff --git a/src/libcore/slice.rs b/src/libcore/slice.rs index a8becb315b201..6a8bea001bf1f 100644 --- a/src/libcore/slice.rs +++ b/src/libcore/slice.rs @@ -1163,7 +1163,7 @@ macro_rules! iterator { /// Immutable slice iterator #[experimental = "needs review"] -pub struct Items<'a, T> { +pub struct Items<'a, T: 'a> { ptr: *const T, end: *const T, marker: marker::ContravariantLifetime<'a> @@ -1206,7 +1206,7 @@ impl<'a, T> RandomAccessIterator<&'a T> for Items<'a, T> { /// Mutable slice iterator. #[experimental = "needs review"] -pub struct MutItems<'a, T> { +pub struct MutItems<'a, T: 'a> { ptr: *mut T, end: *mut T, marker: marker::ContravariantLifetime<'a>, From f14cb96b07517b5638bf966e6f6e30b946552912 Mon Sep 17 00:00:00 2001 From: Dan Schatzberg Date: Thu, 4 Sep 2014 17:59:28 -0400 Subject: [PATCH 2/4] Use RawPtr::offset when size_of::() > 0 --- src/libcollections/vec.rs | 6 +++++- 1 file changed, 5 insertions(+), 1 deletion(-) diff --git a/src/libcollections/vec.rs b/src/libcollections/vec.rs index 6fdf2fce0a244..b3a3609bdce1c 100644 --- a/src/libcollections/vec.rs +++ b/src/libcollections/vec.rs @@ -848,7 +848,11 @@ impl Vec { let ptr = self.ptr; let cap = self.cap; let begin = self.ptr as *const T; - let end = (self.ptr as uint + self.len()) as *const T; + let end = if mem::size_of::() == 0 { + (ptr as uint + self.len()) as *const T; + } else { + ptr.offset(self.len() as int) + }; mem::forget(self); MoveItems { allocation: ptr, cap: cap, ptr: begin, end: end } } From 0c63a4a4f58fe8f7e989fa431af860ce00ea0980 Mon Sep 17 00:00:00 2001 From: Dan Schatzberg Date: Thu, 2 Oct 2014 11:19:47 -0400 Subject: [PATCH 3/4] Add tests for MoveItems --- src/libcollections/vec.rs | 33 +++++++++++++++++++++++++++++++++ 1 file changed, 33 insertions(+) diff --git a/src/libcollections/vec.rs b/src/libcollections/vec.rs index b3a3609bdce1c..9c3842dfd9c22 100644 --- a/src/libcollections/vec.rs +++ b/src/libcollections/vec.rs @@ -2565,6 +2565,39 @@ mod tests { assert_eq!(v.map_in_place(|_| ZeroSized).as_slice(), [ZeroSized, ZeroSized].as_slice()); } + #[test] + fn test_move_items() { + let mut vec = vec!(1i, 2, 3); + let mut vec2 : Vec = vec!(); + for i in vec.move_iter() { + vec2.push(i); + } + assert!(vec2 == vec!(1i, 2, 3)); + assert!(vec.empty()); + } + + #[test] + fn test_move_items_reverse() { + let mut vec = vec!(1i, 2, 3); + let mut vec2 : Vec = vec!(); + for i in vec.move_iter().rev() { + vec2.push(i); + } + assert!(vec2 == vec!(3i, 2, 1)); + assert!(vec.empty()); + } + + #[test] + fn test_move_items_zero_sized() { + let mut vec = vec!((), (), ()); + let mut vec2 : Vec<()> = vec!(); + for i in vec.move_iter() { + vec2.push(i); + } + assert!(vec2 == vec!((), (), ())); + assert!(vec.empty()); + } + #[bench] fn bench_new(b: &mut Bencher) { b.iter(|| { From 49e593c3d6494e210c215b4d353270616450980f Mon Sep 17 00:00:00 2001 From: Dan Schatzberg Date: Thu, 2 Oct 2014 14:06:31 -0400 Subject: [PATCH 4/4] Add fixes for new lifetime bounds --- src/libcollections/btree/map.rs | 4 ++-- src/libcollections/vec.rs | 15 ++++++--------- 2 files changed, 8 insertions(+), 11 deletions(-) diff --git a/src/libcollections/btree/map.rs b/src/libcollections/btree/map.rs index b0ba225462150..ab1605954337e 100644 --- a/src/libcollections/btree/map.rs +++ b/src/libcollections/btree/map.rs @@ -46,12 +46,12 @@ struct AbsEntries { } /// An iterator over a BTreeMap's entries. -pub struct Entries<'a, K, V> { +pub struct Entries<'a, K: 'a, V: 'a> { inner: AbsEntries> } /// A mutable iterator over a BTreeMap's entries. -pub struct MutEntries<'a, K, V> { +pub struct MutEntries<'a, K: 'a, V: 'a> { inner: AbsEntries> } diff --git a/src/libcollections/vec.rs b/src/libcollections/vec.rs index 9c3842dfd9c22..9ada9ede4356f 100644 --- a/src/libcollections/vec.rs +++ b/src/libcollections/vec.rs @@ -849,9 +849,9 @@ impl Vec { let cap = self.cap; let begin = self.ptr as *const T; let end = if mem::size_of::() == 0 { - (ptr as uint + self.len()) as *const T; + (ptr as uint + self.len()) as *const T } else { - ptr.offset(self.len() as int) + ptr.offset(self.len() as int) as *const T }; mem::forget(self); MoveItems { allocation: ptr, cap: cap, ptr: begin, end: end } @@ -1788,7 +1788,7 @@ impl MoveItems { pub fn unwrap(mut self) -> Vec { unsafe { for _x in self { } - let MoveItems { allocation, cap, iter: _iter } = self; + let MoveItems { allocation, cap, ptr: _ptr, end: _end } = self; mem::forget(self); Vec { ptr: allocation, cap: cap, len: 0 } } @@ -2569,33 +2569,30 @@ mod tests { fn test_move_items() { let mut vec = vec!(1i, 2, 3); let mut vec2 : Vec = vec!(); - for i in vec.move_iter() { + for i in vec.into_iter() { vec2.push(i); } assert!(vec2 == vec!(1i, 2, 3)); - assert!(vec.empty()); } #[test] fn test_move_items_reverse() { let mut vec = vec!(1i, 2, 3); let mut vec2 : Vec = vec!(); - for i in vec.move_iter().rev() { + for i in vec.into_iter().rev() { vec2.push(i); } assert!(vec2 == vec!(3i, 2, 1)); - assert!(vec.empty()); } #[test] fn test_move_items_zero_sized() { let mut vec = vec!((), (), ()); let mut vec2 : Vec<()> = vec!(); - for i in vec.move_iter() { + for i in vec.into_iter() { vec2.push(i); } assert!(vec2 == vec!((), (), ())); - assert!(vec.empty()); } #[bench]