Skip to content
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

Negative step size causes infinite loop #8572

Closed
watzon opened this issue Dec 10, 2019 · 14 comments
Closed

Negative step size causes infinite loop #8572

watzon opened this issue Dec 10, 2019 · 14 comments

Comments

@watzon
Copy link
Contributor

watzon commented Dec 10, 2019

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
Copy link
Member

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

@bew
Copy link
Contributor

bew commented Dec 10, 2019

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
Copy link
Contributor

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
Copy link
Contributor Author

watzon commented Feb 23, 2020

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
Copy link
Contributor

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

@asterite
Copy link
Member

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

@straight-shoota
Copy link
Member

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

@vlazar
Copy link
Contributor

vlazar commented Feb 24, 2020

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
Copy link
Contributor

Ok, I will try to cover all cases.

@igor-alexandrov
Copy link
Contributor

igor-alexandrov commented Feb 24, 2020

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
Copy link
Member

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
Copy link
Contributor

I added iteration over Range with negative step: 3fd4a32.

It is still a WIP.

@BlobCodes
Copy link
Contributor

The following code:

p (0..10).step(-1).to_a
p (10..0).step(-1).to_a
p (10..0).step(0).to_a

Gives the following output using Crystal 1.0.0:

[]
[10, 9, 8, 7, 6, 5, 4, 3, 2, 1, 0]
Unhandled exception: Zero step size (ArgumentError)
  from /usr/share/crystal/src/number.cr:201:5 in 'step:to:by:exclusive'
  from /usr/share/crystal/src/range.cr:266:7 in 'step'
  from test4.cr:3:3 in '__crystal_main'
  from /usr/share/crystal/src/crystal/main.cr:110:5 in 'main_user_code'
  from /usr/share/crystal/src/crystal/main.cr:96:7 in 'main'
  from /usr/share/crystal/src/crystal/main.cr:119:3 in 'main'
  from __libc_start_main
  from ../sysdeps/x86_64/start.S:122:0 in '_start'
  from ???

I think this can be closed.

@straight-shoota
Copy link
Member

Yes, zero step size directly errors since #10130.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

No branches or pull requests

7 participants