refactor(codegen): Layer 1 slice 1 — migrate lower_array_method.rs onto the rooting API (#7615)

[proggeramlug]

Layer 1 campaign slice 1 (#7615): crates/perry-codegen/src/lower_array_method.rs migrated end to end onto crate::rooting, following the template #7617 established. 1 module, 1199 → 1346 lines, 37 raw sites, 40 hazard sites — the map's highest density. The whole module lands in one PR; the ledger notes no outstanding boundary.

The hazard was structural, not incidental

lower_array_method lowered the receiver above the match:

let recv_box = lower_expr(ctx, object)?;      // NaN-boxed array pointer
match property { … }                          // every arm then lowers ITS OWN arguments

So every one of the ~30 arms held a heap pointer in an SSA register across the lowering of its arguments. For any argument that runs user code — a callback literal (js_closure_new allocates), a call, an array literal — that is #7453's window, in arr.map(cb), arr.filter(cb), arr.sort(cmp), arr.concat(f()), arr.splice(i, n, mk()). The module had zero rooting references before this.

What is actually new, stated against #7280

This is the part worth reading, because the module was partially protected already and the PR should not claim otherwise.

root_reload.rs (#7280) is a post-pass that re-reads a shadow slot below every collection point that can run under it. Where the receiver is a shadow-slotted local, it already fired — visible in the baseline IR as an out-of-sequence register (%r309 = load double, ptr %r7 emitted after %r250). So for const a = [...]; a.sort(cmp) the old code was not stale.

What #7280 structurally cannot cover, and what this migration closes:

1. A receiver reassigned by its own argument. GC: #7154's residual is NOT fixed — the loop-polls config is red 0/30, and stock zod alone fails 5/40 #7280 deliberately bails when a store to the slot can run in the window, because re-loading would observe the assignment — operand_is_reloadable's documented miscompile. It leaves the register alone, and the register is stale. Baseline IR, verbatim:

   %r243 = load double, ptr @perry_global_p_unrooted_recv_ts__1   ; the receiver
   %r244 = call double @perry_fn_p_unrooted_recv_ts__reassign()   ; allocates AND reassigns
   %r245 = bitcast double %r243 to i64                            ; stale
   %r246 = and i64 %r245, 281474976710655
   %r249 = call i64 @js_array_concat_variadic(i64 %r246, ptr %r247, i32 1)

   After:

   %r249 = load double, ptr @perry_global_p_unrooted_recv_ts__1
   %r250 = bitcast double %r249 to i64
   store i64 %r250, ptr %r37                        ; root store, ABOVE the window
   call void @js_shadow_slot_bind(i32 3, ptr %r37)
   %r251 = call double @perry_fn_p_unrooted_recv_ts__reassign()
   %r252 = load i64, ptr %r37                       ; re-read, BELOW it
   %r254 = and i64 %r252, 281474976710655
   %r257 = call i64 @js_array_concat_variadic(i64 %r254, ptr %r255, i32 1)
   store i64 0, ptr %r37                            ; release

   A temp root is the only strategy that gives both the call-time value and a rewritten address. Same shape on indexOf.

2. Argument-to-argument windows. arr.splice(mk(), mk(), mk(), mk()) held each evaluated argument in a register across the next one's lowering; no slot, so nothing to reload.

3. A dead duplicate unbox in sort. The comparator path unboxed the receiver above the if, lowered the comparator, then unboxed again. %r243 = and i64 %r242, 281474976710655 in the baseline is referenced exactly once — its own definition. Removed; it is the only instruction this PR deletes.

How it is migrated

One rooting::with_operands_rooted around the whole match, over the receiver plus the arguments the arm consumes. Those are declared in one place, lowered_arg_count, and the arms then only emit — lower_expr does not appear in this file, which is what makes "no operand register crosses a collection point" a property of the module rather than of each arm's author.

The asymmetry that makes the table safe is stated in its doc: under-counting is loud (an arm indexes a value that is not there and panics during codegen, on the first program that reaches it), over-counting is benign (JS evaluates every argument anyway, so lowering one the arm ignores is a spec fix at worst). Counts are exactly what each arm lowered before, so nothing changes in this slice. Two unit tests state the table independently and assert it never claims more than it was given.

IR-identity evidence

Program set: 7 purpose-built probes reaching all 46 of this module's distinctive callees — every arm including the thisArg array-like family, the runtime-dispatch family, the typed-array set/subarray arms, and the two receiver shapes #7280 cannot reach. Compiled PERRY_RS4GC=0 PERRY_GC_MOVING_LOOP_POLLS=1 PERRY_INLINE_SHADOW_SLOT=0 PERRY_NO_AUTO_OPTIMIZE=1 --trace llvm, PERRY_RUNTIME_DIR pinned, both arms.

• 95 functions compared; 90 identical, 5 differ — and all 5 are main, the only function in each probe containing an array-method call. 2 of 7 modules are identical outright.

• Whole-corpus net instruction delta, every line of it:

  +55 / +53 / +53 / +53	store i64 0, ptr % · load i64 · store i64 · bitcast i64→double
  +27 / +27 / +17 / +17 / +8 / +8 / +1 / +1	js_shadow_slot_bind / js_shadow_slot_set at slots 1–4
  +19 / +2 / +1	bitcast double→i64 · alloca i64 · js_shadow_frame_enter(i32 5)
  −36	load double, ptr % — GC: #7154's residual is NOT fixed — the loop-polls config is red 0/30, and stock zod alone fails 5/40 #7280's reloads, replaced by root reads
  −1 / −1	the dead sort unbox (bitcast + and)
  −1	js_shadow_frame_enter(i32 3) (frame grew to 5 slots)

  Non-root-plumbing added: 0. Non-plumbing removed: 1 kind, the dead sort unbox, verified dead by reference count.

The comparison masks register names and compares each function as a multiset, because main here is a long run of near-identical console.log(a.method(…)) statements and difflib aligns statement N's inserted plumbing against statement N+1's body — it reported 1426 spurious additions for a 171-instruction delta. What the multiset gives up is ORDER, which is the property a rooting change is about, so order is checked separately: by reading the unified diff of the small cases (shown above) and by the dominance checker over the whole corpus.

Behavioural A/B: all 7 probes, both arms — identical stdout and exit code; and the new arm is byte-identical again under PERRY_GC_ZEAL=1 PERRY_GC_PROTECT_FROMSPACE=1 PERRY_GC_PROTECT_FROMSPACE_DEPTH=64.

Ledger sabotage — result recorded

Reintroduced the escape hatch into the migrated module (a real, compiling temp_root_push_double + temp_root_truncate pair in the reverse arm):

reintroduced shape	compiles?	caught by
reach back into expr::temp_root	yes	the ledger test — red, naming lower_array_method.rs:375 and :376

Same answer #7617 measured, now confirmed for a second module: the API does not make the bug fail to compile, and the ledger is what denies the escape hatch. Reverted; the file names temp_root only in one prose line of its module doc, which escape_hatch_uses strips as a comment.

Verification (local — the CI backlog is deep, so this is the evidence)

• gc-root-dominance, both gated modes, on the post-change compiler: corpus 129/129 sources → 149 modules, 2452 functions, 9810 root stores → 0 violations; --seeded-violations 40 → 40 planted, 40 caught, 0 missed; --unrooted-allocas --moving-only → 0. Baseline arm: same corpus, 9803 root stores, 7860 gc-capable allocas, also 0/0. The +7 / +1 delta is how you can tell the gate's subject was live rather than absent.
• All four checker static audits: --self-test, --audit-alloc-re, --audit-poll-capable, --audit-immovable-sources.
• cargo test -p perry-codegen --lib — 691 pass, including both new lowered_arg_count tests and all four ledger tests. --doc — both compile_fail,E0499 arms still reject.
• cargo test -p perry-runtime --no-fail-fast — 1886 pass, 0 fail.
• ./run_parity_tests.sh --filter test_gap_array against the pinned oracle (node 26.5.1), both arms: 13/13 PASS, 0 parity fail, 0 compile fail, 0 crashed, 0 skipped — identical failure sets, because the set is empty. Full-suite result noted below.
• Lint gates: cargo fmt --all -- --check, workspace_architecture.py (+self-test), check_file_size.sh (1346 lines, under the cap), gc_store_site_inventory.py (+self-test), addr_class_inventory.py (+self-test), class_id_collisions.py, raw_handle_debt.py (+self-test, 998 = baseline), gc_gate_wiring_check.py (+self-test), binding_pins.mjs --check.

Not run locally: the dependency-scale (zod) dominance corpus, which needs npm ci.

Unrelated finding: lint is already red on main

python3 scripts/check_test_registration.py fails on a pristine origin/main:

DARK TEST test-files/test_gap_repsel_element_shape_loop_clone.ts
  exists on disk but is not registered in test-parity/gc_repsel_corpus.txt, so
  scripts/gc_repsel_matrix.sh (gc-stress, gc-moving-witnesses) never runs it.

That file landed in #7612 (96e034809). Neither it nor test-parity/gc_repsel_corpus.txt is in this PR's diff, and registering a test changes what gc-stress / gc-moving-witnesses execute — a gate change with its own measurement, not something to smuggle into a refactor. Flagging it rather than fixing it: a required-and-red gate means every merge is bypassing it, which is hazard 2 in CLAUDE.md.

Not in this PR

No version bump. No behaviour change beyond the itemised rooting fixes and the one dead instruction. No new combinator — the toString arm's unbox_str_handle window (#7213) is deliberately left, with its reasoning recorded in the module header: closing it needs a combinator that roots across a collection point this module emits rather than one it infers from an operand list, and that should arrive with the slice that needs it. No gate widening. No further modules.

Advances #7615. Slice 1b (expr/arrays_finds.rs, expr/array_methods.rs) is next.

Summary by CodeRabbit

• Documentation

  • Updated project documentation to record completed runtime migration work and verification results.
  • Clarified migration boundaries and current completion status.
  • Added a changelog entry covering rooting improvements and validation results.

• Chores

  • Incremented the project version from 0.5.1353 to 0.5.1354.

• Bug Fixes

  • Improved memory-safety handling during array method operations.

[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: a571af44-2f55-46b2-80b6-4fc2eb2eb87d

📥 Commits

Reviewing files that changed from the base of the PR and between d441f81 and 392feae.

⛔ Files ignored due to path filters (1)

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

📒 Files selected for processing (3)

• CLAUDE.md
• Cargo.toml
• crates/perry-codegen/src/lower_array_method.rs

---

📝 Walkthrough

Walkthrough

The migration ledger now includes lower_array_method.rs in MIGRATED_MODULES. The changelog records its rooting migration and verification results. The workspace and documentation versions are updated to 0.5.1354.

Changes

Array method rooting migration

Layer / File(s)	Summary
Record migrated module crates/perry-codegen/src/rooting.rs, changelog.d/7618-layer1-slice1-array-method-rooting.md	The ledger lists lower_array_method.rs as migrated. The changelog records the rooting strategy, verification results, and deferred toString case.
Update package version Cargo.toml, CLAUDE.md	The workspace package version and documented current version change from 0.5.1353 to 0.5.1354.

Estimated code review effort: 1 (Trivial) | ~3 minutes

Possibly related issues

• PerryTS/perry issue 7615: Tracks the same rooting migration campaign and migration-ledger update.

Possibly related PRs

• PerryTS/perry#7192: Targets related GC-rooting and collection-window issues in code-generation lowering paths.
• PerryTS/perry#7617: Migrates another codegen module and updates the shared migration ledger.
• PerryTS/perry#6975: Addresses analogous GC-rooting gaps in code-generation lowering paths.

Suggested labels: bug

🚥 Pre-merge checks | ✅ 5

✅ Passed checks (5 passed)

Check name	Status	Explanation
Title check	✅ Passed	The title clearly and concisely identifies the migration of lower_array_method.rs onto the rooting API.
Description check	✅ Passed	The description thoroughly covers the change, rationale, issue reference, verification results, limitations, and unrelated findings.
Docstring Coverage	✅ Passed	No functions found in the changed files to evaluate docstring coverage. Skipping docstring coverage check.
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.

✨ Finishing Touches

🧪 Generate unit tests (beta)

• Create PR with unit tests
• Commit unit tests in branch refactor/layer1-slice1-lower-array-method

---

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]

Audit before merge — verified, merged as v0.5.1354

Structural claims verified on the branch: lower_expr appears once in the
file — in the doc comment explaining its absence; temp_root once, likewise;
the module is in the ledger, and my sabotage (a real temp_root_push_i64 call
appended) turns migrated_modules_do_not_reach_past_the_rooting_api red.

The genuinely-new coverage verified behaviourally: the
receiver-reassigned-by-its-own-argument case — the one #7280's root_reload
deliberately bails on, since re-loading the slot would observe the assignment
and miscompile — matches node exactly on my probe:

a.concat(reassign())  →  [1,2,3,7], a = [9,9]     (concat sees the OLD receiver)
x.concat((x = …, …))  →  per-iteration correct

The honest scope correction is the report's best content. The map said "40
hazard sites"; the agent found #7280 already covered the shadow-slotted-receiver
majority (visible in baseline IR as the out-of-sequence re-read), and said so
rather than claiming 40 closed. What this PR actually buys — the reassignment
case, argument-to-argument windows, one dead unbox — is smaller and real. A
campaign whose slice reports inflate coverage would converge on false
confidence; this one set the opposite precedent on slice 1.

Re-run here: root-dominance both modes (129/129, 0 violations, 40/40
seeded, unrooted-allocas 0 — root stores up 7 vs baseline, the gate's subject
live), codegen 690/0, the full lint script set (all exit 0 — including
check_test_registration.py, now that #7619 repaired what #7612's merge
broke), fmt clean. One runtime-suite failure did not reproduce on two
consecutive full reruns — #7365's known flake, and this PR touches no runtime
code.

Process note folded into this merge cycle: the slice agent's flag of the
red lint on main (the dark #7612 test) was fixed first as #7619, and my merge
checklist is replaced by enumerating the lint job's script set from test.yml
— the second breakage via a memorized subset is the last.