Repository navigation
aot: state the VCALL verdict in level-1 lowered code - #743
Merged
Merged
Conversation
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>
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>
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
The same source answered differently depending on whether it had been through
rbgo build.NameError: undefined local variable or method 'nope_xyz' for mainNameError✅NoMethodError❌A program changing meaning because it was compiled is the worst shape a divergence can take: neither lane is obviously wrong to look at, and nothing in the source says which one you are running.
Cause
internal/aot/codegen.golowersOpSendto a baredispatchSendpreceded bysetSendNoKW, so the call site's keyword verdict travelled into generated code and its 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 noOpSendhandler to inherit it from.Fix
VM.setSendVCalljoinssetSendNoKWas a seam generated code can call, and codegen emits both verdicts together.How it was reproduced — and why a first attempt could not have found it
Through
rbgo build, before and after. A runtime probe — call the method a few thousand times and watch for it to be lowered — can never see this: level-1 lowering happens at build time, not at execution, so that probe was measuring the interpreter no matter how long it ran.The control, read carefully
nope_xyz()answersNameErrorin both lanes on this branch. That is theast.Call.Parengap which #742 closes, not this one.What this change fixes is the two lanes disagreeing. Whether the answer they now agree on is MRI's depends on #742 landing.
Test
Both directions —
FlagSendVCallset and unset — because emitting the call with a constant would satisfy a one-sided check. Neither subtest skips, verified explicitly, since a skipped subtest reads as a pass.Ablated: with the codegen line reverted, both fail naming the missing verdict:
It also asserts the keyword verdict is still stated beside it, so adding one cannot displace the other.
Gates
go test ./...green on Go 1.27.1 withCGO_ENABLED=0;gofmtclean;GOOS=wasip1,GOOS=js GOARCH=wasmand-tags rbgo_closed ./internal/...all build.🤖 Generated with Claude Code