fix: keep main green after the #9302/#9303/#9307 batch by proggeramlug · Pull Request #9313 · PerryTS/perry

proggeramlug · 2026-08-31T17:38:39Z

Follow-up to the batch merged today. Three things, all validated together as one build before the batch went in:

#9303's masked-reads commit. #9303 was stacked on #9294; once #9294 merged, the PR branch conflicted against main. The remaining delta is carried here as its original commit, so authorship is preserved.

Two files back under the 2000-line cap. ic_miss.rs (1965 on main) and array/tests.rs (1980) were each within ~35 lines of the gate, and #9302 and #9307 tipped them to 2035 and 2003. The C3C PIC test module and the Array.prototype method-discriminator tests move to sibling files, following the existing tests_strict_dense.rs convention. No behaviour change.

The dead arm after the combined accumulator catch-all. #9303 adds a combined _ => arm but leaves the _ => false it supersedes, making it an unreachable pattern:

warning: unreachable pattern
  --> crates/perry-codegen/src/stmt/stable_packed_accumulator.rs:86:17

With -D warnings that's a build failure. Removing it is the fix #9308 correctly identified.

Also carries a cargo fmt pass — #9302 landed with formatting rustfmt reformats, so cargo fmt --check is red on main as of now.

Validation: perry-codegen + perry-hir + perry-transform (74 suites) and perry-runtime (8 suites) both green under RUST_TEST_THREADS=1, plus all four PRs' own integration tests (issue_9287_overflow_slot_ic, numeric_range_add_resume, dense_accumulator_masked_reads, issue_9253_affine_range_index — 16 tests). Release build clean with no new warnings; all nine lint ratchets pass.

Summary by CodeRabbit

Performance Improvements
- Improved performance for numeric accumulation over masked array reads by enabling optimized dense processing.
- Preserved safe fallback behavior for string-based accumulators and string elements.
Bug Fixes
- Improved handling of masked indexes in optimized loops.
- Strengthened validation for overflow property-cache behavior and prototype method detection.
Tests
- Added coverage for masked-read accumulation, garbage collection, property-cache invalidation, and built-in method discrimination.
Maintenance
- Reorganized tests and formatting without changing runtime behavior.

…clone 17_loop_data_dependent: 475 ms -> 219 ms against node's 220 ms on an idle Mac mini -- parity, from 2.16x. Sums bit-identical across 100M data-dependent float recurrence steps. sum = sum * x[i & 63] + x[(i * 7) & 63] // rejected sum = sum * x[i & 63] // admitted The discriminator was the accumulator's static numeric proof. `+` can be concatenation, so the dense tier's per-statement proof demands both operands numeric; a reassigned accumulator has no such proof, because its own writes read the guarded array, whose element proof only exists once the guard has run. A chicken-and-egg that `*` never faces -- multiplication needs only the weaker inert fact. Confirmed by instrumenting the two conjuncts of the dense LocalSet arm: the failing one is the proof, on exactly the fixtures whose accumulator writes contain a plain-array read. The matcher now peels the accumulator: when the proof fails on the LocalSet target of a self-accumulating write, it retries with the target treated as numeric BY CONTRACT, records it pending, and then verifies every pending local with the same collector the lowering runs (`collect_numeric_accumulators`), rejecting the whole dense match with its own named trace reasons (`accumulator_needs_single_array`, `accumulator_not_provable`) if the two disagree -- so the clone can never contain a dynamic `+` under facts that forbid one. The contract is enforced at run time twice over: the clone's entry emits a genuine-double tag check on the accumulator, and the dense entry guard validates the whole masked window hole-free. A string-seeded accumulator and a string element both route to the slow copy and produce node's concatenation, verified under PERRY_GC_FORCE_EVACUATE. Supporting changes: * `accumulator_rhs_is_numeric` accepts masked static-window reads of the tracked array (`masked_reads_validated`), sound because the dense guard validated the window union hole-free. Fixing that exposed a match-arm reachability bug: `_ if offset_reads_inlined` was a guarded catch-all, so ANY arm placed after it was unreachable whenever the flag was set -- the first version of this change sat exactly there and verified as a no-op. The two tests are now one combined catch-all. * `emit_range_loop_accumulator_admission` admits a masked-only single array (counter-bearing arrays keep priority; multiple arrays still decline). * `MaskedWindowArrayFact` carries `numeric_accumulators` so `is_numeric_expr` sees admitted accumulators while the clone lowers -- without this the add inside the clone would stay dynamic, which is a collecting call under facts that assume none (the #9259 cascade shape). Mirrors the string-window fact's field (#9160). perry-codegen lib 1378/0; packed-loop integration suite 59/0 across 11 files; 3 new regression tests (admission + node-identical result, string-seeded accumulator, string element), each under forced evacuation. Claude-Session: https://claude.ai/code/session_01Pcq6j6y57TdKSR2Zx2D187

… cap #9302 and #9307 each tipped a file that was already within ~35 lines of the 2000-line gate. Extracts the C3C PIC test module and the Array.prototype method-discriminator tests into sibling files; no behaviour change.

…tor arm #9303's combined `_ =>` arm made the trailing `_ => false` unreachable, which is a `-D warnings` failure. Removing it is #9308's fix, which the combined arm needs to be complete.

coderabbitai · 2026-08-31T17:39:56Z

Caution

Review failed

The pull request is closed.

ℹ️ Recent review info

⚙️ Run configuration

Configuration used: defaults

Review profile: CHILL

Plan: Pro Plus

Run ID: a8f467ef-bbbf-444f-8c7e-08860ce424e8

📥 Commits

Reviewing files that changed from the base of the PR and between 8f253df and bbab1e4.

📒 Files selected for processing (15)

changelog.d/9313-train13-followup.md
changelog.d/dense-accumulator-masked-reads.md
crates/perry-codegen/src/expr/mod.rs
crates/perry-codegen/src/stmt/loops.rs
crates/perry-codegen/src/stmt/masked_window_region.rs
crates/perry-codegen/src/stmt/stable_packed_accumulator.rs
crates/perry-codegen/src/stmt/stable_packed_loop.rs
crates/perry-codegen/src/type_analysis/numeric.rs
crates/perry-runtime/src/array/tests.rs
crates/perry-runtime/src/array/tests_proto_discriminator.rs
crates/perry-runtime/src/object/field_get_set/ic_miss.rs
crates/perry-runtime/src/object/field_get_set/ic_miss/c3c_pic_tests.rs
crates/perry-runtime/src/proxy/put_value.rs
crates/perry/tests/dense_accumulator_masked_reads.rs
crates/perry/tests/issue_9287_overflow_slot_ic.rs

📝 Walkthrough

Walkthrough

The codegen now admits validated numeric accumulators in dense masked-window clones. It records these locals for lowering and rejects unproven cases. Runtime regression tests move into external modules, with additional formatting-only and changelog updates.

Changes

Dense masked-read accumulator

Layer / File(s)	Summary
Accumulator proof and masked-index admission `crates/perry-codegen/src/stmt/stable_packed_accumulator.rs`, `crates/perry-codegen/src/expr/mod.rs`	Accumulator analysis accepts validated masked reads and propagates the validation flag through recursive expression checks.
Dense clone admission and lowering `crates/perry-codegen/src/stmt/loops.rs`, `crates/perry-codegen/src/stmt/masked_window_region.rs`, `crates/perry-codegen/src/stmt/stable_packed_loop.rs`, `crates/perry-codegen/src/type_analysis/numeric.rs`	Dense range matching tracks pending accumulators, verifies them with the lowering collector, and records them in `MaskedWindowArrayFact`.
Masked accumulator integration tests and changelog `crates/perry/tests/dense_accumulator_masked_reads.rs`, `changelog.d/dense-accumulator-masked-reads.md`	Tests cover dense-copy generation, numeric and string behavior, string elements, and moving-GC execution.

Runtime test extraction

Layer / File(s)	Summary
Array builtin discriminator test module `crates/perry-runtime/src/array/tests.rs`, `crates/perry-runtime/src/array/tests_proto_discriminator.rs`	The prototype discriminator test moves to an external module and retains checks for builtin identity, method names, and non-pointer slots.
C3C PIC regression test module `crates/perry-runtime/src/object/field_get_set/ic_miss.rs`, `crates/perry-runtime/src/object/field_get_set/ic_miss/c3c_pic_tests.rs`	C3C PIC tests move to an external file and cover accessor isolation, class-instance tokens, compaction, and fresh-instance consistency.

File-size and formatting follow-up

Layer / File(s)	Summary
File-size cleanup and formatting `changelog.d/9313-train13-followup.md`, `crates/perry-runtime/src/object/field_get_set/ic_miss.rs`, `crates/perry-runtime/src/proxy/put_value.rs`, `crates/perry/tests/issue_9287_overflow_slot_ic.rs`	The changelog records file-size cleanup. Several runtime expressions and test arguments receive formatting-only changes.

Estimated code review effort: 4 (Complex) | ~45 minutes

Sequence Diagram(s)

sequenceDiagram
  participant RangeLoopMatcher
  participant DenseBodyCollector
  participant NumericAccumulatorCollector
  participant MaskedWindowArrayFact
  RangeLoopMatcher->>DenseBodyCollector: collect pending accumulator locals
  DenseBodyCollector->>NumericAccumulatorCollector: verify pending locals
  NumericAccumulatorCollector-->>RangeLoopMatcher: accept or reject dense match
  RangeLoopMatcher->>MaskedWindowArrayFact: record numeric_accumulators

✨ 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/train13-followup

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

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

Ralph Küpper added 5 commits August 31, 2026 19:35

fix(codegen): drop the now-dead catch-all after the combined accumula…

57ec7b4

…tor arm #9303's combined `_ =>` arm made the trailing `_ => false` unreachable, which is a `-D warnings` failure. Removing it is #9308's fix, which the combined arm needs to be complete.

style: rustfmt

774c5d1

chore: changelog fragment for the train13 follow-up

bbab1e4

proggeramlug merged commit d6794cf into main Aug 31, 2026
17 of 18 checks passed

proggeramlug deleted the fix/train13-followup branch August 31, 2026 17:38

proggeramlug mentioned this pull request Aug 31, 2026

codegen: a float accumulator over masked reads earns the dense range clone — 17_loop_data_dependent at node parity (475→219ms) #9303

Closed

proggeramlug mentioned this pull request Sep 1, 2026

cc --help parity gate down on main: defineProperty-on-non-object at startup — window pins #9326 (indexed writes with a recorded custom array prototype) #9341

Open

Navigation Menu

Uh oh!

Uh oh!

fix: keep main green after the #9302/#9303/#9307 batch - #9313

fix: keep main green after the #9302/#9303/#9307 batch#9313
proggeramlug merged 5 commits into
mainfrom
fix/train13-followup

proggeramlug commented Aug 31, 2026 •

edited by coderabbitai Bot

Loading

Uh oh!

Uh oh!

coderabbitai Bot commented Aug 31, 2026 •

edited

Loading

Review failed

Walkthrough

Changes

Sequence Diagram(s)

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

1 participant

Uh oh!

Uh oh!

Conversation

proggeramlug commented Aug 31, 2026 • edited by coderabbitai Bot Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Summary by CodeRabbit

Uh oh!

Uh oh!

coderabbitai Bot commented Aug 31, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Review failed

Walkthrough

Changes

Sequence Diagram(s)

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

1 participant

proggeramlug commented Aug 31, 2026 •

edited by coderabbitai Bot

Loading

coderabbitai Bot commented Aug 31, 2026 •

edited

Loading