Compiled async/async-gen/async-arrow block-scope shadow leaks (#766) + async-gen nested-block regression tests (#768)#798
Merged
Conversation
…) + async-gen nested-block regression tests (#768) #766 (compiled async functions, async generators, async arrows): wire the reusable GeneratorBlockScopeRenamer (#711) into AsyncStateAnalyzer/ AsyncMoveNextEmitter, AsyncGeneratorStateAnalyzer/AsyncGeneratorMoveNextEmitter, and the hand-rolled async-arrow analysis (AnalyzeAsyncArrow) + AsyncArrowMoveNextEmitter. A nested-block let/const that shadows an enclosing body-level binding (or parameter) now gets its own per-binding storage instead of aliasing the outer binding's hoisted state-machine field. Each analyzer keys hoisting by storage name; each emitter retokens the declaration/reference/operator nodes before resolution (a renamed binding is never a captured/DC name, so the existing function-DC routing falls through unchanged). Added a Compute(Expr.ArrowFunction) overload to the renamer and an optional BlockScopeRenames field to the async analyses. New emitter overrides registered in EmitterSyncTests.AllowedOverrides. #768 (interpreter async generator: nested-block const/let read at a yield before the first await): already resolved on main by the merged lazy-coroutine async-generator rewrite (which drives the body through the real Interpreter.ExecuteBlockAsync, so nested blocks scope correctly). This PR adds regression tests confirming and guarding it in both interpreter and compiler. (An earlier revision of this branch fixed the older eager-drain driver directly; dropped on rebase onto current main, where the rewrite supersedes it.) #767 (closure-captured shadow) intentionally deferred: a correct fix needs the program-wide ClosureAnalyzer + name-keyed display classes to understand block-scope renames — out of scope here (astronomically rare, low priority). Pinpointed blocker documented on the issue. Tests: new AsyncBlockScopeShadowTests (18, interpreter + compiler). Touched-area sweep (async/generator/block-scope/emitter-sync/yield) 1899 passing.
f2594d2 to
e842f4f
Compare
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.
Closes #766. Closes #768.
Extends the block-scope shadow disambiguation from #711 (compiled generators) to the async suspension state machines, and adds regression coverage for the interpreter async-generator nested-block case. Rebased onto current
main.#766 — compiled async functions, async generators, async arrows
A
let/constdeclared in a nested block that shadows an enclosing body-level binding (or parameter) aliased the outer binding's hoisted state-machine field instead of getting its own slot — the async analog of #711.The reusable
GeneratorBlockScopeRenamer(node-keyed, deterministic, no AST mutation) is now consulted by:AsyncStateAnalyzer/AsyncMoveNextEmitter(async functions)AsyncGeneratorStateAnalyzer/AsyncGeneratorMoveNextEmitter(async generators)AnalyzeAsyncArrow+AsyncArrowMoveNextEmitter(async arrows; added aCompute(Expr.ArrowFunction)overload and an_arrowBlockScopeRenames/ArrowStorageNamepass through the bespoke walker)Each analyzer keys its hoisting decision by storage name; each emitter retokens the declaration / reference / operator node before resolution, so a shadow lands on its own field/local. A renamed binding is never a captured/DC name, so the existing function-DC routing falls through unchanged. New emitter overrides are registered in
EmitterSyncTests.AllowedOverrides. The async analyses gained an optionalBlockScopeRenamesfield (null ⇒ treated as empty, e.g. for the manually-built arrow analyses).#768 — interpreter async generator (nested-block binding before first await)
Already resolved on
mainby the merged lazy-coroutine async-generator rewrite, which drives the body through the realInterpreter.ExecuteBlockAsync(so nested blocks scope correctly) rather than a bespoke statement walker that defined into the generator's root closure. This PR adds regression tests confirming and guarding it in both modes.(An earlier revision of this branch fixed the older eager-drain driver directly; that was dropped on rebase since the rewrite on
mainsupersedes it. Likewise, an earlier build-break fix was dropped —mainalready carries the equivalent fix.)#767 — deferred (intentionally)
The residual #711 case (a nested-block shadow whose name is also captured by a nested closure) requires the program-wide
ClosureAnalyzer+ name-keyed display classes to understand block-scope renames — a deep, broad-blast-radius change for a case the issue itself marks "astronomically rare; low priority." Kept open; the precise blocker is documented in a comment on the issue.Testing
AsyncBlockScopeShadowTests— 18 tests across interpreter + compiler (async fn / arrow / gen shadowing; Interpreter async generator: nested-block const yielded before any await reads as undefined #768 distinct-name cases).