fix(codegen): PreallocateBoxes must not shadow a module-level global (#7521)

[proggeramlug]

Closes #7521.

Root cause

Not in diagnostics_channel, and not a closure-capture bug. traceCallback is
a bystander — the minimal repro has no traceCallback in it, and no array:

import { tracingChannel } from "node:diagnostics_channel";   // any import at all
{
  const events: string[] = [];
  function t1(a: any) { events.push("fn:" + a); }
  t1("A");
  console.log(JSON.stringify(events));   // node: ["fn:A"]   perry: []
}

Objects, numbers and strings lose the same way — inside t1, the captured
binding simply reads undefined, so acc.n++ throws
Cannot read properties of undefined and events.push(...) silently
no-ops. The import matters only because it makes the file an ES module,
hence strict, which is what selects the affected lowering. Everything else
about the failing gap test (traceCallback, subscribers, arrows, the array)
was incidental.

crates/perry-codegen/src/stmt/mod.rs, emit_preallocate_boxes: it allocated
a heap box — and, decisively, a ctx.locals slot — for every id in a
Stmt::PreallocateBoxes directive, including ids that
codegen/module_globals_emit.rs had already promoted to
@perry_global_<mod>__<id>.

Codegen already has a rule for this collision, spelled out identically at every
read and write site in expr/literals_vars.rs and stmt/let_stmt.rs:

ctx.boxed_vars.contains(id) && !ctx.module_globals.contains_key(id)

The module global wins — it is already the shared, forward-visible, GC-rooted
cell a box would provide. emit_preallocate_boxes was the one place that never
checked, and the stale ctx.locals entry it left behind is consulted before
ctx.module_globals on both the Stmt::Let reuse path (let_stmt.rs:322) and
the LocalGet/LocalSet store paths (literals_vars.rs:642, :814). So the
declaration wrote its value into the local box-pointer slot, the module global
was never stored, and the closure — which reads through the global, with zero
captures — saw the undefined the global was defined with.

The emitted IR, before:

%r2  = call i64 @js_box_alloc_bits(i64 <undefined>)   ; PreallocateBoxes([1])
%r8  = call i64 @js_closure_alloc(ptr @perry_closure_m3_ts__0, i32 0)
%r14 = call i64 @js_array_alloc(i32 0)                ; const events = []
store ... %r16, ptr %r3                               ; -> the box-pointer slot

@perry_global_m3_ts__1 is defined, registered as a GC root, read and written
by the closure — and never stored by main.

Why now

c6ed8175d (#6853) is not the window. lower_strict_block_fn_decls was added
by #7105 (76fa7b4c9, 2026-08-01), which made a bare block at the top level
of any ES module emit PreallocateBoxes for its let/const bindings. That is
the first time a promoted module-level id was ever handed to
emit_preallocate_boxes. The two commits the issue records as PASS
(17c0ff952, c6ed8175d, both 2026-07-30) both predate it, so the recorded
evidence is consistent — no bisect was needed once the diff was in view.

Fix

Skip promoted ids in emit_preallocate_boxes. The global is statically
initialized to TAG_UNDEFINED, which is exactly what a non-TDZ prealloc box
seeds, so nothing is lost. The TDZ variant is skipped too and the reason is
written into the code: module-global reads are a raw load double @g with no
js_box_get_bits choke point, so seeding TAG_TDZ there would leak the
sentinel into arithmetic instead of throwing a ReferenceError — strictly worse
than the undefined a forward read gets today.

Tests

crates/perry-codegen/src/stmt/prealloc_module_global_tests.rs — an in-src
unit test module, deliberately not crates/*/tests/*.rs, so it runs in the
per-PR cargo-test gate forever rather than only nightly (CLAUDE.md #5960).
The gap test that surfaced this (test_gap_diagchannel_3082_3084_3085_3086.ts)
is in neither gap_snapshot.json nor known_failures.json and parity is
tag-gated, which is how this sat unnoticed since 2026-07-30.

Four tests, and they are sabotage-checked rather than merely green — with the
ctx.module_globals guard reverted:

test	with guard	guard reverted
the_captured_module_binding_is_promoted_to_a_global (fixture premise)	ok	ok
a_preallocated_box_does_not_swallow_the_module_global_store	ok	FAILED
prealloc_is_a_no_op_for_a_promoted_binding	ok	FAILED
a_function_local_prealloc_still_gets_its_box (#569/#6044 not gutted)	ok	ok

The store assertion counts store … ptr @perry_global_… lines inside main()
specifically. A plain contains was not enough and was caught doing nothing:
main also takes the global's address for js_gc_register_global_root, which
is emitted whether or not the declaration ever writes the cell, so the first
draft of that test passed under sabotage.

Validation (local — CI has a deep backlog and may not report)

• test_gap_diagchannel_3082_3084_3085_3086.ts byte-matches
  node --experimental-strip-types (Node 26.5.1), exit 0, all four blocks
  (node:diagnostics_channel: forward all runStores callback arguments #3082, node:diagnostics_channel: reject symbol tracingChannel names #3084, node:diagnostics_channel: preserve non-callable bindStore transform errors #3085, node:diagnostics_channel: honor traceCallback position and arguments #3086) — not just the one that was failing.
• 5 hand-built reduction probes (traceCallback, plain channel, no channel at
  all, function decl / function expression / arrow, array / object / number /
  string accumulators) all match node exactly.
• cargo test -p perry-codegen --lib — 671 passed, 0 failed.
• cargo test -p perry-hir --lib — 281 passed, 0 failed.
• RUST_TEST_THREADS=1 cargo test -p perry-runtime --lib — 1812 passed, 0 failed.
• Parity sweep over every gap test containing a module-top-level bare block
  (the exact shape this touches) plus a broader gap slice — all pass.
• cargo fmt --all -- --check clean; scripts/check_file_size.sh OK;
  scripts/raw_handle_debt.py 998 (baseline 998, unchanged).

Scope note

The blast radius is wider than one gap test: every ES module with a
top-level { … } block containing a function declaration that reads a
sibling let/const compiled to a silently-empty binding. Nothing threw, so
this class of breakage was invisible except where a test happened to print the
accumulated value.

https://claude.ai/code/session_019EHcmXKArA7m42SihYCcgH

Summary by CodeRabbit

• Bug Fixes

  • Fixed module-global bindings being incorrectly allocated as local boxes, preventing empty or invalid bindings and preserving correct global access.
  • Ensured temporal dead zone handling remains correct for promoted module globals.
  • Preserved heap-box allocation for ordinary function-local captured bindings.

• Tests

  • Added regression coverage for module-global and captured-binding behavior.
  • Removed an outdated known-failure entry.

• Chores

  • Updated the documented and package version to 0.5.1328.

[coderabbitai]

Caution

Review failed

The pull request is closed.

ℹ️ Recent review info

⚙️ Run configuration

Configuration used: defaults

Review profile: CHILL

Plan: Pro Plus

Run ID: adc6d5f3-aeaf-471c-9ace-bd93a87adf6e

📥 Commits

Reviewing files that changed from the base of the PR and between b5c8e9c and 5b25de5.

⛔ Files ignored due to path filters (1)

• Cargo.lock is excluded by !**/*.lock

📒 Files selected for processing (6)

• CLAUDE.md
• Cargo.toml
• changelog.d/7580-prealloc-boxes-module-global.md
• crates/perry-codegen/src/stmt/mod.rs
• crates/perry-codegen/src/stmt/prealloc_module_global_tests.rs
• test-parity/known_failures.json

---

📝 Walkthrough

Walkthrough

This change prevents PreallocateBoxes from allocating local boxes for bindings promoted to module globals. It adds IR-level regression tests, removes the stale parity known-failure entry, updates the changelog, and increments the project version.

Changes

Module-global preallocation

Layer / File(s)	Summary
Codegen guard and regression coverage crates/perry-codegen/src/stmt/mod.rs, crates/perry-codegen/src/stmt/prealloc_module_global_tests.rs	emit_preallocate_boxes skips module-global identifiers, including TDZ requests. Regression tests verify global storage and continued box allocation for function-local bindings.
Release metadata and changelog changelog.d/7580-prealloc-boxes-module-global.md, test-parity/known_failures.json, CLAUDE.md, Cargo.toml	The fix is documented, the stale parity suppression is removed, and the project version changes from 0.5.1327 to 0.5.1328.

Estimated code review effort: 3 (Moderate) | ~20 minutes

Suggested labels: bug, parity

Suggested reviewers: thehypnoo

✨ 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/7521-tracecallback-lost-events

---

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.}

[proggeramlug]

Raw GC-ratchet A/B, both arms same session

Both arms built from one target directory with the identical -p perry -p perry-runtime-static -p perry-stdlib-static set, main arm produced by checking out origin/main's three touched files, measured back to back with gc_ratchet.py measure --repeats 3.

compared 156 cells across 12 probes

SEMANTIC differences (retention + evacuation counters): 1
  12_large_live_set   heap_used_bytes   main=59943752 mine=59946056  (+0.004%)

host-dependent differences (rss/wall): 36
  ... every rss/peak_rss delta within +-0.6%
  ... wall_ms faster on this branch on all 12 probes (-1.27% .. -13.25%)

The one semantic cell is 12_large_live_set.heap_used_bytes, the single cell the ratchet's own probe_override takes out of the gating family as conservative-stack-scan sample noise (#7554 for the gate, #7558 for the cause): documented spread 9,072 B over 36 runs, and this delta is 2,304 B. Every retention and evacuation counter on the other eleven probes is bit-identical, and every probe checksum matches.

check --profile shared_ci reports the same ten gating breaches with the same values on both arms:

probe	metric	baseline	both arms	delta
02_survivor_promotion	heap_used_bytes	9,418,232	9,678,792	+2.77%
03_cross_gen_writes	copied_objects	13,893	8,212	-40.89%
03_cross_gen_writes	copied_bytes	990,736	590,688	-40.38%
03_cross_gen_writes	promoted_objects	4,752	0	-100.00%
03_cross_gen_writes	promoted_bytes	210,736	0	-100.00%
04_dead_after_deep_stack	copied_objects	11,268	565	-94.99%
04_dead_after_deep_stack	copied_bytes	663,512	44,688	-93.26%
04_dead_after_deep_stack	promoted_objects	4,752	10	-99.79%
04_dead_after_deep_stack	promoted_bytes	210,744	440	-99.79%
05_closure_capture	heap_used_bytes	6,378,392	7,426,960	+16.44%

That last row is #7559 verbatim. It does not move here in either direction — this PR is not its cause and not its fix. The eight two-sided breaches on 03/04 are the "improvements wearing a two-sided band" #7559 describes.

Caveat stated plainly: this pair ran on the shared dev machine (load 42), so only the semantic families — which the harness documents at 0.000% spread and which are machine-independent — carry weight. The wall-time table in the PR body is from the pinned quiet host.

Correction to the issue brief, and one extra change

The brief said the gap test is in neither gap_snapshot.json nor
known_failures.json. That is right for gap_snapshot.json, but
known_failures.json does carry it — added 2026-07-04, "issue": "3082",
"reason": "diagnostics_channel cluster (#3082/#3084/#3085/#3086); standing per #5917.". So the two gates disagreed: the gap gate saw an untriaged failure, and
the tag-gated parity gate saw an accepted one.

That entry is now stale — the feature cluster it was filed for has long since
landed, and with this fix the test byte-matches
node --experimental-strip-types (26.5.1) with exit 0 across all four blocks.
scripts/parity_known_failures.py only computes failures - allowed, so unlike
gc_root_dominance_allowlist.json (where an entry matching nothing is a red
build) a stale entry here never fails — it would silence a future regression of
exactly the bug this PR fixes. Retired in 3dab6cf.

Caveat stated plainly: that verification is macOS/arm64 only. If the test turns
out to fail on Linux for an unrelated reason, it surfaces in the tag-gated
parity job rather than here.

[proggeramlug]

Audit before merge — verified independently, merged as v0.5.1328

Reproduced the minimal case on a clean build first, because the root cause
is a long way from the issue's title and worth confirming from scratch:

with    import: perry=[]         node=["fn:A"]     <- broken
without import: perry=["fn:A"]   node=["fn:A"]     <- fine

That is the whole bug in four lines, and it confirms the report's key
reframing: traceCallback is a bystander, and the import matters only because
it makes the file an ES module, hence strict, which selects the affected
lowering. It also explains why the three probes I posted on #7521 were all green
— none of them was a module.

This is much wider than one gap test. Every ES module with a top-level bare
block whose function declaration reads a sibling let/const compiled to a
silently-empty binding. Nothing threw. Six days on main.

Post-fix, verified on my own build: minimal repro correct, and the gap test
byte-identical to node 26.5.1, exit 0, all 23 lines.

Sabotage-verified the new tests myself. Commenting out the
ctx.module_globals.contains_key(id) guard takes exactly the two positive tests
red — a_preallocated_box_does_not_swallow_the_module_global_store and
prealloc_is_a_no_op_for_a_promoted_binding — while the fixture-premise test
and the #569/#6044 "still gets its box" control stay green. That is the right
shape: the controls prove the tests are not passing for a trivial reason.

Worth singling out, because it is the kind of thing that usually ships
unnoticed: the store assertion counts store … ptr @perry_global_… lines
inside main() rather than using contains, because main also takes the
global's address for js_gc_register_global_root — which is emitted whether or
not the declaration ever writes the cell. The first draft of that test passed
under sabotage for that reason. Catching your own vacuous assertion is the
part of this report I trust most.

Gates re-run here: raw_handle_debt 998 (baseline 998), check_file_size.sh
clean, cargo fmt --check clean.

One correction to the fragment, applied before merge. It stated the gap test
was in "neither gap_snapshot.json nor known_failures.json" while the same
fragment — and the diff — retire its known_failures.json entry. It was absent
from gap_snapshot.json and suppressed by a stale known_failures.json entry.
Reworded, since the fragment is the permanent record.

The suppression finding deserved its own ticket and now has one: #7582.
known_failures.json computes failures - allowed, so a stale entry is inert —
it never fails and never asks to be removed. That is a fifth way a gate can be
unable to fail, and the most insidious, because the job is genuinely green and
genuinely running. gc_root_dominance_allowlist.json already solves this in
this repo ("an entry that matches nothing FAILS the build"), so the fix is to
adopt that property, together with #797's per-entry provenance.