Remove deprecated Range::step_by (use Iterator::step_by instead)

[scottmcm]

Follow-up to #41439 (comment)

While doing so, remove the now-unused step, steps_between, and is_negative methods from Step.

Mostly simple, but needed two interesting changes:

• Override Iterator::size_hint for iter::StepBy (so hints aren't lost)
• Override Iterator::size_hint for ops::RangeFrom (so (0..).size_hint() returns what (0..).step_by(1).size_hint() used to)

(It turns out that (0..).step_by(d) is used in a bunch of tests, from cycle to vec_deque.)

Incidentally fixes #41477

[rust-highfive]

r? @alexcrichton

(rust_highfive has picked a reviewer for you, use r? to override)

[Mark-Simulacrum]

Marking as needing crater, since we could potentially break a lot of crates with this change. cc @brson, @eddyb

[ivandardi]

Hm... The iterator_step_by entry is missing from this list. Do you mind adding it, please? 😃

[ivandardi]

The current StepBy (in src/libcore/iter/mod.rs) description is An iterator that steps by n elements every iteration. It's badly worded (by me, sorry), and I think reusing the old range::StepBy description, but adapted, works better: An adapter for stepping iterators by a custom amount.. Could you please change that?

[scottmcm]

Updated with @ivandardi's suggestions.

Crater is a good point. If it would help, I could split this into sequential PRs:

1. Add the size_hint overrides: non-breaking
2. Make Range::step_by redirect to Iterator::step_by, and alias the type: catches the interesting breaks in the parameter type change from T to usize
3. Remove the now-unused methods on Step: breaking for implementors
4. Actually remove the old step_by methods, type, and feature: breaking, but mechanical to fix

[alexcrichton]

Thanks for the PR @scottmcm! Just to make sure I understand the current state of affairs:

• Stabilize step_by by adding it to Iterator (issue #27741) #41439 landed Iterator::step_by and the unstable Range::step_by was deprecated and/or renamed to a different name.
• This PR is removing the deprecations of Range::step_by, only leaving Iterator::step_by

If that's true, then I've got a few questions:

• Everything here is unstable, right? How come we need crater?
• Do you know what the remaining users of the Step trait are? If so, can you summarize them?

[scottmcm]

@alexcrichton Yup, that's the state of things.

Everything here is unstable, right? How come we need crater?

I guess a better statement is that this is a removal of a fairly old feature, so it might be worth thinking about the impact even though it's only to nightly. But we probably don't need the certainty of a crater run, a GitHub search is likely sufficient.

Is there a general preference between a breaking change going under a new feature (so the error is up-front and there's something to search) or re-using the existing feature (so basic but common uses -- here step_by a positive literal -- just continue to work)?

Do you know what the remaining users of the Step trait are? If so, can you summarize them?

It's still used for

impl<A: Step> Iterator for ops::Range<A>
impl<A: Step> Iterator for ops::RangeFrom<A>
impl<A: Step> Iterator for ops::RangeInclusive<A>

(and all the more-specific iterators as well)

I think the best thing to do with Step is move it to a different tracking issue from step_by, since the trait is irrelevant to stabilizing the new method.

[alexcrichton]

Ok cool, thanks for the clarifications! We historically have had a process around removing deprecated features from the standard library: (a) add a replacement (b) deprecate an api (c) wait a cycle and (d) delete the api. Nowadays though the amount of widely-used unstable apis in libstd seems to have drastically diminished so such a process may not be so required any more.

In that sense a breaking change could be ok here if there's not a whole lot of users, but if there's a lot of users we may wish for a cycle of deprecation before removal. Does that make sense?

[scottmcm]

That makes sense. Thanks for the advice; I just got too excited at deleting stuff 🙂

I'll close this and open some new, smaller PRs.