diff --git a/internal/vm/builtins.go b/internal/vm/builtins.go index 3624d91d..93336714 100644 --- a/internal/vm/builtins.go +++ b/internal/vm/builtins.go @@ -7129,14 +7129,10 @@ func nativePuts(vm *VM, self object.Value, args []object.Value, _ *Proc) object. } // The receiver already IS $stdout (a custom object whose #puts resolves to this // method): write straight to it, as MRI's rb_io_puts(argc, argv, recv) does, so - // the object's own #write receives the bytes. A $stdout that answers no #write - // (one MRI would have rejected at assignment) falls back to the underlying - // stream instead of raising. - if vm.respondsToDynamic(self, "write") { - vm.ioPuts(self, args) - } else { - vm.ioPuts(vm.curStdout(), args) - } + // the object's own #write receives the bytes. out came from stdoutValue, which + // has already applied the "lost its #write" fallback, so there is no second + // test of it here — the rule lives in stdSink alone. + vm.ioPuts(out, args) return object.NilV } @@ -7167,16 +7163,6 @@ func nativePrintf(vm *VM, _ object.Value, args []object.Value, _ *Proc) object.V return object.NilV } -// stdoutValue returns the object currently bound to $stdout (an IOObj, a -// StringIO, or a host-supplied object), falling back to the default stdout IO -// when the global is unset. -func (vm *VM) stdoutValue() object.Value { - if v, ok := vm.globals["$stdout"]; ok { - return v - } - return vm.curStdout() -} - // coerceFormatString returns the format string for Kernel#sprintf/#format/#printf: // a String directly, otherwise the result of the argument's #to_str (MRI coerces // the format argument with #to_str, never #to_s). A missing #to_str or a @@ -7205,13 +7191,10 @@ func (vm *VM) coerceFormatString(v object.Value) string { // terminated by the output record separator $\ when those globals are set. // Reference: ruby/ruby v3_4_0 io.c (rb_f_print -> rb_io_print). func nativePrint(vm *VM, _ object.Value, args []object.Value, _ *Proc) object.Value { + // stdoutValue already applies the "lost its #write" fallback, so #print needs no + // test of its own — the rule lives in stdSink. out := vm.stdoutValue() - // A $stdout that answers no #write (one MRI would have rejected at assignment) - // falls back to the underlying stream rather than raising. - if !vm.respondsToDynamic(out, "write") { - out = vm.curStdout() - } - write := func(s string) { vm.send(out, "write", []object.Value{object.NewString(s)}, nil) } + write := func(s string) { vm.ioWrite(out, s) } if len(args) == 0 { if lastLine := vm.gvar("$_"); !object.IsNil(lastLine) { write(vm.displayStr(lastLine)) @@ -7263,10 +7246,17 @@ func (vm *VM) pInspect(v object.Value) string { return v.Inspect() } +// nativeP implements Kernel#p — io.c rb_f_p (io.c:9089) → rb_p_write (io.c:9018), +// which writes the inspected form and rb_default_rs to $stdout as TWO pieces +// through rb_io_writev. rb_p_write's `RB_TYPE_P(r_stdout, T_FILE) && +// rb_method_basic_definition_p(…)` branch is not a semantic fork — both arms +// produce the same bytes and the same #write dispatch for a non-IO — so rbgo takes +// the general one. Writing through curStdout() instead, as this used to, dropped +// every `p` aimed at a $stdout that only answers #write. func nativeP(vm *VM, _ object.Value, args []object.Value, _ *Proc) object.Value { - o := vm.curStdout() + out := vm.stdoutValue() for _, a := range args { - o.writeStr(vm.pInspect(a) + "\n") + vm.ioWritev(out, vm.pInspect(a), "\n") } switch len(args) { case 0: @@ -12225,26 +12215,34 @@ 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: the message is written to stderr -// with the CALLING frame's "path:lineno: warning: " in front (rb_warn builds it -// through rb_warning_string, which starts at the current frame), and it is -// silenced only under -W0, where $VERBOSE is nil. `ruby -e 'p [1].index(1){}'` -// prints "-e:1: warning: given block not used". +// 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". +// +// 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")) { return } - vm.curStderr().writeStr(vm.warnUplevelPrefix(0) + msg + "\n") + vm.writeWarningStr(vm.warnUplevelPrefix(0) + msg + "\n") } -// rbWarning1 is error.c's rb_warning, 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`. +// 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 { return } - vm.curStderr().writeStr(vm.warnUplevelPrefix(0) + msg + "\n") + vm.writeWarningStr(vm.warnUplevelPrefix(0) + msg + "\n") } // symbolNames interns the frozen String that Symbol#name answers. MRI's symbol diff --git a/internal/vm/io.go b/internal/vm/io.go index 717cb575..ede1c99d 100644 --- a/internal/vm/io.go +++ b/internal/vm/io.go @@ -537,7 +537,14 @@ func (vm *VM) registerIO() { if !object.IsNil(uplevel) { b.WriteString(vm.warnUplevelPrefix(lev)) } - write := func(s string) { b.WriteString(s) } + // Kernel#warn assembles ONE message through rb_io_puts's line shaping and + // then hands it to Warning.warn, so the pieces rb_io_writev would have + // written separately are simply appended to the same buffer here. + write := func(ss ...string) { + for _, s := range ss { + b.WriteString(s) + } + } for _, a := range pos { vm.ioPutsValueRec(write, a, nil) } @@ -570,17 +577,10 @@ func (vm *VM) registerIO() { return object.NilV } } - // error.c rb_warn_category consults rb_warning_warn_arity(): a Warning.warn - // that takes exactly one argument is called WITHOUT the category keyword, - // so a program that overrides it with `def Warning.warn(message)` is not - // handed an argument it cannot take. - msg := object.NewString(b.String()) - if m := vm.resolveClassMethod(vm.consts["Warning"].(*RClass), "warn"); m != nil && methodArity(m) == 1 { - return vm.send(vm.consts["Warning"], "warn", []object.Value{msg}, nil) - } - kw := object.NewHash() - kw.Set(object.SymVal("category"), category) - return vm.send(vm.consts["Warning"], "warn", []object.Value{msg, kw}, nil) + // Dispatch through warningWarn, the one place that performs a Warning.warn + // call (and applies error.c rb_warn_category's arity rule) — the same door + // every internal rb_warn now uses via writeWarningStr. + return vm.warningWarn(object.NewString(b.String()), category) }) // File streams: File.open returns a buffered, file-backed IO carrying the @@ -1286,6 +1286,13 @@ func ioFlush(o *IOObj) { // curStdout / curStderr / curStdin return the IO currently bound to the global, // falling back to the raw VM writer when a host rebinds it to a non-IO value. +// +// These answer an *IOObj, so they are for the callers that genuinely need a +// stream OBJECT (Rack's rack.errors, Puma's log sink, an ARGF delegate). They are +// NOT the way to emit output: $stdout and $stderr are duck-typed in MRI and hold +// any object answering #write (see stdoutValue / stderrValue below), so a write +// dispatched off an *IOObj obtained here silently ignores such a binding. Emit +// through ioWrite / ioWritev / writeWarningStr instead. func (vm *VM) curStdout() *IOObj { return vm.curIO("$stdout", vm.out, "STDOUT") } func (vm *VM) curStderr() *IOObj { return vm.curIO("$stderr", vm.errOut, "STDERR") } @@ -1296,6 +1303,170 @@ func (vm *VM) curIO(global string, w io.Writer, label string) *IOObj { return &IOObj{cls: vm.consts["IO"].(*RClass), w: w, label: label} } +// stdoutValue and stderrValue answer the OBJECT $stdout / $stderr is bound to — +// io.c's rb_ractor_stdout / rb_ractor_stderr, read by stdout_getter (io.c:9222) +// and stderr_getter (io.c:9236) at v3_4_0. That object need not be an IO, and in +// general promises exactly one method: the setters stdout_setter (io.c:9215) and +// stderr_setter (io.c:9228) both begin +// +// must_respond_to(id_write, val, id); +// +// which admits ANY object answering #write and rejects the rest with +// +// TypeError: $stderr must have write method, Mute given +// +// So the standard streams are duck-typed at the language level, and every write +// below dispatches #write rather than assuming a stream object. +func (vm *VM) stdoutValue() object.Value { return vm.stdSink("$stdout", vm.curStdout) } +func (vm *VM) stderrValue() object.Value { return vm.stdSink("$stderr", vm.curStderr) } + +// stdSink resolves one of the two output globals to the object a write should be +// dispatched to, with one non-MRI fallback: a value that does NOT answer #write +// resolves to the underlying stream instead. MRI cannot reach that state through +// assignment (must_respond_to refuses), but it can be reached by removing the +// method afterwards — `$stdout = o; class << o; undef_method :write; end` — where +// MRI raises NoMethodError and rbgo keeps writing, so a Go host embedding the VM +// is not killed by a program mutating its own stream. That is an implementation +// choice, not a differential case. +func (vm *VM) stdSink(global string, fallback func() *IOObj) object.Value { + if v, ok := vm.globals[global]; ok && v != nil && vm.respondsToDynamic(v, "write") { + return v + } + return fallback() +} + +// ioWrite is io.c's rb_io_write (v3_4_0, io.c:2296) — in full: +// +// VALUE rb_io_write(VALUE io, VALUE str) { return rb_funcallv(io, id_write, 1, &str); } +// +// It is UNCONDITIONALLY a dynamic #write send. There is no fast path for a real +// IO at this level, and no caller of it chooses one: the fast path lives one +// level DOWN, inside IO#write's own C body (io_writev, io.c:2220), which is +// reached only because method lookup on a real IO lands there — and io_writev +// itself ends in `rb_funcallv(io, id_write, argc, argv)` the moment +// rb_io_check_io reports the receiver is not an IO ("port is not IO, call write +// method for it", io.c:2231). That is why this, and not a test at each call site, +// is where rbgo dispatches: the rule has exactly one home. +func (vm *VM) ioWrite(sink object.Value, s string) { + vm.send(sink, "write", []object.Value{object.NewString(s)}, nil) +} + +// ioWritev is io.c's rb_io_writev (v3_4_0, io.c:2302), the multi-piece form that +// rb_io_puts (io.c:8959) and rb_p_write (io.c:9018) use. The piece count is +// OBSERVABLE to a duck-typed stream, and MRI splits on the receiver's #write +// ARITY, not on the piece count alone: +// +// if (argc > 1 && rb_obj_method_arity(io, id_write) == 1) { … one call per piece … } +// return rb_funcallv(io, id_write, argc, argv); +// +// So `puts "hi"` on a `def write(*a)` stream sees write("hi", "\n") — TWO +// arguments — while a `def write(x)` stream sees write("hi") then write("\n"). +// Joining the pieces into one "hi\n" argument, as this file used to, is visible +// to any stream that records what it was handed. +// +// It takes at least one piece, and deliberately carries no guard for zero: MRI's +// rb_io_writev is never reached with argc 0 either (rb_io_puts always builds n >= 1, +// and its no-argument case goes through rb_io_write), so a guard here would be a +// branch no test could reach honestly. +func (vm *VM) ioWritev(sink object.Value, strs ...string) { + if len(strs) > 1 && vm.writeArityIsOne(sink) { + vm.warnOutdatedWrite(sink) + for _, s := range strs { + vm.ioWrite(sink, s) + } + return + } + args := make([]object.Value, len(strs)) + for i, s := range strs { + args[i] = object.NewString(s) + } + vm.send(sink, "write", args, nil) +} + +// writeArityIsOne reports whether sink's #write takes exactly one argument — +// rb_obj_method_arity(io, id_write) == 1 in rb_io_writev. A splat or an optional +// parameter gives a negative arity, which is not "one", so only a strictly +// single-argument #write triggers the split. +func (vm *VM) writeArityIsOne(sink object.Value) bool { + m := vm.findMethod(sink, "write") + return m != nil && methodArity(m) == 1 +} + +// warnOutdatedWrite emits rb_io_writev's deprecation notice (io.c:2306-2315) for +// a stream whose #write accepts only one argument. Its gates are MRI's three: +// +// - the receiver is not the current $stderr — warning ABOUT $stderr would +// re-enter the stream being complained about; +// - $VERBOSE is TRUE: rb_category_warning "reports only in verbose mode" +// (error.c:420, RTEST(ruby_verbose)), so a false $VERBOSE is not enough, +// unlike rb_warn's !NIL_P test; +// - the :deprecated category is enabled, which it is NOT by default in Ruby 3+, +// which is why this line is almost never seen. +// +// The separator names how #write was reached: '.' with the OBJECT itself when it +// lives on the object's singleton, '#' with the class otherwise +// (RCLASS_SINGLETON_P(klass) ? (klass = io, '.') : '#'). +// +// The name is rendered with INSPECT, not #to_s. The directive is `%+"PRIsVALUE"` +// and the '+' flag means rb_inspect, which is observable: an object carrying +// @marker and its own #to_s is named `#` by MRI, not by +// 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 { + return + } + if !vm.send(vm.consts["Warning"], "[]", []object.Value{object.SymVal("deprecated")}, nil).Truthy() { + return + } + name, sep := vm.moduleToSStr(vm.classOf(sink)), "#" + if sc := vm.objSingleton(sink); sc != nil && lookupMethod(sc, "write") != nil { + name, sep = vm.pInspect(sink), "." + } + vm.writeWarningStr(vm.warnUplevelPrefix(0) + name + sep + + "write is outdated interface which accepts just one argument\n") +} + +// writeWarningStr is error.c's rb_write_warning_str (v3_4_0, error.c:374): +// +// static void rb_write_warning_str(VALUE str) { rb_warning_warn(rb_mWarning, str); } +// +// and rb_warning_warn (error.c:341) is `rb_funcallv(mod, id_warn, 1, &str)` — a +// DYNAMIC send of Warning.warn. Every interpreter-generated warning ends here: +// rb_warn (error.c:466), rb_warning (:497), rb_enc_warn (:489), rb_compile_warn +// and rb_warn_deprecated all funnel through it. Two consequences follow, and +// rbgo used to get both wrong by writing to curStderr() directly: +// +// - a program that overrides Warning.warn intercepts rbgo's OWN warnings, not +// just the ones Kernel#warn raised; +// - $stderr is reached only by the DEFAULT Warning.warn, whose body is +// `$stderr.write(message)` — an ordinary send, so a $stderr that merely +// answers #write collects the warning. +// +// This is the single dispatch point for internal warnings, one level above +// $stderr, so the duck-typing rule is stated once here instead of at each +// emitter. A StringIO test double cannot witness either consequence: a StringIO +// IS an *IOObj in rbgo, so it satisfied the old type assertion and collected the +// warning either way. +func (vm *VM) writeWarningStr(msg string) { + vm.warningWarn(object.NewString(msg), object.NilV) +} + +// warningWarn performs the Warning.warn call itself, applying error.c +// rb_warn_category's arity rule (error.c:355): rb_warning_warn_arity() asks how +// many arguments the CURRENT Warning.warn takes, and a one-argument override is +// called WITHOUT the category keyword, so `def Warning.warn(message)` is never +// handed an argument it cannot accept. +func (vm *VM) warningWarn(msg *object.String, category object.Value) object.Value { + w := vm.consts["Warning"] + if m := vm.resolveClassMethod(w.(*RClass), "warn"); m != nil && methodArity(m) == 1 { + return vm.send(w, "warn", []object.Value{msg}, nil) + } + kw := object.NewHash() + kw.Set(object.SymVal("category"), category) + return vm.send(w, "warn", []object.Value{msg, kw}, nil) +} + // ioWriteAll writes every argument to o and returns the total byte count, the // shared core of IO#write and IO#write_nonblock. Each argument is coerced to a // String (rb_obj_as_string / #to_s); an all-empty write returns 0 before any @@ -2635,12 +2806,17 @@ func ioGetsParagraph(o *IOObj, limit int, chomp bool) object.Value { // ioPuts writes args to self with Kernel#puts semantics (arrays flattened, a // trailing newline added unless already present; no args ⇒ a lone newline). -// Every piece is emitted through self's #write method (via vm.send) rather than -// the buffer directly, so a stream that overrides #write sees puts's output, as -// in MRI; each value is also stringified through its (possibly user-defined) -// #to_s. +// Every piece is emitted through ioWritev — self's #write, dispatched — rather +// than the buffer directly, so a stream that overrides #write sees puts's +// output, as in MRI; each value is also stringified through its (possibly +// user-defined) #to_s. +// +// The line and its terminator go to ioWritev as TWO pieces, because rb_io_puts +// (io.c:8959) builds `args[]` that way and hands both to rb_io_writev in one +// call. A stream that records its arguments sees write("hi", "\n"), not +// write("hi\n"). func (vm *VM) ioPuts(self object.Value, args []object.Value) { - write := func(s string) { vm.send(self, "write", []object.Value{object.NewString(s)}, nil) } + write := func(ss ...string) { vm.ioWritev(self, ss...) } if len(args) == 0 { write("\n") return @@ -2654,10 +2830,10 @@ func (vm *VM) ioPuts(self object.Value, args []object.Value) { // self-referential array: puts recurses into nested arrays (each element on its // own line), so a member that is its own container is written as "[...]" (as MRI // does) rather than looping forever. seen tracks the arrays currently expanding. -func (vm *VM) ioPutsValueRec(write func(string), v object.Value, seen map[*object.Array]struct{}) { +func (vm *VM) ioPutsValueRec(write func(...string), v object.Value, seen map[*object.Array]struct{}) { if arr, ok := v.(*object.Array); ok { if _, rec := seen[arr]; rec { - write("[...]\n") + write("[...]", "\n") return } if seen == nil { @@ -2681,10 +2857,19 @@ func (vm *VM) ioPutsValueRec(write func(string), v object.Value, seen map[*objec return } } - if s := vm.displayStr(v); strings.HasSuffix(s, "\n") { + // rb_io_puts (io.c:8979-8990) appends rb_default_rs as a SECOND piece rather + // than concatenating it, and builds three shapes, all of which this mirrors: + // an EMPTY line writes the separator alone (`args[n++] = rb_default_rs`, n=1); + // a line already ending in a newline writes itself alone (n=1); anything else + // writes line and separator as two pieces (n=2). + s := vm.displayStr(v) + switch { + case s == "": + write("\n") + case strings.HasSuffix(s, "\n"): write(s) - } else { - write(s + "\n") + default: + write(s, "\n") } } diff --git a/internal/vm/io_class.go b/internal/vm/io_class.go index f2897ac0..018c0b1a 100644 --- a/internal/vm/io_class.go +++ b/internal/vm/io_class.go @@ -1125,16 +1125,22 @@ func isDashString(v object.Value) bool { return ok && s.Str() == "-" } -// rbWarn emits an MRI rb_warn-style warning line to the current $stderr, but only -// when $VERBOSE is non-nil (false or true) — the gate rb_warn applies. Writing to -// curStderr (rather than Kernel#warn) keeps the warning off stdout while still -// honouring a $stderr reassigned to a StringIO (mspec's `complain` matcher). Used -// for the encoding options that IO.new silently overrides. +// 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.curStderr().writeStr(fmt.Sprintf(format, a...) + "\n") + vm.writeWarningStr(fmt.Sprintf(format, a...) + "\n") } // ioOptEncoding returns the canonical encoding name selected by the :encoding / diff --git a/internal/vm/io_duck_stream_test.go b/internal/vm/io_duck_stream_test.go new file mode 100644 index 00000000..17f56f5e --- /dev/null +++ b/internal/vm/io_duck_stream_test.go @@ -0,0 +1,428 @@ +package vm_test + +import ( + "bytes" + "strings" + "testing" + + "github.com/go-embedded-ruby/ruby/internal/compiler" + "github.com/go-embedded-ruby/ruby/internal/vm" + "github.com/go-ruby-parser/parser" +) + +// evalDuck runs src on a VM whose program output and diagnostic stream are two +// DISTINCT buffers and returns both, plus any runtime error. Both are needed: +// the defect these tests witness did not lose the message, it sent it to the +// RAW DESCRIPTOR instead of to the Ruby object $stderr held, so a test that +// checked only "did the text appear somewhere" passed either way. +// +// The script is named so a warning's "path:lineno: " prefix matches what MRI +// prints for the same file. +func evalDuck(t *testing.T, src string) (out, errOut string, runErr error) { + t.Helper() + prog, err := parser.Parse(src) + if err != nil { + t.Fatalf("parse error: %v", err) + } + iseq, err := compiler.Compile(prog) + if err != nil { + t.Fatalf("compile error: %v", err) + } + iseq.Name = "duck.rb" + var o, e bytes.Buffer + machine := vm.NewWithStderr(&o, &e) + machine.SetScriptName("duck.rb") + _, runErr = machine.Run(iseq) + return o.String(), e.String(), runErr +} + +// duckClass defines, on ONE line, a stream double that answers ONLY #write. +// +// That is the whole point. MRI's $stdout/$stderr setters (io.c stdout_setter +// :9215, stderr_setter :9228) admit any object answering #write, and rb_io_write +// (io.c:2296) is an unconditional rb_funcallv(io, id_write, …). rbgo used to +// type-assert the global to *IOObj and fall back to the raw descriptor for +// anything else. +// +// A StringIO CANNOT witness that, because StringIO IS an *IOObj in rbgo: it +// satisfied the old type assertion and collected the output either way. The +// matchers that were supposed to catch this (mspec's `complain`/`output`) used a +// StringIO, which is why the defect survived. Every WITNESS test below therefore +// uses this class, never a StringIO. +// +// It keeps the bytes in @b and, separately, the ARGUMENT LISTS it was handed in +// @log, because the piece count rb_io_writev passes is observable and was also +// wrong. +const duckClass = `class Duck; def initialize; @b = +""; @log = []; end; def write(*a); @log << a.map(&:to_s); a.each { |x| @b << x.to_s }; a.sum { |x| x.to_s.bytesize }; end; def collected; @b; end; def log; @log; end; end` + "\n" + +// TestDuckTypedStreamsCollectInternalOutput is the witness for the wave-40 +// defect: internal warnings (error.c rb_warn / rb_warning, reached through +// rb_write_warning_str -> Warning.warn -> $stderr.write) and Kernel#p went to +// the raw descriptor whenever $stderr / $stdout held anything that was not an +// *IOObj, and MRI sends #write to whatever the global holds. +// +// Each name says WITNESS or GUARD, and the label is MEASURED by running this exact +// file against `origin/main` rather than assumed. Across this file's three tests: +// 11 WITNESS cases, all of which fail there, and 6 GUARD cases, all of which pass. +// Nothing is mislabelled in either direction. +// +// The GUARDs here cover the two shapes rb_io_puts already got right (a line that +// already ends in a newline, and an empty line) and the default-off :deprecated +// gate. They are present because the fix rewrote the piece-splitting that produces +// all three shapes — a change that could easily have made "always two pieces" out +// of a rule that is not. +// +// Every case asserts BOTH streams. That is not decoration: the old code did not +// lose the text, it wrote it to the raw descriptor, so an assertion on one buffer +// alone passes either way. One case here originally did exactly that and passed +// on the parent commit for the wrong reason (see "Kernel#p reaches a write-only +// $stdout"); it now routes its verdict through the other stream. +// +// Expected values are byte-for-byte from MRI 4.0.5 (ruby 4.0.5 +PRISM +// arm64-darwin25) running the same script. +func TestDuckTypedStreamsCollectInternalOutput(t *testing.T) { + cases := []struct { + name string + src string + wantOut, wantErr string + }{ + // error.c rb_warn via rbWarn1, reached from Array#fetch's + // `if (block_given && argc == 2) rb_warn("block supersedes default value + // argument")`. This is the case the wave was opened on. + { + name: "WITNESS rb_warn reaches a write-only $stderr", + src: duckClass + `$VERBOSE = true +s = Duck.new +$stderr = s +[1, 2].fetch(5, 1) { |i| i } +$stderr = STDERR +print s.collected`, + wantOut: "duck.rb:5: warning: block supersedes default value argument\n", + wantErr: "", + }, + // error.c rb_warning via rbWarning1 — the quieter sibling, which needs + // $VERBOSE == true rather than merely non-nil. Reached from + // rb_ary_initialize, which uses rb_warning rather than rb_warn for its + // "given block not used". + { + name: "WITNESS rb_warning reaches a write-only $stderr", + src: duckClass + `$VERBOSE = true +s = Duck.new +$stderr = s +Array.new { |i| i } +$stderr = STDERR +print s.collected`, + wantOut: "duck.rb:5: warning: given block not used\n", + wantErr: "", + }, + // io.c rb_f_p -> rb_p_write, which writes to $stdout, not $stderr: the same + // type assertion lived on that side too. + // + // The verdict goes to the DIAGNOSTIC stream, not to stdout. Writing it to + // stdout made this case pass on the parent commit for the wrong reason: the + // leak landed in the very buffer the assertion read, so "{a: 1}\n" appeared + // there whether the duck had collected it or not. Splitting the two means an + // empty duck now shows up as text in the wrong stream. + { + name: "WITNESS Kernel#p reaches a write-only $stdout", + src: duckClass + `s = Duck.new +$stdout = s +p({a: 1}) +$stdout = STDOUT +$stderr.write s.collected`, + wantOut: "", + wantErr: "{a: 1}\n", + }, + // rb_p_write hands the inspected form and rb_default_rs to rb_io_writev as + // TWO pieces (io.c:9020-9022). rbgo concatenated them into one argument. + { + name: "WITNESS Kernel#p writes value and separator as two pieces", + src: duckClass + `s = Duck.new +$stdout = s +p 7 +$stdout = STDOUT +p s.log`, + wantOut: `[["7", "\n"]]` + "\n", + wantErr: "", + }, + // rb_io_puts builds args[] the same way (io.c:8979-8990): line, then + // rb_default_rs, in one rb_io_writev call. + { + name: "WITNESS Kernel#puts writes line and separator as two pieces", + src: duckClass + `s = Duck.new +$stdout = s +puts "hi" +$stdout = STDOUT +p s.log`, + wantOut: `[["hi", "\n"]]` + "\n", + wantErr: "", + }, + // …and appends nothing when the line already ends in a newline (n stays 1), + // so the piece count is not simply "always two". + { + name: "GUARD Kernel#puts writes one piece when the line already ends in a newline", + src: duckClass + `s = Duck.new +$stdout = s +puts "hi\n" +$stdout = STDOUT +p s.log`, + wantOut: `[["hi\n"]]` + "\n", + wantErr: "", + }, + // An EMPTY line writes the separator ALONE (`args[n++] = rb_default_rs`), + // the third shape rb_io_puts builds. + { + name: "GUARD Kernel#puts writes the separator alone for an empty line", + src: duckClass + `s = Duck.new +$stdout = s +puts "" +$stdout = STDOUT +p s.log`, + wantOut: `[["\n"]]` + "\n", + wantErr: "", + }, + // rb_io_writev splits on the receiver's #write ARITY, not on the piece + // count: a strictly one-argument #write gets one call per piece + // (io.c:2304-2318). + { + name: "WITNESS an arity-one write receives one call per piece", + src: `class One; def initialize; @log = []; end; def write(x); @log << [x.to_s]; 1; end; def log; @log; end; end +s = One.new +$stdout = s +puts "hi" +$stdout = STDOUT +p s.log`, + wantOut: `[["hi"], ["\n"]]` + "\n", + wantErr: "", + }, + // error.c rb_write_warning_str is rb_warning_warn(rb_mWarning, str), a + // DYNAMIC send, so an overridden Warning.warn intercepts the interpreter's + // OWN warnings — not only the ones Kernel#warn raised. rbgo wrote past + // Warning.warn entirely, so the override never ran and the text still + // reached the descriptor. + { + name: "WITNESS an overridden Warning.warn intercepts an internal rb_warn", + src: `seen = [] +mod = Module.new do + define_method(:warn) { |msg, category: nil, **kw| seen << msg; nil } +end +Warning.extend mod +$VERBOSE = true +[1, 2].fetch(5, 1) { |i| i } +p seen`, + wantOut: `["duck.rb:7: warning: block supersedes default value argument\n"]` + "\n", + wantErr: "", + }, + // rb_io_writev's deprecation notice (io.c:2306-2315), whose three gates are + // the receiver not being $stderr, $VERBOSE == true, and the :deprecated + // category enabled — which is off by default, so this asserts the gates as + // much as the message. + { + name: "WITNESS an arity-one write draws the outdated-interface notice", + src: `class One; def initialize; @b = +""; end; def write(x); @b << x.to_s; 1; end; def collected; @b; end; end +out = One.new +err = Duck2 = Class.new { def initialize; @b = +""; end; def write(*a); a.each { |x| @b << x.to_s }; 1; end; def collected; @b; end }.new +$VERBOSE = true +Warning[:deprecated] = true +$stdout = out +$stderr = err +puts "hi" +$stdout = STDOUT +$stderr = STDERR +print err.collected`, + wantOut: "duck.rb:8: warning: One#write is outdated interface which accepts just one argument\n", + wantErr: "", + }, + // …and the same setup with :deprecated left at its DEFAULT draws no notice at + // all. rb_category_warning checks rb_warning_category_enabled_p, and + // :deprecated is off by default in Ruby 3+, which is why the line above is + // almost never seen in practice. This case is what makes that gate a + // measured claim rather than a comment. + { + name: "GUARD the outdated-interface notice stays silent with :deprecated off", + src: duckClass + `err = Duck.new +out = Object.new +def out.write(x); 1; end +$VERBOSE = true +$stdout = out +$stderr = err +puts "hi" +$stdout = STDOUT +$stderr = STDERR +print err.collected.inspect +print " " +print Warning[:deprecated].inspect`, + wantOut: `"" false`, + wantErr: "", + }, + // The same notice when #write lives on the object's SINGLETON: MRI switches + // the separator to '.' and names the object rather than its class + // (`RCLASS_SINGLETON_P(klass) ? (klass = io, '.') : '#'`). The name is + // rendered with rb_inspect -- the format's '+' flag -- which is why the + // object carries an ivar and an overridden #to_s here: MRI shows + // `#` and ignores the #to_s, and the first version of + // warnOutdatedWrite used #to_s and printed "TO_S_CALLED" instead. + // + // Only the prefix and suffix are asserted, because the middle is a heap + // address; pinning it would be pinning the allocator. + { + name: "WITNESS a singleton write is named with '.' and its inspect form", + src: duckClass + `err = Duck.new +out = Object.new +out.instance_variable_set(:@marker, 42) +def out.write(x); 1; end +def out.to_s; "TO_S_CALLED"; end +$VERBOSE = true +Warning[:deprecated] = true +$stdout = out +$stderr = err +puts "hi" +$stdout = STDOUT +$stderr = STDERR +c = err.collected +print c.start_with?("duck.rb:11: warning: #.write is outdated interface which accepts just one argument\n") +print " " +print c.include?("TO_S_CALLED")`, + wantOut: "true true false", + wantErr: "", + }, + } + for _, c := range cases { + t.Run(c.name, func(t *testing.T) { + out, errOut, err := evalDuck(t, c.src) + if err != nil { + t.Fatalf("runtime error: %v", err) + } + if out != c.wantOut { + t.Errorf("collected = %q, want %q", out, c.wantOut) + } + // The second half of the witness: on the parent commit the message was + // HERE instead, so a test asserting only `out` would pass both ways. + if errOut != c.wantErr { + t.Errorf("leaked to the raw descriptor = %q, want %q", errOut, c.wantErr) + } + }) + } +} + +// TestDuckTypedStreamFailureDirections pins the two directions MRI defines for a +// stream that cannot do the job. Both matched on the parent commit for the +// explicit paths ($stderr.write) and neither did for the internal ones, so the +// rb_warn rows are WITNESSES and the $stderr.write rows are GUARDS that the +// dispatch change did not weaken them. +func TestDuckTypedStreamFailureDirections(t *testing.T) { + cases := []struct { + name, src, wantErrMsg, kind string + }{ + // io.c must_respond_to (:9193), called by stderr_setter before the + // assignment takes effect. Note the class: MRI raises TypeError HERE, at the + // assignment, and never the NoMethodError a later #write would give. + { + kind: "GUARD", + name: "an object answering neither #write nor #puts cannot be assigned", + src: `class Mute; end +$stderr = Mute.new`, + wantErrMsg: "$stderr must have write method, Mute given", + }, + { + kind: "GUARD", + name: "the same refusal for $stdout", + src: `class Mute; end +$stdout = Mute.new`, + wantErrMsg: "$stdout must have write method, Mute given", + }, + // A #write that raises must propagate: rb_io_write does not rescue, so the + // exception escapes the warning site. On the parent commit the warning never + // reached this #write at all, so nothing was raised — this is a WITNESS. + { + kind: "WITNESS", + name: "a raising #write propagates out of an internal rb_warn", + src: `class Boom; def write(*a); raise RuntimeError, "boom from write"; end; end +$VERBOSE = true +$stderr = Boom.new +[1, 2].fetch(5, 1) { |i| i }`, + wantErrMsg: "boom from write", + }, + { + kind: "WITNESS", + name: "a raising #write propagates out of Kernel#p", + src: `class Boom; def write(*a); raise RuntimeError, "boom from write"; end; end +$stdout = Boom.new +p 1`, + wantErrMsg: "boom from write", + }, + { + kind: "GUARD", + name: "a raising #write propagates out of an explicit $stderr.write", + src: `class Boom; def write(*a); raise RuntimeError, "boom from write"; end; end +$stderr = Boom.new +$stderr.write "x"`, + wantErrMsg: "boom from write", + }, + } + for _, c := range cases { + t.Run(c.kind+" "+c.name, func(t *testing.T) { + _, _, err := evalDuck(t, c.src) + if err == nil { + t.Fatalf("expected an error containing %q, got none", c.wantErrMsg) + } + if !strings.Contains(err.Error(), c.wantErrMsg) { + t.Errorf("error = %v, want one containing %q", err, c.wantErrMsg) + } + }) + } +} + +// TestDuckTypedStreamsKeepWorkingForRealStreams is a GUARD, not a witness: it +// passes on the parent commit too. It exists because the change moved every write +// off the *IOObj fast path and onto a #write dispatch, and the thing most likely +// to break is the case that used to be the only one handled — the untouched +// standard streams, and a $stderr bound to a real IO. +func TestDuckTypedStreamsKeepWorkingForRealStreams(t *testing.T) { + cases := []struct { + name string + src string + wantOut, wantErr string + }{ + { + name: "the original $stderr still receives an internal warning", + src: `$VERBOSE = true` + "\n" + `[1, 2].fetch(5, 1) { |i| i }` + "\n" + `print "data"`, + wantOut: "data", + wantErr: "duck.rb:2: warning: block supersedes default value argument\n", + }, + { + name: "p and puts still reach the original $stdout", + src: "p 1\nputs \"two\"\nprint \"three\"", + wantOut: "1\ntwo\nthree", + wantErr: "", + }, + { + name: "a StringIO $stderr still collects a warning", + src: `require "stringio" +$VERBOSE = true +s = StringIO.new +$stderr = s +[1, 2].fetch(5, 1) { |i| i } +$stderr = STDERR +print s.string`, + wantOut: "duck.rb:5: warning: block supersedes default value argument\n", + wantErr: "", + }, + } + for _, c := range cases { + t.Run(c.name, func(t *testing.T) { + out, errOut, err := evalDuck(t, c.src) + if err != nil { + t.Fatalf("runtime error: %v", err) + } + if out != c.wantOut { + t.Errorf("stdout = %q, want %q", out, c.wantOut) + } + if errOut != c.wantErr { + t.Errorf("stderr = %q, want %q", errOut, c.wantErr) + } + }) + } +}