Skip to content

distinguish overflow and unimplemented in Step::steps_between #130867

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Merged
merged 1 commit into from
Nov 22, 2024
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
2 changes: 1 addition & 1 deletion compiler/rustc_abi/src/lib.rs
Original file line number Diff line number Diff line change
Expand Up @@ -624,7 +624,7 @@ impl AddAssign for Size {
#[cfg(feature = "nightly")]
impl Step for Size {
#[inline]
fn steps_between(start: &Self, end: &Self) -> Option<usize> {
fn steps_between(start: &Self, end: &Self) -> (usize, Option<usize>) {
u64::steps_between(&start.bytes(), &end.bytes())
}

Expand Down
2 changes: 1 addition & 1 deletion compiler/rustc_index_macros/src/newtype.rs
Original file line number Diff line number Diff line change
Expand Up @@ -122,7 +122,7 @@ impl Parse for Newtype {
#gate_rustc_only
impl ::std::iter::Step for #name {
#[inline]
fn steps_between(start: &Self, end: &Self) -> Option<usize> {
fn steps_between(start: &Self, end: &Self) -> (usize, Option<usize>) {
<usize as ::std::iter::Step>::steps_between(
&Self::index(*start),
&Self::index(*end),
Expand Down
128 changes: 66 additions & 62 deletions library/core/src/iter/range.rs
Original file line number Diff line number Diff line change
Expand Up @@ -22,23 +22,21 @@ unsafe_impl_trusted_step![AsciiChar char i8 i16 i32 i64 i128 isize u8 u16 u32 u6
/// The *predecessor* operation moves towards values that compare lesser.
#[unstable(feature = "step_trait", issue = "42168")]
pub trait Step: Clone + PartialOrd + Sized {
/// Returns the number of *successor* steps required to get from `start` to `end`.
/// Returns the bounds on the number of *successor* steps required to get from `start` to `end`
/// like [`Iterator::size_hint()`][Iterator::size_hint()].
///
/// Returns `None` if the number of steps would overflow `usize`
/// (or is infinite, or if `end` would never be reached).
/// Returns `(usize::MAX, None)` if the number of steps would overflow `usize`, or is infinite.
///
/// # Invariants
///
/// For any `a`, `b`, and `n`:
///
/// * `steps_between(&a, &b) == Some(n)` if and only if `Step::forward_checked(&a, n) == Some(b)`
/// * `steps_between(&a, &b) == Some(n)` if and only if `Step::backward_checked(&b, n) == Some(a)`
/// * `steps_between(&a, &b) == Some(n)` only if `a <= b`
/// * Corollary: `steps_between(&a, &b) == Some(0)` if and only if `a == b`
/// * Note that `a <= b` does _not_ imply `steps_between(&a, &b) != None`;
/// this is the case when it would require more than `usize::MAX` steps to get to `b`
/// * `steps_between(&a, &b) == None` if `a > b`
fn steps_between(start: &Self, end: &Self) -> Option<usize>;
/// * `steps_between(&a, &b) == (n, Some(n))` if and only if `Step::forward_checked(&a, n) == Some(b)`
/// * `steps_between(&a, &b) == (n, Some(n))` if and only if `Step::backward_checked(&b, n) == Some(a)`
/// * `steps_between(&a, &b) == (n, Some(n))` only if `a <= b`
/// * Corollary: `steps_between(&a, &b) == (0, Some(0))` if and only if `a == b`
/// * `steps_between(&a, &b) == (0, None)` if `a > b`
fn steps_between(start: &Self, end: &Self) -> (usize, Option<usize>);

/// Returns the value that would be obtained by taking the *successor*
/// of `self` `count` times.
Expand Down Expand Up @@ -169,7 +167,7 @@ pub trait Step: Clone + PartialOrd + Sized {
/// For any `a`:
///
/// * if there exists `b` such that `b < a`, it is safe to call `Step::backward_unchecked(a, 1)`
/// * if there exists `b`, `n` such that `steps_between(&b, &a) == Some(n)`,
/// * if there exists `b`, `n` such that `steps_between(&b, &a) == (n, Some(n))`,
/// it is safe to call `Step::backward_unchecked(a, m)` for any `m <= n`.
/// * Corollary: `Step::backward_unchecked(a, 0)` is always safe.
///
Expand Down Expand Up @@ -261,12 +259,13 @@ macro_rules! step_integer_impls {
step_unsigned_methods!();

#[inline]
fn steps_between(start: &Self, end: &Self) -> Option<usize> {
fn steps_between(start: &Self, end: &Self) -> (usize, Option<usize>) {
if *start <= *end {
// This relies on $u_narrower <= usize
Some((*end - *start) as usize)
let steps = (*end - *start) as usize;
(steps, Some(steps))
} else {
None
(0, None)
}
}

Expand Down Expand Up @@ -294,16 +293,17 @@ macro_rules! step_integer_impls {
step_signed_methods!($u_narrower);

#[inline]
fn steps_between(start: &Self, end: &Self) -> Option<usize> {
fn steps_between(start: &Self, end: &Self) -> (usize, Option<usize>) {
if *start <= *end {
// This relies on $i_narrower <= usize
//
// Casting to isize extends the width but preserves the sign.
// Use wrapping_sub in isize space and cast to usize to compute
// the difference that might not fit inside the range of isize.
Some((*end as isize).wrapping_sub(*start as isize) as usize)
let steps = (*end as isize).wrapping_sub(*start as isize) as usize;
(steps, Some(steps))
} else {
None
(0, None)
}
}

Expand Down Expand Up @@ -359,11 +359,15 @@ macro_rules! step_integer_impls {
step_unsigned_methods!();

#[inline]
fn steps_between(start: &Self, end: &Self) -> Option<usize> {
fn steps_between(start: &Self, end: &Self) -> (usize, Option<usize>) {
if *start <= *end {
usize::try_from(*end - *start).ok()
if let Ok(steps) = usize::try_from(*end - *start) {
(steps, Some(steps))
} else {
(usize::MAX, None)
}
} else {
None
(0, None)
}
}

Expand All @@ -385,16 +389,22 @@ macro_rules! step_integer_impls {
step_signed_methods!($u_wider);

#[inline]
fn steps_between(start: &Self, end: &Self) -> Option<usize> {
fn steps_between(start: &Self, end: &Self) -> (usize, Option<usize>) {
if *start <= *end {
match end.checked_sub(*start) {
Some(result) => usize::try_from(result).ok(),
Some(result) => {
if let Ok(steps) = usize::try_from(result) {
(steps, Some(steps))
} else {
(usize::MAX, None)
}
}
// If the difference is too big for e.g. i128,
// it's also gonna be too big for usize with fewer bits.
None => None,
None => (usize::MAX, None),
}
} else {
None
(0, None)
}
}

Expand Down Expand Up @@ -433,18 +443,26 @@ step_integer_impls! {
#[unstable(feature = "step_trait", reason = "recently redesigned", issue = "42168")]
impl Step for char {
#[inline]
fn steps_between(&start: &char, &end: &char) -> Option<usize> {
fn steps_between(&start: &char, &end: &char) -> (usize, Option<usize>) {
let start = start as u32;
let end = end as u32;
if start <= end {
let count = end - start;
if start < 0xD800 && 0xE000 <= end {
usize::try_from(count - 0x800).ok()
if let Ok(steps) = usize::try_from(count - 0x800) {
(steps, Some(steps))
} else {
(usize::MAX, None)
}
} else {
usize::try_from(count).ok()
if let Ok(steps) = usize::try_from(count) {
(steps, Some(steps))
} else {
(usize::MAX, None)
}
}
} else {
None
(0, None)
}
}

Expand Down Expand Up @@ -512,7 +530,7 @@ impl Step for char {
#[unstable(feature = "step_trait", reason = "recently redesigned", issue = "42168")]
impl Step for AsciiChar {
#[inline]
fn steps_between(&start: &AsciiChar, &end: &AsciiChar) -> Option<usize> {
fn steps_between(&start: &AsciiChar, &end: &AsciiChar) -> (usize, Option<usize>) {
Step::steps_between(&start.to_u8(), &end.to_u8())
}

Expand Down Expand Up @@ -554,7 +572,7 @@ impl Step for AsciiChar {
#[unstable(feature = "step_trait", reason = "recently redesigned", issue = "42168")]
impl Step for Ipv4Addr {
#[inline]
fn steps_between(&start: &Ipv4Addr, &end: &Ipv4Addr) -> Option<usize> {
fn steps_between(&start: &Ipv4Addr, &end: &Ipv4Addr) -> (usize, Option<usize>) {
u32::steps_between(&start.to_bits(), &end.to_bits())
}

Expand Down Expand Up @@ -586,7 +604,7 @@ impl Step for Ipv4Addr {
#[unstable(feature = "step_trait", reason = "recently redesigned", issue = "42168")]
impl Step for Ipv6Addr {
#[inline]
fn steps_between(&start: &Ipv6Addr, &end: &Ipv6Addr) -> Option<usize> {
fn steps_between(&start: &Ipv6Addr, &end: &Ipv6Addr) -> (usize, Option<usize>) {
u128::steps_between(&start.to_bits(), &end.to_bits())
}

Expand Down Expand Up @@ -690,11 +708,8 @@ impl<A: Step> RangeIteratorImpl for ops::Range<A> {

#[inline]
default fn spec_advance_by(&mut self, n: usize) -> Result<(), NonZero<usize>> {
let available = if self.start <= self.end {
Step::steps_between(&self.start, &self.end).unwrap_or(usize::MAX)
} else {
0
};
let steps = Step::steps_between(&self.start, &self.end);
let available = steps.1.unwrap_or(steps.0);

let taken = available.min(n);

Expand Down Expand Up @@ -731,11 +746,8 @@ impl<A: Step> RangeIteratorImpl for ops::Range<A> {

#[inline]
default fn spec_advance_back_by(&mut self, n: usize) -> Result<(), NonZero<usize>> {
let available = if self.start <= self.end {
Step::steps_between(&self.start, &self.end).unwrap_or(usize::MAX)
} else {
0
};
let steps = Step::steps_between(&self.start, &self.end);
let available = steps.1.unwrap_or(steps.0);

let taken = available.min(n);

Expand Down Expand Up @@ -775,11 +787,8 @@ impl<T: TrustedStep> RangeIteratorImpl for ops::Range<T> {

#[inline]
fn spec_advance_by(&mut self, n: usize) -> Result<(), NonZero<usize>> {
let available = if self.start <= self.end {
Step::steps_between(&self.start, &self.end).unwrap_or(usize::MAX)
} else {
0
};
let steps = Step::steps_between(&self.start, &self.end);
let available = steps.1.unwrap_or(steps.0);

let taken = available.min(n);

Expand Down Expand Up @@ -819,11 +828,8 @@ impl<T: TrustedStep> RangeIteratorImpl for ops::Range<T> {

#[inline]
fn spec_advance_back_by(&mut self, n: usize) -> Result<(), NonZero<usize>> {
let available = if self.start <= self.end {
Step::steps_between(&self.start, &self.end).unwrap_or(usize::MAX)
} else {
0
};
let steps = Step::steps_between(&self.start, &self.end);
let available = steps.1.unwrap_or(steps.0);

let taken = available.min(n);

Expand All @@ -846,8 +852,7 @@ impl<A: Step> Iterator for ops::Range<A> {
#[inline]
fn size_hint(&self) -> (usize, Option<usize>) {
if self.start < self.end {
let hint = Step::steps_between(&self.start, &self.end);
(hint.unwrap_or(usize::MAX), hint)
Step::steps_between(&self.start, &self.end)
} else {
(0, Some(0))
}
Expand All @@ -856,7 +861,7 @@ impl<A: Step> Iterator for ops::Range<A> {
#[inline]
fn count(self) -> usize {
if self.start < self.end {
Step::steps_between(&self.start, &self.end).expect("count overflowed usize")
Step::steps_between(&self.start, &self.end).1.expect("count overflowed usize")
} else {
0
}
Expand Down Expand Up @@ -980,11 +985,11 @@ impl<A: Step> DoubleEndedIterator for ops::Range<A> {
// Safety:
// The following invariants for `Step::steps_between` exist:
//
// > * `steps_between(&a, &b) == Some(n)` only if `a <= b`
// > * Note that `a <= b` does _not_ imply `steps_between(&a, &b) != None`;
// > * `steps_between(&a, &b) == (n, Some(n))` only if `a <= b`
// > * Note that `a <= b` does _not_ imply `steps_between(&a, &b) != (n, None)`;
// > this is the case when it would require more than `usize::MAX` steps to
// > get to `b`
// > * `steps_between(&a, &b) == None` if `a > b`
// > * `steps_between(&a, &b) == (0, None)` if `a > b`
//
// The first invariant is what is generally required for `TrustedLen` to be
// sound. The note addendum satisfies an additional `TrustedLen` invariant.
Expand Down Expand Up @@ -1253,10 +1258,8 @@ impl<A: Step> Iterator for ops::RangeInclusive<A> {
return (0, Some(0));
}

match Step::steps_between(&self.start, &self.end) {
Some(hint) => (hint.saturating_add(1), hint.checked_add(1)),
None => (usize::MAX, None),
}
let hint = Step::steps_between(&self.start, &self.end);
(hint.0.saturating_add(1), hint.1.and_then(|steps| steps.checked_add(1)))
}

#[inline]
Expand All @@ -1266,6 +1269,7 @@ impl<A: Step> Iterator for ops::RangeInclusive<A> {
}

Step::steps_between(&self.start, &self.end)
.1
.and_then(|steps| steps.checked_add(1))
.expect("count overflowed usize")
}
Expand Down
35 changes: 23 additions & 12 deletions library/core/tests/iter/traits/step.rs
Original file line number Diff line number Diff line change
Expand Up @@ -2,26 +2,37 @@ use core::iter::*;

#[test]
fn test_steps_between() {
assert_eq!(Step::steps_between(&20_u8, &200_u8), Some(180_usize));
assert_eq!(Step::steps_between(&-20_i8, &80_i8), Some(100_usize));
assert_eq!(Step::steps_between(&-120_i8, &80_i8), Some(200_usize));
assert_eq!(Step::steps_between(&20_u32, &4_000_100_u32), Some(4_000_080_usize));
assert_eq!(Step::steps_between(&-20_i32, &80_i32), Some(100_usize));
assert_eq!(Step::steps_between(&-2_000_030_i32, &2_000_050_i32), Some(4_000_080_usize));
assert_eq!(Step::steps_between(&20_u8, &200_u8), (180_usize, Some(180_usize)));
assert_eq!(Step::steps_between(&-20_i8, &80_i8), (100_usize, Some(100_usize)));
assert_eq!(Step::steps_between(&-120_i8, &80_i8), (200_usize, Some(200_usize)));
assert_eq!(
Step::steps_between(&20_u32, &4_000_100_u32),
(4_000_080_usize, Some(4_000_080_usize))
);
assert_eq!(Step::steps_between(&-20_i32, &80_i32), (100_usize, Some(100_usize)));
assert_eq!(
Step::steps_between(&-2_000_030_i32, &2_000_050_i32),
(4_000_080_usize, Some(4_000_080_usize))
);

// Skip u64/i64 to avoid differences with 32-bit vs 64-bit platforms

assert_eq!(Step::steps_between(&20_u128, &200_u128), Some(180_usize));
assert_eq!(Step::steps_between(&-20_i128, &80_i128), Some(100_usize));
assert_eq!(Step::steps_between(&20_u128, &200_u128), (180_usize, Some(180_usize)));
assert_eq!(Step::steps_between(&-20_i128, &80_i128), (100_usize, Some(100_usize)));
if cfg!(target_pointer_width = "64") {
assert_eq!(Step::steps_between(&10_u128, &0x1_0000_0000_0000_0009_u128), Some(usize::MAX));
assert_eq!(
Step::steps_between(&10_u128, &0x1_0000_0000_0000_0009_u128),
(usize::MAX, Some(usize::MAX))
);
}
assert_eq!(Step::steps_between(&10_u128, &0x1_0000_0000_0000_000a_u128), None);
assert_eq!(Step::steps_between(&10_i128, &0x1_0000_0000_0000_000a_i128), None);
assert_eq!(Step::steps_between(&10_u128, &0x1_0000_0000_0000_000a_u128), (usize::MAX, None));
assert_eq!(Step::steps_between(&10_i128, &0x1_0000_0000_0000_000a_i128), (usize::MAX, None));
assert_eq!(
Step::steps_between(&-0x1_0000_0000_0000_0000_i128, &0x1_0000_0000_0000_0000_i128,),
None,
(usize::MAX, None),
);

assert_eq!(Step::steps_between(&100_u32, &10_u32), (0, None));
}

#[test]
Expand Down
2 changes: 1 addition & 1 deletion tests/ui/impl-trait/example-calendar.rs
Original file line number Diff line number Diff line change
Expand Up @@ -156,7 +156,7 @@ impl<'a, 'b> std::ops::Add<&'b NaiveDate> for &'a NaiveDate {
}

impl std::iter::Step for NaiveDate {
fn steps_between(_: &Self, _: &Self) -> Option<usize> {
fn steps_between(_: &Self, _: &Self) -> (usize, Option<usize>) {
unimplemented!()
}

Expand Down
4 changes: 2 additions & 2 deletions tests/ui/issues/issue-48006.rs
Original file line number Diff line number Diff line change
Expand Up @@ -6,10 +6,10 @@ use std::iter::Step;

#[cfg(target_pointer_width = "16")]
fn main() {
assert!(Step::steps_between(&0u32, &u32::MAX).is_none());
assert!(Step::steps_between(&0u32, &u32::MAX).1.is_none());
}

#[cfg(any(target_pointer_width = "32", target_pointer_width = "64"))]
fn main() {
assert!(Step::steps_between(&0u32, &u32::MAX).is_some());
assert!(Step::steps_between(&0u32, &u32::MAX).1.is_some());
}
Loading