Auto merge of #27826 - sfackler:wait-timeout-enum, r=alexcrichton

Returning a primitive bool results in a somewhat confusing API - does
`true` indicate success - i.e. no timeout, or that a timeout has
occurred? An explicitly named enum makes it clearer.

[breaking-change]

r? @alexcrichton 

2 files changed, 42 insertions, 21 deletions

diff --git a/src/libstd/sync/condvar.rs b/src/libstd/sync/condvar.rs
index fc0e08139f4..851726d91c6 100644
--- a/src/libstd/sync/condvar.rs
+++ b/src/libstd/sync/condvar.rs
@@ -18,6 +18,20 @@ use sys_common::poison::{self, LockResult};
 use sys::time::SteadyTime;
 use time::Duration;
 
+/// A type indicating whether a timed wait on a condition variable returned
+/// due to a time out or not.
+#[derive(Debug, PartialEq, Eq, Copy, Clone)]
+#[unstable(feature = "wait_timeout", reason = "newly added", issue = "27772")]
+pub struct WaitTimeoutResult(bool);
+
+impl WaitTimeoutResult {
+    /// Returns whether the wait was known to have timed out.
+    #[unstable(feature = "wait_timeout", reason = "newly added", issue = "27772")]
+    pub fn timed_out(&self) -> bool {
+        self.0
+    }
+}
+
 /// A Condition Variable
 ///
 /// Condition variables represent the ability to block a thread such that it
@@ -170,8 +184,8 @@ impl Condvar {
     /// preemption or platform differences that may not cause the maximum
     /// amount of time waited to be precisely `dur`.
     ///
-    /// The returned boolean is `false` only if the timeout is known
-    /// to have elapsed.
+    /// The returned `WaitTimeoutResult` value indicates if the timeout is
+    /// known to have elapsed.
     ///
     /// Like `wait`, the lock specified will be re-acquired when this function
     /// returns, regardless of whether the timeout elapsed or not.
@@ -179,7 +193,7 @@ impl Condvar {
                issue = "27772")]
     pub fn wait_timeout<'a, T>(&self, guard: MutexGuard<'a, T>,
                                dur: Duration)
-                               -> LockResult<(MutexGuard<'a, T>, bool)> {
+                               -> LockResult<(MutexGuard<'a, T>, WaitTimeoutResult)> {
         unsafe {
             let me: &'static Condvar = &*(self as *const _);
             me.inner.wait_timeout(guard, dur)
@@ -199,7 +213,7 @@ impl Condvar {
                                        guard: MutexGuard<'a, T>,
                                        dur: Duration,
                                        f: F)
-                                       -> LockResult<(MutexGuard<'a, T>, bool)>
+                                       -> LockResult<(MutexGuard<'a, T>, WaitTimeoutResult)>
             where F: FnMut(LockResult<&mut T>) -> bool {
         unsafe {
             let me: &'static Condvar = &*(self as *const _);
@@ -278,7 +292,13 @@ impl StaticCondvar {
                issue = "27717")]
     pub fn wait_timeout_ms<'a, T>(&'static self, guard: MutexGuard<'a, T>, ms: u32)
                                   -> LockResult<(MutexGuard<'a, T>, bool)> {
-        self.wait_timeout(guard, Duration::from_millis(ms as u64))
+        match self.wait_timeout(guard, Duration::from_millis(ms as u64)) {
+            Ok((guard, timed_out)) => Ok((guard, !timed_out.timed_out())),
+            Err(poison) => {
+                let (guard, timed_out) = poison.into_inner();
+                Err(PoisonError::new((guard, !timed_out.timed_out())))
+            }
+        }
     }
 
     /// Waits on this condition variable for a notification, timing out after a
@@ -291,17 +311,17 @@ impl StaticCondvar {
     pub fn wait_timeout<'a, T>(&'static self,
                                guard: MutexGuard<'a, T>,
                                timeout: Duration)
-                               -> LockResult<(MutexGuard<'a, T>, bool)> {
-        let (poisoned, success) = unsafe {
+                               -> LockResult<(MutexGuard<'a, T>, WaitTimeoutResult)> {
+        let (poisoned, result) = unsafe {
             let lock = mutex::guard_lock(&guard);
             self.verify(lock);
             let success = self.inner.wait_timeout(lock, timeout);
-            (mutex::guard_poison(&guard).get(), success)
+            (mutex::guard_poison(&guard).get(), WaitTimeoutResult(!success))
         };
         if poisoned {
-            Err(PoisonError::new((guard, success)))
+            Err(PoisonError::new((guard, result)))
         } else {
-            Ok((guard, success))
+            Ok((guard, result))
         }
     }
 
@@ -319,7 +339,7 @@ impl StaticCondvar {
                                        guard: MutexGuard<'a, T>,
                                        dur: Duration,
                                        mut f: F)
-                                       -> LockResult<(MutexGuard<'a, T>, bool)>
+                                       -> LockResult<(MutexGuard<'a, T>, WaitTimeoutResult)>
             where F: FnMut(LockResult<&mut T>) -> bool {
         // This could be made more efficient by pushing the implementation into
         // sys::condvar
@@ -332,11 +352,11 @@ impl StaticCondvar {
             let now = SteadyTime::now();
             let consumed = &now - &start;
             let guard = guard_result.unwrap_or_else(|e| e.into_inner());
-            let (new_guard_result, no_timeout) = if consumed > dur {
-                (Ok(guard), false)
+            let (new_guard_result, timed_out) = if consumed > dur {
+                (Ok(guard), WaitTimeoutResult(true))
             } else {
                 match self.wait_timeout(guard, dur - consumed) {
-                    Ok((new_guard, no_timeout)) => (Ok(new_guard), no_timeout),
+                    Ok((new_guard, timed_out)) => (Ok(new_guard), timed_out),
                     Err(err) => {
                         let (new_guard, no_timeout) = err.into_inner();
                         (Err(PoisonError::new(new_guard)), no_timeout)
@@ -344,16 +364,17 @@ impl StaticCondvar {
                 }
             };
             guard_result = new_guard_result;
-            if !no_timeout {
+            if timed_out.timed_out() {
                 let result = f(guard_result
                                     .as_mut()
                                     .map(|g| &mut **g)
                                     .map_err(|e| PoisonError::new(&mut **e.get_mut())));
+                let result = WaitTimeoutResult(!result);
                 return poison::map_result(guard_result, |g| (g, result));
             }
         }
 
-        poison::map_result(guard_result, |g| (g, true))
+        poison::map_result(guard_result, |g| (g, WaitTimeoutResult(false)))
     }
 
     /// Wakes up one blocked thread on this condvar.
@@ -508,10 +529,10 @@ mod tests {
         static S: AtomicUsize = AtomicUsize::new(0);
 
         let g = M.lock().unwrap();
-        let (g, success) = C.wait_timeout_with(g, Duration::new(0, 1000), |_| {
+        let (g, timed_out) = C.wait_timeout_with(g, Duration::new(0, 1000), |_| {
             false
         }).unwrap();
-        assert!(!success);
+        assert!(timed_out.timed_out());
 
         let (tx, rx) = channel();
         let _t = thread::spawn(move || {
@@ -535,7 +556,7 @@ mod tests {
 
         let mut state = 0;
         let day = 24 * 60 * 60;
-        let (_g, success) = C.wait_timeout_with(g, Duration::new(day, 0), |_| {
+        let (_g, timed_out) = C.wait_timeout_with(g, Duration::new(day, 0), |_| {
             assert_eq!(state, S.load(Ordering::SeqCst));
             tx.send(()).unwrap();
             state += 1;
@@ -544,7 +565,7 @@ mod tests {
                 _ => true,
             }
         }).unwrap();
-        assert!(success);
+        assert!(!timed_out.timed_out());
     }
 
     #[test]
diff --git a/src/libstd/sync/mod.rs b/src/libstd/sync/mod.rs
index 28fab5a2c9d..ae21503cf67 100644
--- a/src/libstd/sync/mod.rs
+++ b/src/libstd/sync/mod.rs
@@ -21,7 +21,7 @@ pub use alloc::arc::{Arc, Weak};
 pub use core::atomic;
 
 pub use self::barrier::{Barrier, BarrierWaitResult};
-pub use self::condvar::{Condvar, StaticCondvar, CONDVAR_INIT};
+pub use self::condvar::{Condvar, StaticCondvar, WaitTimeoutResult, CONDVAR_INIT};
 pub use self::mutex::MUTEX_INIT;
 pub use self::mutex::{Mutex, MutexGuard, StaticMutex};
 pub use self::once::{Once, ONCE_INIT};