…ields
core/set/divide_spec.rb: pass=8 fail=1 -> pass=9 fail=0 error=0.
ruby/spec asserts this one both ways five lines apart, which is how a reader
tells a version swap from a defect:
ruby_version_is "4.0" ret.sort.should == [[1, 2], [2, 1]]
ruby_version_is ""..."4.0" ret.sort.should == [[1, 1], [1, 2], [2, 1], [2, 2]]
(core/set/divide_spec.rb:35 and :43.)
## Upstream
set_divide_arity2 (set.c:923 at v4.0.9) is UNION-FIND over each unordered pair
once:
for (long i = 0; i < size - 1; i++) {
VALUE item1 = RARRAY_AREF(items, i);
for (long j = i + 1; j < size; j++) {
VALUE item2 = RARRAY_AREF(items, j);
if (RTEST(rb_yield_values(2, item1, item2)) &&
RTEST(rb_yield_values(2, item2, item1))) {
set_divide_union_find_merge(uf_parents, i, j, tmp_array);
}
}
}
rbgo ran Tarjan's strongly-connected components over the full n x n relation --
what MRI's lib/set.rb did up to 3.x.
## ⛔ These are different FUNCTIONS, not two spellings of one
The spec example only checks which pairs are yielded. The result changed too,
and nothing in ruby/spec covers it. A directed cycle 1->2->3->1 with no reverse
edge is ONE strongly-connected component and has no mutually-related pair at
all:
f = ->(x, y) { [[1, 2], [2, 3], [3, 1]].include?([x, y]) }
Set[1, 2, 3].divide(&f)
# SCC (before): [[1, 2, 3]]
# MRI 4.0.7 (after): [[1], [2], [3]]
That case is why the whole algorithm is replaced rather than the loop bounds
adjusted.
## ⛔ The && short-circuits, and the spec cannot see it
set.c:940 is `RTEST(yield(a,b)) && RTEST(yield(b,a))`, so a falsy first answer
means the reversed pair is never yielded. The spec's own example cannot detect
this, because its block body is `ret << [x, y]` -- which returns the Array and
is always truthy. Measured on 4.0.7 with a block that answers false: 3 yields
for a 3-element Set, where rbgo made 9.
## ⛔ A one-element Set yields nothing
No pairs exist, so the block is never called. rbgo called it once, with the
element against itself.
## The outer Set is a plain Set on BOTH paths
set_i_divide builds it with set_alloc_with_size(rb_cSet, ...) (set.c:1002) and
set_divide_arity2 with set_s_create(0, 0, rb_cSet) (set.c:946). The arity-1
path here said self.class, so a Set subclass got a subclass back. Only the
SUBSETS take the receiver's class.
## Measured
rubydiff against /opt/homebrew/opt/ruby@4.0/bin/ruby, 19 shapes plus 5
algorithm discriminators: DIVERGE on 4 before, AGREE after.
20 rows in internal/vm/set_divide_test.go, each measured on 4.0.7. Nine of them
cover behaviour no ruby/spec example asserts.
## ⛔ Deferred: the full specdelta sweep
1-minute load average was 70.52 on this host when the change was ready.
specdelta has a per-file timeout, so a sweep under that load manufactures
regressions that are not there. It will be run and reported on this PR when the
machine is quiet; core/set ran clean in the meantime (divide 9/9, classify 4/4,
subtract 2/2).
## Not fixed here, and filed instead
Set cannot be subclassed at all in rbgo: set.go:144 and :151 ignore the
receiver and always build a plain Set, so `Class.new(Set)[1,2].class` is Set.
That is why the subsets' class cannot be asserted yet. It is a wider change
than this one and belongs on its own.
Refs #799
Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
Closes
core/set/divide_spec.rbfrom#799:pass=8 fail=1→pass=9 fail=0 error=0.ruby/spec asserts this one both ways, five lines apart — which is how a reader tells a version swap from a defect:
(
core/set/divide_spec.rb:35and:43.)Upstream
set_divide_arity2is union-find over each unordered pair once:rbgo ran Tarjan's strongly-connected components over the full n×n relation — what MRI's
lib/set.rbdid up to 3.x.⛔ These are different functions, not two spellings of one
The spec example checks only which pairs are yielded. The result changed too, and no ruby/spec example covers it. A directed cycle
1→2→3→1with no reverse edge is one strongly-connected component, and has no mutually-related pair at all:That case is why the algorithm is replaced rather than the loop bounds adjusted.
⛔ The
&&short-circuits, and the spec cannot see itset.c:940isRTEST(yield(a,b)) && RTEST(yield(b,a)), so a falsy first answer means the reversed pair is never yielded. The spec's own example cannot detect this — its block body isret << [x, y], which returns the Array and is always truthy. Measured on 4.0.7 with a block answeringfalse: 3 yields for a 3-element Set, where rbgo made 9.⛔ A one-element Set yields nothing
No pairs exist, so the block is never called. rbgo called it once, with the element against itself.
The outer Set is a plain
Seton both pathsset_i_dividebuilds it withset_alloc_with_size(rb_cSet, …)(set.c:1002) andset_divide_arity2withset_s_create(0, 0, rb_cSet)(set.c:946). The arity-1 path here saidself.class. Only the subsets take the receiver's class.Measured
rubydiffagainst/opt/homebrew/opt/ruby@4.0/bin/rubyover 19 shapes plus 5 algorithm discriminators: DIVERGE on 4 before, AGREE after.20 rows in
internal/vm/set_divide_test.go, each measured on 4.0.7. Nine of them cover behaviour no ruby/spec example asserts.⛔ Deferred: the full
specdeltasweepThe 1-minute load average was 70.52 on this host when the change was ready.
specdeltahas a per-file timeout, so a sweep under that load manufactures regressions that are not there. I will run it and report it on this PR when the machine is quiet.core/setran clean in the meantime —divide9/9,classify4/4,subtract2/2.Not fixed here, filed instead
Setcannot be subclassed at all in rbgo:set.go:144and:151ignore the receiver and always build a plainSet, soClass.new(Set)[1,2].classisSet. That is why the subsets' class is not asserted yet. It is a wider change and belongs on its own.🤖 Generated with Claude Code