auto merge of #8294 : erickt/rust/map-move, r=bblum

According to #7887, we've decided to use the syntax of `fn map<U>(f: &fn(&T) -> U) -> U`, which passes a reference to the closure, and to `fn map_move<U>(f: &fn(T) -> U) -> U` which moves the value into the closure. This PR adds these `.map_move()` functions to `Option` and `Result`.

In addition, it has these other minor features:
 
* Replaces a couple uses of `option.get()`, `result.get()`, and `result.get_err()` with `option.unwrap()`, `result.unwrap()`, and `result.unwrap_err()`. (See #8268 and #8288 for a more thorough adaptation of this functionality.
* Removes `option.take_map()` and `option.take_map_default()`. These two functions can be easily written as `.take().map_move(...)`.
* Adds a better error message to `result.unwrap()` and `result.unwrap_err()`.

12 files changed, 92 insertions, 60 deletions

diff --git a/src/libstd/hashmap.rs b/src/libstd/hashmap.rs
index fbc471c0ae0..3484a5e7d6e 100644
--- a/src/libstd/hashmap.rs
+++ b/src/libstd/hashmap.rs
@@ -238,7 +238,7 @@ impl<K:Hash + Eq,V> HashMap<K, V> {
         let len_buckets = self.buckets.len();
         let bucket = self.buckets[idx].take();
 
-        let value = do bucket.map_consume |bucket| {
+        let value = do bucket.map_move |bucket| {
             bucket.value
         };
 
@@ -479,7 +479,7 @@ impl<K: Hash + Eq, V> HashMap<K, V> {
 impl<K: Hash + Eq, V: Clone> HashMap<K, V> {
     /// Like `find`, but returns a copy of the value.
     pub fn find_copy(&self, k: &K) -> Option<V> {
-        self.find(k).map_consume(|v| (*v).clone())
+        self.find(k).map_move(|v| (*v).clone())
     }
 
     /// Like `get`, but returns a copy of the value.
diff --git a/src/libstd/iterator.rs b/src/libstd/iterator.rs
index 1be398966bb..29f54bd10fb 100644
--- a/src/libstd/iterator.rs
+++ b/src/libstd/iterator.rs
@@ -674,7 +674,7 @@ impl<A, T: Iterator<A>> IteratorUtil<A> for T {
                     Some((y, y_val))
                 }
             }
-        }).map_consume(|(x, _)| x)
+        }).map_move(|(x, _)| x)
     }
 
     #[inline]
@@ -689,7 +689,7 @@ impl<A, T: Iterator<A>> IteratorUtil<A> for T {
                     Some((y, y_val))
                 }
             }
-        }).map_consume(|(x, _)| x)
+        }).map_move(|(x, _)| x)
     }
 }
 
@@ -1382,7 +1382,7 @@ impl<'self, A, T: Iterator<A>, B, U: Iterator<B>> Iterator<B> for
                     return Some(x)
                 }
             }
-            match self.iter.next().map_consume(|x| (self.f)(x)) {
+            match self.iter.next().map_move(|x| (self.f)(x)) {
                 None => return self.backiter.chain_mut_ref(|it| it.next()),
                 next => self.frontiter = next,
             }
@@ -1414,7 +1414,7 @@ impl<'self,
                     y => return y
                 }
             }
-            match self.iter.next_back().map_consume(|x| (self.f)(x)) {
+            match self.iter.next_back().map_move(|x| (self.f)(x)) {
                 None => return self.frontiter.chain_mut_ref(|it| it.next_back()),
                 next => self.backiter = next,
             }
diff --git a/src/libstd/local_data.rs b/src/libstd/local_data.rs
index c2a60e1c0e9..a73809d202c 100644
--- a/src/libstd/local_data.rs
+++ b/src/libstd/local_data.rs
@@ -110,16 +110,16 @@ fn test_tls_multitask() {
     set(my_key, @~"parent data");
     do task::spawn {
         // TLS shouldn't carry over.
-        assert!(get(my_key, |k| k.map(|&k| *k)).is_none());
+        assert!(get(my_key, |k| k.map_move(|k| *k)).is_none());
         set(my_key, @~"child data");
-        assert!(*(get(my_key, |k| k.map(|&k| *k)).unwrap()) ==
+        assert!(*(get(my_key, |k| k.map_move(|k| *k)).unwrap()) ==
                 ~"child data");
         // should be cleaned up for us
     }
     // Must work multiple times
-    assert!(*(get(my_key, |k| k.map(|&k| *k)).unwrap()) == ~"parent data");
-    assert!(*(get(my_key, |k| k.map(|&k| *k)).unwrap()) == ~"parent data");
-    assert!(*(get(my_key, |k| k.map(|&k| *k)).unwrap()) == ~"parent data");
+    assert!(*(get(my_key, |k| k.map_move(|k| *k)).unwrap()) == ~"parent data");
+    assert!(*(get(my_key, |k| k.map_move(|k| *k)).unwrap()) == ~"parent data");
+    assert!(*(get(my_key, |k| k.map_move(|k| *k)).unwrap()) == ~"parent data");
 }
 
 #[test]
@@ -127,7 +127,7 @@ fn test_tls_overwrite() {
     static my_key: Key<@~str> = &Key;
     set(my_key, @~"first data");
     set(my_key, @~"next data"); // Shouldn't leak.
-    assert!(*(get(my_key, |k| k.map(|&k| *k)).unwrap()) == ~"next data");
+    assert!(*(get(my_key, |k| k.map_move(|k| *k)).unwrap()) == ~"next data");
 }
 
 #[test]
diff --git a/src/libstd/option.rs b/src/libstd/option.rs
index ea1bddcdb4b..66b30d8dd03 100644
--- a/src/libstd/option.rs
+++ b/src/libstd/option.rs
@@ -208,6 +208,12 @@ impl<T> Option<T> {
         match *self { Some(ref mut x) => Some(f(x)), None => None }
     }
 
+    /// Applies a function to the contained value or returns a default
+    #[inline]
+    pub fn map_default<'a, U>(&'a self, def: U, f: &fn(&'a T) -> U) -> U {
+        match *self { None => def, Some(ref t) => f(t) }
+    }
+
     /// Maps a `Some` value from one type to another by a mutable reference,
     /// or returns a default value.
     #[inline]
@@ -218,21 +224,15 @@ impl<T> Option<T> {
     /// As `map`, but consumes the option and gives `f` ownership to avoid
     /// copying.
     #[inline]
-    pub fn map_consume<U>(self, f: &fn(v: T) -> U) -> Option<U> {
-        match self { None => None, Some(v) => Some(f(v)) }
-    }
-
-    /// Applies a function to the contained value or returns a default
-    #[inline]
-    pub fn map_default<'a, U>(&'a self, def: U, f: &fn(&'a T) -> U) -> U {
-        match *self { None => def, Some(ref t) => f(t) }
+    pub fn map_move<U>(self, f: &fn(T) -> U) -> Option<U> {
+        match self { Some(x) => Some(f(x)), None => None }
     }
 
     /// As `map_default`, but consumes the option and gives `f`
     /// ownership to avoid copying.
     #[inline]
-    pub fn map_consume_default<U>(self, def: U, f: &fn(v: T) -> U) -> U {
-        match self { None => def, Some(v) => f(v) }
+    pub fn map_move_default<U>(self, def: U, f: &fn(T) -> U) -> U {
+        match self { None => def, Some(t) => f(t) }
     }
 
     /// Take the value out of the option, leaving a `None` in its place.
@@ -241,20 +241,6 @@ impl<T> Option<T> {
         util::replace(self, None)
     }
 
-    /// As `map_consume`, but swaps a None into the original option rather
-    /// than consuming it by-value.
-    #[inline]
-    pub fn take_map<U>(&mut self, blk: &fn(T) -> U) -> Option<U> {
-        self.take().map_consume(blk)
-    }
-
-    /// As `map_consume_default`, but swaps a None into the original option
-    /// rather than consuming it by-value.
-    #[inline]
-    pub fn take_map_default<U> (&mut self, def: U, blk: &fn(T) -> U) -> U {
-        self.take().map_consume_default(def, blk)
-    }
-
     /// Apply a function to the contained value or do nothing.
     /// Returns true if the contained value was mutated.
     pub fn mutate(&mut self, f: &fn(T) -> T) -> bool {
diff --git a/src/libstd/os.rs b/src/libstd/os.rs
index b0e1f35b4a0..f246a61a4d5 100644
--- a/src/libstd/os.rs
+++ b/src/libstd/os.rs
@@ -498,9 +498,7 @@ pub fn self_exe_path() -> Option<Path> {
         }
     }
 
-    do load_self().map |pth| {
-        Path(*pth).dir_path()
-    }
+    load_self().map_move(|path| Path(path).dir_path())
 }
 
 
diff --git a/src/libstd/result.rs b/src/libstd/result.rs
index 91f42edf0ae..3e429c6116d 100644
--- a/src/libstd/result.rs
+++ b/src/libstd/result.rs
@@ -152,6 +152,40 @@ impl<T, E: ToStr> Result<T, E> {
     /// Call a method based on a previous result
     ///
     /// If `self` is `Ok` then the value is extracted and passed to `op`
+    /// whereupon `op`s result is wrapped in `Ok` and returned. if `self` is
+    /// `Err` then it is immediately returned.  This function can be used to
+    /// compose the results of two functions.
+    ///
+    /// Example:
+    ///
+    ///     let res = do read_file(file).map_move |buf| {
+    ///         parse_bytes(buf)
+    ///     }
+    #[inline]
+    pub fn map_move<U>(self, op: &fn(T) -> U) -> Result<U,E> {
+        match self {
+          Ok(t) => Ok(op(t)),
+          Err(e) => Err(e)
+        }
+    }
+
+    /// Call a method based on a previous result
+    ///
+    /// If `self` is `Err` then the value is extracted and passed to `op`
+    /// whereupon `op`s result is wrapped in an `Err` and returned. if `self` is
+    /// `Ok` then it is immediately returned.  This function can be used to pass
+    /// through a successful result while handling an error.
+    #[inline]
+    pub fn map_err_move<F>(self, op: &fn(E) -> F) -> Result<T,F> {
+        match self {
+          Ok(t) => Ok(t),
+          Err(e) => Err(op(e))
+        }
+    }
+
+    /// Call a method based on a previous result
+    ///
+    /// If `self` is `Ok` then the value is extracted and passed to `op`
     /// whereupon `op`s result is returned. if `self` is `Err` then it is
     /// immediately returned. This function can be used to compose the results
     /// of two functions.
@@ -312,7 +346,9 @@ pub fn iter_vec2<S, T, U: ToStr>(ss: &[S], ts: &[T],
 #[cfg(test)]
 mod tests {
     use super::*;
+
     use either;
+    use str::OwnedStr;
 
     pub fn op1() -> Result<int, ~str> { Ok(666) }
 
@@ -359,14 +395,26 @@ mod tests {
 
     #[test]
     pub fn test_impl_map() {
-        assert_eq!(Ok::<~str, ~str>(~"a").map(|_x| ~"b"), Ok(~"b"));
-        assert_eq!(Err::<~str, ~str>(~"a").map(|_x| ~"b"), Err(~"a"));
+        assert_eq!(Ok::<~str, ~str>(~"a").map(|x| (~"b").append(*x)), Ok(~"ba"));
+        assert_eq!(Err::<~str, ~str>(~"a").map(|x| (~"b").append(*x)), Err(~"a"));
     }
 
     #[test]
     pub fn test_impl_map_err() {
-        assert_eq!(Ok::<~str, ~str>(~"a").map_err(|_x| ~"b"), Ok(~"a"));
-        assert_eq!(Err::<~str, ~str>(~"a").map_err(|_x| ~"b"), Err(~"b"));
+        assert_eq!(Ok::<~str, ~str>(~"a").map_err(|x| (~"b").append(*x)), Ok(~"a"));
+        assert_eq!(Err::<~str, ~str>(~"a").map_err(|x| (~"b").append(*x)), Err(~"ba"));
+    }
+
+    #[test]
+    pub fn test_impl_map_move() {
+        assert_eq!(Ok::<~str, ~str>(~"a").map_move(|x| x + "b"), Ok(~"ab"));
+        assert_eq!(Err::<~str, ~str>(~"a").map_move(|x| x + "b"), Err(~"a"));
+    }
+
+    #[test]
+    pub fn test_impl_map_err_move() {
+        assert_eq!(Ok::<~str, ~str>(~"a").map_err_move(|x| x + "b"), Ok(~"a"));
+        assert_eq!(Err::<~str, ~str>(~"a").map_err_move(|x| x + "b"), Err(~"ab"));
     }
 
     #[test]
diff --git a/src/libstd/rt/comm.rs b/src/libstd/rt/comm.rs
index a060059f5fc..0cf223f3029 100644
--- a/src/libstd/rt/comm.rs
+++ b/src/libstd/rt/comm.rs
@@ -159,7 +159,7 @@ impl<T> ChanOne<T> {
                     // Port is blocked. Wake it up.
                     let recvr = BlockedTask::cast_from_uint(task_as_state);
                     if do_resched {
-                        do recvr.wake().map_consume |woken_task| {
+                        do recvr.wake().map_move |woken_task| {
                             Scheduler::run_task(woken_task);
                         };
                     } else {
@@ -381,7 +381,7 @@ impl<T> Drop for ChanOne<T> {
                     // The port is blocked waiting for a message we will never send. Wake it.
                     assert!((*this.packet()).payload.is_none());
                     let recvr = BlockedTask::cast_from_uint(task_as_state);
-                    do recvr.wake().map_consume |woken_task| {
+                    do recvr.wake().map_move |woken_task| {
                         Scheduler::run_task(woken_task);
                     };
                 }
diff --git a/src/libstd/rt/kill.rs b/src/libstd/rt/kill.rs
index 3372c13b877..789c7531eca 100644
--- a/src/libstd/rt/kill.rs
+++ b/src/libstd/rt/kill.rs
@@ -402,10 +402,10 @@ impl KillHandle {
                     || {
                         // Prefer to check tombstones that were there first,
                         // being "more fair" at the expense of tail-recursion.
-                        others.take().map_consume_default(true, |f| f()) && {
+                        others.take().map_move_default(true, |f| f()) && {
                             let mut inner = this.take().unwrap();
                             (!inner.any_child_failed) &&
-                                inner.child_tombstones.take_map_default(true, |f| f())
+                                inner.child_tombstones.take().map_move_default(true, |f| f())
                         }
                     }
                 }
@@ -424,7 +424,7 @@ impl KillHandle {
                     let others = Cell::new(other_tombstones); // :(
                     || {
                         // Prefer fairness to tail-recursion, as in above case.
-                        others.take().map_consume_default(true, |f| f()) &&
+                        others.take().map_move_default(true, |f| f()) &&
                             f.take()()
                     }
                 }
@@ -493,7 +493,7 @@ impl Death {
         { use util; util::ignore(group); }
 
         // Step 1. Decide if we need to collect child failures synchronously.
-        do self.on_exit.take_map |on_exit| {
+        do self.on_exit.take().map_move |on_exit| {
             if success {
                 // We succeeded, but our children might not. Need to wait for them.
                 let mut inner = self.kill_handle.take_unwrap().unwrap();
@@ -501,7 +501,7 @@ impl Death {
                     success = false;
                 } else {
                     // Lockless access to tombstones protected by unwrap barrier.
-                    success = inner.child_tombstones.take_map_default(true, |f| f());
+                    success = inner.child_tombstones.take().map_move_default(true, |f| f());
                 }
             }
             on_exit(success);
@@ -510,12 +510,12 @@ impl Death {
         // Step 2. Possibly alert possibly-watching parent to failure status.
         // Note that as soon as parent_handle goes out of scope, the parent
         // can successfully unwrap its handle and collect our reported status.
-        do self.watching_parent.take_map |mut parent_handle| {
+        do self.watching_parent.take().map_move |mut parent_handle| {
             if success {
                 // Our handle might be None if we had an exit callback, and
                 // already unwrapped it. But 'success' being true means no
                 // child failed, so there's nothing to do (see below case).
-                do self.kill_handle.take_map |own_handle| {
+                do self.kill_handle.take().map_move |own_handle| {
                     own_handle.reparent_children_to(&mut parent_handle);
                 };
             } else {
diff --git a/src/libstd/rt/sched.rs b/src/libstd/rt/sched.rs
index 1a75f2569b5..c2c12c6e3c0 100644
--- a/src/libstd/rt/sched.rs
+++ b/src/libstd/rt/sched.rs
@@ -325,7 +325,7 @@ impl Scheduler {
     /// As enqueue_task, but with the possibility for the blocked task to
     /// already have been killed.
     pub fn enqueue_blocked_task(&mut self, blocked_task: BlockedTask) {
-        do blocked_task.wake().map_consume |task| {
+        do blocked_task.wake().map_move |task| {
             self.enqueue_task(task);
         };
     }
@@ -533,7 +533,7 @@ impl Scheduler {
                 sched.enqueue_blocked_task(last_task);
             }
         };
-        opt.map_consume(Local::put);
+        opt.map_move(Local::put);
     }
 
     // The primary function for changing contexts. In the current
diff --git a/src/libstd/rt/task.rs b/src/libstd/rt/task.rs
index 4c5e4bdc3c1..e732ef67b5b 100644
--- a/src/libstd/rt/task.rs
+++ b/src/libstd/rt/task.rs
@@ -465,10 +465,10 @@ mod test {
         do run_in_newsched_task() {
             static key: local_data::Key<@~str> = &local_data::Key;
             local_data::set(key, @~"data");
-            assert!(*local_data::get(key, |k| k.map(|&k| *k)).unwrap() == ~"data");
+            assert!(*local_data::get(key, |k| k.map_move(|k| *k)).unwrap() == ~"data");
             static key2: local_data::Key<@~str> = &local_data::Key;
             local_data::set(key2, @~"data");
-            assert!(*local_data::get(key2, |k| k.map(|&k| *k)).unwrap() == ~"data");
+            assert!(*local_data::get(key2, |k| k.map_move(|k| *k)).unwrap() == ~"data");
         }
     }
 
diff --git a/src/libstd/str.rs b/src/libstd/str.rs
index cd7aac020ed..c4bd2c5435a 100644
--- a/src/libstd/str.rs
+++ b/src/libstd/str.rs
@@ -1849,7 +1849,7 @@ impl<'self> StrSlice<'self> for &'self str {
         } else {
             self.matches_index_iter(needle)
                 .next()
-                .map_consume(|(start, _end)| start)
+                .map_move(|(start, _end)| start)
         }
     }
 
diff --git a/src/libstd/task/spawn.rs b/src/libstd/task/spawn.rs
index 527b20b0e90..7486a78837c 100644
--- a/src/libstd/task/spawn.rs
+++ b/src/libstd/task/spawn.rs
@@ -500,7 +500,7 @@ impl RuntimeGlue {
             OldTask(ptr) => rt::rust_task_kill_other(ptr),
             NewTask(handle) => {
                 let mut handle = handle;
-                do handle.kill().map_consume |killed_task| {
+                do handle.kill().map_move |killed_task| {
                     let killed_task = Cell::new(killed_task);
                     do Local::borrow::<Scheduler, ()> |sched| {
                         sched.enqueue_task(killed_task.take());
@@ -682,7 +682,7 @@ fn spawn_raw_newsched(mut opts: TaskOpts, f: ~fn()) {
         // Child task runs this code.
 
         // If child data is 'None', the enlist is vacuously successful.
-        let enlist_success = do child_data.take().map_consume_default(true) |child_data| {
+        let enlist_success = do child_data.take().map_move_default(true) |child_data| {
             let child_data = Cell::new(child_data); // :(
             do Local::borrow::<Task, bool> |me| {
                 let (child_tg, ancestors, is_main) = child_data.take();
@@ -854,7 +854,7 @@ fn spawn_raw_oldsched(mut opts: TaskOpts, f: ~fn()) {
             // Even if the below code fails to kick the child off, we must
             // send Something on the notify channel.
 
-            let notifier = notify_chan.map_consume(|c| AutoNotify(c));
+            let notifier = notify_chan.map_move(|c| AutoNotify(c));
 
             if enlist_many(OldTask(child), &child_arc, &mut ancestors) {
                 let group = @@mut Taskgroup(child_arc, ancestors, is_main, notifier);