From 276c0fc3fd11224840f81e3f883a1ced6b369fae Mon Sep 17 00:00:00 2001 From: michirakara Date: Wed, 25 Sep 2024 18:09:30 -0700 Subject: [PATCH] distinguish overflow and unimplemented in Step::steps_between --- core/src/iter/range.rs | 128 +++++++++++++++++---------------- core/tests/iter/traits/step.rs | 35 +++++---- 2 files changed, 89 insertions(+), 74 deletions(-) diff --git a/core/src/iter/range.rs b/core/src/iter/range.rs index da4f68a0de4fb..4fa719de5ebf0 100644 --- a/core/src/iter/range.rs +++ b/core/src/iter/range.rs @@ -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; + /// * `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); /// Returns the value that would be obtained by taking the *successor* /// of `self` `count` times. @@ -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. /// @@ -261,12 +259,13 @@ macro_rules! step_integer_impls { step_unsigned_methods!(); #[inline] - fn steps_between(start: &Self, end: &Self) -> Option { + fn steps_between(start: &Self, end: &Self) -> (usize, Option) { 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) } } @@ -294,16 +293,17 @@ macro_rules! step_integer_impls { step_signed_methods!($u_narrower); #[inline] - fn steps_between(start: &Self, end: &Self) -> Option { + fn steps_between(start: &Self, end: &Self) -> (usize, Option) { 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) } } @@ -359,11 +359,15 @@ macro_rules! step_integer_impls { step_unsigned_methods!(); #[inline] - fn steps_between(start: &Self, end: &Self) -> Option { + fn steps_between(start: &Self, end: &Self) -> (usize, Option) { 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) } } @@ -385,16 +389,22 @@ macro_rules! step_integer_impls { step_signed_methods!($u_wider); #[inline] - fn steps_between(start: &Self, end: &Self) -> Option { + fn steps_between(start: &Self, end: &Self) -> (usize, Option) { 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) } } @@ -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 { + fn steps_between(&start: &char, &end: &char) -> (usize, Option) { 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) } } @@ -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 { + fn steps_between(&start: &AsciiChar, &end: &AsciiChar) -> (usize, Option) { Step::steps_between(&start.to_u8(), &end.to_u8()) } @@ -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 { + fn steps_between(&start: &Ipv4Addr, &end: &Ipv4Addr) -> (usize, Option) { u32::steps_between(&start.to_bits(), &end.to_bits()) } @@ -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 { + fn steps_between(&start: &Ipv6Addr, &end: &Ipv6Addr) -> (usize, Option) { u128::steps_between(&start.to_bits(), &end.to_bits()) } @@ -690,11 +708,8 @@ impl RangeIteratorImpl for ops::Range { #[inline] default fn spec_advance_by(&mut self, n: usize) -> Result<(), NonZero> { - 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); @@ -731,11 +746,8 @@ impl RangeIteratorImpl for ops::Range { #[inline] default fn spec_advance_back_by(&mut self, n: usize) -> Result<(), NonZero> { - 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); @@ -775,11 +787,8 @@ impl RangeIteratorImpl for ops::Range { #[inline] fn spec_advance_by(&mut self, n: usize) -> Result<(), NonZero> { - 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); @@ -819,11 +828,8 @@ impl RangeIteratorImpl for ops::Range { #[inline] fn spec_advance_back_by(&mut self, n: usize) -> Result<(), NonZero> { - 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); @@ -846,8 +852,7 @@ impl Iterator for ops::Range { #[inline] fn size_hint(&self) -> (usize, Option) { 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)) } @@ -856,7 +861,7 @@ impl Iterator for ops::Range { #[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 } @@ -980,11 +985,11 @@ impl DoubleEndedIterator for ops::Range { // 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. @@ -1253,10 +1258,8 @@ impl Iterator for ops::RangeInclusive { 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] @@ -1266,6 +1269,7 @@ impl Iterator for ops::RangeInclusive { } Step::steps_between(&self.start, &self.end) + .1 .and_then(|steps| steps.checked_add(1)) .expect("count overflowed usize") } diff --git a/core/tests/iter/traits/step.rs b/core/tests/iter/traits/step.rs index 3d82a40cd2941..bf935af397c71 100644 --- a/core/tests/iter/traits/step.rs +++ b/core/tests/iter/traits/step.rs @@ -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]