From 2e2261042c3dd70157001e592ba5da7e3a2ccf99 Mon Sep 17 00:00:00 2001 From: Chase Southwood Date: Fri, 12 Dec 2014 00:04:47 -0600 Subject: [PATCH 1/4] Use wrapper structs for `BTreeMap`'s iterators. Using a type alias for iterator implementations is fragile since this exposes the implementation to users of the iterator, and any changes could break existing code. This commit changes the keys and values iterators of `BTreeMap` to use proper new types, rather than type aliases. However, since it is fair-game to treat a type-alias as the aliased type, this is a: [breaking-change]. --- src/libcollections/btree/map.rs | 34 +++++++++++++++++++++++++++------ 1 file changed, 28 insertions(+), 6 deletions(-) diff --git a/src/libcollections/btree/map.rs b/src/libcollections/btree/map.rs index e49a8ddbe5ab8..e45e91b93ba42 100644 --- a/src/libcollections/btree/map.rs +++ b/src/libcollections/btree/map.rs @@ -26,6 +26,7 @@ use std::hash::{Writer, Hash}; use core::default::Default; use core::{iter, fmt, mem}; use core::fmt::Show; +use core::iter::Map; use ring_buf::RingBuf; @@ -107,12 +108,14 @@ pub struct MoveEntries { } /// An iterator over a BTreeMap's keys. -pub type Keys<'a, K, V> = - iter::Map<(&'a K, &'a V), &'a K, Entries<'a, K, V>, fn((&'a K, &'a V)) -> &'a K>; +pub struct Keys<'a, K: 'a, V: 'a> { + inner: Map<(&'a K, &'a V), &'a K, Entries<'a, K, V>, fn((&'a K, &'a V)) -> &'a K> +} /// An iterator over a BTreeMap's values. -pub type Values<'a, K, V> = - iter::Map<(&'a K, &'a V), &'a V, Entries<'a, K, V>, fn((&'a K, &'a V)) -> &'a V>; +pub struct Values<'a, K: 'a, V: 'a> { + inner: Map<(&'a K, &'a V), &'a V, Entries<'a, K, V>, fn((&'a K, &'a V)) -> &'a V> +} /// A view into a single entry in a map, which may either be vacant or occupied. pub enum Entry<'a, K:'a, V:'a> { @@ -1061,6 +1064,25 @@ impl DoubleEndedIterator<(K, V)> for MoveEntries { impl ExactSizeIterator<(K, V)> for MoveEntries {} +impl<'a, K, V> Iterator<&'a K> for Keys<'a, K, V> { + fn next(&mut self) -> Option<(&'a K)> { self.inner.next() } + fn size_hint(&self) -> (uint, Option) { self.inner.size_hint() } +} +impl<'a, K, V> DoubleEndedIterator<&'a K> for Keys<'a, K, V> { + fn next_back(&mut self) -> Option<(&'a K)> { self.inner.next_back() } +} +impl<'a, K, V> ExactSizeIterator<&'a K> for Keys<'a, K, V> {} + + +impl<'a, K, V> Iterator<&'a V> for Values<'a, K, V> { + fn next(&mut self) -> Option<(&'a V)> { self.inner.next() } + fn size_hint(&self) -> (uint, Option) { self.inner.size_hint() } +} +impl<'a, K, V> DoubleEndedIterator<&'a V> for Values<'a, K, V> { + fn next_back(&mut self) -> Option<(&'a V)> { self.inner.next_back() } +} +impl<'a, K, V> ExactSizeIterator<&'a V> for Values<'a, K, V> {} + impl<'a, K: Ord, V> VacantEntry<'a, K, V> { /// Sets the value of the entry with the VacantEntry's key, @@ -1211,7 +1233,7 @@ impl BTreeMap { pub fn keys<'a>(&'a self) -> Keys<'a, K, V> { fn first((a, _): (A, B)) -> A { a } - self.iter().map(first) + Keys { inner: self.iter().map(first) } } /// Gets an iterator over the values of the map. @@ -1232,7 +1254,7 @@ impl BTreeMap { pub fn values<'a>(&'a self) -> Values<'a, K, V> { fn second((_, b): (A, B)) -> B { b } - self.iter().map(second) + Values { inner: self.iter().map(second) } } /// Return the number of elements in the map. From b7e09d8d9e4fb8d3232ca72ea40aba3479329dff Mon Sep 17 00:00:00 2001 From: Chase Southwood Date: Fri, 12 Dec 2014 23:14:57 -0600 Subject: [PATCH 2/4] Use wrapper structs for `BTreeSet`'s iterators. Using a type alias for iterator implementations is fragile since this exposes the implementation to users of the iterator, and any changes could break existing code. This commit changes the iterators of `BTreeSet` to use proper new types, rather than type aliases. However, since it is fair-game to treat a type-alias as the aliased type, this is a: [breaking-change]. --- src/libcollections/btree/set.rs | 34 +++++++++++++++++++++++++++------ 1 file changed, 28 insertions(+), 6 deletions(-) diff --git a/src/libcollections/btree/set.rs b/src/libcollections/btree/set.rs index cd01c008fe1bc..83c34594ec36a 100644 --- a/src/libcollections/btree/set.rs +++ b/src/libcollections/btree/set.rs @@ -18,7 +18,7 @@ use std::hash::Hash; use core::borrow::BorrowFrom; use core::default::Default; use core::{iter, fmt}; -use core::iter::Peekable; +use core::iter::{Peekable, Map}; use core::fmt::Show; // FIXME(conventions): implement bounded iterators @@ -33,11 +33,14 @@ pub struct BTreeSet{ } /// An iterator over a BTreeSet's items. -pub type Items<'a, T> = Keys<'a, T, ()>; +pub struct Items<'a, T: 'a> { + iter: Keys<'a, T, ()> +} /// An owning iterator over a BTreeSet's items. -pub type MoveItems = - iter::Map<(T, ()), T, MoveEntries, fn((T, ())) -> T>; +pub struct MoveItems { + iter: Map<(T, ()), T, MoveEntries, fn((T, ())) -> T> +} /// A lazy iterator producing elements in the set difference (in-order). pub struct DifferenceItems<'a, T:'a> { @@ -82,7 +85,7 @@ impl BTreeSet { /// Gets an iterator over the BTreeSet's contents. #[unstable = "matches collection reform specification, waiting for dust to settle"] pub fn iter<'a>(&'a self) -> Items<'a, T> { - self.map.keys() + Items { iter: self.map.keys() } } /// Gets an iterator for moving out the BtreeSet's contents. @@ -90,7 +93,7 @@ impl BTreeSet { pub fn into_iter(self) -> MoveItems { fn first((a, _): (A, B)) -> A { a } - self.map.into_iter().map(first) + MoveItems { iter: self.map.into_iter().map(first) } } } @@ -505,6 +508,25 @@ impl Show for BTreeSet { } } +impl<'a, T> Iterator<&'a T> for Items<'a, T> { + fn next(&mut self) -> Option<&'a T> { self.iter.next() } + fn size_hint(&self) -> (uint, Option) { self.iter.size_hint() } +} +impl<'a, T> DoubleEndedIterator<&'a T> for Items<'a, T> { + fn next_back(&mut self) -> Option<&'a T> { self.iter.next_back() } +} +impl<'a, T> ExactSizeIterator<&'a T> for Items<'a, T> {} + + +impl Iterator for MoveItems { + fn next(&mut self) -> Option { self.iter.next() } + fn size_hint(&self) -> (uint, Option) { self.iter.size_hint() } +} +impl DoubleEndedIterator for MoveItems { + fn next_back(&mut self) -> Option { self.iter.next_back() } +} +impl ExactSizeIterator for MoveItems {} + /// Compare `x` and `y`, but return `short` if x is None and `long` if y is None fn cmp_opt(x: Option<&T>, y: Option<&T>, short: Ordering, long: Ordering) -> Ordering { From adec1beb43914204d4f9415cab6d24d5b5a553a2 Mon Sep 17 00:00:00 2001 From: Chase Southwood Date: Fri, 12 Dec 2014 01:02:19 -0600 Subject: [PATCH 3/4] Use wrapper structs for `HashMap`'s iterators. Using a type alias for iterator implementations is fragile since this exposes the implementation to users of the iterator, and any changes could break existing code. This commit changes the keys and values iterators of `HashMap` to use proper new types, rather than type aliases. However, since it is fair-game to treat a type-alias as the aliased type, this is a: [breaking-change]. --- src/libstd/collections/hash/map.rs | 64 ++++++++++++++---------------- src/libstd/collections/hash/set.rs | 5 +-- 2 files changed, 31 insertions(+), 38 deletions(-) diff --git a/src/libstd/collections/hash/map.rs b/src/libstd/collections/hash/map.rs index 2a8d97eed05bc..d22e7b2f764f5 100644 --- a/src/libstd/collections/hash/map.rs +++ b/src/libstd/collections/hash/map.rs @@ -20,7 +20,7 @@ use cmp::{max, Eq, Equiv, PartialEq}; use default::Default; use fmt::{mod, Show}; use hash::{Hash, Hasher, RandomSipHasher}; -use iter::{mod, Iterator, IteratorExt, FromIterator, Extend}; +use iter::{mod, Iterator, IteratorExt, FromIterator, Extend, Map}; use kinds::Sized; use mem::{mod, replace}; use num::{Int, UnsignedInt}; @@ -859,7 +859,7 @@ impl, V, S, H: Hasher> HashMap { pub fn keys(&self) -> Keys { fn first((a, _): (A, B)) -> A { a } - self.iter().map(first) + Keys { inner: self.iter().map(first) } } /// An iterator visiting all values in arbitrary order. @@ -883,7 +883,7 @@ impl, V, S, H: Hasher> HashMap { pub fn values(&self) -> Values { fn second((_, b): (A, B)) -> B { b } - self.iter().map(second) + Values { inner: self.iter().map(second) } } /// An iterator visiting all key-value pairs in arbitrary order. @@ -1335,6 +1335,16 @@ pub struct MoveEntries { > } +/// HashMap keys iterator +pub struct Keys<'a, K: 'a, V: 'a> { + inner: Map<(&'a K, &'a V), &'a K, Entries<'a, K, V>, fn((&'a K, &'a V)) -> &'a K> +} + +/// HashMap values iterator +pub struct Values<'a, K: 'a, V: 'a> { + inner: Map<(&'a K, &'a V), &'a V, Entries<'a, K, V>, fn((&'a K, &'a V)) -> &'a V> +} + /// A view into a single occupied location in a HashMap pub struct OccupiedEntry<'a, K:'a, V:'a> { elem: FullBucket>, @@ -1365,36 +1375,28 @@ enum VacantEntryState { } impl<'a, K, V> Iterator<(&'a K, &'a V)> for Entries<'a, K, V> { - #[inline] - fn next(&mut self) -> Option<(&'a K, &'a V)> { - self.inner.next() - } - #[inline] - fn size_hint(&self) -> (uint, Option) { - self.inner.size_hint() - } + #[inline] fn next(&mut self) -> Option<(&'a K, &'a V)> { self.inner.next() } + #[inline] fn size_hint(&self) -> (uint, Option) { self.inner.size_hint() } } impl<'a, K, V> Iterator<(&'a K, &'a mut V)> for MutEntries<'a, K, V> { - #[inline] - fn next(&mut self) -> Option<(&'a K, &'a mut V)> { - self.inner.next() - } - #[inline] - fn size_hint(&self) -> (uint, Option) { - self.inner.size_hint() - } + #[inline] fn next(&mut self) -> Option<(&'a K, &'a mut V)> { self.inner.next() } + #[inline] fn size_hint(&self) -> (uint, Option) { self.inner.size_hint() } } impl Iterator<(K, V)> for MoveEntries { - #[inline] - fn next(&mut self) -> Option<(K, V)> { - self.inner.next() - } - #[inline] - fn size_hint(&self) -> (uint, Option) { - self.inner.size_hint() - } + #[inline] fn next(&mut self) -> Option<(K, V)> { self.inner.next() } + #[inline] fn size_hint(&self) -> (uint, Option) { self.inner.size_hint() } +} + +impl<'a, K, V> Iterator<&'a K> for Keys<'a, K, V> { + #[inline] fn next(&mut self) -> Option<(&'a K)> { self.inner.next() } + #[inline] fn size_hint(&self) -> (uint, Option) { self.inner.size_hint() } +} + +impl<'a, K, V> Iterator<&'a V> for Values<'a, K, V> { + #[inline] fn next(&mut self) -> Option<(&'a V)> { self.inner.next() } + #[inline] fn size_hint(&self) -> (uint, Option) { self.inner.size_hint() } } impl<'a, K, V> OccupiedEntry<'a, K, V> { @@ -1448,14 +1450,6 @@ impl<'a, K, V> VacantEntry<'a, K, V> { } } -/// HashMap keys iterator -pub type Keys<'a, K, V> = - iter::Map<(&'a K, &'a V), &'a K, Entries<'a, K, V>, fn((&'a K, &'a V)) -> &'a K>; - -/// HashMap values iterator -pub type Values<'a, K, V> = - iter::Map<(&'a K, &'a V), &'a V, Entries<'a, K, V>, fn((&'a K, &'a V)) -> &'a V>; - impl, V, S, H: Hasher + Default> FromIterator<(K, V)> for HashMap { fn from_iter>(iter: T) -> HashMap { let (lower, _) = iter.size_hint(); diff --git a/src/libstd/collections/hash/set.rs b/src/libstd/collections/hash/set.rs index 745a8298ee8a5..3993a23140b45 100644 --- a/src/libstd/collections/hash/set.rs +++ b/src/libstd/collections/hash/set.rs @@ -22,7 +22,7 @@ use iter; use option::Option::{Some, None, mod}; use result::Result::{Ok, Err}; -use super::map::{HashMap, Entries, MoveEntries, INITIAL_CAPACITY}; +use super::map::{HashMap, MoveEntries, Keys, INITIAL_CAPACITY}; // FIXME(conventions): implement BitOr, BitAnd, BitXor, and Sub @@ -617,8 +617,7 @@ impl, S, H: Hasher + Default> Default for HashSet { } /// HashSet iterator -pub type SetItems<'a, K> = - iter::Map<(&'a K, &'a ()), &'a K, Entries<'a, K, ()>, fn((&'a K, &'a ())) -> &'a K>; +pub type SetItems<'a, K> = Keys<'a, K, ()>; /// HashSet move iterator pub type SetMoveItems = iter::Map<(K, ()), K, MoveEntries, fn((K, ())) -> K>; From e38de8a7ba65d0680b4cc4eb12ca8d75502e9eae Mon Sep 17 00:00:00 2001 From: Chase Southwood Date: Sat, 13 Dec 2014 12:36:05 -0600 Subject: [PATCH 4/4] Use wrapper structs for `HashSet`'s iterators. Using a type alias for iterator implementations is fragile since this exposes the implementation to users of the iterator, and any changes could break existing code. This commit changes the iterators of `HashSet` to use proper new types, rather than type aliases. However, since it is fair-game to treat a type-alias as the aliased type, this is a: [breaking-change]. --- src/libstd/collections/hash/set.rs | 79 ++++++++++++++++++++++-------- 1 file changed, 59 insertions(+), 20 deletions(-) diff --git a/src/libstd/collections/hash/set.rs b/src/libstd/collections/hash/set.rs index 3993a23140b45..084056600b5b0 100644 --- a/src/libstd/collections/hash/set.rs +++ b/src/libstd/collections/hash/set.rs @@ -17,9 +17,8 @@ use default::Default; use fmt::Show; use fmt; use hash::{Hash, Hasher, RandomSipHasher}; -use iter::{Iterator, IteratorExt, FromIterator, FilterMap, Chain, Repeat, Zip, Extend, repeat}; -use iter; use option::Option::{Some, None, mod}; +use iter::{Iterator, IteratorExt, FromIterator, Map, FilterMap, Chain, Repeat, Zip, Extend, repeat}; use result::Result::{Ok, Err}; use super::map::{HashMap, MoveEntries, Keys, INITIAL_CAPACITY}; @@ -252,7 +251,7 @@ impl, S, H: Hasher> HashSet { /// ``` #[unstable = "matches collection reform specification, waiting for dust to settle"] pub fn iter<'a>(&'a self) -> SetItems<'a, T> { - self.map.keys() + SetItems { iter: self.map.keys() } } /// Creates a consuming iterator, that is, one that moves each value out @@ -279,7 +278,7 @@ impl, S, H: Hasher> HashSet { pub fn into_iter(self) -> SetMoveItems { fn first((a, _): (A, B)) -> A { a } - self.map.into_iter().map(first) + SetMoveItems { iter: self.map.into_iter().map(first) } } /// Visit the values representing the difference. @@ -312,7 +311,7 @@ impl, S, H: Hasher> HashSet { if !other.contains(elt) { Some(elt) } else { None } } - repeat(other).zip(self.iter()).filter_map(filter) + SetAlgebraItems { iter: repeat(other).zip(self.iter()).filter_map(filter) } } /// Visit the values representing the symmetric difference. @@ -337,8 +336,8 @@ impl, S, H: Hasher> HashSet { /// ``` #[unstable = "matches collection reform specification, waiting for dust to settle"] pub fn symmetric_difference<'a>(&'a self, other: &'a HashSet) - -> Chain, SetAlgebraItems<'a, T, H>> { - self.difference(other).chain(other.difference(self)) + -> SymDifferenceItems<'a, T, H> { + SymDifferenceItems { iter: self.difference(other).chain(other.difference(self)) } } /// Visit the values representing the intersection. @@ -366,7 +365,7 @@ impl, S, H: Hasher> HashSet { if other.contains(elt) { Some(elt) } else { None } } - repeat(other).zip(self.iter()).filter_map(filter) + SetAlgebraItems { iter: repeat(other).zip(self.iter()).filter_map(filter) } } /// Visit the values representing the union. @@ -387,9 +386,8 @@ impl, S, H: Hasher> HashSet { /// assert_eq!(diff, [1i, 2, 3, 4].iter().map(|&x| x).collect()); /// ``` #[unstable = "matches collection reform specification, waiting for dust to settle"] - pub fn union<'a>(&'a self, other: &'a HashSet) - -> Chain, SetAlgebraItems<'a, T, H>> { - self.iter().chain(other.difference(self)) + pub fn union<'a>(&'a self, other: &'a HashSet) -> UnionItems<'a, T, H> { + UnionItems { iter: self.iter().chain(other.difference(self)) } } /// Return the number of elements in the set @@ -617,20 +615,61 @@ impl, S, H: Hasher + Default> Default for HashSet { } /// HashSet iterator -pub type SetItems<'a, K> = Keys<'a, K, ()>; +pub struct SetItems<'a, K: 'a> { + iter: Keys<'a, K, ()> +} /// HashSet move iterator -pub type SetMoveItems = iter::Map<(K, ()), K, MoveEntries, fn((K, ())) -> K>; +pub struct SetMoveItems { + iter: Map<(K, ()), K, MoveEntries, fn((K, ())) -> K> +} // `Repeat` is used to feed the filter closure an explicit capture // of a reference to the other set -/// Set operations iterator -pub type SetAlgebraItems<'a, T, H> = FilterMap< - (&'a HashSet, &'a T), - &'a T, - Zip>, SetItems<'a, T>>, - for<'b> fn((&HashSet, &'b T)) -> Option<&'b T>, ->; +/// Set operations iterator, used directly for intersection and difference +pub struct SetAlgebraItems<'a, T: 'a, H: 'a> { + iter: FilterMap< + (&'a HashSet, &'a T), + &'a T, + Zip>, SetItems<'a, T>>, + for<'b> fn((&HashSet, &'b T)) -> Option<&'b T>, + > +} + +/// Symmetric difference iterator. +pub struct SymDifferenceItems<'a, T: 'a, H: 'a> { + iter: Chain, SetAlgebraItems<'a, T, H>> +} + +/// Set union iterator. +pub struct UnionItems<'a, T: 'a, H: 'a> { + iter: Chain, SetAlgebraItems<'a, T, H>> +} + +impl<'a, K> Iterator<&'a K> for SetItems<'a, K> { + fn next(&mut self) -> Option<&'a K> { self.iter.next() } + fn size_hint(&self) -> (uint, Option) { self.iter.size_hint() } +} + +impl Iterator for SetMoveItems { + fn next(&mut self) -> Option { self.iter.next() } + fn size_hint(&self) -> (uint, Option) { self.iter.size_hint() } +} + +impl<'a, T, H> Iterator<&'a T> for SetAlgebraItems<'a, T, H> { + fn next(&mut self) -> Option<&'a T> { self.iter.next() } + fn size_hint(&self) -> (uint, Option) { self.iter.size_hint() } +} + +impl<'a, T, H> Iterator<&'a T> for SymDifferenceItems<'a, T, H> { + fn next(&mut self) -> Option<&'a T> { self.iter.next() } + fn size_hint(&self) -> (uint, Option) { self.iter.size_hint() } +} + +impl<'a, T, H> Iterator<&'a T> for UnionItems<'a, T, H> { + fn next(&mut self) -> Option<&'a T> { self.iter.next() } + fn size_hint(&self) -> (uint, Option) { self.iter.size_hint() } +} #[cfg(test)] mod test_set {