auto merge of #5796 : nikomatsakis/rust/issue-5656-fix-map-iteration, r=nikomatsakis

Revert map.each to something which takes two parameters rather than a tuple.  The current setup iterates over `BaseIter<(&'self K, &'self V)>` where 'self is a lifetime declared *in the `each()` method*.  You can't place such a type in the impl declaration.  The compiler currently allows it, but this will not be legal under #5656 and I'm pretty sure it's not sound now.  It's too bad that maps can't implement `BaseIter` (at least not over a tuple as they do here) but I think it has to be this way for the time being.

r? @thestinger

6 files changed, 56 insertions, 68 deletions

diff --git a/src/libstd/json.rs b/src/libstd/json.rs
index d733a60f34f..90a745aaeb9 100644
--- a/src/libstd/json.rs
+++ b/src/libstd/json.rs
@@ -926,7 +926,7 @@ impl Eq for Json {
                     &Object(ref d1) => {
                         if d0.len() == d1.len() {
                             let mut equal = true;
-                            for d0.each |&(k, v0)| {
+                            for d0.each |k, v0| {
                                 match d1.find(k) {
                                     Some(v1) if v0 == v1 => { },
                                     _ => { equal = false; break }
@@ -989,12 +989,12 @@ impl Ord for Json {
                         let mut d1_flat = ~[];
 
                         // FIXME #4430: this is horribly inefficient...
-                        for d0.each |&(k, v)| {
+                        for d0.each |k, v| {
                              d0_flat.push((@copy *k, @copy *v));
                         }
                         d0_flat.qsort();
 
-                        for d1.each |&(k, v)| {
+                        for d1.each |k, v| {
                             d1_flat.push((@copy *k, @copy *v));
                         }
                         d1_flat.qsort();
@@ -1125,7 +1125,7 @@ impl<A:ToJson> ToJson for ~[A] {
 impl<A:ToJson + Copy> ToJson for HashMap<~str, A> {
     fn to_json(&self) -> Json {
         let mut d = HashMap::new();
-        for self.each |&(key, value)| {
+        for self.each |key, value| {
             d.insert(copy *key, value.to_json());
         }
         Object(~d)
diff --git a/src/libstd/net_url.rs b/src/libstd/net_url.rs
index f7ffa7435cf..b8e0d9d9b2a 100644
--- a/src/libstd/net_url.rs
+++ b/src/libstd/net_url.rs
@@ -216,7 +216,7 @@ pub fn encode_form_urlencoded(m: &HashMap<~str, ~[~str]>) -> ~str {
     let mut out = ~"";
     let mut first = true;
 
-    for m.each |&(key, values)| {
+    for m.each |key, values| {
         let key = encode_plus(*key);
 
         for values.each |value| {
diff --git a/src/libstd/serialize.rs b/src/libstd/serialize.rs
index e1ab59fb2b3..c2f0d9cb43f 100644
--- a/src/libstd/serialize.rs
+++ b/src/libstd/serialize.rs
@@ -595,7 +595,7 @@ impl<
     fn encode(&self, e: &E) {
         do e.emit_map(self.len()) {
             let mut i = 0;
-            for self.each |&(key, val)| {
+            for self.each |key, val| {
                 e.emit_map_elt_key(i, || key.encode(e));
                 e.emit_map_elt_val(i, || val.encode(e));
                 i += 1;
@@ -659,7 +659,7 @@ impl<
     fn encode(&self, e: &E) {
         do e.emit_map(self.len()) {
             let mut i = 0;
-            for self.each |&(key, val)| {
+            for self.each |key, val| {
                 e.emit_map_elt_key(i, || key.encode(e));
                 e.emit_map_elt_val(i, || val.encode(e));
                 i += 1;
@@ -717,7 +717,7 @@ impl<
     fn encode(&self, e: &E) {
         do e.emit_map(self.len()) {
             let mut i = 0;
-            for self.each |&(key, val)| {
+            for self.each |key, val| {
                 e.emit_map_elt_key(i, || key.encode(e));
                 e.emit_map_elt_val(i, || val.encode(e));
                 i += 1;
diff --git a/src/libstd/smallintmap.rs b/src/libstd/smallintmap.rs
index b6c5ec03068..811cd710a62 100644
--- a/src/libstd/smallintmap.rs
+++ b/src/libstd/smallintmap.rs
@@ -14,7 +14,7 @@
  */
 
 use core::container::{Container, Mutable, Map, Set};
-use core::iter::{BaseIter, ReverseIter};
+use core::iter::{BaseIter};
 use core::option::{Some, None};
 use core::prelude::*;
 
@@ -22,32 +22,6 @@ pub struct SmallIntMap<T> {
     priv v: ~[Option<T>],
 }
 
-impl<'self, V> BaseIter<(uint, &'self V)> for SmallIntMap<V> {
-    /// Visit all key-value pairs in order
-    fn each(&self, it: &fn(&(uint, &'self V)) -> bool) {
-        for uint::range(0, self.v.len()) |i| {
-            match self.v[i] {
-              Some(ref elt) => if !it(&(i, elt)) { break },
-              None => ()
-            }
-        }
-    }
-
-    fn size_hint(&self) -> Option<uint> { Some(self.len()) }
-}
-
-impl<'self, V> ReverseIter<(uint, &'self V)> for SmallIntMap<V> {
-    /// Visit all key-value pairs in reverse order
-    fn each_reverse(&self, it: &fn(&(uint, &'self V)) -> bool) {
-        for uint::range_rev(self.v.len(), 0) |i| {
-            match self.v[i - 1] {
-              Some(ref elt) => if !it(&(i - 1, elt)) { break },
-              None => ()
-            }
-        }
-    }
-}
-
 impl<V> Container for SmallIntMap<V> {
     /// Return the number of elements in the map
     fn len(&const self) -> uint {
@@ -76,14 +50,24 @@ impl<V> Map<uint, V> for SmallIntMap<V> {
         self.find(key).is_some()
     }
 
+    /// Visit all key-value pairs in order
+    fn each(&self, it: &fn(&uint, &'self V) -> bool) {
+        for uint::range(0, self.v.len()) |i| {
+            match self.v[i] {
+              Some(ref elt) => if !it(&i, elt) { break },
+              None => ()
+            }
+        }
+    }
+
     /// Visit all keys in order
     fn each_key(&self, blk: &fn(key: &uint) -> bool) {
-        self.each(|&(k, _)| blk(&k))
+        self.each(|k, _| blk(k))
     }
 
     /// Visit all values in order
     fn each_value(&self, blk: &fn(value: &V) -> bool) {
-        self.each(|&(_, v)| blk(v))
+        self.each(|_, v| blk(v))
     }
 
     /// Iterate over the map and mutate the contained values
@@ -149,6 +133,16 @@ pub impl<V> SmallIntMap<V> {
     /// Create an empty SmallIntMap
     fn new() -> SmallIntMap<V> { SmallIntMap{v: ~[]} }
 
+    /// Visit all key-value pairs in reverse order
+    fn each_reverse(&self, it: &fn(uint, &'self V) -> bool) {
+        for uint::range_rev(self.v.len(), 0) |i| {
+            match self.v[i - 1] {
+              Some(ref elt) => if !it(i - 1, elt) { break },
+              None => ()
+            }
+        }
+    }
+
     fn get(&self, key: &uint) -> &'self V {
         self.find(key).expect("key not present")
     }
diff --git a/src/libstd/treemap.rs b/src/libstd/treemap.rs
index d0868da4408..041ea855cb3 100644
--- a/src/libstd/treemap.rs
+++ b/src/libstd/treemap.rs
@@ -82,24 +82,6 @@ impl<K: Ord + TotalOrd, V> Ord for TreeMap<K, V> {
     fn gt(&self, other: &TreeMap<K, V>) -> bool { lt(other, self) }
 }
 
-impl<'self, K: TotalOrd, V> BaseIter<(&'self K, &'self V)> for TreeMap<K, V> {
-    /// Visit all key-value pairs in order
-    fn each(&self, f: &fn(&(&'self K, &'self V)) -> bool) {
-        each(&self.root, f)
-    }
-    fn size_hint(&self) -> Option<uint> { Some(self.len()) }
-}
-
-impl<'self, K: TotalOrd, V>
-    ReverseIter<(&'self K, &'self V)>
-    for TreeMap<K, V>
-{
-    /// Visit all key-value pairs in reverse order
-    fn each_reverse(&self, f: &fn(&(&'self K, &'self V)) -> bool) {
-        each_reverse(&self.root, f);
-    }
-}
-
 impl<K: TotalOrd, V> Container for TreeMap<K, V> {
     /// Return the number of elements in the map
     fn len(&const self) -> uint { self.length }
@@ -122,12 +104,19 @@ impl<K: TotalOrd, V> Map<K, V> for TreeMap<K, V> {
         self.find(key).is_some()
     }
 
+    /// Visit all key-value pairs in order
+    fn each(&self, f: &fn(&'self K, &'self V) -> bool) {
+        each(&self.root, f)
+    }
+
     /// Visit all keys in order
-    fn each_key(&self, f: &fn(&K) -> bool) { self.each(|&(k, _)| f(k)) }
+    fn each_key(&self, f: &fn(&K) -> bool) {
+        self.each(|k, _| f(k))
+    }
 
     /// Visit all values in order
     fn each_value(&self, f: &fn(&V) -> bool) {
-        self.each(|&(_, v)| f(v))
+        self.each(|_, v| f(v))
     }
 
     /// Iterate over the map and mutate the contained values
@@ -180,14 +169,19 @@ pub impl<K: TotalOrd, V> TreeMap<K, V> {
     /// Create an empty TreeMap
     fn new() -> TreeMap<K, V> { TreeMap{root: None, length: 0} }
 
+    /// Visit all key-value pairs in reverse order
+    fn each_reverse(&'self self, f: &fn(&'self K, &'self V) -> bool) {
+        each_reverse(&self.root, f);
+    }
+
     /// Visit all keys in reverse order
     fn each_key_reverse(&self, f: &fn(&K) -> bool) {
-        self.each_reverse(|&(k, _)| f(k))
+        self.each_reverse(|k, _| f(k))
     }
 
     /// Visit all values in reverse order
     fn each_value_reverse(&self, f: &fn(&V) -> bool) {
-        self.each_reverse(|&(_, v)| f(v))
+        self.each_reverse(|_, v| f(v))
     }
 
     /// Get a lazy iterator over the key-value pairs in the map.
@@ -538,18 +532,18 @@ pub impl<K: TotalOrd, V> TreeNode<K, V> {
 }
 
 fn each<'r, K: TotalOrd, V>(node: &'r Option<~TreeNode<K, V>>,
-                            f: &fn(&(&'r K, &'r V)) -> bool) {
+                            f: &fn(&'r K, &'r V) -> bool) {
     for node.each |x| {
         each(&x.left, f);
-        if f(&(&x.key, &x.value)) { each(&x.right, f) }
+        if f(&x.key, &x.value) { each(&x.right, f) }
     }
 }
 
 fn each_reverse<'r, K: TotalOrd, V>(node: &'r Option<~TreeNode<K, V>>,
-                                    f: &fn(&(&'r K, &'r V)) -> bool) {
+                                    f: &fn(&'r K, &'r V) -> bool) {
     for node.each |x| {
         each_reverse(&x.right, f);
-        if f(&(&x.key, &x.value)) { each_reverse(&x.left, f) }
+        if f(&x.key, &x.value) { each_reverse(&x.left, f) }
     }
 }
 
@@ -796,7 +790,7 @@ mod test_treemap {
             let &(k, v) = x;
             assert!(map.find(&k).unwrap() == &v)
         }
-        for map.each |&(map_k, map_v)| {
+        for map.each |map_k, map_v| {
             let mut found = false;
             for ctrl.each |x| {
                 let &(ctrl_k, ctrl_v) = x;
@@ -912,7 +906,7 @@ mod test_treemap {
         assert!(m.insert(1, 2));
 
         let mut n = 0;
-        for m.each |&(k, v)| {
+        for m.each |k, v| {
             assert!(*k == n);
             assert!(*v == n * 2);
             n += 1;
@@ -930,7 +924,7 @@ mod test_treemap {
         assert!(m.insert(1, 2));
 
         let mut n = 4;
-        for m.each_reverse |&(k, v)| {
+        for m.each_reverse |k, v| {
             assert!(*k == n);
             assert!(*v == n * 2);
             n -= 1;
diff --git a/src/libstd/workcache.rs b/src/libstd/workcache.rs
index 3e494d0236e..c4b450810aa 100644
--- a/src/libstd/workcache.rs
+++ b/src/libstd/workcache.rs
@@ -145,7 +145,7 @@ impl WorkMap {
 impl<S:Encoder> Encodable<S> for WorkMap {
     fn encode(&self, s: &S) {
         let mut d = ~[];
-        for self.each |&(k, v)| {
+        for self.each |k, v| {
             d.push((copy *k, copy *v))
         }
         sort::tim_sort(d);
@@ -319,7 +319,7 @@ impl TPrep for Prep {
     }
 
     fn all_fresh(&self, cat: &str, map: &WorkMap) -> bool {
-        for map.each |&(k, v)| {
+        for map.each |k, v| {
             if ! self.is_fresh(cat, k.kind, k.name, *v) {
                 return false;
             }