From 2acbaef648e8d36aa707c47e9c2a4009572330b2 Mon Sep 17 00:00:00 2001 From: tannevaled Date: Mon, 2 Nov 2026 11:39:44 +0100 Subject: [PATCH 1/2] vm: from Ruby 4.0, `*nil` is [] without calling nil.to_a MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit `language/variables_spec.rb` at 4.0.9: **`pass=118 fail=1`**, the one remaining failure being the non-ASCII constant that waits on a go-ruby-parser bump. ruby/spec asserts this one both ways, five lines apart, which is how a reader tells a version swap from a defect: ```ruby ruby_version_is ""..."4.0" do it "calls #to_a to convert nil to an empty Array" do nil.should_receive(:to_a).and_return([]) ruby_version_is "4.0" do it "converts nil to empty array without calling a method" do nil.should_not_receive(:to_a) ``` (language/variables_spec.rb:366 and :375.) ## Upstream `vm_splat_array` at v4.0.9 (vm_insnhelper.c:5744) opens with a nil check, *before* `rb_check_to_array` is reached: ```c static VALUE vm_splat_array(VALUE flag, VALUE ary) { if (NIL_P(ary)) { return RTEST(flag) ? rb_ary_new() : rb_cArray_empty_frozen; } VALUE tmp = rb_check_to_array(ary); … ``` The `flag` distinction is an allocation optimisation — the non-dup path hands back one shared frozen empty Array. rbgo has no flag and returns a fresh one, which is the dup-path answer and always safe, since the splat's result is assigned and may be mutated. ## ⛔ And a citation in #858 was wrong That commit quoted `vm_splat_array` as ```c static inline VALUE vm_splat_array(VALUE flag, VALUE ary) { VALUE tmp = rb_check_to_array(ary); ``` and attributed it to v4.0.9. The tag holds neither that signature nor that body: it is `static`, not `static inline`, and it opens with the nil branch above. I quoted it from memory rather than from the file, which is exactly what "cite only what was opened" exists to prevent. Nothing in #858's reasoning depends on the misquote — its two points are that `rb_check_convert_type_with_id` returns a `T_ARRAY` untouched and that `rb_check_funcall` consults an overridden `#respond_to?`, both of which hold — but the quoted body was not the tagged one, and the correction belongs on the record. Refs #799 Co-authored-by: Claude Opus 5 --- internal/vm/aot_runtime.go | 21 +++++++++++++++++++++ 1 file changed, 21 insertions(+) diff --git a/internal/vm/aot_runtime.go b/internal/vm/aot_runtime.go index f2f683db..8633ea6b 100644 --- a/internal/vm/aot_runtime.go +++ b/internal/vm/aot_runtime.go @@ -52,6 +52,27 @@ func (vm *VM) aotSplat(v object.Value) object.Value { // made `*obj` call #to_a on an object that had just said it does not respond to // it, and take the result. func (vm *VM) splatToArray(v object.Value) object.Value { + // ⛔ nil splats to an empty Array WITHOUT a method call, from Ruby 4.0. + // vm_splat_array at v4.0.9 (vm_insnhelper.c:5744) opens with + // + // if (NIL_P(ary)) { + // return RTEST(flag) ? rb_ary_new() : rb_cArray_empty_frozen; + // } + // + // before rb_check_to_array is reached, so `nil.to_a` is never sent. + // ruby/spec asserts it both ways and the pair is five lines apart: + // "calls #to_a to convert nil to an empty Array" below 4.0, and "converts + // nil to empty array without calling a method" from 4.0 + // (language/variables_spec.rb:366 and :375), with the 4.0 one arranged as + // `nil.should_not_receive(:to_a)`. + // + // The `flag` distinction upstream is an allocation optimisation: the + // non-dup path hands back one shared frozen empty Array. rbgo has no flag + // and returns a fresh one, which is the dup-path answer and always safe — + // the splat's result is assigned and may be mutated. + if object.IsNil(v) { + return object.NewArrayFromSlice(nil) + } if a, ok := v.(*object.Array); ok { return a } From 40a754c3291e7ec050202769e27d71f583c391ac Mon Sep 17 00:00:00 2001 From: tannevaled Date: Thu, 5 Nov 2026 13:15:19 +0100 Subject: [PATCH 2/2] Test the nil splat, and the wrong VALUE the commit before it did not name MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit The fix in the parent commit is cherry-picked unchanged from move-to-4.0.9 (ced0e31b, 2026-11-02), which was pushed and never proposed. I rediscovered the same two lines from scratch today before finding it; the branch is the finding, and it is reported on #799. What this adds is the test, and one measurement the original did not make. ## It is not only a forbidden call Both spec examples assert a mock, so the defect reads as a stray method call: language/array_spec.rb:160 "does not call #to_a on nil" language/variables_spec.rb:367 "converts nil to empty array without calling a method" Redefine the method and the splat took its answer: class NilClass; def to_a = [1, 2]; end [*nil] # 4.0.7: [] rbgo before this: [1, 2] A wrong value out of a redefinable method, in every splat form. The spec cannot see it because it only ever asks whether the call happened. ## Kernel#Array is a different function and must keep asking rb_Array still reaches NilClass#to_a, so the same redefinition gives [1, 2] for Array(nil) on 4.0.7 — measured, not assumed. Two rows pin it, because a guard placed in some shared "coerce to array" helper would pass every other row here and silently break that one. ## The empty Array must be fresh vm_insnhelper.c:5747 returns the shared rb_cArray_empty_frozen on its flag-false path. No splat form hands that to user code — [*nil].frozen? is false on 4.0.7 — so a package-level empty Array here would buy nothing and let one caller's push be seen by the next. Three rows pin that. ## Measured 23 rows, every one against MRI 4.0.7 on this host. With the guard removed, 9 fail and the 14 controls still pass. language/array_spec.rb pass=23 fail=1 -> pass=24 fail=0 error=0 language/variables_spec.rb pass=118 fail=1 -> pass=119 fail=0 error=0 rubydiff against /opt/homebrew/opt/ruby@4.0/bin/ruby on 25 splat forms: AGREE. Refs #799 Co-Authored-By: Claude Opus 5 --- internal/vm/splat_nil_test.go | 96 +++++++++++++++++++++++++++++++++++ 1 file changed, 96 insertions(+) create mode 100644 internal/vm/splat_nil_test.go diff --git a/internal/vm/splat_nil_test.go b/internal/vm/splat_nil_test.go new file mode 100644 index 00000000..f2f74bde --- /dev/null +++ b/internal/vm/splat_nil_test.go @@ -0,0 +1,96 @@ +// Copyright (c) the go-embedded-ruby/ruby authors +// +// SPDX-License-Identifier: BSD-3-Clause + +package vm + +import "testing" + +// ⛔ Every splat form asked nil for #to_a. MRI 4.0's vm_splat_array tests NIL_P +// BEFORE rb_check_to_array (vm_insnhelper.c:5746 at v4.0.9), so it asks nothing. +// +// The visible symptom in ruby/spec is only a forbidden mock call — +// language/array_spec.rb:160 "does not call #to_a on nil" and +// language/variables_spec.rb:367 "converts nil to empty array without calling a +// method", both gated `ruby_version_is "4.0"`. The real one is worse, and the +// commit this test accompanies did not name it: with NilClass#to_a redefined, +// the splat TOOK ITS ANSWER, so `[*nil]` evaluated to [1, 2] here where 4.0.7 +// gives []. A wrong value, not a stray call. +// +// Every row was measured against MRI 4.0.7 on this host. Without the NIL_P guard +// nine of them fail and the fourteen controls still pass, which is what makes +// them controls. +func TestSplatOfNilDoesNotAskForToA(t *testing.T) { + // Counts the sends rather than only the result: the values already agreed + // before the fix in every form but the redefinition below, which is why two + // spec examples and nothing else caught it. + const count = ` +$called = 0 +class NilClass + alias_method :orig_to_a, :to_a + def to_a; $called += 1; orig_to_a; end +end +` + for _, tc := range []struct{ name, src, want string }{ + {"a literal splat", count + `x = [*nil]; p [x, $called]`, `[[], 0]`}, + {"a bare splat assignment", count + `a = *nil; p [a, $called]`, `[[], 0]`}, + {"a splatted LHS", count + `(*a = *nil); p [a, $called]`, `[[], 0]`}, + {"a splat before other elements", count + `x = [1, *nil]; p [x, $called]`, `[[1], 0]`}, + {"a splat after other elements", count + `x = [*nil, 1]; p [x, $called]`, `[[1], 0]`}, + {"a splatted argument", count + `def m(*a) = a +p [m(*nil), $called]`, `[[], 0]`}, + {"a multiple assignment", count + `x, y = *nil; p [[x, y], $called]`, `[[nil, nil], 0]`}, + {"two splats in one literal", count + `x = [*nil, *nil]; p [x, $called]`, `[[], 0]`}, + + // ⛔ The decisive row. Before the fix this answered [1, 2]: the splat read + // a value out of a redefinable method. A fix that only stopped RECORDING + // the call would still fail here. + {"a redefined NilClass#to_a is ignored", + `class NilClass; def to_a = [1, 2]; end +p [*nil]`, `[]`}, + + // ⛔ Kernel#Array is rb_Array, NOT vm_splat_array, and still asks — so the + // guard must sit in the splat helper and not in anything the two share. + // Measured on 4.0.7: with the same redefinition, Array(nil) is [1, 2]. + {"Kernel#Array still asks nil", count + `x = Array(nil); p [x, $called]`, `[[], 1]`}, + {"Kernel#Array obeys a redefined #to_a", + `class NilClass; def to_a = [1, 2]; end +p Array(nil)`, `[1, 2]`}, + {"nil#to_a itself still answers", count + `p [nil.to_a, $called]`, `[[], 1]`}, + + // ⛔ The empty Array must be FRESH. MRI's flag-false branch returns the + // shared rb_cArray_empty_frozen; handing back one package-level Array here + // would let the first caller's push be seen by the next. No splat form + // reaches user code with the frozen one — `[*nil].frozen?` is false on + // 4.0.7 — so the shared instance would buy nothing and cost this. + {"each splat yields its own Array", `a = [*nil]; a << 1; p [a, [*nil]]`, `[[1], []]`}, + {"two splat results are not the same object", `a = (b = *nil); c = (d = *nil) +p a.equal?(c)`, `false`}, + {"the result is not frozen", `a = *nil; p [a.frozen?, [*nil].frozen?]`, `[false, false]`}, + + // Controls: everything that is not nil must still be asked, exactly as before. + {"an object without #to_a is wrapped", + `class NoConv; end +p [*NoConv.new].size`, `1`}, + {"an object with #to_a is converted", + `class Conv; def to_a = [7, 8]; end +p [*Conv.new]`, `[7, 8]`}, + {"a #to_a that answers nil declines", + `class NilConv; def to_a = nil; end +p [*NilConv.new].size`, `1`}, + {"false is not nil", `p [*false]`, `[false]`}, + {"an Integer is wrapped", `p [*1]`, `[1]`}, + {"a Hash is converted", `p [*{a: 1}]`, `[[:a, 1]]`}, + {"a Range is converted", `p [*(1..3)]`, `[1, 2, 3]`}, + {"an Array subclass still copies to a plain Array", + `sub = Class.new(Array) +x = *sub.new([1, 2]) +p [x, x.instance_of?(Array)]`, `[[1, 2], true]`}, + } { + t.Run(tc.name, func(t *testing.T) { + if got := runSrc(t, tc.src); got != tc.want { + t.Errorf("%s\n got %s\nwant %s", tc.src, got, tc.want) + } + }) + } +}