[Bug #21513] Raise on converting endless range to set #13902
Conversation
range.c
Outdated
| range_to_set(VALUE range) | ||
| { | ||
| if (NIL_P(RANGE_END(range))) { | ||
| rb_raise(rb_eRangeError, "cannot convert endless range to a set"); |
There was a problem hiding this comment.
I believe that endless range error should be the same error type as beginless range error. Calling to_set on a beginless range currently raises TypeError. This method should probably have beginless range raise RangeError instead, and include a test and an example for that.
There was a problem hiding this comment.
Makes sense. Should we also tweak Range#to_a to raise RangeError in both cases?
./ruby -e '(..1).to_a'
-e:1:in 'Range#each': can't iterate from NilClass (TypeError)There was a problem hiding this comment.
Makes sense. Should we also tweak
Range#to_ato raiseRangeErrorin both cases?
@nobu what do you think?
There was a problem hiding this comment.
It feels clearer for me.
@knu Any thoughts?
If we will change it too, maybe ruby/spec also needs to be updated?
There was a problem hiding this comment.
We should perhaps check the size of a given Enumerable in Set.new and raise an error when it is infinity.
There was a problem hiding this comment.
We should perhaps check the size of a given Enumerable in Set.new and raise an error when it is infinity.
Seems good to me. I'll try to follow this path
There was a problem hiding this comment.
I tried out this idea and it seems to work fine (we now raise ArgumentError error for any endless enumerable, e.g. 1.upto(Float::INFINITY).
$ ./ruby -e 'Set.new((1..nil))'
-e:1:in 'Set#initialize': cannot initialize Set from a enumerable with infinite size (ArgumentError)
from -e:1:in '<main>'
$ ./ruby -e '(1..nil).to_set'
-e:1:in 'Enumerable#to_set': cannot initialize Set from a enumerable with infinite size (ArgumentError)
from -e:1:in '<main>'
viralpraxis
force-pushed
the
raise-on-endless-range-to-set
branch
2 times, most recently
from
4b9f122 to
31a3861
Compare
|
There's a SEGV in the ZJIT test suite: https://github.com/ruby/ruby/actions/runs/16305765996/job/46051383888?pr=13902 Not sure if it's related to my changes |
viralpraxis
force-pushed
the
raise-on-endless-range-to-set
branch
from
31a3861 to
a5a7db6
Compare
ZJIT error seems entirely unrelated to your change from a quick glance |
|
Looks like a weird GC issue related to marking though so paging in @k0kubun for good measure |
viralpraxis
force-pushed
the
raise-on-endless-range-to-set
branch
from
a5a7db6 to
e1b6f53
Compare
viralpraxis
changed the title
viralpraxis
force-pushed
the
raise-on-endless-range-to-set
branch
from
e1b6f53 to
ca35402
Compare
This comment has been minimized.
This comment has been minimized.
|
I agree that it's probably not related to the changes. We're looking into GC issues with ZJIT. |
viralpraxis
force-pushed
the
raise-on-endless-range-to-set
branch
4 times, most recently
from
4fcebf3 to
76fe90b
Compare
ref: https://bugs.ruby-lang.org/issues/21513 Before this patch, trying to convert endless range (e.g. `(1..)`) to set (using `to_set`) would hang
viralpraxis
force-pushed
the
raise-on-endless-range-to-set
branch
from
76fe90b to
7753557
Compare
Before this patch, trying to convert endless range (e.g.
(1..)or any other inifinte enumerable) to set (usingto_set) would hang