You signed in with another tab or window. Reload to refresh your session.You signed out in another tab or window. Reload to refresh your session.You switched accounts on another tab or window. Reload to refresh your session.Dismiss alert
{{ message }}
Repository navigation
Range#max(n) ignores the block's ordering, and the one spec for it cannot tell #891
Found while porting range_max for beginless ranges (#799's five core/range/max_spec.rb examples); pre-existing and unrelated, so it was
not folded into that change.
Measured against MRI 4.0.7 on darwin/arm64
(1..10).max(2) { |a, b| -(a <=> b) }
rbgo => [10, 9]
MRI => [1, 2]
(1..10).max(2) { |a, b| a <=> b } # agree by coincidence
rbgo => [10, 9]
MRI => [10, 9]
The first case is the whole bug: the block reverses the ordering, so the two maxima under that comparison are the two smallest elements. rbgo answers the
two largest, because it never consults the block.
Why
range_max dispatches on the block before the argument:
if (rb_block_given_p() || (EXCL(range) && !nm)) {
...
returnrb_call_super(argc, argv); /* Enumerable#max(n) WITH the block */
}
elseif (argc) { ... } /* the arithmetic path */
rbgo's native checks len(args) > 0 first and takes the arithmetic path, which
has no way to use a block. The block is silently dropped.
Why nothing caught it
core/range/max_spec.rb has exactly one example for this shape:
describe"Range#max given a block and an integer argument"doit"returns n elements the block determines to be the maximum"do(1..10).max(2){|a,b| a <=> b}.should == [10,9]endend
Its block is the IDENTITY ordering, so the arithmetic answer and the block
answer are the same array. The example passes against an implementation that
ignores the block entirely — it cannot distinguish them. A second example with
a reversed comparison would fail today.
What would close it
Enumerable#max(n) with a block, reached when a block is given, with the
guard order matching range_max — the beginless RangeError already moved to
the top of the method in the port, so only the dispatch to the block path is
missing. Worth adding the reversed-comparison example to ruby/spec upstream at
the same time, since the gap is in the corpus and not only in rbgo.
Activity