Don’t reset RangeInclusive to 1...0 after last iteration.

Instead try to set start to end+1, and if that overflows set end to start-1.

Author: SimonSapin

src/libcore/iter/range.rs

@@ -30,34 +30,13 @@ pub trait Step: Clone + PartialOrd + Sized {
     /// without overflow.
     fn steps_between(start: &Self, end: &Self) -> Option<usize>;
 
-    /// Replaces this step with `1`, returning itself
-    fn replace_one(&mut self) -> Self;
-
-    /// Replaces this step with `0`, returning itself
-    fn replace_zero(&mut self) -> Self;
-
     /// Add an usize, returning None on overflow
     fn add_usize(&self, n: usize) -> Option<Self>;
 
     /// Subtracts an usize, returning None on overflow
     fn sub_usize(&self, n: usize) -> Option<Self>;
 }
 
-// These are still macro-generated because the integer literals resolve to different types.
-macro_rules! step_identical_methods {
-    () => {
-        #[inline]
-        fn replace_one(&mut self) -> Self {
-            mem::replace(self, 1)
-        }
-
-        #[inline]
-        fn replace_zero(&mut self) -> Self {
-            mem::replace(self, 0)
-        }
-    }
-}
-
 macro_rules! step_impl_unsigned {
     ($($t:ty)*) => ($(
         #[unstable(feature = "step_trait",
@@ -90,8 +69,6 @@ macro_rules! step_impl_unsigned {
                     Err(_) => None,
                 }
             }
-
-            step_identical_methods!();
         }
     )*)
 }
@@ -148,8 +125,6 @@ macro_rules! step_impl_signed {
                     Err(_) => None,
                 }
             }
-
-            step_identical_methods!();
         }
     )*)
 }
@@ -174,8 +149,6 @@ macro_rules! step_impl_no_between {
             fn sub_usize(&self, n: usize) -> Option<Self> {
                 self.checked_sub(n as $t)
             }
-
-            step_identical_methods!();
         }
     )*)
 }
@@ -338,8 +311,15 @@ impl<A: Step> Iterator for ops::RangeInclusive<A> {
                 Some(mem::replace(&mut self.start, n))
             },
             Some(Equal) => {
-                let last = self.start.replace_one();
-                self.end.replace_zero();
+                let last;
+                if let Some(end_plus_one) = self.end.add_usize(1) {
+                    last = mem::replace(&mut self.start, end_plus_one);
+                } else {
+                    last = self.start.clone();
+                    // `start == end`, and `end + 1` underflowed.
+                    // `start - 1` overflowing would imply a type with only one valid value?
+                    self.end = self.start.sub_usize(1).expect("overflow in RangeInclusive::next");
+                }
                 Some(last)
             },
             _ => None,
@@ -370,16 +350,26 @@ impl<A: Step> Iterator for ops::RangeInclusive<A> {
                     return Some(plus_n)
                 }
                 Some(Equal) => {
-                    self.start.replace_one();
-                    self.end.replace_zero();
+                    if let Some(end_plus_one) = self.end.add_usize(1) {
+                        self.start = end_plus_one
+                    } else {
+                        // `start == end`, and `end + 1` underflowed.
+                        // `start - 1` overflowing would imply a type with only one valid value?
+                        self.end = self.start.sub_usize(1).expect("overflow in RangeInclusive::nth")
+                    }
                     return Some(plus_n)
                 }
                 _ => {}
             }
         }
 
-        self.start.replace_one();
-        self.end.replace_zero();
+        if let Some(end_plus_one) = self.end.add_usize(1) {
+            self.start = end_plus_one
+        } else {
+            // `start == end`, and `end + 1` underflowed.
+            // `start - 1` overflowing would imply a type with only one valid value?
+            self.end = self.start.sub_usize(1).expect("overflow in RangeInclusive::nth")
+        }
         None
     }
 }
@@ -397,8 +387,15 @@ impl<A: Step> DoubleEndedIterator for ops::RangeInclusive<A> {
                 Some(mem::replace(&mut self.end, n))
             },
             Some(Equal) => {
-                let last = self.end.replace_zero();
-                self.start.replace_one();
+                let last;
+                if let Some(start_minus_one) = self.start.sub_usize(1) {
+                    last = mem::replace(&mut self.end, start_minus_one);
+                } else {
+                    last = self.end.clone();
+                    // `start == end`, and `start - 1` underflowed.
+                    // `end + 1` overflowing would imply a type with only one valid value?
+                    self.start = self.start.add_usize(1).expect("overflow in RangeInclusive::next_back");
+                }
                 Some(last)
             },
             _ => None,

src/libcore/tests/iter.rs

@@ -1080,20 +1080,41 @@ fn test_range() {
 fn test_range_inclusive_exhaustion() {
     let mut r = 10...10;
     assert_eq!(r.next(), Some(10));
-    assert_eq!(r, 1...0);
+    assert_eq!(r, 11...10);
+    assert_eq!(r.next(), None);
+    assert_eq!(r, 11...10);
 
     let mut r = 10...10;
     assert_eq!(r.next_back(), Some(10));
+    assert_eq!(r, 10...9);
+    assert_eq!(r.next_back(), None);
+    assert_eq!(r, 10...9);
+
+    let mut r = 0_u32...0;
+    assert_eq!(r.next_back(), Some(0));
+    assert_eq!(r, 1...0);
+    assert_eq!(r.next_back(), None);
     assert_eq!(r, 1...0);
 
     let mut r = 10...12;
     assert_eq!(r.nth(2), Some(12));
-    assert_eq!(r, 1...0);
+    assert_eq!(r, 13...12);
 
     let mut r = 10...12;
     assert_eq!(r.nth(5), None);
-    assert_eq!(r, 1...0);
+    assert_eq!(r, 13...12);
+
+    let mut r = 127_i8...127;
+    assert_eq!(r.next(), Some(127));
+    assert_eq!(r, 127...126);
+    assert_eq!(r.next(), None);
+    assert_eq!(r, 127...126);
 
+    let mut r = 255_u8...255;
+    assert_eq!(r.next(), Some(255));
+    assert_eq!(r, 255...254);
+    assert_eq!(r.next(), None);
+    assert_eq!(r, 255...254);
 }
 
 #[test]
@@ -1142,7 +1163,7 @@ fn test_range_inclusive_nth() {
     assert_eq!(r.is_empty(), false);
     assert_eq!(r.nth(10), None);
     assert_eq!(r.is_empty(), true);
-    assert_eq!(r, 1...0);  // We may not want to document/promise this detail
+    assert_eq!(r, 21...20);
 }
 
 #[test]
@@ -1263,40 +1284,6 @@ fn test_chain_fold() {
 }
 
 #[test]
-fn test_step_replace_unsigned() {
-    let mut x = 4u32;
-    let y = x.replace_zero();
-    assert_eq!(x, 0);
-    assert_eq!(y, 4);
-
-    x = 5;
-    let y = x.replace_one();
-    assert_eq!(x, 1);
-    assert_eq!(y, 5);
-}
-
-#[test]
-fn test_step_replace_signed() {
-    let mut x = 4i32;
-    let y = x.replace_zero();
-    assert_eq!(x, 0);
-    assert_eq!(y, 4);
-
-    x = 5;
-    let y = x.replace_one();
-    assert_eq!(x, 1);
-    assert_eq!(y, 5);
-}
-
-#[test]
-fn test_step_replace_no_between() {
-    let mut x = 4u128;
-    let y = x.replace_zero();
-    assert_eq!(x, 0);
-    assert_eq!(y, 4);
-
-    x = 5;
-    let y = x.replace_one();
-    assert_eq!(x, 1);
-    assert_eq!(y, 5);
+fn test_step_add_usize() {
+    assert_eq!((-120_i8).add_usize(200), Some(80));
 }