Skip to content

Commit

Permalink
Auto merge of rust-lang#130867 - michirakara:steps_between, r=dtolnay
Browse files Browse the repository at this point in the history
distinguish overflow and unimplemented in Step::steps_between
  • Loading branch information
bors committed Nov 22, 2024
2 parents 40d6e2c + 276c0fc commit 23a1b31
Show file tree
Hide file tree
Showing 2 changed files with 89 additions and 74 deletions.
128 changes: 66 additions & 62 deletions 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 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

0 comments on commit 23a1b31

Please sign in to comment.