Skip to content

Fix spurious TDZ when a for-header reads a name the loop body shadows#2709

Merged
lahma merged 1 commit into
sebastienros:mainfrom
svenrog:fix/for-header-shadowed-lexical-tdz
Jul 18, 2026
Merged

Fix spurious TDZ when a for-header reads a name the loop body shadows#2709
lahma merged 1 commit into
sebastienros:mainfrom
svenrog:fix/for-header-shadowed-lexical-tdz

Conversation

@svenrog

@svenrog svenrog commented Jul 18, 2026

Copy link
Copy Markdown
Contributor

Summary

Fix spurious TDZ when a for-header reads a name the loop body shadows

Details

Loop-body lexical flattening (#2610) folds an eligible body block's let/const
bindings into the pooled loop environment. The eligibility gate (NamesOverlap)
only checked body names against the header's declared names — it missed the case
where a body name shadows an outer binding that a header expression references:

for (let l = r; l < o; l++) { let r = e[l], o = t.get(r); ... }

Here r/o in the header resolve to outer bindings, but flattening folded the
body's r/o slots into the loop env, so the header read them while still
uninitialized → ReferenceError: r has not been initialized. V8/SpiderMonkey/JSC
run it fine. This is the exact shape in Turbopack's module-factory runtime, so it
aborted rendering of Turbopack/Next.js bundles. The bug fires from the test and
update positions too, not just init.

Fix: add HeaderReferencesAnyBodyName, declining to flatten when any body slot
name appears as an identifier in the init/test/update. It over-approximates in the
safe direction (a matching property name or nested-scope declaration only forgoes
the optimization), mirroring EnvironmentEscapeAstVisitor.ClosureReferencesAny.
The scan runs once at statement construction, so there is no per-iteration cost,
and flattening still applies to loops whose headers reference no shadowed name.

Regression introduced in 4.12.0 (#2610); worked in 4.11.0.

Linked issue

Fixes #2708

Test plan

  • Added or updated unit tests in Jint.Tests (LoopBodyFlatteningTests: init/test/update header refs + a reduced Turbopack module-factory shape)
  • Ran dotnet test --configuration Release locally (3629 net10.0 / 3566 net472, 0 failures)
  • For ECMAScript spec changes: ran Jint.Tests.Test262 and confirmed no regressions (99431 pass, 0 fail, 135 skip)
  • For interop changes: covered in Jint.Tests/Runtime/Interop — n/a
  • For perf changes: included before/after numbers from Jint.Benchmark — n/a (narrows an existing optimization; construction-time guard only)

Breaking change?

No.

Loop-body lexical flattening (sebastienros#2610) folds an eligible body block's let/const
bindings into the pooled loop environment. The eligibility gate (NamesOverlap)
only checked body names against the header's declared names, missing the case
where a body name shadows an outer binding a header expression references:

    for (let l = r; l < o; l++) { let r = e[l], o = t.get(r); ... }

The header's r/o resolve to outer bindings, but flattening folded the body's
r/o slots into the loop env, so the header read them while still uninitialized
-> "r has not been initialized". V8 runs it fine. This is the shape in
Turbopack's module-factory runtime, so it aborted Turbopack/Next.js renders.

Add HeaderReferencesAnyBodyName, declining to flatten when any body slot name
appears as an identifier in init/test/update. Over-approximates in the safe
direction, mirroring EnvironmentEscapeAstVisitor.ClosureReferencesAny. The scan
runs once at construction, so there is no per-iteration cost, and flattening
still applies to headers that reference no shadowed name.

Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>

@lahma lahma left a comment

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thank you!

@lahma
lahma merged commit 3c7bb6c into sebastienros:main Jul 18, 2026
4 checks passed
@svenrog
svenrog deleted the fix/for-header-shadowed-lexical-tdz branch July 18, 2026 12:52
lahma added a commit that referenced this pull request Jul 22, 2026
…osure (#2739)

ForLoopMayCapture scanned init declarator initializers, test, update and
body for capturing closures, but never the declarator binding patterns
themselves — a destructuring default like

    for (let i = 0, { f = () => i } = {}; i < 3; i++) { fns.push(f); }

embeds a closure the scan missed, so the per-iteration environment was
reused/pooled in place and the escaped closure observed the final value
of i (3) instead of the captured iteration's (0, matching V8), or a
reset binding after the loop re-entered.

Same eligibility-scan family as the for-header TDZ fix (#2709); the
gate only declines an optimization, generic per-iteration semantics
take over.


Claude-Session: https://claude.ai/code/session_0115tQFNyyQqc1HQGPLUgZND

Co-authored-by: Claude Fable 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.

Issue with lexical scoping in for-loop per-iteration environments

2 participants