step_by() API change

[leonardo-m]

In past step_by() needed an argument of the same type of the range:

#![feature(iterator_step_by)]
fn main() {
    for i in (1u32 .. 10u32).step_by(2u32) {
        println!("{}", i);
    }
}

But now it gives an error and asks for an usize:

error[E0308]: mismatched types
 --> ...\test.rs:3:38
  |
3 |     for i in (1u32 .. 10u32).step_by(2u32) {
  |                                      ^^^^ expected usize, found u32

Is this desired (and I have to change all my Nightly code)?

This is especially bad when I've used negative step values:

(a - 2 .. 1).step_by(-2)

Now you need to write something like:

(2 + a % 2 .. a - 1).rev().step_by(2)

See also #43012

[bstrie]

Is this a deliberate regression? Trying to follow along with the discussion for the new step_by, but it seems quite tangled and I don't see a definitive RFC.

cc @SimonSapin based on your comments in #43012

[SimonSapin]

#27741 (comment) looks like a decision to move step_by to the Iterator trait, based on Iterator::nth. Since nth takes usize, it seems expected to me that the new step_by does too.

One way to think of it is as a number of iterations rather than a "distance" between points.

[leonardo-m]

There are situations where I'd like to write:

for i in (20u32 .. 0u32).step_by(-2) { }

That is, negative stepping even when the range is on unsigned values :-) (This code was disallowed even by the old step_by, but it's handy).

[kennytm]

@leonardo-m This has been discussed before and is quickly dismissed because it is confusing. Should (20 .. 0).step_by(-2) produce [20, 18, ..., 2] or [19, 17, ..., 1] or [18, 16, ..., 0]? (Similar confusion exists for (0 .. 4).step_by(-2).)

I guess you need to write your own iterator or function producing StepBy<Either<Range, Rev<Range>>> in those cases where the step can be positive or negative.

[leonardo-m]

I think it should give [20, 18, 16, 14, 12, 10, 8, 6, 4, 2].

Do you suggest to close this issue down then?

[kennytm]

@leonardo-m I do think Iterator::step_by taking usize is expected. There is no "same type of the range" if you want to also support ["x", "y", "z"].iter().step_by(2).

[leonardo-m]

It's a quite limiting API. What about using isize?

[cramertj]

@leonardo-m What would that do? Just Iterator::rev and then step_by?

[leonardo-m]

It's a single operation. ['a', 'b', 'c', 'd', 'e'].iter().step_by(-2) could give something like ['e', 'c', 'a'].

[cramertj]

Right-- I'm asking how that operation should be implemented generically for iterators. It seems like the most obvious way would be to call Iterator::rev and then step_by, which you could just do yourself: ['a', 'b', 'c', 'd', 'e'].iter().rev().step_by(2).

[leonardo-m]

The "-2" isn't always hard-coded, in general it's a variable, and it can change at run-time. So you don't know if you should put a rev() there or not. If you look in other threads people are proposing solutions to this... a way to reverse the scanning conditionally...

[SimonSapin]

As I’ve said elsewhere, I ended up writing this for Servo:

struct MaybeReverse<I> {
    iter: I,
    reverse: bool,
}

impl<I: DoubleEndedIterator> Iterator for MaybeReverse<I> {
    type Item = I::Item;

    fn next(&mut self) -> Option<I::Item> {
        if self.reverse {
            self.iter.next_back()
        } else {
            self.iter.next()
        }
    }
}

[leonardo-m]

Yes, I saw that...

step_by() is going to be a very commonly used function, and I think most times it will be called on ranges of integral values. Currently you can't use it ( #43064 ). If you want Rust code to compile fast you need to keep extrinsic complexity low and abstraction as low as possible, especially for commonly used things.

[SimonSapin]

@leonardo-m #43064 is not controversial. I’m working on a PR.

[SimonSapin]

#43077 should fix the performance problem.

[leonardo-m]

Thank you. In my last comment I was talking about general Rust code compilation speed (while Issue 43064 is about run-time performance).

Code where you don't know at compile-time if you need to step forwards or backwards exists, I think I have such code in some matrix scanning strategies, but it's uncommon.

[leonardo-m]

I close this down because most API discussions already happened elsewhere, and they are settled.