diff --git a/internal/vm/builtins.go b/internal/vm/builtins.go index f3958161..558817d9 100644 --- a/internal/vm/builtins.go +++ b/internal/vm/builtins.go @@ -4,6 +4,7 @@ import ( "fmt" "math" "math/big" + "slices" "sort" "strconv" "strings" @@ -1714,74 +1715,29 @@ func (vm *VM) bootstrap() { delete(table, name) return val }) - // Module#constants returns the names (as symbols) of the constants defined - // directly in the module/class. With inherit (the default, true) the constants - // of ancestor modules/classes are included too, excluding Object's. Names are - // returned in sorted order; MRI leaves the order unspecified, and sorting keeps - // results deterministic across platforms. + // Module#constants returns the names (as symbols) of the public constants of + // the module/class; see constantNames for the inherit semantics. vm.cModule.define("constants", func(vm *VM, self object.Value, args []object.Value, _ *Proc) object.Value { inherit := true if len(args) > 0 { inherit = args[0].Truthy() } - seen := map[string]bool{} - var names []string - // A pending autoload counts as a constant: MRI's rb_autoload_str reserves - // the name with an undefined value, so Module#constants lists it before the - // file is loaded. Reference: ruby/ruby v3_4_0 variable.c - // autoload_synchronized → const_set(module, name, Qundef). - add := func(c *RClass) { - for name := range c.consts { - if !seen[name] { - seen[name] = true - names = append(names, name) - } - } - for name := range c.autoloads { - if !seen[name] { - seen[name] = true - names = append(names, name) - } - } - } - cls := self.(*RClass) - if inherit { - for _, anc := range vm.ancestors(cls) { - if anc == vm.cObject || anc == vm.cBasicObject { - continue - } - add(anc) - } - } else { - add(cls) - } - sort.Strings(names) - out := make([]object.Value, len(names)) - for i, n := range names { - out[i] = object.Symbol(n) - } - return object.NewArrayFromSlice(out) + return symbolArray(vm.constantNames(self.(*RClass), inherit)) }) // Module.constants (the SINGLETON method, distinct from Module#constants) has // two forms in MRI (rb_mod_s_constants): with no argument it returns the // constants accessible in the caller's scope — at the top level, every // top-level constant, equal to Object.constants; with an argument it behaves - // like Module#constants on the receiver (Module's own + inherited). It shadows - // the instance method only for the Module class and its subclasses (a plain - // class's `.constants` still finds the instance method through the Class chain). - // Reference: ruby/ruby v3_4_0 variable.c rb_mod_s_constants. + // like Module#constants on the receiver (Module's own + inherited), and so does + // a subclass of Module that inherits the singleton (`mod != rb_cModule`). + // Reference: ruby/ruby v3_4_1 eval.c rb_mod_s_constants. instConstants := vm.cModule.methods["constants"] vm.cModule.smethods["constants"] = &Method{name: "constants", owner: vm.cModule, native: func(vm *VM, self object.Value, args []object.Value, blk *Proc) object.Value { - if len(args) > 0 { + if len(args) > 0 || self != object.Value(vm.cModule) { return vm.invoke(instConstants, self, args, blk) } - names := vm.topLevelConstNames() - out := make([]object.Value, len(names)) - for i, n := range names { - out[i] = object.Symbol(n) - } - return object.NewArrayFromSlice(out) + return symbolArray(vm.lexicalConstantNames()) }} // Module#remove_const is a PRIVATE instance method in MRI // (rb_define_private_method(rb_cModule, "remove_const", …)), reachable as a @@ -10062,26 +10018,156 @@ func (vm *VM) fireConstAdded(cls *RClass, name string) { } } -// topLevelConstNames returns the sorted, de-duplicated names of every top-level -// constant, which back the no-argument form of Module.constants (equal to -// Object.constants in MRI). Top-level constants live in two tables in rbgo: the -// core classes/modules registered in vm.consts and the user-defined ones written -// to cObject.consts, so the union of the two is enumerated. Only well-formed -// constant names are reported, filtering any internal bookkeeping key. -func (vm *VM) topLevelConstNames() []string { - set := map[string]bool{} - for name := range vm.consts { - set[name] = true +// constantNames returns the names of cls's public constants, the answer +// of Module#constants. It follows MRI (ruby/ruby v3_4_1 variable.c): +// +// - inherit=false is rb_local_constants: cls's own table only. +// - inherit=true is rb_mod_const_of + rb_const_list: cls and its ancestors up +// the superclass chain, breaking on reaching Object unless cls IS Object — +// so Object.constants lists every top-level constant plus Kernel's and +// BasicObject's, while String.constants leaves all of those out. +// - A name is reported once, and its NEAREST definition decides (cv_i_update +// keeps the first entry): a private_constant nearer the receiver hides a +// public one of the same name further up. +// - list_i / rb_local_constants_i push only public entries, so a +// private_constant is never listed. +// - A pending autoload counts as a constant (autoload_synchronized reserves +// the name with const_set(module, name, Qundef)). +// +// Order: rb_const_list walks an insertion-ordered st_table filled table by table +// along the chain, so the receiver's names come first, then each ancestor's, as +// rbgo does. Within one table MRI yields the slot order of its id_table hash +// (id_table.c rb_id_table_foreach), which the documentation leaves unspecified; +// rbgo sorts each table's names, so results are deterministic. The cost is +// O(n log n) in the number of constants. +func (vm *VM) constantNames(cls *RClass, inherit bool) []string { + if !inherit { + return publicConstNames([]*RClass{cls}) + } + return publicConstNames(vm.constOf(cls)) +} + +// constOf is the table chain rb_mod_const_of visits for cls: cls and its +// ancestors, cut before Object unless cls is Object. +func (vm *VM) constOf(cls *RClass) []*RClass { + var stop *RClass + if cls != vm.cObject { + stop = vm.cObject + } + return vm.ancestorsUntil(cls, stop) +} + +// lexicalConstantNames answers the no-argument Module.constants, MRI's +// rb_mod_s_constants: the tables of every class on the caller's lexical cref +// chain (innermost first, ending at the top level, Object), then rb_mod_const_of +// of the innermost one. At the top level that is exactly Object.constants. +func (vm *VM) lexicalConstantNames() []string { + var cref *RClass + if n := len(vm.frameCrefs); n > 0 { + cref = vm.frameCrefs[n-1] + } + lex := vm.nesting(cref) + if len(lex) == 0 { + return vm.constantNames(vm.cObject, true) + } + chain := make([]*RClass, 0, len(lex)+1) + chain = append(append(chain, lex...), vm.cObject) + return publicConstNames(append(chain, vm.constOf(lex[0])...)) +} + +// publicConstNames lists the public constant names of the tables in chain, table +// by table in chain order (each table's names sorted), the nearest (earliest) +// table deciding each name's visibility. A name already in an earlier table of +// the chain is skipped by probing those tables directly: the chain is short (a +// handful of ancestors), so this costs no per-name set insertion. +func publicConstNames(chain []*RClass) []string { + size := 0 + for _, c := range chain { + size += len(c.consts) + len(c.autoloads) + } + names := make([]string, 0, size) + for i, c := range chain { + earlier := chain[:i] + keep := func(name string) bool { + return !c.privateConsts[name] && !inConstTables(earlier, name) + } + if len(c.autoloads) == 0 { + for _, name := range c.sortedConstKeys() { + if keep(name) { + names = append(names, name) + } + } + continue + } + // Pending autoloads are few: merge them into a fresh sorted list. + start := len(names) + for _, name := range c.sortedConstKeys() { + if keep(name) { + names = append(names, name) + } + } + for name := range c.autoloads { + if _, both := c.consts[name]; !both && keep(name) { + names = append(names, name) + } + } + slices.Sort(names[start:]) + } + return names +} + +// sortedConstKeys returns the names of c.consts, sorted, reusing the copy cached +// on c while it still matches the table. The table is written directly at many +// sites, so the cache cannot be invalidated on write; it is validated instead: a +// list of distinct names as long as the table, every one of them in the table, +// is exactly the table's key set. Validation is n map probes, cheaper than the +// n log n string comparisons of a fresh sort. Callers must not modify the result. +func (c *RClass) sortedConstKeys() []string { + if keys := c.constKeys; len(keys) == len(c.consts) && allConstKeys(keys, c.consts) { + return keys } - for name := range vm.cObject.consts { - set[name] = true + keys := make([]string, 0, len(c.consts)) + for name := range c.consts { + keys = append(keys, name) } - names := make([]string, 0, len(set)) - for name := range set { - names = append(names, name) + slices.Sort(keys) + c.constKeys = keys + return keys +} + +// allConstKeys reports whether every name in keys is a key of table. +func allConstKeys(keys []string, table map[string]object.Value) bool { + for _, name := range keys { + if _, ok := table[name]; !ok { + return false + } } - sort.Strings(names) - return names + return true +} + +// inConstTables reports whether name is a constant or a pending autoload of any +// of tables. +func inConstTables(tables []*RClass, name string) bool { + for _, c := range tables { + if _, ok := c.consts[name]; ok { + return true + } + if _, ok := c.autoloads[name]; ok { + return true + } + } + return false +} + +// symbolArray builds a Ruby Array of the Symbols named by names. Boxing each +// directly beats the SymVal intern here: its sync.Map probe costs more than the +// allocation it saves (measured: 0.24 vs 0.36 ms for 10 000 names). +func symbolArray(names []string) *object.Array { + out := make([]object.Value, len(names)) + for i, n := range names { + out[i] = object.Symbol(n) + } + return object.NewArrayFromSlice(out) } // cvarNameArg coerces a class-variable name argument (Symbol or String) to its diff --git a/internal/vm/module_constants_internal_test.go b/internal/vm/module_constants_internal_test.go new file mode 100644 index 00000000..66949e53 --- /dev/null +++ b/internal/vm/module_constants_internal_test.go @@ -0,0 +1,86 @@ +// Copyright (c) the go-embedded-ruby/ruby authors +// +// SPDX-License-Identifier: BSD-3-Clause + +package vm + +import ( + "fmt" + "io" + "slices" + "strings" + "testing" + + "github.com/go-embedded-ruby/ruby/internal/compiler" + "github.com/go-embedded-ruby/ruby/internal/object" + "github.com/go-ruby-parser/parser" +) + +// TestSortedConstKeysRevalidates covers the cache behind Module#constants: it is +// reused while the table is unchanged, and rebuilt when a name is removed and +// another added (same size, so only the membership probe can see it) or when the +// table grows. +func TestSortedConstKeysRevalidates(t *testing.T) { + c := &RClass{consts: map[string]object.Value{"B": object.NilV, "A": object.NilV}} + first := c.sortedConstKeys() + if !slices.Equal(first, []string{"A", "B"}) { + t.Fatalf("keys = %q", first) + } + if again := c.sortedConstKeys(); &again[0] != &first[0] { + t.Error("an unchanged table did not reuse the cache") + } + delete(c.consts, "A") + c.consts["C"] = object.NilV + if got := c.sortedConstKeys(); !slices.Equal(got, []string{"B", "C"}) { + t.Errorf("after swap keys = %q", got) + } + c.consts["D"] = object.NilV + if got := c.sortedConstKeys(); !slices.Equal(got, []string{"B", "C", "D"}) { + t.Errorf("after growth keys = %q", got) + } +} + +// TestPublicConstNamesAutoloadAlsoDefined covers a name that is both a constant +// and a pending autoload of one table (the autoload is being loaded): it is +// listed once, and the table's names stay sorted with the autoloads merged in. +func TestPublicConstNamesAutoloadAlsoDefined(t *testing.T) { + c := &RClass{ + consts: map[string]object.Value{"M": object.NilV, "Z": object.NilV}, + autoloads: map[string]string{"M": "m", "A": "a"}, + } + if got := publicConstNames([]*RClass{c}); !slices.Equal(got, []string{"A", "M", "Z"}) { + t.Errorf("names = %q", got) + } + // A pending autoload in a nearer table claims the name: the farther + // table's constant of the same name is not listed again. + near := &RClass{autoloads: map[string]string{"X": "x"}} + far := &RClass{consts: map[string]object.Value{"X": object.NilV}} + if got := publicConstNames([]*RClass{near, far}); !slices.Equal(got, []string{"X"}) { + t.Errorf("chained names = %q", got) + } +} + +// BenchmarkObjectConstants measures Object.constants with 10 000 user-defined +// top-level constants, the size issue #833's performance check uses. +func BenchmarkObjectConstants(b *testing.B) { + var src strings.Builder + for i := range 10000 { + fmt.Fprintf(&src, "BENCH_K%d = %d\n", i, i) + } + prog, err := parser.Parse(src.String()) + if err != nil { + b.Fatal(err) + } + iseq, err := compiler.Compile(prog) + if err != nil { + b.Fatal(err) + } + v := New(io.Discard) + if _, err := v.Run(iseq); err != nil { + b.Fatal(err) + } + b.ReportAllocs() + for b.Loop() { + symbolArray(v.constantNames(v.cObject, true)) + } +} diff --git a/internal/vm/module_constants_test.go b/internal/vm/module_constants_test.go new file mode 100644 index 00000000..26b1d8a9 --- /dev/null +++ b/internal/vm/module_constants_test.go @@ -0,0 +1,97 @@ +// Copyright (c) the go-embedded-ruby/ruby authors +// +// SPDX-License-Identifier: BSD-3-Clause + +package vm_test + +import ( + "fmt" + "strings" + "testing" +) + +// TestModuleConstantsInherit covers Module#constants and the Module.constants +// singleton against MRI's rb_mod_constants / rb_mod_const_of / rb_const_list +// (ruby/ruby v3_4_1 variable.c) and rb_mod_s_constants (eval.c). Every +// expectation was produced by MRI 3.4 on the same source (issue #833: Object +// .constants used to be empty because the inherit walk skipped Object always). +func TestModuleConstantsInherit(t *testing.T) { + cases := []struct{ src, want string }{ + // Object.constants lists every top-level constant, core and user-defined. + {`class Zed833; end +c = Object.constants +p [c.size > 50, c.include?(:Zed833), c.include?(:String), c.include?(:Comparable), c.include?(:Math)]`, + "[true, true, true, true, true]\n"}, + {`p Object.constants == Object.constants(true), Object.constants(false).include?(:Integer)`, + "true\ntrue\n"}, + // For Object the walk goes on through Kernel and Object's prepends; for any + // other class it stops before Object (rb_mod_const_of's break). + {`module Kernel; KC833 = 1; end +module P833; PC833 = 1; end +class Object; prepend P833; end +p [Object.constants.include?(:KC833), String.constants.include?(:KC833)] +p [Object.constants.include?(:PC833), String.constants.include?(:PC833)]`, + "[true, false]\n[true, false]\n"}, + // A private constant is never listed, inherit or not, at any level. + {`X833 = 1 +class Object; private_constant :X833; end +p [Object.constants.include?(:X833), Object.constants(false).include?(:X833), Module.constants.include?(:X833)] +p Object.const_defined?(:X833)`, + "[false, false, false]\ntrue\n"}, + {`module M833; A = 1; B = 2; private_constant :B; end +class K833; include M833; C = 3; end +class K2833 < K833; D = 4; end +p K833.constants, K833.constants(false), M833.constants, K2833.constants`, + "[:C, :A]\n[:C]\n[:A]\n[:D, :C, :A]\n"}, + // The nearest definition decides: a private QA in R hides Q's public QA. + {`module Q833; QA = 1; end +module R833; QA = 2; private_constant :QA; end +class S833; include Q833; include R833; end +p S833.constants`, + "[]\n"}, + // A pending autoload is listed before its file loads. + {`autoload :Lazy833, "nope_not_here_833" +p [Object.constants.include?(:Lazy833), Module.constants.include?(:Lazy833)]`, + "[true, true]\n"}, + // Module.constants with no argument: at the top level it equals + // Object.constants; inside a class body it adds the lexical scope. + {`p Module.constants == Object.constants`, "true\n"}, + {`class Foo833 + INNER = 1 + p [Module.constants.include?(:INNER), Module.constants.include?(:String)] +end`, "[true, true]\n"}, + {`module Outer833; O = 1; module Inner833; I = 1 + p [Module.constants.include?(:O), Module.constants.include?(:I), Module.constants.include?(:Inner833)] +end; end`, "[true, true, true]\n"}, + // An argument, or a Module subclass receiver, means Module#constants. + {`p [Module.constants(false).include?(:Zed833b), Module.constants(true).include?(:String)]`, + "[false, false]\n"}, + {`class MyMod833 < Module; MC = 1; end +p MyMod833.constants`, "[:MC]\n"}, + // Ancestors other than Object still count for a plain class, listed table + // by table from the receiver up (rb_const_list's insertion order). + {`module Mix833; MIXED = 1; end +class P833b; PARENT = 1; end +class C833 < P833b; include Mix833; OWN = 1; end +p C833.constants`, "[:OWN, :MIXED, :PARENT]\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) + } + } +} + +// TestObjectConstantsScales guards against a quadratic Object.constants: 10 000 +// top-level constants are all listed, each exactly once. +func TestObjectConstantsScales(t *testing.T) { + var b strings.Builder + for i := range 10000 { + fmt.Fprintf(&b, "K833_%d = %d\n", i, i) + } + b.WriteString(`c = Object.constants +p [c.size == c.uniq.size, c.count { |s| s.to_s.start_with?("K833_") }]`) + if got, want := eval(t, b.String()), "[true, 10000]\n"; got != want { + t.Errorf("got %q want %q", got, want) + } +} diff --git a/internal/vm/object_model.go b/internal/vm/object_model.go index b49cb662..4fce0249 100644 --- a/internal/vm/object_model.go +++ b/internal/vm/object_model.go @@ -280,10 +280,13 @@ type RClass struct { methods map[string]*Method smethods map[string]*Method // singleton (class) methods: def self.foo consts map[string]object.Value - cvars map[string]object.Value // class variables (@@name), shared down the hierarchy - ivars map[string]object.Value // the class object's OWN instance variables (@name with self = the class) - includes []*RClass // modules mixed in via include (most recent last) - prepends []*RClass // modules mixed in via prepend (most recent last), ahead of own methods + // constKeys caches consts' names, sorted, for Module#constants; see + // sortedConstKeys for why it is validated rather than invalidated. + constKeys []string + cvars map[string]object.Value // class variables (@@name), shared down the hierarchy + ivars map[string]object.Value // the class object's OWN instance variables (@name with self = the class) + includes []*RClass // modules mixed in via include (most recent last) + prepends []*RClass // modules mixed in via prepend (most recent last), ahead of own methods // lexParent is the lexically-enclosing class/module this one was defined in // (its Module.nesting parent), used to resolve bare constants by lexical // scope. nil for top-level definitions and for anonymous classes that have @@ -1141,6 +1144,14 @@ func lookupOwnOrIncluded(c *RClass, name string) *Method { // prepends/includes, and each ancestor appearing once (first occurrence wins). // This is the basis of both Module#ancestors and super. func (vm *VM) ancestors(c *RClass) []*RClass { + return vm.ancestorsUntil(c, nil) +} + +// ancestorsUntil is ancestors cut where the superclass walk reaches stop: stop, +// its prepends and everything above it are left out (a nil stop walks the whole +// chain). It is the shape of MRI's rb_mod_const_of, which follows RCLASS_SUPER +// and breaks on reaching Object for any receiver other than Object itself. +func (vm *VM) ancestorsUntil(c, stop *RClass) []*RClass { var out []*RClass seen := map[*RClass]bool{} push := func(k *RClass) { @@ -1159,7 +1170,7 @@ func (vm *VM) ancestors(c *RClass) []*RClass { addTree(m.includes[i]) } } - for k := c; k != nil; k = k.super { + for k := c; k != nil && k != stop; k = k.super { for i := len(k.prepends) - 1; i >= 0; i-- { addTree(k.prepends[i]) } diff --git a/internal/vm/racc_puppet_batch_test.go b/internal/vm/racc_puppet_batch_test.go index cb067ff6..488a1092 100644 --- a/internal/vm/racc_puppet_batch_test.go +++ b/internal/vm/racc_puppet_batch_test.go @@ -97,9 +97,10 @@ func TestModuleConstants(t *testing.T) { {"class Foo; BAR=1; BAZ=2; end; p Foo.constants", "[:BAR, :BAZ]"}, {"module M; X=9; end; p M.constants", "[:X]"}, {"module Empty; end; p Empty.constants", "[]"}, - // inherit=true (default) walks ancestors but skips Object's constants. + // inherit=true (default) walks ancestors, receiver first, but stops before + // Object (MRI 3.4 prints [:OWN, :MIXED, :PARENT]). {"module Mix; MIXED=1; end; class P; PARENT=1; end; class C < P; include Mix; OWN=1; end; p C.constants", - "[:MIXED, :OWN, :PARENT]"}, + "[:OWN, :MIXED, :PARENT]"}, // inherit=false reports only the receiver's own constants. {"class P2; PARENT=1; end; class C2 < P2; OWN=1; end; p C2.constants(false)", "[:OWN]"},