Project

General

Profile

Actions

Misc #17637

open

Endless ranges with `nil` boundary weird behavior

Added by gud (gud gud) 8 months ago. Updated 8 months ago.

Status:
Open
Priority:
Normal
Assignee:
-
[ruby-core:102558]

Description

Basically it's about this https://andycroll.com/ruby/watch-out-for-nils-in-ranges/

Since Ruby 2.6 we have this weird syntax (0..nil) which is really really bug prone

e.g. we have dynamic upper boundary like

lower = 0
upper = some_method(arg1, arg2)

(lower..upper).each do { |s| some_method2(s) }

We rarely do nil checks in Ruby so it's really easy to have Infinity loop in the end.
Previous Argument error was more intuitive since it throws exception instead of silently looping forever.

  • some additional strange behavior:
(0..nil).count
=> Infinity

(0..Float::INFINITY).count
=> hangs, I guess same infinity loop

Having explicit parameter Float::INFINITY (as in previous versions) looks more like a proper design instead of allowing nil as a valid parameter.

You may think of it as I would like to have a range from 0 to nothing, what is it actually ?
And I guess the answer is Nothing.
Fixing (0..Float::INFINITY).count this case it also important I believe.

Tested on ruby 2.7.1p83


Related issues

Related to Ruby master - Bug #14845: Endless Range with nilClosedmatz (Yukihiro Matsumoto)Actions
Actions #1

Updated by mame (Yusuke Endoh) 8 months ago

  • Related to Bug #14845: Endless Range with nil added

Updated by zverok (Victor Shepelev) 8 months ago

I believe that using nil as a signifier of the "open end" is a compromise due to Ruby's ranges polymorphism. You can have range from a string, from time, from date, from any custom comparable class, how you'd signify the "open end" in this case? In some statically typed language it probably could've been some Infinity<Type>, but in Ruby... IDK, maybe the alternative would be some "generic" Infinity constant/special value (as incompatible with any type as nil is, but having different semantics), but it would be too large a change.

Updated by marcandre (Marc-Andre Lafortune) 8 months ago

It's not clear what you are proposing. If it is to restore previous behavior, this won't be acceptable because of compatibility.

Note: you should be using size (lazy), not count (typically exhaustive):

(0..nil).size # => Infinity
(0..Float::INFINITY).size # => Infinity

Updated by mame (Yusuke Endoh) 8 months ago

Hi, I proposed and implemented a endless range.

This is a trade-off between early failure and usability/consistency.
While the feature is indeed error-prone in some cases, it is more consistent and useful.

It is possible to allow only (1..) and deny (1..nil).
In fact, (1..nil) used to raise an exception for a short period of development phase of Ruby 2.6.

https://github.com/ruby/ruby/commit/48de2ea5f9b9067779acb0f7f76e5f879f2b42c0

But, to create a conditionally endless range, we need to write max ? (1..max) : (1..) or Range.new(1, max) if (1..nil) is prohibited.
The current behavior allows to just write (1..max). Thus, it was reverted.

It is very difficult to change the behavior from now because of the compatibility issue.
But as I recall, this is the third time for me to see this issue reported.
(The first is #14845. I couldn't find the second but I think someone said it in GitHub comments or else.)
If it is a major source of bugs, and if a conditionally endless range is very rare, I'm personally open for the change.

Actions #5

Updated by gud (gud gud) 8 months ago

What I would like to have in a programming language is standard library explicitly designed.
So my example from the description with range 0 to Nothing speaks for itself.

0..Float::INFINITY is pretty intuitive but 0..nil looks more like a bug (I understand this is personal view)

But, to create a conditionally endless range, we need to write max ? (1..max) : (1..) or Range.new(1, max) if (1..nil) is prohibited.
The current behavior allows to just write (1..max). Thus, it was reverted.

I see, but on the opposite hand to handle this nil-case you need something like:

lower = 0
upper = some_method(arg1, arg2)

raise ArgumentError unless upper

(lower..upper).each do { |s| some_method2(s) } # or unless upper

which isn't handy also.

This is all about trade-offs and I understand that, but having (1..) syntax only is way less implicit and is a good way to leave this functionality if it's really needed, but make a code base slightly cleaner and more intuitive.

Actions #6

Updated by Dan0042 (Daniel DeLorme) 8 months ago

mame (Yusuke Endoh) wrote in #note-4:

It is possible to allow only (1..) and deny (1..nil).

+1 for that. I was surprised that (1..nil) is allowed. If you have (1..x) and x is nil, it seems more likely to me that it was unexpectedly nil rather than intended as conditionally endless range.
Maybe something more explicit could be allowed, like (1..:endless), then (1..x||:endless) would be possible without ambiguity.

Actions #7

Updated by Eregon (Benoit Daloze) 8 months ago

If we change anything here, it should probably be done with beginless Ranges too for consistency.
And then we'd have 4 cases instead of 1 like in mame (Yusuke Endoh)'s reply.
Also (..) doesn't parse, one needs (nil..).

I think it's not worth breaking compatibility,
especially considering that third-party libraries most likely already rely on Range#{begin/end} == nil => beginless/endless.

How often does one actually get bugs based on this?
I would guess it's pretty rare, most Ranges are on numeric values or Strings, and if you get a nil out of arithmetic, there is definitely something quite fishy in the code.
It seems a good idea to validate upper in the example if it is potentially very high or not numeric, and the same would apply if upper is Float::INFINITY or 1.0/0.0.

Actions #8

Updated by Dan0042 (Daniel DeLorme) 8 months ago

What I was suggesting was to use a symbol when creating an endless range, but the end value would still be nil.

(1..).end         #=>nil
(1..nil).end      #error
(1..:endless).end #=>nil

It seems pretty easy to accidentally create an endless range, like (1..values.max) if values is empty. Until 2.6 this kind of validation was built-in so this would be restoring broken functionality; imho this is a bug fix. Although the fact there has been few bug reports in the last 2 years means this is not likely a large problem.

I'm very surprised that (nil..nil) is even valid.

Actions #9

Updated by gud (gud gud) 8 months ago

Also (..) doesn't parse, one needs (nil..).

Wow, I think it's absolutely OK that (..) doesn't parse. Like is it Ruby or https://uk.wikipedia.org/wiki/Brainfuck
And (nil..) what is that ? Like really if I saw that code 5 years ago without debugging I would say it shouldn't parse as well, my guess would be this is a bug.

My main point is: we already had (0..Float::INFINITY) in previous versions having that we had proper ArgumentError on nils. So I am not sure why we had to expand the std library for new syntax which bring a lot of confusing e.g.:

a = 1
b = 1
a == b => true
(a...b).size => 0

a = 'a'
b = 'a'
a == b => true
(a...b).size => 0

And now "special case"

a = nil
b = nil
a == b => true
(a...b).size => Infinity

And (1..nil) iterates for ever which I can read like nil === Float::INFINITY but actually this returns false and nil.to_i returns 0, but (0..nil.to_i).size => 1

If this is not confusing for newcomers then what is ?

I think it's not worth breaking compatibility,
especially considering that third-party libraries most likely already rely on Range#{begin/end} == nil => beginless/endless.

I am trying to think of a good use case for that stuff and I can't find an answer.

If you need an infinite loop you can always use well known while true or even loop do in Ruby. And those are more common and intuitive way to do such a thing.
You can't iterate over nil..nil because really, what is that ?
Sometimes it's good to revert the stuff even if it's already been using (like pipe operator), still I think the usage of it is not that common.

How often does one actually get bugs based on this?

Not so often but it happens and when it happens it creates a memory leak.

I had a begin/rescue ArugmentError around my code and had a feeling I was OK and then this happened. BOOM.

I also do have a feeling that regular usage of range e.g. (lower..upper) is really common in Ruby code, so making this syntax dangerous for a better usability of conditional endless range which I guess is really rare... feels wrong.

There is a lot of text and please don't get it personal but sometimes I guess this "syntax sugar" stuff drives Ruby in a wrong direction. Aliases/3-4 ways to do the same thing and so on.

Actions

Also available in: Atom PDF