internal/vm: one gate and one composition for rb_warn, so the diagnostics fire, carry their position, and a redefined constant warns twice (#641, #699)

internal/vm/autoload.go

@@ -257,18 +257,22 @@ func (vm *VM) tryAutoload(cls *RClass, name string) bool {
 // It goes through rb_warning, not rb_warn, so only $VERBOSE == true shows it —
 // the same gate warnCircularRequire applies — and it is emitted at most once
 // per registration because the caller has already retired the entry.
+//
+// rbWarning, not a curStderr() write: rb_warning ends in rb_write_warning_str
+// like every other internal warning, so an overridden Warning.warn intercepts
+// this one too, and it carries the file:lineno prefix warning_string composes. The
+// gate was ALSO reading vm.globals["$VERBOSE"] out of the map, which is unset
+// until a program assigns it, so a Bool type assertion there could never be true
+// on a run that only passed -w.
 func (vm *VM) warnAutoloadDidNotDefine(cls *RClass, name, path string) {
 	if _, defined := cls.consts[name]; defined {
 		return
 	}
-	if v, ok := vm.globals["$VERBOSE"].(object.Bool); !ok || !bool(v) {
-		return
-	}
 	qualified := name
 	if cls != vm.cObject {
 		qualified = vm.moduleToSStr(cls) + "::" + name
 	}
-	vm.curStderr().writeStr("warning: Expected " + path + " to define " + qualified + " but it didn't\n")
+	vm.rbWarning("Expected " + path + " to define " + qualified + " but it didn't")
 }
 
 // featureLoaded reports whether path names a file that has already been loaded,

internal/vm/builtins.go

@@ -1,6 +1,7 @@
 package vm
 
 import (
+	"fmt"
 	"math"
 	"math/big"
 	"sort"
@@ -1706,13 +1707,12 @@ func (vm *VM) bootstrap() {
 		name := vm.coerceConstName(args[0])
 		// Overwriting an already-initialised constant (a real value, not a pending
 		// autoload — those live in cls.autoloads, so cls.consts misses them and no
-		// warning fires) warns "already initialized constant X". MRI uses rb_warn
-		// here, which fires whenever $VERBOSE is non-nil (false or true), suppressed
-		// only when $VERBOSE is nil. Reference: ruby/ruby v3_4_0 variable.c
-		// rb_const_set → const_set_raise / rb_warn on the already-set case.
-		if _, existed := cls.consts[name]; existed && !object.IsNil(vm.globals["$VERBOSE"]) {
-			vm.warnRedefineConst(cls, name)
-		}
+		// warning fires) is variable.c's const_tbl_update redefinition arm, the same
+		// one a syntactic `X = 2` reaches, so it goes through the same helper: both
+		// lines, the file:lineno prefix, and rb_warn's gate. This used to open-code a
+		// third behaviour of its own — a raw vm.globals read for the gate (never
+		// seeded, so always silent) and Kernel#warn for the sink.
+		vm.warnAlreadyInitialized(cls, name)
 		// Route through assignConstIn so an anonymous class/module bound here gains
 		// the qualified name of its constant (Ruby's "permanent name on first
 		// constant binding" rule) — the same path a `Foo::Bar = ...` literal takes.
@@ -3103,7 +3103,7 @@ func (vm *VM) bootstrap() {
 			// rb_ary_initialize uses rb_warning here, not rb_warn, so this one is
 			// only heard under -w / $VERBOSE = true.
 			if blk != nil {
-				vm.rbWarning1("given block not used")
+				vm.rbWarning("given block not used")
 			}
 			return self
 		}
@@ -3132,7 +3132,7 @@ func (vm *VM) bootstrap() {
 		// Build the array incrementally so that if the block calls break, the array
 		// is left holding the elements produced before the break, as MRI does.
 		if blk != nil && len(args) == 2 {
-			vm.rbWarn1("block supersedes default value argument")
+			vm.rbWarn("block supersedes default value argument")
 		}
 		arr.Elems = make([]object.Value, 0, n)
 		for i := int64(0); i < n; i++ {
@@ -3258,7 +3258,7 @@ func (vm *VM) bootstrap() {
 		//     if (block_given && argc == 2) rb_warn("block supersedes default value argument");
 		// so the warning fires whether or not the index is in range.
 		if blk != nil && len(args) == 2 {
-			vm.rbWarn1("block supersedes default value argument")
+			vm.rbWarn("block supersedes default value argument")
 		}
 		v, orig, ok := vm.arrayFetchAt(a, args[0])
 		if ok {
@@ -4425,7 +4425,7 @@ func (vm *VM) bootstrap() {
 		// array.c v3_4_0 rb_ary_index: with a value argument the block is dead,
 		// and MRI says so — `if (rb_block_given_p()) rb_warn("given block not used");`
 		if len(args) > 0 && blk != nil {
-			vm.rbWarn1("given block not used")
+			vm.rbWarn("given block not used")
 		}
 		for i := 0; i < len(a.Elems); i++ {
 			var match bool
@@ -4453,7 +4453,7 @@ func (vm *VM) bootstrap() {
 		}
 		// rb_ary_rindex warns for the dead block exactly as rb_ary_index does.
 		if len(args) > 0 && blk != nil {
-			vm.rbWarn1("given block not used")
+			vm.rbWarn("given block not used")
 		}
 		for i := len(a.Elems) - 1; i >= 0; i-- {
 			// A block may shrink the array; realign to the new end and re-check size
@@ -5036,7 +5036,7 @@ func (vm *VM) bootstrap() {
 		// hash.c v3_4_0 rb_hash_fetch_m warns on the clash before the lookup:
 		//     if (block_given && argc == 2) rb_warn("block supersedes default value argument");
 		if blk != nil && len(args) == 2 {
-			vm.rbWarn1("block supersedes default value argument")
+			vm.rbWarn("block supersedes default value argument")
 		}
 		if v, ok := self.(*object.Hash).Get(args[0]); ok {
 			return v
@@ -12215,34 +12215,85 @@ func (vm *VM) qualifiedConstName(scope *RClass, name string) string {
 	return vm.moduleToSStr(scope) + "::" + name
 }
 
-// rbWarn1 is error.c's rb_warn at tag v3_4_0 (error.c:466): the message gets the
-// CALLING frame's "path:lineno: warning: " in front (rb_warn builds it through
-// rb_warning_string, which starts at the current frame), it is silenced only
-// under -W0 where $VERBOSE is nil (`if (!NIL_P(ruby_verbose))`), and it is then
-// handed to rb_write_warning_str. `ruby -e 'p [1].index(1){}'` prints
-// "-e:1: warning: given block not used".
+// rbWarn is error.c's rb_warn at tag v3_4_0 (error.c:466). Three things happen
+// here and NOWHERE else, because a rule restated at its call sites drifts — and
+// this one had drifted three ways at once:
+//
+//  1. the gate is warnEnabled (`!NIL_P(ruby_verbose)`), so the default
+//     $VERBOSE == false speaks and only -W0's nil silences it;
+//  2. the message is composed by warningString, which is error.c's
+//     warning_string → warn_vsprintf → err_vcatf: the CURRENT Ruby frame's
+//     "path:lineno: ", then the literal "warning: ", then the text, then "\n".
+//     Callers pass the text ALONE — no "warning: " of their own;
+//  3. the sink is writeWarningStr, error.c's rb_write_warning_str, which is
+//     `rb_funcallv(Warning, :warn, str)` — so an overridden Warning.warn
+//     intercepts it and a $stderr that merely answers #write collects it.
 //
-// It emits through writeWarningStr — Warning.warn — and NOT to curStderr(), which
-// was the defect: rb_warn reaches $stderr only via the default Warning.warn's
-// `$stderr.write(message)`, so an overridden Warning.warn intercepts it and a
-// $stderr that merely answers #write collects it. See writeWarningStr in io.go
-// for the C chain and for why a StringIO double could witness neither.
-func (vm *VM) rbWarn1(msg string) {
-	if object.IsNil(vm.gvar("$VERBOSE")) {
+// `ruby -e 'p [1].index(1){}'` prints "-e:1: warning: given block not used".
+func (vm *VM) rbWarn(msg string) {
+	if !vm.warnEnabled() {
+		return
+	}
+	vm.writeWarningStr(vm.warningString(msg))
+}
+
+// rbWarnf is rbWarn with a format string. It exists only so a caller building a
+// message out of values does not reach for fmt itself; the gate, the prefix and
+// the sink are still rbWarn's.
+func (vm *VM) rbWarnf(format string, a ...any) {
+	vm.rbWarn(fmt.Sprintf(format, a...))
+}
+
+// rbWarning is error.c's rb_warning (error.c:497), the quieter sibling of
+// rb_warn: same composition and same sink, but the stricter RTEST gate, so it
+// prints only when $VERBOSE is TRUE (ruby -w). This is why the specs that assert
+// one of these pass `verbose: true` to `complain`.
+func (vm *VM) rbWarning(msg string) {
+	if !vm.warningEnabled() {
 		return
 	}
-	vm.writeWarningStr(vm.warnUplevelPrefix(0) + msg + "\n")
+	vm.writeWarningStr(vm.warningString(msg))
 }
 
-// rbWarning1 is error.c's rb_warning (error.c:497), the quieter sibling of
-// rb_warn: it prints only when $VERBOSE is TRUE (ruby -w), which is why the specs
-// that assert one of these pass `verbose: true` to `complain`. Same sink as
-// rbWarn1 — rb_warning also ends in rb_write_warning_str.
-func (vm *VM) rbWarning1(msg string) {
-	if v := vm.gvar("$VERBOSE"); v != object.True {
+// rbWarningf is rbWarning with a format string, standing to rbWarning as rbWarnf
+// stands to rbWarn.
+func (vm *VM) rbWarningf(format string, a ...any) {
+	vm.rbWarning(fmt.Sprintf(format, a...))
+}
+
+// rbCompileWarn is error.c's rb_compile_warn (error.c:397): rb_warn's gate and
+// rb_warn's sink, but an EXPLICIT source position instead of the current frame's.
+// MRI uses it for a diagnostic that points where the interpreter is not standing
+// any more — the "previous definition of X was here" line beside a constant
+// redefinition, whose position is the entry's own ce->file / ce->line.
+func (vm *VM) rbCompileWarn(file string, line int, msg string) {
+	if !vm.warnEnabled() {
 		return
 	}
-	vm.writeWarningStr(vm.warnUplevelPrefix(0) + msg + "\n")
+	vm.writeWarningStr(warningStringAt(file, line, msg))
+}
+
+// warningString is error.c's warning_string (error.c:450): warn_vsprintf over the
+// position rb_source_location_cstr reports for the current frame.
+func (vm *VM) warningString(msg string) string {
+	return vm.warnUplevelPrefix(0) + msg + "\n"
+}
+
+// warningStringAt is warn_vsprintf (error.c:381) with a caller-supplied position,
+// formatted by err_vcatf (error.c:125): the file, then ":line" only when the line
+// is non-zero, then ": ", then "warning: ". A file of "" drops the whole location
+// — err_vcatf tests `if (file)` — leaving the bare "warning: " prefix, which is
+// also what a constant defined by this VM's own bootstrap has (MRI: a ce->file of
+// nil, so no second line at all — see warnAlreadyInitialized).
+func warningStringAt(file string, line int, msg string) string {
+	if file == "" {
+		return "warning: " + msg + "\n"
+	}
+	loc := file
+	if line != 0 {
+		loc += ":" + strconv.Itoa(line)
+	}
+	return loc + ": warning: " + msg + "\n"
 }
 
 // symbolNames interns the frozen String that Symbol#name answers. MRI's symbol

internal/vm/env.go

@@ -70,7 +70,7 @@ func (vm *VM) registerENV() {
 	def("fetch", func(vm *VM, _ object.Value, args []object.Value, blk *Proc) object.Value {
 		key := vm.envStr(args[0])
 		if blk != nil && len(args) > 1 {
-			vm.rbWarn("warning: block supersedes default value argument")
+			vm.rbWarn("block supersedes default value argument")
 		}
 		if v, ok := envLookup(key); ok {
 			return envStrV(v)

internal/vm/env_wave16_test.go

@@ -116,8 +116,14 @@ func TestENVWave16(t *testing.T) {
 		{`ENV.clear; ENV["A"]="1"; p ENV.fetch("A")`, `"1"`},
 		{`ENV.clear; p ENV.fetch("X","d")`, `"d"`},
 		// block + default: the block supersedes the default (and MRI warns).
-		{`ENV.clear; ENV["A"]="1"; p ENV.fetch("A","d"){|k| "blk"}`, `"1"`},
-		{`ENV.clear; p ENV.fetch("X","d"){|k| "blk:#{k}"}`, `"blk:X"`},
+		// hash.c env_fetch warns through rb_warn when a default AND a block are given,
+		// so the default $VERBOSE (false) prints it, prefixed with the caller's
+		// position. Measured on MRI 4.0.5. These two rows asserted silence, which was
+		// only true while rbgo's rb_warn gate read an unset slot.
+		{`ENV.clear; ENV["A"]="1"; p ENV.fetch("A","d"){|k| "blk"}`,
+			"(rbgo):1: warning: block supersedes default value argument\n" + `"1"`},
+		{`ENV.clear; p ENV.fetch("X","d"){|k| "blk:#{k}"}`,
+			"(rbgo):1: warning: block supersedes default value argument\n" + `"blk:X"`},
 		{`ENV.clear; p ENV.fetch("X"){|k| "blk:#{k}"}`, `"blk:X"`},
 		{`ENV.clear; begin; ENV.fetch("X"); rescue KeyError => e; p [e.message, e.key, e.receiver.equal?(ENV)]; end`,
 			`["key not found: \"X\"", "X", true]`},

internal/vm/getoptlong_test.go

@@ -169,12 +169,15 @@ g = GetoptLong.new(["-a",GetoptLong::NO_ARGUMENT])
 ARGV.replace(["-a","z"])
 r=[]; g.each{|o,a| r<<[o,a]}; p r; p ARGV`,
 			"[[\"-a\", \"\"]]\n[\"z\"]\n"},
-		// A non-Array ARGV (reassigned) is treated as empty: nothing to scan.
+		// A non-Array ARGV (reassigned) is treated as empty: nothing to scan. The
+		// reassignment warns: ARGV is already initialised, and MRI warns there too
+		// (`ruby -e 'ARGV = "x"'` prints "-e:1: warning: already initialized constant
+		// ARGV"), with no "previous definition" line because ARGV is defined natively.
 		{golReq + `
 ARGV = "notarray"
 g = GetoptLong.new(["-a",GetoptLong::NO_ARGUMENT])
 p g.get`,
-			"[nil, nil]\n"},
+			"(rbgo):2: warning: already initialized constant ARGV\n[nil, nil]\n"},
 	}
 	for _, c := range cases {
 		if got := eval(t, c.src); got != c.want {

internal/vm/globals.go

@@ -202,12 +202,41 @@ func (vm *VM) verboseSlot(name string) object.Value {
 	return object.Bool(false)
 }
 
+// $VERBOSE has THREE states in MRI, not two, and the two warning families split
+// on exactly that (error.c v3_4_0):
+//
+//	nil   (-W0)          every rb_warn and rb_warning is silent
+//	false (the default)  rb_warn speaks, rb_warning stays quiet
+//	true  (-w / -W2)     both speak
+//
+// verbose_setter (ruby.c v3_4_0) coerces anything truthy to Qtrue, so these are
+// the only three values a slot can hold, and `$VERBOSE = 1` reads back as true.
+//
+// warnEnabled and warningEnabled are the ONLY two places that decide. Every gate
+// in this VM asks one of them, because the defect they replace was a second
+// representation of one value: four call sites read vm.globals["$VERBOSE"]
+// straight out of the map, which is unset until a program assigns it, while every
+// Ruby-level read went through verboseSlot and answered false. The gate read the
+// representation nobody writes, so `$VERBOSE.nil?` agreed on both engines and no
+// Ruby-level probe could see it. Reading the slot through verboseSlot here
+// removes the second representation rather than seeding it, so a future `delete`
+// or a fresh VM cannot re-open the same hole.
+func (vm *VM) warnEnabled() bool {
+	return !object.IsNil(vm.verboseSlot("$VERBOSE"))
+}
+
+// warningEnabled is rb_warning's stricter gate, `RTEST(ruby_verbose)`: only a
+// TRUE $VERBOSE. It is what ruby/spec's `complain(verbose: true)` arranges.
+func (vm *VM) warningEnabled() bool {
+	return truthyValue(vm.verboseSlot("$VERBOSE"))
+}
+
 // warnDeprecatedGvar emits a special-variable deprecation warning through
 // Warning.warn, but only while Warning[:deprecated] is enabled — error.c v3_4_0
 // deprecation_warning_enabled() requires both a non-nil $VERBOSE and the
 // category, and the category is off by default, so an ordinary run is silent.
 func (vm *VM) warnDeprecatedGvar(msg string) {
-	if object.IsNil(vm.verboseSlot("$VERBOSE")) {
+	if !vm.warnEnabled() {
 		return
 	}
 	w := vm.consts["Warning"]

internal/vm/io.go

@@ -511,7 +511,7 @@ func (vm *VM) registerIO() {
 		// `if (!NIL_P(ruby_verbose) && argc > 0)`, so a $VERBOSE of nil (the -W0
 		// level) makes Kernel#warn a no-op — nothing is written, nothing is
 		// validated and Warning.warn is not called. $VERBOSE false still warns.
-		if object.IsNil(vm.verboseSlot("$VERBOSE")) || len(pos) == 0 {
+		if !vm.warnEnabled() || len(pos) == 0 {
 			return object.NilV
 		}
 		lev := int64(0)
@@ -608,7 +608,7 @@ func (vm *VM) registerIO() {
 		// rb_io_s_new warns when handed a block it will not call: File.new always
 		// returns the open stream, and only File.open yields it.
 		if blk != nil {
-			vm.rbWarn("warning: File::new() does not take block; use File::open() instead")
+			vm.rbWarn("File::new() does not take block; use File::open() instead")
 		}
 		return vm.openFileArgs(fileRecvClass(self, cFile), args)
 	}}
@@ -1413,7 +1413,7 @@ func (vm *VM) writeArityIsOne(sink object.Value) bool {
 // whatever that #to_s returns. Measured — the first version of this function used
 // #to_s and printed the overridden value.
 func (vm *VM) warnOutdatedWrite(sink object.Value) {
-	if sink == vm.stderrValue() || vm.gvar("$VERBOSE") != object.True {
+	if sink == vm.stderrValue() || !vm.warningEnabled() {
 		return
 	}
 	if !vm.send(vm.consts["Warning"], "[]", []object.Value{object.SymVal("deprecated")}, nil).Truthy() {

internal/vm/io_class.go

@@ -5,7 +5,6 @@
 package vm
 
 import (
-	"fmt"
 	"os"
 	"strings"
 
@@ -1071,7 +1070,7 @@ func (vm *VM) extractEncodingOption(opts *object.Hash, ms *ioModeSpec) bool {
 		if !hasExt {
 			which = "internal"
 		}
-		vm.rbWarn("Ignoring encoding parameter '%s': %s_encoding is used", vm.displayStr(encV), which)
+		vm.rbWarnf("Ignoring encoding parameter '%s': %s_encoding is used", vm.displayStr(encV), which)
 		hasEnc = false
 	}
 	extName := ""
@@ -1125,24 +1124,6 @@ func isDashString(v object.Value) bool {
 	return ok && s.Str() == "-"
 }
 
-// rbWarn emits an MRI rb_warn-style warning line, but only when $VERBOSE is
-// non-nil (false or true) — the gate rb_warn applies. Used for the encoding
-// options that IO.new silently overrides.
-//
-// It goes through writeWarningStr (error.c rb_write_warning_str → Warning.warn),
-// like every other rb_warn here. It used to write to curStderr() instead, and the
-// comment that justified it — "still honouring a $stderr reassigned to a StringIO
-// (mspec's `complain` matcher)" — names the reason the defect survived: a StringIO
-// IS an *IOObj in rbgo, so it satisfied the type assertion curStderr() makes and
-// the harness's own double could never show the warning being dropped for a
-// $stderr that merely answers #write.
-func (vm *VM) rbWarn(format string, a ...any) {
-	if object.IsNil(vm.globals["$VERBOSE"]) {
-		return
-	}
-	vm.writeWarningStr(fmt.Sprintf(format, a...) + "\n")
-}
-
 // ioOptEncoding returns the canonical encoding name selected by the :encoding /
 // :external_encoding option, or "" when none is given.
 func (vm *VM) ioOptEncoding(opts *object.Hash) string {