Skip to content

compiler: read HashLit.Braced and Call.Paren from go-ruby-parser v0.9.0 - #742

Merged
tannevaled merged 2 commits into
mainfrom
wave-i-parser-braced-paren
Sep 29, 2026
Merged

tannevaled merged 2 commits into
mainfrom
wave-i-parser-braced-paren

Conversation

@tannevaled

Copy link
Copy Markdown
Contributor

Consumes go-ruby-parser v0.9.0, released this morning with two fields added for exactly this: ast.HashLit.Braced (upstream #40/#42) and ast.Call.Paren (#41/#43).

Measured

2,206 *_spec.rb under language/ + core/, SPEC_SHA 87b1631, swept from a real path — a symlinked corpus answers differently, which is issue #741 — with the installed shim verified byte-identical to this branch's.

base 23,471
branch 23,482
+11 raw, +10 attributable, ZERO regressed
file
language/keyword_arguments_spec.rb 14 → 20 +6 — Braced
language/method_spec.rb 157 → 160 +3
language/send_spec.rb 71 → 72 +1 — Paren
core/random/bytes_spec.rb 8 → 9 the known 8/9 flake, not a mover

Every witness, both directions

def f(*a, **k) = [a, k]
f({x: 1})  # [[{x: 1}], {}]   positional
f(x: 1)    # [[], {x: 1}]     keywords
f(h)       # [[{y: 2}], {}]   a variable was always fine
f(**h)     # [[], {y: 2}]
f({})      # [[{}], {}]       an EMPTY POSITIONAL Hash
f()        # [[], {}]         nothing at all
nope_xyz      # NameError:     undefined local variable or method 'nope_xyz' for main
nope_xyz()    # NoMethodError: undefined method 'nope_xyz' for main
nope_xyz!     # NoMethodError  — tFID is an FCALL
nope_xyz?     # NoMethodError
nope_xyz { }  # NoMethodError  — a literal block makes an FCALL
1.nope_xyz    # NoMethodError

All byte-for-byte against ruby 4.0.5.

One guard removed

vm.vcallNameShape re-checked the name half of the VCALL rule inside the VM, because the compiler could not see the parentheses and set FlagSendVCall for nope! as well. With Paren the compiler states the whole rule, so the VM reads the flag alone. Two guards for one rule is how they drift apart.

Two things the change uncovered, fixed here because they are its consequences

The MSpec shim's complain took a keyword verbose: where upstream mspec takes a positional options Hash. It worked only because the compiler could not tell complain(re, {verbose: true}) from complain(re, verbose: true). Read correctly the braced form is positional, a keyword-only signature rejects it, and core/regexp/{new,compile}_spec.rb each went −2 with ArgumentError.

One signature covers all 46 call sites — the 18 written bare and the 2 braced — because Ruby converts keyword arguments to a positional Hash when the callee declares none, and rbgo does the same (measured against the oracle before relying on it).

Worth naming how this surfaced: the sweep reported those −2 while a direct run of the same spec reported 38. run.sh:66 copies its own spec_helper.rb over the corpus on every run, so the judge tree's shim was overwriting the corrected one. Same hazard as #741's path frame: the measurement carries state the pin does not cover.

cmd/freeze-prelude took its paths relative to the working directory, so go generate ./internal/vm — which is what TestEmbeddedPreludeMatchesSource tells you to run — could not work at all, since go:generate runs in the directory of the file carrying the directive. It now locates the module root itself, and a missing go.mod is fatal rather than a silent write to the wrong place. Both spellings verified and idempotent.

The frozen prelude is regenerated: 11 lines, all send flags — the compiler recording what it previously could not.

Gates

go test ./... green on Go 1.27.1 with CGO_ENABLED=0; gofmt -l and go vet clean; GOOS=wasip1, GOOS=js GOARCH=wasm and -tags rbgo_closed ./internal/... all build.

For whoever merges #739

The shim fix moves core/regexp/{new,compile}_spec.rb up by 2 each on any binary, so that PR's per-file baseline wants re-recording once this lands.

🤖 Generated with Claude Code

tannevaled and others added 2 commits September 29, 2026 10:17
Two written-form bits the parser did not carry before, both released in
v0.9.0 for exactly this, and both closing a gap this repository already
documented as needing them.

HashLit.Braced. `f({k: 1})` passes a POSITIONAL Hash and `f(k: 1)` passes
KEYWORDS; until now both parsed to the same *ast.HashLit, so lastArgIsPositional
answered false for either and rbgo read a braced literal at a call site as
keywords. Braced is MRI's own nd_brace bit -- `primary: tLBRACE assoc_list '}'`
sets it (parse.y-ruby_4_0:4415-4419) while rb_node_hash_new leaves it 0 for a
hash the grammar assembled from bare pairs (parse.y-ruby_4_0:11784-11791) -- and
MRI reads it back for this very question in aryset_check
(parse.y-ruby_4_0:13669).

hasTrailingKwSplat and compileSplatItemsMode consult it too: a braced hash is
never a keyword splat whatever it contains, because `f({**h})` writes a
positional Hash whose contents came from a splat.

Call.Paren. `nope` raises NameError and `nope()` raises NoMethodError, because
`fcall: operation` feeding `method_call: fcall paren_args`
(parse.y-ruby_4_0:3572-3577 and 5242-5248) builds NEW_FCALL where a bare name
reaching gettable builds NEW_VCALL (parse.y-ruby_4_0:13086). The compiler now
states the whole VCALL rule -- no receiver, no parentheses, no arguments, no
block, and a plain-identifier name, since `operation` admits tFID and `foo!`
is a parenthesis-less FCALL (parse.y-ruby_4_0:4370-4374, 6696-6698).

Measured on ruby 4.0.5 under BOTH --parser=parse.y and --parser=prism, which
agree on every row.

Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
Second half of the v0.9.0 bump. The first commit read ast.HashLit.Braced and
ast.Call.Paren; this one updates the tests that pinned the old answers, removes
a guard the exactness made redundant, and fixes two things the change uncovered.

MEASURED, 2206 *_spec.rb under language/+core/, SPEC_SHA 87b1631, swept from a
REAL path (a symlinked corpus answers differently -- issue #741) with the shim
verified byte-identical to this branch's:

    base   23471
    branch 23482      +11 raw, +10 attributable, ZERO regressed

    language/keyword_arguments_spec.rb  14 -> 20   (+6, Braced)
    language/method_spec.rb            157 -> 160  (+3)
    language/send_spec.rb               71 -> 72   (+1, Paren)
    core/random/bytes_spec.rb            8 -> 9    the known 8/9 flake, not a mover

Every witness matches ruby 4.0.5 byte for byte, both directions and every edge:
f({x: 1}) positional, f(x: 1) keywords, f(h) positional, f(**h) keywords, f({})
an empty POSITIONAL Hash where f() passes nothing; nope_xyz NameError, nope_xyz()
NoMethodError, nope_xyz! and nope_xyz? NoMethodError (tFID is an FCALL), a
literal block NoMethodError, an explicit receiver NoMethodError.

ONE GUARD REMOVED. vm.vcallNameShape re-checked the NAME half of the VCALL rule
in the VM, because the compiler could not see the parentheses and set the flag
for `nope!` too. With Paren the compiler states the whole rule, so the VM now
reads the flag alone: two guards for one rule is how they drift apart.

TWO THINGS THE CHANGE UNCOVERED, both fixed here because they are its
consequences, not separate work:

  - The MSpec shim's `complain` took a KEYWORD `verbose:`, where upstream mspec
    takes a POSITIONAL options Hash. It worked only because the compiler could
    not tell `complain(re, {verbose: true})` from `complain(re, verbose: true)`.
    Read correctly, the braced form is positional and a keyword-only signature
    rejects it -- core/regexp/{new,compile}_spec.rb went -2 each with
    ArgumentError. One signature covers all 46 call sites, the 18 bare-keyword
    and the 2 braced, because Ruby converts keywords to a positional Hash when
    the callee declares none, and rbgo does the same (measured).
  - cmd/freeze-prelude took its paths relative to the working directory, so
    `go generate ./internal/vm` -- which is what the drift test tells you to run
    -- could not work at all: go:generate runs in the directory of the file
    carrying the directive. It now finds the module root itself, and a missing
    go.mod is fatal rather than a silent write to the wrong place. Both spellings
    verified, and idempotent.

The frozen prelude is regenerated: 11 lines, all send flags, which is the
compiler now recording what it could not before.

NOTE FOR WHOEVER MERGES #739 (the per-file baseline): the shim fix moves
core/regexp/{new,compile}_spec.rb up by 2 each on ANY binary, so that baseline
wants re-recording after this lands.

Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
@tannevaled
tannevaled merged commit ed36c41 into main Sep 29, 2026
9 checks passed
@tannevaled
tannevaled deleted the wave-i-parser-braced-paren branch September 29, 2026 12:33
tannevaled added a commit that referenced this pull request Sep 29, 2026
* aot: state the VCALL verdict in level-1 lowered code

The same source answered differently depending on whether it had been through
`rbgo build`:

    def probe_bare; nope_xyz; end
    probe_bare
      MRI          NameError: undefined local variable or method 'nope_xyz'
      interpreted  NameError
      AOT-built    NoMethodError      <-- the lowered lane disagreed

internal/aot/codegen.go lowers OpSend to a bare dispatchSend preceded by
setSendNoKW, so the keyword verdict of the call site travelled and the VCALL
verdict did not. The level-2 lane already restates it per send (aotSend); this
is the level-1 lane, which lowers whole method bodies and has no OpSend handler
to inherit it from.

So VM.setSendVCall joins setSendNoKW as a seam generated code can call, and
codegen emits both verdicts together. Reproduced through `rbgo build` before and
after -- the runtime probe that first looked for this could never have found it,
since level-1 lowering happens at build time and not at execution.

The parenthesised form is the control worth reading carefully: `nope_xyz()`
answers NameError in BOTH lanes on this branch, which is the ast.Call.Paren gap
that #742 closes, not this one. What this change fixes is the lanes DISAGREEING;
whether the answer they agree on is MRI's depends on #742 landing.

The unit test asserts both directions, because emitting the call with a constant
would satisfy a one-sided check, and neither subtest skips. Ablated: with the
codegen line reverted, both fail naming the missing verdict.

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

* vm: exercise the setSendVCall seam, which only generated code calls

The per-function coverage ratchet failed this PR on both POSIX lanes, naming
one function:

    internal/vm/vm.go  setSendVCall  0.0% (was fully covered)

That is the gate working, and the diagnosis is exact: the seam is called only
from AOT-GENERATED Go, which no unit test compiles or runs, so nothing in the
suite reached it. Its sibling setSendNoKW is at 100% because three tests drive
it directly -- wave29_kwargs_test.go, aot_e2e_gen_test.go and
wave_g_vcall_test.go -- and the new one had no such test.

Covered by exercising the BEHAVIOUR, not the line: a compiled body of exactly
the shape codegen.go now emits for a bare identifier -- both verdicts stated,
then a bare dispatchSend -- following the pattern
TestWaveGVCallVerdictIsClearedForACompiledBody already uses. Both directions,
because a seam that always said "true" would satisfy one of them.

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

---------

Co-authored-by: Claude Opus 5 <noreply@anthropic.com>
tannevaled added a commit that referenced this pull request Oct 4, 2026
…statement's (#763)

A block ISeq's FirstLine is MRI's location.first_lineno -- the line the block's
`{`, `do` or `->` sits on, which Ruby shows through Proc#source_location. The
compiler took it from the enclosing scope's current line (c.push seeds
firstLine from the parent's curLine), and that is the same answer only when the
block opens on the enclosing statement's first line. So every block inside a
multi-line literal reported the literal's OPENING line:

    h = {                      # line 1
      :a => proc {             # line 2
        1
      },
      :b => proc { 2 },        # line 5
      :c => proc {             # line 6
      },
    }
    h[:a].source_location[1]   ruby 4.0.5  2    before  1
    h[:b].source_location[1]   ruby 4.0.5  5    before  1
    h[:c].source_location[1]   ruby 4.0.5  6    before  1

and the same for a multi-line array, a `do…end`, an arrow lambda and a default
in a multi-line parameter list -- 1, 8, 13 where ruby says 4, 10, 15.

The body's first line would NOT have done, which is why this needed the parser
and not a cheaper guess here: ruby answers 2 for :a, whose first statement is
on line 3, and :c has no body statement at all. The line was not in the tree,
so this could not be fixed downstream alone; parser v0.11.0 records it as
ast.Block.Line (go-ruby-parser/parser#46).

Block.Line == 0 means the parser recorded none -- a hand-built AST -- and the
inherited line stands, which is what it always was.

The control is that INSTRUCTION lines do not move. Backtraces, __LINE__ and
Kernel#caller inside a block were already right, and the fix also seeds the
block builder's curLine, so it could have shifted them. Measured unchanged, and
pinned: for a block opening on line 2 whose one statement is on line 3, the
scope's FirstLine is 2 and the instruction's line is 3.

The frozen prelude was regenerated, as its drift test requires. The same
`go generate ./internal/vm` also rewrites aot_e2e_gen_test.go, which has no
drift test and was already stale on main by the vcall change of #742 -- see
#762; its regeneration rides along here because the two generators share one
directive.

Closes #758

Co-authored-by: Claude Opus 5 <noreply@anthropic.com>
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