auto merge of #8526 : blake2-ppc/rust/either-result, r=catamorphism

Retry of PR #8471

Replace the remaining functions marked for issue #8228 with similar functions that are iterator-based.

Change `either::{lefts, rights}` to be iterator-filtering instead of returning a vector.

Replace `map_vec`, `map_vec2`, `iter_vec2` in std::result with three functions:

* `result::collect` gathers `Iterator<Result<V, U>>` to `Result<~[V], U>`
* `result::fold` folds `Iterator<Result<T, E>>` to `Result<V, E>`
* `result::fold_` folds `Iterator<Result<T, E>>` to `Result<(), E>`

2 files changed, 115 insertions, 82 deletions

diff --git a/src/libstd/either.rs b/src/libstd/either.rs
index 132ebc72960..5d988965e8c 100644
--- a/src/libstd/either.rs
+++ b/src/libstd/either.rs
@@ -16,7 +16,7 @@ use option::{Some, None};
 use clone::Clone;
 use container::Container;
 use cmp::Eq;
-use iterator::Iterator;
+use iterator::{Iterator, FilterMap};
 use result::Result;
 use result;
 use str::StrSlice;
@@ -116,40 +116,44 @@ impl<L, R> Either<L, R> {
     }
 }
 
-// FIXME: #8228 Replaceable by an external iterator?
-/// Extracts from a vector of either all the left values
-pub fn lefts<L: Clone, R>(eithers: &[Either<L, R>]) -> ~[L] {
-    do vec::build_sized(eithers.len()) |push| {
-        for elt in eithers.iter() {
-            match *elt {
-                Left(ref l) => { push((*l).clone()); }
-                _ => { /* fallthrough */ }
-            }
+/// An iterator yielding the `Left` values of its source
+pub type Lefts<L, R, Iter> = FilterMap<'static, Either<L, R>, L, Iter>;
+
+/// An iterator yielding the `Right` values of its source
+pub type Rights<L, R, Iter> = FilterMap<'static, Either<L, R>, R, Iter>;
+
+/// Extracts all the left values
+pub fn lefts<L, R, Iter: Iterator<Either<L, R>>>(eithers: Iter)
+    -> Lefts<L, R, Iter> {
+    do eithers.filter_map |elt| {
+        match elt {
+            Left(x) => Some(x),
+            _ => None,
         }
     }
 }
 
-// FIXME: #8228 Replaceable by an external iterator?
-/// Extracts from a vector of either all the right values
-pub fn rights<L, R: Clone>(eithers: &[Either<L, R>]) -> ~[R] {
-    do vec::build_sized(eithers.len()) |push| {
-        for elt in eithers.iter() {
-            match *elt {
-                Right(ref r) => { push((*r).clone()); }
-                _ => { /* fallthrough */ }
-            }
+/// Extracts all the right values
+pub fn rights<L, R, Iter: Iterator<Either<L, R>>>(eithers: Iter)
+    -> Rights<L, R, Iter> {
+    do eithers.filter_map |elt| {
+        match elt {
+            Right(x) => Some(x),
+            _ => None,
         }
     }
 }
 
+
 // FIXME: #8228 Replaceable by an external iterator?
 /// Extracts from a vector of either all the left values and right values
 ///
 /// Returns a structure containing a vector of left values and a vector of
 /// right values.
 pub fn partition<L, R>(eithers: ~[Either<L, R>]) -> (~[L], ~[R]) {
-    let mut lefts: ~[L] = ~[];
-    let mut rights: ~[R] = ~[];
+    let n_lefts = eithers.iter().count(|elt| elt.is_left());
+    let mut lefts = vec::with_capacity(n_lefts);
+    let mut rights = vec::with_capacity(eithers.len() - n_lefts);
     for elt in eithers.move_iter() {
         match elt {
             Left(l) => lefts.push(l),
@@ -182,42 +186,42 @@ mod tests {
     #[test]
     fn test_lefts() {
         let input = ~[Left(10), Right(11), Left(12), Right(13), Left(14)];
-        let result = lefts(input);
+        let result = lefts(input.move_iter()).to_owned_vec();
         assert_eq!(result, ~[10, 12, 14]);
     }
 
     #[test]
     fn test_lefts_none() {
         let input: ~[Either<int, int>] = ~[Right(10), Right(10)];
-        let result = lefts(input);
+        let result = lefts(input.move_iter()).to_owned_vec();
         assert_eq!(result.len(), 0u);
     }
 
     #[test]
     fn test_lefts_empty() {
         let input: ~[Either<int, int>] = ~[];
-        let result = lefts(input);
+        let result = lefts(input.move_iter()).to_owned_vec();
         assert_eq!(result.len(), 0u);
     }
 
     #[test]
     fn test_rights() {
         let input = ~[Left(10), Right(11), Left(12), Right(13), Left(14)];
-        let result = rights(input);
+        let result = rights(input.move_iter()).to_owned_vec();
         assert_eq!(result, ~[11, 13]);
     }
 
     #[test]
     fn test_rights_none() {
         let input: ~[Either<int, int>] = ~[Left(10), Left(10)];
-        let result = rights(input);
+        let result = rights(input.move_iter()).to_owned_vec();
         assert_eq!(result.len(), 0u);
     }
 
     #[test]
     fn test_rights_empty() {
         let input: ~[Either<int, int>] = ~[];
-        let result = rights(input);
+        let result = rights(input.move_iter()).to_owned_vec();
         assert_eq!(result.len(), 0u);
     }
 
diff --git a/src/libstd/result.rs b/src/libstd/result.rs
index 9de5e69148a..9ae901d60bc 100644
--- a/src/libstd/result.rs
+++ b/src/libstd/result.rs
@@ -18,8 +18,7 @@ use either;
 use iterator::Iterator;
 use option::{None, Option, Some, OptionIterator};
 use vec;
-use vec::{OwnedVector, ImmutableVector};
-use container::Container;
+use vec::OwnedVector;
 use to_str::ToStr;
 use str::StrSlice;
 
@@ -269,86 +268,76 @@ pub fn map_opt<T, U: ToStr, V>(o_t: &Option<T>,
     }
 }
 
-// FIXME: #8228 Replaceable by an external iterator?
-/// Maps each element in the vector `ts` using the operation `op`.  Should an
-/// error occur, no further mappings are performed and the error is returned.
-/// Should no error occur, a vector containing the result of each map is
-/// returned.
+/// Takes each element in the iterator: if it is an error, no further
+/// elements are taken, and the error is returned.
+/// Should no error occur, a vector containing the values of each Result
+/// is returned.
 ///
 /// Here is an example which increments every integer in a vector,
 /// checking for overflow:
 ///
-///     fn inc_conditionally(x: uint) -> result<uint,str> {
+///     fn inc_conditionally(x: uint) -> Result<uint, &'static str> {
 ///         if x == uint::max_value { return Err("overflow"); }
 ///         else { return Ok(x+1u); }
 ///     }
-///     map(~[1u, 2u, 3u], inc_conditionally).chain {|incd|
-///         assert!(incd == ~[2u, 3u, 4u]);
-///     }
+///     let v = [1u, 2, 3];
+///     let res = collect(v.iter().map(|&x| inc_conditionally(x)));
+///     assert!(res == Ok(~[2u, 3, 4]));
 #[inline]
-pub fn map_vec<T,U,V>(ts: &[T], op: &fn(&T) -> Result<V,U>)
-                      -> Result<~[V],U> {
-    let mut vs: ~[V] = vec::with_capacity(ts.len());
-    for t in ts.iter() {
-        match op(t) {
-          Ok(v) => vs.push(v),
-          Err(u) => return Err(u)
+pub fn collect<T, E, Iter: Iterator<Result<T, E>>>(mut iterator: Iter)
+    -> Result<~[T], E> {
+    let (lower, _) = iterator.size_hint();
+    let mut vs: ~[T] = vec::with_capacity(lower);
+    for t in iterator {
+        match t {
+            Ok(v) => vs.push(v),
+            Err(u) => return Err(u)
         }
     }
-    return Ok(vs);
+    Ok(vs)
 }
 
-// FIXME: #8228 Replaceable by an external iterator?
-/// Same as map, but it operates over two parallel vectors.
+/// Perform a fold operation over the result values from an iterator.
 ///
-/// A precondition is used here to ensure that the vectors are the same
-/// length.  While we do not often use preconditions in the standard
-/// library, a precondition is used here because result::t is generally
-/// used in 'careful' code contexts where it is both appropriate and easy
-/// to accommodate an error like the vectors being of different lengths.
+/// If an `Err` is encountered, it is immediately returned.
+/// Otherwise, the folded value is returned.
 #[inline]
-pub fn map_vec2<S, T, U: ToStr, V>(ss: &[S], ts: &[T],
-                                   op: &fn(&S,&T) -> Result<V,U>) -> Result<~[V],U> {
-    assert!(vec::same_length(ss, ts));
-    let n = ts.len();
-    let mut vs = vec::with_capacity(n);
-    let mut i = 0u;
-    while i < n {
-        match op(&ss[i],&ts[i]) {
-          Ok(v) => vs.push(v),
-          Err(u) => return Err(u)
+pub fn fold<T, V, E,
+            Iter: Iterator<Result<T, E>>>(
+            mut iterator: Iter,
+            mut init: V,
+            f: &fn(V, T) -> V)
+         -> Result<V, E> {
+    for t in iterator {
+        match t {
+            Ok(v) => init = f(init, v),
+            Err(u) => return Err(u)
         }
-        i += 1u;
     }
-    return Ok(vs);
+    Ok(init)
 }
 
-// FIXME: #8228 Replaceable by an external iterator?
-/// Applies op to the pairwise elements from `ss` and `ts`, aborting on
-/// error.  This could be implemented using `map_zip()` but it is more efficient
-/// on its own as no result vector is built.
+/// Perform a trivial fold operation over the result values
+/// from an iterator.
+///
+/// If an `Err` is encountered, it is immediately returned.
+/// Otherwise, a simple `Ok(())` is returned.
 #[inline]
-pub fn iter_vec2<S, T, U: ToStr>(ss: &[S], ts: &[T],
-                                 op: &fn(&S,&T) -> Result<(),U>) -> Result<(),U> {
-    assert!(vec::same_length(ss, ts));
-    let n = ts.len();
-    let mut i = 0u;
-    while i < n {
-        match op(&ss[i],&ts[i]) {
-          Ok(()) => (),
-          Err(u) => return Err(u)
-        }
-        i += 1u;
-    }
-    return Ok(());
+pub fn fold_<T, E, Iter: Iterator<Result<T, E>>>(
+             iterator: Iter)
+          -> Result<(), E> {
+    fold(iterator, (), |_, _| ())
 }
 
+
 #[cfg(test)]
 mod tests {
     use super::*;
 
     use either;
+    use iterator::range;
     use str::OwnedStr;
+    use vec::ImmutableVector;
 
     pub fn op1() -> Result<int, ~str> { Ok(666) }
 
@@ -431,4 +420,44 @@ mod tests {
         assert_eq!(r.to_either(), either::Right(100));
         assert_eq!(err.to_either(), either::Left(404));
     }
+
+    #[test]
+    fn test_collect() {
+        assert_eq!(collect(range(0, 0)
+                           .map(|_| Ok::<int, ()>(0))),
+                   Ok(~[]));
+        assert_eq!(collect(range(0, 3)
+                           .map(|x| Ok::<int, ()>(x))),
+                   Ok(~[0, 1, 2]));
+        assert_eq!(collect(range(0, 3)
+                           .map(|x| if x > 1 { Err(x) } else { Ok(x) })),
+                   Err(2));
+
+        // test that it does not take more elements than it needs
+        let functions = [|| Ok(()), || Err(1), || fail!()];
+
+        assert_eq!(collect(functions.iter().map(|f| (*f)())),
+                   Err(1));
+    }
+
+    #[test]
+    fn test_fold() {
+        assert_eq!(fold_(range(0, 0)
+                        .map(|_| Ok::<(), ()>(()))),
+                   Ok(()));
+        assert_eq!(fold(range(0, 3)
+                        .map(|x| Ok::<int, ()>(x)),
+                        0, |a, b| a + b),
+                   Ok(3));
+        assert_eq!(fold_(range(0, 3)
+                        .map(|x| if x > 1 { Err(x) } else { Ok(()) })),
+                   Err(2));
+
+        // test that it does not take more elements than it needs
+        let functions = [|| Ok(()), || Err(1), || fail!()];
+
+        assert_eq!(fold_(functions.iter()
+                        .map(|f| (*f)())),
+                   Err(1));
+    }
 }