From 06eab0b206725bf93719299bf261f3b6b0317c42 Mon Sep 17 00:00:00 2001 From: tannevaled Date: Mon, 28 Sep 2026 16:45:06 +0200 Subject: [PATCH] Give a Binding the scope chain it was captured in, and register block_given? MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Two defects, reported with a third as "three faces, one cause" — frameBinding not walking the enclosing Env chain. Measured against ruby 4.0.5 the faces are three INDEPENDENT causes, and the history has now confirmed it the hard way: cause A (a String instance_eval/class_eval running in the caller's local scope) shipped on its own as #727, and with #727 merged every witness below still failed. This change is the other two. B. A Binding recorded only the innermost frame's locals. MRI's binding pins an ep whose chain is intact, so pm_eval_make_iseq declares one compile scope per parent_iseq (vm_eval.c ruby_4_0:1702-1732) and a name two scopes out compiles to a depth-2 reference into the live env. The Binding now carries a (depth, slot) map built by walking the frame stack the way rb_f_local_variables does (:2755-2787), and retargetLocals moves the compiled eval body's operands onto it — the runtime chain was already correct, only the operands were wrong, and by exactly the name's depth. The Binding also carries the block that was in scope, which is what an eval'd `yield` and `block_given?` read: vm_set_eval_stack runs the eval iseq on &bind->block (:2023-2031). `def m; eval("yield"); end; m { 7 }` raised LocalJumpError. C. block_given? and iterator? existed only as the compiler's OpBlockGiven, so send(:block_given?) raised and Kernel.private_instance_methods(false) said false. MRI registers both with rb_define_global_function beside the optimised path and keeps both (:2879-2880). The body is rb_f_block_given_p (:2810-2817), which answers about the frame vm_get_ruby_level_caller_cfp lands on and never about itself — so the frame now publishes its block. OpBlockGiven is untouched. Kernel#local_variables follows, held back until B landed: its body IS that walk, so shipping it earlier would have shipped a method answering [] inside a block where ruby 4.0.5 answers [:y]. Three silent faces are pinned by VALUE: binding.local_variables answered [[]], an eval'd assignment to an enclosing local went nowhere, and local_variable_set wrote to a slot that named something else — each a wrong answer, no exception. TestSpecificEvalStringInheritsTheBlockScopeGap (#727) asserted this gap was still open, and becomes TestSpecificEvalStringSeesTheBlockScope. Its three sources are measured SEPARATELY rather than sharing one expectation: its failure message predicted [10] for all three and its comment predicted 2 for the nested case, and ruby 4.0.5 answers [10], [10] and a bare 10. A write through the nested eval is added, since getlocal and setlocal are different operands. parseCompileEvalFn goes too. #727 deleted compileEval and left the seam as out of its scope; with compileEval gone it has no caller at all, and a package-level var needs none to compile, which is exactly how it would have sat there. KNOWN LIMIT, pinned in a test: MRI has a second arm for a scope whose frame has already returned (vm_collect_local_variables_in_heap reads the escaped env's own local table). An rbgo Env carries no name table, so a Binding taken in a block whose enclosing frame has since returned names only what is still on the stack. Unchanged here, neither caused nor closed. Co-Authored-By: Claude Opus 5 --- internal/vm/binding.go | 199 +++++++++++++--- internal/vm/binding_eval_open.go | 17 +- internal/vm/eval.go | 7 + internal/vm/frontend_closed.go | 7 - internal/vm/frontend_open.go | 7 - internal/vm/kernel_introspect.go | 66 ++++++ internal/vm/vm.go | 125 +++++++++- internal/vm/wave_b_binding_test.go | 72 +++--- internal/vm/wave_f_binding_test.go | 351 +++++++++++++++++++++++++++++ 9 files changed, 780 insertions(+), 71 deletions(-) create mode 100644 internal/vm/wave_f_binding_test.go diff --git a/internal/vm/binding.go b/internal/vm/binding.go index e6b0e42f..56deff6e 100644 --- a/internal/vm/binding.go +++ b/internal/vm/binding.go @@ -1,6 +1,9 @@ package vm -import "github.com/go-embedded-ruby/ruby/internal/object" +import ( + "github.com/go-embedded-ruby/ruby/internal/bytecode" + "github.com/go-embedded-ruby/ruby/internal/object" +) // Binding captures a frame's local-variable environment, self and definee, so // code can be eval'd against it later (Binding#eval, eval(str, binding)) and its @@ -12,9 +15,32 @@ type Binding struct { definee *RClass file string // source file the binding was captured in ("" for compiled-in code) line int // 1-based line the binding was captured at (MRI's bind->first_lineno) - names []string // slot index → local name (original ISeq locals, then injected ones) + names []string // name index → local name (this frame's locals, then the enclosing scopes', then injected ones) added []string // names injected via local_variable_set/eval, in insertion order + // locs places each entry of names in the ENV CHAIN: names[i] lives at + // locs[i].slot of env.ancestor(locs[i].depth). A nil locs means the identity + // map — every name at depth 0, slot == index — which is what a binding with no + // enclosing scope has and what every Binding was before this field existed. + // + // The chain is the whole of the block-scope defect. MRI's binding pins an ep, + // and the ep chain out to the method's is intact, so eval_make_iseq -> + // pm_eval_make_iseq builds ONE COMPILE SCOPE PER parent_iseq + // (vm_eval.c ruby_4_0:1702-1732: `do { scopes_count++; } while ((iseq = + // ISEQ_BODY(iseq)->parent_iseq));`) and a name found two scopes out compiles to + // a depth-2 reference into the LIVE env. rbgo recorded only the innermost + // frame's locals, so `[1].map { eval("y") }` could not see the y beside the + // map, and `binding.local_variables` there answered [] — a wrong answer with no + // exception. + locs []bindLoc + + // block is the block in scope at the capture site — MRI's bind->block, whose + // ep is what an eval run through this binding inherits. It is why + // `def m; eval("yield"); end; m { 7 }` is 7 and `[1].map { eval("block_given?") }` + // inside a method with a block is [true] on ruby 4.0.5: the eval frame is not a + // fresh block-less scope, it continues the captured one. + block *Proc + // method is the __method__/__callee__ pair of the frame the binding was // captured in, so eval'd code reports the enclosing method. MRI reaches the // same answer from the binding's own frame (rb_vm_frame_method_entry on the @@ -40,7 +66,16 @@ func (b *Binding) ToS() string { return "#" } func (b *Binding) Inspect() string { return "#" } func (b *Binding) Truthy() bool { return true } -// slotOf returns the env slot of a named local, or -1. +// bindLoc is one name's place in a Binding's environment chain: how many parent +// links out, and which slot there. It is MRI's (level, index) operand pair, the +// one getlocal/setlocal carry. +type bindLoc struct{ depth, slot int } + +// slotOf returns the NAME INDEX of a local, or -1. The index is not a slot: use +// at() to reach the value, since a name from an enclosing scope lives in an +// ancestor env. The first match wins, which is what makes an inner local SHADOW +// an outer one of the same name — collectBindingLocals adds inner first, and +// MRI's local_var_list is a hash keyed by name for the same reason. func (b *Binding) slotOf(name string) int { for i, n := range b.names { if n == name { @@ -50,6 +85,101 @@ func (b *Binding) slotOf(name string) int { return -1 } +// at resolves name index i to the environment holding it and the slot within it. +func (b *Binding) at(i int) (*Env, int) { + if b.locs == nil { + return b.env, i + } + l := b.locs[i] + return b.env.ancestor(l.depth), l.slot +} + +// ensureLocs materialises the identity map so a later append can extend it. A +// Binding built without one (the top level's, Proc#binding's) has names and slots +// in step; once ANY entry is appended the two can no longer be assumed aligned, +// so the invariant len(locs) == len(names) has to start holding before the first +// append rather than after it. +func (b *Binding) ensureLocs() { + if b.locs != nil || len(b.names) == 0 { + return + } + b.locs = make([]bindLoc, len(b.names)) + for i := range b.names { + b.locs[i] = bindLoc{slot: i} + } +} + +// addLocal appends a binding-only local (local_variable_set, or a name an eval +// string declares at its top scope) to the binding's innermost env — depth 0, +// which is the scope MRI's vm_bind_update_env extends. +func (b *Binding) addLocal(name string, v object.Value) { + b.ensureLocs() + b.env.slots = append(b.env.slots, v) + b.names = append(b.names, name) + b.added = append(b.added, name) + b.locs = append(b.locs, bindLoc{slot: len(b.env.slots) - 1}) +} + +// retargetLocals moves a compiled eval body's references to the binding's locals +// from the ONE borrowed compile scope the front end builds onto the binding's +// real environment chain. +// +// MRI never needs this step: pm_eval_make_iseq declares one parser scope per +// parent_iseq and hands the parser the names at each level, so the compiler +// itself emits getlocal/setlocal with the right (level, index) — vm_eval.c +// ruby_4_0:1702-1760. rbgo's CompileEvalWithLocals builds a SINGLE synthetic +// parent holding a flat name list, so every binding local compiles to depth 1. +// The RUNTIME chain is already correct — the eval frame's env.parent IS the +// binding's env, whose own parent is the enclosing scope — so only the operands +// are wrong, and they are wrong by exactly the name's depth. +// +// The rewrite is total and local: at tree level k inside the eval (the eval body +// itself is 0), depth k reaches the eval body's own env and depth k+1 is the +// borrowed scope — the deepest reference the compiler can emit there, since the +// borrowed scope is the outermost one it knows. So `B == k+1` identifies a +// reference to the binding and nothing else. A child that BREAKS the env chain (a +// `def` or a class body written inside the eval) cannot reach past itself, so its +// instructions never carry a depth that high and descending into it with k+1 +// rewrites nothing — which is why the walk does not need to tell a block child +// from a method child. +func (b *Binding) retargetLocals(iseq *bytecode.ISeq) { + if b.locs == nil { + return + } + // The identity map is what a binding with no enclosing scope has, and it is + // the overwhelmingly common one: skip the walk rather than rewrite operands + // to the values they already hold. + identity := true + for i, l := range b.locs { + if l.depth != 0 || l.slot != i { + identity = false + break + } + } + if identity { + return + } + b.retargetLevel(iseq, 0) +} + +func (b *Binding) retargetLevel(iseq *bytecode.ISeq, k int) { + for i := range iseq.Insns { + in := &iseq.Insns[i] + if in.Op != bytecode.OpGetLocal && in.Op != bytecode.OpSetLocal { + continue + } + if in.B != k+1 || in.A < 0 || in.A >= len(b.locs) { + continue + } + l := b.locs[in.A] + in.A = l.slot + in.B += l.depth + } + for _, c := range iseq.Children { + b.retargetLevel(c, k+1) + } +} + func (vm *VM) registerBinding() { cBinding := newClass("Binding", vm.cObject) vm.consts["Binding"] = cBinding @@ -130,34 +260,19 @@ func (vm *VM) registerBinding() { env: b.env, self: b.self, definee: b.definee, + block: b.block, file: b.file, line: b.line, method: b.method, names: append([]string(nil), b.names...), + locs: append([]bindLoc(nil), b.locs...), added: append([]string(nil), b.added...), } } cBinding.define("dup", bindingDup) cBinding.define("clone", bindingDup) cBinding.define("local_variables", func(_ *VM, self object.Value, _ []object.Value, _ *Proc) object.Value { - b := self.(*Binding) - seen := map[string]bool{} - var elems []object.Value - add := func(n string) { - if n != "" && !seen[n] { // skip anonymous slots (pattern subjects etc.) - seen[n] = true - elems = append(elems, object.Symbol(n)) - } - } - // MRI lists local_variable_set-injected locals first (most-recent first), - // then the binding's original locals in slot order. - for i := len(b.added) - 1; i >= 0; i-- { - add(b.added[i]) - } - for _, n := range b.names[:len(b.names)-len(b.added)] { - add(n) - } - return object.NewArrayFromSlice(elems) + return self.(*Binding).localVariableNames() }) cBinding.define("local_variable_get", func(vm *VM, self object.Value, args []object.Value, _ *Proc) object.Value { b := self.(*Binding) @@ -166,19 +281,24 @@ func (vm *VM) registerBinding() { if i < 0 { raise("NameError", "local variable '%s' is not defined for %s", name, b.ToS()) } - return b.env.slots[i] + e, slot := b.at(i) + return e.slots[slot] }) cBinding.define("local_variable_set", func(vm *VM, self object.Value, args []object.Value, _ *Proc) object.Value { b := self.(*Binding) name := vm.bindingVarName(args[0]) if i := b.slotOf(name); i >= 0 { - b.env.slots[i] = args[1] + // The write lands where the name LIVES, which for a binding taken inside a + // block is an ANCESTOR env. Writing b.env.slots[i] instead put the value in + // the block's own frame under an index that named something else there, so + // `[1].each { b = binding }; b.local_variable_set(:q, 43)` left q at 42 and + // raised nothing — one of this defect's silent faces. + e, slot := b.at(i) + e.slots[slot] = args[1] } else { // A new binding-local: extend the name map, the environment and the // injected-locals list (which local_variables surfaces first). - b.names = append(b.names, name) - b.added = append(b.added, name) - b.env.slots = append(b.env.slots, args[1]) + b.addLocal(name, args[1]) } return args[1] }) @@ -187,6 +307,33 @@ func (vm *VM) registerBinding() { }) } +// localVariableNames is the Symbol array Binding#local_variables and +// Kernel#local_variables both answer with. It is one body because in MRI they are +// one walk: bind_local_variables calls rb_vm_bind_local_variables, and +// rb_f_local_variables performs the same collection over the caller's frames +// (vm_eval.c ruby_4_0:2755-2787). Names are deduplicated, which is how an inner +// local shadowing an outer one is listed once — MRI's local_var_list is a hash. +func (b *Binding) localVariableNames() object.Value { + seen := map[string]bool{} + var elems []object.Value + add := func(n string) { + if n != "" && !seen[n] { // skip anonymous slots (pattern subjects etc.) + seen[n] = true + elems = append(elems, object.Symbol(n)) + } + } + // MRI lists local_variable_set-injected locals first (most-recent first), + // then the binding's own locals — this frame's in slot order, then each + // enclosing scope's, which is the order collectBindingLocals built. + for i := len(b.added) - 1; i >= 0; i-- { + add(b.added[i]) + } + for _, n := range b.names[:len(b.names)-len(b.added)] { + add(n) + } + return object.NewArrayFromSlice(elems) +} + // bindingVarName coerces a local-variable name argument to a Go string: a Symbol // or String directly, otherwise an object responding to #to_str (MRI's implicit // String conversion). Anything else raises TypeError with MRI's message. diff --git a/internal/vm/binding_eval_open.go b/internal/vm/binding_eval_open.go index 7f667f37..3201ad0c 100644 --- a/internal/vm/binding_eval_open.go +++ b/internal/vm/binding_eval_open.go @@ -30,14 +30,17 @@ func (vm *VM) bindingEval(b *Binding, src string, loc evalLocation) object.Value // binding's frame (depth 1) rather than the throwaway eval-block env. if newNames := bindingNewLocals(iseq.Locals); len(newNames) > 0 { for _, n := range newNames { - b.names = append(b.names, n) - b.added = append(b.added, n) - b.env.slots = append(b.env.slots, object.NilV) + b.addLocal(n, object.NilV) } // Re-seeding the same program with additional valid local names cannot // introduce a compile error, so the earlier check already covered it. iseq, _ = compiler.CompileEvalWithLocals(prog, b.names, loc.line) } + // The front end resolves every binding local at depth 1 against one borrowed + // scope; a binding captured inside a BLOCK has its names spread over an env + // chain, so the operands are moved onto it here. A no-op for the identity map, + // which is what a method-level or top-level binding carries. + b.retargetLocals(iseq) iseq.Name = "(eval)" // The location is the eval's own, exactly as on the no-binding path: MRI // reaches eval_string_with_scope through the same rb_f_eval, so the file and @@ -52,7 +55,13 @@ func (vm *VM) bindingEval(b *Binding, src string, loc evalLocation) object.Value // the capturing frame recorded rides on the Binding for exactly this. mc := b.method vm.pendingMethodCtx = &mc - return vm.exec(iseq, b.self, nil, b.definee, "", b.env, nil, nil, nil, nil) + // The eval frame inherits the binding's BLOCK, not an empty one: MRI runs the + // eval iseq on the captured block's ep (vm_set_eval_stack(ec, iseq, NULL, + // &bind->block), vm_eval.c ruby_4_0:2023-2031), so `yield` and `block_given?` + // inside the string answer about the frame the binding came from. Measured on + // ruby 4.0.5: `def m; eval("yield"); end; m { 7 }` is 7, and rbgo raised + // LocalJumpError for want of this argument. + return vm.exec(iseq, b.self, nil, b.definee, "", b.env, b.block, nil, nil, nil) } // bindingNewLocals returns the named top-scope locals a compiled eval body diff --git a/internal/vm/eval.go b/internal/vm/eval.go index ce955af8..1ae6c9b0 100644 --- a/internal/vm/eval.go +++ b/internal/vm/eval.go @@ -108,6 +108,13 @@ func (vm *VM) evalLineno(v object.Value) int { // would be a path no MRI behaviour asks for. Deleting it rather than leaving it // uncalled is the point, exactly as for evalDefinee below: an unreachable // compile path is the shape a later wave would trust. +// +// Its front-end seam parseCompileEvalFn (frontend_open.go / frontend_closed.go) +// goes with it, which #727 deliberately left in place as out of its scope: with +// compileEval gone it has no caller at all. A package-level var needs none to +// compile, which is exactly why it would have sat there unnoticed. +// openParseCompileEval stays, reached through openParseCompile by require and +// the prelude. // raiseEvalSyntaxError raises the SyntaxError for a string that would not parse, // with the LOCATION in front of the message as MRI reports it: MRI's parser diff --git a/internal/vm/frontend_closed.go b/internal/vm/frontend_closed.go index a7ddff56..f8e06bd1 100644 --- a/internal/vm/frontend_closed.go +++ b/internal/vm/frontend_closed.go @@ -15,10 +15,3 @@ func frontendDropped(string) (*bytecode.ISeq, error) { raise("NotImplementedError", "eval/require of source is unavailable in a closed-world binary (built with rbgo build --closed, without the front-end)") return nil, nil // unreachable: raise panics } - -// parseCompileEvalFn is the closed-world stub of the eval seam (see -// frontend_open.go): it refuses for the same reason and with the same message. -var parseCompileEvalFn = func(src string, firstLine int) (*bytecode.ISeq, error) { - _ = firstLine - return frontendDropped(src) -} diff --git a/internal/vm/frontend_open.go b/internal/vm/frontend_open.go index 52edbb58..1659a1c4 100644 --- a/internal/vm/frontend_open.go +++ b/internal/vm/frontend_open.go @@ -20,13 +20,6 @@ func openParseCompile(src string) (*bytecode.ISeq, error) { return openParseCompileEval(src, 1) } -// parseCompileEvalFn is parseCompileFn for a string-eval given an explicit first -// line: the offset must reach the COMPILER (see compiler.CompileEval), because -// `__LINE__` becomes an Integer in the constant pool that no later pass can -// distinguish from any other. It is a second seam rather than an argument to the -// first so the many require/prelude callers keep the shape they have. -var parseCompileEvalFn = openParseCompileEval - func openParseCompileEval(src string, firstLine int) (*bytecode.ISeq, error) { prog, err := parser.Parse(src) if err != nil { diff --git a/internal/vm/kernel_introspect.go b/internal/vm/kernel_introspect.go index 78cc92e9..63951319 100644 --- a/internal/vm/kernel_introspect.go +++ b/internal/vm/kernel_introspect.go @@ -141,6 +141,72 @@ func (vm *VM) registerKernelIntrospection() { vm.cKernel.smethods[name] = &Method{name: name, owner: vm.cKernel, native: fn} } + // block_given? / iterator? are METHODS in MRI, not only the compiler's + // optimised instruction: rb_define_global_function("iterator?", + // rb_f_iterator_p, 0) and rb_define_global_function("block_given?", + // rb_f_block_given_p, 0) sit side by side in Init_vm_eval (vm_eval.c + // ruby_4_0:2879-2880). MRI keeps BOTH — the specialised instruction the + // compiler emits for the bareword AND the real method — so `send(:block_given?)`, + // `method(:block_given?)` and Kernel.private_instance_methods all work. rbgo + // had only the instruction (OpBlockGiven), which is why every one of those + // raised NoMethodError, and why Kernel.private_instance_methods(false) said + // false where MRI says true. + // + // The body is rb_f_block_given_p (vm_eval.c ruby_4_0:2810-2817): it asks about + // the frame vm_get_ruby_level_caller_cfp lands on — the CALLING Ruby frame — + // and not about itself. callerBlockGiven is that walk; registering a body that + // reported its own block would answer wrongly for every caller. + vm.cObject.define("block_given?", func(vm *VM, _ object.Value, args []object.Value, _ *Proc) object.Value { + if len(args) != 0 { + raise("ArgumentError", "wrong number of arguments (given %d, expected 0)", len(args)) + } + return object.Bool(vm.callerBlockGiven()) + }) + // rb_f_iterator_p is rb_f_block_given_p behind a deprecation notice + // (vm_eval.c ruby_4_0:2826-2831). rb_warn_deprecated reports only when the + // :deprecated warning category is enabled — off by default since Ruby 3 — + // which is why `iterator?` is silent under a plain `ruby file.rb` and speaks + // under `ruby -W:deprecated`. Both measured on ruby 4.0.5. + vm.cObject.define("iterator?", func(vm *VM, _ object.Value, args []object.Value, _ *Proc) object.Value { + if len(args) != 0 { + raise("ArgumentError", "wrong number of arguments (given %d, expected 0)", len(args)) + } + if vm.send(vm.consts["Warning"], "[]", []object.Value{object.SymVal("deprecated")}, nil).Truthy() { + vm.writeWarningStr(vm.warnUplevelPrefix(0) + "iterator? is deprecated; use block_given? instead\n") + } + return object.Bool(vm.callerBlockGiven()) + }) + // Both are rb_define_global_function, i.e. the module-function split: a PRIVATE + // instance method and a public singleton on Kernel. `Object.new.block_given?` + // therefore raises "private method 'block_given?' called for …" rather than + // "undefined method", and `Kernel.block_given?` answers false. Both measured. + // Kernel#local_variables is rb_f_local_variables (vm_eval.c + // ruby_4_0:2755-2787), installed by rb_define_global_function alongside the + // two above. It was DEFERRED until the Binding walked its enclosing scopes, + // deliberately: its body IS that walk, so shipping it first would have shipped + // a method that answered [] inside a block where ruby 4.0.5 answers [:y] — + // an MRI-divergent method rather than a missing one. + // + // It reads the caller's frame for the same reason Kernel#binding does: a native + // pushes no frame, so the top of the stack is the calling Ruby frame — the cfp + // vm_get_ruby_level_caller_cfp lands on. Sharing localVariableNames with + // Binding#local_variables is not a convenience: in MRI the two ARE the same + // collection, and letting them drift is how they would come to disagree about + // shadowing or about the order. + vm.cObject.define("local_variables", func(vm *VM, _ object.Value, args []object.Value, _ *Proc) object.Value { + if len(args) != 0 { + raise("ArgumentError", "wrong number of arguments (given %d, expected 0)", len(args)) + } + return vm.callerBinding().localVariableNames() + }) + + for _, name := range []string{"block_given?", "iterator?", "local_variables"} { + vm.setInstanceVisibility(vm.cObject, name, visPrivate) + fn := vm.cObject.methods[name].native + vm.cKernel.methods[name] = &Method{name: name, owner: vm.cKernel, native: fn, vis: visPrivate} + vm.cKernel.smethods[name] = &Method{name: name, owner: vm.cKernel, native: fn} + } + // caller(start=1, length=nil) / caller(range): a best-effort backtrace as a // String array, listed nearest-first. Like MRI, `start` omits that many // innermost levels (caller == caller(1) drops the frame that called caller; diff --git a/internal/vm/vm.go b/internal/vm/vm.go index 6e295cfe..eabf2b5e 100644 --- a/internal/vm/vm.go +++ b/internal/vm/vm.go @@ -249,6 +249,22 @@ type frameCode struct { env *Env self object.Value definee *RClass + + // block is the block in scope for this frame — MRI's + // VM_CF_BLOCK_HANDLER(cfp), which reads the block handler off the frame's + // LOCAL ep (VM_CF_LEP), so a block frame reports its enclosing METHOD's block + // rather than one of its own. rbgo reaches the same answer because exec's + // `block` parameter is already the method-level block for a block frame (a + // block Proc carries it as Proc.block, which is what makes `yield` inside a + // block reach the enclosing method's block). + // + // It is published for the same reason env/self/definee are: a NATIVE method + // pushes no frame, so Kernel#block_given? — rb_f_block_given_p (vm_eval.c + // ruby_4_0:2810-2817), which answers about the frame + // vm_get_ruby_level_caller_cfp lands on, NOT about itself — has nothing to + // read otherwise. That is exactly why block_given? could only ever be the + // OpBlockGiven intrinsic here and not a method. + block *Proc } // frameBinding builds a Binding for the innermost frame at or below index i that @@ -270,14 +286,21 @@ func (vm *VM) frameBinding(i int) *Binding { if fc.env == nil || fc.iseq == nil { continue } + // The Binding outlives the frame, so the env chain it pins must not be + // recycled — and now that the chain is WALKED, every ancestor is reachable + // through it, which is exactly what markEnvCaptured's own walk covers. + // OpBinding marked only because it was the single door; Kernel#binding came + // in through frameBinding without marking at all. + markEnvCaptured(fc.env) b := &Binding{ env: fc.env, self: fc.self, definee: fc.definee, + block: fc.block, file: fc.iseq.File, line: vm.frameLine(i), - names: append([]string(nil), fc.iseq.Locals...), } + vm.collectBindingLocals(b, i) // A Binding is transparent to Kernel#__method__ / #__callee__: code eval'd // through it reports the method the binding was captured in, which is the // pair that frame recorded. @@ -293,6 +316,78 @@ func (vm *VM) frameBinding(i int) *Binding { return vm.newToplevelBinding() } +// collectBindingLocals fills b.names/b.locs for a binding captured at frame i: +// this frame's locals at depth 0, then each ENCLOSING scope's at depth 1, 2, … +// +// It is rb_f_local_variables (vm_eval.c ruby_4_0:2755-2787) walked the same way. +// MRI adds ISEQ_BODY(cfp->iseq)->local_table for the frame it is on, and while +// the frame is a BLOCK (!VM_ENV_LOCAL_P(cfp->ep)) it takes the previous ep +// (VM_CF_PREV_EP) and steps back through the control frames until it finds the +// one whose ep IS that ep — `while (cfp->ep != ep) cfp = PREVIOUS(cfp)` — then +// repeats. Matching each parent Env against the frame that published it is that +// same search: rbgo's frame stack is MRI's cfp stack and Env.parent is its ep +// chain, and a method frame's env has no parent, which is where both stop. +// +// The ORDER is MRI's too, and observable: local_variables lists the innermost +// scope's names first, so an inner local of the same name shadows an outer one +// (slotOf takes the first match) exactly as the compiler resolves it. +// +// LIMIT, stated because it is a real divergence and not an oversight: MRI has a +// second arm for a scope whose frame has already returned — +// vm_collect_local_variables_in_heap reads the escaped env's own local table, +// because an MRI env carries a pointer to its iseq. An rbgo Env carries only +// slots and a parent, so a Binding taken inside a block belonging to a frame that +// has SINCE RETURNED (a Proc stored and called later) names the scopes still on +// the stack and stops. Everything measured for this wave — a binding taken while +// the enclosing frame is live, which is every shape in the defect report — takes +// the first arm. +func (vm *VM) collectBindingLocals(b *Binding, i int) { + fc := vm.frameCode[i] + // Depth 0 keeps EVERY slot, anonymous ones included and in slot order, so the + // innermost scope's name index is still its slot. Nothing depends on that any + // more — locs carries the slot — but a binding with no enclosing scope then + // has the identity map it always had, and reads the same in a dump. + b.names = append(b.names, fc.iseq.Locals...) + b.locs = make([]bindLoc, len(b.names)) + for k := range b.locs { + b.locs[k] = bindLoc{slot: k} + } + seen := make(map[string]bool, len(b.names)) + for _, n := range b.names { + if n != "" { + seen[n] = true + } + } + j := i - 1 + depth := 1 + for e := fc.env.parent; e != nil; depth++ { + k := -1 + for ; j >= 0; j-- { + if vm.frameCode[j].env == e { + k = j + break + } + } + if k < 0 { + return + } + if fk := vm.frameCode[k]; fk.iseq != nil { + for slot, n := range fk.iseq.Locals { + // An anonymous slot cannot be named from an eval and has nothing to + // list; only depth 0 keeps them, for the slot identity above. + if n == "" || seen[n] { + continue + } + seen[n] = true + b.names = append(b.names, n) + b.locs = append(b.locs, bindLoc{depth: depth, slot: slot}) + } + } + e = e.parent + j = k - 1 + } +} + // callerBinding is the Binding a NATIVE method's caller would get from // Kernel#binding. A native pushes no frame, so the top of the frame stack IS the // calling Ruby frame — the cfp rb_vm_get_binding_creatable_next_cfp lands on once @@ -301,6 +396,33 @@ func (vm *VM) callerBinding() *Binding { return vm.frameBinding(len(vm.frameNames) - 1) } +// callerBlockGiven answers Kernel#block_given? / #iterator? for a NATIVE method's +// caller: rb_f_block_given_p takes ec->cfp, steps to the PREVIOUS control frame +// and runs vm_get_ruby_level_caller_cfp over it, then tests +// VM_CF_BLOCK_HANDLER(cfp) != VM_BLOCK_HANDLER_NONE (vm_eval.c +// ruby_4_0:2810-2817). A native pushes no frame in rbgo, so the top of the frame +// stack already IS that cfp; the walk outward only has to skip a frame whose +// scope exec has not published yet, exactly as frameBinding does. +// +// It answers about the CALLING frame, never about itself — which is why merely +// registering a method that reported its own block would answer wrongly, and why +// the frame has to carry the block at all. With no Ruby frame at all (cfp == +// NULL) MRI answers false, and so does the empty walk here. +func (vm *VM) callerBlockGiven() bool { + i := len(vm.frameNames) - 1 + if i >= len(vm.frameCode) { + i = len(vm.frameCode) - 1 + } + for ; i >= 0; i-- { + fc := vm.frameCode[i] + if fc.env == nil || fc.iseq == nil { + continue + } + return fc.block != nil + } + return false +} + func (vm *VM) uncaughtBacktrace(e RubyError) []object.Value { if !object.IsNil(e.Obj) { if bt, ok := getIvar(e.Obj, backtraceIvar).(*object.Array); ok { @@ -1670,6 +1792,7 @@ func (vm *VM) exec(iseq *bytecode.ISeq, self object.Value, args []object.Value, vm.frameCode[myFrame].env = env vm.frameCode[myFrame].self = self vm.frameCode[myFrame].definee = definee + vm.frameCode[myFrame].block = block // frameCrefs mirrors frameNames too; the cref is filled in below once lexCref // is known (a nil placeholder keeps the stacks aligned until then). vm.frameCrefs = append(vm.frameCrefs, nil) diff --git a/internal/vm/wave_b_binding_test.go b/internal/vm/wave_b_binding_test.go index ca3ac22e..730adda7 100644 --- a/internal/vm/wave_b_binding_test.go +++ b/internal/vm/wave_b_binding_test.go @@ -138,33 +138,53 @@ func TestSpecificEvalStringSeesCallerLocals(t *testing.T) { } } -// TestSpecificEvalStringInheritsTheBlockScopeGap records a divergence this change -// does NOT close, so that nobody reads the tests above as a claim that it did. +// TestSpecificEvalStringSeesTheBlockScope was TestSpecificEvalStringInheritsTheBlockScopeGap +// until wave F closed the gap it recorded. It is kept, converted rather than +// deleted, because it is the join between two changes: #727 put instance_eval's +// String form onto Kernel#eval's mechanism, and this asserts that the mechanism +// then carried BOTH of them across the same fix. // -// rbgo's frameBinding (internal/vm/vm.go) builds a Binding from the frame's own -// iseq.Locals and does not walk the enclosing Env chain, so a binding captured -// inside a BLOCK cannot see the locals of the method or top level around it. MRI -// does: `y = 5; [1].map { eval("y * 2") }` is [10] on ruby 4.0.5. +// The gap it used to pin was frameBinding building a Binding from the frame's own +// iseq.Locals without walking the enclosing Env chain. It now walks it, the way +// rb_f_local_variables steps back through the control frames (vm_eval.c +// ruby_4_0:2755-2787), and the eval body's operands are retargeted onto that +// chain because pm_eval_make_iseq declares one compile scope per parent_iseq +// (:1702-1732). // -// That gap is Kernel#eval's already — it predates this change and behaves -// identically there — and instance_eval now shares it precisely BECAUSE the two -// were joined onto one mechanism. Pinning it keeps the two in step: when -// frameBinding learns to walk the parent env, this test fails and both want -// is [10]. -func TestSpecificEvalStringInheritsTheBlockScopeGap(t *testing.T) { - const evalSrc = `y = 5; p([1].map { eval("y * 2") })` - const ievalSrc = `y = 5; p([1].map { Object.new.instance_eval("y * 2") })` - // A NESTED string-eval is the same gap wearing a different hat: the outer - // eval's own frame resolves `y` up-scope rather than owning a slot for it, so - // the inner callerBinding cannot see it either. ruby 4.0.5 answers 2. - const nestedSrc = `y = 5; p Object.new.instance_eval("Object.new.instance_eval('y * 2')")` - const want = "undefined method 'y' for " - for _, src := range []string{evalSrc, ievalSrc, nestedSrc} { - class, msg := evalErr(t, src) - if class != "NoMethodError" || len(msg) < len(want) || msg[:len(want)] != want { - t.Errorf("src=%q: got %s:%q; expected the shared block-scope gap (%s%s…). "+ - "If frameBinding now walks the enclosing env, both of these answer [10] on "+ - "ruby 4.0.5 and this test should become that assertion.", src, class, msg, "NoMethodError:", want) - } +// Each source is measured SEPARATELY against ruby 4.0.5 rather than sharing one +// expectation, and the third is why: the old test's failure message predicted +// "[10]" for all of them, and its comment predicted 2 for the nested case. Both +// were wrong. The nested eval is not inside a map, so it answers the bare 10 — +// measured, not deduced. +func TestSpecificEvalStringSeesTheBlockScope(t *testing.T) { + for _, c := range []struct{ name, src, want string }{ + // Kernel#eval in a block: the gap's original shape. + {"eval_in_block", `y = 5; p([1].map { eval("y * 2") })`, "[10]"}, + // instance_eval(String) in a block: it reaches the same frameBinding since + // #727, so it had to flip with the one above and not separately. + {"instance_eval_in_block", `y = 5; p([1].map { Object.new.instance_eval("y * 2") })`, "[10]"}, + // A NESTED string-eval, which #727 grouped here as the same cause: the outer + // eval's frame resolves y UP-SCOPE instead of owning a slot, so the inner + // callerBinding could not see it. It is the same cause, and it is NOT the + // same answer — no array here. + {"nested_string_eval", `y = 5; p Object.new.instance_eval("Object.new.instance_eval('y * 2')")`, "10"}, + } { + t.Run(c.name, func(t *testing.T) { + if got := eval(t, c.src); got != c.want+"\n" { + t.Errorf("src=%q got=%q want=%q (ruby 4.0.5)", c.src, got, c.want+"\n") + } + }) + } +} + +// TestNestedStringEvalWritesReachTheOuterLocal is the write direction of the +// nested case above, which the gap test never covered: reading up-scope through +// two evals and writing up-scope through two evals are different operands +// (getlocal vs setlocal) and the retargeting has to move both. ruby 4.0.5 answers +// 99. +func TestNestedStringEvalWritesReachTheOuterLocal(t *testing.T) { + const src = `y = 5; Object.new.instance_eval("Object.new.instance_eval('y = 99')"); p y` + if got, want := eval(t, src), "99\n"; got != want { + t.Errorf("got %q, want %q (ruby 4.0.5)", got, want) } } diff --git a/internal/vm/wave_f_binding_test.go b/internal/vm/wave_f_binding_test.go new file mode 100644 index 00000000..68aade69 --- /dev/null +++ b/internal/vm/wave_f_binding_test.go @@ -0,0 +1,351 @@ +package vm + +import ( + "bytes" + "strings" + "testing" + + "github.com/go-embedded-ruby/ruby/internal/bytecode" + "github.com/go-embedded-ruby/ruby/internal/object" +) + +// Wave F pins three defects that the report handed over as ONE ("three faces, one +// cause"). Measurement against ruby 4.0.5 refuted that, and the table in +// TestWaveFScopeWitnesses is the refutation: each witness names the single fix +// that moves it, and two witnesses move only when TWO of them are present. +// +// A a STRING instance_eval/class_eval/module_eval runs in the CALLER'S local +// scope (eval.go evalUnder — eval_string_with_cref, vm_eval.c +// ruby_4_0:1983-2016). No block is needed to see this one fail. +// B a Binding carries the ENV CHAIN it was captured in, and the block that was +// in scope there (vm.go collectBindingLocals, binding.go retargetLocals — +// pm_eval_make_iseq's one compile scope per parent_iseq, vm_eval.c +// ruby_4_0:1702-1732, and rb_f_local_variables' cfp walk, :2755-2787). +// C block_given?/iterator? are real Kernel methods, not only the compiler's +// instruction (rb_define_global_function, vm_eval.c ruby_4_0:2879-2880). +// +// Two of the faces are SILENT — a wrong answer with no exception — which is why +// they are pinned by value and not by "it did not raise": `[1].map { +// binding.local_variables }` answered [[]] and an assignment through a Binding +// taken in a block landed in the wrong slot without complaint. + +// TestWaveFScopeWitnesses runs every witness and compares against the answer +// measured on ruby 4.0.5. Each `want` in this table was read off the oracle, not +// derived from rbgo. +func TestWaveFScopeWitnesses(t *testing.T) { + for _, tc := range []struct { + name, src, want, fix string + }{ + // --- A: a string instance_eval/class_eval sees the caller's locals. The + // receiver's own scope is irrelevant and no block is involved. + {"instance_eval_string_method_scope", + `def m; y = 5; Object.new.instance_eval("y * 2"); end; p m`, "10", "A"}, + {"class_eval_string_method_scope", + `def m; y = 5; String.class_eval("y * 2"); end; p m`, "10", "A"}, + {"module_eval_string_method_scope", + `def m; y = 5; String.module_eval("y * 2"); end; p m`, "10", "A"}, + // A must not cost the definee: a `def` inside instance_eval(String) is still + // a SINGLETON method of the receiver, and inside class_eval an instance + // method of the class. Routing both through the caller's binding replaces + // only self and the cref, exactly as eval_under does. + {"instance_eval_string_defines_singleton", + `o = Object.new; o.instance_eval("def only_mine; 1; end"); p [o.only_mine, Object.new.respond_to?(:only_mine)]`, + "[1, false]", "A"}, + {"class_eval_string_defines_instance_method", + `class WFA; end; WFA.class_eval("def im; 2; end"); p WFA.new.im`, "2", "A"}, + {"instance_eval_string_self", + `o = "s"; p o.instance_eval("self")`, `"s"`, "A"}, + + // --- B: a Binding taken inside a block reaches the enclosing scope. + // SILENT face — the answer was [[]] and nothing was raised. + {"binding_local_variables_in_block", + `def m; y = 5; [1].map { binding.local_variables }; end; p m`, "[[:y]]", "B"}, + {"eval_reads_enclosing_local_from_block", + `def m; y = 5; [1].map { eval("y") }; end; p m`, "[5]", "B"}, + {"eval_reads_two_blocks_out", + `def m; w = 7; [1].map { [1].map { eval("w") } }; end; p m`, "[[7]]", "B"}, + // SILENT face — the write went nowhere and z stayed 1. + {"eval_writes_enclosing_local_from_block", + `def m; z = 1; [1].each { eval("z = 99") }; z; end; p m`, "99", "B"}, + // SILENT face — local_variable_set reported the value it had just stored in + // the WRONG slot, so the read agreed with it and the real local did not move. + {"local_variable_set_through_block_binding", + `def m; b = nil; q = 42; [1].each { b = binding }; b.local_variable_set(:q, 43); [b.local_variable_get(:q), q]; end; p m`, + "[43, 43]", "B"}, + {"local_variable_get_through_block_binding", + `def m; b = nil; q = 42; [1].each { b = binding }; [b.local_variable_get(:q), b.local_variable_defined?(:q)]; end; p m`, + "[42, true]", "B"}, + // An inner local of the same name SHADOWS the outer one: slotOf takes the + // first match and collectBindingLocals adds the innermost scope first. + {"inner_local_shadows_enclosing", + `def m; y = 1; [1].map { |x| y = 2; eval("y") }; end; p m`, "[2]", "B"}, + {"block_param_reaches_binding", + `def m; [7].map { |v| eval("v") }; end; p m`, "[7]", "B"}, + // B also carries the capture-site BLOCK, which is what an eval'd `yield` and + // `block_given?` read. + {"eval_string_inherits_the_frame_block", + `def m; eval("yield"); end; p(m { 7 })`, "7", "B"}, + {"block_given_through_eval_in_block", + `def m; [1].map { eval("block_given?") }; end; p [m, m {}]`, "[[false], [true]]", "B"}, + {"binding_eval_sees_the_captured_block", + `def m; b = binding; b.eval("block_given?"); end; p [m, m {}]`, "[false, true]", "B"}, + + // --- A and B TOGETHER: neither alone answers these. + {"instance_eval_string_inside_a_block", + `def m; y = 5; [1].map { Object.new.instance_eval("y * 2") }; end; p m`, "[10]", "A+B"}, + {"instance_eval_string_yields_to_the_frame_block", + `def m; Object.new.instance_eval("yield"); end; p(m { 8 })`, "8", "A+B"}, + + // --- C: block_given? / iterator? as real methods, answering about the + // CALLING frame (rb_f_block_given_p walks to the caller's cfp). + {"send_block_given", + `def m; self.send(:block_given?); end; p [m, m {}]`, "[false, true]", "C"}, + {"method_object_block_given", + `def m; method(:block_given?).call; end; p [m, m {}]`, "[false, true]", "C"}, + {"iterator_p_is_a_method", + `def m; self.send(:iterator?); end; p [m, m {}]`, "[false, true]", "C"}, + {"kernel_lists_them_private", + `p Kernel.private_instance_methods(false).values_at(*[]).class; p [:block_given?, :iterator?].map { |s| Kernel.private_instance_methods(false).include?(s) }`, + "Array\n[true, true]", "C"}, + {"kernel_singleton_is_public", + `p Kernel.block_given?`, "false", "C"}, + {"owner_is_kernel", + `p Kernel.instance_method(:block_given?).owner`, "Kernel", "C"}, + // rb_define_global_function makes the Object-side copy PRIVATE, so an + // explicit receiver raises the PRIVATE NoMethodError, not "undefined method". + {"explicit_receiver_is_private", + `begin; Object.new.block_given?; rescue NoMethodError => e; p e.message.start_with?("private method"); end`, + "true", "C"}, + {"respond_to_include_all", + `p [Object.new.respond_to?(:block_given?), Object.new.respond_to?(:block_given?, true)]`, + "[false, true]", "C"}, + // The optimised instruction MUST keep working beside the method — MRI keeps + // both. A bareword inside a block still answers about the enclosing METHOD's + // block, because the block handler is read off the LOCAL ep (VM_CF_LEP). + {"bareword_intrinsic_still_answers", + `def m; block_given?; end; p [m, m {}]`, "[false, true]", "C"}, + {"bareword_in_a_block_reads_the_methods_block", + `def m; [1].map { block_given? }; end; p [m, m {}]`, "[[false], [true]]", "C"}, + {"arity_is_zero", + `begin; self.send(:block_given?, 1); rescue ArgumentError => e; p e.message; end`, + `"wrong number of arguments (given 1, expected 0)"`, "C"}, + } { + t.Run(tc.name, func(t *testing.T) { + if got := runSrc(t, tc.src); got != tc.want { + t.Errorf("fix %s: %s\n got %q\n want %q (ruby 4.0.5)", tc.fix, tc.src, got, tc.want) + } + }) + } +} + +// TestWaveFWitnessTableIsDiscriminating is the known-bad control for the table +// above. A table of expectations is only a measurement if a WRONG answer fails +// it, and every entry in TestWaveFScopeWitnesses is a plain string compare — +// which is exactly the shape that silently passes when the harness stops running +// the program at all. Three earlier sweeps in this campaign reported a clean +// total from a runner that read nothing. +// +// So: run a program whose answer is known to be wrong against the same helper and +// require the comparison to REJECT it, and run the fixed one and require the +// comparison to accept it. If runSrc ever returns "" for everything, the first +// half still passes and the second half fails, which is the point. +func TestWaveFWitnessTableIsDiscriminating(t *testing.T) { + const src = `def m; y = 5; [1].map { eval("y") }; end; p m` + const correct = "[5]" + const knownBad = "[[]]" // what rbgo answered for the sibling witness before fix B + + got := runSrc(t, src) + if got != correct { + t.Fatalf("control: %s = %q, want %q — the witness itself is broken", src, got, correct) + } + if got == knownBad { + t.Fatalf("control: the comparison accepted the known-bad answer %q", knownBad) + } + // And the negative direction: a witness whose `want` is wrong must fail. + if runSrc(t, src) == "this can never be the output of p" { + t.Fatal("control: the comparison accepted an impossible answer") + } +} + +// TestWaveFBindingChainResolution drives the Binding's (depth, slot) map in +// package, including the two cases the Ruby-level witnesses cannot reach on +// their own: a name resolved through a THREE-level chain, and the identity map a +// chainless binding keeps. +func TestWaveFBindingChainResolution(t *testing.T) { + outer := &Env{slots: []object.Value{object.IntValue(1), object.IntValue(2)}} + mid := &Env{slots: []object.Value{object.IntValue(3)}, parent: outer} + inner := &Env{slots: []object.Value{object.IntValue(4)}, parent: mid} + + b := &Binding{ + env: inner, + names: []string{"i", "m", "o0", "o1"}, + locs: []bindLoc{{0, 0}, {1, 0}, {2, 0}, {2, 1}}, + } + for _, tc := range []struct { + name string + want int64 + }{{"i", 4}, {"m", 3}, {"o0", 1}, {"o1", 2}} { + idx := b.slotOf(tc.name) + if idx < 0 { + t.Fatalf("slotOf(%q) = -1", tc.name) + } + e, slot := b.at(idx) + if got := e.slots[slot]; got != object.IntValue(tc.want) { + t.Errorf("%s = %v, want %d", tc.name, got, tc.want) + } + } + // A write must land in the ANCESTOR env, not in the innermost one under an + // index that names something else there — the silent face of the defect. + i := b.slotOf("o1") + e, slot := b.at(i) + e.slots[slot] = object.IntValue(99) + if outer.slots[1] != object.IntValue(99) { + t.Errorf("write through the chain landed in %v, not in the outer env", inner.slots) + } + if inner.slots[0] != object.IntValue(4) { + t.Errorf("write through the chain disturbed the inner env: %v", inner.slots) + } + + // A binding with no locs is the identity map, which is what every + // method-level and top-level binding carries. + flat := &Binding{env: &Env{slots: []object.Value{object.IntValue(7), object.IntValue(8)}}, names: []string{"a", "b"}} + if e, slot := flat.at(flat.slotOf("b")); e.slots[slot] != object.IntValue(8) { + t.Errorf("identity map: b = %v, want 8", e.slots[slot]) + } + // ensureLocs must materialise it BEFORE the first append, or names and slots + // drift apart the moment a binding-only local is added. + flat.addLocal("c", object.IntValue(9)) + if got := len(flat.locs); got != 3 { + t.Fatalf("after addLocal, len(locs) = %d, want 3 (parallel to names)", got) + } + if e, slot := flat.at(flat.slotOf("a")); e.slots[slot] != object.IntValue(7) { + t.Errorf("after addLocal, a = %v, want 7", e.slots[slot]) + } + if e, slot := flat.at(flat.slotOf("c")); e.slots[slot] != object.IntValue(9) { + t.Errorf("after addLocal, c = %v, want 9", e.slots[slot]) + } +} + +// TestWaveFIteratorPDeprecationWarning drives rb_f_iterator_p's warning arm +// (vm_eval.c ruby_4_0:2826-2831). rb_warn_deprecated reports only when the +// :deprecated category is enabled — off by default since Ruby 3 — so the arm is +// unreachable without turning it on, which is also why `iterator?` looks silent. +// The text and the "file:line: warning: " prefix are ruby 4.0.5's, measured. +func TestWaveFIteratorPDeprecationWarning(t *testing.T) { + got := runWarn(t, "Warning[:deprecated] = true\ndef m; iterator?; end\np m\n") + if want := "iterator? is deprecated; use block_given? instead"; !strings.Contains(got, want) { + t.Errorf("stderr = %q, want it to contain %q", got, want) + } + if !strings.Contains(got, ":2: warning: ") { + t.Errorf("stderr = %q, want the caller's line in the prefix", got) + } + // And the control: with the category OFF — the default — nothing is written. + if quiet := runWarn(t, "def m; iterator?; end\np m\n"); strings.Contains(quiet, "deprecated") { + t.Errorf("warned with the :deprecated category off: %q", quiet) + } +} + +// TestWaveFIteratorPArity covers iterator?'s own arity check; block_given?'s is +// driven from the witness table. +func TestWaveFIteratorPArity(t *testing.T) { + const src = `begin; self.send(:iterator?, 1); rescue ArgumentError => e; p e.message; end` + if got, want := runSrc(t, src), `"wrong number of arguments (given 1, expected 0)"`; got != want { + t.Errorf("got %q, want %q", got, want) + } +} + +// TestWaveFCallerBlockGivenStructuralCases drives callerBlockGiven's two arms +// that no Ruby program reaches, the way TestFrameBindingWalksOutward does for +// frameBinding: frameNames deeper than frameCode (the clamp), a frame whose scope +// exec has not published (skipped, as rb_vm_get_binding_creatable_next_cfp skips +// one it cannot use), and NO Ruby frame at all — MRI's `cfp != NULL &&` arm, which +// answers false. +func TestWaveFCallerBlockGivenStructuralCases(t *testing.T) { + vm := New(&bytes.Buffer{}) + iseq := &bytecode.ISeq{File: "f.rb"} + + // No Ruby frame at all: the walk finds nothing and answers false (cfp == NULL). + vm.frameNames, vm.frameCode = nil, nil + if vm.callerBlockGiven() { + t.Error("with no frame at all, want false") + } + + // An unpublished frame (env == nil) is skipped and the one below it answers. + blk := &Proc{} + vm.frameNames = []string{"a", "b"} + vm.frameCode = []frameCode{ + {iseq: iseq, env: &Env{}, block: blk}, + {iseq: iseq}, // scope not published yet + } + if !vm.callerBlockGiven() { + t.Error("an unpublished frame must be skipped, not answered from") + } + // Same shape with no block below: the skip must not invent one. + vm.frameCode[0].block = nil + if vm.callerBlockGiven() { + t.Error("skipping to a block-less frame must answer false") + } + + // frameNames deeper than frameCode: the index is clamped to the top of + // frameCode rather than read past it. + vm.frameNames = []string{"a", "b", "c"} + vm.frameCode = []frameCode{{iseq: iseq, env: &Env{}, block: blk}} + if !vm.callerBlockGiven() { + t.Error("an index past frameCode must clamp to its top frame") + } +} + +// TestWaveFBindingStopsAtAReturnedFrame pins a KNOWN DIVERGENCE from ruby 4.0.5, +// deliberately, because it is the boundary of what collectBindingLocals can see +// and a later wave should find it stated rather than rediscover it. +// +// MRI's rb_f_local_variables has two arms (vm_eval.c ruby_4_0:2763-2785): walk the +// control frames while the enclosing scopes are still on the stack, and otherwise +// vm_collect_local_variables_in_heap, which reads the ESCAPED env's own local +// table. An MRI env can do that because it carries a pointer to its iseq; an rbgo +// Env carries only slots and a parent, so when the enclosing frame has already +// RETURNED there is nothing left to read a name from and the walk stops. +// +// ruby 4.0.5 answers [:q, :e, :pr] here; rbgo answers []. This is unchanged from +// before the wave — the fix neither caused it nor closed it — and closing it needs +// a name table on the Env, which is another wave's file. +func TestWaveFBindingStopsAtAReturnedFrame(t *testing.T) { + const src = `pr = nil +[1].each { q = 1; pr = proc { binding.local_variables } } +p pr.call` + if got, want := runSrc(t, src), "[]"; got != want { + t.Errorf("got %q, want %q — if this now reports names, the heap arm arrived and the comment above is stale", got, want) + } +} + +// TestWaveFKernelLocalVariables covers Kernel#local_variables, which this wave +// held back until fix B landed: its body IS the enclosing-scope walk, so shipping +// it earlier would have shipped a method answering [] inside a block where ruby +// 4.0.5 answers [:y] — divergent rather than merely absent. Every `want` measured +// on ruby 4.0.5. +func TestWaveFKernelLocalVariables(t *testing.T) { + for _, tc := range []struct{ name, src, want string }{ + {"method_scope", `def m; a = 1; b = 2; local_variables; end; p m`, "[:a, :b]"}, + // The one the deferral was about. + {"through_a_block", `def m; y = 5; [1].map { local_variables }; end; p m`, "[[:y]]"}, + // Like Binding#local_variables it lists the whole local TABLE, so a variable + // assigned later in the same scope is already named. + {"names_a_later_local", `def m; c = local_variables; d = 1; c; end; p m`, "[:c, :d]"}, + {"through_send", `def m; q = 1; self.send(:local_variables); end; p m`, "[:q]"}, + {"is_private_on_kernel", `p Kernel.private_instance_methods(false).include?(:local_variables)`, "true"}, + {"explicit_receiver_is_private", + `begin; Object.new.local_variables; rescue NoMethodError => e; p e.message.start_with?("private method"); end`, "true"}, + {"arity_is_zero", + `begin; self.send(:local_variables, 1); rescue ArgumentError => e; p e.message; end`, + `"wrong number of arguments (given 1, expected 0)"`}, + // A local shadowed by an inner one of the same name is listed ONCE, because + // MRI collects into a hash keyed by name. + {"shadowed_name_listed_once", `def m; y = 1; [1].map { |x| y = 2; local_variables }; end; p m`, "[[:x, :y]]"}, + } { + t.Run(tc.name, func(t *testing.T) { + if got := runSrc(t, tc.src); got != tc.want { + t.Errorf("%s\n got %q\n want %q (ruby 4.0.5)", tc.src, got, tc.want) + } + }) + } +}