From 09d8711d7863158e0a27ea9496c545a5869a368e Mon Sep 17 00:00:00 2001 From: tannevaled Date: Thu, 5 Nov 2026 11:32:17 +0100 Subject: [PATCH] =?UTF-8?q?ObjectSpace.=5Fid2ref=20is=20deprecated=20in=20?= =?UTF-8?q?4.0=20=E2=80=94=20and=20the=20gate=20it=20needs=20was=20wrong?= =?UTF-8?q?=20already?= MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit One more of the 27 examples from #799. `core/objectspace/_id2ref_spec.rb` goes 0 → 1; the whole file is that one example at 4.0, the eleven others being `ruby_version_is ""..."4.0"`. gc.c calls rb_category_warn(RB_WARN_CATEGORY_DEPRECATED, "ObjectSpace._id2ref is deprecated"). The method still answers; it only says so. ## The find is not the deprecation, it is the gate Writing this meant copying the nearest precedent — `iterator?`, which rbgo already deprecates. Measuring that precedent against MRI 4.0.7 first showed it wrong in two ways, and copying it would have spread both: $VERBOSE = nil; Warning[:deprecated] = true; iterator? rbgo "…:13: warning: iterator? is deprecated; use block_given? instead" MRI "" MRI's deprecation_warning_enabled() wants a NON-NIL $VERBOSE *and* the category. rbgo checked the category alone, so it spoke where MRI is silent — and `$VERBOSE = nil` is precisely how a program asks for that silence. Measured in all four combinations: nil/on silent, false/on warns, true/off silent, true/on warns. Second, with a `Warning.warn` override installed: module Warning; def self.warn(msg, category: nil) = $seen << [msg, category]; end rbgo [..., nil] MRI [..., :deprecated] The category is not decoration: a handler that filters on it saw nil. Both are our code, so both are fixed here rather than filed. warnDeprecated is MRI's gate in one place — the two-condition test, the ":: warning: " head, Warning.warn with category: :deprecated — and `iterator?` now goes through it. ## Measured rubydiff against MRI 4.0.7, three probe programs: AGREE on all of them, including the four $VERBOSE × Warning[:deprecated] combinations and the Warning.warn interception. The argument check stays ahead of the warning, so `ObjectSpace._id2ref` with no argument raises ArgumentError in silence — a warning emitted first would make a plain mistake look like a deprecation. There is a test for it. warnDeprecated: 100.0% covered. ## Not done here The full internal/vm suite was NOT run locally for this change: the machine is at a 1-minute load average of 12.5, and the one run attempted hit #895 — TestUDPSocketLoopback hanging for 38 minutes until the suite's 40-minute timeout. CI runs it on this pull request. Co-Authored-By: Claude Opus 5 --- internal/vm/deprecation_warning_test.go | 106 ++++++++++++++++++++++++ internal/vm/globals.go | 28 +++++++ internal/vm/kernel_introspect.go | 8 +- internal/vm/objectspace.go | 6 ++ 4 files changed, 145 insertions(+), 3 deletions(-) create mode 100644 internal/vm/deprecation_warning_test.go diff --git a/internal/vm/deprecation_warning_test.go b/internal/vm/deprecation_warning_test.go new file mode 100644 index 00000000..45327097 --- /dev/null +++ b/internal/vm/deprecation_warning_test.go @@ -0,0 +1,106 @@ +// Copyright (c) the go-embedded-ruby/ruby authors +// +// SPDX-License-Identifier: BSD-3-Clause + +package vm + +import "testing" + +// capture installs a Warning.warn that records the message (with the file name +// folded away, so the expectation is about the LINE and the text) and the +// category, which is how MRI delivers rb_category_warn. +const capture = ` +$seen = [] +module Warning + def self.warn(msg, category: nil) + $seen << [msg.sub(/\A.*?:(\d+): /, 'L\1: '), category] + end +end +` + +// ObjectSpace._id2ref is deprecated in Ruby 4.0 — gc.c calls +// rb_category_warn(RB_WARN_CATEGORY_DEPRECATED, "ObjectSpace._id2ref is +// deprecated"). It still answers; it only says so. +// +// ⛔ THE GATE IS TWO CONDITIONS and only one is obvious. MRI's +// deprecation_warning_enabled() wants a NON-NIL $VERBOSE *and* +// Warning[:deprecated]. Measured on MRI 4.0.7: `$VERBOSE = nil` with the +// category on is silent; `$VERBOSE = false` with it on warns; `$VERBOSE = true` +// with the category OFF is silent. Every row below is one of those runs. +func TestId2refIsDeprecated(t *testing.T) { + for _, tc := range []struct{ name, src, want string }{ + {"it warns when the category is on", + capture + `Warning[:deprecated] = true +ObjectSpace._id2ref(nil.object_id) +p $seen`, + `[["L9: warning: ObjectSpace._id2ref is deprecated\n", :deprecated]]`}, + + {"the category reaches a Warning.warn override", + capture + `Warning[:deprecated] = true +ObjectSpace._id2ref(nil.object_id) +p $seen[0][1]`, + `:deprecated`}, + + {"it is silent when the category is off", + capture + `Warning[:deprecated] = false +ObjectSpace._id2ref(nil.object_id) +p $seen`, + `[]`}, + + // ⛔ $VERBOSE = nil silences it even with the category on. Checking the + // category alone — which is what this repository's `iterator?` did + // before warnDeprecated existed — makes rbgo speak where MRI says + // nothing, and nil is how a program asks for exactly that silence. + {"a nil $VERBOSE silences it", + capture + `Warning[:deprecated] = true +$VERBOSE = nil +ObjectSpace._id2ref(nil.object_id) +p $seen`, + `[]`}, + + {"a false $VERBOSE does not silence it", + capture + `Warning[:deprecated] = true +$VERBOSE = false +ObjectSpace._id2ref(nil.object_id) +p $seen.size`, + `1`}, + + {"a true $VERBOSE alone is not enough", + capture + `$VERBOSE = true +ObjectSpace._id2ref(nil.object_id) +p $seen`, + `[]`}, + + // The deprecation does not replace the behaviour. + {"it still answers", + capture + `Warning[:deprecated] = true +s = "hi" +p ObjectSpace._id2ref(s.object_id).equal?(s)`, + `true`}, + + // ⛔ The arity check stays FIRST: calling it with no argument raises + // without warning, as MRI does. A warning emitted before the check + // would make a plain mistake look like a deprecation. + {"a missing argument raises without warning", + capture + `Warning[:deprecated] = true +begin; ObjectSpace._id2ref; rescue ArgumentError; end +p $seen`, + `[]`}, + + // The same gate, on the method warnDeprecated was extracted from. + {"iterator? follows the same gate", + capture + `Warning[:deprecated] = true +$VERBOSE = nil +iterator? +$VERBOSE = false +iterator? +p $seen.map { |m, c| [m[/L\d+/], c] }`, + `[["L12", :deprecated]]`}, + } { + t.Run(tc.name, func(t *testing.T) { + if got := runSrc(t, tc.src); got != tc.want { + t.Errorf("%s\n got %s\nwant %s", tc.src, got, tc.want) + } + }) + } +} diff --git a/internal/vm/globals.go b/internal/vm/globals.go index 9826458c..bca7d9f0 100644 --- a/internal/vm/globals.go +++ b/internal/vm/globals.go @@ -283,6 +283,34 @@ func (vm *VM) warnDeprecatedGvar(msg string) { vm.send(w, "warn", []object.Value{object.NewString("warning: " + msg + "\n")}, nil) } +// warnDeprecated is error.c's rb_category_warn(RB_WARN_CATEGORY_DEPRECATED, …): +// the caller's file and line, then the message, carrying the :deprecated +// category, and emitted only when deprecation_warning_enabled() agrees. +// +// ⛔ THAT GATE IS TWO CONDITIONS, and only one of them is the obvious one. +// MRI requires a NON-NIL $VERBOSE *and* Warning[:deprecated]; measured on +// MRI 4.0.7, `$VERBOSE = nil` with the category on is silent, while +// `$VERBOSE = false` with the category on warns. Checking only the category — +// which is what `iterator?` did here until this function existed — makes rbgo +// speak where MRI says nothing, and `$VERBOSE = nil` is how a program asks for +// exactly that silence. +// +// The category is not decoration either: a `Warning.warn` override receives it +// as `category: :deprecated`, and a handler that filters on it saw nil from the +// old call site. +func (vm *VM) warnDeprecated(msg string) { + if !vm.warnEnabled() { + return + } + w := vm.consts["Warning"] + if w == nil || !vm.send(w, "[]", []object.Value{object.SymVal("deprecated")}, nil).Truthy() { + return + } + // warnUplevelPrefix already ends in "warning: " — it is the whole + // ":: warning: " head MRI prints, not just the location. + vm.warningWarn(object.NewString(vm.warnUplevelPrefix(0)+msg+"\n"), object.SymVal("deprecated")) +} + // gvarGet is variable.c's rb_gvar_get (variable.c:1052 at v4.0.9): the // RUBY-LEVEL read of a global, which is gvar plus the warning MRI emits for a // global nothing ever assigned. The getgvar instruction and the two AOT tiers diff --git a/internal/vm/kernel_introspect.go b/internal/vm/kernel_introspect.go index 03ec73ef..5931fe3f 100644 --- a/internal/vm/kernel_introspect.go +++ b/internal/vm/kernel_introspect.go @@ -218,9 +218,11 @@ func (vm *VM) registerKernelIntrospection() { 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") - } + // ⛔ This used to test the category alone and write the string itself, + // which warned where MRI is silent ($VERBOSE = nil) and handed a + // Warning.warn override `category: nil`. warnDeprecated is MRI's own + // gate; both divergences were measured against 4.0.7 before the change. + vm.warnDeprecated("iterator? is deprecated; use block_given? instead") return object.Bool(vm.callerBlockGiven()) }) // Both are rb_define_global_function, i.e. the module-function split: a PRIVATE diff --git a/internal/vm/objectspace.go b/internal/vm/objectspace.go index c9075ee5..f3038da2 100644 --- a/internal/vm/objectspace.go +++ b/internal/vm/objectspace.go @@ -209,6 +209,12 @@ func (vm *VM) registerObjectSpace() { if len(args) == 0 { raise("ArgumentError", "wrong number of arguments (given 0, expected 1)") } + // Deprecated in 4.0 (gc.c: rb_category_warn(RB_WARN_CATEGORY_DEPRECATED, + // "ObjectSpace._id2ref is deprecated")). It still answers; it only says + // so, and only when the :deprecated category is on — the argument check + // above stays FIRST, so `ObjectSpace._id2ref` with no argument raises + // without warning, as MRI does. + vm.warnDeprecated("ObjectSpace._id2ref is deprecated") id := intArg(args[0]) if id&1 == 1 { // odd => fixnum id 2n+1 return object.IntValue(id >> 1)