-
Notifications
You must be signed in to change notification settings - Fork 12.9k
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
Tracking issue for step_by
stabilization
#27741
Comments
Nominating for P-High. |
triage: P-high |
cc #23588 |
We should probably start moving on this in the next cycle or drop the P-high. |
Would definitely like some help getting this over the finish line. I'm happy to mentor! |
The library subteam decided to not move this to FCP this cycle. The API is pretty likely to need revision in one form or another and that should be taken care of before a FCP happens. I believe discussion is still going on in the internals post though! Also moving to P-medium |
It seems odd to me that the |
Where do we stand on this at the moment? This is a feature I'd very much like to use in stable, and am willing to provide help to get this finalized (with some mentoring). |
Based on the documentation, I would expect |
Why do you expect that? Overflowing the range of an integer type is an error (it panics in debug mode), it’s not the expected end of a |
@SimonSapin The example for for i in (0u8..).step_by(2) {
println!("{}", i);
} I think that this example is misleading: its explanation says "This prints all even u8 values.", but instead it overflows on panic in debug and loops forever in release. |
Indeed, that example is wrong. How does this look? #31172 |
Ditto @dschatzberg's comment. Imagine something like stepping through a range of two |
A sequence of floats like 0.1, 0.2, ..., 0.5 is so common in numerical codes that Matlab has a built-in operator for it: But I understand your rigorous approach but it would be handy to have |
Also, I think we should consider inclusion of which, to my eyes, is more generic (applicable to all iterators) |
IMHO, step_by should be a general abstraction for all iterators as mentioned by @nodakai and the current implementation that uses addition must be at most a specialization for efficiency sake for integral types (now that Rust has specialization implemented). |
yigal100's approach seems sensible, in which case Another option would be to drop Neither of these options requires a The questions which I think need answering are: 1 What are the main use-cases?In my mind these are: a. producing a sequential list of array indices (i.e. non-negative integers), usually stepping by +1 but possibly also -1, 2, etc., and 2 What restrictions are there on syntax / implementation?As far as I am aware, the Is there a fundamental reason that 3 Flexibility, optimizability and intuitivenessWhich is clearer, Is there any point allowing negative steps? Maybe not for indices since One thing I like about the |
Team member @SimonSapin has proposed to merge this. The next step is review by the rest of the tagged teams: No concerns currently listed. Once a majority of reviewers approve (and none object), this will enter its final comment period. If you spot a major issue that hasn't been raised at any point in this process, please speak up! See this document for info about what commands tagged team members can give me. |
Is there an RFC or post or something that documents the behavior being proposed for stabilization? I find it extremely hard to extract from this issue what is exactly being proposed for stabilization, what is the rationale, alternatives that have been explored, etc. I am not suggesting that this needs an RFC, but at least a summary of the work done and why what we have now is better and suitable for stabilization that what we had before. As a maintainer of collections implement step_by I would find such information useful to update my collections accordingly. |
@gnzlbg Yeah, this issue has a lot of history and can be hard to read now. https://doc.rust-lang.org/std/iter/trait.Iterator.html#method.step_by
|
@SimonSapin thanks for the summary, that is really helpful. |
I agree that step_by is good for iterators in general, but we risk a situation where we have to advise not using .step_by on ranges, which is going to be uphill ergonomically. I had hopes With the right tooling and right method name, a “increment type matching” stepping adaptor for ranges might still be easy to find for users. Problems with
|
Could two different named methods ("step" and step_by"?) solve this issue, having one for ranges that takes an usize, and one for intervals that takes the same type as the interval? |
Can't we have the
Or something similar is this doesn't work out or if it's not desirable to add a new associated type to the |
@ivandardi If we did that |
@bluss How about renaming |
I’m ok with |
Semantics and naming choices like |
Then how about |
🔔 This is now entering its final comment period, as per the review above. 🔔 |
The final comment period is now complete. |
I looked at overriding mov r8d, eax
lea r9d, [rcx - 1]
lea eax, [rcx - 2]
xor eax, r9d
add eax, r8d
mov r9d, ecx
add r9d, 6
setb r8b
cmp r9d, edx
jae .LBB1_6
add ecx, 7
test r8b, r8b
je .LBB1_4 which doesn't simplify further because of the slightly-awkward semantics of I think this'd be perfectly efficient in terms of a "return next and also step forward" method, since the Range iterator for that would just return |
Stabilize Iterator::step_by Fixes #27741
Could we amend a note to While specializations of |
Update (@SimonSapin): this is now the tracking issue for an iterator adaptor based on
Iterator::nth
:The
Step
trait used for making ranges iterators is now tracked at #42168.Original issue:
The
step_by
method makes it possible to step through ranges with arbitrary-sized steps. There are several issues that need to be addressed before we can stabilize it:Step
trait, which is currently a bit of a messZero
/One
, tracked hereThe text was updated successfully, but these errors were encountered: