fix(codegen): keep scope groups out of a later switch case (#11771)

[proggeramlug]

Closes #11771

Root cause

scope_env::analysis numbers statements in pre-order and admits a binding to a scope context object only when every reference sits in [home, end(home list)]. list_end is recorded as the counter after the list, i.e. one past its last statement — the number of the next statement — but dominating_home compared it inclusively (hi > end rejects, so hi == end passed).

For a switch case, the next statement is the first statement of the following case, which is in the binding's lexical scope but skips its let. In the #4926 fixture (case 1: let x …; case 2: return typeof x) x was grouped, its scope object is allocated only by case 1's PreallocateBoxes, and entering at case 2 loaded the group root's entry value (TAG_UNDEFINED) and dereferenced it (vmovsd (%rax) with rax = 0x7ffc000000000001 in readSkipped$spec_i32).

Fix

Make the bound exclusive (>= end) for both declarations and references, and document list_end as exclusive. Such a binding falls back to its per-binding cell, whose frame slot is entry-initialized (#4926), so the skipped read is undefined and the skipped write a no-op, as before #11710. No GC-visible change: no new raw pointer, the group/cell paths themselves are unchanged.

Validation (perrymaster, dev profile)

• cargo test -p perry --test issue_4926_boxed_slot_skipped_init: SIGSEGV (139) before, passes after.
• New scope_env unit tests: the next-case read (pass shape and the lowered PreallocateBoxes-in-case shape) is refused, and a reference in the home list's last statement still groups. Both refusal tests fail with the old comparison.
• cargo test -p perry-codegen --lib: 1852 passed.
• Scope / box / arguments / TDZ / capture integration tests (13 suites) pass, except loop_lexical_tdz, whose commonjs-default-O0 arm fails identically with the fix reverted (exception transport failed (reason=3) on this Linux dev build), so it is pre-existing and unrelated.
• 18 scope / box / TDZ / switch / closure gap tests (test_gap_gc_scope_*, test_gap_gc_box_cells, test_gap_10464_box_cell_release, test_gap_*tdz*, test_gap_*switch*, test_gap_closures, …) under PERRY_GC_FORCE_EVACUATE=1 PERRY_GC_VERIFY_EVACUATION=1 match Node byte-for-byte.
• cargo fmt --all -- --check, RUSTFLAGS="-D warnings" cargo check -p perry -p perry-codegen -p perry-runtime --tests.

Summary by CodeRabbit

• Bug Fixes

  • Fixed variable handling across switch cases so that a skipped declaration no longer causes a later case’s read to return an unintended value.
  • Preserved scope grouping when references remain within the declaration’s case.

• Tests

  • Added regression coverage for switch cases with skipped declarations and references within the same case.

[coderabbitai]

Navigate logical layers of code changes, visualize relationships, and explore their blast radius.

No actionable comments were generated in the recent review. 🎉

ℹ️ Recent review info

⚙️ Run configuration

• Configuration used: defaults
• Review profile: CHILL
• Plan: Advanced
• Run ID: 2a044698-28e8-407c-a6ea-b6b11786c508

📥 Commits

Reviewing files that changed from the base of the PR and between 9f39f73 and 59a5f6e.

📒 Files selected for processing (3)

• changelog.d/11774-scope-ctx-switch-case-dominance.md
• crates/perry-codegen/src/scope_env/analysis.rs
• crates/perry-codegen/src/scope_env/tests.rs

Included review availability: This review used your included allowance. Your plan provides up to 8 included reviews per hour; 6 remain after this review.

---

📝 Walkthrough

Walkthrough

Scope analysis now uses an exclusive statement-list end when determining whether declarations and references belong to a binding’s dominance interval. Regression tests cover skipped switch-case bindings and an in-list control case.

Changes

Scope-context switch-case dominance

Layer / File(s)	Summary
Exclusive dominance bound crates/perry-codegen/src/scope_env/analysis.rs, changelog.d/11774-scope-ctx-switch-case-dominance.md	dominating_home now rejects declarations and references at the list end. The documentation and changelog describe the exclusive bound and its switch-case example.
Switch-case regression tests crates/perry-codegen/src/scope_env/tests.rs	Tests verify that a later-case read prevents grouping, including when the case begins with lowered preallocation. A control test verifies that a reference in the home list’s final statement still permits grouping.

Priority: ➖ Normal

Estimated code review effort: 2 (Simple) | ~10 minutes

Change: Bug fix · Severity of issue fixed: Medium

Merge Risk: ⚪ Minimal · up to 59a5f

No actionable merge-blocking issue is identified; the change is mergeable after normal checks.

Security Architecture Review

Security architecture risk: 🔵 Low · up to 59a5f

The change conservatively narrows an optimization and directs skipped-initialization cases through existing guarded storage access. No introduced security concern was identified in the inspected paths, but broader specialized execution paths were not fully verified.

Retained concerns
No architecture-level concerns identified.

Security review details

Security Blast Radius

• inferred — The evidenced security-relevant outcome is memory access in generated functions containing affected binding and switch-case shapes. Supplied program structure influences grouping, but the change narrows eligibility rather than expanding input authority. The evidence does not establish a remotely exposed service, tenant boundary, or deployment-wide attack scope.

Trust Boundaries and Controls

• observed — The fallback cell accessors validate the pointer's heap object start, cell type, size and forwarding status before dereferencing. Enclosing-function boxed reads explicitly select the checked accessor, providing the relevant control when a later case bypasses allocation.

Resilience and Maintainability Implications

• observed — The inspected lifecycle preserves fresh cell or scope-object allocation on repeated execution while reusing initialized frame slots. Release dispatch distinguishes grouped members from individual cells using the same storage-membership decision, avoiding a newly separate cleanup policy for rejected bindings.

🚥 Pre-merge checks | ✅ 5

✅ Passed checks (5 passed)

Check name	Status	Explanation
Linked Issues check	✅ Passed	The PR addresses the coding requirement in [#11771]. BodyFacts::can_group now treats the statement-list end as exclusive for declarations and references. A reference in a later switch case therefo…
Out of Scope Changes check	✅ Passed	The changed files stay within [#11771]. The implementation changes scope dominance, the tests verify the switch-case regression, and the changelog documents the fix. No unrelated behavior or unrelated…
Docstring Coverage	✅ Passed	Docstring coverage is 85.71% which is sufficient. The required threshold is 80.00%. Docstring coverage is scoped to functions touched by this diff. Analyzed 7 functions across 2 files. (1 skipped: 1 u…
Title check	✅ Passed	The title clearly and concisely describes the main change: keeping scope groups out of later switch cases.
Description check	✅ Passed	The description explains the root cause and fix, references issue #11771, and provides detailed validation results. It uses headings that differ from the repository template and omits the checklist, b…

✨ Finishing Touches

📝 Generate docstrings

• Commit to this branch
• Create a new PR

🧪 Generate unit tests (beta)

• Commit to this branch
• Create a new PR

• Autopilot · Keep fixing CodeRabbit findings and required CI, and resolving merge conflicts

Autopilot is currently an internal CodeRabbit preview.

---

Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out.

❤️ Share

• X
• Mastodon
• Reddit
• LinkedIn

_{Comment @coderabbitai help to get the list of available commands.}