Change advance(_back)_by to return usize instead of Result<(), usize>

A successful advance is now signalled by returning `0` and other values now represent the remaining number
of steps that couldn't be advanced as opposed to the amount of steps that have been advanced during a partial advance_by.

This simplifies adapters a bit, replacing some `match`/`if` with arithmetic. Whether this is beneficial overall depends
on whether `advance_by` is mostly used as a building-block for other iterator methods and adapters or whether
we also see uses by users where `Result` might be more useful.

Author: the8472

library/alloc/src/collections/vec_deque/into_iter.rs

@@ -54,14 +54,14 @@ impl<T, A: Allocator> Iterator for IntoIter<T, A> {
     }
 
     #[inline]
-    fn advance_by(&mut self, n: usize) -> Result<(), usize> {
+    fn advance_by(&mut self, n: usize) -> usize {
         if self.inner.len < n {
             let len = self.inner.len;
             self.inner.clear();
-            Err(len)
+            len - n
         } else {
             self.inner.drain(..n);
-            Ok(())
+            0
         }
     }
 
@@ -182,14 +182,14 @@ impl<T, A: Allocator> DoubleEndedIterator for IntoIter<T, A> {
     }
 
     #[inline]
-    fn advance_back_by(&mut self, n: usize) -> Result<(), usize> {
+    fn advance_back_by(&mut self, n: usize) -> usize {
         let len = self.inner.len;
         if len >= n {
             self.inner.truncate(len - n);
-            Ok(())
+            0
         } else {
             self.inner.clear();
-            Err(len)
+            n - len
         }
     }
 

library/alloc/src/collections/vec_deque/iter.rs

@@ -55,13 +55,13 @@ impl<'a, T> Iterator for Iter<'a, T> {
         }
     }
 
-    fn advance_by(&mut self, n: usize) -> Result<(), usize> {
-        let m = match self.i1.advance_by(n) {
-            Ok(_) => return Ok(()),
-            Err(m) => m,
-        };
+    fn advance_by(&mut self, n: usize) -> usize {
+        let remaining = self.i1.advance_by(n);
+        if remaining == 0 {
+            return 0;
+        }
         mem::swap(&mut self.i1, &mut self.i2);
-        self.i1.advance_by(n - m).map_err(|o| o + m)
+        self.i1.advance_by(remaining)
     }
 
     #[inline]
@@ -125,14 +125,13 @@ impl<'a, T> DoubleEndedIterator for Iter<'a, T> {
         }
     }
 
-    fn advance_back_by(&mut self, n: usize) -> Result<(), usize> {
-        let m = match self.i2.advance_back_by(n) {
-            Ok(_) => return Ok(()),
-            Err(m) => m,
-        };
-
+    fn advance_back_by(&mut self, n: usize) -> usize {
+        let remaining = self.i2.advance_back_by(n);
+        if remaining == 0 {
+            return 0;
+        }
         mem::swap(&mut self.i1, &mut self.i2);
-        self.i2.advance_back_by(n - m).map_err(|o| m + o)
+        self.i2.advance_back_by(remaining)
     }
 
     fn rfold<Acc, F>(self, accum: Acc, mut f: F) -> Acc

library/alloc/src/collections/vec_deque/iter_mut.rs

@@ -47,13 +47,13 @@ impl<'a, T> Iterator for IterMut<'a, T> {
         }
     }
 
-    fn advance_by(&mut self, n: usize) -> Result<(), usize> {
-        let m = match self.i1.advance_by(n) {
-            Ok(_) => return Ok(()),
-            Err(m) => m,
-        };
+    fn advance_by(&mut self, n: usize) -> usize {
+        let remaining = self.i1.advance_by(n);
+        if remaining == 0 {
+            return 0;
+        }
         mem::swap(&mut self.i1, &mut self.i2);
-        self.i1.advance_by(n - m).map_err(|o| o + m)
+        self.i1.advance_by(remaining)
     }
 
     #[inline]
@@ -117,14 +117,13 @@ impl<'a, T> DoubleEndedIterator for IterMut<'a, T> {
         }
     }
 
-    fn advance_back_by(&mut self, n: usize) -> Result<(), usize> {
-        let m = match self.i2.advance_back_by(n) {
-            Ok(_) => return Ok(()),
-            Err(m) => m,
-        };
-
+    fn advance_back_by(&mut self, n: usize) -> usize {
+        let remaining = self.i2.advance_back_by(n);
+        if remaining == 0 {
+            return 0;
+        }
         mem::swap(&mut self.i1, &mut self.i2);
-        self.i2.advance_back_by(n - m).map_err(|o| m + o)
+        self.i2.advance_back_by(remaining)
     }
 
     fn rfold<Acc, F>(self, accum: Acc, mut f: F) -> Acc

library/alloc/src/vec/into_iter.rs

@@ -213,7 +213,7 @@ impl<T, A: Allocator> Iterator for IntoIter<T, A> {
     }
 
     #[inline]
-    fn advance_by(&mut self, n: usize) -> Result<(), usize> {
+    fn advance_by(&mut self, n: usize) -> usize {
         let step_size = self.len().min(n);
         let to_drop = ptr::slice_from_raw_parts_mut(self.ptr as *mut T, step_size);
         if T::IS_ZST {
@@ -227,10 +227,7 @@ impl<T, A: Allocator> Iterator for IntoIter<T, A> {
         unsafe {
             ptr::drop_in_place(to_drop);
         }
-        if step_size < n {
-            return Err(step_size);
-        }
-        Ok(())
+        n - step_size
     }
 
     #[inline]
@@ -313,7 +310,7 @@ impl<T, A: Allocator> DoubleEndedIterator for IntoIter<T, A> {
     }
 
     #[inline]
-    fn advance_back_by(&mut self, n: usize) -> Result<(), usize> {
+    fn advance_back_by(&mut self, n: usize) -> usize {
         let step_size = self.len().min(n);
         if T::IS_ZST {
             // SAFETY: same as for advance_by()
@@ -327,10 +324,7 @@ impl<T, A: Allocator> DoubleEndedIterator for IntoIter<T, A> {
         unsafe {
             ptr::drop_in_place(to_drop);
         }
-        if step_size < n {
-            return Err(step_size);
-        }
-        Ok(())
+        n - step_size
     }
 }
 

library/alloc/tests/vec.rs

@@ -1,4 +1,5 @@
 use core::alloc::{Allocator, Layout};
+use core::assert_eq;
 use core::iter::IntoIterator;
 use core::ptr::NonNull;
 use std::alloc::System;
@@ -1062,21 +1063,21 @@ fn test_into_iter_leak() {
 
 #[test]
 fn test_into_iter_advance_by() {
-    let mut i = [1, 2, 3, 4, 5].into_iter();
-    i.advance_by(0).unwrap();
-    i.advance_back_by(0).unwrap();
+    let mut i = vec![1, 2, 3, 4, 5].into_iter();
+    assert_eq!(i.advance_by(0), 0);
+    assert_eq!(i.advance_back_by(0), 0);
     assert_eq!(i.as_slice(), [1, 2, 3, 4, 5]);
 
-    i.advance_by(1).unwrap();
-    i.advance_back_by(1).unwrap();
+    assert_eq!(i.advance_by(1), 0);
+    assert_eq!(i.advance_back_by(1), 0);
     assert_eq!(i.as_slice(), [2, 3, 4]);
 
-    assert_eq!(i.advance_back_by(usize::MAX), Err(3));
+    assert_eq!(i.advance_back_by(usize::MAX), usize::MAX - 3);
 
-    assert_eq!(i.advance_by(usize::MAX), Err(0));
+    assert_eq!(i.advance_by(usize::MAX), usize::MAX);
 
-    i.advance_by(0).unwrap();
-    i.advance_back_by(0).unwrap();
+    assert_eq!(i.advance_by(0), 0);
+    assert_eq!(i.advance_back_by(0), 0);
 
     assert_eq!(i.len(), 0);
 }
@@ -1124,7 +1125,7 @@ fn test_into_iter_zst() {
     for _ in vec![C; 5].into_iter().rev() {}
 
     let mut it = vec![C, C].into_iter();
-    it.advance_by(1).unwrap();
+    assert_eq!(it.advance_by(1), 0);
     drop(it);
 
     let mut it = vec![C, C].into_iter();

library/core/src/array/iter.rs

@@ -284,20 +284,19 @@ impl<T, const N: usize> Iterator for IntoIter<T, N> {
         self.next_back()
     }
 
-    fn advance_by(&mut self, n: usize) -> Result<(), usize> {
-        let original_len = self.len();
-
+    fn advance_by(&mut self, n: usize) -> usize {
         // This also moves the start, which marks them as conceptually "dropped",
         // so if anything goes bad then our drop impl won't double-free them.
         let range_to_drop = self.alive.take_prefix(n);
+        let remaining = n - range_to_drop.len();
 
         // SAFETY: These elements are currently initialized, so it's fine to drop them.
         unsafe {
             let slice = self.data.get_unchecked_mut(range_to_drop);
             ptr::drop_in_place(MaybeUninit::slice_assume_init_mut(slice));
         }
 
-        if n > original_len { Err(original_len) } else { Ok(()) }
+        remaining
     }
 }
 
@@ -334,20 +333,19 @@ impl<T, const N: usize> DoubleEndedIterator for IntoIter<T, N> {
         })
     }
 
-    fn advance_back_by(&mut self, n: usize) -> Result<(), usize> {
-        let original_len = self.len();
-
+    fn advance_back_by(&mut self, n: usize) -> usize {
         // This also moves the end, which marks them as conceptually "dropped",
         // so if anything goes bad then our drop impl won't double-free them.
         let range_to_drop = self.alive.take_suffix(n);
+        let remaining = n - range_to_drop.len();
 
         // SAFETY: These elements are currently initialized, so it's fine to drop them.
         unsafe {
             let slice = self.data.get_unchecked_mut(range_to_drop);
             ptr::drop_in_place(MaybeUninit::slice_assume_init_mut(slice));
         }
 
-        if n > original_len { Err(original_len) } else { Ok(()) }
+        remaining
     }
 }
 

library/core/src/iter/adapters/by_ref_sized.rs

@@ -26,7 +26,7 @@ impl<I: Iterator> Iterator for ByRefSized<'_, I> {
     }
 
     #[inline]
-    fn advance_by(&mut self, n: usize) -> Result<(), usize> {
+    fn advance_by(&mut self, n: usize) -> usize {
         I::advance_by(self.0, n)
     }
 
@@ -62,7 +62,7 @@ impl<I: DoubleEndedIterator> DoubleEndedIterator for ByRefSized<'_, I> {
     }
 
     #[inline]
-    fn advance_back_by(&mut self, n: usize) -> Result<(), usize> {
+    fn advance_back_by(&mut self, n: usize) -> usize {
         I::advance_back_by(self.0, n)
     }
 

library/core/src/iter/adapters/chain.rs

@@ -95,38 +95,33 @@ where
     }
 
     #[inline]
-    fn advance_by(&mut self, n: usize) -> Result<(), usize> {
-        let mut rem = n;
-
+    fn advance_by(&mut self, mut n: usize) -> usize {
         if let Some(ref mut a) = self.a {
-            match a.advance_by(rem) {
-                Ok(()) => return Ok(()),
-                Err(k) => rem -= k,
+            n = a.advance_by(n);
+            if n == 0 {
+                return n;
             }
             self.a = None;
         }
 
         if let Some(ref mut b) = self.b {
-            match b.advance_by(rem) {
-                Ok(()) => return Ok(()),
-                Err(k) => rem -= k,
-            }
+            n = b.advance_by(n);
             // we don't fuse the second iterator
         }
 
-        if rem == 0 { Ok(()) } else { Err(n - rem) }
+        n
     }
 
     #[inline]
     fn nth(&mut self, mut n: usize) -> Option<Self::Item> {
         if let Some(ref mut a) = self.a {
-            match a.advance_by(n) {
-                Ok(()) => match a.next() {
-                    None => n = 0,
+            n = match a.advance_by(n) {
+                0 => match a.next() {
+                    None => 0,
                     x => return x,
                 },
-                Err(k) => n -= k,
-            }
+                k => k,
+            };
 
             self.a = None;
         }
@@ -186,38 +181,33 @@ where
     }
 
     #[inline]
-    fn advance_back_by(&mut self, n: usize) -> Result<(), usize> {
-        let mut rem = n;
-
+    fn advance_back_by(&mut self, mut n: usize) -> usize {
         if let Some(ref mut b) = self.b {
-            match b.advance_back_by(rem) {
-                Ok(()) => return Ok(()),
-                Err(k) => rem -= k,
+            n = b.advance_back_by(n);
+            if n == 0 {
+                return n;
             }
             self.b = None;
         }
 
         if let Some(ref mut a) = self.a {
-            match a.advance_back_by(rem) {
-                Ok(()) => return Ok(()),
-                Err(k) => rem -= k,
-            }
+            n = a.advance_back_by(n);
             // we don't fuse the second iterator
         }
 
-        if rem == 0 { Ok(()) } else { Err(n - rem) }
+        n
     }
 
     #[inline]
     fn nth_back(&mut self, mut n: usize) -> Option<Self::Item> {
         if let Some(ref mut b) = self.b {
-            match b.advance_back_by(n) {
-                Ok(()) => match b.next_back() {
-                    None => n = 0,
+            n = match b.advance_back_by(n) {
+                0 => match b.next_back() {
+                    None => 0,
                     x => return x,
                 },
-                Err(k) => n -= k,
-            }
+                k => k,
+            };
 
             self.b = None;
         }

library/core/src/iter/adapters/copied.rs

@@ -89,7 +89,7 @@ where
     }
 
     #[inline]
-    fn advance_by(&mut self, n: usize) -> Result<(), usize> {
+    fn advance_by(&mut self, n: usize) -> usize {
         self.it.advance_by(n)
     }
 
@@ -130,7 +130,7 @@ where
     }
 
     #[inline]
-    fn advance_back_by(&mut self, n: usize) -> Result<(), usize> {
+    fn advance_back_by(&mut self, n: usize) -> usize {
         self.it.advance_back_by(n)
     }
 }