summaryrefslogtreecommitdiff
diff options
context:
space:
mode:
authorSam Bostock <sam.bostock@shopify.com>2020-07-03 22:56:07 -0400
committerJeremy Evans <code@jeremyevans.net>2020-07-04 10:02:24 -0700
commitbf1a6771f305ea286a3ae575676924551c03e857 (patch)
treeaa12dbd8c25e1072ab5aa9714dee5431b17d8ded
parent9fc564cfef22a2a4eb7af8b2e1df84e4d473cae6 (diff)
Fix non-numeric exclusive Range#minmax bug
The implementation of Range#minmax added in d5c60214c45 causes the following incorrect behaviour: ('a'...'c').minmax => ["a", ["a", "b"]] instead of ('a'...'c').minmax => ["a", "b"] This is because the C implementation of Range#minmax (range_minmax) directly delegates to the C implementation of Range#min (range_min) and Range#max (range_max), without changing the execution context. Range#max's C implementation (range_max), when given a non-numeric exclusive range, delegates to super, which is meant to call Enumerable#max. However, because range_max is called directly by range_minmax, super calls Enumerable#minmax instead, causing the incorrect nesting. Perhaps it is possible to change the execution context in an optimized manner, but the simplest solution seems to be to just explicitly delegate from Range#minmax to Range#min and Range#max.
Notes
Notes: Merged: https://github.com/ruby/ruby/pull/3285
-rw-r--r--range.c5
-rw-r--r--test/ruby/test_range.rb3
2 files changed, 7 insertions, 1 deletions
diff --git a/range.c b/range.c
index 1903b0c1f8c..65e863c0fb9 100644
--- a/range.c
+++ b/range.c
@@ -1266,7 +1266,10 @@ range_minmax(VALUE range)
if (rb_block_given_p()) {
return rb_call_super(0, NULL);
}
- return rb_assoc_new(range_min(0, NULL, range), range_max(0, NULL, range));
+ return rb_assoc_new(
+ rb_funcall(range, rb_intern("min"), 0),
+ rb_funcall(range, rb_intern("max"), 0)
+ );
}
int
diff --git a/test/ruby/test_range.rb b/test/ruby/test_range.rb
index b37dbbc4331..3953b3ecc22 100644
--- a/test/ruby/test_range.rb
+++ b/test/ruby/test_range.rb
@@ -146,6 +146,9 @@ class TestRange < Test::Unit::TestCase
assert_equal([nil, nil], (0...0).minmax)
assert_equal([2, 1], (1..2).minmax{|a, b| b <=> a})
+
+ assert_equal(['a', 'c'], ('a'..'c').minmax)
+ assert_equal(['a', 'b'], ('a'...'c').minmax)
end
def test_initialize_twice