Skip to content

fix(hir): suppress Annex B.3.3 legacy var across for-head/destructuring-catch lexicals (#5346)#5384

Closed
proggeramlug wants to merge 1 commit into
mainfrom
fix/5346-annexb-b33-residual
Closed

fix(hir): suppress Annex B.3.3 legacy var across for-head/destructuring-catch lexicals (#5346)#5384
proggeramlug wants to merge 1 commit into
mainfrom
fix/5346-annexb-b33-residual

Conversation

@proggeramlug

@proggeramlug proggeramlug commented Jun 18, 2026

Copy link
Copy Markdown
Contributor

Summary

Closes the dominant cluster from #5346 (follow-up to #5297/#5319): 94× An initialized binding is not created in test262 annexB/language.

These are the B.3.3 skip cases. A sloppy block-nested function f(){} gets an enclosing-scope legacy var f only when replacing it with var f would not be an early error. #5319's collector seeded the forbidden set with the body's own top-level lexicals and each descended block's let/const/class, but missed lexicals introduced by intermediate scopes between the body and the nested declaration:

  • a let/const for-headfor (let f; ;) { { function f(){} } }, for (let f in/of …) { … } — lexically scopes the loop body, so an enclosing var f collides → early error → var suppressed;
  • a destructuring catch parametercatch ({ f }) { { function f(){} } } — lexically binds its names. A simple catch (f) is exempt: Annex B.3.4 lets var f alias it, so it does not gate B.3.3.

Perry created the legacy var regardless, so f was observable (and a function after the block ran) where the spec keeps it unbound — reading f must throw ReferenceError and typeof f stay "undefined".

Fix

Seed the for-head (let/const only) and destructuring-catch lexical names into the forbidden set as the B.3.3 traversal descends into each of those bodies (annexb_nested_stmt in lower_decl/block.rs). Pure addition to the forbidden-set computation — no other lowering path is touched. The switch skip variant already worked (case lexicals were collected).

Validation

  • New behavior matches node v26 exactly for the for / for-in / for-of / switch / try shapes (f throws ReferenceError, typeof f === "undefined" before and after the block).
  • test262 annexB/language (--all-features, node v26 differential): all 112 function-code/global-code *skip-early-err-{for,for-in,for-of,switch,try} cases now pass; overall annexB parity 645/970 → 776/1026 (75.6%).
  • The eval-code variants of the same shape still fail — they need eval enablement (separate item in test262 annexB/language tail — 303 fails after #5319 (B.3.3 residual + missed-negatives + dynamic eval) #5346).
  • No regressions in the positive B.3.3 cases. cargo test -p perry-hir green.

Remaining #5346 clusters (out of scope here): 72× dynamic eval (AOT limit), 73× Annex B missed-negatives, and the existing-fn-no-init hoisting-precedence SameValue cluster.

🤖 Generated with Claude Code

Summary by CodeRabbit

  • Bug Fixes
    • Improved scoping correctness in for loops and try-catch blocks to ensure consistent behavior with JavaScript standards.

…ng-catch lexicals (#5346)

B.3.3 gives a sloppy block-nested `function f(){}` an enclosing-scope `var f`
ONLY when replacing it with `var f` would not be an early error. The collector's
`forbidden` set already accounted for the body's own top-level lexicals and each
descended block's `let`/`const`/`class`, but missed lexical bindings introduced
by intermediate scopes between the body and the nested declaration:

- a `let`/`const` for-head — `for (let f; ;) { { function f(){} } }`,
  `for (let f in/of …) { … }` — lexically scopes the loop body, so an enclosing
  `var f` collides and is an early error;
- a *destructuring* catch parameter — `catch ({ f }) { { function f(){} } }` —
  lexically binds its names (a *simple* `catch (f)` is exempt: Annex B.3.4 lets
  `var f` alias it, so it does not gate B.3.3).

Perry created the legacy `var` regardless, so `f` was observable (and a function
after the block ran) where the spec keeps it unbound — `f` must throw
ReferenceError and `typeof f` stay `"undefined"`. Seed the for-head and
destructuring-catch lexical names into the forbidden set as the traversal
descends into each of those bodies.

test262 `annexB/language` (node v26 differential, `--all-features`): all 112
`function-code`/`global-code` `*skip-early-err-{for,for-in,for-of,switch,try}`
cases now pass (the 94× "An initialized binding is not created" cluster);
overall annexB parity 645/970 -> 776/1026 (75.6%). The eval-code variants of the
same shape still need eval enablement (separate issue). Pure addition to the
B.3.3 forbidden-set computation — no other lowering path is affected.

Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
@coderabbitai

coderabbitai Bot commented Jun 18, 2026

Copy link
Copy Markdown

Review Change Stack

No actionable comments were generated in the recent review. 🎉

ℹ️ Recent review info
⚙️ Run configuration

Configuration used: defaults

Review profile: CHILL

Plan: Pro Plus

Run ID: 9c081f93-9c5f-44f3-9a40-fb2eed989ef2

📥 Commits

Reviewing files that changed from the base of the PR and between 2b30f9f and 7542a4c.

📒 Files selected for processing (1)
  • crates/perry-hir/src/lower_decl/block.rs

📝 Walkthrough

Walkthrough

A new internal helper annexb_forhead_lexical_names is added to collect let/const binding names from for-loop variable declaration heads. annexb_nested_stmt is updated to clone and extend the Annex B forbidden set with those names before traversing for, for..in, and for..of bodies, and to extend the forbidden set with destructured binding names when a catch parameter is an array/object pattern.

Changes

Annex B B.3.3 forbidden set refinements

Layer / File(s) Summary
annexb_forhead_lexical_names helper and for-loop forbidden set extension
crates/perry-hir/src/lower_decl/block.rs
Adds annexb_forhead_lexical_names to collect lexical binding names from let/const for-heads (returning early for var), and updates annexb_nested_stmt to clone and extend the forbidden set with those names before descending into for, for..in, and for..of bodies.
Destructuring catch parameter forbidden set extension
crates/perry-hir/src/lower_decl/block.rs
Modifies the try/catch branch of annexb_nested_stmt to extend the forbidden set with destructured binding names when the catch parameter is an object or array pattern; non-destructuring catch params leave the forbidden set unchanged.

Estimated code review effort

🎯 2 (Simple) | ⏱️ ~10 minutes

Possibly related issues

Possibly related PRs

  • PerryTS/perry#5319: Introduced the Annex B B.3.3 block-function name collection and forbidden-set traversal machinery in block.rs that this PR directly extends.
  • PerryTS/perry#5356: Targets the same annexb_nested_stmt forbidden-set logic in block.rs for for-head lexical bindings and destructuring catch parameters, directly overlapping with this PR's changes.

Poem

🐇 Hop through the for-loop, quick as a hare,
Let-bindings forbidden — now properly there!
A catch with destructuring? No var shall sneak past,
The Annex B rulebook is tight to the last.
This bunny enforces each scope with great care! 🌿

🚥 Pre-merge checks | ✅ 5
✅ Passed checks (5 passed)
Check name Status Explanation
Title check ✅ Passed The title accurately and specifically describes the main fix: suppressing Annex B.3.3 legacy var declarations across for-head and destructuring-catch lexicals.
Description check ✅ Passed The description includes all required sections (Summary, Changes, Related issue, Test plan) with substantive content demonstrating thorough testing and validation.
Docstring Coverage ✅ Passed Docstring coverage is 100.00% which is sufficient. The required threshold is 80.00%.
Linked Issues check ✅ Passed Check skipped because no linked issues were found for this pull request.
Out of Scope Changes check ✅ Passed Check skipped because no linked issues were found for this pull request.

✏️ Tip: You can configure your own custom pre-merge checks in the settings.

✨ Finishing Touches
📝 Generate docstrings
  • Create stacked PR
  • Commit on current branch
🧪 Generate unit tests (beta)
  • Create PR with unit tests
  • Commit unit tests in branch fix/5346-annexb-b33-residual
⚔️ Resolve merge conflicts
  • Resolve merge conflict in branch fix/5346-annexb-b33-residual

Comment @coderabbitai help to get the list of available commands and usage tips.

@proggeramlug

Copy link
Copy Markdown
Contributor Author

Closing as a duplicate — superseded by #5356 (commit 6964193), already merged to main, which fixes this exact #5346 B.3.3 skip-case cluster with a functionally identical approach (seed for-head + destructuring-catch lexicals into the forbidden set, with the Annex B.3.4 simple-catch exemption). My branch was based on a stale local main at the #5319 commit, so I didn't see #5356 had already landed. Rebased onto current main, this commit produces no net diff.

@proggeramlug proggeramlug deleted the fix/5346-annexb-b33-residual branch June 18, 2026 07:37
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.

1 participant