Crystal: Negative step size causes infinite loop

[watzon]

This should probably be an error if it's not supported, but it also doesn't say anywhere that it isn't. If you attempt to use a negative step size it results in an infinite loop. Here's a workable example:

(0..10).step(-1).each { |i| puts i }

Ideally this should throw an exception, or ideally, actually work by stepping through the range in reverse.

[straight-shoota]

(0..10).step(-1) should be a noop because you can't reach 10 from 0 with -1 steps.

[bew]

Btw I would expect this to work: (10..0).step(-1).to_a # => [] (https://carc.in/#/r/86bf)
But maybe it's another issue?

[igor-alexandrov]

Btw I would expect this to work: (10..0).step(-1).to_a # => [] (https://carc.in/#/r/86bf)
But maybe it's another issue?

I believe that negative step should raise ArgementError.
@straight-shoota @watzon what do you think?

[watzon]

If it's not going to be supported then I'd say it should. Ideally it would be nice to have reverse ranges supported at least.

[igor-alexandrov]

I can take care of this. @straight-shoota can you assign this to me please?

[asterite]

It should raise on negative step, and zero step. Like in Ruby.

[straight-shoota]

Negative steps in large-to-small range sounds perfectly valid to me ((10..0).step(-1)).

[vlazar]

For the record Ruby does this

p (10..0).step(-1).to_a # => [10, 9, 8, 7, 6, 5, 4, 3, 2, 1, 0] 
p (0..10).step(-1).to_a # => []
p (10..0).step(0).to_a  # => infinite loop
p (0..10).step(0).to_a  # => infinite loop

[igor-alexandrov]

Ok, I will try to cover all cases.

[igor-alexandrov]

What should we do with Float step? Currently it is not supported too:

icr(0.33.0) > (0..10).step(0.1).to_a
Showing last frame. Use --error-trace for full trace.

In /usr/local/Cellar/crystal/0.33.0/src/range.cr:435:15

 435 | @step.times { @current = @current.succ }

Error: undefined method 'times' for Float64

But it works in Ruby 2.6.5 and 2.7.0:

jupiter:~/workspace/crystal (master)$ irb
irb(main):001:0> (0..10).step(0.1).to_a
=> [0.0, 0.1, 0.2, 0.30000000000000004, 0.4, 0.5, 0.6000000000000001, 0.7000000000000001, 0.8, 0.9, 1.0, 1.1, 1.2000000000000002, 1.3, 1.4000000000000001, 1.5, 1.6, 1.7000000000000002, 1.8, 1.9000000000000001, 2.0, 2.1, 2.2, 2.3000000000000003, 2.4000000000000004, 2.5, 2.6, 2.7, 2.8000000000000003, 2.9000000000000004, 3.0, 3.1, 3.2, 3.3000000000000003, 3.4000000000000004, 3.5, 3.6, 3.7, 3.8000000000000003, 3.9000000000000004, 4.0, 4.1000000000000005, 4.2, 4.3, 4.4, 4.5, 4.6000000000000005, 4.7, 4.800000000000001, 4.9, 5.0, 5.1000000000000005, 5.2, 5.300000000000001, 5.4, 5.5, 5.6000000000000005, 5.7, 5.800000000000001, 5.9, 6.0, 6.1000000000000005, 6.2, 6.300000000000001, 6.4, 6.5, 6.6000000000000005, 6.7, 6.800000000000001, 6.9, 7.0, 7.1000000000000005, 7.2, 7.300000000000001, 7.4, 7.5, 7.6000000000000005, 7.7, 7.800000000000001, 7.9, 8.0, 8.1, 8.200000000000001, 8.3, 8.4, 8.5, 8.6, 8.700000000000001, 8.8, 8.9, 9.0, 9.1, 9.200000000000001, 9.3, 9.4, 9.5, 9.600000000000001, 9.700000000000001, 9.8, 9.9, 10.0]

[asterite]

I think it's fine if we make it work with numbers. These are little things that provide value, even if they are inconsistent with how ranges work in general.

[igor-alexandrov]

I added iteration over Range with negative step: https://github.com/crystal-lang/crystal/commit/3fd4a32382e24c81df7b891a8da5ee1143fc78af.

It is still a WIP.