Skip to content

foobar() is an FCALL: parentheses stop a local from shadowing a method - #904

Merged
tannevaled merged 1 commit into
mainfrom
send-paren-fcall
Nov 6, 2026
Merged

tannevaled merged 1 commit into
mainfrom
send-paren-fcall

Conversation

@tannevaled

Copy link
Copy Markdown
Contributor

Closes one of the five failures in language/send_spec.rb from #799: pass=73 fail=5 → pass=74 fail=4.

foobar = 100
def foobar; 200; end

foobar      # 100, the local — correct
foobar()    # 100 as well. MRI says 200.

compileCall took the local-variable shortcut for any receiver-less, argument-less, block-less call (compiler.go:1423):

if v.Recv == nil && v.Block == nil && len(v.Args) == 0 {
    if depth, slot, ok := b.resolve(v.Name); ok {
        b.emit(bytecode.OpGetLocal, slot, depth)

foobar() matches all three conjuncts, so the read won over the method for every name the scope happened to have assigned.

⛔ The bit was already read thirty lines below

foobar() is fcall paren_args (parse.y-ruby_4_0:3572-3577, 5242-5248), which builds NEW_FCALL and never consults the local table. The vcall flag already says exactly this, cites the same productions, and reads ast.Call.Paren:

vcall := v.Recv == nil && v.Block == nil && !v.Safe && !v.Paren && len(v.Args) == 0 && vcallName(v.Name)

One bit, needed in two places, read in one of them. The fix is that conjunct.

Measured against MRI

cmd/oracle against /opt/homebrew/opt/ruby@4.0/bin/ruby (ruby 4.0.7 (2026-09-15 revision 229531a6cf) +PRISM [arm64-darwin25]): nine probe rows, treatment matching MRI byte for byte where the control differed on four. It is not only the spec's example:

written control treatment / MRI
only_local = 7; only_local() 7 NoMethodError
callable = ->{…}; callable() #<Proc (lambda)> NoMethodError
def v;9;end … [v, v(), self.v] [1, 1, 9] [1, 9, 9]

⛔ Control and treatment printed the same number of bytes on the first probe — 211 vs 211, because 100 and 200 are the same length. A comparison by length would have called this parity. cmd/oracle compares content, which is the reason it exists.

Negative control

With the conjunct disabled, 5 rows fail and 6 controls still pass: the bare name, a call with arguments, the NameError/NoMethodError miss pair, the eval'd binding local, and a block closing over a local. The controls are not passing vacuously.

The eval row is the one that matters for the other direction — the shortcut exists because the parser emits a Call, not a VarRef, for an identifier whose assignment it never saw, which is how an eval'd string reaches the binding locals seeded into its scope. Narrow the shortcut too far and that row fails.

go test ./...: 14 packages ok, 0 failures.

Also here: cmd/oracle stops reporting an agreement as its own defect

Measuring this change, the three-engine run printed:

=== the engines DISAGREED ===
  ruby vs rbgo-main:  stdout 150 vs 101 bytes
  ruby vs rbgo-fixed: nothing — which is a defect in this tool, not an agreement

differences() has carried that sentinel since it was written. It was right, and a sentence naming a defect is not a fix for it: the line kept printing — in the exact shape the tool exists for, reference / control / treatment — and answered which engine agrees backwards, on the line carrying the result.

The agreeing engine is now named as agreeing; sameAnswer is one definition shared with agreed(), so the pairwise check cannot drift from the all-engines one and report "the engines DISAGREED" while naming every pair as agreeing; and a three-engine row covers it.

⛔ The corpus sweep is running, not skipped

This changes how every parenthesised receiver-less call resolves, which is a far wider blast radius than a unit test can cover — a break would land in code that happens to have a local sharing a method's name, which is what a 2206-file corpus has and eleven table rows do not.

specdelta -dir core,language -move none -timeout 5m is running against rbgo at e5619211 as control. I will post it on this PR, with its KILLED on timeout count: non-zero means the load decided a count and the run is void.

(The first attempt was void for a different reason and is worth recording: I pointed it at a control built from 86d69f17, seven merged PRs behind, which would have attributed all seven to this one line. The second attempt ran with a treatment binary that a cancelled build had never produced — specdelta refused it with exit 2, a refusal, rather than reporting a clean table.)

Not fixed here, to be filed

def withblock; yield; end
withblock = 5
p(withblock { 7 })   # rbgo: parse error at line 3: expected ), got "{"   MRI: 7

It needs all three lines — the brace block alone parses — and rbgo at e5619211 fails identically, so it predates this change. It is out of this test and gets its own issue.

Refs #799

🤖 Generated with Claude Code

language/send_spec.rb: pass=73 fail=5 -> pass=74 fail=4.

    foobar = 100
    def foobar; 200; end
    foobar      # 100, the local -- correct
    foobar()    # 100 as well, and MRI says 200

compiler.go took the local-variable shortcut for any receiver-less,
argument-less, block-less call:

    if v.Recv == nil && v.Block == nil && len(v.Args) == 0 {
        if depth, slot, ok := b.resolve(v.Name); ok {

`foobar()` matches all three, so the read won over the method for every name
the scope happened to have assigned.

## The bit was already read thirty lines below

`foobar()` is `fcall paren_args` (parse.y-ruby_4_0:3572-3577, 5242-5248), which
builds NEW_FCALL and never consults the local table. The vcall flag already
says exactly this, cites the same productions, and uses ast.Call.Paren:

    vcall := v.Recv == nil && v.Block == nil && !v.Safe && !v.Paren && ...

One bit, needed in two places, read in one of them.

## Measured

cmd/oracle against /opt/homebrew/opt/ruby@4.0/bin/ruby (ruby 4.0.7 2026-09-15
revision 229531a6cf +PRISM): nine probe rows, treatment matches MRI BYTE FOR
BYTE where the control differed on four of them. Not just the spec's example:

    only_local = 7 ; only_local()     7              -> NoMethodError
    callable = ->{} ; callable()      #<Proc>        -> NoMethodError
    def v;9;end; v=1; [v, v(), self.v]  [1, 1, 9]    -> [1, 9, 9]

⛔ Control and treatment printed the SAME NUMBER OF BYTES on the first probe
(211 vs 211) -- 100 and 200 are the same length. A comparison by length would
have called this parity, which is why cmd/oracle compares content.

## Negative control

With the conjunct disabled, 5 rows fail and 6 CONTROLS STILL PASS -- the bare
name, a call with arguments, the NameError/NoMethodError miss pair, the eval'd
binding local, and a block closing over a local. The controls are not passing
vacuously, and the eval row is the one that would catch narrowing the shortcut
too far: the parser emits a Call, not a VarRef, for an identifier whose
assignment it never saw, which is how an eval'd string reaches binding locals.

go test ./... : 14 packages ok, 0 failures.

## Also here: cmd/oracle stops reporting an agreement as its own defect

Measuring this change, the three-engine run printed

    === the engines DISAGREED ===
      ruby vs rbgo-main:  stdout 150 vs 101 bytes
      ruby vs rbgo-fixed: nothing -- which is a defect in this tool, not an
                          agreement

differences() has carried that sentinel since it was written. It was RIGHT, and
a sentence naming a defect is not a fix for it: the line kept printing, in the
exact shape the tool exists for -- reference, control, treatment -- and
answered "which engine agrees" backwards, on the line carrying the result.

The agreeing engine is now named as agreeing, `sameAnswer` is ONE definition
shared with agreed() so a pairwise check cannot drift from the all-engines one,
and a three-engine row covers it.

## Not fixed here

`p(withblock { 7 })` after `withblock = 5` is a parse error in rbgo and prints
7 in MRI. It needs all three lines -- the brace block alone parses -- and
rbgo at e561921 fails identically, so it predates this change. Filed
separately; the row is out of this test.

Refs #799

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

Copy link
Copy Markdown
Contributor Author

The corpus sweep, as promised — and it closes four examples, not one

PROVENANCE
  control   rbgo-main0    — built from e5619211c5d8   (main)
  treatment rbgo-trtparen — built from b060cc65b9a9   (this branch)
MOVE none declared: both engines read the same corpus, so every delta below is the ENGINE

core/random/bytes_spec.rb       9 →   8   -1   ⛔ WENT RED
language/block_spec.rb        164 → 165   +1
language/def_spec.rb           63 →  64   +1
language/lambda_spec.rb        56 →  57   +1
language/send_spec.rb          73 →  74   +1

2206 file(s): control 23695 passing, treatment 23698 passing, +3
5 file(s) moved, 1 regressed, 1 example(s) WENT RED

-dir core,language -move none -timeout 5m, every file printed. KILLED on timeout: 0 — no count in that table was decided by the clock, which is the control that makes it quotable under a load average of 6–9.

⛔ The -1 is a coin flip, and it was proved rather than recognised

core/random/bytes_spec.rb is documented in scripts/conformance/rubyspec/README.md as answering 8 or 9 on the same binary — its BASELINE row is pinned to 8 by hand for exactly that reason. Recognising the filename is not a verdict, so it was judged:

$ specdelta -dir core/random -repeat 3 …
core/random/bytes_spec.rb   8   8   +0  ⛔ UNSTABLE: an engine disagreed with itself, delta excluded

10 file(s): control 65 passing, treatment 65 passing, +0
0 file(s) moved, 0 regressed

⛔ And the four gains were judged by the same rule

A flake that lands in your favour is just as false, and applying -repeat 3 only to the result one dislikes is how a gain gets banked that is not there. So the whole suite the gains are in was re-run at three runs per engine:

$ specdelta -dir language -repeat 3 …
language/block_spec.rb        164 → 165   +1
language/def_spec.rb           63 →  64   +1
language/lambda_spec.rb        56 →  57   +1
language/send_spec.rb          73 →  74   +1

80 file(s): control 2591 passing, treatment 2595 passing, +4
4 file(s) moved, 0 regressed

0 UNSTABLE, 0 WENT RED, 0 KILLED. The engine effect is +4; the +3 in the full run is those four minus one coin flip.

⭐ What the three unexpected files were asserting

The PR claimed one example. The corpus had the rule written four times, and rbgo failed all four — these are the exact descriptions that stopped failing:

file example
language/send_spec.rb Invoking a method | with same names as existing variables is ok
language/def_spec.rb An instance method with a default argument | calls a method with the same name as the local when explicitly using ()
language/block_spec.rb Post-args with optional args | calls an existing method with the same name as the argument if explicitly using ()
language/lambda_spec.rb A lambda literal -> () { } … with circular optional argument reference | calls an existing method with the same name as the argument…

Three of them say the same thing in different words — "explicitly using ()" — in three different grammatical contexts: a default argument, a post-arg with optionals, and a lambda's circular optional reference. All four failed with expected nil to == 1 or expected 100 to == 200, which is the local's value where the method's was due.

That is worth more than the +4: the corpus had already named this rule four times over, so the defect was not obscure, it was unlooked-for. Reading send_spec alone found one quarter of it.

Still failing in send_spec, unchanged by this PR

raises NameError with $! as a cause                                    — cause is nil
raises NoMethodError with $! as a cause                                — cause is nil
raises TypeError if block object isn't a Proc …                        — NoMethodError instead
raises a SyntaxError with both a literal block and an object as block  — nothing raised

Three defects: exception cause is not propagated; &obj without to_proc raises the wrong class (a 4.0 behaviour — the example is inside ruby_version_is "4.0"); and f(&blk) { } is accepted silently rather than refused, so which block wins is undefined. Each needs its own change.

🤖 Generated with Claude Code

@tannevaled
tannevaled merged commit 0b8ea8f into main Nov 6, 2026
26 checks passed
@tannevaled
tannevaled deleted the send-paren-fcall branch November 6, 2026 09:18
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.

1 participant