Step::replace_one should put a one, not a zero (Issue #41492)

[scottmcm]

Turns out all six of the replace_* impls were backwards.

[rust-highfive]

Thanks for the pull request, and welcome! The Rust team is excited to review your changes, and you should hear from @sfackler (or someone else) soon.

If any changes to this PR are deemed necessary, please add them as extra commits. This ensures that the reviewer can see what has changed since they last reviewed the code. Due to the way GitHub handles out-of-date commits, this should also make it reasonably obvious what issues have or haven't been addressed. Large or tricky changes may require several passes of review and changes.

Please see the contribution instructions for more information.

[sfackler]

I haven't really been following the Step trait design, but if these impls were this broken, it seems like the level of testing of that trait is pretty lacking.

[carols10cents]

@sfackler there are some tests in this PR, are you asking for more tests? Just trying to understand what the next steps are on this PR :)

[sfackler]

I am trying to understand why these methods exist if no one has noticed for so long that they return the wrong value :)

[scottmcm]

For context, I encountered this trying to implement this proposal rust-lang/rfcs#1980 after the discussion in #28237 (comment).

As for why it wasn't noticed, it looks like std only uses it when producing a RangeInclusive::Empty, and apparently nobody ever cared about the value inside that. (Which I suppose is an interesting argument that the field in that variant is also useless.) Edit: actually, the only use is an implementation detail of StepBy<A, ops::RangeInclusive<A>>::next, which uses replace_zero but never actually exposes that zero.

I agree that the Step trait is sub-optimal, but that's already tracked by #27741. I'd propose making this fix and having the "do these methods make sense" discussion there.

[sfackler]

This is certainly an improvement on the status quo

@bors r+

[bors]

📌 Commit f8c6436 has been approved by sfackler