Repository navigation
ssa2ast: preserve deferred updates to named results - #1164
Merged
Merged
Conversation
Member
|
@luames please rebase |
The SSA-to-AST conversion allocates a fresh heap cell for a named result. Its generated return reads that cell before deferred calls mutate it, so the caller observes the old value. Record the current wrong result in the existing control-flow fixture, with a TODO to flip the assertion alongside the fix. Refs burrowers#1163
Control-flow rewriting converts named return variables into separate heap allocations. Generated return statements read those allocations before defers run, so a defer that changes a named result has no effect on the value returned to the caller. For a function with a defer, connect a named result to its SSA allocation when every return reads that same allocation. Writes through the allocation then update the actual named result, which Go returns after deferred calls finish. Extend the existing control-flow script for bare and explicit returns, multiple results, and a nested closure. Functions without defers retain the previous conversion path. Fixes burrowers#1163
luames
force-pushed
the
fix/named-defer-controlflow
branch
from
October 8, 2026 07:47
5ad1b01 to
0fb7401
Compare
luantak
approved these changes
Oct 8, 2026
Contributor
Author
|
Rebased onto the latest master, preserving both the named-result regressions and the upstream range-over-function tests. The focused control-flow test, ssa2ast/ctrlflow tests, and vet pass locally. All six CI jobs pass on the rebased head, |
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
2 participants
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.
Fixes #1163. The control-flow converter gives a named return value a separate heap cell, then generates a return that snapshots the cell before defers run. A deferred update to that result is lost even though plain Go returns the updated value.
When every SSA return reads the same allocation for a named result, connect that allocation to the declared result itself. Limit this change to functions with defers; conversion without defers is unchanged. The existing control-flow fixture now checks a bare return, both branches of an explicit/bare multiple-result function, and a named result in a nested closure. The assertion failed against the old converter at
exec ./mainwithpanic: deferred named result was lostand passed after the fix. A separate fixed-seed reproducer prints239 152 42under both plain Go and Garble; before the fix, Garble printed139 52 0.Tests:
GOTOOLCHAIN=go1.27.0 TMPDIR=/root/investigation-tmp go test -run '^TestScript$/^ctrlflow$' -count=1 .;go test ./internal/ssa2ast ./internal/ctrlflow;go vet ./internal/ssa2ast ./internal/ctrlflow; fixture Go source and production code checked withgofmt, andgit diff --checkpassed. No full local suite was run.Implemented by Hermes Agent using gpt-6-sol, acting on behalf of @luantak.