Repository navigation
Tracking issue for step_trait stabilization #42168
Description
Activity
PR #43077 does items 1
and 3in the original message of this issue.I’ve removed the i128/u128 stuff from my PR because I suspect my mixed-signdeness mixed-width interger arithmetic was buggy. I also massaged the
Steptrait some more and came up with this:/// Supporting trait for allowing ranges of various different types to be iterators. #[unstable(feature = "step_trait", reason = "recently redesigned", issue = "42168")] pub trait Step: Clone + PartialOrd + Sized { /// 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) -> Option<usize>; /// “Go forward” (for integers, add) the given number of steps, returning None on overflow. fn forward(&self, step_count: usize) -> Option<Self>; /// “Go backward” (for integers, subtract) the given number of steps, returning None on overflow. fn backward(&self, step_count: usize) -> Option<Self>; /// Modify the given inclusive range so that it becomes empty, /// for example by setting it to `1...0`. fn make_inclusive_range_empty(range: &mut ops::RangeInclusive<Self>); }
How does it look?
I’m a bit uncertain of the exact impls for integers, there is up to 6 cases to consider: {smaller, same width, larger} than usize/isize × {signed, unsigned}.
- added a commit that references this issue
on Jul 8, 2017 I’ve tweaked this design some more and opened #43127. I believe that PR fixes every issue I know of around the
Steptrait.70 remaining items
- added a commit that references this issue
on Apr 6, 2026 - added a commit that references this issue
on Apr 8, 2026 - added a commit that references this issue
on Jun 11, 2026 - added a commit that references this issue
on Aug 7, 2026 Do we really want to allow
forwardto saturate? I think that is very confusing and semantically wrong. There is no sense in which 255 is the successor of 255 (onNonZero<u8>). There is a sense in which 0 is the successor of 255 (onu8), so wrapping makes sense -- but saturating does not.If 255 has a successor on
NonZero<u8>, it would be 1 (that'd be my interpretation of "wrapping" for this type). That would be strictly better than saturating, IMO. But really I thinkforwardshould just always panic for such cases where there's no "good" successor: we should allow panic and wrapping, but nothing else. Asking for the successor of 255 onNonZero<u8>is clearly buggy code (it panics in debug builds after all). Falling back to wrapping arithmetic is an accepted trade-off when the performance cost of detecting the bug is too high, but falling back to pretty much nonsensical behavior (such as saturation) is so much worse that I don't think this is a good trade-off.Reacted by Lucas and Gabriel Bjørnager JensenDo we really want to allow
forwardto saturate? I think that is very confusing and semantically wrong. There is no sense in which 255 is the successor of 255 (onNonZero<u8>). There is a sense in which 0 is the successor of 255 (onu8), so wrapping makes sense -- but saturating does not.
[...]I was worried about the performance impact of this, but at least on x86, wrapping behavior takes the same number of instructions as saturating behavior (and both are branchless):
#[unsafe(no_mangle)] fn sat_add_nonzero(x: u32, y: u32) -> u32 { // add edi, esi // mov eax, -1 // cmovae eax, edi // ret x.saturating_add(y) } #[unsafe(no_mangle)] fn wrapping_add_nonzero(x: u32, y: u32) -> u32 { // mov eax, edi // add eax, esi // adc eax, 0 // ret let (res, ovfl) = x.overflowing_add(y); // an if branch here generates the same code core::hint::select_unpredictable(ovfl, res.wrapping_add(1), res) }
See on godbolt: https://godbolt.org/z/qbxdfoGWG
I think it's overly strict on the allowed behaviours when given an invalid increment. To me, it's a safe
forward_uncheckedmore than an unwrappedforward_checked, where you replace "UB" in the definition offorward_uncheckedwith "panics or returns an arbitrary value."In defense of saturation, though, for
NonZero<u8>theaddcworks if the increment is at most 256. But it can be anyusize, what happens when it wraps two or three times? Rather than figuring outmod 255, ifforwardalways saturates it doesn't matter how many times your increment would wrap, the answer is always 255. This holds for other bounded integer types too, e.g. aGridIndex(u8)that validates the input number is in[0, 99]; rather than calculatemod 100, can just saturate at 99 with asubandcmov.Reacted by Techcable"returns an arbitrary value" is a great way to break programs. That feels like a gross violation of the usual principles of API design that the standard library follows.
Lucky enough, the spec is nowhere near as permissive as that. But it allows "saturation", and that's already going too far IMO.
View all comments
Split off from #27741 because the stabilization path for
step_byhas moved to being on iterators (#41439), and thus not using theSteptrait.step,steps_between, andis_negativeonce Range::step_by is deletedreplace_zeroandreplace_onewith something more useful (some options: Make RangeInclusive just a two-field struct (amend 1192) rfcs#1980 (comment))steps_between_by_oneso thatRange<u128>can beTrustedLen(rather than it only working well with types that fit inusize)steps_betweenshould work Step::steps_between does not distinguish overflow and unimplemented (unstable) #48117core::iterorcore::range?(and probably more)