Skip to content

fix: an END block records no splat, instead of claiming one at index 0 - #52

Merged
tannevaled merged 1 commit into
mainfrom
fix/exe-block-splat-index
Oct 9, 2026
Merged

tannevaled merged 1 commit into
mainfrom
fix/exe-block-splat-index

Conversation

@tannevaled

Copy link
Copy Markdown
Contributor

One line, and a Go zero value that means something

END { } desugars to at_exit { }, and the Block it built left SplatIndex
at Go's zero value. Zero is a perfectly good index, and this field's "no
splat" value is -1:

SplatIndex int // index of the top-level *splat param in Params, or -1

So the block announced a *splat parameter at position 0 with no parameters to
hold it.

Nothing here crashed, which is why it lasted

A parser does not run what it builds, so the cost landed entirely in a consumer.
go-embedded-ruby's compiler believed the claim and emitted a rest-binding that
read past an empty environment:

$ echo 'END { puts "E" }' > p.rb && rbgo p.rb
panic: runtime error: slice bounds out of range [:1] with capacity 0
$ echo $?
2

An uncontained Go panic from ordinary Ruby — END has been in the language
since 1.0 — which for an embedder takes the host process down rather than
returning an error. go-embedded-ruby/ruby#805.

Verified end to end with a replace directive. With this one line:

rbgo MRI 4.0.7
END { puts "E" } E E
puts "main" + END main E main E
BEGIN + main + END B main E B main E

The test that matters is the invariant, not the case

Two of the three &ast.Block literals in parser.go already set SplatIndex —
the two that build their value from parsed parameters. The one that forgot
is the one with no parameters to parse, and that is the shape any future
desugaring will have.

So TestEveryBlockHasAConsistentSplatIndex parses a corpus of seventeen sources
and asserts, over every Block in every tree, that SplatIndex is -1 or a
valid index into Params.

  • It walks by reflection rather than a type switch over the AST, so a node
    added later is covered without anyone remembering to extend the walker — not
    remembering is precisely the failure this guard exists to catch.
  • It fails when the walk finds no Block at all, because a walk that read
    nothing is indistinguishable from a tree with no defects.

Both tests were checked in the reverse direction: reverting the one-line fix
turns both red, with the guard naming the block and its bad index.

🤖 Generated with Claude Code

`END { }` desugars to `at_exit { }`, and the Block it built left SplatIndex at
Go's zero value. Zero is a perfectly good index, and this field's "no splat"
value is -1 -- so the block announced a *splat parameter at position 0 with no
parameters to hold it.

Nothing here crashed, because a parser does not run what it builds. The cost
landed entirely in a consumer: go-embedded-ruby's compiler believed the claim
and emitted a rest-binding that read past an empty environment.

    $ echo 'END { puts "E" }' > p.rb && rbgo p.rb
    panic: runtime error: slice bounds out of range [:1] with capacity 0
    $ echo $?
    2

An uncontained Go panic from ordinary Ruby -- in the language since 1.0 -- which
for an embedder takes the host process down rather than returning an error.
go-embedded-ruby/ruby#805.

Verified end to end with a replace directive: with this one line, `END { }`,
`main` + `END`, and `BEGIN` + `main` + `END` all match MRI 4.0.7 byte for byte.

Two of the three &ast.Block literals in this file already set SplatIndex -- the
two that build their value from parsed parameters. The one that forgot is the
one with no parameters to parse, which is the shape any future desugaring will
have, so the test that matters here is not the case but the INVARIANT:
TestEveryBlockHasAConsistentSplatIndex parses a corpus and asserts, over every
Block in every tree, that SplatIndex is -1 or an index into Params. It walks by
reflection rather than a type switch, so a node added later is covered without
anyone remembering to extend it -- not remembering is the failure it exists to
catch -- and it fails when the walk finds no Block at all, since a walk that
reads nothing looks exactly like a tree with no defects.

Both tests were checked in the reverse direction: reverting the one-line fix
turns both red.

Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
@tannevaled
tannevaled merged commit 9f3311f into main Oct 9, 2026
11 checks passed
@tannevaled
tannevaled deleted the fix/exe-block-splat-index branch October 9, 2026 19:08
tannevaled added a commit to go-embedded-ruby/ruby that referenced this pull request Oct 10, 2026
#809)

`END { }` -- ordinary Ruby, in the language since 1.0 -- ended the process with
an uncontained Go panic when the handler ran at exit:

    $ echo 'END { puts "E" }' > p.rb && rbgo p.rb
    panic: runtime error: slice bounds out of range [:1] with capacity 0
    $ echo $?
    2

For a program that is a crash instead of an answer. For an EMBEDDER it is worse:
ruby.Run does not return an error, it takes the host process down, and nothing
the host writes around the call prevents it. #805.

The cause was upstream and is one line. `END { }` desugars to `at_exit { }`, and
the Block the parser built left SplatIndex at Go's ZERO VALUE -- which is a
perfectly good index, where that field's "no splat" value is -1. So the block
announced a *splat parameter at position 0 with no parameters to hold it, and
this compiler's rest-binding believed it and read past an empty environment.
go-ruby-parser/parser#52, released as v0.13.1 (a patch: no exported API moved).

The regression test lives here as well as upstream, and that is the point rather
than belt and braces: the defect was INVISIBLE IN THE AST. The parser's own
suite was green throughout, because a parser does not run what it builds -- the
entire cost landed on the consumer that lowers a Block. A downgrade of this
dependency brings the crash back, and this is what notices.

It asserts the ORDERING too, not just the absence of a crash: END and at_exit
handlers run LIFO, measured on both engines (MRI 4.0.7 and rbgo agree, "second
registered" then "first registered"). BEGIN and at_exit are in the same test as
controls, so a regression in any of the three is attributable to the right half.

The README row added this morning said END crashes the process. It did, and this
is the commit that stops it, so the row goes in the same change -- a document
that is wrong on the day it ships is the thing yesterday's doc sweep was about.

Closes #805.

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