From c6e7890e1312412ecf70490b3f00f674fb027f8a Mon Sep 17 00:00:00 2001 From: blake2-ppc Date: Fri, 12 Jul 2013 02:31:08 +0200 Subject: [PATCH 1/4] dlist: Fix bug in DList::merge Did not properly allow runs from the `other` list to be merged in. The test case was using a wrong expected value. Edited docs for merge so they explain more clearly what it does. Also make sure insert_ordered is marked pub. --- src/libextra/dlist.rs | 29 ++++++++++++++++++----------- 1 file changed, 18 insertions(+), 11 deletions(-) diff --git a/src/libextra/dlist.rs b/src/libextra/dlist.rs index fc6d05fcb589a..61db14316fbe9 100644 --- a/src/libextra/dlist.rs +++ b/src/libextra/dlist.rs @@ -300,19 +300,26 @@ impl DList { self.push_back(elt); } - /// Merge, using the function `f`; take `a` if `f(a, b)` is true, else `b`. + /// Merge DList `other` into this DList, using the function `f`. + /// Iterate the both DList with `a` from self and `b` from `other`, and + /// put `a` in the result if `f(a, b)` is true, else `b`. /// /// O(max(N, M)) pub fn merge(&mut self, mut other: DList, f: &fn(&T, &T) -> bool) { { let mut it = self.mut_iter(); + let mut elt = it.next(); loop { - match (it.next(), other.front()) { - (None , _ ) => break, - (_ , None ) => return, - (Some(x), Some(y)) => if f(x, y) { loop } + let take_a = match (&mut elt, other.front()) { + (_ , None) => return, + (&None, _ ) => break, + (&Some(ref mut x), Some(y)) => f(*x, y), + }; + if take_a { + elt = it.next() + } else { + it.insert_before(other.pop_front().unwrap()); } - it.insert_before(other.pop_front().unwrap()); } } self.append(other); @@ -351,11 +358,11 @@ impl DList { } } -/// Insert sorted in ascending order -/// -/// O(N) impl DList { - fn insert_ordered(&mut self, elt: T) { + /// Insert `elt` sorted in ascending order + /// + /// O(N) + pub fn insert_ordered(&mut self, elt: T) { self.insert_when(elt, |a, b| a.cmp(b) != cmp::Less); } } @@ -758,7 +765,7 @@ mod tests { assert_eq!(m.len(), len); check_links(&m); let res = m.consume_iter().collect::<~[int]>(); - assert_eq!(res, ~[-1, 0, 0, 1, 0, 3, 5, 6, 7, 2, 7, 7, 9]); + assert_eq!(res, ~[-1, 0, 0, 0, 1, 3, 5, 6, 7, 2, 7, 7, 9]); } #[test] From 89a0c99dbee1c1327e8f8a8e5127127e2b3de88e Mon Sep 17 00:00:00 2001 From: blake2-ppc Date: Fri, 12 Jul 2013 03:24:59 +0200 Subject: [PATCH 2/4] dlist: Implement DoubleEndedIterator and use for .iter() and .rev_iter() --- src/libextra/dlist.rs | 72 ++++++++++++++++++++----------------------- 1 file changed, 33 insertions(+), 39 deletions(-) diff --git a/src/libextra/dlist.rs b/src/libextra/dlist.rs index 61db14316fbe9..feafce58e6e9e 100644 --- a/src/libextra/dlist.rs +++ b/src/libextra/dlist.rs @@ -26,7 +26,7 @@ use std::cast; use std::cmp; use std::ptr; use std::util; -use std::iterator::FromIterator; +use std::iterator::{FromIterator, InvertIterator}; use container::Deque; @@ -46,17 +46,10 @@ struct Node { priv value: T, } -/// DList iterator -pub struct ForwardIterator<'self, T> { - priv list: &'self DList, - priv next: &'self Link, - priv nelem: uint, -} - -/// DList reverse iterator -pub struct ReverseIterator<'self, T> { - priv list: &'self DList, - priv next: Rawlink>, +/// Double-ended DList iterator +pub struct DListIterator<'self, T> { + priv head: &'self Link, + priv tail: Rawlink>, priv nelem: uint, } @@ -327,13 +320,13 @@ impl DList { /// Provide a forward iterator - pub fn iter<'a>(&'a self) -> ForwardIterator<'a, T> { - ForwardIterator{nelem: self.len(), list: self, next: &self.list_head} + pub fn iter<'a>(&'a self) -> DListIterator<'a, T> { + DListIterator{nelem: self.len(), head: &self.list_head, tail: self.list_tail} } /// Provide a reverse iterator - pub fn rev_iter<'a>(&'a self) -> ReverseIterator<'a, T> { - ReverseIterator{nelem: self.len(), list: self, next: self.list_tail} + pub fn rev_iter<'a>(&'a self) -> InvertIterator<&'a T, DListIterator<'a, T>> { + self.iter().invert() } /// Provide a forward iterator with mutable references @@ -367,15 +360,18 @@ impl DList { } } -impl<'self, A> Iterator<&'self A> for ForwardIterator<'self, A> { +impl<'self, A> Iterator<&'self A> for DListIterator<'self, A> { #[inline] fn next(&mut self) -> Option<&'self A> { - match *self.next { + if self.nelem == 0 { + return None; + } + match *self.head { None => None, - Some(ref next) => { + Some(ref head) => { self.nelem -= 1; - self.next = &next.next; - Some(&next.value) + self.head = &head.next; + Some(&head.value) } } } @@ -385,6 +381,22 @@ impl<'self, A> Iterator<&'self A> for ForwardIterator<'self, A> { } } +impl<'self, A> DoubleEndedIterator<&'self A> for DListIterator<'self, A> { + fn next_back(&mut self) -> Option<&'self A> { + if self.nelem == 0 { + return None; + } + match self.tail.resolve() { + None => None, + Some(prev) => { + self.nelem -= 1; + self.tail = prev.prev; + Some(&prev.value) + } + } + } +} + // MutForwardIterator is different because it implements ListInsertion, // and can modify the list during traversal, used in insert_when and merge. impl<'self, A> Iterator<&'self mut A> for MutForwardIterator<'self, A> { @@ -419,24 +431,6 @@ impl<'self, A> Iterator<&'self mut A> for MutForwardIterator<'self, A> { } } -impl<'self, A> Iterator<&'self A> for ReverseIterator<'self, A> { - #[inline] - fn next(&mut self) -> Option<&'self A> { - match self.next.resolve() { - None => None, - Some(prev) => { - self.nelem -= 1; - self.next = prev.prev; - Some(&prev.value) - } - } - } - - fn size_hint(&self) -> (uint, Option) { - (self.nelem, Some(self.nelem)) - } -} - impl<'self, A> Iterator<&'self mut A> for MutReverseIterator<'self, A> { #[inline] fn next(&mut self) -> Option<&'self mut A> { From e1d5d1c049608cf182ddc91c98d9700089a35600 Mon Sep 17 00:00:00 2001 From: blake2-ppc Date: Fri, 12 Jul 2013 03:50:05 +0200 Subject: [PATCH 3/4] dlist: Use DoubleEndedIterator for .consume_rev_iter() --- src/libextra/dlist.rs | 16 ++++------------ 1 file changed, 4 insertions(+), 12 deletions(-) diff --git a/src/libextra/dlist.rs b/src/libextra/dlist.rs index feafce58e6e9e..6085065460793 100644 --- a/src/libextra/dlist.rs +++ b/src/libextra/dlist.rs @@ -72,11 +72,6 @@ pub struct ConsumeIterator { priv list: DList } -/// DList reverse consuming iterator -pub struct ConsumeRevIterator { - priv list: DList -} - /// Rawlink is a type like Option but for holding a raw pointer impl Rawlink { /// Like Option::None for Rawlink @@ -346,8 +341,8 @@ impl DList { } /// Consume the list into an iterator yielding elements by value, in reverse - pub fn consume_rev_iter(self) -> ConsumeRevIterator { - ConsumeRevIterator{list: self} + pub fn consume_rev_iter(self) -> InvertIterator> { + self.consume_iter().invert() } } @@ -494,11 +489,8 @@ impl Iterator for ConsumeIterator { } } -impl Iterator for ConsumeRevIterator { - fn next(&mut self) -> Option { self.list.pop_back() } - fn size_hint(&self) -> (uint, Option) { - (self.list.length, Some(self.list.length)) - } +impl DoubleEndedIterator for ConsumeIterator { + fn next_back(&mut self) -> Option { self.list.pop_back() } } impl> FromIterator for DList { From c095a5c6cb5f749613672ab26c492fd55459b125 Mon Sep 17 00:00:00 2001 From: blake2-ppc Date: Fri, 12 Jul 2013 04:23:15 +0200 Subject: [PATCH 4/4] dlist: Use a DoubleEndedIterator for .mut_iter() and .mut_rev_iter() Unify the mutable iterators too. Switch the ListInsertion trait to use method .insert_next() and .peek_next() for list mutation. .insert_next() inserts an element into the list that will not appear in iteration, of course; so the length of the iteration can not change during iteration. --- src/libextra/dlist.rs | 188 ++++++++++++++++++++++++------------------ 1 file changed, 109 insertions(+), 79 deletions(-) diff --git a/src/libextra/dlist.rs b/src/libextra/dlist.rs index 6085065460793..283a726988ba2 100644 --- a/src/libextra/dlist.rs +++ b/src/libextra/dlist.rs @@ -53,17 +53,11 @@ pub struct DListIterator<'self, T> { priv nelem: uint, } -/// DList mutable iterator -pub struct MutForwardIterator<'self, T> { +/// Double-ended mutable DList iterator +pub struct MutDListIterator<'self, T> { priv list: &'self mut DList, - priv curs: Rawlink>, - priv nelem: uint, -} - -/// DList mutable reverse iterator -pub struct MutReverseIterator<'self, T> { - priv list: &'self mut DList, - priv next: Rawlink>, + priv head: Rawlink>, + priv tail: Rawlink>, priv nelem: uint, } @@ -279,13 +273,14 @@ impl DList { { let mut it = self.mut_iter(); loop { - match it.next() { + match it.peek_next() { None => break, - Some(x) => if f(x, &elt) { it.insert_before(elt); return } + Some(x) => if f(x, &elt) { break } } + it.next(); } + it.insert_next(elt); } - self.push_back(elt); } /// Merge DList `other` into this DList, using the function `f`. @@ -296,17 +291,16 @@ impl DList { pub fn merge(&mut self, mut other: DList, f: &fn(&T, &T) -> bool) { { let mut it = self.mut_iter(); - let mut elt = it.next(); loop { - let take_a = match (&mut elt, other.front()) { - (_ , None) => return, - (&None, _ ) => break, - (&Some(ref mut x), Some(y)) => f(*x, y), + let take_a = match (it.peek_next(), other.front()) { + (_ , None) => return, + (None, _ ) => break, + (Some(ref mut x), Some(y)) => f(*x, y), }; if take_a { - elt = it.next() + it.next(); } else { - it.insert_before(other.pop_front().unwrap()); + it.insert_next(other.pop_front().unwrap()); } } } @@ -325,13 +319,22 @@ impl DList { } /// Provide a forward iterator with mutable references - pub fn mut_iter<'a>(&'a mut self) -> MutForwardIterator<'a, T> { - MutForwardIterator{nelem: self.len(), list: self, curs: Rawlink::none()} + pub fn mut_iter<'a>(&'a mut self) -> MutDListIterator<'a, T> { + let head_raw = match self.list_head { + Some(ref mut h) => Rawlink::some(*h), + None => Rawlink::none(), + }; + MutDListIterator{ + nelem: self.len(), + head: head_raw, + tail: self.list_tail, + list: self + } } - /// Provide a reverse iterator with mutable references - pub fn mut_rev_iter<'a>(&'a mut self) -> MutReverseIterator<'a, T> { - MutReverseIterator{nelem: self.len(), list: self, next: self.list_tail} + pub fn mut_rev_iter<'a>(&'a mut self) -> InvertIterator<&'a mut T, + MutDListIterator<'a, T>> { + self.mut_iter().invert() } @@ -392,31 +395,21 @@ impl<'self, A> DoubleEndedIterator<&'self A> for DListIterator<'self, A> { } } -// MutForwardIterator is different because it implements ListInsertion, -// and can modify the list during traversal, used in insert_when and merge. -impl<'self, A> Iterator<&'self mut A> for MutForwardIterator<'self, A> { +impl<'self, A> Iterator<&'self mut A> for MutDListIterator<'self, A> { #[inline] fn next(&mut self) -> Option<&'self mut A> { - match self.curs.resolve() { - None => { - match self.list.list_head { - None => None, - Some(ref mut head) => { - self.nelem -= 1; - self.curs = Rawlink::some(*head); - Some(&mut head.value) - } - } - } - Some(curs) => { - match curs.next { - None => None, - Some(ref mut head) => { - self.nelem -= 1; - self.curs = Rawlink::some(*head); - Some(&mut head.value) - } - } + if self.nelem == 0 { + return None; + } + match self.head.resolve() { + None => None, + Some(next) => { + self.nelem -= 1; + self.head = match next.next { + Some(ref mut node) => Rawlink::some(&mut **node), + None => Rawlink::none(), + }; + Some(&mut next.value) } } } @@ -426,37 +419,39 @@ impl<'self, A> Iterator<&'self mut A> for MutForwardIterator<'self, A> { } } -impl<'self, A> Iterator<&'self mut A> for MutReverseIterator<'self, A> { +impl<'self, A> DoubleEndedIterator<&'self mut A> for MutDListIterator<'self, A> { #[inline] - fn next(&mut self) -> Option<&'self mut A> { - match self.next.resolve() { + fn next_back(&mut self) -> Option<&'self mut A> { + if self.nelem == 0 { + return None; + } + match self.tail.resolve() { None => None, Some(prev) => { self.nelem -= 1; - self.next = prev.prev; + self.tail = prev.prev; Some(&mut prev.value) } } } - - fn size_hint(&self) -> (uint, Option) { - (self.nelem, Some(self.nelem)) - } } + /// Allow mutating the DList while iterating pub trait ListInsertion { - /// Insert `elt` just previous to the most recently yielded element - fn insert_before(&mut self, elt: A); + /// Insert `elt` just after to the most recently yielded element + fn insert_next(&mut self, elt: A); /// Provide a reference to the next element, without changing the iterator fn peek_next<'a>(&'a mut self) -> Option<&'a mut A>; } -impl<'self, A> ListInsertion for MutForwardIterator<'self, A> { - fn insert_before(&mut self, elt: A) { - match self.curs.resolve() { - None => { self.list.push_front(elt); self.next(); } +impl<'self, A> ListInsertion for MutDListIterator<'self, A> { + fn insert_next(&mut self, elt: A) { + // Insert an element before `self.head` so that it is between the + // previously yielded element and self.head. + match self.head.resolve() { + None => { self.list.push_back(elt); } Some(node) => { let prev_node = match node.prev.resolve() { None => return self.list.push_front(elt), @@ -472,12 +467,9 @@ impl<'self, A> ListInsertion for MutForwardIterator<'self, A> { } fn peek_next<'a>(&'a mut self) -> Option<&'a mut A> { - match self.curs.resolve() { - None => self.list.front_mut(), - Some(curs) => match curs.next { - None => None, - Some(ref mut node) => Some(&mut node.value), - } + match self.head.resolve() { + None => None, + Some(head) => Some(&mut head.value), } } } @@ -680,6 +672,24 @@ mod tests { assert_eq!(it.next(), None); } + #[test] + fn test_iterator_double_end() { + let mut n = DList::new(); + assert_eq!(n.iter().next(), None); + n.push_front(4); + n.push_front(5); + n.push_front(6); + let mut it = n.iter(); + assert_eq!(it.size_hint(), (3, Some(3))); + assert_eq!(it.next().unwrap(), &6); + assert_eq!(it.size_hint(), (2, Some(2))); + assert_eq!(it.next_back().unwrap(), &4); + assert_eq!(it.size_hint(), (1, Some(1))); + assert_eq!(it.next_back().unwrap(), &5); + assert_eq!(it.next_back(), None); + assert_eq!(it.next(), None); + } + #[test] fn test_rev_iter() { let m = generate_test(); @@ -708,25 +718,45 @@ mod tests { let mut n = DList::new(); assert!(n.mut_iter().next().is_none()); n.push_front(4); + n.push_back(5); let mut it = n.mut_iter(); - assert_eq!(it.size_hint(), (1, Some(1))); + assert_eq!(it.size_hint(), (2, Some(2))); + assert!(it.next().is_some()); assert!(it.next().is_some()); assert_eq!(it.size_hint(), (0, Some(0))); assert!(it.next().is_none()); } + #[test] + fn test_iterator_mut_double_end() { + let mut n = DList::new(); + assert!(n.mut_iter().next_back().is_none()); + n.push_front(4); + n.push_front(5); + n.push_front(6); + let mut it = n.mut_iter(); + assert_eq!(it.size_hint(), (3, Some(3))); + assert_eq!(*it.next().unwrap(), 6); + assert_eq!(it.size_hint(), (2, Some(2))); + assert_eq!(*it.next_back().unwrap(), 4); + assert_eq!(it.size_hint(), (1, Some(1))); + assert_eq!(*it.next_back().unwrap(), 5); + assert!(it.next_back().is_none()); + assert!(it.next().is_none()); + } + #[test] fn test_insert_prev() { let mut m = list_from(&[0,2,4,6,8]); let len = m.len(); { let mut it = m.mut_iter(); - it.insert_before(-2); + it.insert_next(-2); loop { match it.next() { None => break, Some(elt) => { - it.insert_before(*elt + 1); + it.insert_next(*elt + 1); match it.peek_next() { Some(x) => assert_eq!(*x, *elt + 2), None => assert_eq!(8, *elt), @@ -734,12 +764,12 @@ mod tests { } } } - it.insert_before(0); - it.insert_before(1); + it.insert_next(0); + it.insert_next(1); } check_links(&m); assert_eq!(m.len(), 3 + len * 2); - assert_eq!(m.consume_iter().collect::<~[int]>(), ~[-2,1,0,3,2,5,4,7,6,9,0,1,8]); + assert_eq!(m.consume_iter().collect::<~[int]>(), ~[-2,0,1,2,3,4,5,6,7,8,9,0,1]); } #[test] @@ -853,7 +883,7 @@ mod tests { fn bench_collect_into(b: &mut test::BenchHarness) { let v = &[0, ..64]; do b.iter { - let _: DList = v.iter().transform(|&x|x).collect(); + let _: DList = v.iter().transform(|x| *x).collect(); } } #[bench] @@ -917,7 +947,7 @@ mod tests { let v = &[0, ..128]; let m: DList = v.iter().transform(|&x|x).collect(); do b.iter { - for m.iter().advance |_| {} + assert!(m.iter().len_() == 128); } } #[bench] @@ -925,7 +955,7 @@ mod tests { let v = &[0, ..128]; let mut m: DList = v.iter().transform(|&x|x).collect(); do b.iter { - for m.mut_iter().advance |_| {} + assert!(m.mut_iter().len_() == 128); } } #[bench] @@ -933,7 +963,7 @@ mod tests { let v = &[0, ..128]; let m: DList = v.iter().transform(|&x|x).collect(); do b.iter { - for m.rev_iter().advance |_| {} + assert!(m.rev_iter().len_() == 128); } } #[bench] @@ -941,7 +971,7 @@ mod tests { let v = &[0, ..128]; let mut m: DList = v.iter().transform(|&x|x).collect(); do b.iter { - for m.mut_rev_iter().advance |_| {} + assert!(m.mut_rev_iter().len_() == 128); } } #[bench]