Bug #17034: Unexpected behavior in #max for beginless range

• Status: Closed
• Assignee: -
• Target version: -
• ruby -v: 2.7.1p83 (2020-03-31 revision a0c7c23c9c) [x86_64-darwin19]
• Backport: 2.5: DONTNEED, 2.6: DONTNEED, 2.7: REQUIRED

[citizen428 (Michael Kohl)]

When calling max on a beginless range, a non-intuitive error gets raised:

r = ..9
r.max
# ArgumentError: comparison of NilClass with 9 failed

There's a check for NIL_P(RANGE_BEG(range)) but it's inside another check which is false for the example case above:

if (rb_block_given_p() || (EXCL(range) && !nm) || argc) {
        if (NIL_P(RANGE_BEG(range))) {
            rb_raise(rb_eRangeError, "cannot get the maximum of beginless range with custom comparison method");
        }
        return rb_call_super(argc, argv);
    }

The first part of the condition is false since there is no block, and even though I'm not sure what EXCL does the second part of the condition will be false due to !nm (nm will be true because of FIXNUM_P(e)). So I think the error gets raised here:

int c = OPTIMIZED_CMP(b, e, cmp_opt);

I think this is not ideal. Possible solutions:

1. Return e (RANGE_END(range) for beginless ranges or
2. return a RangeError with a message like "cannot get the maximum of beginless range" (similar to .min).

Happy to provide a patch if people want this changed and can agree on what the new behavior should be.

[citizen428 (Michael Kohl)]

• File range-max-beginless.patch range-max-beginless.patch added

citizen428 (Michael Kohl) wrote:

I think this is not ideal. Possible solutions:

1. Return e (RANGE_END(range) for beginless ranges or
2. return a RangeError with a message like "cannot get the maximum of beginless range" (similar to .min).

I had some time and looked into this. For inclusive integer ranges r.end will now be returned, no other behavior changes. Patch attached, but since the diff is very small I also opened a PR:

https://github.com/ruby/ruby/pull/3328/

[jeremyevans0 (Jeremy Evans)]

citizen428 (Michael Kohl) wrote:

I think this is not ideal. Possible solutions:

1. Return e (RANGE_END(range) for beginless ranges or
2. return a RangeError with a message like "cannot get the maximum of beginless range" (similar to .min).

Happy to provide a patch if people want this changed and can agree on what the new behavior should be.

I think a RangeError is more correct. Without having a beginning, you cannot know the increment value, and therefore cannot know the maximum value. People that want the end of the range should use Range#end, not Range#max. However, I can see where Range#max returning the end for an inclusive beginless range could potentially be more useful.

[citizen428 (Michael Kohl)]

jeremyevans0 (Jeremy Evans) wrote in #note-2:

I think a RangeError is more correct. Without having a beginning, you cannot know the increment value, and therefore cannot know the maximum value. People that want the end of the range should use Range#end, not Range#max. However, I can see where Range#max returning the end for an inclusive beginless range could potentially be more useful.

The current PR only returns the last value if it's an inclusive beginless range of integers:

(..2).max
#=> 2

Exclusive range ((...2).max): TypeError (cannot exclude end value with non Integer begin value)
Non-integer range ((...2.0).max): TypeError (cannot exclude non Integer end value)
Non-numeric range ((...'c').max): TypeError (cannot get the maximum of beginless range with custom comparison method)

That said I'm happy to change it to a RangeError if that's the overall consensus.

[nobu (Nobuyoshi Nakada)]

• Backport changed from 2.5: UNKNOWN, 2.6: UNKNOWN, 2.7: UNKNOWN to 2.5: DONTNEED, 2.6: DONTNEED, 2.7: REQUIRED

I agree that RangeError would be better, but it seems a separate issue as TypeError is used already in other cases.

• Status changed from Open to Closed

Applied in changeset git|8a5ad2b77d7a24e4f8f4fef179ae5efced935f91.

---

Fix Range#max for beginless Integer ranges [Bug #17034]

• Fix Range#max for beginless Integer ranges
• Update test/ruby/test_range.rb
• Fix formatting

https://github.com/ruby/ruby/pull/3328

Co-authored-by: Nobuyoshi Nakada [e-mail]