From b2c68fec4fa82c54f5dfe958fd4ebc2e4e73b322 Mon Sep 17 00:00:00 2001 From: tannevaled Date: Sat, 26 Sep 2026 08:45:46 +0200 Subject: [PATCH 1/4] Singleton classes of nil/true/false, and a metaclass chain that reaches Class MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit MRI read: class.c v3_4_0 — special_singleton_class_of (:2199), rb_special_singleton_class (:2210), singleton_class_of (:2224-2260), rb_singleton_class (:2299) and make_metaclass (:766-792). Two defects, one file apart, both measured through the repaired ruby/spec shim against MRI 4.0.5 run through the same shim. 1. `nil.singleton_class` raised TypeError. singleton_class_of has TWO arms for immediates. The first raises "can't define singleton" — T_FIXNUM, T_BIGNUM, T_FLOAT, T_SYMBOL (class.c:2228). The second, just below it, answers special_singleton_class_of for T_FALSE / T_TRUE / T_NIL (class.c:2236): nil, true and false have no room for a per-object singleton, so Ruby hands back their CLASS — NilClass, TrueClass, FalseClass. rbgo drew no such line and raised for all seven. specialSingletonClass is that function; ensureSingleton consults it first, so every door into a singleton class opens the same one: #singleton_class, `class << nil`, #define_singleton_method, #extend and #instance_eval. A method defined "on" nil is therefore an ordinary instance method of NilClass, reached by every nil in the program, which is what MRI does. Note the two classes are NOT singleton classes: #singleton_class? is false and #superclass is Object. A fabricated singleton would have answered both the other way. 2. `BasicObject.singleton_class.superclass` was nil. make_metaclass ends the metaclass chain at Class: RCLASS_SET_SUPER(metaclass, super ? ENSURE_EIGENCLASS(super) : rb_cClass) (class.c:786) BasicObject is the only class with no superclass, so one missing link left every singleton class of a class cut off from Class: #ancestors did not include it, its instance methods were not #method_defined?, and its class methods drew no #respond_to?. closeMetaclassChain wires it once at boot, before any metaclass is asked for, since RClass.metaClass cannot reach cClass from where it runs. Measured, per file, rbgo before -> after against MRI 4.0.5 through the same shim: language/singleton_class_spec.rb 35 -> 47 (MRI 53) language/metaclass_spec.rb 18 -> 21 (MRI 24) language/variables_spec.rb 113 -> 114 (MRI 118) language/ total 2314 -> 2330 (MRI 2627) No file regressed. TestSingletonClassErrors pinned `class << nil` as a TypeError — it pinned the divergence. Updated to the MRI-verified set (Fixnum, Symbol, Float, Bignum: the raising arm, all four now covered) with the nil/true/false side moved to TestSpecialSingletonClass. Left, all of them behind builtins.go, which another agent owns this wave: #is_a? resolves through classOf rather than the singleton class, so `o.is_a?(o.singleton_class)` and the two "subclass of Class's singleton class" examples still fail; Class#new / #allocate do not refuse a singleton class (and #new panics on one, recovered as an ArgumentError); and a frozen object's singleton class is not frozen. Six examples in singleton_class_spec. Co-Authored-By: Claude Opus 5 --- internal/vm/object_model.go | 48 ++++++++++++- internal/vm/singleton.go | 22 ++++++ internal/vm/singleton_test.go | 12 +++- internal/vm/wave32_singleton_test.go | 101 +++++++++++++++++++++++++++ 4 files changed, 179 insertions(+), 4 deletions(-) create mode 100644 internal/vm/wave32_singleton_test.go diff --git a/internal/vm/object_model.go b/internal/vm/object_model.go index c45966d6..72bd675a 100644 --- a/internal/vm/object_model.go +++ b/internal/vm/object_model.go @@ -414,10 +414,40 @@ func (vm *VM) objSingleton(v object.Value) *RClass { return vm.extSingletons[v] } +// specialSingletonClass is MRI's special_singleton_class_of (class.c v3_4_0:2199): +// nil, true and false are immediates with no room for a per-object singleton, so +// Ruby answers their CLASS instead — NilClass, TrueClass, FalseClass. It is not a +// stand-in but the real thing: singleton_class_of returns it directly for T_NIL / +// T_TRUE / T_FALSE (class.c v3_4_0:2237-2242), which is why `def nil.foo` and +// `class << nil` define an instance method on NilClass, `nil.singleton_class` +// equal?s NilClass, and `nil.extend(M)` includes M into NilClass. +// +// The three classes are ordinary (non-singleton) classes, so #singleton_class? +// is false for them and their #superclass is Object — both of which MRI reports +// and neither of which a fabricated singleton class would. +// +// nil is returned for every other value, including Integer/Float/Symbol, which +// raise TypeError ("can't define singleton") rather than answering a class. +func (vm *VM) specialSingletonClass(v object.Value) *RClass { + switch t := v.(type) { + case object.Nil: + return vm.cNilClass + case object.Bool: + if bool(t) { + return vm.cTrueClass + } + return vm.cFalseClass + } + return nil +} + // ensureSingleton returns v's singleton class, creating it on first use. It // handles *RObject (inline field) and other reference values (side table). A -// second bool reports success; immediate values (Integer/Symbol/true/false/nil) -// and classes/modules (which use a metaclass instead) are not eligible here. +// second bool reports success; immediate values (Integer/Symbol) and +// classes/modules (which use a metaclass instead) are not eligible here. +// nil/true/false answer their special singleton class (see +// specialSingletonClass), so a method defined "on" them lands on +// NilClass/TrueClass/FalseClass exactly as in MRI. func (vm *VM) ensureSingleton(v object.Value) (*RClass, bool) { switch t := v.(type) { case *RObject: @@ -425,6 +455,9 @@ func (vm *VM) ensureSingleton(v object.Value) (*RClass, bool) { case *RClass: return nil, false // classes use metaClass() } + if sc := vm.specialSingletonClass(v); sc != nil { + return sc, true + } if !hasIdentitySingleton(v) { return nil, false } @@ -465,6 +498,17 @@ func (c *RClass) metaClass() *RClass { // The metaclass superclass is the superclass's metaclass, so a class-method // `super` (def self.foo / class << self) walks to the inherited class method: // # -> # -> ... This mirrors MRI's metaclass chain. + // + // The chain has to END somewhere, and MRI ends it at Class: + // + // RCLASS_SET_SUPER(metaclass, super ? ENSURE_EIGENCLASS(super) : rb_cClass) + // (make_metaclass, class.c v3_4_0:786) + // + // The only class with no superclass is BasicObject, so that last link is + // wired once at boot by closeMetaclassChain rather than here, where cClass + // is not reachable. Without it `#.superclass` is nil, + // every singleton class of a class fails `.ancestors.include?(Class)`, and + // the instance and class methods of Class are invisible from one. if c.super != nil { mc.super = c.super.metaClass() } diff --git a/internal/vm/singleton.go b/internal/vm/singleton.go index 5078ed53..5aae55b2 100644 --- a/internal/vm/singleton.go +++ b/internal/vm/singleton.go @@ -2,11 +2,33 @@ package vm import "github.com/go-embedded-ruby/ruby/internal/object" +// closeMetaclassChain wires the LAST link of the metaclass chain, which +// RClass.metaClass cannot reach on its own: the metaclass of a class with no +// superclass takes Class as its superclass — +// +// RCLASS_SET_SUPER(metaclass, super ? ENSURE_EIGENCLASS(super) : rb_cClass) +// (make_metaclass, class.c v3_4_0:786) +// +// BasicObject is the only such class, so one link closes the whole chain: +// # -> # -> # -> Class -> Module -> +// Object -> BasicObject. Every singleton class of a class then answers +// `.ancestors.include?(Class)`, inherits Class's instance methods, and responds +// to Class's class methods, none of which it could before. +// +// It runs once at boot, before any metaclass has been asked for, so the eager +// `mc.super = c.super.metaClass()` recursion inside metaClass finds the link +// already in place rather than building a chain that stops at nil. +func (vm *VM) closeMetaclassChain() { + vm.cBasicObject.metaClass().super = vm.cClass +} + // registerSingleton installs the per-object singleton-method API: // define_singleton_method and extend. Both work on ordinary objects (via a // lazily-created singleton class) and on classes/modules (via their class // methods). The `extended` hook fires when a module is mixed into an object. func (vm *VM) registerSingleton() { + vm.closeMetaclassChain() + vm.cObject.define("define_singleton_method", func(vm *VM, self object.Value, args []object.Value, blk *Proc) object.Value { body := blk if body == nil { diff --git a/internal/vm/singleton_test.go b/internal/vm/singleton_test.go index f3a95076..f92ba797 100644 --- a/internal/vm/singleton_test.go +++ b/internal/vm/singleton_test.go @@ -99,12 +99,20 @@ func TestSingletonClassOfExpression(t *testing.T) { } // TestSingletonClassErrors covers `class << target` for a target with no -// singleton class (an immediate value), which MRI rejects with a TypeError. +// singleton class, which MRI rejects with a TypeError. +// +// The set is exactly singleton_class_of's `rb_raise(rb_eTypeError, "can't +// define singleton")` arm — T_FIXNUM, T_BIGNUM, T_FLOAT and T_SYMBOL (class.c +// v3_4_0:2228-2234). nil/true/false are NOT in it: they fall to the arm just +// below, which answers special_singleton_class_of (class.c v3_4_0:2236-2242), +// so `class << nil` opens NilClass. TestSpecialSingletonClass pins that side; +// the `class << nil` case that used to sit here pinned the divergence. func TestSingletonClassErrors(t *testing.T) { for _, c := range []struct{ src, want string }{ {"x = 5\nclass << x; def z; 1; end; end", "can't define singleton"}, {"class << :sym; def z; 1; end; end", "can't define singleton"}, - {"class << nil; def z; 1; end; end", "can't define singleton"}, + {"x = 1.5\nclass << x; def z; 1; end; end", "can't define singleton"}, + {"x = 10**40\nclass << x; def z; 1; end; end", "can't define singleton"}, } { if err := runErr(t, c.src); err == nil || !strings.Contains(err.Error(), c.want) { t.Errorf("src=%q got=%v want %q", c.src, err, c.want) diff --git a/internal/vm/wave32_singleton_test.go b/internal/vm/wave32_singleton_test.go new file mode 100644 index 00000000..79486bf9 --- /dev/null +++ b/internal/vm/wave32_singleton_test.go @@ -0,0 +1,101 @@ +// Copyright (c) the go-embedded-ruby/ruby authors +// +// SPDX-License-Identifier: BSD-3-Clause + +package vm + +import "testing" + +// TestSpecialSingletonClass pins MRI's special_singleton_class_of (class.c +// v3_4_0:2199): nil, true and false are immediates with no room for a per-object +// singleton class, so Ruby answers their CLASS instead. singleton_class_of +// returns it directly for T_NIL / T_TRUE / T_FALSE (class.c v3_4_0:2236-2242), +// one arm BELOW the "can't define singleton" raise that covers Integer, Bignum, +// Float and Symbol — which is the distinction rbgo did not draw: it raised for +// all seven. +// +// The consequence is not only that #singleton_class answers: a method defined +// "on" nil is an ordinary instance method of NilClass, reached by every nil in +// the program, and `nil.extend(M)` includes M into NilClass. +func TestSpecialSingletonClass(t *testing.T) { + for _, c := range []struct{ src, want string }{ + // The class itself, not a fabricated singleton wrapping it. + {`p nil.singleton_class`, "NilClass\n"}, + {`p true.singleton_class`, "TrueClass\n"}, + {`p false.singleton_class`, "FalseClass\n"}, + {`p nil.singleton_class.equal?(NilClass)`, "true\n"}, + {`p true.singleton_class.equal?(TrueClass)`, "true\n"}, + {`p false.singleton_class.equal?(FalseClass)`, "true\n"}, + + // It is an ORDINARY class: #singleton_class? is false and #superclass is + // Object, neither of which a fabricated singleton class would report. + {`p nil.singleton_class.singleton_class?`, "false\n"}, + {`p nil.singleton_class.superclass`, "Object\n"}, + {`p true.singleton_class.superclass`, "Object\n"}, + + // `class << nil` opens NilClass, so its defs and constants land there. + {`class << nil; def z; 42; end; end; p nil.z`, "42\n"}, + {`p(class << true; self; end)`, "TrueClass\n"}, + {`class << false; K = 7; end; p FalseClass::K`, "7\n"}, + + // define_singleton_method and extend route through the same class. + {`nil.define_singleton_method(:dsm) { :ok }; p nil.dsm`, ":ok\n"}, + {`p nil.extend(Module.new { def em; :em; end }).em`, ":em\n"}, + + // The four that DO raise stay raising (the arm above). + {`begin; 1.singleton_class; rescue => e; p [e.class, e.message]; end`, + "[TypeError, \"can't define singleton\"]\n"}, + {`begin; :a.singleton_class; rescue => e; p [e.class, e.message]; end`, + "[TypeError, \"can't define singleton\"]\n"}, + {`begin; 1.5.singleton_class; rescue => e; p [e.class, e.message]; end`, + "[TypeError, \"can't define singleton\"]\n"}, + {`begin; (10**40).singleton_class; rescue => e; p [e.class, e.message]; end`, + "[TypeError, \"can't define singleton\"]\n"}, + } { + if got := eval(t, c.src); got != c.want { + t.Errorf("src=%q\n got=%q\nwant=%q", c.src, got, c.want) + } + } +} + +// TestMetaclassChainEndsAtClass pins make_metaclass's last link: the metaclass +// of a class with no superclass takes Class as its superclass — +// +// RCLASS_SET_SUPER(metaclass, super ? ENSURE_EIGENCLASS(super) : rb_cClass) +// (class.c v3_4_0:786) +// +// BasicObject is the only such class, so the one link closes the chain for every +// class in the program. Without it `#.superclass` was nil and +// a singleton class of a class could see nothing of Class: not its ancestors, +// not its instance methods, not its class methods. +func TestMetaclassChainEndsAtClass(t *testing.T) { + for _, c := range []struct{ src, want string }{ + {`p BasicObject.singleton_class.superclass`, "Class\n"}, + {`p Object.singleton_class.superclass`, "#\n"}, + {`class A; end; p A.singleton_class.superclass`, "#\n"}, + + // The chain now reaches Class, so a class's singleton class inherits it. + {`class A; end; p A.singleton_class.ancestors.include?(Class)`, "true\n"}, + {`class A; end; p A.singleton_class.ancestors`, + "[#, #, #, Class, Module, Object, Kernel, BasicObject]\n"}, + + // Instance methods of Class are visible through it (the shape + // language/singleton_class_spec.rb asserts). + {`class A; end +class Class; def example_im; end; end +p A.singleton_class.method_defined?(:example_im, true)`, "true\n"}, + + // And so are Class's own class methods. + {`class A; end +class Class; def self.example_cm; end; end +p A.singleton_class.respond_to?(:example_cm)`, "true\n"}, + + // Class.singleton_class is unaffected: Class HAS a superclass (Module), so + // it follows the ordinary rule rather than the root fallback. + {`p Class.singleton_class.superclass`, "#\n"}, + } { + if got := eval(t, c.src); got != c.want { + t.Errorf("src=%q\n got=%q\nwant=%q", c.src, got, c.want) + } + } +} From 744a1420624944c27d13a7dce3ae99a1b7e61d04 Mon Sep 17 00:00:00 2001 From: tannevaled Date: Sat, 26 Sep 2026 08:54:36 +0200 Subject: [PATCH 2/4] Method#arity / #parameters see post parameters, and Proc#arity sees them after a splat MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit MRI read: rb_iseq_min_max_arity (proc.c v3_4_0:1073-1080), rb_iseq_parameters (iseq.c v3_4_0:3578-3660) and setup_parameters_complex / args_setup_post_parameters (vm_args.c v3_4_0:595, 878-892). param.post_num is the count of positional parameters that bind from the TAIL, and it is independent of has_rest: `def m(a, *r, c)` has post_num 1 and so does `def m(a=1, b)`. Neither rb_iseq_min_max_arity's `lead_num + post_num` nor rb_iseq_parameters' `[post_start, post_start+post_num) -> :req` consults the splat. rbgo had each half wrong in a DIFFERENT place, in opposite directions, which is why #650 — which fixed Proc#arity for the no-splat shape — could not have found either: * the METHOD half (iseqRequiredPositional, iseqHasOptional, buildParamsList) knew only the after-splat post run. `def m(a=1, b)` answered arity -1 for MRI's -2 and parameters [[:opt, :a], [:opt, :b]] for MRI's [[:opt, :a], [:req, :b]] — the post parameter reported as optional. * the PROC half (Proc#arityVal) read is.PostCount directly, which is 0 whenever there IS a splat. `proc { |a, *b, c| }.arity` answered -2 for MRI's -3, and so did the lambda. iseqPostCount already reported both shapes; every reader now goes through it, and iseqPostStart names the index rb_iseq_parameters walks from. One rule, one place: the binder in exec was already right, and reads the same pair. That last part cost a defect on the way in. iseqRequiredPositional is now lead+post, so exec's `minReq := iseqRequiredPositional(iseq) + iseq.PostCount` counted the no-splat post run twice and rejected `m(9)` for `def m(a=1, b)` with "given 1, expected 2". TestPostParamsBindFromTheTail is the control that caught it — the reflective counts are worth nothing unless the binder agrees with them, so it calls the methods rather than only asking their arity. Verified byte-for-byte against MRI 4.0.5 across the arity/parameters matrix (def, define_method, lambda, non-lambda proc, Method#to_proc, UnboundMethod, splat and no-splat, keywords and block). language/ total unchanged at 2330: core/method/arity_spec.rb and core/proc/arity_spec.rb both stop on a shared- example the shim does not implement — under MRI too — so the win here is behavioural and reflective rather than one the ratchet can show. Co-Authored-By: Claude Opus 5 --- internal/vm/method.go | 34 ++++++---- internal/vm/method_reflect2.go | 17 +++-- internal/vm/object_model.go | 25 +++++--- internal/vm/vm.go | 6 +- internal/vm/wave32_arity_test.go | 103 +++++++++++++++++++++++++++++++ 5 files changed, 158 insertions(+), 27 deletions(-) create mode 100644 internal/vm/wave32_arity_test.go diff --git a/internal/vm/method.go b/internal/vm/method.go index 4935e3b5..00a3ee13 100644 --- a/internal/vm/method.go +++ b/internal/vm/method.go @@ -165,26 +165,36 @@ func methodISeq(m *Method) *bytecode.ISeq { } } -// iseqRequiredPositional counts an ISeq's required positional parameters: the -// NumRequired leading ones plus any required parameters after a *splat (the -// "post" params, which occupy the Params slots following SplatIndex). +// iseqRequiredPositional counts an ISeq's required positional parameters: +// MRI's lead_num + post_num (rb_iseq_min_max_arity, proc.c v3_4_0:1073-1079). +// The post ones are those after a *splat AND — when there is none — the +// trailing required run of `def m(a=1, b)`, which iseqPostCount reports in +// both shapes. func iseqRequiredPositional(is *bytecode.ISeq) int { - req := is.NumRequired - if is.SplatIndex >= 0 { - req += len(is.Params) - is.SplatIndex - 1 - } - return req + return is.NumRequired + iseqPostCount(is) +} + +// iseqPostStart is the index of the first post parameter in Params: the slot +// after a *splat, or — with no splat — the start of the trailing required run. +// It is len(Params) when there are none, so `i >= iseqPostStart(is)` is false +// for every parameter. MRI keeps it as param.post_start (vm_core.h) and +// rb_iseq_parameters walks [post_start, post_start+post_num) as :req +// (iseq.c v3_4_0:3629-3641). +func iseqPostStart(is *bytecode.ISeq) int { + return len(is.Params) - iseqPostCount(is) } // iseqHasOptional reports whether an ISeq has at least one optional positional -// parameter: with a splat, the slots between NumRequired and SplatIndex; without -// one, everything after NumRequired (post-without-splat is treated as optional, -// which the ISeq encoding does not distinguish). +// parameter: MRI's opt_num > 0. With a splat those are the slots between +// NumRequired and SplatIndex; without one they are the slots between +// NumRequired and the start of the post run — NOT everything after NumRequired, +// which counted `def m(a=1, b)`'s required b as an optional and cost the method +// side of #arity a whole argument. func iseqHasOptional(is *bytecode.ISeq) bool { if is.SplatIndex >= 0 { return is.SplatIndex > is.NumRequired } - return len(is.Params) > is.NumRequired + return iseqPostStart(is) > is.NumRequired } // keywordCounts reports how many of an ISeq's keyword parameters are required diff --git a/internal/vm/method_reflect2.go b/internal/vm/method_reflect2.go index 66d4a12c..fc018e7f 100644 --- a/internal/vm/method_reflect2.go +++ b/internal/vm/method_reflect2.go @@ -216,19 +216,28 @@ func methodParameters(m *Method) object.Value { // Method#parameters and Proc#parameters. reqKind selects the symbol used for a // required positional (:req for a method or a lambda, :opt for a non-lambda // proc, which never enforces its positionals). The positional shape — leading -// required, optionals, a *rest, then post-splat required — is read from the -// iseq, followed by the keyword, keyword-rest and block parameters. A nil iseq -// is a native (ISeq-less) callable: MRI reports a single catch-all rest. +// required, optionals, a *rest, then the POST required — is read from the iseq, +// followed by the keyword, keyword-rest and block parameters. A nil iseq is a +// native (ISeq-less) callable: MRI reports a single catch-all rest. +// +// The post run is the one rb_iseq_parameters walks last among the positionals, +// [post_start, post_start+post_num), emitting :req for each (iseq.c +// v3_4_0:3629-3641). It exists WITHOUT a splat too — `def m(a=1, b)` has +// lead_num 0, opt_num 1, post_num 1 — which is what iseqPostStart reports and +// what this loop used to miss: it treated everything after the leading required +// run as optional unless a splat separated it, so `def m(a=1, b)` answered +// [[:opt, :a], [:opt, :b]] where MRI answers [[:opt, :a], [:req, :b]]. func buildParamsList(is *bytecode.ISeq, reqKind string) []object.Value { if is == nil { return []object.Value{object.NewArray(object.Symbol("rest"))} } var out []object.Value + postStart := iseqPostStart(is) for i, name := range is.Params { switch { case i == is.SplatIndex: out = append(out, paramPair("rest", displayAnon(name, fwdRestSentinel, "*"))) - case i < is.NumRequired || (is.SplatIndex >= 0 && i > is.SplatIndex): + case i < is.NumRequired || i >= postStart: out = append(out, positionalParam(reqKind, name)) default: out = append(out, positionalParam("opt", name)) diff --git a/internal/vm/object_model.go b/internal/vm/object_model.go index 72bd675a..5ff85b06 100644 --- a/internal/vm/object_model.go +++ b/internal/vm/object_model.go @@ -2010,11 +2010,16 @@ func (p *Proc) arityVal() int { return p.nativeArity } is := p.iseq - // Required count: the leading required positionals PLUS the trailing post ones - // (rb_iseq_min_max_arity returns lead_num + post_num, proc.c v3_4_0), plus one - // when any keyword is mandatory (a single required-keyword group counts as one - // required arg). - req := is.NumRequired + is.PostCount + // Required count: the leading required positionals PLUS the post ones + // (rb_iseq_min_max_arity returns lead_num + post_num, proc.c v3_4_0:1079), + // plus one when any keyword is mandatory (a single required-keyword group + // counts as one required arg). + // + // param.post_num is the count in BOTH shapes — after a splat and, with no + // splat, the trailing required run — which is what iseqPostCount reports. + // Reading is.PostCount directly saw only the second: PostCount is 0 whenever + // there IS a splat, so `proc { |a, *b, c| }.arity` answered -2 for MRI's -3. + req := is.NumRequired + iseqPostCount(is) hasReqKw, hasOptKw := false, is.KwRestSlot >= 0 for _, r := range is.KwRequired { if r { @@ -2029,13 +2034,13 @@ func (p *Proc) arityVal() int { // A *splat is always variadic. For a lambda, an optional positional, an // optional keyword or a keyword-rest also makes it variadic; a non-lambda proc // stays fixed on those and reports the positive required count. - end := len(is.Params) - if is.SplatIndex >= 0 { - end = is.SplatIndex - } + // + // "Has an optional positional" is opt_num > 0, which iseqHasOptional answers + // for both shapes; counting every slot past NumRequired would count a post + // parameter as an optional. variadic := is.SplatIndex >= 0 if p.isLambda { - variadic = variadic || end > is.NumRequired || hasOptKw + variadic = variadic || iseqHasOptional(is) || hasOptKw } if variadic { return -(req + 1) diff --git a/internal/vm/vm.go b/internal/vm/vm.go index e963aa21..b4c3792c 100644 --- a/internal/vm/vm.go +++ b/internal/vm/vm.go @@ -1364,7 +1364,11 @@ func (vm *VM) exec(iseq *bytecode.ISeq, self object.Value, args []object.Value, // MRI's min_argc is lead_num + post_num whether or not there is a rest // parameter (setup_parameters_complex, vm_args.c v3_4_0:595), so a post // parameter with no splat is required too: `def m(a=1, b)` rejects m(). - minReq := iseqRequiredPositional(iseq) + iseq.PostCount + // + // iseqRequiredPositional IS that sum, in both shapes (it reads iseqPostCount, + // the same npost above). Adding iseq.PostCount to it as well would count the + // no-splat post run TWICE and reject `def m(a=1, b)` called as m(9). + minReq := iseqRequiredPositional(iseq) if len(args) < minReq || (iseq.SplatIndex < 0 && len(args) > len(iseq.Params)) { var expected string switch { diff --git a/internal/vm/wave32_arity_test.go b/internal/vm/wave32_arity_test.go new file mode 100644 index 00000000..d278d398 --- /dev/null +++ b/internal/vm/wave32_arity_test.go @@ -0,0 +1,103 @@ +// Copyright (c) the go-embedded-ruby/ruby authors +// +// SPDX-License-Identifier: BSD-3-Clause + +package vm + +import "testing" + +// TestPostParamsArityAndParameters pins the POST positional parameters — +// MRI's param.post_num — on every callable that reports a parameter shape. +// +// rb_iseq_min_max_arity's minimum is lead_num + post_num (proc.c v3_4_0:1079) +// and rb_iseq_parameters emits :req for [post_start, post_start+post_num) +// (iseq.c v3_4_0:3629-3641). Neither reads has_rest: post parameters exist in +// BOTH shapes — after a *splat, and, with no splat, as the trailing required +// run of `def m(a=1, b)`. +// +// rbgo had each half of that wrong in a different place, so the two answers +// disagreed with MRI in opposite directions and #650, which fixed Proc#arity +// for the no-splat shape, could not have found either: +// +// - the METHOD side (methodArity, buildParamsList) knew only the after-splat +// post run, so `def m(a=1, b)` answered arity -1 for MRI's -2 and +// parameters [[:opt, :a], [:opt, :b]] for MRI's [[:opt, :a], [:req, :b]]; +// - the PROC side (Proc#arityVal) read is.PostCount directly, which is 0 +// whenever there IS a splat, so `proc { |a, *b, c| }` answered -2 for -3. +// +// iseqPostCount already reported both; every reader now goes through it. +func TestPostParamsArityAndParameters(t *testing.T) { + for _, c := range []struct{ src, want string }{ + // No splat, post run of 1. The shape the issue names. + {`def m(a=1, b); end; p method(:m).arity`, "-2\n"}, + {`def m(a=1, b); end; p method(:m).parameters`, "[[:opt, :a], [:req, :b]]\n"}, + {`def m(a=1, b); end; p self.class.instance_method(:m).arity`, "-2\n"}, + {`def m(a=1, b); end; p self.class.instance_method(:m).parameters`, + "[[:opt, :a], [:req, :b]]\n"}, + {`def m(a=1, b); end; p method(:m).to_proc.arity`, "-2\n"}, + + // Leading required, an optional, then a post run. + {`def m(a, b=1, c); end; p method(:m).arity`, "-3\n"}, + {`def m(a, b=1, c); end; p method(:m).parameters`, + "[[:req, :a], [:opt, :b], [:req, :c]]\n"}, + {`def m(a=1, b=2, c, d); end; p method(:m).arity`, "-3\n"}, + {`def m(a=1, b=2, c, d); end; p method(:m).parameters`, + "[[:opt, :a], [:opt, :b], [:req, :c], [:req, :d]]\n"}, + + // A define_method body reports the same shape as a def. + {`c = Class.new { define_method(:dm) { |a=1, b| } } +p [c.instance_method(:dm).arity, c.instance_method(:dm).parameters]`, + "[-2, [[:opt, :a], [:req, :b]]]\n"}, + + // The splat shape, which the PROC half got wrong. + {`p proc { |a, *b, c| }.arity`, "-3\n"}, + {`p lambda { |a, *b, c| }.arity`, "-3\n"}, + {`p lambda { |a=1, b| }.arity`, "-2\n"}, + {`p lambda { |a=1, b| }.parameters`, "[[:opt, :a], [:req, :b]]\n"}, + + // A non-lambda proc enforces no positional, so it reports its post + // parameters as :opt and stays on the POSITIVE required count — the + // is_proc branch of rb_iseq_parameters (iseq.c v3_4_0:3610-3628). + {`p proc { |a=1, b| }.arity`, "1\n"}, + {`p proc { |a=1, b| }.parameters`, "[[:opt, :a], [:opt, :b]]\n"}, + {`p proc { |a, *b, c| }.parameters`, "[[:opt, :a], [:rest, :b], [:opt, :c]]\n"}, + + // Everything together, unchanged: the shapes that were already right. + {`def m(a, b=1, *r, c, d:, **k, &bl); end; p method(:m).arity`, "-4\n"}, + {`def m(a, b=1, *r, c, d:, **k, &bl); end; p method(:m).parameters`, + "[[:req, :a], [:opt, :b], [:rest, :r], [:req, :c], [:keyreq, :d], " + + "[:keyrest, :k], [:block, :bl]]\n"}, + {`def m(a, b); end; p [method(:m).arity, method(:m).parameters]`, + "[2, [[:req, :a], [:req, :b]]]\n"}, + {`def m(a=1); end; p [method(:m).arity, method(:m).parameters]`, + "[-1, [[:opt, :a]]]\n"}, + {`def m(*a); end; p [method(:m).arity, method(:m).parameters]`, + "[-1, [[:rest, :a]]]\n"}, + } { + if got := eval(t, c.src); got != c.want { + t.Errorf("src=%q\n got=%q\nwant=%q", c.src, got, c.want) + } + } +} + +// TestPostParamsBindFromTheTail is the CONTROL for the reflection above: the +// counts are only worth pinning if the binder agrees with them. A post +// parameter binds from the END of the argument list before the optionals are +// filled from what is left (setup_parameters_complex, vm_args.c v3_4_0:878-892), +// so `m(9)` gives the post parameter the 9 and leaves the optional on its +// default — the opposite of front-to-back binding, which would report the same +// arity while running the wrong body. +func TestPostParamsBindFromTheTail(t *testing.T) { + for _, c := range []struct{ src, want string }{ + {`def m(a=1, b); [a, b]; end; p m(9)`, "[1, 9]\n"}, + {`def m(a=1, b); [a, b]; end; p m(8, 9)`, "[8, 9]\n"}, + {`def m(a, b=2, c); [a, b, c]; end; p m(1, 3)`, "[1, 2, 3]\n"}, + {`def m(a=1, b); end; begin; m; rescue ArgumentError => e; p e.message; end`, + "\"wrong number of arguments (given 0, expected 1..2)\"\n"}, + {`p proc { |a=5, b, c, d| [a, b, c, d] }.call(1, 2)`, "[5, 1, 2, nil]\n"}, + } { + if got := eval(t, c.src); got != c.want { + t.Errorf("src=%q\n got=%q\nwant=%q", c.src, got, c.want) + } + } +} From 7db88662ef00f822cf709e226ed86aa898d931da Mon Sep 17 00:00:00 2001 From: tannevaled Date: Sat, 26 Sep 2026 09:10:08 +0200 Subject: [PATCH 3/4] #663: !~, ! and != are dispatched, not synthesised MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit MRI read: object.c v3_4_0 — rb_obj_not (:264), rb_obj_not_equal (:280), rb_obj_not_match (:1678), and the three rb_define_method calls that install them on BasicObject and Kernel (:4265, :4266, :4427); compile.c v3_4_0's specialisation table (:4260-4297); vm_insnhelper.c v3_4_0 vm_opt_not (:7019) and vm_opt_neq (:6665). In MRI none of the three is a compiler-synthesised negation. All three are real methods. `!` and `!=` get SPECIALISED instructions, opt_not and opt_neq, which compute the answer in place only while the receiver's method is still the built-in cfunc (vm_method_cfunc_is) and otherwise fall back to the send. `!~` gets no specialisation at all: idNMatch is absent from the opt_* table, so every `a !~ b` is an ordinary method call. The issue asked whether one cause had produced three defects. Measured against MRI 4.0.5 it had produced three, but each with a different shape — so fixing only the one the issue named would have left two: * `!~` was compiled to `!(a =~ b)`. A user-defined #!~ was DEAD CODE, and so was Kernel#!~ itself: `Object.new.send(:!~, 1)` reported '!~' missing where MRI reports '=~'. This is what inverted 11 assertions in the conformance shim, which spells `!~` on its ShouldProxy. * unary `!` was inlined UNCONDITIONALLY. A redefined #! never ran, anywhere — not from a class, not from a singleton, not from a module. * `!=` dispatched, but hasCustomNeq walked vm.classOf(a).super only. A singleton `def o.!=` and a module-supplied one (o.extend M) were both invisible and the opcode inverted #== instead. Half right is the hardest kind to notice: the class case worked, so the mechanism looked present. One resolver now answers all three. overriddenNegation resolves the name through the FULL dispatch chain — singleton class and extended modules first, then the class ancestry — and reports it only when it is not the BasicObject default, which is vm_method_cfunc_is. notValue is opt_not; hasCustomNeq is one line over the same resolver; the compiler emits a plain send for `!~`. Kernel#!~ goes in the prelude, in Ruby, because the body IS Ruby: send #=~ and negate, so a redefined #=~ or a mock expecting one is what runs. That puts the whole reflective surface where MRI has it — #respond_to?(:!~), Kernel.instance_method(:!~), #method(:!~), and #send. Three call sites had to move together or the interpreter and the AOT-lowered bodies would answer differently for the same source: internal/aot/codegen.go and level2.go now emit vm.notValue rather than an inline negation. Their two pinned expectations were updated to the MRI-verified form. The frozen prelude was regenerated and is NOT byte-identical: it carries the new / !~ ISeqs, which is the check that the emitter saw the addition. Cost: an opt_not now consults a per-instruction inline cache (the CALL_DATA MRI's opt_not carries) instead of negating blind. Measured serially, three runs each, on a loop that does nothing but negate 10M times: 1.11s -> 1.20s user, about +8% of a synthetic worst case. Measured, per file, rbgo before -> after against MRI 4.0.5 through the same shim: core/kernel/not_match_spec.rb 2 (+1 error) -> 3 (MRI 3) core/exception/full_message_spec.rb 4 -> 8 (MRI 20) language/match_spec.rb 1 -> 2 (MRI 7) language/defined_spec.rb 255 -> 256 (MRI 257) language/ total 2330 -> 2332 (MRI 2627) core/basicobject/not_spec.rb and not_equal_spec.rb were already at MRI's 2 and 8 and stayed there: they only exercise the defaults. Left: Object.new.method(:!~).parameters answers [[:req, :other]] where MRI answers [[:req]], because MRI's is a cfunc with no parameter name and a Ruby definition must spell one. Nothing in language/ or core/ reads it. Touches two files outside the wave's exclusive list — internal/vm/arith.go (one function body, where hasCustomNeq lives) and internal/vm/prelude.rb (one method appended). Neither is on the do-not-touch list; fixing `!=` and giving `!~` a real method could not be done from the exclusive set alone, and leaving either half would have left the other lying. Closes #663. Co-Authored-By: Claude Opus 5 --- internal/aot/codegen.go | 5 +- internal/aot/codegen_test.go | 2 +- internal/aot/level2.go | 5 +- internal/aot/level2_test.go | 2 +- internal/compiler/compiler.go | 16 ++- internal/vm/arith.go | 13 +-- internal/vm/object_model.go | 89 +++++++++++++++ internal/vm/prelude.rb | 16 +++ internal/vm/prelude_frozen_gen.go | 38 ++++++- internal/vm/vm.go | 26 +++-- internal/vm/wave32_negation_test.go | 162 ++++++++++++++++++++++++++++ 11 files changed, 352 insertions(+), 22 deletions(-) create mode 100644 internal/vm/wave32_negation_test.go diff --git a/internal/aot/codegen.go b/internal/aot/codegen.go index 56e50164..e796937c 100644 --- a/internal/aot/codegen.go +++ b/internal/aot/codegen.go @@ -135,7 +135,10 @@ func (g *gen) emit(pc int) (string, bool) { case bytecode.OpNeg: return line("s%d = negate(s%d)", d-1, d-1), true case bytecode.OpNot: - return line("s%d = object.Bool(!s%d.Truthy())", d-1, d-1), true + // vm.notValue, not an inline negation: MRI's opt_not dispatches a redefined + // #! (vm_insnhelper.c v3_4_0:7019), and a lowered body must answer the same + // as the interpreter it replaces. + return line("s%d = vm.notValue(s%d)", d-1, d-1), true case bytecode.OpTruthy: return line("s%d = object.Bool(s%d.Truthy())", d-1, d-1), true case bytecode.OpJump: diff --git a/internal/aot/codegen_test.go b/internal/aot/codegen_test.go index d7ed642b..7f0bccad 100644 --- a/internal/aot/codegen_test.go +++ b/internal/aot/codegen_test.go @@ -48,7 +48,7 @@ func TestCompileSupported(t *testing.T) { {"arith", "def m(a, b) = a * b + a / b - a % b\nm(1, 2)", "m", "OpMul"}, {"cmp", "def m(a, b) = a > b == (a <= b)\nm(1, 2)", "m", "OpGt"}, {"cmp2", "def m(a, b) = (a >= b) != (a < b)\nm(1, 2)", "m", "OpGe"}, - {"not", "def m(a) = !a\nm(1)", "m", "!s0.Truthy()"}, + {"not", "def m(a) = !a\nm(1)", "m", "vm.notValue(s0)"}, {"neg", "def m(a) = -a\nm(1)", "m", "negate("}, {"or", "def m(a) = a || 1\nm(1)", "m", "s1.Truthy() { goto"}, {"and", "def m(a) = a && 1\nm(1)", "m", "!s1.Truthy() { goto"}, diff --git a/internal/aot/level2.go b/internal/aot/level2.go index 205c0058..f12dd865 100644 --- a/internal/aot/level2.go +++ b/internal/aot/level2.go @@ -170,7 +170,10 @@ func (s *mscope) emit(pc int) (string, bool) { case bytecode.OpNeg: return line("%s = negate(%s)", s.sv(d-1), s.sv(d-1)), true case bytecode.OpNot: - return line("%s = object.Bool(!%s.Truthy())", s.sv(d-1), s.sv(d-1)), true + // vm.notValue, not an inline negation: MRI's opt_not dispatches a redefined + // #! (vm_insnhelper.c v3_4_0:7019), and a lowered body must answer the same + // as the interpreter it replaces. + return line("%s = vm.notValue(%s)", s.sv(d-1), s.sv(d-1)), true case bytecode.OpTruthy: return line("%s = object.Bool(%s.Truthy())", s.sv(d-1), s.sv(d-1)), true case bytecode.OpJump: diff --git a/internal/aot/level2_test.go b/internal/aot/level2_test.go index 921f216b..b090fa93 100644 --- a/internal/aot/level2_test.go +++ b/internal/aot/level2_test.go @@ -35,7 +35,7 @@ func TestCompileMainSupported(t *testing.T) { {"cmp", "a=1;b=2;puts((a=b))", "bytecode.OpLt"}, {"cmp2", "a=1;b=2;puts((a>b)!=(a<=b))", "bytecode.OpGt"}, {"neg", "x=5; puts(-x)", "negate("}, - {"not", "puts(!true)", "object.Bool(!s0_1.Truthy())"}, + {"not", "puts(!true)", "vm.notValue(s0_1)"}, {"whileloop", "i=0; i+=1 while i<3; puts i", "goto L"}, {"ifbranch", "x=1; puts(x>0 ? :a : :b)", "if !s0_1.Truthy() { goto"}, {"or_dup", "a=nil; puts(a || 7)", "if s0_2.Truthy() { goto"}, diff --git a/internal/compiler/compiler.go b/internal/compiler/compiler.go index ddd4bcb9..e9f176f0 100644 --- a/internal/compiler/compiler.go +++ b/internal/compiler/compiler.go @@ -779,11 +779,21 @@ func (c *Compiler) compileNode1(n ast.Node) { return } if v.Op == "!~" { - // `a !~ b` is the truthy-negation of `a =~ b`. + // `a !~ b` is a SEND of :!~. MRI gives it no specialised instruction at + // all — idNMatch is absent from the opt_* table that turns a send into + // opt_not / opt_neq / opt_regexpmatch2 (compile.c v3_4_0:4260-4297) — so + // every `!~` is an ordinary method call, and Kernel#!~ supplies the + // default `!(self =~ other)` (rb_obj_not_match, object.c v3_4_0:1678, + // installed at :4427). + // + // Compiling the negation here instead made a user-defined #!~ DEAD CODE: + // the send that would reach it was never emitted, so `a !~ b` silently + // ran #=~ and inverted it. That is #663, and it also inverted 11 + // assertions in the conformance shim, which spells `!~` on its + // ShouldProxy. c.compileNode(v.Left) c.compileNode(v.Right) - b.emit(bytecode.OpSend, b.addName("=~"), 1) - b.emit(bytecode.OpNot, 0, 0) + b.emit(bytecode.OpSend, b.addName("!~"), 1) return } // `/…(?…)…/ =~ str`: when the left side is a *literal* regexp with diff --git a/internal/vm/arith.go b/internal/vm/arith.go index fb79ad2d..a6cd4eb7 100644 --- a/internal/vm/arith.go +++ b/internal/vm/arith.go @@ -237,12 +237,13 @@ func bigOp(op bytecode.Op, a, b *big.Int) object.Value { // anywhere below BasicObject (whose default `!=` returns !(self == other)). When // it does, the OpNeq opcode must dispatch that method rather than invert `==`. func (vm *VM) hasCustomNeq(a object.Value) bool { - for c := vm.classOf(a); c != nil && c != vm.cBasicObject; c = c.super { - if lookupOwnOrIncluded(c, "!=") != nil { - return true - } - } - return false + // overriddenNegation resolves through the FULL dispatch chain — the receiver's + // singleton class and extended modules first, then its class ancestry — which + // is what vm_method_cfunc_is asks of the resolved method (vm_insnhelper.c + // v3_4_0:7019). Walking vm.classOf(a).super alone, as this did, could not see + // a `def o.!=` or an `o.extend M` that supplies one, and the opcode inverted + // #== for both. + return vm.overriddenNegation(a, "!=") != nil } func (vm *VM) binaryOp(op bytecode.Op, a, b object.Value) object.Value { diff --git a/internal/vm/object_model.go b/internal/vm/object_model.go index 5ff85b06..db8ab22a 100644 --- a/internal/vm/object_model.go +++ b/internal/vm/object_model.go @@ -1860,6 +1860,95 @@ func (vm *VM) findMethod(recv object.Value, name string) *Method { return nil } +// defaultNegation returns the BasicObject definition of "!" or "!=" — the ones +// rbgo installs for rb_obj_not and rb_obj_not_equal (object.c v3_4_0:264, :280; +// rb_define_method at :4265-4266). Memoised, because overriddenNegation asks on +// every unary `!`. +// +// A user who REDEFINES BasicObject#! installs a different *Method, which no +// longer matches the memo, so overriddenNegation reports it as an override and +// it gets dispatched — which is what MRI does, since vm_opt_not then finds a +// cfunc other than rb_obj_not. +func (vm *VM) defaultNegation(name string) *Method { + if m, ok := vm.defaultNeg[name]; ok { + return m + } + m := lookupMethod(vm.cBasicObject, name) + if m == nil { + // Asked before the bootstrap installed it: do not memoise nil, or every + // later `!` would see an "override". + return nil + } + if vm.defaultNeg == nil { + vm.defaultNeg = map[string]*Method{} + } + vm.defaultNeg[name] = m + return m +} + +// overriddenNegation resolves name ("!" or "!=") through recv's FULL dispatch +// chain — its singleton class and extended modules first, then its class +// ancestry — and returns the method only when it is NOT the BasicObject +// default. nil means "nobody redefined it", so the caller may compute the +// answer inline. +// +// This is vm_method_cfunc_is(iseq, cd, recv, rb_obj_not) (vm_insnhelper.c +// v3_4_0:7019): MRI's opt_not and opt_neq are SPECIALISATIONS of an ordinary +// send, and they fall back to that send whenever the receiver's method is not +// the built-in cfunc. The negation is never simply inlined by the compiler. +// +// Resolving through the whole chain is the part rbgo's previous `!=` check +// missed: it walked vm.classOf(a).super only, so a singleton `def o.!=` and a +// module-supplied `!=` (o.extend M) were both invisible and the opcode inverted +// #== instead. +func (vm *VM) overriddenNegation(recv object.Value, name string) *Method { + m := vm.findMethod(recv, name) + if m == nil || m == vm.defaultNegation(name) { + return nil + } + return m +} + +// notValue is `!recv`: MRI's opt_not, which dispatches a redefined #! and +// otherwise negates truthiness in place (vm_insnhelper.c v3_4_0:7019-7027). +// The interpreter uses notValueCached; this uncached form is what AOT-lowered +// bodies call. +func (vm *VM) notValue(recv object.Value) object.Value { + if m := vm.overriddenNegation(recv, "!"); m != nil { + return vm.invoke(m, recv, nil, nil) + } + return object.Bool(!recv.Truthy()) +} + +// notValueCached is notValue with the per-instruction inline cache MRI's +// opt_not carries as its CALL_DATA, so the common case — nobody has redefined +// #! for this receiver's class — costs a class-pointer compare rather than an +// ancestry walk on every negation. +// +// ic.method here means "the OVERRIDE to dispatch", and nil means "negate in +// place" — not lookupCached's "no method at all". The two readers never meet: +// an OpNot instruction is never an OpSend site, so its cache slot belongs to +// this one alone. That is the same argument ic.regexp rests on. +func (vm *VM) notValueCached(ic *inlineCache, recv object.Value) object.Value { + if _, isClass := recv.(*RClass); isClass || vm.objSingleton(recv) != nil { + // A per-object method cannot be cached against a shared class. + return vm.notValue(recv) + } + c := vm.classOf(recv) + serial := methodSerial() + if ic.class != c || ic.serial != serial { + m := undefAsNil(lookupMethod(c, "!")) + if m == vm.defaultNegation("!") { + m = nil // still rb_obj_not, so opt_not stays inline + } + ic.class, ic.method, ic.serial = c, m, serial + } + if ic.method != nil { + return vm.invoke(ic.method, recv, nil, nil) + } + return object.Bool(!recv.Truthy()) +} + func (vm *VM) send(recv object.Value, name string, args []object.Value, blk *Proc) object.Value { // A class receiver consults its singleton-method chain (def self.foo, and // inherited class methods) before the generic Class instance methods. diff --git a/internal/vm/prelude.rb b/internal/vm/prelude.rb index b5696631..0d601c80 100644 --- a/internal/vm/prelude.rb +++ b/internal/vm/prelude.rb @@ -2568,3 +2568,19 @@ def warn(message, category: nil) end extend self end + +# Kernel#!~ is rb_obj_not_match (object.c): a REAL method, installed on Kernel +# (`rb_define_method(rb_mKernel, "!~", rb_obj_not_match, 1)`), not something the +# compiler synthesises. MRI has no specialised instruction for `!~` — idNMatch +# is absent from compile.c's opt_* table — so every `a !~ b` is an ordinary send +# that lands here unless the receiver defines its own, which is exactly what +# makes a user-defined #!~ reachable (issue #663). +# +# It lives in the prelude rather than in Go because the body IS Ruby: send #=~ +# and negate the result, so a redefined #=~ (or a mock that expects one) is the +# thing that runs. +module Kernel + def !~(other) + !(self =~ other) + end +end diff --git a/internal/vm/prelude_frozen_gen.go b/internal/vm/prelude_frozen_gen.go index 082cafe7..a3fd724a 100644 --- a/internal/vm/prelude_frozen_gen.go +++ b/internal/vm/prelude_frozen_gen.go @@ -11,15 +11,15 @@ import ( func embeddedPrelude() *bytecode.ISeq { return &bytecode.ISeq{ Name: "
", - Insns: []bytecode.Instr{{Op: 43}, {Op: 9}, {Op: 43, A: 1, B: 1}, {Op: 9}, {Op: 42, A: 2, B: 2}, {Op: 9}, {Op: 42, A: 3, B: 3}, {Op: 9}, {Op: 42, A: 4, B: 4}, {Op: 9}, {Op: 42, A: 5, B: 5}, {Op: 9}, {Op: 42, A: 6, B: 6}, {Op: 9}, {Op: 42, A: 7, B: 7}, {Op: 9}, {Op: 42, A: 8, B: 8}, {Op: 9}, {Op: 42, A: 9, B: 9}, {Op: 9}, {Op: 42, A: 10, B: 10}, {Op: 9}, {Op: 43, A: 11, B: 11}, {Op: 9}, {Op: 42, A: 12, B: 12}, {Op: 9}, {Op: 43, A: 13, B: 13}, {Op: 9}, {Op: 43, A: 14, B: 14}, {Op: 9}, {Op: 42, A: 15, B: 15}, {Op: 9}, {Op: 42, A: 16, B: 16}, {Op: 9}, {Op: 44, A: 17, B: 17}, {Op: 9}, {Op: 42, A: 18, B: 18}, {Op: 9}, {Op: 43, A: 19, B: 19}, {Op: 9}, {Op: 42, A: 20, B: 20}, {Op: 9}, {Op: 43, A: 21, B: 21}, {Op: 9}, {Op: 43, A: 22, B: 22}, {Op: 9}, {Op: 42, A: 23, B: 23}, {Op: 9}, {Op: 33, A: 23}, {Op: 41, A: 24}, {Op: 9}, {Op: 42, A: 25, B: 24}, {Op: 9}, {Op: 43, A: 26, B: 25}, {Op: 53}}, - Names: []string{"Comparable", "Enumerable", "Numeric", "Integer", "Float", "String", "Symbol", "Array", "Range", "Hash", "Set", "Gem", "OpenStruct", "Forwardable", "SingleForwardable", "Delegator", "SimpleDelegator", "DelegateClass", "Pathname", "Singleton", "ERB", "Slim", "Haml", "OptionParser", "OptParse", "Tempfile", "Warning"}, + Insns: []bytecode.Instr{{Op: 43}, {Op: 9}, {Op: 43, A: 1, B: 1}, {Op: 9}, {Op: 42, A: 2, B: 2}, {Op: 9}, {Op: 42, A: 3, B: 3}, {Op: 9}, {Op: 42, A: 4, B: 4}, {Op: 9}, {Op: 42, A: 5, B: 5}, {Op: 9}, {Op: 42, A: 6, B: 6}, {Op: 9}, {Op: 42, A: 7, B: 7}, {Op: 9}, {Op: 42, A: 8, B: 8}, {Op: 9}, {Op: 42, A: 9, B: 9}, {Op: 9}, {Op: 42, A: 10, B: 10}, {Op: 9}, {Op: 43, A: 11, B: 11}, {Op: 9}, {Op: 42, A: 12, B: 12}, {Op: 9}, {Op: 43, A: 13, B: 13}, {Op: 9}, {Op: 43, A: 14, B: 14}, {Op: 9}, {Op: 42, A: 15, B: 15}, {Op: 9}, {Op: 42, A: 16, B: 16}, {Op: 9}, {Op: 44, A: 17, B: 17}, {Op: 9}, {Op: 42, A: 18, B: 18}, {Op: 9}, {Op: 43, A: 19, B: 19}, {Op: 9}, {Op: 42, A: 20, B: 20}, {Op: 9}, {Op: 43, A: 21, B: 21}, {Op: 9}, {Op: 43, A: 22, B: 22}, {Op: 9}, {Op: 42, A: 23, B: 23}, {Op: 9}, {Op: 33, A: 23}, {Op: 41, A: 24}, {Op: 9}, {Op: 42, A: 25, B: 24}, {Op: 9}, {Op: 43, A: 26, B: 25}, {Op: 9}, {Op: 43, A: 27, B: 26}, {Op: 53}}, + Names: []string{"Comparable", "Enumerable", "Numeric", "Integer", "Float", "String", "Symbol", "Array", "Range", "Hash", "Set", "Gem", "OpenStruct", "Forwardable", "SingleForwardable", "Delegator", "SimpleDelegator", "DelegateClass", "Pathname", "Singleton", "ERB", "Slim", "Haml", "OptionParser", "OptParse", "Tempfile", "Warning", "Kernel"}, NumRequired: 0, SplatIndex: -1, PostCount: 0, KwRestSlot: -1, BlockSlot: -1, NumLocals: 0, - Lines: []bytecode.LineEntry{{PC: 0, Line: 10}, {PC: 2, Line: 116}, {PC: 4, Line: 975}, {PC: 6, Line: 990}, {PC: 8, Line: 1001}, {PC: 10, Line: 1014}, {PC: 12, Line: 1018}, {PC: 14, Line: 1027}, {PC: 16, Line: 1036}, {PC: 18, Line: 1069}, {PC: 20, Line: 1084}, {PC: 22, Line: 1456}, {PC: 24, Line: 1676}, {PC: 26, Line: 1755}, {PC: 28, Line: 1786}, {PC: 30, Line: 1806}, {PC: 32, Line: 1846}, {PC: 34, Line: 1859}, {PC: 36, Line: 1899}, {PC: 38, Line: 2070}, {PC: 40, Line: 2106}, {PC: 42, Line: 2170}, {PC: 44, Line: 2273}, {PC: 46, Line: 2372}, {PC: 48, Line: 2417}, {PC: 51, Line: 2427}, {PC: 53, Line: 2524}}, + Lines: []bytecode.LineEntry{{PC: 0, Line: 10}, {PC: 2, Line: 116}, {PC: 4, Line: 975}, {PC: 6, Line: 990}, {PC: 8, Line: 1001}, {PC: 10, Line: 1014}, {PC: 12, Line: 1018}, {PC: 14, Line: 1027}, {PC: 16, Line: 1036}, {PC: 18, Line: 1069}, {PC: 20, Line: 1084}, {PC: 22, Line: 1456}, {PC: 24, Line: 1676}, {PC: 26, Line: 1755}, {PC: 28, Line: 1786}, {PC: 30, Line: 1806}, {PC: 32, Line: 1846}, {PC: 34, Line: 1859}, {PC: 36, Line: 1899}, {PC: 38, Line: 2070}, {PC: 40, Line: 2106}, {PC: 42, Line: 2170}, {PC: 44, Line: 2273}, {PC: 46, Line: 2372}, {PC: 48, Line: 2417}, {PC: 51, Line: 2427}, {PC: 53, Line: 2524}, {PC: 55, Line: 2582}}, FirstLine: 1, Children: []*bytecode.ISeq{ &bytecode.ISeq{ @@ -8021,6 +8021,38 @@ func embeddedPrelude() *bytecode.ISeq { }, Super: "", }, + &bytecode.ISeq{ + Name: "", + Insns: []bytecode.Instr{{Op: 44}, {Op: 53}}, + Names: []string{"!~"}, + NumRequired: 0, + SplatIndex: -1, + PostCount: 0, + KwRestSlot: -1, + BlockSlot: -1, + NumLocals: 0, + Lines: []bytecode.LineEntry{{PC: 0, Line: 2583}}, + FirstLine: 2582, + Children: []*bytecode.ISeq{ + &bytecode.ISeq{ + Name: "!~", + Insns: []bytecode.Instr{{Op: 5}, {Op: 11}, {Op: 30, B: 1}, {Op: 25}, {Op: 53}}, + Names: []string{"=~"}, + Params: []string{"other"}, + NumRequired: 1, + SplatIndex: -1, + PostCount: 0, + KwRestSlot: -1, + BlockSlot: -1, + NumLocals: 1, + Locals: []string{"other"}, + Lines: []bytecode.LineEntry{{PC: 0, Line: 109}}, + FirstLine: 2583, + Super: "", + }, + }, + Super: "", + }, }, Super: "", } diff --git a/internal/vm/vm.go b/internal/vm/vm.go index b4c3792c..ee278e37 100644 --- a/internal/vm/vm.go +++ b/internal/vm/vm.go @@ -464,11 +464,17 @@ type VM struct { regexpTimeout object.Value // Regexp.timeout default (Float seconds or nil) globals map[string]object.Value // user-assigned $globals cTrueClass, cFalseClass, cNilClass *RClass - cRegexp, cMatchData *RClass - cException *RClass - backtraceLocationClass *RClass // Thread::Backtrace::Location (Exception#backtrace_locations) - curExc object.Value // most recently rescued exception (for bare `raise`) - catchTags []object.Value // active Kernel#catch tags, innermost last (for Kernel#throw) + // defaultNeg memoises the BasicObject definitions of "!" and "!=" — the + // records rb_obj_not / rb_obj_not_equal correspond to. overriddenNegation + // compares against them to decide whether a negation may be computed inline + // or must be dispatched, which is what MRI's opt_not / opt_neq do with + // vm_method_cfunc_is. See defaultNegation. + defaultNeg map[string]*Method + cRegexp, cMatchData *RClass + cException *RClass + backtraceLocationClass *RClass // Thread::Backtrace::Location (Exception#backtrace_locations) + curExc object.Value // most recently rescued exception (for bare `raise`) + catchTags []object.Value // active Kernel#catch tags, innermost last (for Kernel#throw) loaded map[string]bool // require/require_relative: features loaded once featureHooks map[string]func() // built-in feature -> body run once on its first require (e.g. shellwords) @@ -1975,7 +1981,15 @@ func (vm *VM) exec(iseq *bytecode.ISeq, self object.Value, args []object.Value, push(negate(negArg)) } case bytecode.OpNot: - push(object.Bool(!pop().Truthy())) + // MRI's opt_not: a SPECIALISATION of `recv.!`, not an inlining of it. + // It negates truthiness only while the receiver's #! is still + // rb_obj_not, and otherwise falls back to the send + // (vm_insnhelper.c v3_4_0:7019). notValueCached is that check, + // carrying the per-instruction cache opt_not carries as CALL_DATA. + if caches == nil { + caches = iseqCaches(iseq) + } + push(vm.notValueCached(&caches[pc], pop())) case bytecode.OpTruthy: push(object.Bool(pop().Truthy())) case bytecode.OpRaiseNoMatch: diff --git a/internal/vm/wave32_negation_test.go b/internal/vm/wave32_negation_test.go new file mode 100644 index 00000000..d4163657 --- /dev/null +++ b/internal/vm/wave32_negation_test.go @@ -0,0 +1,162 @@ +// Copyright (c) the go-embedded-ruby/ruby authors +// +// SPDX-License-Identifier: BSD-3-Clause + +package vm + +import "testing" + +// TestNegationOperatorsDispatch covers issue #663 and the two neighbours it +// asked about. +// +// In MRI none of `!`, `!=` and `!~` is a compiler-synthesised negation. All +// three are real methods on BasicObject / Kernel — rb_obj_not, rb_obj_not_equal +// and rb_obj_not_match (object.c v3_4_0:264, :280, :1678; installed at :4265, +// :4266, :4427). `!` and `!=` get SPECIALISED instructions, opt_not and +// opt_neq, which compute the answer in place only while the receiver's method +// is still the built-in cfunc and otherwise fall back to the send +// (vm_insnhelper.c v3_4_0:6665, :7019). `!~` gets no specialisation at all: +// idNMatch is absent from compile.c's opt_* table (v3_4_0:4260-4297). +// +// Measured against MRI 4.0.5, rbgo diverged on all three, each for a different +// reason: +// +// - `!~` was compiled as `!(a =~ b)`, so a user-defined #!~ was DEAD CODE and +// `Object.new.send(:!~, 1)` reported the wrong missing method; +// - `!` was inlined unconditionally, so a redefined #! never ran anywhere; +// - `!=` dispatched only for a method found on the class chain below +// BasicObject, so a singleton `def o.!=` and a module-supplied one +// (o.extend M) were both invisible and the opcode inverted #== instead. +func TestNegationOperatorsDispatch(t *testing.T) { + for _, c := range []struct{ src, want string }{ + // A class definition reaches all three. + {`class C + def !~(o); "cls-nmatch"; end + def !=(o); "cls-neq"; end + def !; "cls-not"; end +end +c = C.new +p [c !~ 1, c != 1, !c]`, `["cls-nmatch", "cls-neq", "cls-not"]` + "\n"}, + + // A SINGLETON definition reaches all three. This is the half the old `!=` + // check could not see: it walked classOf(a).super, which skips the + // singleton class entirely. + {`o = Object.new +o.define_singleton_method(:!~) { |x| "sing-nmatch" } +o.define_singleton_method(:!=) { |x| "sing-neq" } +o.define_singleton_method(:!) { "sing-not" } +p [o !~ 1, o != 1, !o]`, `["sing-nmatch", "sing-neq", "sing-not"]` + "\n"}, + + // A module mixed in with #extend reaches all three, for the same reason. + {`m = Module.new do + def !~(x); "mod-nmatch"; end + def !=(x); "mod-neq"; end + def !; "mod-not"; end +end +q = Object.new.extend(m) +p [q !~ 1, q != 1, !q]`, `["mod-nmatch", "mod-neq", "mod-not"]` + "\n"}, + + // A BasicObject subclass: below BasicObject, so the class chain reaches it. + {`class D < BasicObject + def !=(o); "bo-neq"; end + def !; "bo-not"; end +end +d = D.new +::Kernel.p [d != 1, !d]`, `["bo-neq", "bo-not"]` + "\n"}, + + // The DEFAULTS, unchanged. Each still answers what the built-in does, so + // the specialisation is a specialisation and not a behaviour change. + {`p [!true, !false, !nil, !0, !"", ![]]`, + "[false, true, true, false, false, false]\n"}, + {`p [1 != 2, 1 != 1, "a" != "a", nil != nil, Object.new != Object.new]`, + "[true, false, false, false, true]\n"}, + {`p ["abc" !~ /b/, "abc" !~ /z/, nil !~ /z/]`, "[false, true, true]\n"}, + + // Kernel#!~ is a REAL method now, not a compiler shape, so the whole + // reflective surface answers as MRI's does. + {`p Object.new.respond_to?(:!~)`, "true\n"}, + {`p Kernel.instance_method(:!~).owner`, "Kernel\n"}, + {`p Object.new.method(:!~).arity`, "1\n"}, + + // …and #send reaches it, which is the difference that shows the default is + // really `!(self =~ other)`: the missing method reported is #=~, not #!~. + {`begin; Object.new.send(:!~, 1); rescue NoMethodError => e; p e.message; end`, + "\"undefined method '=~' for an instance of Object\"\n"}, + + // The default sends #=~, so a redefined #=~ is what runs under it. + {`class E; def =~(o); o == 7; end; end +p [E.new !~ 7, E.new !~ 8]`, "[false, true]\n"}, + + // `!=` still dispatches #== when nobody redefined #!= — not an identity + // compare (rb_obj_not_equal calls id_eq, object.c v3_4_0:282). + {`class F; def ==(o); true; end; end +p [F.new != F.new, F.new != 1]`, "[false, false]\n"}, + + // And a REDEFINED #! is dispatched from inside a lowered/branching + // position too, not only from a `p` argument. + {`class G; def !; :truthy_never; end; end +g = G.new +r = [] +3.times { r << !g } +p r`, "[:truthy_never, :truthy_never, :truthy_never]\n"}, + } { + if got := eval(t, c.src); got != c.want { + t.Errorf("src=%q\n got=%q\nwant=%q", c.src, got, c.want) + } + } +} + +// TestNegationCacheInvalidates is the CONTROL for the inline cache +// notValueCached keeps: a cached "nobody redefined #!" verdict must not survive +// the redefinition that makes it false. MRI invalidates the same verdict +// through its method-state serial; rbgo's cache re-validates against +// globalMethodSerial, which every definition path bumps. +// +// Each case negates the SAME receiver class at the SAME instruction before and +// after the change, so a stale cache shows up as the old answer coming back. +func TestNegationCacheInvalidates(t *testing.T) { + for _, c := range []struct{ src, want string }{ + // Fill the cache with the default, then redefine #! on the class. + {`class H; end +h = H.new +r = [] +2.times do |i| + class H; def !; :custom; end; end if i == 1 + r << !h +end +p r`, "[false, :custom]\n"}, + + // Fill the cache with the default, then give the OBJECT a singleton #!. + {`i = Object.new +r = [] +2.times do |n| + i.define_singleton_method(:!) { :sing } if n == 1 + r << !i +end +p r`, "[false, :sing]\n"}, + + // Same for #!=, whose verdict goes through the same resolver. + {`class J; end +j = J.new +r = [] +2.times do |n| + class J; def !=(o); :custom_neq; end; end if n == 1 + r << (j != 1) +end +p r`, "[true, :custom_neq]\n"}, + + // And the other direction: remove the override and the default returns. + {`class K; def !; :custom; end; end +k = K.new +r = [] +2.times do |n| + K.send(:remove_method, :!) if n == 1 + r << !k +end +p r`, "[:custom, false]\n"}, + } { + if got := eval(t, c.src); got != c.want { + t.Errorf("src=%q\n got=%q\nwant=%q", c.src, got, c.want) + } + } +} From 85f1440f5ffc9e83ece3606eb0346bfb588f1782 Mon Sep 17 00:00:00 2001 From: tannevaled Date: Sat, 26 Sep 2026 09:25:25 +0200 Subject: [PATCH 4/4] A refinement activated in a block body now reaches the blocks written in it MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit MRI read: rb_vm_cref_replace_with_duplicated_cref (vm.c v3_4_0:1931) → vm_cref_replace_with_duplicated_cref (vm_insnhelper.c v3_4_0:912), called from mod_using (eval.c v3_4_0:1547) and top_using (eval.c v3_4_0:1890); vm_cref_push and its pushed_by_eval flag (vm_insnhelper.c); rb_yield_refine_block and rb_mod_refine (eval.c). `using` does not merely record a module. It REPLACES the calling frame's cref with a duplicate that carries the refinements, so every block created after it in that frame captures the replacement and every scope pushed inside it chains to it. MRI then walks the whole chain in rb_method_entry_with_refinements. rbgo records one *RClass per frame, and adoptRefinementCref already performed that replacement on the frame's cref ENTRY — which is why `1.foo` written DIRECTLY in a `Module.new { using R; … }` body already worked. What did not work was everything written inside it: the block-literal path read the lexCref LOCAL, settled before `using` ran and frozen at where the block was written (top level, for a Module.new body). Measured against MRI 4.0.5, a refinement activated in such a body reached NO nested block at all — not Module.new {}, not Class.new {}, not #class_eval or #module_eval, not #instance_eval or #instance_exec, not even `[0].each { }`. Three links, because rbgo has no chain and MRI's has to be approximated by its ends: 1. frameCref() reads the frame's cref ENTRY rather than the local, so a block literal captures the post-`using` replacement, as MRI captures it at block-creation time; 2. refinedFor consults the frame's DEFINEE and then that cref. The definee is MRI's pushed link — the refinement module inside a `refine` block, the eval target inside a class_eval — and the cref is the textual one it was pushed onto. Consulting only the cref cost 3 examples in refine_spec, because a refine block's own sibling refinements hang off the pushed link; consulting only the definee was the original defect. Both, innermost first, is what MRI's walk does. 3. Proc.refDefinee carries the second end onto a block, because a method of a class whose body called `using` has the TEXTUAL scope as its frame cref — so a lambda written in that method had no link back at all. refDefinee feeds refinement lookup ONLY. Constant resolution still follows cref alone, because MRI SKIPS a pushed_by_eval cref when resolving a constant, and TestCrefChainKeepsItsOtherEnd is the control for that: a constant inside a class_eval block still resolves textually, Module.nesting is still the textual nesting, and a `def` there still lands on the eval target. Measured, per file, rbgo before -> after against MRI 4.0.5 through the same shim: core/module/using_spec.rb 14 -> 17 (MRI 20) core/module/refine_spec.rb 29 -> 30 (MRI 38) core/ total 19922 -> 19926 No file regressed; language/ is unchanged at 2332 (the shapes are all in core/). Left in using_spec: "is active for block called via instance_eval", where the receiver is a String the method holds in an ivar — a fourth link this two-ended approximation does not reach; and the two "is not active …" examples, which were failing before this change and are over-activation, not under-. Most of refine_spec's remaining gap is indirect dispatch — #send, #respond_to?, #method, Symbol#to_proc, #binding honouring refinements — which lives in refinement.go, owned by another agent this wave. Co-Authored-By: Claude Opus 5 --- internal/vm/object_model.go | 22 ++++++ internal/vm/vm.go | 83 ++++++++++++++++++-- internal/vm/wave32_cref_test.go | 129 ++++++++++++++++++++++++++++++++ 3 files changed, 227 insertions(+), 7 deletions(-) create mode 100644 internal/vm/wave32_cref_test.go diff --git a/internal/vm/object_model.go b/internal/vm/object_model.go index db8ab22a..06843ec1 100644 --- a/internal/vm/object_model.go +++ b/internal/vm/object_model.go @@ -82,6 +82,18 @@ type Proc struct { // was written, so a bare constant inside the block resolves by the same // lexical nesting as the surrounding method/class body. nil means top level. cref *RClass + // refDefinee is the OTHER end of the cref chain at block-creation time: the + // creating frame's definee, when it differs from cref. MRI keeps a chain and + // a frame that eval'd into another scope has a cref pushed on top of the one + // it was written under (vm_cref_push with pushed_by_eval), so BOTH links can + // carry refinements and rb_method_entry_with_refinements walks both. rbgo + // records one *RClass per frame, so a block has to carry the second end + // itself or a refinement activated in a Class.new / Module.new / class_eval + // body stops at the first block written inside it. + // + // It feeds refinement lookup ONLY. Constant resolution still follows cref + // alone, because MRI SKIPS a pushed_by_eval cref when resolving a constant. + refDefinee *RClass // home identifies the method (or top-level) activation a non-local `return` // inside this block unwinds to — the activation where the block literal was // written. nil for synthesized procs that never carry an explicit return. @@ -2506,3 +2518,13 @@ func ivarTable(self object.Value) map[string]object.Value { } return nil } + +// refDefineeOf picks the second end of the cref chain to record on a block +// literal: the creating frame's definee when it differs from the frame's cref, +// else nil (the two ends coincide and one pointer says it). See Proc.refDefinee. +func refDefineeOf(definee, cref *RClass) *RClass { + if definee == cref { + return nil + } + return definee +} diff --git a/internal/vm/vm.go b/internal/vm/vm.go index ee278e37..49dfe1b2 100644 --- a/internal/vm/vm.go +++ b/internal/vm/vm.go @@ -1641,6 +1641,75 @@ func (vm *VM) exec(iseq *bytecode.ISeq, self object.Value, args []object.Value, // (a native, which pushes no frame of its own) can read the call site's cref. vm.frameCrefs[frameCrefsDepth-1] = lexCref + // frameCref is THIS frame's live cref: the scope a refinement lookup resolves + // against, and the cref a block literal created here captures. + // + // It is normally lexCref. But `using` does not merely record a module: it + // REPLACES the calling frame's cref with a duplicate that carries the + // refinements — rb_vm_cref_replace_with_duplicated_cref (vm.c v3_4_0:1931) → + // vm_cref_replace_with_duplicated_cref (vm_insnhelper.c v3_4_0:912), called + // from mod_using (eval.c:1547) and top_using (eval.c:1890). A block created + // AFTER that `using`, in the same frame, therefore captures the replacement, + // and so does every scope that chains to it. + // + // rbgo's adoptRefinementCref performs that replacement on the frame's cref + // ENTRY, which is why `1.foo` directly inside a `Module.new { using R; … }` + // body already worked. The block literals did not: they read the lexCref + // LOCAL, settled before `using` ran and frozen at where the block was + // written — top level, for a `Module.new` body. So a refinement activated in + // such a body reached no nested block at all: not `Module.new {}`, not + // `Class.new {}`, not #instance_eval / #instance_exec / #class_eval, not even + // a plain `each`. + // + // Reading the entry instead of the local is the whole fix, and it is read at + // block-CREATION time, as MRI captures it. Guarded by anyRefinements, so a + // program with no refinement pays a bool test on the hot block-literal path + // and the entry always equals the local anyway. + frameCref := func() *RClass { + if vm.anyRefinements && frameCrefsDepth-1 < len(vm.frameCrefs) { + if c := vm.frameCrefs[frameCrefsDepth-1]; c != nil { + return c + } + } + return lexCref + } + + // refinedFor resolves a send against the refinements active HERE. + // + // MRI keeps a cref CHAIN and looks in every link: a frame that eval'd into + // another scope has a cref pushed on top of the one it was written under — + // vm_cref_push(..., pushed_by_eval=TRUE) for class_eval / instance_eval, and + // the same for a `refine` block, whose pushed cref's klass is the refinement + // module itself (rb_yield_refine_block, eval.c v3_4_0). Both links can carry + // refinements, and rb_method_entry_with_refinements walks them all. + // + // rbgo records one *RClass per frame, so the chain is these two ends: the + // DEFINEE is the pushed link (the refinement inside a `refine` block, the + // eval target inside a class_eval) and frameCref is the textual one it was + // pushed onto. Innermost first, as MRI pushes them. + // + // Consulting only the definee lost every refinement activated in an + // enclosing Module.new / Class.new / class_eval body; consulting only the + // cref lost the ones a `refine` block sees from its own holder. + refinedFor := func(recv object.Value, name string) *Method { + if m := vm.refinedMethod(definee, recv, name); m != nil { + return m + } + if c := frameCref(); c != definee { + if m := vm.refinedMethod(c, recv, name); m != nil { + return m + } + } + // The third link: when this frame is a block, the scope its literal was + // created UNDER — a `using` in a Class.new body reaches a lambda written + // in a method of that class only through this. + if selfBlock != nil && selfBlock.refDefinee != nil && + selfBlock.refDefinee != definee && selfBlock.refDefinee != frameCref() { + return vm.refinedMethod(selfBlock.refDefinee, recv, name) + } + return nil + } + // methodDefinee is the class a `def` (and `alias`/`undef`) in this frame lands // on. It is the definee, except under instance_eval/instance_exec, where the // method-def target is switched to the receiver's singleton class while the @@ -2030,7 +2099,7 @@ func (vm *VM) exec(iseq *bytecode.ISeq, self object.Value, args []object.Value, base := len(stack) - argc recv := stack[base-1] if vm.anyRefinements { - if rm := vm.refinedMethod(definee, recv, name); rm != nil { + if rm := refinedFor(recv, name); rm != nil { if in.Flags&bytecode.FlagSendExplicit != 0 { vm.checkVisibility(recv, name, rm, self) } @@ -2090,7 +2159,7 @@ func (vm *VM) exec(iseq *bytecode.ISeq, self object.Value, args []object.Value, recv := pop() // A literal block: capture this frame's env, self, block. markEnvCaptured(env) - blk := &Proc{iseq: iseq.Children[in.C-1], env: env, defLocals: iseq.Locals, self: self, block: block, cref: lexCref, home: homeTarget(), superName: homeSuperName, superDefinee: homeSuperDefinee, superArgs: homeSuperArgs, dmBody: homeDmBody, methodCtx: fm} + blk := &Proc{iseq: iseq.Children[in.C-1], env: env, defLocals: iseq.Locals, self: self, block: block, cref: frameCref(), refDefinee: refDefineeOf(definee, frameCref()), home: homeTarget(), superName: homeSuperName, superDefinee: homeSuperDefinee, superArgs: homeSuperArgs, dmBody: homeDmBody, methodCtx: fm} vm.sendNoKW = in.Flags&bytecode.FlagSendNoKW != 0 // MRI: the call info decides, not the callee if res, done := vm.enforceSendVisRoute(in.Flags, recv, name, callArgs, blk, self); done { push(res) @@ -2098,7 +2167,7 @@ func (vm *VM) exec(iseq *bytecode.ISeq, self object.Value, args []object.Value, continue } if vm.anyRefinements { - if rm := vm.refinedMethod(definee, recv, name); rm != nil { + if rm := refinedFor(recv, name); rm != nil { push(vm.invokeInPlace(rm, recv, callArgs, blk)) pc++ continue @@ -2118,7 +2187,7 @@ func (vm *VM) exec(iseq *bytecode.ISeq, self object.Value, args []object.Value, bblk := vm.toBlock(blockVal) vm.sendNoKW = in.Flags&bytecode.FlagSendNoKW != 0 // MRI: the call info decides, not the callee if vm.anyRefinements { - if rm := vm.refinedMethod(definee, recv, bname); rm != nil { + if rm := refinedFor(recv, bname); rm != nil { push(vm.invokeInPlace(rm, recv, callArgs, bblk)) pc++ continue @@ -2281,7 +2350,7 @@ func (vm *VM) exec(iseq *bytecode.ISeq, self object.Value, args []object.Value, superBlk := block if in.C > 0 { // an explicit `super(...) { … }` literal block overrides the frame block markEnvCaptured(env) - superBlk = &Proc{iseq: iseq.Children[in.C-1], env: env, defLocals: iseq.Locals, self: self, block: block, cref: lexCref, home: homeTarget(), superName: homeSuperName, superDefinee: homeSuperDefinee, superArgs: homeSuperArgs, dmBody: homeDmBody, methodCtx: fm} + superBlk = &Proc{iseq: iseq.Children[in.C-1], env: env, defLocals: iseq.Locals, self: self, block: block, cref: frameCref(), refDefinee: refDefineeOf(definee, frameCref()), home: homeTarget(), superName: homeSuperName, superDefinee: homeSuperDefinee, superArgs: homeSuperArgs, dmBody: homeDmBody, methodCtx: fm} } var superArgs []object.Value zsuperKW := false @@ -2323,7 +2392,7 @@ func (vm *VM) exec(iseq *bytecode.ISeq, self object.Value, args []object.Value, superBlk = vm.toBlock(pop()) case in.C > 1: // a literal `super(*a) { … }` block, from child C-2 markEnvCaptured(env) - superBlk = &Proc{iseq: iseq.Children[in.C-2], env: env, defLocals: iseq.Locals, self: self, block: block, cref: lexCref, home: homeTarget(), superName: homeSuperName, superDefinee: homeSuperDefinee, superArgs: homeSuperArgs, dmBody: homeDmBody, methodCtx: fm} + superBlk = &Proc{iseq: iseq.Children[in.C-2], env: env, defLocals: iseq.Locals, self: self, block: block, cref: frameCref(), refDefinee: refDefineeOf(definee, frameCref()), home: homeTarget(), superName: homeSuperName, superDefinee: homeSuperDefinee, superArgs: homeSuperArgs, dmBody: homeDmBody, methodCtx: fm} } argsArr := pop().(*object.Array) suElems, suNoKW := applyKWSplat(argsArr.Elems, in.Flags) @@ -2650,7 +2719,7 @@ func (vm *VM) exec(iseq *bytecode.ISeq, self object.Value, args []object.Value, var blk *Proc if in.C > 0 { markEnvCaptured(env) - blk = &Proc{iseq: iseq.Children[in.C-1], env: env, defLocals: iseq.Locals, self: self, block: block, cref: lexCref, home: homeTarget(), superName: homeSuperName, superDefinee: homeSuperDefinee, superArgs: homeSuperArgs, dmBody: homeDmBody, methodCtx: fm} + blk = &Proc{iseq: iseq.Children[in.C-1], env: env, defLocals: iseq.Locals, self: self, block: block, cref: frameCref(), refDefinee: refDefineeOf(definee, frameCref()), home: homeTarget(), superName: homeSuperName, superDefinee: homeSuperDefinee, superArgs: homeSuperArgs, dmBody: homeDmBody, methodCtx: fm} } saElems, saNoKW := applyKWSplat(argsArr.Elems, in.Flags) vm.sendNoKW = saNoKW // MRI: the call info decides, not the callee diff --git a/internal/vm/wave32_cref_test.go b/internal/vm/wave32_cref_test.go new file mode 100644 index 00000000..a97cd164 --- /dev/null +++ b/internal/vm/wave32_cref_test.go @@ -0,0 +1,129 @@ +// Copyright (c) the go-embedded-ruby/ruby authors +// +// SPDX-License-Identifier: BSD-3-Clause + +package vm + +import "testing" + +// TestRefinementReachesBlocksOfItsScope pins the reach of a `using`. +// +// MRI keeps a cref CHAIN, and `using` does not merely record a module: it +// REPLACES the calling frame's cref with a duplicate carrying the refinements +// — rb_vm_cref_replace_with_duplicated_cref (vm.c v3_4_0:1931) → +// vm_cref_replace_with_duplicated_cref (vm_insnhelper.c v3_4_0:912), from +// mod_using (eval.c:1547) and top_using (eval.c:1890). Every block created +// after it in that frame captures the replacement, every scope pushed inside it +// chains to it, and rb_method_entry_with_refinements walks the whole chain. +// +// rbgo records one *RClass per frame, and the block-literal path read the +// lexCref LOCAL, settled before `using` ran. A refinement activated in a +// Module.new / Class.new body therefore reached NOTHING written inside it — not +// a nested Module.new, not Class.new, not #class_eval, not #instance_eval or +// #instance_exec, not even a plain `each`. +// +// Every expectation here was taken from MRI 4.0.5. +func TestRefinementReachesBlocksOfItsScope(t *testing.T) { + for _, c := range []struct{ src, want string }{ + // Every block shape written inside the body that called `using`. + {`R = Module.new { refine(Integer) { def foo; "R"; end } } +r = [] +Module.new do + using R + r << (1.foo rescue "ERR") + Module.new { r << (1.foo rescue "ERR") } + Class.new { r << (1.foo rescue "ERR") } + [0].each { r << (1.foo rescue "ERR") } + Object.new.instance_exec { r << (1.foo rescue "ERR") } + Object.new.instance_eval { r << (1.foo rescue "ERR") } +end +p r`, `["R", "R", "R", "R", "R", "R"]` + "\n"}, + + // …and a block whose DEFINEE is switched to an unrelated class. + {`R = Module.new { refine(Integer) { def foo; "R"; end } } +class Zz; end +r = [] +Module.new do + using R + Zz.class_eval { r << (1.foo rescue "ERR") } + Zz.module_eval { r << (1.foo rescue "ERR") } +end +p r`, `["R", "R"]` + "\n"}, + + // A method of the class whose body called `using`, and the blocks and + // lambdas written inside THAT method — the third link, which a Proc has to + // carry itself because the method frame's cref is the textual one. + {`R = Module.new { refine(Integer) { def foo; "R"; end } } +c = Class.new do + using R + def direct; 1.foo rescue "ERR"; end + def in_block; [0].map { 1.foo rescue "ERR" }.first; end + def via_call; b = -> { 1.foo rescue "ERR" }; b.call; end + def via_iexec; b = -> { 1.foo rescue "ERR" }; self.instance_exec(&b); end +end +o = c.new +p [o.direct, o.in_block, o.via_call, o.via_iexec]`, + `["R", "R", "R", "R"]` + "\n"}, + } { + if got := eval(t, c.src); got != c.want { + t.Errorf("src=%q\n got=%q\nwant=%q", c.src, got, c.want) + } + } +} + +// TestCrefChainKeepsItsOtherEnd is the CONTROL: widening refinement lookup to +// the second end of the chain must not disturb what the FIRST end answers, nor +// what a cref is otherwise for. +// +// Two things could have broken and one of them briefly did. Resolving +// refinements from the frame's cref ALONE cost three examples in +// core/module/refine_spec.rb, because a `refine` block's own refinements hang +// off the pushed link (its klass is the refinement module, rb_yield_refine_block) +// and not off the textual one. And a cref is chiefly for CONSTANTS: MRI skips a +// pushed_by_eval cref when resolving one, so a constant inside a class_eval +// block must still resolve textually rather than against the eval target. +func TestCrefChainKeepsItsOtherEnd(t *testing.T) { + for _, c := range []struct{ src, want string }{ + // A refine block sees its holder's sibling refinements (the pushed link). + {`M = Module.new do + refine(String) { def sa; "sa"; end } + refine(Integer) { def sb; "x".sa + "sb"; end } +end +using M +p 1.sb`, "\"sasb\"\n"}, + + // A constant inside a class_eval block resolves TEXTUALLY: :top, not the + // :inq defined on the eval target. + {`KTOP = :top +class Q; KTOP = :inq; end +Q.class_eval { p KTOP }`, ":top\n"}, + + // …and Module.nesting inside it is still the textual nesting. + {`class Q; end +Q.class_eval { p Module.nesting }`, "[]\n"}, + {`module P + class Q; end + Q.class_eval { p Module.nesting } +end`, "[P]\n"}, + + // A `def` inside a class_eval lands on the eval target while its body's + // constants keep the textual scope. + {`CT = :outer +class Q2; CT = :inner; end +Q2.class_eval { def ct; CT; end } +p [Q2.new.ct, Q2.instance_method(:ct).owner]`, "[:outer, Q2]\n"}, + + // With no refinement anywhere, nothing about block cref capture moved. + {`class A3; end +A3.class_eval do + def foo; :foo; end + XC = 1 +end +p [A3.new.foo, A3.const_defined?(:XC, false), Object.const_defined?(:XC, false)]`, + "[:foo, false, true]\n"}, + } { + if got := eval(t, c.src); got != c.want { + t.Errorf("src=%q\n got=%q\nwant=%q", c.src, got, c.want) + } + } +}