From a0643ee9ae5726edaa382a1a125319688477ec98 Mon Sep 17 00:00:00 2001 From: Huon Wilson Date: Tue, 7 Jan 2014 00:14:37 +1100 Subject: [PATCH 1/6] std::trie: add an mutable-values iterator. --- src/libstd/trie.rs | 168 +++++++++++++++++++++++++++++++++++++++++++++ 1 file changed, 168 insertions(+) diff --git a/src/libstd/trie.rs b/src/libstd/trie.rs index 4f3f253d5e2..b66472c72cb 100644 --- a/src/libstd/trie.rs +++ b/src/libstd/trie.rs @@ -156,6 +156,16 @@ impl TrieMap { } } + /// Get an iterator over the key-value pairs in the map, with the + /// ability to mutate the values. + pub fn mut_iter<'a>(&'a mut self) -> TrieMapMutIterator<'a, T> { + TrieMapMutIterator { + stack: ~[self.root.children.mut_iter()], + remaining_min: self.length, + remaining_max: self.length + } + } + // If `upper` is true then returns upper_bound else returns lower_bound. #[inline] fn bound<'a>(&'a self, key: uint, upper: bool) -> TrieMapIterator<'a, T> { @@ -202,6 +212,63 @@ impl TrieMap { pub fn upper_bound<'a>(&'a self, key: uint) -> TrieMapIterator<'a, T> { self.bound(key, true) } + // If `upper` is true then returns upper_bound else returns lower_bound. + #[inline] + fn mut_bound<'a>(&'a mut self, key: uint, upper: bool) -> TrieMapMutIterator<'a, T> { + // we need an unsafe pointer here because we are borrowing + // references to the internals of each of these + // nodes. + // + // However, we're allowed to flaunt rustc like this because we + // never actually modify the "shape" of the nodes. The only + // place that mutation is can actually occur is of the actual + // values of the TrieMap (as the return value of the + // iterator), i.e. we can never cause a deallocation of any + // TrieNodes so this pointer is always valid. + let mut node = &mut self.root as *mut TrieNode; + + let mut idx = 0; + let mut it = TrieMapMutIterator { + stack: ~[], + remaining_min: 0, + remaining_max: self.length + }; + loop { + let children = unsafe {&mut (*node).children}; + let child_id = chunk(key, idx); + match children[child_id] { + Internal(ref mut n) => { + node = &mut **n as *mut TrieNode; + } + External(stored, _) => { + if stored < key || (upper && stored == key) { + it.stack.push(children.mut_slice_from(child_id + 1).mut_iter()); + } else { + it.stack.push(children.mut_slice_from(child_id).mut_iter()); + } + return it; + } + Nothing => { + it.stack.push(children.mut_slice_from(child_id + 1).mut_iter()); + return it + } + } + it.stack.push(children.mut_slice_from(child_id + 1).mut_iter()); + idx += 1; + } + } + + /// Get an iterator pointing to the first key-value pair whose key is not less than `key`. + /// If all keys in the map are less than `key` an empty iterator is returned. + pub fn mut_lower_bound<'a>(&'a mut self, key: uint) -> TrieMapMutIterator<'a, T> { + self.mut_bound(key, false) + } + + /// Get an iterator pointing to the first key-value pair whose key is greater than `key`. + /// If all keys in the map are not greater than `key` an empty iterator is returned. + pub fn mut_upper_bound<'a>(&'a mut self, key: uint) -> TrieMapMutIterator<'a, T> { + self.mut_bound(key, true) + } } impl FromIterator<(uint, T)> for TrieMap { @@ -482,6 +549,47 @@ impl<'a, T> Iterator<(uint, &'a T)> for TrieMapIterator<'a, T> { } } +/// Forward iterator over the key-value pairs of a map, with the +/// values being mutable. +pub struct TrieMapMutIterator<'a, T> { + priv stack: ~[vec::VecMutIterator<'a, Child>], + priv remaining_min: uint, + priv remaining_max: uint +} + +impl<'a, T> Iterator<(uint, &'a mut T)> for TrieMapMutIterator<'a, T> { + fn next(&mut self) -> Option<(uint, &'a mut T)> { + while !self.stack.is_empty() { + match self.stack[self.stack.len() - 1].next() { + None => { + self.stack.pop(); + } + Some(child) => { + match *child { + Internal(ref mut node) => { + self.stack.push(node.children.mut_iter()); + } + External(key, ref mut value) => { + self.remaining_max -= 1; + if self.remaining_min > 0 { + self.remaining_min -= 1; + } + return Some((key, value)); + } + Nothing => {} + } + } + } + } + return None; + } + + #[inline] + fn size_hint(&self) -> (uint, Option) { + (self.remaining_min, Some(self.remaining_max)) + } +} + /// Forward iterator over a set pub struct TrieSetIterator<'a> { priv iter: TrieMapIterator<'a, ()> @@ -712,6 +820,30 @@ mod test_map { assert_eq!(i, last - first); } + #[test] + fn test_mut_iter() { + let mut empty_map : TrieMap = TrieMap::new(); + assert!(empty_map.mut_iter().next().is_none()); + + let first = uint::max_value - 10000; + let last = uint::max_value; + + let mut map = TrieMap::new(); + for x in range(first, last).invert() { + map.insert(x, x / 2); + } + + let mut i = 0; + for (k, v) in map.mut_iter() { + assert_eq!(k, first + i); + *v -= k / 2; + i += 1; + } + assert_eq!(i, last - first); + + assert!(map.iter().all(|(_, &v)| v == 0)); + } + #[test] fn test_bound() { let empty_map : TrieMap = TrieMap::new(); @@ -753,6 +885,42 @@ mod test_map { assert_eq!(ub.next(), None); } } + + #[test] + fn test_mut_bound() { + let empty_map : TrieMap = TrieMap::new(); + assert_eq!(empty_map.lower_bound(0).next(), None); + assert_eq!(empty_map.upper_bound(0).next(), None); + + let mut m_lower = TrieMap::new(); + let mut m_upper = TrieMap::new(); + for i in range(0u, 100) { + m_lower.insert(2 * i, 4 * i); + m_upper.insert(2 * i, 4 * i); + } + + for i in range(0u, 199) { + let mut lb_it = m_lower.mut_lower_bound(i); + let (k, v) = lb_it.next().unwrap(); + let lb = i + i % 2; + assert_eq!(lb, k); + *v -= k; + } + + for i in range(0u, 198) { + let mut ub_it = m_upper.mut_upper_bound(i); + let (k, v) = ub_it.next().unwrap(); + let ub = i + 2 - i % 2; + assert_eq!(ub, k); + *v -= k; + } + + assert!(m_lower.mut_lower_bound(199).next().is_none()); + assert!(m_upper.mut_upper_bound(198).next().is_none()); + + assert!(m_lower.iter().all(|(_, &x)| x == 0)); + assert!(m_upper.iter().all(|(_, &x)| x == 0)); + } } #[cfg(test)] From f07c74d93ad2b5292267e5829c4c8493211aa835 Mon Sep 17 00:00:00 2001 From: Huon Wilson Date: Tue, 7 Jan 2014 00:17:38 +1100 Subject: [PATCH 2/6] std::trie: remove some obsolete internal iterators. --- src/libextra/serialize.rs | 22 +++----- src/libstd/trie.rs | 102 ++------------------------------------ 2 files changed, 11 insertions(+), 113 deletions(-) diff --git a/src/libextra/serialize.rs b/src/libextra/serialize.rs index e7ccb91fb75..59f7f2a2ffc 100644 --- a/src/libextra/serialize.rs +++ b/src/libextra/serialize.rs @@ -768,14 +768,11 @@ impl< > Encodable for TrieMap { fn encode(&self, e: &mut E) { e.emit_map(self.len(), |e| { - let mut i = 0; - self.each(|key, val| { - e.emit_map_elt_key(i, |e| key.encode(e)); - e.emit_map_elt_val(i, |e| val.encode(e)); - i += 1; - true + for (i, (key, val)) in self.iter().enumerate() { + e.emit_map_elt_key(i, |e| key.encode(e)); + e.emit_map_elt_val(i, |e| val.encode(e)); + } }); - }) } } @@ -799,13 +796,10 @@ impl< impl Encodable for TrieSet { fn encode(&self, s: &mut S) { s.emit_seq(self.len(), |s| { - let mut i = 0; - self.each(|e| { - s.emit_seq_elt(i, |s| e.encode(s)); - i += 1; - true - }); - }) + for (i, e) in self.iter().enumerate() { + s.emit_seq_elt(i, |s| e.encode(s)); + } + }) } } diff --git a/src/libstd/trie.rs b/src/libstd/trie.rs index b66472c72cb..08805c88c7b 100644 --- a/src/libstd/trie.rs +++ b/src/libstd/trie.rs @@ -111,30 +111,6 @@ impl TrieMap { self.root.each_reverse(f) } - /// Visit all key-value pairs in order - #[inline] - pub fn each<'a>(&'a self, f: |&uint, &'a T| -> bool) -> bool { - self.root.each(f) - } - - /// Visit all keys in order - #[inline] - pub fn each_key(&self, f: |&uint| -> bool) -> bool { - self.each(|k, _| f(k)) - } - - /// Visit all values in order - #[inline] - pub fn each_value<'a>(&'a self, f: |&'a T| -> bool) -> bool { - self.each(|_, v| f(v)) - } - - /// Iterate over the map and mutate the contained values - #[inline] - pub fn mutate_values(&mut self, f: |&uint, &mut T| -> bool) -> bool { - self.root.mutate_values(f) - } - /// Visit all keys in reverse order #[inline] pub fn each_key_reverse(&self, f: |&uint| -> bool) -> bool { @@ -331,10 +307,6 @@ impl TrieSet { self.map.remove(value) } - /// Visit all values in order - #[inline] - pub fn each(&self, f: |&uint| -> bool) -> bool { self.map.each_key(f) } - /// Visit all values in reverse order #[inline] pub fn each_reverse(&self, f: |&uint| -> bool) -> bool { @@ -395,17 +367,6 @@ impl TrieNode { } impl TrieNode { - fn each<'a>(&'a self, f: |&uint, &'a T| -> bool) -> bool { - for elt in self.children.iter() { - match *elt { - Internal(ref x) => if !x.each(|i,t| f(i,t)) { return false }, - External(k, ref v) => if !f(&k, v) { return false }, - Nothing => () - } - } - true - } - fn each_reverse<'a>(&'a self, f: |&uint, &'a T| -> bool) -> bool { for elt in self.children.rev_iter() { match *elt { @@ -416,19 +377,6 @@ impl TrieNode { } true } - - fn mutate_values<'a>(&'a mut self, f: |&uint, &mut T| -> bool) -> bool { - for child in self.children.mut_iter() { - match *child { - Internal(ref mut x) => if !x.mutate_values(|i,t| f(i,t)) { - return false - }, - External(k, ref mut v) => if !f(&k, v) { return false }, - Nothing => () - } - } - true - } } // if this was done via a trait, the key could be generic @@ -691,46 +639,6 @@ mod test_map { } } - #[test] - fn test_each() { - let mut m = TrieMap::new(); - - assert!(m.insert(3, 6)); - assert!(m.insert(0, 0)); - assert!(m.insert(4, 8)); - assert!(m.insert(2, 4)); - assert!(m.insert(1, 2)); - - let mut n = 0; - m.each(|k, v| { - assert_eq!(*k, n); - assert_eq!(*v, n * 2); - n += 1; - true - }); - } - - #[test] - fn test_each_break() { - let mut m = TrieMap::new(); - - for x in range(uint::max_value - 10000, uint::max_value).invert() { - m.insert(x, x / 2); - } - - let mut n = uint::max_value - 10000; - m.each(|k, v| { - if n == uint::max_value - 5000 { false } else { - assert!(n < uint::max_value - 5000); - - assert_eq!(*k, n); - assert_eq!(*v, n / 2); - n += 1; - true - } - }); - } - #[test] fn test_each_reverse() { let mut m = TrieMap::new(); @@ -943,13 +851,9 @@ mod test_set { let expected = [x, y]; - let mut i = 0; - - trie.each(|x| { - assert_eq!(expected[i], *x); - i += 1; - true - }); + for (i, x) in trie.iter().enumerate() { + assert_eq!(expected[i], x); + } } #[test] From fe03caedf0d2e32c41bb1c5169fb0162c8af6b28 Mon Sep 17 00:00:00 2001 From: Huon Wilson Date: Tue, 7 Jan 2014 01:00:19 +1100 Subject: [PATCH 3/6] std::trie: use macros to share code between the iterator implementations. --- src/libstd/trie.rs | 263 ++++++++++++++++++++++----------------------- 1 file changed, 130 insertions(+), 133 deletions(-) diff --git a/src/libstd/trie.rs b/src/libstd/trie.rs index 08805c88c7b..b6995a1d24c 100644 --- a/src/libstd/trie.rs +++ b/src/libstd/trie.rs @@ -141,40 +141,95 @@ impl TrieMap { remaining_max: self.length } } +} +// FIXME #5846 we want to be able to choose between &x and &mut x +// (with many different `x`) below, so we need to optionally pass mut +// as a tt, but the only thing we can do with a `tt` is pass them to +// other macros, so this takes the `& ` token +// sequence and forces their evalutation as an expression. (see also +// `item!` below.) +macro_rules! addr { ($e:expr) => { $e } } + +macro_rules! bound { + ($iterator_name:ident, + // the current treemap + self = $this:expr, + // the key to look for + key = $key:expr, + // are we looking at the upper bound? + is_upper = $upper:expr, + + // method names for slicing/iterating. + slice_from = $slice_from:ident, + iter = $iter:ident, + + // see the comment on `addr!`, this is just an optional mut, but + // there's no 0-or-1 repeats yet. + mutability = $($mut_:tt)*) => { + { + // # For `mut` + // We need an unsafe pointer here because we are borrowing + // mutable references to the internals of each of these + // mutable nodes, while still using the outer node. + // + // However, we're allowed to flaunt rustc like this because we + // never actually modify the "shape" of the nodes. The only + // place that mutation is can actually occur is of the actual + // values of the TrieMap (as the return value of the + // iterator), i.e. we can never cause a deallocation of any + // TrieNodes so the raw pointer is always valid. + // + // # For non-`mut` + // We like sharing code so much that even a little unsafe won't + // stop us. + let this = $this; + let mut node = addr!(& $($mut_)* this.root as * $($mut_)* TrieNode); + + let key = $key; + + let mut idx = 0; + let mut it = $iterator_name { + stack: ~[], + remaining_min: 0, + remaining_max: this.length + }; + // this addr is necessary for the `Internal` pattern. + addr!(loop { + let children = unsafe {addr!(& $($mut_)* (*node).children)}; + let child_id = chunk(key, idx); + match children[child_id] { + Internal(ref $($mut_)* n) => { + node = addr!(& $($mut_)* **n as * $($mut_)* TrieNode); + } + External(stored, _) => { + if stored < key || ($upper && stored == key) { + it.stack.push(children.$slice_from(child_id + 1).$iter()); + } else { + it.stack.push(children.$slice_from(child_id).$iter()); + } + return it; + } + Nothing => { + it.stack.push(children.$slice_from(child_id + 1).$iter()); + return it + } + } + it.stack.push(children.$slice_from(child_id + 1).$iter()); + idx += 1; + }) + } + } +} + +impl TrieMap { // If `upper` is true then returns upper_bound else returns lower_bound. #[inline] fn bound<'a>(&'a self, key: uint, upper: bool) -> TrieMapIterator<'a, T> { - let mut node: &'a TrieNode = &self.root; - let mut idx = 0; - let mut it = TrieMapIterator { - stack: ~[], - remaining_min: 0, - remaining_max: self.length - }; - loop { - let children = &node.children; - let child_id = chunk(key, idx); - match children[child_id] { - Internal(ref n) => { - node = &**n; - it.stack.push(children.slice_from(child_id + 1).iter()); - } - External(stored, _) => { - if stored < key || (upper && stored == key) { - it.stack.push(children.slice_from(child_id + 1).iter()); - } else { - it.stack.push(children.slice_from(child_id).iter()); - } - return it; - } - Nothing => { - it.stack.push(children.slice_from(child_id + 1).iter()); - return it - } - } - idx += 1; - } + bound!(TrieMapIterator, self = self, + key = key, is_upper = upper, + slice_from = slice_from, iter = iter, + mutability = ) } /// Get an iterator pointing to the first key-value pair whose key is not less than `key`. @@ -191,47 +246,10 @@ impl TrieMap { // If `upper` is true then returns upper_bound else returns lower_bound. #[inline] fn mut_bound<'a>(&'a mut self, key: uint, upper: bool) -> TrieMapMutIterator<'a, T> { - // we need an unsafe pointer here because we are borrowing - // references to the internals of each of these - // nodes. - // - // However, we're allowed to flaunt rustc like this because we - // never actually modify the "shape" of the nodes. The only - // place that mutation is can actually occur is of the actual - // values of the TrieMap (as the return value of the - // iterator), i.e. we can never cause a deallocation of any - // TrieNodes so this pointer is always valid. - let mut node = &mut self.root as *mut TrieNode; - - let mut idx = 0; - let mut it = TrieMapMutIterator { - stack: ~[], - remaining_min: 0, - remaining_max: self.length - }; - loop { - let children = unsafe {&mut (*node).children}; - let child_id = chunk(key, idx); - match children[child_id] { - Internal(ref mut n) => { - node = &mut **n as *mut TrieNode; - } - External(stored, _) => { - if stored < key || (upper && stored == key) { - it.stack.push(children.mut_slice_from(child_id + 1).mut_iter()); - } else { - it.stack.push(children.mut_slice_from(child_id).mut_iter()); - } - return it; - } - Nothing => { - it.stack.push(children.mut_slice_from(child_id + 1).mut_iter()); - return it - } - } - it.stack.push(children.mut_slice_from(child_id + 1).mut_iter()); - idx += 1; - } + bound!(TrieMapMutIterator, self = self, + key = key, is_upper = upper, + slice_from = mut_slice_from, iter = mut_iter, + mutability = mut) } /// Get an iterator pointing to the first key-value pair whose key is not less than `key`. @@ -464,39 +482,6 @@ pub struct TrieMapIterator<'a, T> { priv remaining_max: uint } -impl<'a, T> Iterator<(uint, &'a T)> for TrieMapIterator<'a, T> { - fn next(&mut self) -> Option<(uint, &'a T)> { - while !self.stack.is_empty() { - match self.stack[self.stack.len() - 1].next() { - None => { - self.stack.pop(); - } - Some(ref child) => { - match **child { - Internal(ref node) => { - self.stack.push(node.children.iter()); - } - External(key, ref value) => { - self.remaining_max -= 1; - if self.remaining_min > 0 { - self.remaining_min -= 1; - } - return Some((key, value)); - } - Nothing => {} - } - } - } - } - return None; - } - - #[inline] - fn size_hint(&self) -> (uint, Option) { - (self.remaining_min, Some(self.remaining_max)) - } -} - /// Forward iterator over the key-value pairs of a map, with the /// values being mutable. pub struct TrieMapMutIterator<'a, T> { @@ -505,39 +490,51 @@ pub struct TrieMapMutIterator<'a, T> { priv remaining_max: uint } -impl<'a, T> Iterator<(uint, &'a mut T)> for TrieMapMutIterator<'a, T> { - fn next(&mut self) -> Option<(uint, &'a mut T)> { - while !self.stack.is_empty() { - match self.stack[self.stack.len() - 1].next() { - None => { - self.stack.pop(); - } - Some(child) => { - match *child { - Internal(ref mut node) => { - self.stack.push(node.children.mut_iter()); - } - External(key, ref mut value) => { - self.remaining_max -= 1; - if self.remaining_min > 0 { - self.remaining_min -= 1; - } - return Some((key, value)); - } - Nothing => {} - } - } - } - } - return None; - } +// FIXME #5846: see `addr!` above. +macro_rules! item { ($i:item) => {$i}} - #[inline] - fn size_hint(&self) -> (uint, Option) { - (self.remaining_min, Some(self.remaining_max)) +macro_rules! iterator_impl { + ($name:ident, + iter = $iter:ident, + mutability = $($mut_:tt)*) => { + item!(impl<'a, T> Iterator<(uint, &'a $($mut_)* T)> for $name<'a, T> { + fn next(&mut self) -> Option<(uint, &'a $($mut_)* T)> { + while !self.stack.is_empty() { + match self.stack[self.stack.len() - 1].next() { + None => { + self.stack.pop(); + } + Some(child) => { + addr!(match *child { + Internal(ref $($mut_)* node) => { + self.stack.push(node.children.$iter()); + } + External(key, ref $($mut_)* value) => { + self.remaining_max -= 1; + if self.remaining_min > 0 { + self.remaining_min -= 1; + } + return Some((key, value)); + } + Nothing => {} + }) + } + } + } + return None; + } + + #[inline] + fn size_hint(&self) -> (uint, Option) { + (self.remaining_min, Some(self.remaining_max)) + } + }) } } +iterator_impl! { TrieMapIterator, iter = iter, mutability = } +iterator_impl! { TrieMapMutIterator, iter = mut_iter, mutability = mut } + /// Forward iterator over a set pub struct TrieSetIterator<'a> { priv iter: TrieMapIterator<'a, ()> From 3395f9d6a10aa912ab88de2e8d5b4f7de407413a Mon Sep 17 00:00:00 2001 From: Huon Wilson Date: Tue, 7 Jan 2014 01:42:55 +1100 Subject: [PATCH 4/6] std::trie: Add some iteration/search benchmarks. --- src/libstd/trie.rs | 60 ++++++++++++++++++++++++++++++++++++++++++++++ 1 file changed, 60 insertions(+) diff --git a/src/libstd/trie.rs b/src/libstd/trie.rs index b6995a1d24c..94b7b881ea0 100644 --- a/src/libstd/trie.rs +++ b/src/libstd/trie.rs @@ -828,6 +828,66 @@ mod test_map { } } +#[cfg(test)] +mod bench_map { + use super::*; + use prelude::*; + use rand::{weak_rng, Rng}; + use extra::test::BenchHarness; + + #[bench] + fn bench_iter_small(bh: &mut BenchHarness) { + let mut m = TrieMap::::new(); + let mut rng = weak_rng(); + for _ in range(0, 20) { + m.insert(rng.gen(), rng.gen()); + } + + bh.iter(|| for _ in m.iter() {}) + } + + #[bench] + fn bench_iter_large(bh: &mut BenchHarness) { + let mut m = TrieMap::::new(); + let mut rng = weak_rng(); + for _ in range(0, 1000) { + m.insert(rng.gen(), rng.gen()); + } + + bh.iter(|| for _ in m.iter() {}) + } + + #[bench] + fn bench_lower_bound(bh: &mut BenchHarness) { + let mut m = TrieMap::::new(); + let mut rng = weak_rng(); + for _ in range(0, 1000) { + m.insert(rng.gen(), rng.gen()); + } + + bh.iter(|| { + for _ in range(0, 10) { + m.lower_bound(rng.gen()); + } + }); + } + + #[bench] + fn bench_upper_bound(bh: &mut BenchHarness) { + let mut m = TrieMap::::new(); + let mut rng = weak_rng(); + for _ in range(0, 1000) { + m.insert(rng.gen(), rng.gen()); + } + + bh.iter(|| { + for _ in range(0, 10) { + m.upper_bound(rng.gen()); + } + }); + } +} + #[cfg(test)] mod test_set { use super::*; From 7e446af759e86e77a5f4a8e9bc6d6c22072b25ae Mon Sep 17 00:00:00 2001 From: Huon Wilson Date: Tue, 7 Jan 2014 01:45:10 +1100 Subject: [PATCH 5/6] std::trie: make lower_bound and upper_bound about 15% faster. I believe this is mainly due to code-size reduction. Before: test [...]::bench_lower_bound ... bench: 818 ns/iter (+/- 100) test [...]::bench_upper_bound ... bench: 939 ns/iter (+/- 34) After: test [...]::bench_lower_bound ... bench: 698 ns/iter (+/- 60) test [...]::bench_upper_bound ... bench: 817 ns/iter (+/- 20) --- src/libstd/trie.rs | 20 ++++++++++---------- 1 file changed, 10 insertions(+), 10 deletions(-) diff --git a/src/libstd/trie.rs b/src/libstd/trie.rs index 94b7b881ea0..d864cde2953 100644 --- a/src/libstd/trie.rs +++ b/src/libstd/trie.rs @@ -198,24 +198,24 @@ macro_rules! bound { addr!(loop { let children = unsafe {addr!(& $($mut_)* (*node).children)}; let child_id = chunk(key, idx); - match children[child_id] { + let (slice_idx, ret) = match children[child_id] { Internal(ref $($mut_)* n) => { node = addr!(& $($mut_)* **n as * $($mut_)* TrieNode); + (child_id + 1, false) } External(stored, _) => { - if stored < key || ($upper && stored == key) { - it.stack.push(children.$slice_from(child_id + 1).$iter()); + (if stored < key || ($upper && stored == key) { + child_id + 1 } else { - it.stack.push(children.$slice_from(child_id).$iter()); - } - return it; + child_id + }, true) } Nothing => { - it.stack.push(children.$slice_from(child_id + 1).$iter()); - return it + (child_id + 1, true) } - } - it.stack.push(children.$slice_from(child_id + 1).$iter()); + }; + it.stack.push(children.$slice_from(slice_idx).$iter()); + if ret { return it } idx += 1; }) } From 167d533fe0624963cb3f836ebce06a457043c816 Mon Sep 17 00:00:00 2001 From: Huon Wilson Date: Tue, 7 Jan 2014 02:09:07 +1100 Subject: [PATCH 6/6] extra::treemap: use the dummy-macro trick with items to make the iterator macro properly hygiene. Requires less repetition of `mut` or not too. --- src/libextra/treemap.rs | 29 +++++++++++++---------------- 1 file changed, 13 insertions(+), 16 deletions(-) diff --git a/src/libextra/treemap.rs b/src/libextra/treemap.rs index f4fd81437fc..9fe419b06d2 100644 --- a/src/libextra/treemap.rs +++ b/src/libextra/treemap.rs @@ -327,14 +327,12 @@ pub struct TreeMapMutRevIterator<'a, K, V> { // other macros, so this takes the `& ` token // sequence and forces their evalutation as an expression. macro_rules! addr { ($e:expr) => { $e }} +// putting an optional mut into type signatures +macro_rules! item { ($i:item) => { $i }} macro_rules! define_iterator { ($name:ident, $rev_name:ident, - // the type of the values of the treemap in the return value of - // the iterator (i.e. &V or &mut V). This is non-hygienic in the - // name of the lifetime. - value_type = $value_type:ty, // the function to go from &m Option<~TreeNode> to *m TreeNode deref = $deref:ident, @@ -343,10 +341,11 @@ macro_rules! define_iterator { // there's no support for 0-or-1 repeats. addr_mut = $($addr_mut:tt)* ) => { - // private methods on the forward iterator - impl<'a, K, V> $name<'a, K, V> { + // private methods on the forward iterator (item!() for the + // addr_mut in the next_ return value) + item!(impl<'a, K, V> $name<'a, K, V> { #[inline(always)] - fn next_(&mut self, forward: bool) -> Option<(&'a K, $value_type)> { + fn next_(&mut self, forward: bool) -> Option<(&'a K, &'a $($addr_mut)* V)> { while !self.stack.is_empty() || !self.node.is_null() { if !self.node.is_null() { let node = unsafe {addr!(& $($addr_mut)* *self.node)}; @@ -412,14 +411,14 @@ macro_rules! define_iterator { self.node = ptr::RawPtr::null(); } } - } + }) // the forward Iterator impl. - impl<'a, K, V> Iterator<(&'a K, $value_type)> for $name<'a, K, V> { + item!(impl<'a, K, V> Iterator<(&'a K, &'a $($addr_mut)* V)> for $name<'a, K, V> { /// Advance the iterator to the next node (in order) and return a /// tuple with a reference to the key and value. If there are no /// more nodes, return `None`. - fn next(&mut self) -> Option<(&'a K, $value_type)> { + fn next(&mut self) -> Option<(&'a K, &'a $($addr_mut)* V)> { self.next_(true) } @@ -427,11 +426,11 @@ macro_rules! define_iterator { fn size_hint(&self) -> (uint, Option) { (self.remaining_min, Some(self.remaining_max)) } - } + }) // the reverse Iterator impl. - impl<'a, K, V> Iterator<(&'a K, $value_type)> for $rev_name<'a, K, V> { - fn next(&mut self) -> Option<(&'a K, $value_type)> { + item!(impl<'a, K, V> Iterator<(&'a K, &'a $($addr_mut)* V)> for $rev_name<'a, K, V> { + fn next(&mut self) -> Option<(&'a K, &'a $($addr_mut)* V)> { self.iter.next_(false) } @@ -439,14 +438,13 @@ macro_rules! define_iterator { fn size_hint(&self) -> (uint, Option) { self.iter.size_hint() } - } + }) } } // end of define_iterator define_iterator! { TreeMapIterator, TreeMapRevIterator, - value_type = &'a V, deref = deref, // immutable, so no mut @@ -455,7 +453,6 @@ define_iterator! { define_iterator! { TreeMapMutIterator, TreeMapMutRevIterator, - value_type = &'a mut V, deref = mut_deref, addr_mut = mut