Skip to content

Compiled async/async-gen/async-arrow block-scope shadow leaks (#766) + async-gen nested-block regression tests (#768)#798

Merged
nickna merged 1 commit into
mainfrom
wrk/lucid-brahmagupta-6e80a0
Jun 16, 2026
Merged

Compiled async/async-gen/async-arrow block-scope shadow leaks (#766) + async-gen nested-block regression tests (#768)#798
nickna merged 1 commit into
mainfrom
wrk/lucid-brahmagupta-6e80a0

Conversation

@nickna

@nickna nickna commented Jun 16, 2026

Copy link
Copy Markdown
Owner

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/const declared 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)
  • the hand-rolled AnalyzeAsyncArrow + AsyncArrowMoveNextEmitter (async arrows; added a Compute(Expr.ArrowFunction) overload and an _arrowBlockScopeRenames/ArrowStorageName pass 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 optional BlockScopeRenames field (null ⇒ treated as empty, e.g. for the manually-built arrow analyses).

async function af(): Promise<number> {
  const r = 100;
  { const r = 0; await Promise.resolve(r); }
  return r;        // was 0 (compiled), now 100
}

#768 — interpreter async generator (nested-block binding before 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) 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.

async function* ag(): AsyncGenerator<number> {
  const a = 100;
  { const b = 0; yield b; }   // yields 0 (was undefined on the old eager-drain driver)
  yield a;
}

(An earlier revision of this branch fixed the older eager-drain driver directly; that was dropped on rebase since the rewrite on main supersedes it. Likewise, an earlier build-break fix was dropped — main already 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

…) + 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.
@nickna nickna force-pushed the wrk/lucid-brahmagupta-6e80a0 branch from f2594d2 to e842f4f Compare June 16, 2026 20:00
@nickna nickna changed the title Fix async/async-gen block-scope shadow leaks (#766) + interp async-gen nested-block binding (#768) Compiled async/async-gen/async-arrow block-scope shadow leaks (#766) + async-gen nested-block regression tests (#768) Jun 16, 2026
@nickna nickna merged commit 6c1797e into main Jun 16, 2026
3 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

1 participant