Merge #548

548: Remove our own error types from crossbeam-queue r=taiki-e a=taiki-e

Currently, `crossbeam-queue` has two error types: `PopError`, `PushError`.
* `PopError` is a unit struct with no field, and is returned by `ArrayQueue::pop` and `SegQueue::pop` if the queue is empty.
* `PushError` is a tuple struct with a public field that stores the element that user tried to push, and is returned by `ArrayQueue::push` if the queue is full.

I guess the current API was influenced by channels, but there is only one kind(variant) of error in each, and unlike channels, multiple kinds(variants) of errors do not occur.
And, neither of these provides additional information about the error (fields are all public, so it doesn't seem like they are going to provide them in the future).

So, APIs that return `Option` (on `pop` methods) and `Result` (on `ArrayQueue::push`) without our own errors are simple, and I think it is preferable. 
(Especially for the `pop` methods, it will be more consistent, because all other public `pop` methods in crossbeam return `Option`.)

(Of course, this is a breaking change, but the master already has some breaking changes.)

Co-authored-by: Taiki Endo <te316e89@gmail.com>

crossbeam-channel/benchmarks/segqueue.rs

@@ -30,7 +30,7 @@ fn spsc() {
 
         for _ in 0..MESSAGES {
             loop {
-                if q.pop().is_err() {
+                if q.pop().is_none() {
                     thread::yield_now();
                 } else {
                     break;
@@ -55,7 +55,7 @@ fn mpsc() {
 
         for _ in 0..MESSAGES {
             loop {
-                if q.pop().is_err() {
+                if q.pop().is_none() {
                     thread::yield_now();
                 } else {
                     break;
@@ -82,7 +82,7 @@ fn mpmc() {
             scope.spawn(|_| {
                 for _ in 0..MESSAGES / THREADS {
                     loop {
-                        if q.pop().is_err() {
+                        if q.pop().is_none() {
                             thread::yield_now();
                         } else {
                             break;

crossbeam-queue/src/array_queue.rs

@@ -17,8 +17,6 @@ use core::sync::atomic::{self, AtomicUsize, Ordering};
 
 use crossbeam_utils::{Backoff, CachePadded};
 
-use crate::err::{PopError, PushError};
-
 /// A slot in a queue.
 struct Slot<T> {
     /// The current stamp.
@@ -43,14 +41,14 @@ struct Slot<T> {
 /// # Examples
 ///
 /// ```
-/// use crossbeam_queue::{ArrayQueue, PushError};
+/// use crossbeam_queue::ArrayQueue;
 ///
 /// let q = ArrayQueue::new(2);
 ///
 /// assert_eq!(q.push('a'), Ok(()));
 /// assert_eq!(q.push('b'), Ok(()));
-/// assert_eq!(q.push('c'), Err(PushError('c')));
-/// assert_eq!(q.pop(), Ok('a'));
+/// assert_eq!(q.push('c'), Err('c'));
+/// assert_eq!(q.pop(), Some('a'));
 /// ```
 pub struct ArrayQueue<T> {
     /// The head of the queue.
@@ -144,14 +142,14 @@ impl<T> ArrayQueue<T> {
     /// # Examples
     ///
     /// ```
-    /// use crossbeam_queue::{ArrayQueue, PushError};
+    /// use crossbeam_queue::ArrayQueue;
     ///
     /// let q = ArrayQueue::new(1);
     ///
     /// assert_eq!(q.push(10), Ok(()));
-    /// assert_eq!(q.push(20), Err(PushError(20)));
+    /// assert_eq!(q.push(20), Err(20));
     /// ```
-    pub fn push(&self, value: T) -> Result<(), PushError<T>> {
+    pub fn push(&self, value: T) -> Result<(), T> {
         let backoff = Backoff::new();
         let mut tail = self.tail.load(Ordering::Relaxed);
 
@@ -203,7 +201,7 @@ impl<T> ArrayQueue<T> {
                 // If the head lags one lap behind the tail as well...
                 if head.wrapping_add(self.one_lap) == tail {
                     // ...then the queue is full.
-                    return Err(PushError(value));
+                    return Err(value);
                 }
 
                 backoff.spin();
@@ -218,20 +216,20 @@ impl<T> ArrayQueue<T> {
 
     /// Attempts to pop an element from the queue.
     ///
-    /// If the queue is empty, an error is returned.
+    /// If the queue is empty, `None` is returned.
     ///
     /// # Examples
     ///
     /// ```
-    /// use crossbeam_queue::{ArrayQueue, PopError};
+    /// use crossbeam_queue::ArrayQueue;
     ///
     /// let q = ArrayQueue::new(1);
     /// assert_eq!(q.push(10), Ok(()));
     ///
-    /// assert_eq!(q.pop(), Ok(10));
-    /// assert_eq!(q.pop(), Err(PopError));
+    /// assert_eq!(q.pop(), Some(10));
+    /// assert!(q.pop().is_none());
     /// ```
-    pub fn pop(&self) -> Result<T, PopError> {
+    pub fn pop(&self) -> Option<T> {
         let backoff = Backoff::new();
         let mut head = self.head.load(Ordering::Relaxed);
 
@@ -268,7 +266,7 @@ impl<T> ArrayQueue<T> {
                         let msg = unsafe { slot.value.get().read().assume_init() };
                         slot.stamp
                             .store(head.wrapping_add(self.one_lap), Ordering::Release);
-                        return Ok(msg);
+                        return Some(msg);
                     }
                     Err(h) => {
                         head = h;
@@ -281,7 +279,7 @@ impl<T> ArrayQueue<T> {
 
                 // If the tail equals the head, that means the channel is empty.
                 if tail == head {
-                    return Err(PopError);
+                    return None;
                 }
 
                 backoff.spin();

crossbeam-queue/src/lib.rs

@@ -25,11 +25,9 @@ cfg_if::cfg_if! {
         extern crate alloc;
 
         mod array_queue;
-        mod err;
         mod seg_queue;
 
         pub use self::array_queue::ArrayQueue;
-        pub use self::err::{PopError, PushError};
         pub use self::seg_queue::SegQueue;
     }
 }

crossbeam-queue/src/seg_queue.rs

@@ -8,8 +8,6 @@ use core::sync::atomic::{self, AtomicPtr, AtomicUsize, Ordering};
 
 use crossbeam_utils::{Backoff, CachePadded};
 
-use crate::err::PopError;
-
 // Bits indicating the state of a slot:
 // * If a value has been written into the slot, `WRITE` is set.
 // * If a value has been read from the slot, `READ` is set.
@@ -123,16 +121,16 @@ struct Position<T> {
 /// # Examples
 ///
 /// ```
-/// use crossbeam_queue::{PopError, SegQueue};
+/// use crossbeam_queue::SegQueue;
 ///
 /// let q = SegQueue::new();
 ///
 /// q.push('a');
 /// q.push('b');
 ///
-/// assert_eq!(q.pop(), Ok('a'));
-/// assert_eq!(q.pop(), Ok('b'));
-/// assert_eq!(q.pop(), Err(PopError));
+/// assert_eq!(q.pop(), Some('a'));
+/// assert_eq!(q.pop(), Some('b'));
+/// assert!(q.pop().is_none());
 /// ```
 pub struct SegQueue<T> {
     /// The head of the queue.
@@ -266,20 +264,20 @@ impl<T> SegQueue<T> {
 
     /// Pops an element from the queue.
     ///
-    /// If the queue is empty, an error is returned.
+    /// If the queue is empty, `None` is returned.
     ///
     /// # Examples
     ///
     /// ```
-    /// use crossbeam_queue::{PopError, SegQueue};
+    /// use crossbeam_queue::SegQueue;
     ///
     /// let q = SegQueue::new();
     ///
     /// q.push(10);
-    /// assert_eq!(q.pop(), Ok(10));
-    /// assert_eq!(q.pop(), Err(PopError));
+    /// assert_eq!(q.pop(), Some(10));
+    /// assert!(q.pop().is_none());
     /// ```
-    pub fn pop(&self) -> Result<T, PopError> {
+    pub fn pop(&self) -> Option<T> {
         let backoff = Backoff::new();
         let mut head = self.head.index.load(Ordering::Acquire);
         let mut block = self.head.block.load(Ordering::Acquire);
@@ -304,7 +302,7 @@ impl<T> SegQueue<T> {
 
                 // If the tail equals the head, that means the queue is empty.
                 if head >> SHIFT == tail >> SHIFT {
-                    return Err(PopError);
+                    return None;
                 }
 
                 // If head and tail are not in the same block, set `HAS_NEXT` in head.
@@ -355,7 +353,7 @@ impl<T> SegQueue<T> {
                         Block::destroy(block, offset + 1);
                     }
 
-                    return Ok(value);
+                    return Some(value);
                 },
                 Err(h) => {
                     head = h;

crossbeam-queue/tests/array_queue.rs

@@ -9,11 +9,11 @@ fn smoke() {
     let q = ArrayQueue::new(1);
 
     q.push(7).unwrap();
-    assert_eq!(q.pop(), Ok(7));
+    assert_eq!(q.pop(), Some(7));
 
     q.push(8).unwrap();
-    assert_eq!(q.pop(), Ok(8));
-    assert!(q.pop().is_err());
+    assert_eq!(q.pop(), Some(8));
+    assert!(q.pop().is_none());
 }
 
 #[test]
@@ -92,7 +92,7 @@ fn len() {
         scope.spawn(|_| {
             for i in 0..COUNT {
                 loop {
-                    if let Ok(x) = q.pop() {
+                    if let Some(x) = q.pop() {
                         assert_eq!(x, i);
                         break;
                     }
@@ -124,13 +124,13 @@ fn spsc() {
         scope.spawn(|_| {
             for i in 0..COUNT {
                 loop {
-                    if let Ok(x) = q.pop() {
+                    if let Some(x) = q.pop() {
                         assert_eq!(x, i);
                         break;
                     }
                 }
             }
-            assert!(q.pop().is_err());
+            assert!(q.pop().is_none());
         });
 
         scope.spawn(|_| {
@@ -155,7 +155,7 @@ fn mpmc() {
             scope.spawn(|_| {
                 for _ in 0..COUNT {
                     let n = loop {
-                        if let Ok(x) = q.pop() {
+                        if let Some(x) = q.pop() {
                             break x;
                         }
                     };
@@ -205,7 +205,7 @@ fn drops() {
         scope(|scope| {
             scope.spawn(|_| {
                 for _ in 0..steps {
-                    while q.pop().is_err() {}
+                    while q.pop().is_none() {}
                 }
             });
 

crossbeam-queue/tests/seg_queue.rs

@@ -8,11 +8,11 @@ use rand::{thread_rng, Rng};
 fn smoke() {
     let q = SegQueue::new();
     q.push(7);
-    assert_eq!(q.pop(), Ok(7));
+    assert_eq!(q.pop(), Some(7));
 
     q.push(8);
-    assert_eq!(q.pop(), Ok(8));
-    assert!(q.pop().is_err());
+    assert_eq!(q.pop(), Some(8));
+    assert!(q.pop().is_none());
 }
 
 #[test]
@@ -62,13 +62,13 @@ fn spsc() {
         scope.spawn(|_| {
             for i in 0..COUNT {
                 loop {
-                    if let Ok(x) = q.pop() {
+                    if let Some(x) = q.pop() {
                         assert_eq!(x, i);
                         break;
                     }
                 }
             }
-            assert!(q.pop().is_err());
+            assert!(q.pop().is_none());
         });
         scope.spawn(|_| {
             for i in 0..COUNT {
@@ -92,7 +92,7 @@ fn mpmc() {
             scope.spawn(|_| {
                 for _ in 0..COUNT {
                     let n = loop {
-                        if let Ok(x) = q.pop() {
+                        if let Some(x) = q.pop() {
                             break x;
                         }
                     };
@@ -140,7 +140,7 @@ fn drops() {
         scope(|scope| {
             scope.spawn(|_| {
                 for _ in 0..steps {
-                    while q.pop().is_err() {}
+                    while q.pop().is_none() {}
                 }
             });