From edbb21b1d669374c01f14bfcfeb11ca83853aebe Mon Sep 17 00:00:00 2001 From: tannevaled Date: Fri, 25 Sep 2026 10:08:02 +0200 Subject: [PATCH 1/6] vm: a visibility directive takes an Array, and does nothing when the level already matches MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Two rules from set_method_visibility / rb_export_method that rbgo applied only partly. `private_class_method [:foo]` did not work. MRI's set_method_visibility (ruby/ruby v3_4_0 vm_method.c:2400) treats a SINGLE Array argument as the name list itself, for every directive that routes through it — private / public / protected (through set_visibility) and private_class_method / public_class_method (rb_mod_private_method / rb_mod_public_method at :2658 and :2686). rbgo had the rule inline in the instance-method setter only, so the class-method form raised "[:foo] is not a symbol nor a string". The rule now lives in one helper, visNameList, that both call. `private :m` on a method that is ALREADY private cloned an entry onto the receiver. rb_export_method (vm_method.c:1751) wraps everything after its lookup in `if (METHOD_ENTRY_VISI(me) != visi)`: at the same level it neither rewrites the entry nor adds the ZSUPER one, so child.private_instance_methods(false) stays empty and Module#method_added does not fire. rbgo recorded a visOverride unconditionally. The lookup that feeds the comparison is rb_export_method's, Object fallback included, so `Module.new { private :puts }` is now inert too — Kernel#puts is already private — while `private :frozen?` still records. ruby/spec core/module: private_class_method 7->8, public_class_method 7->8, private 19->20, protected 17->18, public 17->18. Co-Authored-By: Claude Opus 5 --- internal/vm/module_wave30_test.go | 161 ++++++++++++++++++++++++++++++ internal/vm/modulefunc.go | 32 +++--- internal/vm/visibility.go | 23 ++++- 3 files changed, 200 insertions(+), 16 deletions(-) create mode 100644 internal/vm/module_wave30_test.go diff --git a/internal/vm/module_wave30_test.go b/internal/vm/module_wave30_test.go new file mode 100644 index 00000000..9d1961fc --- /dev/null +++ b/internal/vm/module_wave30_test.go @@ -0,0 +1,161 @@ +// Copyright (c) the go-embedded-ruby/ruby authors +// +// SPDX-License-Identifier: BSD-3-Clause + +package vm_test + +import ( + "strings" + "testing" +) + +// TestModuleWave30VisibilityArrayArgument covers MRI's single-Array form of +// every directive that routes through set_method_visibility (ruby/ruby v3_4_0 +// vm_method.c:2400): private / public / protected and their _class_method +// counterparts all take `[:a, :b]` as the name list. Asserted against MRI 4.0. +func TestModuleWave30VisibilityArrayArgument(t *testing.T) { + cases := []struct{ src, want string }{ + // private_class_method with an Array argument marks each element. + {`c = Class.new do + def self.foo() "foo" end + private_class_method [:foo] +end +begin; c.foo; p :called; rescue NoMethodError; p :blocked; end`, ":blocked\n"}, + // public_class_method with an Array argument un-marks each element. + {`c = Class.new do + def self.bar() "bar" end + private_class_method :bar + public_class_method [:bar] +end +p c.bar`, "\"bar\"\n"}, + // Several names in one Array. + {`c = Class.new do + def self.a() end + def self.b() end + private_class_method [:a, :b] +end +p [c.singleton_methods.sort, (begin; c.a; rescue NoMethodError; :blocked; end)]`, + "[[], :blocked]\n"}, + // The instance-method directives keep taking an Array too, and still + // return that array (MRI's set_visibility returns argv[0] when argc == 1). + {`mod = Module.new do + def t1() end + def t2() end +end +p mod.send(:private, [:t1, :t2]) +p mod.private_instance_methods(false).sort`, "[:t1, :t2]\n[:t1, :t2]\n"}, + // A non-Array single argument is still a plain name. + {`c = Class.new do + def self.solo() end + private_class_method :solo +end +p (begin; c.solo; rescue NoMethodError; :blocked; end)`, ":blocked\n"}, + } + for _, c := range cases { + if got := eval(t, c.src); got != c.want { + t.Errorf("src=%q\n got=%q\nwant=%q", c.src, got, c.want) + } + } + // An Array element that is not a name is still a TypeError, and an unknown + // name inside the Array is still a NameError — the Array only supplies the + // list, it does not relax the per-name checks. + if err := runErr(t, `Class.new { def self.f; end; private_class_method [123] }`); err == nil || + !strings.Contains(err.Error(), "TypeError") { + t.Errorf("array with a non-name: got %v, want TypeError", err) + } + if err := runErr(t, `Class.new { private_class_method [:nope] }`); err == nil || + !strings.Contains(err.Error(), "NameError") { + t.Errorf("array with an unknown name: got %v, want NameError", err) + } +} + +// TestModuleWave30VisibilityAlreadyAtThatLevel covers rb_export_method's +// `if (METHOD_ENTRY_VISI(me) != visi)` guard (ruby/ruby v3_4_0 vm_method.c:1751): +// asking for the level a method already has changes nothing, so no ZSUPER entry +// is cloned onto the receiver and Module#method_added does not fire. Asserted +// against MRI 4.0. +func TestModuleWave30VisibilityAlreadyAtThatLevel(t *testing.T) { + cases := []struct{ src, want string }{ + // Re-declaring an inherited PRIVATE method private adds nothing to the child. + {`parent = Module.new { def tm; end; private(:tm) } +child = Module.new { include parent; private(:tm) } +p child.private_instance_methods(false)`, "[]\n"}, + // Same for protected … + {`parent = Module.new { def tm; end; protected(:tm) } +child = Module.new { include parent; protected(:tm) } +p child.protected_instance_methods(false)`, "[]\n"}, + // … and for public, the default level. + {`parent = Module.new { def tm; end } +child = Module.new { include parent; public(:tm) } +p child.public_instance_methods(false)`, "[]\n"}, + // A CHANGE of level still records the override on the child. + {`parent = Module.new { def tm; end } +child = Module.new { include parent; private(:tm) } +p child.private_instance_methods(false)`, "[:tm]\n"}, + // The same, up the superclass chain rather than through an include. + {`parent = Class.new { def tm; end; private(:tm) } +child = Class.new(parent) { private(:tm) } +p child.private_instance_methods(false)`, "[]\n"}, + // A no-op directive does not fire method_added … + {`$fired = [] +parent = Module.new { def tm; end; private(:tm) } +child = Module.new do + include parent + def self.method_added(n) = $fired << n + private(:tm) +end +p $fired`, "[]\n"}, + // … but a real change does. + {`$fired = [] +parent = Module.new { def tm; end } +child = Module.new do + include parent + def self.method_added(n) = $fired << n + private(:tm) +end +p $fired`, "[:tm]\n"}, + // Re-declaring an OWN method at its current level is also inert, and the + // method keeps that level. + {`c = Class.new do + def own; end + private :own + private :own +end +p c.private_instance_methods(false)`, "[:own]\n"}, + // An own method DOES change level when asked for a different one. + {`c = Class.new do + def own; end + private :own + public :own +end +p [c.private_instance_methods(false), c.public_instance_methods(false)]`, + "[[], [:own]]\n"}, + } + for _, c := range cases { + if got := eval(t, c.src); got != c.want { + t.Errorf("src=%q\n got=%q\nwant=%q", c.src, got, c.want) + } + } + // A name that resolves nowhere is still a NameError, on a Class … + if err := runErr(t, `Class.new { private :nope }`); err == nil || + !strings.Contains(err.Error(), "NameError") { + t.Errorf("unknown name on a class: got %v, want NameError", err) + } + // … and on a Module whose Object fallback also misses. + if err := runErr(t, `Module.new { private :definitely_nope_30 }`); err == nil || + !strings.Contains(err.Error(), "NameError") { + t.Errorf("unknown name on a module: got %v, want NameError", err) + } + // A Module reaches Object for a name its own ancestors lack (the + // rb_export_method rb_cObject fallback) and records the override there — but + // only when Object's entry is at a DIFFERENT level: Kernel#puts is already + // private, so `private :puts` in a module body adds nothing. + if got := eval(t, `m = Module.new { private :frozen? } +p m.private_instance_methods(false)`); got != "[:frozen?]\n" { + t.Errorf("Object fallback: got %q", got) + } + if got := eval(t, `m = Module.new { private :puts } +p m.private_instance_methods(false)`); got != "[]\n" { + t.Errorf("Object fallback, same level: got %q", got) + } +} diff --git a/internal/vm/modulefunc.go b/internal/vm/modulefunc.go index 3bc61055..1b84721f 100644 --- a/internal/vm/modulefunc.go +++ b/internal/vm/modulefunc.go @@ -75,17 +75,7 @@ func (vm *VM) registerModuleExtras() { mod.defaultVis, mod.funcMode = vis, false return object.NilV } - // `private [:a, :b]` (an Array argument) marks each element, returning the - // array — MRI accepts a single Array as well as a varargs name list. - if len(args) == 1 { - if arr, ok := args[0].(*object.Array); ok { - for _, a := range arr.Elems { - vm.setInstanceVisibility(mod, nameArg(a), vis) - } - return args[0] - } - } - for _, a := range args { + for _, a := range visNameList(args) { vm.setInstanceVisibility(mod, nameArg(a), vis) } if len(args) == 1 { @@ -159,7 +149,7 @@ func (vm *VM) registerModuleExtras() { // per-receiver override (see setClassMethodVisibility). Returns self, as MRI. classMethodVisibility := func(vm *VM, self object.Value, args []object.Value, vis visibility) object.Value { mod := self.(*RClass) - for _, a := range args { + for _, a := range visNameList(args) { vm.setClassMethodVisibility(mod, nameArg(a), vis) } return self @@ -615,3 +605,21 @@ func allModuleArgs(args []object.Value) bool { } return true } + +// visNameList expands the argument list of a visibility directive into the +// method names it names. MRI's set_method_visibility (ruby/ruby v3_4_0 +// vm_method.c:2400) treats a SINGLE Array argument as the list itself — +// `private [:a, :b]` and `private_class_method [:foo]` mark each element — +// and any other shape as a varargs name list. It is one rule serving every +// directive that routes through set_method_visibility: private / public / +// protected (through set_visibility) and private_class_method / +// public_class_method (rb_mod_private_method / rb_mod_public_method), so it +// lives here once rather than at each call site. +func visNameList(args []object.Value) []object.Value { + if len(args) == 1 { + if arr, ok := args[0].(*object.Array); ok { + return arr.Elems + } + } + return args +} diff --git a/internal/vm/visibility.go b/internal/vm/visibility.go index b52b2ddd..9546e3f7 100644 --- a/internal/vm/visibility.go +++ b/internal/vm/visibility.go @@ -71,20 +71,35 @@ func (vm *VM) setInstanceVisibility(mod *RClass, name string, vis visibility) { vm.setClassMethodVisibility(mod.metaOf, name, vis) return } + // rb_export_method wraps everything after its lookup in + // `if (METHOD_ENTRY_VISI(me) != visi)`: a directive that asks for the level + // the method ALREADY has does nothing at all — it neither rewrites the entry + // nor adds a ZSUPER entry to the receiver. That is why + // `child.private_instance_methods(false)` does NOT list a method an included + // module had already made private. Reference: ruby/ruby v3_4_0 vm_method.c + // rb_export_method. if own, ok := mod.methods[name]; ok && !own.undefined { + if own.vis == vis { + return + } own.vis = vis bumpMethodSerial() return } - if vm.lookupForModuleOp(mod, name) == nil { + inherited := vm.lookupForModuleOp(mod, name) + if inherited == nil && mod.isModule { // A module whose ancestors do not carry the method falls back to Object: // MRI's rb_export_method searches rb_cObject when the receiver is a module, // so `Module.new { private :some_object_method }` records the override on // the module rather than raising. Reference: ruby/ruby v3_4_0 vm_method.c // rb_export_method. - if !mod.isModule || vm.lookupForModuleOp(vm.cObject, name) == nil { - vm.raiseNameError("undefined method '"+name+"' for "+vm.moduleDescription(mod), name) - } + inherited = vm.lookupForModuleOp(vm.cObject, name) + } + if inherited == nil { + vm.raiseNameError("undefined method '"+name+"' for "+vm.moduleDescription(mod), name) + } + if !inherited.undefined && instanceVisibility(mod, name, inherited) == vis { + return } if mod.visOverrides == nil { mod.visOverrides = map[string]visibility{} From 50576ec1f353cab958ab0537141b3985f5bb2960 Mon Sep 17 00:00:00 2001 From: tannevaled Date: Fri, 25 Sep 2026 10:25:07 +0200 Subject: [PATCH 2/6] vm: Module#dup and #clone copy the module, and const_source_location searches like MRI MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Two independent fixes in the Module reflection surface. **Module#dup returned the receiver.** dupValue (builtins.go) has no *RClass case, so `m.dup.equal?(m)` was true for every module and class: a later const_set or def on the "copy" landed on the original, and `Module.new.dup` shared one constant table with its source. MRI allocates a fresh module and fills it in Module#initialize_copy — rb_mod_init_copy, ruby/ruby v3_4_0 class.c — so that is what this adds: cModule gets #initialize_copy (the table copy), #dup and #clone (allocate, run the ordinary copy hooks, then frozen state for clone). The copy takes the superclass, a method table whose entries are re-owned so `super` still walks from it, the singleton table, and copies of the constant, class-variable, instance-variable, autoload, visibility-override and constant-visibility tables. It does NOT take the name: MRI never copies the classpath, which is why Named.dup.name is nil. Two details cost a spec each until they were in. A class that has been `extend`ed carries the mixin on its METACLASS, which MRI copies with rb_singleton_class_clone (class.c) — without that, `Struct.new(:foo).extend(M).dup.hello` raised NoMethodError. And a Struct/Data subclass needs its member layout carried over, MRI's RCLASS_SET_ALLOCATOR, or `Struct.new(:a).dup.new(1)` builds a bare object. class_init_copy_check's two refusals are honoured: BasicObject ("can't copy the root class") and any singleton class ("can't copy singleton class"). **Module#const_source_location answered nil for everything.** The search is now MRI's (object.c rb_mod_const_source_location → variable.c rb_const_location / rb_const_location_from): a String name may carry an "A::B" path and a leading "::", every segment but the last is resolved with const_get and must name a class or module, and the inherit flag is spent on the FIRST segment (`recur = Qfalse` at the foot of the loop), so "ClassA::CS_CONST10" reads ClassA's own table. With inherit the walk covers the whole ancestor chain, Object included — that is how a class receiver reports a toplevel constant — and a module receiver, whose chain never reaches Object, falls back to it explicitly. A pending autoload counts as an entry, as MRI reserves the name with an undefined value. rbgo records no definition site for a constant yet: the one place a Ruby-level assignment lands is vm.assignConstIn, in a file this wave does not own. So a constant that resolves answers [] — MRI's answer for one defined in C, and exactly right for every constant this VM defines in Go. Every remaining failure in const_source_location_spec is now literally "expected [] to == [path, line]": the search is settled, only the table is missing. Known divergence, left alone: MRI returns nil rather than raising for an intermediate path segment whose name was never interned as an ID (rb_check_id gives 0 in rb_mod_const_source_location), so `Object.const_source_location("NOPEX::Foo")` is nil there and NameError here. rbgo has no symbol table to consult, and const_get already raises for the same shape. ruby/spec: core/module/const_source_location 12->13, core/module/remove_class_variable 7->8, core/class/dup 3->6, core/struct/dup and clone unchanged at 2 and 1. Co-Authored-By: Claude Opus 5 --- internal/vm/module_residuals.go | 335 ++++++++++++++++++++++++++++-- internal/vm/module_wave30_test.go | 241 +++++++++++++++++++++ 2 files changed, 563 insertions(+), 13 deletions(-) diff --git a/internal/vm/module_residuals.go b/internal/vm/module_residuals.go index 422e2120..f8e4decf 100644 --- a/internal/vm/module_residuals.go +++ b/internal/vm/module_residuals.go @@ -44,23 +44,57 @@ func (vm *VM) registerModuleResiduals() { return object.Bool(vm.moduleConstDefined(self.(*RClass), args[0], inherit)) }) - // Module#const_source_location(name, inherit=true): in MRI this returns the - // [file, line] where the constant was defined, [] for a constant defined in C, - // or nil when the name resolves to no constant in the search path. rbgo does - // not record where each constant was defined, so a locatable constant cannot - // report its position — we return nil (the same value MRI gives an absent - // constant) after performing MRI's full name validation, so the NameError / - // TypeError contract (a lowercase or malformed name, a Symbol carrying a scope - // path, an empty path segment, or a #to_str that fails/returns non-String) is - // honoured exactly even though the location itself is unavailable. inherit is - // accepted for signature compatibility. Reference: ruby/ruby v3_4_0 variable.c - // rb_const_source_location / rb_const_source_location_at. + // Module#const_source_location(name, inherit=true) answers where a constant + // was defined: [file, line], [] for a constant defined in C — MRI's + // rb_const_location_from returns rb_ary_new() when the entry's file is nil — + // or nil when the name resolves to no constant in the search path. + // + // The SEARCH is MRI's exactly (ruby/ruby v3_4_0 object.c + // rb_mod_const_source_location → variable.c rb_const_source_location / + // rb_const_source_location_at → rb_const_location): a String may carry a + // "A::B" path and a leading "::" toplevel qualifier, every segment but the + // last is resolved with const_get and must name a class or module, and the + // inherit flag is consumed by the FIRST segment only (`recur = Qfalse` at the + // foot of the loop), so "ClassA::CS_CONST10" reads ClassA's OWN table. With + // inherit the walk covers the whole ancestor chain — Object included, which + // is how a class receiver reports a toplevel constant — and a module + // receiver, whose chain never reaches Object, falls back to it explicitly. + // Without inherit only the receiver's own table is read, and Object is + // excluded unless it IS the receiver. + // + // rbgo records no definition site for a constant yet: the single place a + // Ruby-level assignment lands is vm.assignConstIn, and that file belongs to + // another wave. So every constant that RESOLVES answers [] — exactly right + // for the ones this VM defines in Go, which is what MRI's "defined in C + // code" means, and a known gap for the rest. What is decided here is the + // nil/[]/NameError contract and the search itself, which no location table + // would change. vm.cModule.define("const_source_location", func(vm *VM, self object.Value, args []object.Value, _ *Proc) object.Value { - segs, _, orig := vm.constPathSegs(args[0]) - for _, seg := range segs { + if len(args) < 1 || len(args) > 2 { + raise("ArgumentError", "wrong number of arguments (given %d, expected 1..2)", len(args)) + } + mod := self.(*RClass) + recur := len(args) < 2 || args[1].Truthy() + segs, topLevel, orig := vm.constPathSegs(args[0]) + if topLevel { + mod = vm.cObject + } + for i, seg := range segs { if !constNameWellFormed(seg) { raise("NameError", "wrong constant name %s", orig) } + if i == len(segs)-1 { + return vm.constLocation(mod, seg, !recur, recur) + } + v, ok := vm.constSegGet(mod, seg, recur, i == 0) + if !ok { + v = vm.constMissing(mod, seg) // raises NameError by default + } + nc, isCls := v.(*RClass) + if !isCls { + raise("TypeError", "%s does not refer to class/module", orig) + } + mod, recur = nc, false } return object.NilV }) @@ -94,6 +128,79 @@ func (vm *VM) registerModuleResiduals() { return object.NewArrayFromSlice(out) }) + // Module#initialize_copy(orig): the table copy behind Module#dup / #clone. + // MRI's rb_mod_init_copy (ruby/ruby v3_4_0 class.c) gives the copy orig's + // superclass, a fresh method table whose entries are re-owned by the copy + // (clone_method_i), copies of the constant, class-variable and + // instance-variable tables (copy_tables), and a cloned singleton class + // (rb_singleton_class_clone) — but NOT orig's name: a copy is anonymous until + // something binds it to a constant, which is why `Named.dup.name` is nil. + // + // rbgo had no Module case at all in dupValue, so Object#dup returned the + // receiver ITSELF for a class or module: `m.dup.equal?(m)` was true and every + // later const_set or def landed on the original. + vm.cModule.define("initialize_copy", func(vm *VM, self object.Value, args []object.Value, _ *Proc) object.Value { + dst := self.(*RClass) + src, ok := args[0].(*RClass) + if !ok { + raise("TypeError", "initialize_copy should take same class object") + } + if dst == src { // copying from itself does nothing, as Kernel#initialize_copy + return dst + } + // class_init_copy_check (class.c): BasicObject and any singleton class + // refuse to be copied. A singleton class is tied to the one object it is + // attached to, so a second one would be a contradiction. + if src == vm.cBasicObject { + raise("TypeError", "can't copy the root class") + } + if src.isSingleton { + raise("TypeError", "can't copy singleton class") + } + if dst.frozen { + vm.raiseFrozen(dst) + } + copyModuleTables(dst, src) + return dst + }) + + // Module#dup / #clone. They allocate a bare module or class of the receiver's + // kind and run the ordinary copy hooks on it, so a user #initialize_copy + // override still governs what is carried over. dup leaves the copy unfrozen; + // clone copies the frozen state and honours clone(freeze: true/false). + // Reference: ruby/ruby v3_4_0 object.c rb_obj_dup / rb_obj_clone2. + allocCopy := func(src *RClass) *RClass { + cp := newClass("", nil) + cp.isModule = src.isModule + return cp + } + vm.cModule.define("dup", func(vm *VM, self object.Value, _ []object.Value, _ *Proc) object.Value { + src := self.(*RClass) + cp := allocCopy(src) + vm.send(cp, "initialize_dup", []object.Value{src}, nil) + return cp + }) + vm.cModule.define("clone", func(vm *VM, self object.Value, args []object.Value, _ *Proc) object.Value { + src := self.(*RClass) + freezeVal, freezeGiven := cloneFreezeArg(vm, args) + cp := allocCopy(src) + if freezeGiven { + kw := object.NewHash() + kw.Set(object.Symbol("freeze"), freezeVal) + vm.send(cp, "initialize_clone", []object.Value{src, kw}, nil) + } else { + vm.send(cp, "initialize_clone", []object.Value{src}, nil) + } + doFreeze := src.frozen + if b, ok := freezeVal.(object.Bool); ok { + doFreeze = bool(b) + } + if doFreeze { + vm.send(cp, "freeze", nil, nil) + } + return cp + }) + // Module#remove_class_variable(sym): remove a class variable defined DIRECTLY // on the receiver (never one inherited or mixed in) and return its value, // raising NameError when the name is malformed or the variable is absent. @@ -354,3 +461,205 @@ func isConstantPath(s string) bool { } return true } + +// copyModuleTables gives dst the contents of src, as MRI's rb_mod_init_copy does +// through copy_tables + clone_method_i (ruby/ruby v3_4_0 class.c): the +// superclass and mixin chain, a method table whose entries are re-owned by dst +// so `super` still walks from it, the singleton (class-method) table, and the +// constant / class-variable / instance-variable tables. Each map is REBUILT, not +// aliased — that independence is the whole point of a copy. +// +// Deliberately not carried over: the name (a copy is anonymous — MRI never +// copies the classpath, so Named.dup.name is nil), the lexical parent that goes +// with it, the frozen flag (Module#clone sets it separately, Module#dup must +// not), the singleton-class identity (isSingleton / metaOf / attached / meta: +// the copy gets its own, minted lazily from the smethods copied here), and the +// transient class-body state (defaultVis, funcMode) which belongs to a body +// being executed rather than to the module. +func copyModuleTables(dst, src *RClass) { + dst.super = src.super + dst.isModule = src.isModule + dst.methods = make(map[string]*Method, len(src.methods)) + for n, m := range src.methods { + cp := *m + cp.owner = dst + dst.methods[n] = &cp + } + dst.smethods = make(map[string]*Method, len(src.smethods)) + for n, m := range src.smethods { + cp := *m + cp.owner = dst + dst.smethods[n] = &cp + } + // A class that has been `extend`ed carries the mixin on its METACLASS rather + // than in the singleton method table, and `class << self` constants live + // there too. MRI copies the whole thing with rb_singleton_class_clone + // (class.c), which is what keeps Struct.new(:a).extend(M).dup.hello working. + if src.meta != nil { + dm := dst.metaClass() + dm.methods = dst.smethods // keep the metaclass aliasing the COPY's table + dm.includes = append([]*RClass(nil), src.meta.includes...) + dm.prepends = append([]*RClass(nil), src.meta.prepends...) + dm.consts = copyValueTable(src.meta.consts) + dm.cvars = copyValueTable(src.meta.cvars) + dm.ivars = copyValueTable(src.meta.ivars) + dm.visOverrides = copyVisTable(src.meta.visOverrides) + } + dst.consts = copyValueTable(src.consts) + dst.cvars = copyValueTable(src.cvars) + dst.ivars = copyValueTable(src.ivars) + dst.includes = append([]*RClass(nil), src.includes...) + dst.prepends = append([]*RClass(nil), src.prepends...) + dst.autoloads = copyStringTable(src.autoloads) + dst.deprecatedConsts = copyBoolTable(src.deprecatedConsts) + dst.privateConsts = copyBoolTable(src.privateConsts) + dst.visOverrides = copyVisTable(src.visOverrides) + dst.svisOverrides = copyVisTable(src.svisOverrides) + // A Struct/Data subclass keeps its member layout: MRI carries the allocator + // over with RCLASS_SET_ALLOCATOR, which is what makes Struct.new(:a).dup.new(1) + // still build a two-slot Struct rather than a bare object. + dst.structDef = src.structDef + dst.dataDef = src.dataDef + bumpMethodSerial() +} + +// copyValueTable returns a fresh map with the same entries, or nil for a nil +// source (the tables on RClass are created lazily and nil is meaningful). +func copyValueTable(src map[string]object.Value) map[string]object.Value { + if src == nil { + return nil + } + out := make(map[string]object.Value, len(src)) + for k, v := range src { + out[k] = v + } + return out +} + +// copyStringTable is copyValueTable for the autoload path table. +func copyStringTable(src map[string]string) map[string]string { + if src == nil { + return nil + } + out := make(map[string]string, len(src)) + for k, v := range src { + out[k] = v + } + return out +} + +// copyBoolTable is copyValueTable for the deprecated/private constant sets. +func copyBoolTable(src map[string]bool) map[string]bool { + if src == nil { + return nil + } + out := make(map[string]bool, len(src)) + for k, v := range src { + out[k] = v + } + return out +} + +// copyVisTable is copyValueTable for the per-receiver visibility overrides. +func copyVisTable(src map[string]visibility) map[string]visibility { + if src == nil { + return nil + } + out := make(map[string]visibility, len(src)) + for k, v := range src { + out[k] = v + } + return out +} + +// cloneFreezeArg reads the freeze: keyword of Module#clone, returning its value +// and whether it was given. MRI accepts only true, false or nil there +// (object.c rb_obj_clone2), and rejects any other keyword outright. +func cloneFreezeArg(vm *VM, args []object.Value) (object.Value, bool) { + if len(args) == 0 { + return object.NilV, false + } + h, ok := args[len(args)-1].(*object.Hash) + if !ok { + return object.NilV, false + } + var val object.Value = object.NilV + given := false + for _, k := range h.Keys { + if sym, isSym := k.(object.Symbol); isSym && sym == object.Symbol("freeze") { + val, _ = h.Get(k) + given = true + continue + } + raise("ArgumentError", "unknown keyword: %s", k.Inspect()) + } + if given { + switch val.(type) { + case object.Bool, object.Nil: + default: + raise("ArgumentError", "unexpected value for freeze: %s", vm.classOf(val).name) + } + } + return val, given +} + +// constLocation is MRI's rb_const_location (ruby/ruby v3_4_0 variable.c): search +// cls, then — when the search may leave the receiver and cls is a MODULE, whose +// ancestor chain never reaches Object — search Object as well. Object itself is +// never excluded from its own search. exclude/recurse are the pair +// const_source_location (false, true) and const_source_location_at (true, false) +// pass. It answers Ruby nil when nothing is found. +func (vm *VM) constLocation(cls *RClass, name string, exclude, recurse bool) object.Value { + if cls == vm.cObject { + exclude = false + } + if loc, ok := vm.constLocationFrom(cls, name, recurse); ok { + return loc + } + if exclude || !cls.isModule { + return object.NilV + } + if loc, ok := vm.constLocationFrom(vm.cObject, name, recurse); ok { + return loc + } + return object.NilV +} + +// constLocationFrom is MRI's rb_const_location_from: walk cls (and, with +// recurse, its ancestors) for an entry named name. A pending autoload counts as +// an entry — MRI reserves the name with an undefined value, and +// rb_const_location_from finds that entry like any other. +// +// MRI's loop carries one more guard, `if (exclude && klass == rb_cObject) goto +// not_found`. It is not reproduced because neither entry point can reach it: +// exclude is only ever set together with recurse == false, which leaves cls the +// one class walked, and a cls that IS Object has exclude cleared by +// rb_const_location before the walk. Keeping the arm would be a branch no +// argument list can take. +// +// The location itself is always the empty array here: see the note on +// Module#const_source_location. The second result distinguishes "found" from +// "no such constant", which is the difference between [] and nil. +func (vm *VM) constLocationFrom(cls *RClass, name string, recurse bool) (object.Value, bool) { + chain := []*RClass{cls} + if recurse { + chain = vm.ancestors(cls) + } + for _, k := range chain { + if constEntryPresent(k, name) { + return object.NewArrayFromSlice(nil), true + } + } + return nil, false +} + +// constEntryPresent reports whether k's own constant table holds an entry for +// name — a defined constant or a still-pending autoload, which MRI stores as an +// entry with an undefined value. +func constEntryPresent(k *RClass, name string) bool { + if _, ok := k.consts[name]; ok { + return true + } + _, pending := k.autoloads[name] + return pending +} diff --git a/internal/vm/module_wave30_test.go b/internal/vm/module_wave30_test.go index 9d1961fc..53070ad2 100644 --- a/internal/vm/module_wave30_test.go +++ b/internal/vm/module_wave30_test.go @@ -159,3 +159,244 @@ p m.private_instance_methods(false)`); got != "[]\n" { t.Errorf("Object fallback, same level: got %q", got) } } + +// TestModuleWave30ModuleDup covers Module#dup / #clone / #initialize_copy. +// rbgo's dupValue has no class case, so Object#dup used to hand back the +// RECEIVER for a module or class; MRI's rb_mod_init_copy (ruby/ruby v3_4_0 +// class.c) allocates a fresh one and copies its tables. Asserted against MRI 4.0. +func TestModuleWave30ModuleDup(t *testing.T) { + cases := []struct{ src, want string }{ + // A copy is a DIFFERENT object, for a module and for a class. + {`m = Module.new; p m.dup.equal?(m)`, "false\n"}, + {`c = Class.new; p c.dup.equal?(c)`, "false\n"}, + {`m = Module.new; p m.clone.equal?(m)`, "false\n"}, + // Its constant table is independent of the original's. + {`m = Module.new +m.const_set(:A, 1) +d = m.dup +m.const_set(:B, 2) +d.const_set(:C, 3) +p [d.const_defined?(:A, false), d.const_defined?(:B, false), m.const_defined?(:C, false)]`, + "[true, false, false]\n"}, + // So is its method table, and a copied method still finds `super`. + {`class W30P; def v; :p; end; end +class W30C < W30P; def v; [:c, super]; end; end +d = W30C.dup +d.send(:define_method, :extra) { :extra } +p [d.new.v, W30C.method_defined?(:extra)]`, "[[:c, :p], false]\n"}, + // Class methods, instance variables and the superclass come across. + {`class W30S + def self.sm; :sm; end + @iv = 7 + def im; :im; end +end +d = W30S.dup +p [d.sm, d.instance_variable_get(:@iv), d.superclass, d.new.im]`, + "[:sm, 7, Object, :im]\n"}, + // The copy is ANONYMOUS: MRI never copies the classpath. + {`module W30Named; end +p W30Named.dup.name`, "nil\n"}, + {`module W30Named2; end +p W30Named2.clone.name`, "nil\n"}, + // The mixin chain comes across. + {`module W30M; def mm; :mm; end; end +k = Class.new { include W30M } +d = k.dup +p [d.new.mm, d.include?(W30M)]`, "[:mm, true]\n"}, + // A prepended module keeps its place ahead of the copy's own method. + {`module W30Pre; def hi; "pre-" + super; end; end +class W30Base; def hi; "base"; end; prepend W30Pre; end +p W30Base.dup.new.hi`, "\"pre-base\"\n"}, + // A module mixed in with #extend lives on the METACLASS, and comes across + // with it (the jruby/jruby#3686 case ruby/spec pins for Struct). + {`k = Struct.new(:foo) +mod = Module.new { def hello; "hello"; end } +k.extend(mod) +p k.dup.hello`, "\"hello\"\n"}, + // A Struct subclass keeps its member layout (MRI carries the allocator). + {`s = Struct.new(:a, :b).dup +x = s.new(1, 2) +p [x.a, x.b, s.members]`, "[1, 2, [:a, :b]]\n"}, + // A Data subclass too. + {`d = Data.define(:q).dup +p d.new(q: 5).q`, "5\n"}, + // A pending autoload is shared with the copy (ruby/spec relies on this). + {`m = Module.new +m.send(:autoload, :T, "/nonexistent/w30.rb") +p m.dup.autoload?(:T)`, "\"/nonexistent/w30.rb\"\n"}, + // Visibility overrides and private-constant marks come across. + {`parent = Module.new { def tm; end } +m = Module.new +m.send(:include, parent) +m.send(:private, :tm) +m.const_set(:X, 1) +m.send(:private_constant, :X) +d = m.dup +p [d.private_instance_methods(false), (begin; d::X; rescue NameError; :priv; end)]`, + "[[:tm], :priv]\n"}, + // dup leaves the copy unfrozen; clone copies the frozen state, and + // freeze: overrides it either way. + {`f = Class.new.freeze +p [f.dup.frozen?, f.clone.frozen?, f.clone(freeze: false).frozen?]`, + "[false, true, false]\n"}, + {`c = Class.new +p [c.clone.frozen?, c.clone(freeze: true).frozen?, c.clone(freeze: nil).frozen?]`, + "[false, true, false]\n"}, + // initialize_copy from the receiver itself is inert. + {`m = Module.new +p m.send(:initialize_copy, m).equal?(m)`, "true\n"}, + } + for _, c := range cases { + if got := eval(t, c.src); got != c.want { + t.Errorf("src=%q\n got=%q\nwant=%q", c.src, got, c.want) + } + } + // A singleton class refuses to be copied, through dup and through clone … + for _, src := range []string{ + `Class.new.singleton_class.dup`, + `Class.new.singleton_class.clone`, + `Object.new.singleton_class.dup`, + } { + if err := runErr(t, src); err == nil || !strings.Contains(err.Error(), "can't copy singleton class") { + t.Errorf("%s: got %v, want TypeError can't copy singleton class", src, err) + } + } + // … and so does BasicObject, the root class. + if err := runErr(t, `BasicObject.dup`); err == nil || + !strings.Contains(err.Error(), "can't copy the root class") { + t.Errorf("BasicObject.dup: got %v, want TypeError can't copy the root class", err) + } + // initialize_copy needs a module/class argument … + if err := runErr(t, `Module.new.send(:initialize_copy, 1)`); err == nil || + !strings.Contains(err.Error(), "initialize_copy should take same class object") { + t.Errorf("non-module original: got %v, want TypeError", err) + } + // … and refuses to write into a frozen receiver. + if err := runErr(t, `Module.new.freeze.send(:initialize_copy, Module.new)`); err == nil || + !strings.Contains(err.Error(), "FrozenError") { + t.Errorf("frozen receiver: got %v, want FrozenError", err) + } + // clone rejects an unknown keyword and a non-boolean freeze:. + if err := runErr(t, `Class.new.clone(nope: 1)`); err == nil || + !strings.Contains(err.Error(), "unknown keyword") { + t.Errorf("unknown keyword: got %v, want ArgumentError", err) + } + if err := runErr(t, `Class.new.clone(freeze: 1)`); err == nil || + !strings.Contains(err.Error(), "unexpected value for freeze") { + t.Errorf("bad freeze value: got %v, want ArgumentError", err) + } +} + +// TestModuleWave30ConstSourceLocation covers Module#const_source_location's +// SEARCH and its nil / [] / NameError contract — MRI's rb_mod_const_source_location +// (ruby/ruby v3_4_0 object.c) over rb_const_location (variable.c). rbgo records +// no definition site yet, so a constant that resolves answers [] (MRI's answer +// for one defined in C); what is asserted here is which names resolve at all. +// Asserted against MRI 4.0, comparing nil / empty / non-nil rather than the +// path. +func TestModuleWave30ConstSourceLocation(t *testing.T) { + // A constant this VM defines in Go answers [], exactly as MRI does for one + // defined in C. + cases := []struct{ src, want string }{ + {`p Object.const_source_location(:String)`, "[]\n"}, + // An absent name is nil, a present one is not. + {`module W30A; K = 1; end +p [W30A.const_source_location(:K), W30A.const_source_location(:NOPE)]`, "[[], nil]\n"}, + // With inherit (the default) a class reads its superclass … + {`class W30SP; P1 = 1; end +class W30SC < W30SP; end +p [W30SC.const_source_location(:P1), W30SC.const_source_location(:P1, false)]`, + "[[], nil]\n"}, + // … and Object, which is how a class receiver reports a toplevel constant. + {`W30TOP = 1 +class W30SC2; end +p [W30SC2.const_source_location(:W30TOP), W30SC2.const_source_location(:W30TOP, false)]`, + "[[], nil]\n"}, + // A MODULE receiver never reaches Object through its ancestors, so MRI + // searches Object explicitly — but only when inherit is set. + {`W30TOP2 = 1 +module W30MM; end +p [W30MM.const_source_location(:W30TOP2), W30MM.const_source_location(:W30TOP2, false)]`, + "[[], nil]\n"}, + // An included module is on the search path. + {`module W30Mix; MX = 1; end +class W30Host; include W30Mix; end +p W30Host.const_source_location(:MX)`, "[]\n"}, + // A leading "::" reads the top level. + {`W30TOP3 = 1 +module W30MM3; end +p W30MM3.const_source_location("::W30TOP3")`, "[]\n"}, + // A scoped path resolves every segment but the last with const_get, and + // the inherit flag is spent on the FIRST segment: the last one reads the + // reached module's OWN table, so an inherited name there is nil. + {`module W30Outer + module W30Inner; J = 2; end +end +class W30IP; IPC = 1; end +class W30IC < W30IP; end +p [W30Outer.const_source_location("W30Inner::J"), Object.const_source_location("W30IC::IPC")]`, + "[[], nil]\n"}, + // The singleton class of a class is NOT searched. + {`class W30S2; class << self; SC = 1; end; end +p W30S2.const_source_location(:SC)`, "nil\n"}, + // Nor is the lexically containing scope. + {`module W30Cont + CONT = 1 + class W30Child; end +end +p W30Cont::W30Child.const_source_location(:CONT)`, "nil\n"}, + // A private constant IS reported: rb_const_location passes visibility=0. + {`module W30Priv; end +W30Priv.const_set(:PC, 1) +W30Priv.send(:private_constant, :PC) +p W30Priv.const_source_location(:PC)`, "[]\n"}, + // A pending autoload counts as an entry (MRI reserves the name). + {`module W30Auto; end +W30Auto.send(:autoload, :LZ, "/nonexistent/w30auto.rb") +p W30Auto.const_source_location(:LZ)`, "[]\n"}, + // The name is coerced through #to_str. + {`module W30Str; SK = 1; end +n = Object.new +def n.to_str = "SK" +p W30Str.const_source_location(n)`, "[]\n"}, + } + for _, c := range cases { + if got := eval(t, c.src); got != c.want { + t.Errorf("src=%q\n got=%q\nwant=%q", c.src, got, c.want) + } + } + // A leading segment that names a non-module is a TypeError, as in + // rb_mod_const_source_location's "does not refer to class/module". + if err := runErr(t, `W30NOTMOD = 1 +Object.const_source_location("W30NOTMOD::X")`); err == nil || + !strings.Contains(err.Error(), "does not refer to class/module") { + t.Errorf("non-module segment: got %v, want TypeError", err) + } + // Every malformed name MRI rejects. + for _, src := range []string{ + `Module.const_source_location "name"`, + `Module.const_source_location "__CONSTX__"`, + `Module.const_source_location "@K"`, + `Module.const_source_location "!K"`, + `Module.const_source_location "K="`, + `Module.const_source_location "K?"`, + `Module.const_source_location :"::K"`, + `Module.const_source_location :"A::B"`, + `Module.const_source_location "::"`, + } { + if err := runErr(t, src); err == nil || !strings.Contains(err.Error(), "NameError") { + t.Errorf("%s: got %v, want NameError", src, err) + } + } + // A non-name argument is a TypeError … + if err := runErr(t, `Module.const_source_location(123)`); err == nil || + !strings.Contains(err.Error(), "TypeError") { + t.Errorf("Integer name: got %v, want TypeError", err) + } + // … and the arity is 1..2 (rb_check_arity). + for _, src := range []string{`Module.const_source_location`, `Module.const_source_location(:K, true, 3)`} { + if err := runErr(t, src); err == nil || !strings.Contains(err.Error(), "ArgumentError") { + t.Errorf("%s: got %v, want ArgumentError", src, err) + } + } +} From aa61be94ed2d0e4c15618ce639f777c16b24ce0b Mon Sep 17 00:00:00 2001 From: tannevaled Date: Fri, 25 Sep 2026 10:40:16 +0200 Subject: [PATCH 3/6] vm: Module#ruby2_keywords, and a constant name is decided by bytes not runes MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit **Module#ruby2_keywords did not exist.** core/module/ruby2_keywords_spec.rb scored 0 of 14 because its very first line — `ruby2_keywords def mark(*args)` in a `class << self` body — raised NoMethodError and took the whole file down. MRI's rb_mod_ruby2_keywords (ruby/ruby v3_4_0 vm_method.c:2568) checks arity (1+) and the receiver's frozen state, then per name coerces it through rb_check_id (TypeError for anything but a Symbol/String/#to_str), resolves it on the receiver falling back to Object for a module receiver, and raises NameError when it resolves nowhere. Three shapes WARN through rb_warn instead of raising: a name the receiver does not define itself ("can only set in method defining module"), one whose body is not Ruby ("method not defined in Ruby"), and one whose parameters cannot carry the flag ("method accepts keywords or post arguments or method does not accept argument splat" — 4.0 names post arguments there, 3.4 did not, and 4.0 refuses a post-splat positional). It returns nil whatever happened. The flag itself is NOT carried: MRI writes ISEQ_BODY(...)->param.flags.ruby2_keywords, which bytecode.ISeq has no field for, and honouring it belongs to the splat binding — both in another wave's files. This is the same partial contract Proc#ruby2_keywords already ships in this VM, and it is stated as such in the code. The eight examples still failing are all "expected false to == true" from Hash.ruby2_keywords_hash?: the contract is settled, only the flag is missing. 0 -> 6. **A constant name was decided on decoded runes.** rbgo asked unicode.IsLetter of each rune after the first. MRI asks is_identchar of each BYTE — `ISALNUM(*p) || *p == '_' || !ISASCII(*p)` (symbol.c:53) — so any non-ASCII byte is an identifier byte with no character test at all. That is why "A€" and "A B" are constant names in MRI, and why ruby/spec can set one with "CS_CONSTλ".encode("euc-jp"): bytes that are not valid UTF-8 decoded to U+FFFD here, which is not a letter, so const_set refused the name outright. The leading character now follows rb_sym_constant_char_p: ASCII-uppercase, or uppercase or TITLECASE for a multi-byte one — "DŽx" (U+01C4) and "Džx" (U+01C5) are constants, "džx" (U+01C6) is not. Every constant entry point shares this test, so const_set, const_get, const_defined?, remove_const, autoload and const_source_location all move together. core/module/const_defined? 30 -> 31, its MRI ceiling. Known divergence: MRI raises EncodingError for a string whose bytes are invalid in its OWN encoding ("X\xFF" tagged UTF-8). rbgo sees only bytes here — EUC-JP text is invalid UTF-8 too — so it cannot tell the two apart and accepts both. ruby/spec core/module: ruby2_keywords 0 -> 6, const_defined? 30 -> 31. Co-Authored-By: Claude Opus 5 --- internal/vm/module_residuals.go | 54 +++++++-- internal/vm/module_wave30_test.go | 185 ++++++++++++++++++++++++++++++ internal/vm/modulefunc.go | 65 +++++++++++ 3 files changed, 297 insertions(+), 7 deletions(-) diff --git a/internal/vm/module_residuals.go b/internal/vm/module_residuals.go index f8e4decf..9bc44a74 100644 --- a/internal/vm/module_residuals.go +++ b/internal/vm/module_residuals.go @@ -7,6 +7,7 @@ package vm import ( "strings" "unicode" + "unicode/utf8" "github.com/go-embedded-ruby/ruby/internal/object" ) @@ -419,18 +420,57 @@ func hasAutoload(c *RClass, name string) bool { return ok } -// validConstName reports whether s is a well-formed constant name: an uppercase -// first letter followed by letters, digits or underscores. Used to reject names -// like "name", "__X__", "X=" or "X?" as MRI's const_get/const_defined? do. +// constNameWellFormed reports whether s is a well-formed constant name, as MRI +// decides it in rb_enc_symname_type (ruby/ruby v3_4_0 symbol.c): a leading +// character rb_sym_constant_char_p calls a constant character, followed by +// is_identchar characters up to the end of the string. Used to reject "name", +// "__X__", "X=" or "X?" in const_get / const_defined? / const_set / autoload. +// +// The two rules are NOT "letters, digits and underscores", which is what rbgo +// had: +// +// - The leading character is ASCII-uppercase, or — for a multi-byte one — +// uppercase or TITLECASE in its encoding (rb_sym_constant_char_p falls +// through to the titlecase ctype for Unicode). So "DŽx" (U+01C4) and "Džx" +// (U+01C5, titlecase) are constants while "džx" (U+01C6) is not. +// - Every LATER byte passes is_identchar, which is +// `ISALNUM(*p) || *p == '_' || !ISASCII(*p)`: any non-ASCII byte is an +// identifier byte, with no character test at all. That is why "A€" and +// "A\u00A0B" are constant names in MRI, and why a name in a non-UTF-8 +// ASCII-compatible encoding — ruby/spec sets one with +// "CS_CONSTλ".encode("euc-jp") — is one too. Testing the decoded RUNE, as +// rbgo did, rejected all three: bytes that are not valid UTF-8 decode to +// U+FFFD, which is not a letter. +// +// Known divergence: MRI raises EncodingError for a string whose bytes are +// invalid in its OWN encoding ("X\xFF" tagged UTF-8). rbgo sees only the bytes +// here — EUC-JP text is invalid UTF-8 too — so it cannot tell the two apart and +// accepts both. func constNameWellFormed(s string) bool { - r := []rune(s) - if len(r) == 0 || !unicode.IsUpper(r[0]) { + if s == "" { return false } - for _, c := range r[1:] { - if c != '_' && !unicode.IsLetter(c) && !unicode.IsDigit(c) { + lead, size := utf8.DecodeRuneInString(s) + switch { + case size == 1 && s[0] < utf8.RuneSelf: + if s[0] < 'A' || s[0] > 'Z' { return false } + case lead == utf8.RuneError && size == 1: + // A byte that starts no UTF-8 character: the name is in some other + // ASCII-compatible encoding, whose multi-byte characters are neither + // uppercase nor titlecase for rb_sym_constant_char_p. + return false + case !unicode.IsUpper(lead) && !unicode.IsTitle(lead): + return false + } + for i := size; i < len(s); i++ { + c := s[i] + if c >= utf8.RuneSelf || c == '_' || ('0' <= c && c <= '9') || + ('a' <= c && c <= 'z') || ('A' <= c && c <= 'Z') { + continue + } + return false } return true } diff --git a/internal/vm/module_wave30_test.go b/internal/vm/module_wave30_test.go index 53070ad2..0c7307a6 100644 --- a/internal/vm/module_wave30_test.go +++ b/internal/vm/module_wave30_test.go @@ -5,8 +5,10 @@ package vm_test import ( + "fmt" "strings" "testing" + "unicode/utf8" ) // TestModuleWave30VisibilityArrayArgument covers MRI's single-Array form of @@ -400,3 +402,186 @@ Object.const_source_location("W30NOTMOD::X")`); err == nil || } } } + +// TestModuleWave30ConstantNameShape covers the constant-name test every +// constant entry point shares — const_set / const_get / const_defined? / +// remove_const / autoload / const_source_location. It is MRI's +// rb_enc_symname_type (ruby/ruby v3_4_0 symbol.c): rb_sym_constant_char_p for +// the leading character, is_identchar for every byte after it. Asserted against +// MRI 4.0. +func TestModuleWave30ConstantNameShape(t *testing.T) { + // Names MRI accepts. Each is const_set on a fresh module and read back. + for _, name := range []string{ + `CS_A`, // the ordinary shape + `A`, // one character + `A1_b`, // digits, underscore and lowercase after the lead + "Aλ", // a non-ASCII LETTER after the lead + "A€", // a non-ASCII non-letter: is_identchar takes any non-ASCII byte + "A B", // including a non-breaking space + "ΛX", // an uppercase multi-byte lead + "DŽx", // U+01C4 DŽ, uppercase + "Džx", // U+01C5 Dž, TITLECASE — rb_sym_constant_char_p's titlecase arm + } { + src := "m = Module.new\nm.const_set(" + rubyStringLit(name) + ", 7)\np m.const_get(" + rubyStringLit(name) + ")\n" + if got := eval(t, src); got != "7\n" { + t.Errorf("const_set(%q): got %q, want 7", name, got) + } + } + // A name in a non-UTF-8 ASCII-compatible encoding is a name too: its bytes + // are not valid UTF-8, and MRI never decodes them. + if got := eval(t, `m = Module.new +s = "CS_CONSTλ".encode("euc-jp") +m.const_set(s, 7) +p [m.const_get(s), m.const_defined?(s)]`); got != "[7, true]\n" { + t.Errorf("EUC-JP constant name: got %q", got) + } + // Names MRI refuses. + for _, name := range []string{ + `name`, // lowercase lead + `_A`, // underscore lead + `A b`, // a space after the lead + `A-`, // punctuation after the lead + `A=`, // the attrset shape is not a constant + `A?`, + "λX", // a LOWERCASE multi-byte lead + "džx", // U+01C6 dž, lowercase + } { + src := "Module.new.const_set(" + rubyStringLit(name) + ", 7)" + if err := runErr(t, src); err == nil || !strings.Contains(err.Error(), "NameError") { + t.Errorf("const_set(%q): got %v, want NameError", name, err) + } + } + // A multi-byte lead in a non-UTF-8 encoding is not a constant character + // either — nothing MRI calls uppercase lives in those tables. + if err := runErr(t, `Module.new.const_set("λX".encode("euc-jp"), 7)`); err == nil || + !strings.Contains(err.Error(), "NameError") { + t.Errorf("EUC-JP lowercase lead: got %v, want NameError", err) + } + // The empty name. + if err := runErr(t, `Module.new.const_set("", 7)`); err == nil || + !strings.Contains(err.Error(), "NameError") { + t.Errorf("empty name: got %v, want NameError", err) + } +} + +// rubyStringLit renders s as a Ruby double-quoted string literal with every +// non-ASCII byte escaped, so a test source stays plain ASCII whatever the name +// under test contains. +func rubyStringLit(s string) string { + var b strings.Builder + b.WriteByte('"') + for i := 0; i < len(s); i++ { + if c := s[i]; c < utf8.RuneSelf && c != '"' && c != '\\' { + b.WriteByte(c) + } else { + fmt.Fprintf(&b, `\x%02X`, c) + } + } + b.WriteByte('"') + return b.String() +} + +// TestModuleWave30Ruby2Keywords covers Module#ruby2_keywords' MRI-visible +// contract — rb_mod_ruby2_keywords, ruby/ruby v3_4_0 vm_method.c:2568, with +// 4.0's wording for the parameter-shape warning. The flag itself is not carried +// yet (bytecode.ISeq has no param.flags.ruby2_keywords), so what is asserted +// here is everything that does not depend on it. Asserted against MRI 4.0. +func TestModuleWave30Ruby2Keywords(t *testing.T) { + cases := []struct{ src, want string }{ + // It returns nil, and is silent for a method it could mark. + {`o = Object.new +o.singleton_class.class_exec do + def foo(*a) end + p ruby2_keywords(:foo) +end`, "nil\n"}, + // It is a PRIVATE instance method of Module. + {`p Module.private_instance_methods(false).include?(:ruby2_keywords)`, "true\n"}, + {`p (Module.new.ruby2_keywords(:x) rescue $!.class)`, "NoMethodError\n"}, + // A String name is accepted as well as a Symbol. + {`o = Object.new +o.singleton_class.class_exec do + def foo(*a) end + p ruby2_keywords("foo") +end`, "nil\n"}, + // Several names in one call. + {`c = Class.new do + def a(*x) end + def b(*x) end + p ruby2_keywords(:a, :b) +end`, "nil\n"}, + } + for _, c := range cases { + if got := eval(t, c.src); got != c.want { + t.Errorf("src=%q\n got=%q\nwant=%q", c.src, got, c.want) + } + } + // Each shape MRI warns about, with its own wording. $VERBOSE is assigned so + // rb_warn's gate is open, and $stderr is captured. + for _, w := range []struct{ def, want string }{ + // has_rest missing … + {`def foo(a, b, c) end`, "method accepts keywords or post arguments or method does not accept argument splat"}, + // … a required keyword … + {`def foo(*a, b:) end`, "method accepts keywords or post arguments or method does not accept argument splat"}, + // … a keyword splat … + {`def foo(*a, **b) end`, "method accepts keywords or post arguments or method does not accept argument splat"}, + // … and, since 4.0, a post-splat positional. + {`def foo(*a, b) end`, "method accepts keywords or post arguments or method does not accept argument splat"}, + } { + src := `$VERBOSE = false +require "stringio" +$stderr = StringIO.new +c = Class.new do + ` + w.def + ` + ruby2_keywords :foo +end +out = $stderr.string +$stderr = STDERR +p out.include?("Skipping set of ruby2_keywords flag for foo (` + w.want + `)")` + if got := eval(t, src); got != "true\n" { + t.Errorf("warning for %q: got %q", w.def, got) + } + } + // A method whose body is not Ruby (an attr reader) … + if got := eval(t, `$VERBOSE = false +require "stringio" +$stderr = StringIO.new +c = Class.new do + attr_reader :a + ruby2_keywords :a +end +out = $stderr.string +$stderr = STDERR +p out.include?("Skipping set of ruby2_keywords flag for a (method not defined in Ruby)")`); got != "true\n" { + t.Errorf("non-Ruby method warning: got %q", got) + } + // … and one the receiver does not define itself, reached through the + // superclass or (for a module) through Object. + for _, src := range []string{ + `class W30R2KBase; def sp(*a) end; end +class W30R2KSub < W30R2KBase; end +W30R2KSub.class_exec { ruby2_keywords :sp }`, + `Module.new.class_exec { ruby2_keywords :puts }`, + } { + full := `$VERBOSE = false +require "stringio" +$stderr = StringIO.new +` + src + ` +out = $stderr.string +$stderr = STDERR +p out.include?("can only set in method defining module")` + if got := eval(t, full); got != "true\n" { + t.Errorf("inherited-name warning (%s): got %q", src, got) + } + } + // No argument, a bad name, an unknown name and a frozen receiver all raise. + for _, tc := range []struct{ src, want string }{ + {`Module.new.send(:ruby2_keywords)`, "ArgumentError"}, + {`Class.new { ruby2_keywords Object.new }`, "is not a symbol nor a string"}, + {`Class.new { ruby2_keywords :not_existing_w30 }`, "undefined method 'not_existing_w30'"}, + {`Module.new.freeze.send(:ruby2_keywords, :x)`, "FrozenError"}, + } { + if err := runErr(t, tc.src); err == nil || !strings.Contains(err.Error(), tc.want) { + t.Errorf("%s: got %v, want %s", tc.src, err, tc.want) + } + } +} diff --git a/internal/vm/modulefunc.go b/internal/vm/modulefunc.go index 1b84721f..2087b8a0 100644 --- a/internal/vm/modulefunc.go +++ b/internal/vm/modulefunc.go @@ -161,6 +161,71 @@ func (vm *VM) registerModuleExtras() { return classMethodVisibility(vm, self, args, visPublic) }) + // Module#ruby2_keywords(name, ...) marks the named methods so a trailing + // keyword hash passed to them flows through their *rest as a flagged hash. + // MRI's rb_mod_ruby2_keywords (ruby/ruby v3_4_0 vm_method.c:2568) checks the + // arity (1+), the receiver's frozen state, then for each name: coerce it + // (rb_check_id — TypeError for anything but a Symbol/String/#to_str), + // resolve it on the receiver, falling back to Object for a module receiver, + // and raise NameError when it resolves nowhere. A name the receiver does not + // itself define, one whose body is not Ruby, and one whose parameters cannot + // carry the flag each WARN instead of raising, through rb_warn — so the + // warning shows at $VERBOSE == false and is silenced only by nil. It returns + // nil whatever happened. + // + // The flag itself is not carried yet: MRI writes + // ISEQ_BODY(...)->param.flags.ruby2_keywords, which rbgo's bytecode.ISeq has + // no field for, and honouring it belongs to the splat binding. So a markable + // method is accepted silently and behaves as it did before — the same + // partial contract Proc#ruby2_keywords already ships in this VM. What IS + // decided here is every observable that does not depend on the flag: the + // method exists (a program guarded by `respond_to?(:ruby2_keywords, true)` + // no longer dies on NoMethodError), the arity/TypeError/NameError contract, + // and the four warnings. + vm.cModule.define("ruby2_keywords", func(vm *VM, self object.Value, args []object.Value, _ *Proc) object.Value { + mod := self.(*RClass) + if len(args) == 0 { + raise("ArgumentError", "wrong number of arguments (given 0, expected 1+)") + } + if mod.frozen { + vm.raiseFrozen(mod) + } + skip := func(name, why string) { + vm.rbWarn("warning: Skipping set of ruby2_keywords flag for %s (%s)", name, why) + } + for _, a := range args { + name := vm.coerceNameArg(a) + m := vm.lookupForModuleOp(mod, name) + if m == nil && mod.isModule { + m = vm.lookupForModuleOp(vm.cObject, name) + } + if m == nil || m.undefined { + vm.raiseNameError("undefined method '"+name+"' for "+vm.moduleDescription(mod), name) + } + // An own entry that is an `undef` tombstone never reaches here: the + // lookup above finds it first and the UNDEFINED_METHOD_ENTRY_P check + // has already raised. + switch own, isOwn := mod.methods[name]; { + case !isOwn: + // MRI compares the resolved entry's defined_class with the receiver + // (and its origin): a method reached through an ancestor cannot be + // marked from here. + skip(name, "can only set in method defining module") + case methodISeq(own) == nil: + skip(name, "method not defined in Ruby") + case !procRuby2KeywordsMarkable(methodISeq(own)): + // has_rest and neither has_kw nor has_kwrest — and, since Ruby 4.0, + // no post-splat positional either, which is also why 4.0 names post + // arguments in the text where 3.4 did not. + skip(name, "method accepts keywords or post arguments or method does not accept argument splat") + } + } + return object.NilV + }) + // rb_define_private_method(rb_cModule, "ruby2_keywords", ...): it is written + // as a directive in a class or module body, never through a receiver. + vm.setInstanceVisibility(vm.cModule, "ruby2_keywords", visPrivate) + // alias_method: the method form of `alias new old`, returning the new name as // a Symbol (MRI returns a Symbol since 3.0). vm.cModule.define("alias_method", func(vm *VM, self object.Value, args []object.Value, _ *Proc) object.Value { From 162fc9d841ec0d35a4ead1592b17ee3827811908 Mon Sep 17 00:00:00 2001 From: tannevaled Date: Fri, 25 Sep 2026 10:46:18 +0200 Subject: [PATCH 4/6] vm: an autoload entry is settled once its constant is defined, however it got there MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Requiring an autoload's file DIRECTLY left the pending entry behind. tryAutoload clears only the entries it runs itself, so after MZ.autoload :K, path require path # defines MZ::K MZ's autoloads table still named K. Nothing showed while MZ::K existed — every read checks the constant table first — but rbgo then decided "already settled" from $LOADED_FEATURES alone, so restoring $", which ruby/spec does after EVERY example, resurrected an autoload that had already fired: a fresh `autoload :K, path` in the next example reported nil instead of the path. MRI has no such window. const_tbl_update (variable.c) replaces the autoload entry with an ordinary one the moment the constant gets a value, so the entry is gone for good — remove_const and a dropped $" cannot bring it back. rbgo cannot see that instant: a constant assignment lands in vm.assignConstIn, which this wave does not own. So the entry is retired at the first read that finds the constant defined — the same rule, applied lazily — in the three places that consult the table (autoload?, const_defined?/defined?, and the load itself). The window that remains is a read-free stretch between the definition and a remove_const, and it is named in the code. ruby/spec core/module/autoload 62 -> 63 ("the autoload is triggered when the same file is required directly | with a relative path", which failed on the state example 1 had left). Co-Authored-By: Claude Opus 5 --- internal/vm/autoload.go | 36 +++++++++++++++++++++++++- internal/vm/module_wave30_test.go | 42 +++++++++++++++++++++++++++++++ 2 files changed, 77 insertions(+), 1 deletion(-) diff --git a/internal/vm/autoload.go b/internal/vm/autoload.go index 69d73890..4e504ff2 100644 --- a/internal/vm/autoload.go +++ b/internal/vm/autoload.go @@ -107,7 +107,7 @@ func (vm *VM) autoloadPathFor(cls *RClass, name string, inherit bool) object.Val chain = vm.ancestors(cls) } for _, c := range chain { - if _, defined := c.consts[name]; defined { + if retireSettledAutoload(c, name) { return object.NilV } if c.autoloads == nil { @@ -147,6 +147,9 @@ func (vm *VM) autoloadVisible(c *RClass, name string) bool { if c == nil || c.autoloads == nil { return false } + if retireSettledAutoload(c, name) { + return false + } path, ok := c.autoloads[name] if !ok { return false @@ -154,6 +157,34 @@ func (vm *VM) autoloadVisible(c *RClass, name string) bool { return !vm.featureLoaded(path) } +// retireSettledAutoload drops c's pending autoload for name once the constant +// it reserves is DEFINED, and reports whether the constant is defined. +// +// An autoload entry is settled the moment its constant gets a value: MRI's +// const_tbl_update replaces the autoload entry with an ordinary one, so +// Module#autoload? answers nil and the name never triggers a load again, even +// if the constant is removed afterwards and the feature dropped from $". +// rbgo cannot see that instant — a constant assignment lands in +// vm.assignConstIn, which this file does not own — so it retires the entry at +// the first read that finds the constant defined. That is the same rule applied +// lazily; the window it leaves is a read-free stretch between the definition +// and a remove_const, where a re-registered autoload would still look settled. +// +// Without it a DIRECT require of an autoload's file left the entry behind +// (tryAutoload only clears the entries it runs itself), and rbgo then answered +// "settled" from $LOADED_FEATURES alone — so restoring $", as ruby/spec does +// after every example, resurrected an autoload that had already fired. +func retireSettledAutoload(c *RClass, name string) bool { + if c == nil { + return false + } + if _, defined := c.consts[name]; !defined { + return false + } + delete(c.autoloads, name) + return true +} + // registerAutoloadOn records (or replaces) a pending autoload for name on cls. // When the constant is already defined in cls's own table the registration is // dropped, matching MRI where autoload of a defined constant is inert. A fresh @@ -186,6 +217,9 @@ func (vm *VM) tryAutoload(cls *RClass, name string) bool { if cls == nil || cls.autoloads == nil { return false } + if retireSettledAutoload(cls, name) { + return false + } path, ok := cls.autoloads[name] if !ok { return false diff --git a/internal/vm/module_wave30_test.go b/internal/vm/module_wave30_test.go index 0c7307a6..7e696e58 100644 --- a/internal/vm/module_wave30_test.go +++ b/internal/vm/module_wave30_test.go @@ -6,6 +6,9 @@ package vm_test import ( "fmt" + "os" + "path/filepath" + "strconv" "strings" "testing" "unicode/utf8" @@ -585,3 +588,42 @@ p out.include?("can only set in method defining module")` } } } + +// TestModuleWave30AutoloadRetiredByDirectRequire covers an autoload entry that +// is settled by a DIRECT require of its file rather than by the autoload +// itself. MRI's const_tbl_update replaces the autoload entry the moment the +// constant gets a value, so Module#autoload? answers nil for good; rbgo retires +// the entry at the first read that finds the constant defined. Asserted against +// MRI 4.0. +func TestModuleWave30AutoloadRetiredByDirectRequire(t *testing.T) { + dir := t.TempDir() + path := filepath.Join(dir, "w30_autoload_direct.rb") + if err := os.WriteFile(path, []byte("W30Direct::K = :loaded\n"), 0o600); err != nil { + t.Fatal(err) + } + lit := strconv.Quote(path) + // Before the require the entry is pending; after it, and after $" is + // restored and the constant removed, a FRESH autoload for the same file is + // live again — the settled entry did not linger to mask it. + src := `module W30Direct; end +saved = $".dup +W30Direct.autoload(:K, ` + lit + `) +a = W30Direct.autoload?(:K) +require ` + lit + ` +b = W30Direct.autoload?(:K) +$".replace saved +W30Direct.send(:remove_const, :K) +W30Direct.autoload(:K, ` + lit + `) +p [a == ` + lit + `, b, W30Direct.autoload?(:K) == ` + lit + `]` + if got := eval(t, src); got != "[true, nil, true]\n" { + t.Errorf("direct require of an autoload's file: got %q", got) + } + // Reading it back through the retired entry does not re-run the file, and + // Module#constants keeps listing a pending name. + src2 := `module W30Direct2; end +W30Direct2.autoload(:K, ` + lit + `) +p [W30Direct2.constants(false), W30Direct2.const_defined?(:K, false)]` + if got := eval(t, src2); got != "[[:K], true]\n" { + t.Errorf("pending autoload listing: got %q", got) + } +} From 755e63e3e8cdab0f9212934ff52548d491658913 Mon Sep 17 00:00:00 2001 From: tannevaled Date: Fri, 25 Sep 2026 10:56:29 +0200 Subject: [PATCH 5/6] vm: Module#dup and #clone reject a positional argument, and three dead arms go MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Follow-up to the two previous commits, driven by the coverage gate. `Class.new.clone(1)` was silently ignored; MRI raises "wrong number of arguments (given 1, expected 0)" for both #dup and #clone, whose only argument is the freeze: keyword (a trailing Hash). Raising is both the MRI answer and what makes cloneFreezeArg's non-Hash arm reachable. Three arms no argument list can take, removed rather than covered: * retireSettledAutoload's `c == nil` guard, and its calls from autoloadVisible and tryAutoload. Those two are reached only after a constant lookup has MISSED on the same class, so the entry they see is never a settled one; autoloadPathFor is the one read that can reach a class whose table already holds the name, and it is now the only caller. Why is stated where the function is. * const_source_location's tail `return nil` after its segment loop. constPathSegs never yields an empty list, so the last segment always exists and always answers; the loop now walks segs[:len-1] and the last segment is handled after it, which is also closer to MRI's shape (`recur = Qfalse` applies between segments, never before the last read). * copyValueTable's nil guard. newClass is the only *RClass constructor and it always creates consts, cvars and ivars — the only three maps handed to it. copyStringTable / copyBoolTable / copyVisTable keep theirs: those tables really are lazy. TestModuleWave20ConstSourceLocation asserted the old answer (nil for a constant that resolves). It now asserts [], with MRI's real answer and the reason rbgo cannot give it yet written next to it; which names resolve is covered by the wave 30 test. Co-Authored-By: Claude Opus 5 --- internal/vm/autoload.go | 15 ++++++-------- internal/vm/module_residuals.go | 34 +++++++++++++++++++------------ internal/vm/module_wave20_test.go | 23 +++++++++++++-------- internal/vm/module_wave30_test.go | 15 ++++++++++++++ 4 files changed, 56 insertions(+), 31 deletions(-) diff --git a/internal/vm/autoload.go b/internal/vm/autoload.go index 4e504ff2..cbd3b74c 100644 --- a/internal/vm/autoload.go +++ b/internal/vm/autoload.go @@ -147,9 +147,6 @@ func (vm *VM) autoloadVisible(c *RClass, name string) bool { if c == nil || c.autoloads == nil { return false } - if retireSettledAutoload(c, name) { - return false - } path, ok := c.autoloads[name] if !ok { return false @@ -174,10 +171,13 @@ func (vm *VM) autoloadVisible(c *RClass, name string) bool { // (tryAutoload only clears the entries it runs itself), and rbgo then answered // "settled" from $LOADED_FEATURES alone — so restoring $", as ruby/spec does // after every example, resurrected an autoload that had already fired. +// +// autoloadPathFor is the ONLY caller, and that is not an oversight: it is the +// one read that reaches a class whose constant table already holds the name. +// autoloadVisible and tryAutoload are both reached only after a constant lookup +// has MISSED on the same class, so the entry they see can never be a settled +// one, and a guard there would be an arm no argument list can take. func retireSettledAutoload(c *RClass, name string) bool { - if c == nil { - return false - } if _, defined := c.consts[name]; !defined { return false } @@ -217,9 +217,6 @@ func (vm *VM) tryAutoload(cls *RClass, name string) bool { if cls == nil || cls.autoloads == nil { return false } - if retireSettledAutoload(cls, name) { - return false - } path, ok := cls.autoloads[name] if !ok { return false diff --git a/internal/vm/module_residuals.go b/internal/vm/module_residuals.go index 9bc44a74..e36092c0 100644 --- a/internal/vm/module_residuals.go +++ b/internal/vm/module_residuals.go @@ -80,13 +80,14 @@ func (vm *VM) registerModuleResiduals() { if topLevel { mod = vm.cObject } - for i, seg := range segs { + // Every segment but the last names the module to look in next, and only + // the FIRST of them is resolved with inherit (MRI sets recur = Qfalse at + // the foot of its loop). constPathSegs never yields an empty list, so the + // last segment always exists and always answers. + for i, seg := range segs[:len(segs)-1] { if !constNameWellFormed(seg) { raise("NameError", "wrong constant name %s", orig) } - if i == len(segs)-1 { - return vm.constLocation(mod, seg, !recur, recur) - } v, ok := vm.constSegGet(mod, seg, recur, i == 0) if !ok { v = vm.constMissing(mod, seg) // raises NameError by default @@ -97,7 +98,11 @@ func (vm *VM) registerModuleResiduals() { } mod, recur = nc, false } - return object.NilV + last := segs[len(segs)-1] + if !constNameWellFormed(last) { + raise("NameError", "wrong constant name %s", orig) + } + return vm.constLocation(mod, last, !recur, recur) }) // Module#const_missing(sym): the default hook, raising a NameError naming the @@ -175,7 +180,10 @@ func (vm *VM) registerModuleResiduals() { cp.isModule = src.isModule return cp } - vm.cModule.define("dup", func(vm *VM, self object.Value, _ []object.Value, _ *Proc) object.Value { + vm.cModule.define("dup", func(vm *VM, self object.Value, args []object.Value, _ *Proc) object.Value { + if len(args) != 0 { + raise("ArgumentError", "wrong number of arguments (given %d, expected 0)", len(args)) + } src := self.(*RClass) cp := allocCopy(src) vm.send(cp, "initialize_dup", []object.Value{src}, nil) @@ -563,12 +571,10 @@ func copyModuleTables(dst, src *RClass) { bumpMethodSerial() } -// copyValueTable returns a fresh map with the same entries, or nil for a nil -// source (the tables on RClass are created lazily and nil is meaningful). +// copyValueTable returns a fresh map with the same entries. It takes no nil +// guard: newClass is the only *RClass constructor and it always creates the +// consts, cvars and ivars tables, which are the only maps handed to it. func copyValueTable(src map[string]object.Value) map[string]object.Value { - if src == nil { - return nil - } out := make(map[string]object.Value, len(src)) for k, v := range src { out[k] = v @@ -620,8 +626,10 @@ func cloneFreezeArg(vm *VM, args []object.Value) (object.Value, bool) { return object.NilV, false } h, ok := args[len(args)-1].(*object.Hash) - if !ok { - return object.NilV, false + if !ok || len(args) != 1 { + // clone takes no positional argument, only the freeze: keyword, which + // arrives as a single trailing Hash. + raise("ArgumentError", "wrong number of arguments (given %d, expected 0)", len(args)) } var val object.Value = object.NilV given := false diff --git a/internal/vm/module_wave20_test.go b/internal/vm/module_wave20_test.go index 2f7d55c4..4beed5ed 100644 --- a/internal/vm/module_wave20_test.go +++ b/internal/vm/module_wave20_test.go @@ -169,18 +169,23 @@ p r`, "true\n"}, } } -// TestModuleWave20ConstSourceLocation covers Module#const_source_location: rbgo -// does not track constant source positions, so it returns nil for a resolvable -// name, but still enforces MRI's name validation (NameError for a malformed name -// or a Symbol carrying a scope path; TypeError for a failed #to_str). Asserted -// against MRI 4.0. +// TestModuleWave20ConstSourceLocation covers Module#const_source_location's name +// validation: NameError for a malformed name or a Symbol carrying a scope path, +// TypeError for a failed #to_str. Asserted against MRI 4.0. Which names RESOLVE +// is covered by TestModuleWave30ConstSourceLocation; wave 30 gave the method +// MRI's search, so a name that resolves now answers [] rather than nil. func TestModuleWave20ConstSourceLocation(t *testing.T) { cases := []struct{ src, want string }{ - // A well-formed name (resolvable or not) yields nil (no location tracked). - {`class C20; K = 1; end; p C20.const_source_location(:K)`, "nil\n"}, + // A name that resolves to nothing is nil … {`p Object.const_source_location(:CS_ABSENT_20)`, "nil\n"}, - // A String or Symbol both accepted; a valid scoped path is well-formed. - {`p Module.const_source_location("Object")`, "nil\n"}, + // … and one that resolves answers []. MRI says ["file", 1] for a constant + // defined in Ruby and [] for one defined in C; rbgo records no definition + // site yet (a Ruby-level assignment lands in vm.assignConstIn, which has + // no place to stamp one), so it answers [] for both — right for every + // constant this VM defines in Go, a known gap for the rest. See the note + // on Module#const_source_location in module_residuals.go. + {`class C20; K = 1; end; p C20.const_source_location(:K)`, "[]\n"}, + {`p Module.const_source_location("Object")`, "[]\n"}, } for _, c := range cases { if got := eval(t, c.src); got != c.want { diff --git a/internal/vm/module_wave30_test.go b/internal/vm/module_wave30_test.go index 7e696e58..c3d44b29 100644 --- a/internal/vm/module_wave30_test.go +++ b/internal/vm/module_wave30_test.go @@ -281,6 +281,15 @@ p m.send(:initialize_copy, m).equal?(m)`, "true\n"}, !strings.Contains(err.Error(), "FrozenError") { t.Errorf("frozen receiver: got %v, want FrozenError", err) } + // Neither takes a positional argument (MRI: "wrong number of arguments + // (given 1, expected 0)"). + for _, src := range []string{`Class.new.dup(1)`, `Class.new.clone(1)`, `Class.new.clone(1, freeze: true)`} { + if err := runErr(t, src); err == nil || + !strings.Contains(err.Error(), "wrong number of arguments (given 1, expected 0)") && + !strings.Contains(err.Error(), "wrong number of arguments (given 2, expected 0)") { + t.Errorf("%s: got %v, want ArgumentError", src, err) + } + } // clone rejects an unknown keyword and a non-boolean freeze:. if err := runErr(t, `Class.new.clone(nope: 1)`); err == nil || !strings.Contains(err.Error(), "unknown keyword") { @@ -370,6 +379,12 @@ p W30Str.const_source_location(n)`, "[]\n"}, t.Errorf("src=%q\n got=%q\nwant=%q", c.src, got, c.want) } } + // An intermediate segment that resolves to nothing goes through + // #const_missing, whose default raises NameError. + if err := runErr(t, `Object.const_source_location("W30NoSuchMod::X")`); err == nil || + !strings.Contains(err.Error(), "NameError") { + t.Errorf("unresolvable segment: got %v, want NameError", err) + } // A leading segment that names a non-module is a TypeError, as in // rb_mod_const_source_location's "does not refer to class/module". if err := runErr(t, `W30NOTMOD = 1 From 54249f8a891941493a58c84212c20d65091e2cd6 Mon Sep 17 00:00:00 2001 From: tannevaled Date: Fri, 25 Sep 2026 11:01:27 +0200 Subject: [PATCH 6/6] vm: a wrong constant name in a path names the SEGMENT, not the whole path MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit MRI's wrong_name reports the offending SEGMENT when a segment of an "A::B" path is not a constant name — rb_mod_const_get and its siblings set `name = part` before the goto — and the whole string only when the path is malformed AS A PATH: a bare "::", a trailing one, or two in a row, where there is no single segment to blame. rbgo reported the whole string in both cases, so `Module.const_get("lower::X")` said "wrong constant name lower::X" where MRI says "wrong constant name lower". The three walks share one rule, so const_get, const_defined? and const_source_location move together. A single-segment name is unaffected (the segment IS the string), except a leading "::", which MRI strips before naming the segment — and now so does rbgo. Also covers the last statement this wave left uncovered: the well-formedness check on a NON-LAST path segment had no test. Co-Authored-By: Claude Opus 5 --- internal/vm/module_residuals.go | 14 ++++++++++---- internal/vm/module_wave30_test.go | 17 +++++++++++++++++ 2 files changed, 27 insertions(+), 4 deletions(-) diff --git a/internal/vm/module_residuals.go b/internal/vm/module_residuals.go index e36092c0..e272e269 100644 --- a/internal/vm/module_residuals.go +++ b/internal/vm/module_residuals.go @@ -86,7 +86,7 @@ func (vm *VM) registerModuleResiduals() { // last segment always exists and always answers. for i, seg := range segs[:len(segs)-1] { if !constNameWellFormed(seg) { - raise("NameError", "wrong constant name %s", orig) + raise("NameError", "wrong constant name %s", seg) } v, ok := vm.constSegGet(mod, seg, recur, i == 0) if !ok { @@ -100,7 +100,7 @@ func (vm *VM) registerModuleResiduals() { } last := segs[len(segs)-1] if !constNameWellFormed(last) { - raise("NameError", "wrong constant name %s", orig) + raise("NameError", "wrong constant name %s", last) } return vm.constLocation(mod, last, !recur, recur) }) @@ -237,7 +237,7 @@ func (vm *VM) moduleConstGet(cls *RClass, arg object.Value, inherit bool) object var result object.Value for i, seg := range segs { if !constNameWellFormed(seg) { - raise("NameError", "wrong constant name %s", orig) + raise("NameError", "wrong constant name %s", seg) } v, ok := vm.constSegGet(mod, seg, inherit, i == 0) if !ok { @@ -266,7 +266,7 @@ func (vm *VM) moduleConstDefined(cls *RClass, arg object.Value, inherit bool) bo } for i, seg := range segs { if !constNameWellFormed(seg) { - raise("NameError", "wrong constant name %s", orig) + raise("NameError", "wrong constant name %s", seg) } v, ok := vm.constSegDefined(mod, seg, inherit, i == 0) if !ok { @@ -287,6 +287,12 @@ func (vm *VM) moduleConstDefined(cls *RClass, arg object.Value, inherit bool) bo // String, or an object with #to_str) into its path segments, reporting whether a // leading "::" selected the top level. A Symbol may not carry a scope path or // qualifier — MRI treats "::" inside a Symbol name as a malformed constant name. +// +// It also returns the name as given, for the errors that report the WHOLE path: +// MRI's wrong_name names the offending SEGMENT when a segment is not a constant +// name (`name = part` in rb_mod_const_get and its siblings) and the whole string +// only for a path that is malformed as a path — a bare "::", a trailing one, or +// two in a row — where there is no one segment to blame. func (vm *VM) constPathSegs(arg object.Value) (segs []string, topLevel bool, orig string) { name, isSym := vm.constArgToString(arg) orig = name diff --git a/internal/vm/module_wave30_test.go b/internal/vm/module_wave30_test.go index c3d44b29..86878018 100644 --- a/internal/vm/module_wave30_test.go +++ b/internal/vm/module_wave30_test.go @@ -392,6 +392,23 @@ Object.const_source_location("W30NOTMOD::X")`); err == nil || !strings.Contains(err.Error(), "does not refer to class/module") { t.Errorf("non-module segment: got %v, want TypeError", err) } + // A malformed SEGMENT inside a path names the segment, as MRI's wrong_name + // does (`name = part`); a path malformed as a PATH names the whole string, + // because no single segment is to blame. Same rule for const_get and + // const_defined?, which share the walk. + for _, tc := range []struct{ src, want string }{ + {`Module.const_source_location("lower30::X")`, "wrong constant name lower30"}, + {`Module.const_source_location("A=::X")`, "wrong constant name A="}, + {`Module.const_source_location("Object::lower30")`, "wrong constant name lower30"}, + {`Module.const_source_location("::name30")`, "wrong constant name name30"}, + {`Module.const_source_location("Object::::B")`, "wrong constant name Object::::B"}, + {`Module.const_get("lower30::X")`, "wrong constant name lower30"}, + {`Module.const_defined?("lower30::X")`, "wrong constant name lower30"}, + } { + if err := runErr(t, tc.src); err == nil || !strings.Contains(err.Error(), tc.want) { + t.Errorf("%s: got %v, want %q", tc.src, err, tc.want) + } + } // Every malformed name MRI rejects. for _, src := range []string{ `Module.const_source_location "name"`,