Auto merge of #56049 - newpavlov:revert_51601, r=sfackler

Revert #51601

Closes: #55985

Specialization of `StepBy<Range(Inclusive)>` results in an incorrectly behaving code when `step_by` is combined with `skip` or `nth`.

If this will get merged we probably should reopen issues previously closed by #51601 (if there was any).

Author: bors

src/libcore/iter/mod.rs

@@ -319,10 +319,9 @@
 use cmp;
 use fmt;
 use iter_private::TrustedRandomAccess;
-use ops::{self, Try};
+use ops::Try;
 use usize;
 use intrinsics;
-use mem;
 
 #[stable(feature = "rust1", since = "1.0.0")]
 pub use self::iterator::Iterator;
@@ -673,7 +672,12 @@ impl<I> Iterator for StepBy<I> where I: Iterator {
 
     #[inline]
     fn next(&mut self) -> Option<Self::Item> {
-        <Self as StepBySpecIterator>::spec_next(self)
+        if self.first_take {
+            self.first_take = false;
+            self.iter.next()
+        } else {
+            self.iter.nth(self.step)
+        }
     }
 
     #[inline]
@@ -733,78 +737,6 @@ impl<I> Iterator for StepBy<I> where I: Iterator {
     }
 }
 
-// hidden trait for specializing iterator methods
-// could be generalized but is currently only used for StepBy
-trait StepBySpecIterator {
-    type Item;
-    fn spec_next(&mut self) -> Option<Self::Item>;
-}
-
-impl<I> StepBySpecIterator for StepBy<I>
-where
-    I: Iterator,
-{
-    type Item = I::Item;
-
-    #[inline]
-    default fn spec_next(&mut self) -> Option<I::Item> {
-        if self.first_take {
-            self.first_take = false;
-            self.iter.next()
-        } else {
-            self.iter.nth(self.step)
-        }
-    }
-}
-
-impl<T> StepBySpecIterator for StepBy<ops::Range<T>>
-where
-    T: Step,
-{
-    #[inline]
-    fn spec_next(&mut self) -> Option<Self::Item> {
-        self.first_take = false;
-        if !(self.iter.start < self.iter.end) {
-            return None;
-        }
-        // add 1 to self.step to get original step size back
-        // it was decremented for the general case on construction
-        if let Some(n) = self.iter.start.add_usize(self.step+1) {
-            let next = mem::replace(&mut self.iter.start, n);
-            Some(next)
-        } else {
-            let last = self.iter.start.clone();
-            self.iter.start = self.iter.end.clone();
-            Some(last)
-        }
-    }
-}
-
-impl<T> StepBySpecIterator for StepBy<ops::RangeInclusive<T>>
-where
-    T: Step,
-{
-    #[inline]
-    fn spec_next(&mut self) -> Option<Self::Item> {
-        self.first_take = false;
-        self.iter.compute_is_empty();
-        if self.iter.is_empty.unwrap_or_default() {
-            return None;
-        }
-        // add 1 to self.step to get original step size back
-        // it was decremented for the general case on construction
-        if let Some(n) = self.iter.start.add_usize(self.step+1) {
-            self.iter.is_empty = Some(!(n <= self.iter.end));
-            let next = mem::replace(&mut self.iter.start, n);
-            Some(next)
-        } else {
-            let last = self.iter.start.clone();
-            self.iter.is_empty = Some(true);
-            Some(last)
-        }
-    }
-}
-
 // StepBy can only make the iterator shorter, so the len will still fit.
 #[stable(feature = "iterator_step_by", since = "1.28.0")]
 impl<I> ExactSizeIterator for StepBy<I> where I: ExactSizeIterator {}

src/libcore/tests/iter.rs

@@ -1618,6 +1618,13 @@ fn test_range_step() {
     assert_eq!((isize::MIN..isize::MAX).step_by(1).size_hint(), (usize::MAX, Some(usize::MAX)));
 }
 
+#[test]
+fn test_step_by_skip() {
+    assert_eq!((0..640).step_by(128).skip(1).collect::<Vec<_>>(), [128, 256, 384, 512]);
+    assert_eq!((0..=50).step_by(10).nth(3), Some(30));
+    assert_eq!((200..=255u8).step_by(10).nth(3), Some(230));
+}
+
 #[test]
 fn test_range_inclusive_step() {
     assert_eq!((0..=50).step_by(10).collect::<Vec<_>>(), [0, 10, 20, 30, 40, 50]);