From 9347756c0947e3ccd4433febb4c898675f92f7a1 Mon Sep 17 00:00:00 2001 From: Scott McMurray Date: Sat, 20 May 2017 02:12:00 -0700 Subject: [PATCH] Remove deprecated `Range::step_by` (use `Iterator::step_by` instead) Follow-up to https://github.com/rust-lang/rust/pull/41439#issuecomment-302598496 While doing so, remove the now-unused `step`, `steps_between`, and `is_negative` methods from `Step`. Mostly simple, but needed two interesting changes: * Override `Iterator::size_hint` for `iter::StepBy` (so hints aren't lost) * Override `Iterator::size_hint` for `ops::RangeFrom` (so `(0..).size_hint()` returns what `(0..).step_by(1).size_hint()` used to) (It turns out that `(0..).step_by(d)` is used in a bunch of tests, from `cycle` to `vec_deque`.) Incidentally fixes https://github.com/rust-lang/rust/issues/41477 --- src/doc/unstable-book/src/SUMMARY.md | 1 - .../src/library-features/step-by.md | 7 - src/libcollections/tests/lib.rs | 2 +- src/libcore/iter/iterator.rs | 3 +- src/libcore/iter/mod.rs | 16 +- src/libcore/iter/range.rs | 322 +----------------- src/libcore/tests/iter.rs | 75 +++- src/libcore/tests/lib.rs | 1 - src/librand/lib.rs | 2 +- .../run-pass/impl-trait/example-calendar.rs | 12 - src/test/run-pass/range_inclusive.rs | 2 +- .../sync-send-iterators-in-libcore.rs | 2 +- 12 files changed, 95 insertions(+), 350 deletions(-) delete mode 100644 src/doc/unstable-book/src/library-features/step-by.md diff --git a/src/doc/unstable-book/src/SUMMARY.md b/src/doc/unstable-book/src/SUMMARY.md index 39f800591483b..99f46f643098d 100644 --- a/src/doc/unstable-book/src/SUMMARY.md +++ b/src/doc/unstable-book/src/SUMMARY.md @@ -199,7 +199,6 @@ - [sort_internals](library-features/sort-internals.md) - [sort_unstable](library-features/sort-unstable.md) - [splice](library-features/splice.md) - - [step_by](library-features/step-by.md) - [step_trait](library-features/step-trait.md) - [str_checked_slicing](library-features/str-checked-slicing.md) - [str_escape](library-features/str-escape.md) diff --git a/src/doc/unstable-book/src/library-features/step-by.md b/src/doc/unstable-book/src/library-features/step-by.md deleted file mode 100644 index b649496cdd80b..0000000000000 --- a/src/doc/unstable-book/src/library-features/step-by.md +++ /dev/null @@ -1,7 +0,0 @@ -# `step_by` - -The tracking issue for this feature is: [#27741] - -[#27741]: https://github.com/rust-lang/rust/issues/27741 - ------------------------- diff --git a/src/libcollections/tests/lib.rs b/src/libcollections/tests/lib.rs index eae3bf3915f60..1357ac522ceef 100644 --- a/src/libcollections/tests/lib.rs +++ b/src/libcollections/tests/lib.rs @@ -21,7 +21,7 @@ #![feature(placement_in_syntax)] #![feature(rand)] #![feature(splice)] -#![feature(step_by)] +#![feature(iterator_step_by)] #![feature(str_escape)] #![feature(test)] #![feature(unboxed_closures)] diff --git a/src/libcore/iter/iterator.rs b/src/libcore/iter/iterator.rs index 77cbdb98c8304..85149a0f57078 100644 --- a/src/libcore/iter/iterator.rs +++ b/src/libcore/iter/iterator.rs @@ -130,9 +130,10 @@ pub trait Iterator { /// /// ``` /// // an infinite iterator has no upper bound + /// // and the maximum possible lower bound /// let iter = 0..; /// - /// assert_eq!((0, None), iter.size_hint()); + /// assert_eq!((usize::max_value(), None), iter.size_hint()); /// ``` #[inline] #[stable(feature = "rust1", since = "1.0.0")] diff --git a/src/libcore/iter/mod.rs b/src/libcore/iter/mod.rs index 420ff0f71193b..5047f1f916207 100644 --- a/src/libcore/iter/mod.rs +++ b/src/libcore/iter/mod.rs @@ -311,9 +311,6 @@ pub use self::iterator::Iterator; reason = "likely to be replaced by finer-grained traits", issue = "27741")] pub use self::range::Step; -#[unstable(feature = "step_by", reason = "recent addition", - issue = "27741")] -pub use self::range::StepBy as DeprecatedStepBy; #[stable(feature = "rust1", since = "1.0.0")] pub use self::sources::{Repeat, repeat}; @@ -553,6 +550,19 @@ impl Iterator for StepBy where I: Iterator { self.iter.nth(self.step) } } + + #[inline] + fn size_hint(&self) -> (usize, Option) { + let inner_hint = self.iter.size_hint(); + + if self.first_take { + let f = |n| if n == 0 { 0 } else { 1 + (n-1)/(self.step+1) }; + (f(inner_hint.0), inner_hint.1.map(f)) + } else { + let f = |n| n / (self.step+1); + (f(inner_hint.0), inner_hint.1.map(f)) + } + } } /// An iterator that strings two iterators together. diff --git a/src/libcore/iter/range.rs b/src/libcore/iter/range.rs index bd831d638c0c4..21034a8234045 100644 --- a/src/libcore/iter/range.rs +++ b/src/libcore/iter/range.rs @@ -22,22 +22,13 @@ use super::{FusedIterator, TrustedLen}; reason = "likely to be replaced by finer-grained traits", issue = "27741")] pub trait Step: PartialOrd + Sized { - /// Steps `self` if possible. - fn step(&self, by: &Self) -> Option; - /// Returns the number of steps between two step objects. The count is /// inclusive of `start` and exclusive of `end`. /// /// Returns `None` if it is not possible to calculate `steps_between` /// without overflow. - fn steps_between(start: &Self, end: &Self, by: &Self) -> Option; - - /// Same as `steps_between`, but with a `by` of 1 fn steps_between_by_one(start: &Self, end: &Self) -> Option; - /// Tests whether this step is negative or not (going backwards) - fn is_negative(&self) -> bool; - /// Replaces this step with `1`, returning itself fn replace_one(&mut self) -> Self; @@ -57,33 +48,17 @@ macro_rules! step_impl_unsigned { reason = "likely to be replaced by finer-grained traits", issue = "27741")] impl Step for $t { - #[inline] - fn step(&self, by: &$t) -> Option<$t> { - (*self).checked_add(*by) - } #[inline] #[allow(trivial_numeric_casts)] - fn steps_between(start: &$t, end: &$t, by: &$t) -> Option { - if *by == 0 { return None; } + fn steps_between_by_one(start: &Self, end: &Self) -> Option { if *start < *end { // Note: We assume $t <= usize here - let diff = (*end - *start) as usize; - let by = *by as usize; - if diff % by > 0 { - Some(diff / by + 1) - } else { - Some(diff / by) - } + Some((*end - *start) as usize) } else { Some(0) } } - #[inline] - fn is_negative(&self) -> bool { - false - } - #[inline] fn replace_one(&mut self) -> Self { mem::replace(self, 1) @@ -103,11 +78,6 @@ macro_rules! step_impl_unsigned { fn sub_one(&self) -> Self { Sub::sub(*self, 1) } - - #[inline] - fn steps_between_by_one(start: &Self, end: &Self) -> Option { - Self::steps_between(start, end, &1) - } } )*) } @@ -117,42 +87,16 @@ macro_rules! step_impl_signed { reason = "likely to be replaced by finer-grained traits", issue = "27741")] impl Step for $t { - #[inline] - fn step(&self, by: &$t) -> Option<$t> { - (*self).checked_add(*by) - } #[inline] #[allow(trivial_numeric_casts)] - fn steps_between(start: &$t, end: &$t, by: &$t) -> Option { - if *by == 0 { return None; } - let diff: usize; - let by_u: usize; - if *by > 0 { - if *start >= *end { - return Some(0); - } - // Note: We assume $t <= isize here - // Use .wrapping_sub and cast to usize to compute the - // difference that may not fit inside the range of isize. - diff = (*end as isize).wrapping_sub(*start as isize) as usize; - by_u = *by as usize; - } else { - if *start <= *end { - return Some(0); - } - diff = (*start as isize).wrapping_sub(*end as isize) as usize; - by_u = (*by as isize).wrapping_mul(-1) as usize; - } - if diff % by_u > 0 { - Some(diff / by_u + 1) - } else { - Some(diff / by_u) + fn steps_between_by_one(start: &Self, end: &Self) -> Option { + if *start >= *end { + return Some(0); } - } - - #[inline] - fn is_negative(&self) -> bool { - *self < 0 + // Note: We assume $t <= isize here + // Use .wrapping_sub and cast to usize to compute the + // difference that may not fit inside the range of isize. + Some((*end as isize).wrapping_sub(*start as isize) as usize) } #[inline] @@ -174,11 +118,6 @@ macro_rules! step_impl_signed { fn sub_one(&self) -> Self { Sub::sub(*self, 1) } - - #[inline] - fn steps_between_by_one(start: &Self, end: &Self) -> Option { - Self::steps_between(start, end, &1) - } } )*) } @@ -190,20 +129,10 @@ macro_rules! step_impl_no_between { issue = "27741")] impl Step for $t { #[inline] - fn step(&self, by: &$t) -> Option<$t> { - (*self).checked_add(*by) - } - #[inline] - fn steps_between(_a: &$t, _b: &$t, _by: &$t) -> Option { + fn steps_between_by_one(_start: &Self, _end: &Self) -> Option { None } - #[inline] - #[allow(unused_comparisons)] - fn is_negative(&self) -> bool { - *self < 0 - } - #[inline] fn replace_one(&mut self) -> Self { mem::replace(self, 1) @@ -223,11 +152,6 @@ macro_rules! step_impl_no_between { fn sub_one(&self) -> Self { Sub::sub(*self, 1) } - - #[inline] - fn steps_between_by_one(start: &Self, end: &Self) -> Option { - Self::steps_between(start, end, &1) - } } )*) } @@ -244,227 +168,6 @@ step_impl_signed!(i64); step_impl_no_between!(u64 i64); step_impl_no_between!(u128 i128); -/// An adapter for stepping range iterators by a custom amount. -/// -/// The resulting iterator handles overflow by stopping. The `A` -/// parameter is the type being iterated over, while `R` is the range -/// type (usually one of `std::ops::{Range, RangeFrom, RangeInclusive}`. -#[derive(Clone, Debug)] -#[unstable(feature = "step_by", reason = "recent addition", - issue = "27741")] -pub struct StepBy { - step_by: A, - range: R, -} - -impl ops::RangeFrom { - /// Creates an iterator starting at the same point, but stepping by - /// the given amount at each iteration. - /// - /// # Examples - /// - /// ``` - /// #![feature(step_by)] - /// fn main() { - /// let result: Vec<_> = (0..).step_by(2).take(5).collect(); - /// assert_eq!(result, vec![0, 2, 4, 6, 8]); - /// } - /// ``` - #[unstable(feature = "step_by", reason = "recent addition", - issue = "27741")] - pub fn step_by(self, by: A) -> StepBy { - StepBy { - step_by: by, - range: self - } - } -} - -impl ops::Range { - /// Creates an iterator with the same range, but stepping by the - /// given amount at each iteration. - /// - /// The resulting iterator handles overflow by stopping. - /// - /// # Examples - /// - /// ``` - /// #![feature(step_by)] - /// fn main() { - /// let result: Vec<_> = (0..10).step_by(2).collect(); - /// assert_eq!(result, vec![0, 2, 4, 6, 8]); - /// } - /// ``` - #[unstable(feature = "step_by", reason = "recent addition", - issue = "27741")] - pub fn step_by(self, by: A) -> StepBy { - StepBy { - step_by: by, - range: self - } - } -} - -impl ops::RangeInclusive { - /// Creates an iterator with the same range, but stepping by the - /// given amount at each iteration. - /// - /// The resulting iterator handles overflow by stopping. - /// - /// # Examples - /// - /// ``` - /// #![feature(step_by, inclusive_range_syntax)] - /// - /// let result: Vec<_> = (0...10).step_by(2).collect(); - /// assert_eq!(result, vec![0, 2, 4, 6, 8, 10]); - /// ``` - #[unstable(feature = "step_by", reason = "recent addition", - issue = "27741")] - pub fn step_by(self, by: A) -> StepBy { - StepBy { - step_by: by, - range: self - } - } -} - -#[unstable(feature = "step_by", reason = "recent addition", - issue = "27741")] -impl Iterator for StepBy> where - A: Clone, - for<'a> &'a A: Add<&'a A, Output = A> -{ - type Item = A; - - #[inline] - fn next(&mut self) -> Option { - let mut n = &self.range.start + &self.step_by; - mem::swap(&mut n, &mut self.range.start); - Some(n) - } - - #[inline] - fn size_hint(&self) -> (usize, Option) { - (usize::MAX, None) // Too bad we can't specify an infinite lower bound - } -} - -#[unstable(feature = "fused", issue = "35602")] -impl FusedIterator for StepBy> - where A: Clone, for<'a> &'a A: Add<&'a A, Output = A> {} - -#[unstable(feature = "step_by", reason = "recent addition", - issue = "27741")] -impl Iterator for StepBy> { - type Item = A; - - #[inline] - fn next(&mut self) -> Option { - let rev = self.step_by.is_negative(); - if (rev && self.range.start > self.range.end) || - (!rev && self.range.start < self.range.end) - { - match self.range.start.step(&self.step_by) { - Some(mut n) => { - mem::swap(&mut self.range.start, &mut n); - Some(n) - }, - None => { - let mut n = self.range.end.clone(); - mem::swap(&mut self.range.start, &mut n); - Some(n) - } - } - } else { - None - } - } - - #[inline] - fn size_hint(&self) -> (usize, Option) { - match Step::steps_between(&self.range.start, - &self.range.end, - &self.step_by) { - Some(hint) => (hint, Some(hint)), - None => (0, None) - } - } -} - -#[unstable(feature = "fused", issue = "35602")] -impl FusedIterator for StepBy> {} - -#[unstable(feature = "inclusive_range", - reason = "recently added, follows RFC", - issue = "28237")] -impl Iterator for StepBy> { - type Item = A; - - #[inline] - fn next(&mut self) -> Option { - use ops::RangeInclusive::*; - - // this function has a sort of odd structure due to borrowck issues - // we may need to replace self.range, so borrows of start and end need to end early - - let (finishing, n) = match self.range { - Empty { .. } => return None, // empty iterators yield no values - - NonEmpty { ref mut start, ref mut end } => { - let rev = self.step_by.is_negative(); - - // march start towards (maybe past!) end and yield the old value - if (rev && start >= end) || - (!rev && start <= end) - { - match start.step(&self.step_by) { - Some(mut n) => { - mem::swap(start, &mut n); - (None, Some(n)) // yield old value, remain non-empty - }, - None => { - let mut n = end.clone(); - mem::swap(start, &mut n); - (None, Some(n)) // yield old value, remain non-empty - } - } - } else { - // found range in inconsistent state (start at or past end), so become empty - (Some(end.replace_zero()), None) - } - } - }; - - // turn into an empty iterator if we've reached the end - if let Some(end) = finishing { - self.range = Empty { at: end }; - } - - n - } - - #[inline] - fn size_hint(&self) -> (usize, Option) { - use ops::RangeInclusive::*; - - match self.range { - Empty { .. } => (0, Some(0)), - - NonEmpty { ref start, ref end } => - match Step::steps_between(start, - end, - &self.step_by) { - Some(hint) => (hint.saturating_add(1), hint.checked_add(1)), - None => (0, None) - } - } - } -} - -#[unstable(feature = "fused", issue = "35602")] -impl FusedIterator for StepBy> {} - macro_rules! range_exact_iter_impl { ($($t:ty)*) => ($( #[stable(feature = "rust1", since = "1.0.0")] @@ -569,6 +272,11 @@ impl Iterator for ops::RangeFrom where mem::swap(&mut n, &mut self.start); Some(n) } + + #[inline] + fn size_hint(&self) -> (usize, Option) { + (usize::MAX, None) + } } #[unstable(feature = "fused", issue = "35602")] diff --git a/src/libcore/tests/iter.rs b/src/libcore/tests/iter.rs index ad91ba9be58f2..fde6ec1511209 100644 --- a/src/libcore/tests/iter.rs +++ b/src/libcore/tests/iter.rs @@ -9,7 +9,7 @@ // except according to those terms. use core::iter::*; -use core::{i8, i16, isize}; +use core::{i16, isize}; use core::usize; #[test] @@ -147,15 +147,13 @@ fn test_iterator_chain_find() { #[test] fn test_iterator_step_by() { // Identity - // Replace with (0..).step_by(1) after Range::step_by gets removed - let mut it = Iterator::step_by((0..), 1).take(3); + let mut it = (0..).step_by(1).take(3); assert_eq!(it.next(), Some(0)); assert_eq!(it.next(), Some(1)); assert_eq!(it.next(), Some(2)); assert_eq!(it.next(), None); - // Replace with (0..).step_by(3) after Range::step_by gets removed - let mut it = Iterator::step_by((0..), 3).take(4); + let mut it = (0..).step_by(3).take(4); assert_eq!(it.next(), Some(0)); assert_eq!(it.next(), Some(3)); assert_eq!(it.next(), Some(6)); @@ -166,11 +164,66 @@ fn test_iterator_step_by() { #[test] #[should_panic] fn test_iterator_step_by_zero() { - // Replace with (0..).step_by(0) after Range::step_by gets removed - let mut it = Iterator::step_by((0..), 0); + let mut it = (0..).step_by(0); it.next(); } +#[test] +fn test_iterator_step_by_size_hint() { + struct StubSizeHint(usize, Option); + impl Iterator for StubSizeHint { + type Item = (); + fn next(&mut self) -> Option<()> { + self.0 -= 1; + if let Some(ref mut upper) = self.1 { + *upper -= 1; + } + Some(()) + } + fn size_hint(&self) -> (usize, Option) { + (self.0, self.1) + } + } + + // The two checks in each case are needed because the logic + // is different before the first call to `next()`. + + let mut it = StubSizeHint(10, Some(10)).step_by(1); + assert_eq!(it.size_hint(), (10, Some(10))); + it.next(); + assert_eq!(it.size_hint(), (9, Some(9))); + + // exact multiple + let mut it = StubSizeHint(10, Some(10)).step_by(3); + assert_eq!(it.size_hint(), (4, Some(4))); + it.next(); + assert_eq!(it.size_hint(), (3, Some(3))); + + // larger base range, but not enough to get another element + let mut it = StubSizeHint(12, Some(12)).step_by(3); + assert_eq!(it.size_hint(), (4, Some(4))); + it.next(); + assert_eq!(it.size_hint(), (3, Some(3))); + + // smaller base range, so fewer resulting elements + let mut it = StubSizeHint(9, Some(9)).step_by(3); + assert_eq!(it.size_hint(), (3, Some(3))); + it.next(); + assert_eq!(it.size_hint(), (2, Some(2))); + + // infinite upper bound + let mut it = StubSizeHint(usize::MAX, None).step_by(1); + assert_eq!(it.size_hint(), (usize::MAX, None)); + it.next(); + assert_eq!(it.size_hint(), (usize::MAX-1, None)); + + // still infinite with larger step + let mut it = StubSizeHint(7, None).step_by(3); + assert_eq!(it.size_hint(), (3, None)); + it.next(); + assert_eq!(it.size_hint(), (2, None)); +} + #[test] fn test_filter_map() { let it = (0..).step_by(1).take(10) @@ -1008,8 +1061,6 @@ fn test_range() { #[test] fn test_range_step() { assert_eq!((0..20).step_by(5).collect::>(), [0, 5, 10, 15]); - assert_eq!((20..0).step_by(-5).collect::>(), [20, 15, 10, 5]); - assert_eq!((20..0).step_by(-6).collect::>(), [20, 14, 8, 2]); assert_eq!((200..255).step_by(50).collect::>(), [200, 250]); assert_eq!((200..-5).step_by(1).collect::>(), []); assert_eq!((200..200).step_by(1).collect::>(), []); @@ -1017,13 +1068,9 @@ fn test_range_step() { assert_eq!((0..20).step_by(1).size_hint(), (20, Some(20))); assert_eq!((0..20).step_by(21).size_hint(), (1, Some(1))); assert_eq!((0..20).step_by(5).size_hint(), (4, Some(4))); - assert_eq!((20..0).step_by(-5).size_hint(), (4, Some(4))); - assert_eq!((20..0).step_by(-6).size_hint(), (4, Some(4))); assert_eq!((20..-5).step_by(1).size_hint(), (0, Some(0))); assert_eq!((20..20).step_by(1).size_hint(), (0, Some(0))); - assert_eq!((0..1).step_by(0).size_hint(), (0, None)); - assert_eq!((i8::MAX..i8::MIN).step_by(i8::MIN).size_hint(), (2, Some(2))); - assert_eq!((i16::MIN..i16::MAX).step_by(i16::MAX).size_hint(), (3, Some(3))); + assert_eq!((i16::MIN..i16::MAX).step_by(i16::MAX as usize).size_hint(), (3, Some(3))); assert_eq!((isize::MIN..isize::MAX).step_by(1).size_hint(), (usize::MAX, Some(usize::MAX))); } diff --git a/src/libcore/tests/lib.rs b/src/libcore/tests/lib.rs index c52155ead4f0b..1a8f4d6cfcac0 100644 --- a/src/libcore/tests/lib.rs +++ b/src/libcore/tests/lib.rs @@ -31,7 +31,6 @@ #![feature(slice_patterns)] #![feature(sort_internals)] #![feature(sort_unstable)] -#![feature(step_by)] #![feature(step_trait)] #![feature(test)] #![feature(try_from)] diff --git a/src/librand/lib.rs b/src/librand/lib.rs index ca05db15ffeb9..5e56b0d8ab16c 100644 --- a/src/librand/lib.rs +++ b/src/librand/lib.rs @@ -31,7 +31,7 @@ issue = "27703")] #![feature(core_intrinsics)] #![feature(staged_api)] -#![feature(step_by)] +#![feature(iterator_step_by)] #![feature(custom_attribute)] #![feature(specialization)] #![allow(unused_attributes)] diff --git a/src/test/run-pass/impl-trait/example-calendar.rs b/src/test/run-pass/impl-trait/example-calendar.rs index 2a9af26881c77..0b91f7c6b7de1 100644 --- a/src/test/run-pass/impl-trait/example-calendar.rs +++ b/src/test/run-pass/impl-trait/example-calendar.rs @@ -162,22 +162,10 @@ impl<'a, 'b> std::ops::Add<&'b NaiveDate> for &'a NaiveDate { } impl std::iter::Step for NaiveDate { - fn step(&self, by: &Self) -> Option { - Some(self + by) - } - - fn steps_between(_: &Self, _: &Self, _: &Self) -> Option { - unimplemented!() - } - fn steps_between_by_one(_: &Self, _: &Self) -> Option { unimplemented!() } - fn is_negative(&self) -> bool { - false - } - fn replace_one(&mut self) -> Self { mem::replace(self, NaiveDate(0, 0, 1)) } diff --git a/src/test/run-pass/range_inclusive.rs b/src/test/run-pass/range_inclusive.rs index cfa9f6e36e9bc..3067021965a13 100644 --- a/src/test/run-pass/range_inclusive.rs +++ b/src/test/run-pass/range_inclusive.rs @@ -10,7 +10,7 @@ // Test inclusive range syntax. -#![feature(inclusive_range_syntax, inclusive_range, step_by)] +#![feature(inclusive_range_syntax, inclusive_range, iterator_step_by)] use std::ops::{RangeInclusive, RangeToInclusive}; diff --git a/src/test/run-pass/sync-send-iterators-in-libcore.rs b/src/test/run-pass/sync-send-iterators-in-libcore.rs index d12bdf182fa4d..c11a0d391a469 100644 --- a/src/test/run-pass/sync-send-iterators-in-libcore.rs +++ b/src/test/run-pass/sync-send-iterators-in-libcore.rs @@ -14,7 +14,7 @@ #![feature(iter_empty)] #![feature(iter_once)] #![feature(iter_unfold)] -#![feature(step_by)] +#![feature(iterator_step_by)] #![feature(str_escape)] use std::iter::{empty, once, repeat};