Skip to content

internal/vm: freeze refuses def, not only the reflective definers - #800

Merged
tannevaled merged 4 commits into
mainfrom
fix/frozen-def
Oct 9, 2026
Merged

tannevaled merged 4 commits into
mainfrom
fix/frozen-def

Conversation

@tannevaled

Copy link
Copy Markdown
Contributor

Closes #798.

The gap

freeze is how a library says "do not monkey-patch me after boot". rbgo
implemented the frozen check on every reflective definer and not on the
def keyword — which is the way anyone actually monkey-patches. The shape of
that is worse than a missing check: a test of the hardening, written the way one
naturally writes it (define_method, const_set), passed while the hardening
was absent
.

Measured, both engines

Oracle: ruby 4.0.7 (2026-09-15 revision 229531a6cf) +PRISM [arm64-darwin25].

operation MRI rbgo before rbgo after
"x".freeze << "y" FrozenError FrozenError unchanged
[].freeze << 1 FrozenError FrozenError unchanged
{}.freeze[:a] = 1 FrozenError FrozenError unchanged
o.freeze.instance_variable_set(:@a,1) FrozenError FrozenError unchanged
c.send(:define_method, :z) {} FrozenError FrozenError unchanged
c.send(:attr_accessor, :z) FrozenError FrozenError unchanged
c.send(:remove_method, :z) FrozenError FrozenError unchanged
Module.new.freeze.const_set(:Z,1) FrozenError FrozenError unchanged
o = Object.new.freeze; def o.foo; end FrozenError no raise FrozenError
o.define_singleton_method(:foo) {} FrozenError no raise FrozenError
c = Class.new.freeze; c.class_eval { def z; end } FrozenError no raise FrozenError
class << c; def bar; end; end FrozenError no raise FrozenError
c.instance_eval("def self.foo; end") FrozenError no raise FrozenError

Two details the oracle settled and reading did not

Placement. Every immediate reports frozen, and MRI answers def 1.foo with
TypeError: can't define singleton, not FrozenError. So the frozen test has
to run after the can-this-have-a-singleton test. Both orders "work"; only one
matches.

Which flag. For a singleton class the flag that counts belongs to the thing
it stands for, and rbgo records that in two different fields:

field kind of singleton reached by
attached a per-object singleton def o.foo on a frozen o
metaOf a class's metaclass class << c; def m; end; end

The first version of checkDefineFrozen handled only attached, and
class << frozen_class went straight through it — 11 of 12 rows green, which
is exactly the kind of near-miss that reads as done. The test carries that case
explicitly.

Scope

Three call sites, one helper each side:

  • internal/vm/vm.go — OpDefineMethod (covers def in a class/module body,
    under class_eval/module_eval/instance_eval, and inside class << obj)
  • internal/vm/object_model.go — defineSingletonMethod (def o.foo,
    def self.foo, OpDefineSMethod, OpDefineSingletonMethod)
  • internal/vm/singleton.go — define_singleton_method, which duplicates the
    shape rather than calling the above

A negative control is in the test: unfrozen def still defines, so a check that
simply refused everything would not pass.

🤖 Generated with Claude Code

tannevaled and others added 2 commits October 8, 2026 19:59
`freeze` is how a library says "do not monkey-patch me after boot". rbgo
implemented the frozen check on every REFLECTIVE definer -- define_method,
attr_accessor, remove_method, const_set, ivar set -- and not on the `def`
keyword, which is the way anyone actually monkey-patches. So the hardening
could be TESTED green with define_method while being absent in practice.

Measured, both engines, against MRI 4.0.7:

  o = Object.new.freeze; def o.foo; end        was: no raise
  o.define_singleton_method(:foo) {}           was: no raise
  c = Class.new.freeze; c.class_eval { def z; end }   was: no raise
  class << c; def bar; end; end                was: no raise
  c.instance_eval("def self.foo; end")         was: no raise

all five now raise FrozenError with MRI's message, while the eight rows that
already agreed (String/Array/Hash mutation, ivar set, define_method,
attr_accessor, remove_method, const_set) are unchanged.

Two details the measurement settled and reading would not have:

PLACEMENT. Every immediate reports frozen, and MRI answers `def 1.foo` with
TypeError "can't define singleton", not FrozenError -- so the frozen test runs
AFTER the can-this-have-a-singleton test.

WHICH FLAG. For a singleton class the flag that counts belongs to the thing it
stands for, and rbgo records that in two different fields: `attached` for a
per-object singleton, `metaOf` for a class's metaclass. The first version of
checkDefineFrozen handled only `attached`, and `class << frozen_class` went
straight through it; the test now carries that case explicitly.

Closes #798.

Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
The conformance ratchet refused the first version at +26 net examples with
THREE regressions, all the same shape:

  core/false/singleton_method_spec.rb: 0 passing, baseline 1
  core/nil/singleton_method_spec.rb:   0 passing, baseline 1
  core/true/singleton_method_spec.rb:  0 passing, baseline 1

nil, true and false answer `frozen?` with true -- they are immutable -- and MRI
nonetheless ALLOWS a singleton method on them, because it lands on
NilClass/TrueClass/FalseClass. isFrozen answers Ruby's question, so a guard
built on it refuses exactly those three.

What a definition guard needs is "was this frozen by someone", which is a flag.
explicitlyFrozen reads the flag and nothing else; the values that cannot carry
one are not its business, since whether they may have a singleton class is
ensureSingleton's question and MRI answers it with TypeError.

Measured on MRI 4.0.7 while settling it, including one row that looked like a
contradiction and was not:

  nil/true/false .define_singleton_method   OK
  1 / :s / 1.5                              TypeError: can't define singleton
  "str".freeze                              TypeError  <- an interned fstring,
  String.new("ab").freeze                   FrozenError   not a frozen-ness rule
  [].freeze / {}.freeze / Object.new.freeze FrozenError

The three shapes are now negative controls in the test, so the regression
cannot come back silently.

Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
@tannevaled

Copy link
Copy Markdown
Contributor Author

The ratchet refused the first version, and it was right

ruby/spec ratchet on 1e32e7a: +26 examples net, and 3 regressions,
all one shape:

-- REGRESSED (3) --
   core/false/singleton_method_spec.rb: 0 passing, baseline 1 (-1)
   core/nil/singleton_method_spec.rb:   0 passing, baseline 1 (-1)
   core/true/singleton_method_spec.rb:  0 passing, baseline 1 (-1)

nil, true and false answer frozen? with true — they are immutable —
and MRI nonetheless allows a singleton method on them, because it lands on
NilClass/TrueClass/FalseClass. The guard was built on isFrozen, which
answers Ruby's question, so it refused exactly those three.

A definition guard needs "was this frozen by someone", which is a flag — not
"is this frozen by nature", which is a type. explicitlyFrozen reads the
flag and nothing else.

Measured on 4.0.7 while settling it, including one row that read as a
contradiction and was not:

receiver MRI
nil / true / false .define_singleton_method OK
1 / :s / 1.5 TypeError: can't define singleton
"str".freeze TypeError — an interned fstring, not a frozen-ness rule
String.new("ab").freeze FrozenError
[].freeze, {}.freeze, Object.new.freeze FrozenError

The three shapes are now negative controls in the test, so this cannot come back
silently.

Had the gate been a scalar total this would have merged: +26 against a
single floor is a clear improvement, and the three losses are invisible inside
it. The per-file ratchet (#739) is what made them attributable.

🤖 Generated with Claude Code

tannevaled and others added 2 commits October 9, 2026 08:26
…release badge rotting

The ratchet reported 20 files improved and nothing else moved -- no regression,
no new file, no change of loading -- so BASELINE records them and the README's
three claims follow: 23,483 -> 23,512.

HOW it was measured matters here. This machine was carrying a desktop load of
12, and scripts/conformance/rubyspec/README.md says in its own words to record
the baseline on an unloaded machine, because a heavy file that would pass can
exceed the per-file TIMEOUT and be recorded as not loading -- which bakes the
load into the gate. The load was not mine to clear (Transmission, glpi-agent, a
peer session), so the twenty deltas come from the CI run on 496c39d
(ubuntu-latest, TIMEOUT=60) and were applied here rather than re-measured.

Three checks before writing: the pre-edit sum matched the 23,483 the README
claimed, every one of CI's "baseline" columns matched the value recorded in
this file, and the post-edit sum equals the 23,512 CI reported.

Separately, the release badge said v0.1.0 while the latest release was v0.2.0,
cut four days ago, and no test watched it. Rather than retype the number, the
badge now asks shields.io for the latest release: a figure a human maintains by
hand is a figure that rots, which is the same reason the example count has a
test. That one is guarded, and the guard was checked in both directions -- it
fails for the badge and for each of the two prose claims.

Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
… by asking

The macos lane refused the previous commit:

    -- NEWLY BELOW 100% (1) --
       internal/vm/builtins.go explicitlyFrozen  90.0% (was fully covered)

One branch, `case *Regexp`. A named function is a QUESTION -- is the branch
dead, or merely unreached? -- and answering it found a defect rather than dead
code.

Unreached, not dead: a literal /a/ is born frozen in both engines, so defining
on it already raised, and the branch runs. What was missing was a test.

Writing that test turned up the second half. Object#freeze had no *Regexp arm
while isFrozen had read Regexp.frozen all along, so:

    Regexp.new("a").freeze.frozen?   rbgo: false    MRI: true

The literal case hid it, because a literal needs no freeze call to be frozen.
Found by comparing the two switches TERM BY TERM rather than spot-checking:
Regexp was the only type present in one and absent from the other, and a
sampled comparison would have had a good chance of missing exactly it.

Four cases, including the negative control that an unfrozen Regexp still takes
a singleton method -- without it, marking everything frozen would pass.

Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
@tannevaled

Copy link
Copy Markdown
Contributor Author

The coverage gate refused it, and asking why found a defect

-- NEWLY BELOW 100% (1) --
   internal/vm/builtins.go explicitlyFrozen  90.0% (was fully covered)

One branch: case *Regexp. A named function is a question — is the branch
dead, or merely unreached? — and here the answer was the second, with something
behind it.

Unreached, not dead. A literal /a/ is born frozen in both engines, so
defining on it already raised and the branch runs. What was missing was a test.

Writing that test found the defect. Object#freeze had no *Regexp arm,
while isFrozen had read Regexp.frozen all along:

Regexp.new("a").freeze.frozen?    # rbgo: false    MRI 4.0.7: true

The literal case hid it, because a literal needs no freeze call to be frozen —
so every test anyone would naturally write passed.

It was found by comparing the two switches term by term rather than spot
checking. Regexp was the only type present in one and absent from the other,
and a sampled comparison had a good chance of missing exactly it.

Four cases added, including the negative control that an unfrozen Regexp still
takes a singleton method — without it, marking everything frozen would pass.
explicitlyFrozen, checkDefineFrozen and checkSingletonDefineFrozen are all
back at 100.0%.

🤖 Generated with Claude Code

@tannevaled
tannevaled merged commit 95ac275 into main Oct 9, 2026
9 checks passed
@tannevaled
tannevaled deleted the fix/frozen-def branch October 9, 2026 12:54
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

freeze does not stop def: a method can be defined on a frozen object, class or module

1 participant