From 6a3b5bff82c198f799599d1a137d5298000925eb Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Ralph=20K=C3=BCpper?= Date: Fri, 31 Jul 2026 23:47:56 +0200 Subject: [PATCH] =?UTF-8?q?perf(repsel):=20open=20the=20array-element=20es?= =?UTF-8?q?cape=20for=20`Ptr`=20(#7034=20=C2=A73)?= MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit `Ptr` rule 2 disqualifies a local at any escape. #7034 §4 opened `return`; this opens `array/object element` for the array half — `rows.push(row)` and `for (const r of rows) r.field`. A `return` is a terminator, so no use of the local can follow it. An element escape is not: the object stays reachable through the array for the rest of its life. The containment region therefore widens from one local to one local array and everything derived from it, with the array's own uses bounded exactly as an object local's are (`collectors/ptr_shape_elements.rs`, five conjuncts E1-E5, each with its own red set). E5 — an element read is licensed only under `for (let i = 0; i < A.length; i++)` — is the whole difference between this pass and a wrong one: without it `A[i]` can be `undefined`, and a guard-free fixed-offset load masks a NaN-boxed `undefined` into a wild pointer. Measured on a build-then-consume kernel: 0 -> 3 promotions, all consumed, and the promoted body loses all 7 `js_typed_feedback_class_field_get_guard`, all 7 `js_typed_feedback_record_fallback_call` and all 7 `js_object_get_field_by_name_f64` calls (72 -> 51 `js_*`, 1426 -> 1076 IR lines) with every GC-related call count identical. `batch.ts` is unchanged, and dependency JS gains ~nothing: over 180 real CJS modules the element position accounts for 1 of 746 denials. Both findings, and the rule-1 wall they point at instead, are in the changelog fragment. --- .github/workflows/test.yml | 12 + .../workloads/1_json_pipeline/zig/build.sh | 8 + .../3_image_convolution/zig/build.sh | 8 + benchmarks/repsel_census/baseline.json | 32 +- .../fixtures/fixture_ptr_shape_elements.ts | 80 ++ .../7149-ptr-shape-array-element-escape.md | 228 ++++ changelog.d/7160-zig-cache-isolation.md | 1 + .../src/collectors/cjs_scaffolding.rs | 10 + .../perry-codegen/src/collectors/hir_facts.rs | 13 + crates/perry-codegen/src/collectors/mod.rs | 1 + .../perry-codegen/src/collectors/ptr_shape.rs | 106 +- .../src/collectors/ptr_shape_elements.rs | 929 +++++++++++++ .../collectors/ptr_shape_elements_tests.rs | 1208 +++++++++++++++++ .../collectors/ptr_shape_opt_report_tests.rs | 2 + .../src/collectors/ptr_shape_report.rs | 19 +- .../src/collectors/ptr_shape_returns.rs | 24 + .../src/collectors/ptr_shape_returns_tests.rs | 7 + .../compiler_output_harness/repsel_census.py | 7 + .../test_gap_repsel_ptr_shape_elements.ts | 195 +++ test-parity/gc_repsel_corpus.txt | 9 + 20 files changed, 2892 insertions(+), 7 deletions(-) create mode 100644 benchmarks/repsel_census/fixtures/fixture_ptr_shape_elements.ts create mode 100644 changelog.d/7149-ptr-shape-array-element-escape.md create mode 100644 changelog.d/7160-zig-cache-isolation.md create mode 100644 crates/perry-codegen/src/collectors/ptr_shape_elements.rs create mode 100644 crates/perry-codegen/src/collectors/ptr_shape_elements_tests.rs create mode 100644 test-files/test_gap_repsel_ptr_shape_elements.ts diff --git a/.github/workflows/test.yml b/.github/workflows/test.yml index 2ada5dc60b..d91f3d9239 100644 --- a/.github/workflows/test.yml +++ b/.github/workflows/test.yml @@ -1286,6 +1286,18 @@ jobs: echo "::error::two of the six recorders had never fired before it existed." exit 1 fi + # #7034 §3 (the array-element escape) is a SEPARATE analysis + # (collectors/ptr_shape_elements.rs) behind the same knob, and NO + # real corpus workload promotes an element local -- so if it stopped + # issuing facts entirely, every assertion above would still pass and + # this job would stay green. Its own fixture is what makes that + # visible. + if ! printf '%s' "$out" | grep -q "fixture_ptr_shape_elements: ptr-shape-consumed promoted 0"; then + echo "::error::The array-element fixture kept its promotions with" + echo "::error::Ptr disabled. Either the element analysis is not" + echo "::error::behind the knob, or the fixture stopped exercising it." + exit 1 + fi echo "Census correctly went red with PERRY_PTR_SHAPE_LOCALS=0." - name: Upload census reports diff --git a/benchmarks/honest_bench/workloads/1_json_pipeline/zig/build.sh b/benchmarks/honest_bench/workloads/1_json_pipeline/zig/build.sh index 66f65443d5..6c7a2d6ed7 100755 --- a/benchmarks/honest_bench/workloads/1_json_pipeline/zig/build.sh +++ b/benchmarks/honest_bench/workloads/1_json_pipeline/zig/build.sh @@ -1,6 +1,14 @@ #!/bin/bash set -euo pipefail cd "$(dirname "$0")" + +# Keep the compiler caches out of ~/.cache/zig and out of the source tree. The path is stable so +# rebuilds stay warm; point PERRY_ZIG_CACHE_DIR elsewhere for a cold build. +tmp_root="${TMPDIR:-/tmp}" +zig_cache="${PERRY_ZIG_CACHE_DIR:-${tmp_root%/}/perry-zig-cache}" +export ZIG_GLOBAL_CACHE_DIR="$zig_cache/global" +export ZIG_LOCAL_CACHE_DIR="$zig_cache/json_pipeline" + mkdir -p zig-out/bin zig build-exe src/main.zig \ -O ReleaseFast \ diff --git a/benchmarks/honest_bench/workloads/3_image_convolution/zig/build.sh b/benchmarks/honest_bench/workloads/3_image_convolution/zig/build.sh index d8348c88a0..9132c46512 100755 --- a/benchmarks/honest_bench/workloads/3_image_convolution/zig/build.sh +++ b/benchmarks/honest_bench/workloads/3_image_convolution/zig/build.sh @@ -4,6 +4,14 @@ # script against the host target, which has the same version mismatch. set -euo pipefail cd "$(dirname "$0")" + +# Keep the compiler caches out of ~/.cache/zig and out of the source tree. The path is stable so +# rebuilds stay warm; point PERRY_ZIG_CACHE_DIR elsewhere for a cold build. +tmp_root="${TMPDIR:-/tmp}" +zig_cache="${PERRY_ZIG_CACHE_DIR:-${tmp_root%/}/perry-zig-cache}" +export ZIG_GLOBAL_CACHE_DIR="$zig_cache/global" +export ZIG_LOCAL_CACHE_DIR="$zig_cache/image_conv" + mkdir -p zig-out/bin zig build-exe src/main.zig \ -O ReleaseFast \ diff --git a/benchmarks/repsel_census/baseline.json b/benchmarks/repsel_census/baseline.json index e1cea6f0f2..69275442a3 100644 --- a/benchmarks/repsel_census/baseline.json +++ b/benchmarks/repsel_census/baseline.json @@ -61,6 +61,36 @@ "ptr_shape_method": 1 } }, + { + "name": "fixture_ptr_shape_elements", + "role": "liveness", + "source": "benchmarks/repsel_census/fixtures/fixture_ptr_shape_elements.ts", + "floors": { + "ptr-shape": 3, + "ptr-shape-consumed": 3, + "ptr-numarray": 0, + "canonical-i32": 1, + "canonical-u32": 0, + "canonical-str": 0, + "int-valued-ta": 0, + "spec-abi-entry": 1, + "spec-abi-taptr-slot": 0 + }, + "candidates": { + "ptr-shape": 3, + "ptr-numarray": 0, + "canonical-slot": 2, + "int-valued-ta": 0, + "spec-abi": 1 + }, + "unconsumed_mechanisms": {}, + "consumption_sites": { + "ptr_shape_get_number": 3, + "ptr_shape_method": 1, + "class_field_get.shape_proven_load": 1, + "ptr_shape_set": 2 + } + }, { "name": "fixture_ptr_numarray", "role": "liveness", @@ -678,5 +708,5 @@ "consumption_sites": {} } ], - "generated_at": "2026-07-31T09:26:30.260742Z" + "generated_at": "2026-07-31T21:18:40.428173Z" } diff --git a/benchmarks/repsel_census/fixtures/fixture_ptr_shape_elements.ts b/benchmarks/repsel_census/fixtures/fixture_ptr_shape_elements.ts new file mode 100644 index 0000000000..94a289e34d --- /dev/null +++ b/benchmarks/repsel_census/fixtures/fixture_ptr_shape_elements.ts @@ -0,0 +1,80 @@ +// Liveness fixture for the `Ptr` ARRAY-ELEMENT escape (#7034 §3). +// +// The other two `ptr_shape` fixtures prove that a *contained* local promotes +// and that each consumption site fires. Neither of them touches an array, so +// both stay green if `collectors/ptr_shape_elements.rs` stops issuing facts +// entirely — and the 18 real corpus workloads promote zero element locals +// today, so the corpus cannot see it either. Without this file the element +// rule has no gate at all: it would be exactly CLAUDE.md failure mode 4, a +// green job whose subject never ran. +// +// What this program has to get right, all at once: +// +// 1. `rows` must satisfy every element-array conjunct: one `const rows = []` +// binding, only `push` writes of one class, only `.length` and in-bounds +// `rows[i]` reads, and no other use at all. It is deliberately NOT +// returned — `return rows` is admitted by the rule, but returning it +// would let the deforestation pass (`perry-transform/src/deforest`) +// rewrite the local array into a `__deforest_out` PARAMETER, which this +// analysis cannot see. That is a real coverage hole (it is why +// `batch.ts` is unchanged by #7034 §3) and it must not silently make +// this fixture vacuous. +// 2. The producer local `row` must escape ONLY through the push, so its +// promotion is attributable to the element exemption and to nothing +// else. Its field store before the push is what keeps it out of scalar +// replacement (#7115) — without it the object is deleted outright and +// no access site is reached. +// 3. Both read forms must appear: the explicit `const s = rows[i]` inside a +// `i < rows.length` loop, and the `for (const r of rows)` iterator form, +// which desugars to the same shape. If the desugar ever changes, this +// fixture's count drops and the gate goes red — which is the point. +// 4. Every read must be a declared field of `Row`, and no member of the +// group may escape: one `r.extra = 1` anywhere voids the WHOLE group by +// design, and would take this fixture to zero. +// +// Do not "tidy" this file. In particular do not add `return rows`, do not +// hoist the `new Row(...)` into the `push` call (that removes the producer +// local this fixture is here to promote), and do not merge the two read +// loops. + +class Row { + id: number; + weight: number; + score: number; + constructor(id: number, weight: number) { + this.id = id; + this.weight = weight; + this.score = 0; + } + rescore(f: number): number { + return this.weight * f + this.id; + } +} + +function build(n: number): number { + const rows: Row[] = []; + for (let i = 0; i < n; i++) { + // Producer local: its only escape is the push (note 2). + const row = new Row(i, i * 0.5); + row.score = row.weight + 1; + rows.push(row); + } + + let total = 0; + + // Read form A: explicit indexed binding under an `i < rows.length` loop. + for (let i = 0; i < rows.length; i++) { + const s = rows[i]; + s.score = s.score + s.weight; + total = total + s.score + s.id; + } + + // Read form B: `for…of`, which desugars to the same bounded `rows[__idx]`. + for (const r of rows) { + total = total + r.rescore(2) + r.weight; + } + + return total; +} + +console.log("ptr_shape_elements:" + build(6)); diff --git a/changelog.d/7149-ptr-shape-array-element-escape.md b/changelog.d/7149-ptr-shape-array-element-escape.md new file mode 100644 index 0000000000..6669d9b103 --- /dev/null +++ b/changelog.d/7149-ptr-shape-array-element-escape.md @@ -0,0 +1,228 @@ +`Ptr` rule 2 disqualifies a local at any escape. #7034 §4 opened +`return`; this opens **`array/object element`** for the array half — +`rows.push(row)` and `for (const r of rows) r.field`. + +`return` was easy because a return is a **terminator**: no use of the local can +follow it, so every access the pass licensed had already run while the object +was unaliased. An element escape is not. The object stays reachable through the +array for the rest of its life, so the containment region had to widen from +*one local* to *one local array and everything derived from it*, and the +array's own uses had to be bounded exactly as an object local's are. + +## The rule (`collectors/ptr_shape_elements.rs`) + +A region-local `A` is an **element-shape-proven array** of class `C` when all +of: + +- **E1 provenance** — exactly one `Let { mutable: false, init: Array([]) }`, + not boxed, not a module global. The literal must be **empty**: a non-empty + one can carry elisions, whose slots read back as `undefined`. +- **E2 element provenance** — every write is `ArrayPush` of `new C(...)`, + inline or via a local bound by one `Let { init: New { C } }` and pushed + **exactly once**. No other mutator at all — no `pop`/`shift`/`splice`/ + `unshift`/`copyWithin`, no `IndexSet`, no `length` write. That is what makes + `A` dense and monomorphic for its whole lifetime. +- **E3 array containment** — every other use is an E5-licensed element read, a + `.length` read, or `return A` (#7034 §4's terminator exemption, unchanged). + Call argument, closure capture, reassignment, container element, unrecognised + array method: all still disqualify. So does **any element read the `Let` arm + did not license** — `f(A[i])`, `A[i].m()`, `const r = A[0]`. A read cannot + transition a shape, but the reference it hands out can be used to, and rule 2 + never walks a binding this pass did not seed. That is why direct + `A[i].field` is not covered at all today (#7151). +- **E4 admissibility** — `C` passes the same `chain_admissible` gate rule 1 + applies to a `new C(...)` local, and the rule-5 module barrier is clear. +- **E5 in-bounds reads** — `A[i]` is licensed only where `i` is the induction + variable of an enclosing `for (let i = 0; i < A.length; i++)`, written + nowhere else in the region. **This conjunct is the whole difference between + this pass and a wrong one.** Without it `A[i]` can be `undefined`, and a + guard-free fixed-offset load masks a NaN-boxed `undefined` into a wild + pointer. E3 admits no mutator that can shrink `A` and E2 makes it dense, so + `0 <= i < A.length` at the read means `A[i]` is an own element of class `C`. + +`for (const r of A)` desugars to exactly the E5 shape +(`lower/stmt_loops.rs::lazy_or_index_elem`), so the iterator form is covered by +the indexed proof rather than by a second one. + +Then two halves in `ptr_shape.rs`: `A.push(row)` stops disqualifying `row`, and +`const r = A[i]` at a licensed site is rule-1 provenance of `new`-strength. + +**Group integrity.** Every member of an element group — the pushed producers +plus the element-read locals — references objects the *other* members also +reach. One member failing rule 2 (`r.extra = 1`, a closure capture, an opaque +call) can transition the shape the others read guard-free, so the group is +all-or-nothing: `collect_shape_proven_ptr_locals` drops every member when any +one fails. No fixpoint is needed — dropping never admits a member. + +**`numeric_fields` is not claimed** for group members. The numeric proof is an +exhaustive-reachable-store proof that containment makes possible *because no +alias exists*; a group has aliases by construction, and a sibling's +`r.score = "s"` (a declared field, so rule 2 permits it) downgrades the slot's +raw-f64 layout. Same stand-down as `proven_this.rs` and `ptr_shape_returns.rs`. +The shape proof alone still retires the whole guard diamond. + +## What it buys, measured + +On a build-then-consume kernel (40 000 records, produced with a local, then +read back both ways) the promoted function goes **0 → 3 selected, 3 consumed**, +and its emitted IR loses: + +| symbol | base | after | +|---|---:|---:| +| `js_typed_feedback_class_field_get_guard` | 7 | **0** | +| `js_typed_feedback_record_fallback_call` | 7 | **0** | +| `js_object_get_field_by_name_f64` | 7 | **0** | +| all `js_*` calls | 72 | 51 | +| IR lines / blocks | 1426 / 127 | 1076 / 113 | + +**Every other `js_*` call count is identical**, including all of +`js_shadow_slot_bind` (5), `js_write_barrier_root_nanbox` (5), +`js_write_barrier_slot` (1), `js_array_push_f64` (2), `js_gc_loop_safepoint` +(4) and the inline incremental-mark barrier sites (5). Nothing but guard +machinery went away. + +## GC contract, verified in emitted IR + +- The element locals get `js_shadow_slot_bind` in the entry block (slots 3 and + 4 of the probe). `TaPtr`'s callee-side no-bind shortcut is **not** copied — + `GC_TYPE_OBJECT` is movable (#6990, #7019). +- Every access **re-derives** the raw pointer from that alloca: + `load double, ptr %rN` → `and POINTER_MASK` → `inttoptr` → `gep +header` → + `gep index` → `load`. The `for…of` local is reloaded 3× (3 field reads) and + the indexed local 4× (4 access sites); nothing is cached across a safepoint. +- The store of the element into the bound slot is followed by the incremental + mark-barrier check and `js_write_barrier_root_nanbox`. +- Write barriers on element stores are untouched: this pass changes no store + lowering, and `js_array_push_f64` counts are identical between arms. +- The read side uses the `ptr_shape_get_number.plain` / `.coerce` pair — the + 2-instruction plain-finite check with a cold arm — because the group claims + no numeric fields. + +## What this does NOT reach, and the measurements that say so + +**`batch.ts` is unchanged: 2 selected / 1 consumed, identical to `main`.** Both +of its element denials fail for reasons outside this rule: + +- `buildRows`'s `const rows = []; …; return rows` never reaches the analysis — + the **interprocedural deforestation pass** (`perry-transform/src/deforest`) + has already rewritten it into a `__deforest_out` *parameter*, and a parameter + array has no provenance. That transform fires on exactly the + `const a = []; …push…; return a` producer shape this rule targets, which is a + real coverage hole rather than an incidental one. +- `summarize`'s `byBucket` is passed as `rows.reduce(…)`'s seed — a call + argument, so the array escapes (#7034 §1 territory). + +**Dependency JS gets essentially nothing.** #7139 reported that ~103 candidates +its CJS barrier exemption freed were "immediately re-denied by rule 2", and +this position was picked on the assumption that those were element escapes. +They are not. Over **180** real `__esModule` CJS modules from +`real-apps/scriptc/node_modules`, compiled by a #7139-only arm and a +combined (#7139 + this change) arm — both 180/180 — the 746 `Ptr` +candidates deny as: + +| bucket | count | +|---|---:| +| rule 1 — allocation never bound to a local | **506** | +| rule 5 — module barrier still armed | 99 | +| rule 2 — bare reference | 130 | +| rule 2 — call argument | 5 | +| rule 2 — **array element** | **1** | +| rule 2 — closure capture / undeclared property | 1 / 1 | + +Both arms are identical on every line. The rule-2 bare references are all +Perry's own `__cjs_module` wrapper local, and the 506 rule-1 denials break down +as constructor argument 182, statement 162, call argument 84, return 64, array +element 8, initializer 6. **The wall in dependency JS is rule 1 (allocations +never bound to a local), not containment.** + +## Review findings (CodeRabbit, PR #7149) + +Each reviewer reproducer was added as a test **before** any fix, so the finding +had to prove itself red first. + +- **🔴 group integrity did not drop a member's ALIASES.** Genuine, red on HEAD. + The insert loop gives every alias of a promoted root the same fact, and the + removal loop only removed the ids `group_members()` reports — so + `const a = row` kept a guard-free proof of a shape a sibling had just + transitioned. Fixed: the removal now takes the alias closure. Sabotage case + `ALIAS_CLOSURE`. +- **🟠 a tracked array pushed into another array kept its facts.** Genuine, red + on HEAD. `PushValue::Other` disqualified the OUTER array, but the arm skips + `walk_expr` for a `LocalGet` value so the INNER one was never disqualified, + leaving it reachable through `outer[0][0] = …` — an `IndexSet` on an + `IndexGet` that neither walk tracks. Fixed with the reviewer's one-arm patch. + Sabotage case `18_nested_array_push`. +- **🔴 a property store through `A[i]` was admitted for any property.** The + hazard was real; its mechanism (`element_access_is_admissible`) had already + been deleted in the same commit the review was posted against, when the + unlicensed-element-read hole was closed. Both reproducers (`PropertySet` and + `PropertyUpdate`) are **green on HEAD** and kept as permanent regression + tests — sabotage cases `16_element_escape` and `19_element_prop_store` show + which guard now carries them. +- **🟡 `is_empty` covered only `arrays`.** The other three maps are consistent + with it by construction, but nothing enforced that and `is_empty()` gates + every consumer. Assertion added in both directions. +- **🟠 "make `repsel-census` a required status check"** — declined, with the + rationale on the thread: branch protection is admin-only and the deferral is + deliberate project policy (a gate that has never been green blocks every open + PR the moment it is promoted). Tracked as an open follow-through. +- Nitpicks taken: `facts_for` now receives the test's own classes (a mutated + class reaching only `chain_admissible` while dispatch facts came from a + pristine one is the vacuous-pass shape); the accessor fixture's getter no + longer shadows a declared field (it could have passed on field/method + ambiguity); `element_read_seeds` reuses this module's walker instead of a + second copy of the traversal; and the array-ALIAS path — which the whole + `for…of` read form runs through — now has a direct unit test. + +## Validation + +- `cargo test -p perry-codegen --lib`: 453 passed (26 new). +- **Sabotage matrix, 21 conjuncts, each with a disjoint red set** — every guard + deleted in turn, the suite re-run, and the failing tests recorded: push + exemption, in-bounds read, both GC rooting obligations, group integrity, + single-push, empty-literal seed, `const` array binding, shrinking mutators, + indexed store, class agreement, index write count, bare array reference, + closure capture, `.length` receiver, unlicensed element read, unlicensed + element binding, nested-array push, element property store, group integrity, + alias closure. Control green in all 22 runs. Three weaknesses it caught + and fixed: `a_local_pushed_into_two_arrays_is_not_exempt` passed on `HashMap` + iteration order (now asserts the facts directly), the closure-capture test + denied through the body walk rather than the capture list, and + `unbounded_element_read_is_not_provenance` asserted only that the READ was + denied, not that the array was voided — which is the assertion that catches + the element-escape hole above. +- Census: new liveness fixture `fixture_ptr_shape_elements` with floors + `ptr-shape 3 / ptr-shape-consumed 3`; no existing floor moved; the + `PERRY_PTR_SHAPE_LOCALS=0` sabotage step in `repsel-census` now asserts the + element fixture goes to zero too — without that, the whole analysis could + stop issuing facts and every counter in the job would be unchanged. +- New `test_gap_repsel_ptr_shape_elements.ts`, registered in + `test-parity/gc_repsel_corpus.txt`, byte-exact against Node 26.5.1: both read + forms, 200 allocations *between* two field reads of the same element local, + NaN/±Infinity/-0 written through one group member and read through another, + and four arrays that must not be proven. +- `gc_repsel_matrix.sh --arms all --pressure 8`: **PASS=447 UNVER=119 XFAIL=1 + FAIL=0** over 567 cells, `requires=move` arms live. The new gap file is + **PASS in all 21 arm columns with zero UNVER** — every arm, evacuating ones + included, was live on it, so those greens are not the inert-arm kind + (#6942/#6946/#6950). +- Emitted objects are **byte-identical** between arms on `batch.ts`, + `02_loop_overhead`, `04_array_read`, `07_object_create`, `12_binary_trees` + and `15_mandelbrot` — nothing changed where nothing promotes. (#7131 landed, + so object hashing is a valid instrument.) + +## Follow-ups filed + +- **#7150** — deforestation rewrites `const a = []; …push…; return a` into a + `__deforest_out` parameter before this analysis runs, which is why `batch.ts` + is unchanged. The two passes are working against each other. +- **#7151** — the four element-read forms this does not cover (direct + `A[i].field`, callback parameters, `let` bindings, non-empty literals and + shape-preserving mutators), with the measurement each needs first. +- **#7152** — the dependency-JS wall is rule 1 (506 of 746 candidates are + allocations never bound to a local), not containment. `call argument` + (#7034 §1) should not be scheduled on the assumption that it unlocks + dependency JS either: it is 5 of 746. +- **#7153** — pre-existing: reading a field of an out-of-bounds array element + returns `undefined` instead of throwing `TypeError`. Found while writing the + gap test; red on `main` and with the knob off. diff --git a/changelog.d/7160-zig-cache-isolation.md b/changelog.d/7160-zig-cache-isolation.md new file mode 100644 index 0000000000..deaec9225b --- /dev/null +++ b/changelog.d/7160-zig-cache-isolation.md @@ -0,0 +1 @@ +Fixed the two Zig benchmark build scripts (`benchmarks/honest_bench/workloads/1_json_pipeline/zig/build.sh` and `.../3_image_convolution/zig/build.sh`) writing into the developer's home directory. `zig build-exe` ran with no cache override, so every build populated Zig's global cache under the user's home directory and dropped a `.zig-cache` directory into the workload source tree. Both scripts now export `ZIG_GLOBAL_CACHE_DIR` and `ZIG_LOCAL_CACHE_DIR` under the OS temp dir, with the two workloads using separate local-cache subdirectories. The path is stable rather than per-run so rebuilds stay warm without re-fetching the compiler-rt closure; `PERRY_ZIG_CACHE_DIR` relocates it for a cold build. Beyond hygiene this is a reproducibility fix: a build that reads and writes a home-directory cache can pass on a warm machine and fail on a cold one. diff --git a/crates/perry-codegen/src/collectors/cjs_scaffolding.rs b/crates/perry-codegen/src/collectors/cjs_scaffolding.rs index 25413ef366..506688dbe7 100644 --- a/crates/perry-codegen/src/collectors/cjs_scaffolding.rs +++ b/crates/perry-codegen/src/collectors/cjs_scaffolding.rs @@ -621,6 +621,16 @@ mod tests { &classes, &facts, &HashSet::new(), + // #7034 §3: this fixture builds no array, so the element facts are + // empty either way — computed rather than defaulted so the two + // passes cannot drift apart here. + &crate::collectors::ptr_shape_elements::collect_element_shape_facts( + &promotable_body(), + &HashSet::new(), + &HashMap::new(), + &classes, + &facts, + ), ) .is_empty() } diff --git a/crates/perry-codegen/src/collectors/hir_facts.rs b/crates/perry-codegen/src/collectors/hir_facts.rs index ecdfb74945..6c596777f4 100644 --- a/crates/perry-codegen/src/collectors/hir_facts.rs +++ b/crates/perry-codegen/src/collectors/hir_facts.rs @@ -529,6 +529,18 @@ pub(crate) fn collect_type_facts( .chain(non_escaping_object_literals.keys()) .copied() .collect(); + // Representation-selection Phase 3b, #7034 §3: element-shape-proven local + // arrays. Purely syntactic, and computed FIRST — + // `collect_shape_proven_ptr_locals` consumes the facts (push containment, + // `A[i]` provenance, group integrity) and never re-enters this analysis, + // so the two passes cannot recurse. + let element_shape_facts = super::ptr_shape_elements::collect_element_shape_facts( + stmts, + boxed_vars, + module_globals, + classes, + module_dispatch, + ); // Representation-selection Phase 3b: shape-proven pointer locals. Gated // on `PERRY_PTR_SHAPE_LOCALS` and the module-wide §5.2 barrier scan // inside the collector. @@ -539,6 +551,7 @@ pub(crate) fn collect_type_facts( classes, module_dispatch, ¬_bigint_locals, + &element_shape_facts, ); // Representation-selection Phase 4a.3: `Ptr` locals. Gated on // `PERRY_PTR_NUMARRAY_LOCALS`, the module-wide §5.2 barrier scan, and the diff --git a/crates/perry-codegen/src/collectors/mod.rs b/crates/perry-codegen/src/collectors/mod.rs index c7c78a54ae..7d9abe9970 100644 --- a/crates/perry-codegen/src/collectors/mod.rs +++ b/crates/perry-codegen/src/collectors/mod.rs @@ -30,6 +30,7 @@ mod proven_this; mod proven_this_routing_tests; mod ptr_numarray; mod ptr_shape; +mod ptr_shape_elements; mod ptr_shape_report; mod ptr_shape_returns; mod refs; diff --git a/crates/perry-codegen/src/collectors/ptr_shape.rs b/crates/perry-codegen/src/collectors/ptr_shape.rs index 34802a1b49..305048c630 100644 --- a/crates/perry-codegen/src/collectors/ptr_shape.rs +++ b/crates/perry-codegen/src/collectors/ptr_shape.rs @@ -38,6 +38,17 @@ //! (defineProperty / delete / setPrototypeOf / Proxy / mutating Reflect) //! can reach it *through an alias*. //! +//! **Exception — the array-element position (#7034 §3).** `A.push()` does NOT disqualify when `A` is an **element-shape-proven +//! local array** (`collectors/ptr_shape_elements.rs`): that analysis bounds +//! the array's own uses exactly as this rule bounds an object local's, so +//! the containment region widens from one local to one local array and +//! everything derived from it. `const r = A[i]` at an in-bounds site is +//! provenance of the same `new`-strength for the same reason. Group +//! integrity is enforced at the end of the `'cand` loop: if any member of +//! an element group fails rule 2, every member is dropped, because one +//! member adding a property reshapes the objects the others read. +//! //! **Exception — the return position (#7034 §4).** `return ` //! does NOT disqualify. Containment exists to bound the object's aliases //! *while this function still reads it*, and a `return` is a terminator: @@ -114,6 +125,7 @@ use std::collections::{HashMap, HashSet}; use perry_hir::{Class, Expr, Stmt}; +use super::ptr_shape_elements::ElementShapeFacts; use super::ptr_shape_report as report; use super::ptr_shape_report::ShapeDenial; use super::ModuleDispatchFacts; @@ -275,6 +287,7 @@ pub(crate) fn collect_shape_proven_ptr_locals( classes: &HashMap, module_dispatch: &ModuleDispatchFacts, not_bigint_locals: &HashSet, + element_facts: &ElementShapeFacts, ) -> HashMap { if !ptr_shape_locals_enabled() { report_early_bail(stmts, boxed_vars, module_globals, report::GATE_DISABLED); @@ -314,6 +327,16 @@ pub(crate) fn collect_shape_proven_ptr_locals( module_dispatch, &mut candidates, ); + // #7034 §3: `const r = A[i]` at an in-bounds site on an element-shape- + // proven local array is provenance of `new C(...)` strength (module doc, + // rule 2's array-element exception). The seeds are already filtered for + // boxed / module-global / multi-`Let` ids and for the GC rooting + // obligation by `collectors/ptr_shape_elements.rs`. + let mut element_seeded: HashSet = HashSet::new(); + for (id, class_name) in super::ptr_shape_elements::element_read_seeds(stmts, element_facts) { + candidates.insert(id, class_name); + element_seeded.insert(id); + } if candidates.is_empty() { return HashMap::new(); } @@ -376,6 +399,8 @@ pub(crate) fn collect_shape_proven_ptr_locals( disq_reasons: HashMap::new(), escape_ctx: report::ESC_BARE_REFERENCE, return_seeded: &return_seeded, + element_seeded: &element_seeded, + element_facts, in_closure: false, }; walk.walk_stmts(stmts); @@ -481,7 +506,12 @@ pub(crate) fn collect_shape_proven_ptr_locals( // shape proof by itself still retires the whole guard diamond; this // is the same stand-down `collectors/proven_this.rs` makes, for the // same reason. - let numeric_fields = if return_seeded.contains(id) { + let numeric_fields = if return_seeded.contains(id) || element_facts.is_group_member(*id) { + // #7034 §3: an element-group member's object is reachable through + // the array, so the numeric proof's exhaustive-reachable-store + // obligation cannot be discharged from this local's stores alone — + // a sibling member's `r.score = "s"` is a store this proof never + // sees. Same stand-down as the return-seeded case above. HashSet::new() } else { prove_numeric_fields( @@ -519,12 +549,51 @@ pub(crate) fn collect_shape_proven_ptr_locals( } out.insert(*id, fact); } + // #7034 §3 group integrity. Every member of an element group references + // an object the OTHER members also reach, so a member that failed rule 2 + // — `r.extra = 1`, a closure capture, an opaque call — can transition the + // shape of objects the surviving members would then read guard-free. The + // group is therefore all-or-nothing. Dropping is the conservative + // direction and needs no fixpoint: removing members never admits one. + if !element_facts.is_empty() { + for (_, members) in element_facts.group_members() { + if members.iter().any(|m| !out.contains_key(m)) { + // The insert loop above gives every ALIAS of a promoted root + // the same fact, because an alias holds the same object. The + // removal has to follow: dropping `row` while `const a = row` + // keeps a guard-free proof of a shape a sibling just + // transitioned is exactly the hole all-or-nothing exists to + // close. (CodeRabbit, PR #7149.) + let doomed: Vec = members + .iter() + .copied() + .chain( + roots + .iter() + .filter(|(_, r)| members.contains(r)) + .map(|(m, _)| *m), + ) + .collect(); + for m in &doomed { + if out.remove(m).is_some() { + report::deny_local( + *m, + &names, + &depths, + candidates.get(m).map(String::as_str), + report::ESC_ELEMENT_GROUP, + ); + } + } + } + } + } out } /// Collect `Let { mutable: false, init: Some(LocalGet(src)) }` edges — the /// alias shape the exact-receiver inliner emits for compound assigns. -fn collect_alias_edges(stmts: &[Stmt], out: &mut Vec<(u32, u32)>) { +pub(super) fn collect_alias_edges(stmts: &[Stmt], out: &mut Vec<(u32, u32)>) { for s in stmts { match s { Stmt::Let { @@ -685,6 +754,13 @@ struct UseWalk<'a> { /// rather than a `new`. Their `Let` init is an `Expr::Call`, which rule 1 /// would otherwise reject as `LET_INIT_NOT_NEW`. return_seeded: &'a HashSet, + /// #7034 §3: candidates whose provenance is an in-bounds `A[i]` element + /// read. Their `Let` init is an `Expr::IndexGet`, which rule 1 would + /// otherwise reject as `LET_INIT_NOT_NEW`. + element_seeded: &'a HashSet, + /// #7034 §3: the element-shape-proven arrays of this region. Consulted to + /// decide whether a `push` is a contained element position. + element_facts: &'a ElementShapeFacts, /// #7034 §4: are we inside a closure body? A `return ` there /// escapes at an unbounded later time, so the return exemption (module /// doc, rule 2) does NOT apply — only the enclosing function's own @@ -770,6 +846,15 @@ impl<'a> UseWalk<'a> { return; } } + // #7034 §3: an element-read seed's provenance is the + // `A[i]` read. Both operands are bare `LocalGet`s of the + // array and its proven induction variable — neither is a + // `Ptr` candidate, so there is nothing to walk. + if self.element_seeded.contains(id) + && matches!(init.as_ref(), Some(Expr::IndexGet { .. })) + { + return; + } // A candidate whose Let init is not the New (var-redecl // seed) is not provenance-stable. self.disq(*id, report::LET_INIT_NOT_NEW); @@ -1078,9 +1163,22 @@ impl<'a> UseWalk<'a> { Expr::Update { id, .. } => { self.disq(*id, report::ESC_REASSIGNED); } + // #7034 §3: `A.push()` is a contained element + // position when `A` is element-shape-proven — the array's own uses + // are bounded by `collectors/ptr_shape_elements.rs` exactly as + // rule 2 bounds an object local's, so no alias escapes the region. + // Any other array, any other value shape, keeps today's escape. + Expr::ArrayPush { array_id, value } => { + self.disq(*array_id, report::ESC_CONTAINER_MUTATOR); + if let Expr::LocalGet(v) = value.as_ref() { + if self.element_facts.push_is_contained(*v, *array_id) { + return; + } + } + self.with_ctx(report::ESC_ELEMENT, |w| w.walk_expr(value)); + } // Id-keyed variants the child walker cannot see. - Expr::ArrayPush { array_id, .. } - | Expr::ArrayPushSpread { array_id, .. } + Expr::ArrayPushSpread { array_id, .. } | Expr::ArrayUnshift { array_id, .. } | Expr::ArraySplice { array_id, .. } | Expr::ArrayCopyWithin { array_id, .. } => { diff --git a/crates/perry-codegen/src/collectors/ptr_shape_elements.rs b/crates/perry-codegen/src/collectors/ptr_shape_elements.rs new file mode 100644 index 0000000000..698c103f1f --- /dev/null +++ b/crates/perry-codegen/src/collectors/ptr_shape_elements.rs @@ -0,0 +1,929 @@ +//! Representation-selection Phase 3b, **#7034 §3: array-element shape facts**. +//! +//! ## The escape this opens, and why it is not the return escape +//! +//! `collectors/ptr_shape.rs` rule 2 disqualifies a local at any escape. +//! #7034 §4 opened the `return` position; this file opens +//! `array/object element` for the array half — `rows.push(row)` / +//! `for (const r of rows) r.field`. Arrays of records are the data model of +//! real code, and #7139 measured the cost of not having it: of the ~103 +//! `Ptr` candidates its CommonJS barrier exemption freed in real +//! dependency modules, **100 were immediately re-denied by rule 2**. +//! +//! `return` was easy because a return is a **terminator**: no use of the local +//! can follow it, so every access the pass licenses already ran while the +//! object was still unaliased. An element escape is not a terminator — the +//! object stays reachable through the array for the rest of its life, and a +//! write through `arr[i]` to a non-declared field would invalidate the shape +//! proof after the fact. So the containment region has to be widened from +//! *one local* to *one local array and everything derived from it*, and the +//! array's own uses have to be bounded exactly as an object local's are. +//! +//! ## The rule +//! +//! A region-local binding `A` is an **element-shape-proven array** of class +//! `C` when every one of these holds. Each is an independent conjunct with its +//! own red test in `ptr_shape_elements_tests.rs`. +//! +//! * **E1 — array provenance.** Exactly one `Stmt::Let { mutable: false, +//! init: Some(Expr::Array([])) }` binds `A`, and `A` is neither boxed nor a +//! module global. The literal must be **empty**: a non-empty literal can +//! carry elisions (`[,,]` — holes that read back as `undefined`), and +//! admitting one buys nothing that the pushes below do not. +//! * **E2 — element provenance.** Every write into `A` is +//! `Expr::ArrayPush { array_id: A }` whose value is `new C(...)` — inline, +//! or a local bound by exactly one `Let { init: New { C } }` and pushed +//! exactly once. `Expr::New` covers closed object literals too +//! (`__AnonShape_…`), so records qualify. Perry class constructors cannot +//! return an override object, so the dynamic class is *exactly* `C`. No +//! other mutator is admitted at all — not `pop`/`shift`/`splice`/`unshift`/ +//! `copyWithin`, not `IndexSet`, not a `length` write — which is what makes +//! `A` **dense** and monomorphic for its whole lifetime. +//! * **E3 — array containment.** Every *other* use of `A` is an in-bounds +//! element read (E5), a `.length` read, or `return A`. The return exemption +//! is #7034 §4's, unchanged and for the same reason. Anything else — call +//! argument, closure capture, reassignment, `IndexSet`, an unrecognised +//! array method, being an element of another container — disqualifies `A`. +//! * **E4 — class admissibility.** `C` passes the same `chain_admissible` +//! gate rule 1 applies to a `new C(...)` local, and the module-wide rule-5 +//! barrier scan is clear. +//! * **E5 — in-bounds reads.** An element read is licensed only at +//! `A[i]` where `i` is the induction variable of an enclosing +//! `for (let i = 0; i < A.length; i++)` — literally that shape: zero init, +//! `Lt` against `A.length`, `++` update, and `i` written nowhere else in +//! the region. Because E3 admits no mutator that can *shrink* `A`, and +//! because `A` is dense by E2, `0 <= i < A.length` at the read means +//! `A[i]` is an own element, hence an instance of `C`. **This conjunct is +//! the whole difference between this pass and a wrong one**: without it +//! `A[i]` can be `undefined`, and a guard-free fixed-offset load masks a +//! NaN-boxed `undefined` into a wild pointer. +//! +//! `for (const r of A)` desugars to exactly the E5 shape +//! (`lower/stmt_loops.rs::lazy_or_index_elem` — a `__idx` local, `__idx < +//! __arr.length`, `Let r = IndexGet(__arr, __idx)`), so the iterator form is +//! covered by the indexed proof rather than by a second one. +//! +//! ## What the facts are used for +//! +//! Two halves, both consumed in `ptr_shape.rs`: +//! +//! 1. **Producer side.** `rows.push(row)` stops disqualifying `row` +//! ([`ElementShapeFacts::push_is_contained`]). +//! 2. **Reader side.** `const r = A[i]` at a licensed site is rule-1 +//! provenance of `new C(...)` strength +//! ([`ElementShapeFacts::element_read_class`]), so `r` becomes an ordinary +//! Phase 3b candidate and every `r.field` in the loop body lowers +//! guard-free through the machinery that already exists. +//! +//! ## Group integrity — why one failure voids the whole array +//! +//! Every member of an element group (the pushed producers plus the +//! element-read locals) is a reference to an object that the *other* members +//! also reach. If any one of them fails rule 2 — `r.extra = 1` adds a +//! property, a closure captures it, it is passed to an opaque call — the +//! objects in `A` can transition shape, and every other member's guard-free +//! access is then wrong. `ptr_shape.rs` therefore drops the **entire group** +//! when any member fails, rather than dropping the member. See +//! [`ElementShapeFacts::group_members`]. +//! +//! ## `numeric_fields` is deliberately not claimed +//! +//! Phase 3b's numeric-field proof is an *exhaustive reachable store* proof, +//! which containment makes possible because no alias exists. An element group +//! has aliases by construction: a store through one member +//! (`r.score = "s"` — a declared field, so rule 2 permits it) takes the +//! store-side boxed-setter exit and downgrades that slot's raw-f64 layout, +//! and another member's `load double` claiming `JsNumber` would then read +//! NaN-boxed string bits as a number. Group members therefore claim no +//! numeric fields at all — the same stand-down `proven_this.rs` and +//! `ptr_shape_returns.rs` make, for the same reason. The shape proof by +//! itself still retires the whole guard diamond. +//! +//! ## GC contract +//! +//! **No new site holds an object pointer at rest.** An element-read local is +//! an ordinary NaN-boxed slot, shadow-bound by `collect_pointer_typed_locals` +//! / `js_shadow_slot_bind` like any other object local, and every access +//! re-derives the raw pointer from that slot inside one region. `TaPtr`'s +//! callee-side no-bind shortcut is NOT copied — it is sound only for +//! non-movable typed-array storage, and `GC_TYPE_OBJECT` is movable (#6990, +//! #7019). +//! +//! That rooting is a **proof obligation, not an assumption**: +//! `collect_pointer_typed_locals` decides a local's slot from its init +//! expression's inferred type, and `IndexGet` on a local typed +//! `Array(Number)` infers `Number` — no slot. A lying annotation would leave a +//! promoted `Ptr` element in an unrooted alloca and an evacuating minor +//! would move the object without rewriting it. Both the array's declared +//! element type and the element local's own declared type are therefore +//! checked against `pointer_locals::is_definitely_non_pointer_type` before any +//! fact is issued. +//! +//! Write barriers on the `push` are untouched: a proven shape says nothing +//! about whether the stored value is a pointer, and this pass changes no store +//! lowering at all. +//! +//! Gated by `PERRY_PTR_SHAPE_LOCALS` along with the rest of Phase 3b — no new +//! env knob, so there is no new unexercised off-state (CLAUDE.md's GC knob +//! kill-policy). + +use std::collections::{HashMap, HashSet}; + +use perry_hir::types::Type; +use perry_hir::{Class, Expr, Stmt}; + +use super::pointer_locals::is_definitely_non_pointer_type; +use super::ptr_shape::{chain_admissible, ptr_shape_locals_enabled}; +use super::ModuleDispatchFacts; + +/// Element-shape facts for one lowered region. +#[derive(Debug, Default, Clone)] +pub(crate) struct ElementShapeFacts { + /// Array ROOT id -> proven element class. + arrays: HashMap, + /// Array alias id (including the root itself) -> root id. + array_roots: HashMap, + /// Producer local -> (array root it is pushed into, element class). + pushed: HashMap, + /// Element-read local -> (array root it was read from, element class). + element_reads: HashMap, +} + +impl ElementShapeFacts { + pub(crate) fn is_empty(&self) -> bool { + self.arrays.is_empty() + } + + /// Test-only: are ALL fact maps empty? `is_empty` gates every consumer and + /// keys on `arrays` alone; the other three are kept consistent with it by + /// construction, and `tests::is_empty_covers_every_fact_map` is what holds + /// that invariant in place. + #[cfg(test)] + pub(crate) fn debug_all_maps_empty(&self) -> bool { + self.arrays.is_empty() + && self.array_roots.is_empty() + && self.pushed.is_empty() + && self.element_reads.is_empty() + } + + /// Is `value_local`'s push into `array_local` covered by a proven array, + /// so rule 2 may treat that element position as contained? + pub(crate) fn push_is_contained(&self, value_local: u32, array_local: u32) -> bool { + let Some(root) = self.array_roots.get(&array_local) else { + return false; + }; + matches!(self.pushed.get(&value_local), Some((r, _)) if r == root) + } + + /// The proven element class of a licensed `const r = A[i]` binding. + pub(crate) fn element_read_class(&self, local: u32) -> Option<&str> { + self.element_reads.get(&local).map(|(_, c)| c.as_str()) + } + + /// Every local that references an object stored in `root`'s array. + /// + /// Group integrity: `ptr_shape.rs` promotes all of these or none of them. + pub(crate) fn group_members(&self) -> HashMap> { + let mut out: HashMap> = HashMap::new(); + for (id, (root, _)) in self.pushed.iter().chain(self.element_reads.iter()) { + out.entry(*root).or_default().push(*id); + } + out + } + + /// Locals whose object is reachable from an array, and which therefore + /// must not claim numeric fields (module doc). + pub(crate) fn is_group_member(&self, local: u32) -> bool { + self.pushed.contains_key(&local) || self.element_reads.contains_key(&local) + } +} + +/// Entry point: prove the element-shape-proven local arrays of one region. +/// +/// Purely syntactic — it never re-enters +/// [`super::ptr_shape::collect_shape_proven_ptr_locals`], so there is no +/// recursion between the two passes and no fixpoint to converge. Rule 2 +/// containment of the resulting group members is enforced by that pass +/// afterwards, via the group-integrity filter. +pub(crate) fn collect_element_shape_facts( + stmts: &[Stmt], + boxed_vars: &HashSet, + module_globals: &HashMap, + classes: &HashMap, + module_dispatch: &ModuleDispatchFacts, +) -> ElementShapeFacts { + let out = ElementShapeFacts::default(); + if !ptr_shape_locals_enabled() || module_dispatch.has_shape_barrier_sites() { + return out; + } + + // E1: array provenance. + let mut let_counts: HashMap = HashMap::new(); + let mut array_seeds: HashMap = HashMap::new(); + let mut new_lets: HashMap> = HashMap::new(); + let mut elem_let_ty: HashMap = HashMap::new(); + walk_stmts(stmts, &mut |s| { + let Stmt::Let { + id, + ty, + mutable, + init, + .. + } = s + else { + return; + }; + *let_counts.entry(*id).or_insert(0) += 1; + elem_let_ty.insert(*id, ty.clone()); + match init.as_ref() { + Some(Expr::Array(items)) if !*mutable && items.is_empty() => { + if !boxed_vars.contains(id) && !module_globals.contains_key(id) { + array_seeds.insert(*id, ty.clone()); + } + } + Some(Expr::New { class_name, .. }) => { + // Producer-local provenance for E2's `LocalGet` form. A second + // `Let` for the same id poisons it via `let_counts`. + if !boxed_vars.contains(id) && !module_globals.contains_key(id) { + new_lets.insert(*id, Some(class_name.clone())); + } + } + _ => {} + } + }); + array_seeds.retain(|id, _| let_counts.get(id).copied().unwrap_or(0) == 1); + new_lets.retain(|id, _| let_counts.get(id).copied().unwrap_or(0) == 1); + if array_seeds.is_empty() { + return out; + } + + // Array aliases: `const b = a` tracks the same array object. Same shape + // (and same reason) as `ptr_shape.rs`'s alias pre-pass — the `for…of` + // desugar binds `const __arr_N = ` before the loop. + let mut array_roots: HashMap = array_seeds.keys().map(|id| (*id, *id)).collect(); + let mut alias_edges: Vec<(u32, u32)> = Vec::new(); + super::ptr_shape::collect_alias_edges(stmts, &mut alias_edges); + loop { + let mut changed = false; + for (alias, src) in &alias_edges { + if array_roots.contains_key(alias) + || boxed_vars.contains(alias) + || module_globals.contains_key(alias) + || let_counts.get(alias).copied().unwrap_or(0) != 1 + { + continue; + } + if let Some(&root) = array_roots.get(src) { + array_roots.insert(*alias, root); + changed = true; + } + } + if !changed { + break; + } + } + + // E3/E5: the array use walk. + let mut walk = ArrayWalk { + roots: &array_roots, + disqualified: HashSet::new(), + pushes: HashMap::new(), + reads: Vec::new(), + idx_writes: HashMap::new(), + bounded: Vec::new(), + in_closure: false, + }; + walk.walk_stmts(stmts); + let ArrayWalk { + mut disqualified, + pushes, + reads, + idx_writes, + .. + } = walk; + + // E2: one element class per array, from the push sites. + let mut arrays: HashMap = HashMap::new(); + let mut pushed: HashMap = HashMap::new(); + // A producer local pushed more than once is not exempted: two element + // groups would both claim it and the single-group soundness argument + // ("every reference to this object is a member of this group") no longer + // holds. Cheap to count, and it keeps the argument one sentence long. + let mut push_value_counts: HashMap = HashMap::new(); + for sites in pushes.values() { + for site in sites { + if let PushValue::Local(v) = site { + *push_value_counts.entry(*v).or_insert(0) += 1; + } + } + } + for (root, sites) in &pushes { + if disqualified.contains(root) || sites.is_empty() { + continue; + } + let mut class_name: Option<&str> = None; + let mut ok = true; + let mut members: Vec = Vec::new(); + for site in sites { + let name = match site { + PushValue::Fresh(c) => c.as_str(), + PushValue::Local(v) => { + if push_value_counts.get(v).copied().unwrap_or(0) != 1 { + ok = false; + break; + } + match new_lets.get(v) { + Some(Some(c)) => { + members.push(*v); + c.as_str() + } + _ => { + ok = false; + break; + } + } + } + PushValue::Other => { + ok = false; + break; + } + }; + match class_name { + None => class_name = Some(name), + Some(prev) if prev == name => {} + Some(_) => { + ok = false; + break; + } + } + } + let Some(class_name) = class_name.filter(|_| ok) else { + disqualified.insert(*root); + continue; + }; + // E4 + the GC rooting obligation on the array's declared element type. + if !classes.contains_key(class_name) + || !chain_admissible(classes, class_name) + || !array_type_keeps_element_slot(array_seeds.get(root)) + { + disqualified.insert(*root); + continue; + } + arrays.insert(*root, class_name.to_string()); + for m in members { + pushed.insert(m, (*root, class_name.to_string())); + } + } + if arrays.is_empty() { + return ElementShapeFacts::default(); + } + pushed.retain(|_, (root, _)| arrays.contains_key(root)); + + // E5: element-read seeds at licensed sites. + let mut element_reads: HashMap = HashMap::new(); + for read in &reads { + let Some(class_name) = arrays.get(&read.root) else { + continue; + }; + // The induction variable must be written exactly twice in the whole + // region — its `Let` and its `++` — and by the loop that bounds it. + if idx_writes.get(&read.index).copied().unwrap_or(0) != 2 + || boxed_vars.contains(&read.index) + || module_globals.contains_key(&read.index) + { + continue; + } + if boxed_vars.contains(&read.local) || module_globals.contains_key(&read.local) { + continue; + } + if let_counts.get(&read.local).copied().unwrap_or(0) != 1 { + continue; + } + // GC: the binding must keep a shadow slot (module doc). + if elem_let_ty + .get(&read.local) + .is_some_and(is_definitely_non_pointer_type) + { + continue; + } + element_reads.insert(read.local, (read.root, class_name.clone())); + } + + ElementShapeFacts { + arrays, + array_roots, + pushed, + element_reads, + } +} + +/// Would an `A[i]` binding keep its shadow-stack root slot? +/// +/// `collect_pointer_typed_locals` infers an `IndexGet`'s value type from the +/// object's type: `Array(elem)` yields `elem`, and a `Number`/`Boolean`/… +/// element type means "definitely not a pointer", which drops the slot. A +/// declared `number[]` that this pass proved holds `C` instances (Perry does +/// not validate annotations) would then leave a promoted element unrooted — +/// #7019 ships an evacuating minor by default, so that is a live wrong answer, +/// not a theoretical one. Refuse the fact instead of trusting the annotation. +fn array_type_keeps_element_slot(ty: Option<&Type>) -> bool { + match ty { + Some(Type::Array(elem)) => !is_definitely_non_pointer_type(elem), + // Any other declared type either yields no `IndexGet` inference (so + // the binding's own declared type decides, checked separately at the + // read site) or is not an array type at all. + _ => true, + } +} + +// ── The array use walk ───────────────────────────────────────────────────── + +#[derive(Debug, Clone, PartialEq, Eq)] +enum PushValue { + /// `A.push(new C(…))` — inline allocation. + Fresh(String), + /// `A.push(v)` — a local. + Local(u32), + /// Anything else; disqualifies the array. + Other, +} + +struct ReadSite { + /// The array root read from. + root: u32, + /// The induction variable local. + index: u32, + /// The local the element was bound to. + local: u32, +} + +struct ArrayWalk<'a> { + roots: &'a HashMap, + disqualified: HashSet, + pushes: HashMap>, + reads: Vec, + /// local id -> number of writes (`Let` / `LocalSet` / `Update`) anywhere + /// in the region, closures included. + idx_writes: HashMap, + /// Induction variables currently proven `0 <= i < root.length`, innermost + /// last. + bounded: Vec<(u32, u32)>, + in_closure: bool, +} + +impl<'a> ArrayWalk<'a> { + fn root_of(&self, id: u32) -> Option { + self.roots.get(&id).copied() + } + + fn disq(&mut self, id: u32) { + if let Some(root) = self.root_of(id) { + self.disqualified.insert(root); + } + } + + fn note_write(&mut self, id: u32) { + *self.idx_writes.entry(id).or_insert(0) += 1; + } + + fn walk_stmts(&mut self, stmts: &[Stmt]) { + for s in stmts { + self.walk_stmt(s); + } + } + + fn walk_stmt(&mut self, s: &Stmt) { + match s { + Stmt::Let { id, init, .. } => { + self.note_write(*id); + if let Some(e) = init { + // An array alias binding is the tracked edge itself. + if let Expr::LocalGet(src) = e { + if self.root_of(*src).is_some() && self.root_of(*id).is_some() { + return; + } + } + // E5: `const r = A[i]` at a licensed site. This is the + // ONLY position an element read is admitted in — see the + // `Expr::IndexGet` arm for why every other one is an + // escape of the ELEMENT (as opposed to the array). + if let Expr::IndexGet { object, index } = e { + if let (Expr::LocalGet(a), Expr::LocalGet(i)) = + (object.as_ref(), index.as_ref()) + { + if let Some(root) = self.root_of(*a) { + if !self.in_closure + && self.bounded.iter().any(|(bi, br)| bi == i && *br == root) + { + self.reads.push(ReadSite { + root, + index: *i, + local: *id, + }); + return; + } + } + } + } + self.walk_expr(e); + } + } + Stmt::Return(Some(e)) => { + // #7034 §4's terminator exemption, applied to the array. + if !self.in_closure { + if let Expr::LocalGet(id) = e { + if self.root_of(*id).is_some() { + return; + } + } + } + self.walk_expr(e); + } + Stmt::Return(None) => {} + Stmt::Expr(e) => self.walk_expr(e), + Stmt::Throw(e) => self.walk_expr(e), + Stmt::If { + condition, + then_branch, + else_branch, + } => { + self.walk_expr(condition); + self.walk_stmts(then_branch); + if let Some(eb) = else_branch { + self.walk_stmts(eb); + } + } + Stmt::While { condition, body } | Stmt::DoWhile { body, condition } => { + self.walk_expr(condition); + self.walk_stmts(body); + } + Stmt::For { + init, + condition, + update, + body, + } => { + let bound = self.bounded_induction(init.as_deref(), condition, update); + if let Some(init) = init { + self.walk_stmt(init.as_ref()); + } + if let Some(c) = condition { + self.walk_expr(c); + } + if let Some(u) = update { + self.walk_expr(u); + } + let pushed_bound = bound.is_some(); + if let Some(b) = bound { + self.bounded.push(b); + } + self.walk_stmts(body); + if pushed_bound { + self.bounded.pop(); + } + } + Stmt::Try { + body, + catch, + finally, + } => { + self.walk_stmts(body); + if let Some(c) = catch { + self.walk_stmts(&c.body); + } + if let Some(f) = finally { + self.walk_stmts(f); + } + } + Stmt::Switch { + discriminant, + cases, + } => { + self.walk_expr(discriminant); + for case in cases { + if let Some(t) = &case.test { + self.walk_expr(t); + } + self.walk_stmts(&case.body); + } + } + Stmt::Labeled { body, .. } => self.walk_stmt(body.as_ref()), + Stmt::Break + | Stmt::Continue + | Stmt::LabeledBreak(_) + | Stmt::LabeledContinue(_) + | Stmt::PreallocateBoxes(_) + | Stmt::PreallocateTdzBoxes(_) => {} + } + } + + /// E5: recognise `for (let i = 0; i < A.length; i++)` and return + /// `(i, root)`. + fn bounded_induction( + &self, + init: Option<&Stmt>, + condition: &Option, + update: &Option, + ) -> Option<(u32, u32)> { + if self.in_closure { + return None; + } + let Some(Stmt::Let { + id: idx, + mutable: true, + init: Some(zero), + .. + }) = init + else { + return None; + }; + match zero { + Expr::Number(n) if *n == 0.0 => {} + Expr::Integer(0) => {} + _ => return None, + } + let Some(Expr::Compare { + op: perry_hir::CompareOp::Lt, + left, + right, + }) = condition + else { + return None; + }; + if !matches!(left.as_ref(), Expr::LocalGet(l) if l == idx) { + return None; + } + let Expr::PropertyGet { + object, property, .. + } = right.as_ref() + else { + return None; + }; + if property != "length" { + return None; + } + let Expr::LocalGet(a) = object.as_ref() else { + return None; + }; + let root = self.root_of(*a)?; + match update { + Some(Expr::Update { + id, + op: perry_hir::UpdateOp::Increment, + .. + }) if id == idx => {} + _ => return None, + } + Some((*idx, root)) + } + + fn walk_expr(&mut self, e: &Expr) { + match e { + // `A.length` — the only property read admitted on the array. + Expr::PropertyGet { + object, property, .. + } => { + if let Expr::LocalGet(id) = object.as_ref() { + if self.root_of(*id).is_some() { + if property != "length" { + self.disq(*id); + } + return; + } + } + self.walk_expr(object); + } + Expr::PropertySet { + object, + property, + value, + } => { + if let Expr::LocalGet(id) = object.as_ref() { + if self.root_of(*id).is_some() { + // Any property write on the array itself — including + // `A.length = 0`, which would punch holes. + self.disq(*id); + self.walk_expr(value); + return; + } + } + let _ = property; + self.walk_expr(object); + self.walk_expr(value); + } + Expr::PropertyUpdate { object, .. } => { + if let Expr::LocalGet(id) = object.as_ref() { + if self.root_of(*id).is_some() { + self.disq(*id); + return; + } + } + self.walk_expr(object); + } + // E2: the one admitted write. + Expr::ArrayPush { array_id, value } => { + if let Some(root) = self.root_of(*array_id) { + let site = match value.as_ref() { + Expr::New { class_name, .. } => PushValue::Fresh(class_name.clone()), + Expr::LocalGet(v) if self.root_of(*v).is_none() => PushValue::Local(*v), + _ => PushValue::Other, + }; + self.pushes.entry(root).or_default().push(site); + // A tracked array stored as an ELEMENT of another array is + // reachable and mutable through that array — + // `outer[0][0] = x` is an `IndexSet` on an `IndexGet`, + // which neither walk tracks. Making the outer array + // `PushValue::Other` disqualifies the OUTER one only, and + // the arm below deliberately skips `walk_expr` for a + // `LocalGet` value, so the inner one would never reach the + // bare-reference arm. Disqualify it here. A producer local + // is not a tracked root, so the admitted `PushValue::Local` + // case is unaffected. (CodeRabbit, PR #7149.) + if let Expr::LocalGet(v) = value.as_ref() { + self.disq(*v); + return; + } + // Still walk for OTHER arrays nested in the value. + self.walk_expr(value); + return; + } + self.walk_expr(value); + } + // Every other id-keyed container mutator is a hard disqualifier. + Expr::ArrayPushSpread { array_id, .. } + | Expr::ArrayUnshift { array_id, .. } + | Expr::ArraySplice { array_id, .. } + | Expr::ArrayCopyWithin { array_id, .. } => { + self.disq(*array_id); + perry_hir::walker::walk_expr_children(e, &mut |c| self.walk_expr(c)); + } + Expr::ArrayPop(id) | Expr::ArrayShift(id) => self.disq(*id), + Expr::IndexSet { + object, + index, + value, + } => { + if let Expr::LocalGet(id) = object.as_ref() { + self.disq(*id); + } + self.walk_expr(object); + self.walk_expr(index); + self.walk_expr(value); + } + Expr::IndexUpdate { object, index, .. } => { + if let Expr::LocalGet(id) = object.as_ref() { + self.disq(*id); + } + self.walk_expr(object); + self.walk_expr(index); + } + // ★ An element read the `Stmt::Let` arm did not license. + // + // It is tempting to treat this as harmless — a read cannot + // transition a shape — but it hands out a REFERENCE to an element + // that this walk then stops tracking. `f(A[i])` lets an opaque + // callee add a property to an object that a licensed + // `const s = A[k]` reads guard-free; `A[i].m()` runs a method + // whose `this`-flow nothing vetted; `const r = A[0]` (a literal + // index, so unlicensed) binds an element to a local that rule 2 + // never sees, and `r.extra = 1` reshapes it. All three would make + // the OTHER members' fixed-offset loads wrong. + // + // So every unlicensed element read disqualifies the array. Direct + // `A[i].field` access is therefore not covered at all today, and + // widening to it needs the element class in this walk (which is + // only known after the push scan) — tracked in #7151. + Expr::IndexGet { object, index } => { + if let Expr::LocalGet(id) = object.as_ref() { + self.disq(*id); + } + self.walk_expr(object); + self.walk_expr(index); + } + Expr::LocalSet(id, v) => { + self.note_write(*id); + self.disq(*id); + self.walk_expr(v); + } + Expr::Update { id, .. } => { + self.note_write(*id); + self.disq(*id); + } + // Any bare reference to the array in a position the arms above did + // not admit — a call argument, a `new` argument, an element of + // another container, a spread, an untracked `Let` init (mutable, + // boxed, or module-global, so the alias pre-pass skipped it) — + // creates an alias this walk cannot follow. + Expr::LocalGet(id) => self.disq(*id), + Expr::Closure { + body, + captures, + mutable_captures, + .. + } => { + for c in captures.iter().chain(mutable_captures.iter()) { + self.disq(*c); + } + let outer = self.in_closure; + self.in_closure = true; + self.walk_stmts(body); + self.in_closure = outer; + } + _ => { + perry_hir::walker::walk_expr_children(e, &mut |c| self.walk_expr(c)); + } + } + } +} + +/// Statement walker over one region's statement tree. +/// +/// It does NOT descend into closure bodies — those live inside `Expr`s, not +/// `Stmt`s. That is correct for both callers and deliberately so: +/// `collect_element_shape_facts` uses it for the E1 seed scan, where a closure +/// body's locals are its own ids and an outer array referenced from one is +/// disqualified by `ArrayWalk` (which does descend, through its own +/// `Expr::Closure` arm); and `element_read_seeds` uses it for the read seeds, +/// where `bounded_induction` already refuses to license anything under +/// `in_closure`, so a closure-body read is never a seed to find. +fn walk_stmts<'a>(stmts: &'a [Stmt], f: &mut impl FnMut(&'a Stmt)) { + for s in stmts { + f(s); + match s { + Stmt::If { + then_branch, + else_branch, + .. + } => { + walk_stmts(then_branch, f); + if let Some(eb) = else_branch { + walk_stmts(eb, f); + } + } + Stmt::While { body, .. } | Stmt::DoWhile { body, .. } => walk_stmts(body, f), + Stmt::For { init, body, .. } => { + if let Some(init) = init { + walk_stmts(std::slice::from_ref(init.as_ref()), f); + } + walk_stmts(body, f); + } + Stmt::Try { + body, + catch, + finally, + } => { + walk_stmts(body, f); + if let Some(c) = catch { + walk_stmts(&c.body, f); + } + if let Some(fin) = finally { + walk_stmts(fin, f); + } + } + Stmt::Switch { cases, .. } => { + for c in cases { + walk_stmts(&c.body, f); + } + } + Stmt::Labeled { body, .. } => walk_stmts(std::slice::from_ref(body.as_ref()), f), + _ => {} + } + } +} + +/// The `const r = A[i]` bindings this region may seed as rule-1 provenance. +/// +/// The site test is [`collect_element_shape_facts`] above; this only pairs the +/// ids it licensed with the `Stmt::Let` that binds them, so `ptr_shape.rs` +/// never seeds a candidate for an id with no binding in the region it is +/// analysing. +pub(super) fn element_read_seeds( + stmts: &[Stmt], + element_facts: &ElementShapeFacts, +) -> Vec<(u32, String)> { + let mut out = Vec::new(); + if element_facts.is_empty() { + return out; + } + // Reuses this module's one statement walker rather than carrying a second + // copy of the traversal. Two traversals that have to agree, drifting by + // one `Stmt` variant, is a bug class this file cannot afford: a missed + // variant here silently withholds a SEED, and a missed variant in + // `ArrayWalk` silently withholds a DISQUALIFICATION. + walk_stmts(stmts, &mut |s| { + if let Stmt::Let { + id, + init: Some(Expr::IndexGet { .. }), + .. + } = s + { + if let Some(class_name) = element_facts.element_read_class(*id) { + out.push((*id, class_name.to_string())); + } + } + }); + out +} +#[cfg(test)] +#[path = "ptr_shape_elements_tests.rs"] +mod tests; diff --git a/crates/perry-codegen/src/collectors/ptr_shape_elements_tests.rs b/crates/perry-codegen/src/collectors/ptr_shape_elements_tests.rs new file mode 100644 index 0000000000..41180e8c60 --- /dev/null +++ b/crates/perry-codegen/src/collectors/ptr_shape_elements_tests.rs @@ -0,0 +1,1208 @@ +//! #7034 §3 array-element shape facts: both halves of the proof, and the +//! cases that must NOT get one. +//! +//! Every positive test here fails against the pre-#7034-§3 collector (the +//! local was denied with rule 2 "stored into a container", or the `A[i]` +//! binding was never a candidate at all), and **every conjunct of the rule +//! has its own disjoint red set** — each negative test names, in its doc, the +//! single guard whose deletion makes it fail. + +use super::*; +use crate::collectors::PtrShapeLocal; +use perry_hir::types::Type; +use perry_hir::{ClassField, CompareOp, Function, Module, UpdateOp}; + +// ── Fixture builders ─────────────────────────────────────────────────────── + +fn field(name: &str) -> ClassField { + ClassField { + name: name.to_string(), + key_expr: None, + ty: Type::Number, + init: None, + is_private: false, + is_readonly: false, + decorators: Vec::new(), + } +} + +fn class_c() -> Class { + Class { + id: 0, + name: "C".to_string(), + type_params: Vec::new(), + extends: None, + extends_name: None, + native_extends: None, + extends_expr: None, + heritage_lexically_shadowed: false, + fields: vec![field("x"), field("y")], + constructor: None, + methods: Vec::new(), + getters: Vec::new(), + setters: Vec::new(), + static_fields: Vec::new(), + static_methods: Vec::new(), + computed_members: Vec::new(), + decorators: Vec::new(), + is_exported: false, + aliases: Vec::new(), + is_nested: false, + alloc_width_hint: 0, + static_accessor_names: Vec::new(), + static_accessor_fn_ids: Vec::new(), + } +} + +fn class_d() -> Class { + let mut d = class_c(); + d.id = 1; + d.name = "D".to_string(); + d +} + +fn new_of(name: &str) -> Expr { + Expr::New { + class_name: name.to_string(), + args: Vec::new(), + type_args: Vec::new(), + byte_offset: 0, + cap_args_appended: 0, + } +} + +fn new_c() -> Expr { + new_of("C") +} + +/// `const = new C();` +fn let_c(id: u32, name: &str) -> Stmt { + Stmt::Let { + id, + name: name.to_string(), + ty: Type::Named("C".to_string()), + mutable: false, + init: Some(new_c()), + } +} + +/// `const : C[] = [];` +fn let_arr(id: u32, name: &str) -> Stmt { + let_arr_ty( + id, + name, + Type::Array(Box::new(Type::Named("C".to_string()))), + ) +} + +fn let_arr_ty(id: u32, name: &str, ty: Type) -> Stmt { + Stmt::Let { + id, + name: name.to_string(), + ty, + mutable: false, + init: Some(Expr::Array(Vec::new())), + } +} + +fn push(array_id: u32, value: Expr) -> Stmt { + Stmt::Expr(Expr::ArrayPush { + array_id, + value: Box::new(value), + }) +} + +fn read_x(id: u32) -> Stmt { + Stmt::Expr(Expr::PropertyGet { + object: Box::new(Expr::LocalGet(id)), + property: "x".to_string(), + byte_offset: 0, + }) +} + +fn store_x(id: u32) -> Stmt { + Stmt::Expr(Expr::PropertySet { + object: Box::new(Expr::LocalGet(id)), + property: "x".to_string(), + value: Box::new(Expr::Number(1.0)), + }) +} + +/// `for (let = 0; < .length; ++) { body }` +fn bounded_loop(idx: u32, arr: u32, body: Vec) -> Stmt { + bounded_loop_cond( + idx, + Expr::Compare { + op: CompareOp::Lt, + left: Box::new(Expr::LocalGet(idx)), + right: Box::new(Expr::PropertyGet { + object: Box::new(Expr::LocalGet(arr)), + property: "length".to_string(), + byte_offset: 0, + }), + }, + body, + ) +} + +fn bounded_loop_cond(idx: u32, condition: Expr, body: Vec) -> Stmt { + Stmt::For { + init: Some(Box::new(Stmt::Let { + id: idx, + name: format!("i{idx}"), + ty: Type::Number, + mutable: true, + init: Some(Expr::Number(0.0)), + })), + condition: Some(condition), + update: Some(Expr::Update { + id: idx, + op: UpdateOp::Increment, + prefix: false, + }), + body, + } +} + +/// `const = [];` +fn let_elem(id: u32, name: &str, arr: u32, idx: u32) -> Stmt { + let_elem_ty(id, name, arr, idx, Type::Named("C".to_string())) +} + +fn let_elem_ty(id: u32, name: &str, arr: u32, idx: u32, ty: Type) -> Stmt { + Stmt::Let { + id, + name: name.to_string(), + ty, + mutable: false, + init: Some(Expr::IndexGet { + object: Box::new(Expr::LocalGet(arr)), + index: Box::new(Expr::LocalGet(idx)), + }), + } +} + +fn classes_of<'a>(cs: &'a [Class]) -> HashMap { + cs.iter().map(|c| (c.name.clone(), c)).collect() +} + +/// Real module dispatch facts, so the rule-5 barrier flags are the real ones. +/// +/// `classes` is the test's OWN class list, not a pristine `class_c()`. A test +/// that mutates its class (an accessor, a computed member) and then hands the +/// mutation only to `chain_admissible` while the dispatch facts were built +/// from a different class is the classic vacuous-pass shape: it can go green +/// on a rule other than the one under test. `ptr_shape_returns_tests.rs` grew +/// `facts_for_classes` for exactly this reason. +fn facts_for(classes: &HashMap, functions: Vec) -> ModuleDispatchFacts { + let mut hir = Module::new("t"); + let mut names: Vec<&String> = classes.keys().collect(); + names.sort(); + for n in names { + hir.classes.push(classes[n].clone()); + } + hir.functions = functions; + super::super::collect_module_dispatch_facts(&hir) +} + +fn elements(stmts: &[Stmt], classes: &HashMap) -> ElementShapeFacts { + elements_with(stmts, classes, &facts_for(classes, Vec::new())) +} + +fn elements_with( + stmts: &[Stmt], + classes: &HashMap, + facts: &ModuleDispatchFacts, +) -> ElementShapeFacts { + collect_element_shape_facts(stmts, &HashSet::new(), &HashMap::new(), classes, facts) +} + +/// The full Phase 3b verdict, element facts included — what codegen sees. +fn promote(stmts: &[Stmt], classes: &HashMap) -> HashMap { + let facts = facts_for(classes, Vec::new()); + let els = elements_with(stmts, classes, &facts); + super::super::ptr_shape::collect_shape_proven_ptr_locals( + stmts, + &HashSet::new(), + &HashMap::new(), + classes, + &facts, + &HashSet::new(), + &els, + ) +} + +// ── The producer half ────────────────────────────────────────────────────── + +/// `const a = []; const o = new C(); o.x = 1; a.push(o);` — the single most +/// common record-producing idiom there is. Denied by rule 2 before #7034 §3. +/// +/// Sabotage: delete the `push_is_contained` arm in `ptr_shape.rs`'s +/// `Expr::ArrayPush` and this fails. +#[test] +fn pushed_local_is_promoted() { + let c = class_c(); + let cs = [c]; + let classes = classes_of(&cs); + let stmts = vec![ + let_arr(1, "rows"), + let_c(2, "row"), + store_x(2), + push(1, Expr::LocalGet(2)), + ]; + let promoted = promote(&stmts, &classes); + let fact = promoted + .get(&2) + .expect("a local whose only escape is a push into a proven array must promote"); + assert_eq!(fact.class_name, "C"); + assert!( + fact.numeric_fields.is_empty(), + "an element-group member must never claim numeric fields: a sibling's \ + store through the array is a reachable store this proof cannot see" + ); +} + +/// The array is proven even when it is returned — #7034 §4's terminator +/// exemption, applied to the array rather than to the record. +/// +/// Sabotage: delete the `Stmt::Return` arm in `ArrayWalk` and this fails. +#[test] +fn returned_array_is_still_proven() { + let c = class_c(); + let cs = [c]; + let classes = classes_of(&cs); + let stmts = vec![ + let_arr(1, "rows"), + let_c(2, "row"), + push(1, Expr::LocalGet(2)), + Stmt::Return(Some(Expr::LocalGet(1))), + ]; + assert!(promote(&stmts, &classes).contains_key(&2)); +} + +/// **E3.** The array passed as a call argument can be reshaped by the callee, +/// so nothing about its elements is provable. +/// +/// Sabotage: make `ArrayWalk`'s `Expr::LocalGet` arm a no-op and this fails. +#[test] +fn array_escaping_as_a_call_argument_denies_the_push() { + let c = class_c(); + let cs = [c]; + let classes = classes_of(&cs); + let stmts = vec![ + let_arr(1, "rows"), + let_c(2, "row"), + push(1, Expr::LocalGet(2)), + Stmt::Expr(Expr::Call { + callee: Box::new(Expr::FuncRef(9)), + args: vec![Expr::LocalGet(1)], + type_args: Vec::new(), + byte_offset: 0, + }), + ]; + assert!( + !promote(&stmts, &classes).contains_key(&2), + "an array that escapes to an opaque callee proves nothing" + ); +} + +/// **E3.** A closure that captures the array can reshape its elements at an +/// unbounded later time. +/// +/// Sabotage: delete `ArrayWalk`'s `Expr::Closure` arm and this fails. +#[test] +fn array_captured_by_a_closure_denies_the_push() { + let c = class_c(); + let cs = [c]; + let classes = classes_of(&cs); + let stmts = vec![ + let_arr(1, "rows"), + let_c(2, "row"), + push(1, Expr::LocalGet(2)), + Stmt::Expr(Expr::Closure { + func_id: 99, + params: Vec::new(), + return_type: Type::Any, + // Deliberately NOT referenced in the body: the guard under test + // is the capture LIST, and a body reference would deny through + // the ordinary bare-reference arm instead, making this vacuous. + body: Vec::new(), + captures: vec![1], + mutable_captures: Vec::new(), + captures_this: false, + captures_new_target: false, + enclosing_class: None, + is_arrow: true, + is_async: false, + is_generator: false, + is_strict: false, + }), + ]; + assert!(!promote(&stmts, &classes).contains_key(&2)); +} + +/// **E2.** `pop`/`shift`/`splice`/`unshift` are the mutators that make the +/// array non-dense (or that re-index it), which is what E5's in-bounds +/// argument rests on. +/// +/// Sabotage: drop the `ArrayPop`/`ArrayShift` arm and this fails. +#[test] +fn a_shrinking_mutator_denies_the_array() { + let c = class_c(); + let cs = [c]; + let classes = classes_of(&cs); + let stmts = vec![ + let_arr(1, "rows"), + let_c(2, "row"), + push(1, Expr::LocalGet(2)), + Stmt::Expr(Expr::ArrayPop(1)), + ]; + assert!(!promote(&stmts, &classes).contains_key(&2)); +} + +/// **E2.** `A[k] = v` can write any value at any index, including past the +/// end (which punches holes). +/// +/// Sabotage: delete `ArrayWalk`'s `Expr::IndexSet` arm and this fails. +#[test] +fn an_indexed_store_denies_the_array() { + let c = class_c(); + let cs = [c]; + let classes = classes_of(&cs); + let stmts = vec![ + let_arr(1, "rows"), + let_c(2, "row"), + push(1, Expr::LocalGet(2)), + Stmt::Expr(Expr::IndexSet { + object: Box::new(Expr::LocalGet(1)), + index: Box::new(Expr::Number(9.0)), + value: Box::new(Expr::Number(1.0)), + }), + ]; + assert!(!promote(&stmts, &classes).contains_key(&2)); +} + +/// **E2.** Two classes in one array is exactly the polymorphism the fact +/// claims does not happen. +/// +/// Sabotage: delete the `class_name` agreement check and this fails. +#[test] +fn mixed_element_classes_deny_the_array() { + let cs = [class_c(), class_d()]; + let classes = classes_of(&cs); + let stmts = vec![ + let_arr(1, "rows"), + let_c(2, "row"), + push(1, Expr::LocalGet(2)), + push(1, new_of("D")), + ]; + assert!(!promote(&stmts, &classes).contains_key(&2)); +} + +/// **E2.** A value pushed into two arrays belongs to two element groups, and +/// the one-group soundness argument ("every reference to this object is a +/// member of this group") no longer holds. +/// +/// Sabotage: delete the `push_value_counts` check and this fails. +#[test] +fn a_local_pushed_into_two_arrays_is_not_exempt() { + let c = class_c(); + let cs = [c]; + let classes = classes_of(&cs); + let stmts = vec![ + let_arr(1, "a"), + let_arr(3, "b"), + let_c(2, "row"), + push(1, Expr::LocalGet(2)), + push(3, Expr::LocalGet(2)), + ]; + // Asserted on the FACTS, not on promotion: without the guard, `pushed` + // keeps whichever group was recorded last, so exactly one of these two + // would come back true — and a `promote()` assertion would pass or fail + // on `HashMap` iteration order, i.e. be vacuous half the time. + let facts = elements(&stmts, &classes); + assert!( + !facts.push_is_contained(2, 1) && !facts.push_is_contained(2, 3), + "a value in two element groups must be exempt in neither" + ); + assert!(!promote(&stmts, &classes).contains_key(&2)); +} + +/// **E1.** A non-empty array literal can carry elisions, whose slots read +/// back as `undefined`. +/// +/// Sabotage: relax `items.is_empty()` and this fails. +#[test] +fn a_non_empty_array_literal_is_not_a_seed() { + let c = class_c(); + let cs = [c]; + let classes = classes_of(&cs); + let stmts = vec![ + Stmt::Let { + id: 1, + name: "rows".to_string(), + ty: Type::Array(Box::new(Type::Named("C".to_string()))), + mutable: false, + init: Some(Expr::Array(vec![Expr::Undefined])), + }, + let_c(2, "row"), + push(1, Expr::LocalGet(2)), + ]; + assert!(!promote(&stmts, &classes).contains_key(&2)); +} + +/// **E1.** A `let` array can be rebound to a different array between the +/// push and a read. +/// +/// Sabotage: relax the `mutable: false` requirement and this fails. +#[test] +fn a_reassignable_array_binding_is_not_a_seed() { + let c = class_c(); + let cs = [c]; + let classes = classes_of(&cs); + let stmts = vec![ + Stmt::Let { + id: 1, + name: "rows".to_string(), + ty: Type::Array(Box::new(Type::Named("C".to_string()))), + mutable: true, + init: Some(Expr::Array(Vec::new())), + }, + let_c(2, "row"), + push(1, Expr::LocalGet(2)), + ]; + assert!(!promote(&stmts, &classes).contains_key(&2)); +} + +// ── The reader half ──────────────────────────────────────────────────────── + +/// `for (let i = 0; i < a.length; i++) { const r = a[i]; r.x; }` — the read +/// form `for…of` also desugars to. `r` becomes a rule-1 seed. +/// +/// Sabotage: delete the `element_seeded` arm in `ptr_shape.rs`'s `Stmt::Let`, +/// or the `ReadSite` push in `ArrayWalk`, and this fails. +#[test] +fn bounded_element_read_is_provenance() { + let c = class_c(); + let cs = [c]; + let classes = classes_of(&cs); + let stmts = vec![ + let_arr(1, "rows"), + push(1, new_c()), + bounded_loop(5, 1, vec![let_elem(6, "r", 1, 5), read_x(6)]), + ]; + let promoted = promote(&stmts, &classes); + let fact = promoted + .get(&6) + .expect("an in-bounds `a[i]` binding must be a Ptr candidate"); + assert_eq!(fact.class_name, "C"); + assert!( + fact.numeric_fields.is_empty(), + "an element read is aliased through the array by construction" + ); +} + +/// **E5 — the conjunct that separates this pass from a wrong one.** With an +/// unbounded index, `a[i]` can be `undefined`, and a guard-free fixed-offset +/// load would mask a NaN-boxed `undefined` into a wild pointer. +/// +/// Sabotage: accept any `IndexGet` in the `Stmt::Let` arm (drop the +/// `self.bounded` membership test) and this fails. +#[test] +fn unbounded_element_read_is_not_provenance() { + let c = class_c(); + let cs = [c]; + let classes = classes_of(&cs); + let stmts = vec![ + let_arr(1, "rows"), + let_c(2, "row"), + push(1, Expr::LocalGet(2)), + // `const j = 99; const r = rows[j];` — a local index, so the shape of + // the read is identical to the licensed one and the ONLY thing + // denying it is the absence of a bounding loop. + Stmt::Let { + id: 5, + name: "j".to_string(), + ty: Type::Number, + mutable: false, + init: Some(Expr::Number(99.0)), + }, + let_elem(6, "r", 1, 5), + // Rule 2 never walks `r`, so this undeclared write is invisible — + // which is exactly why the unlicensed read has to void the array. + Stmt::Expr(Expr::PropertySet { + object: Box::new(Expr::LocalGet(6)), + property: "extra".to_string(), + value: Box::new(Expr::Number(1.0)), + }), + ]; + let promoted = promote(&stmts, &classes); + assert!( + !promoted.contains_key(&6), + "an out-of-bounds read yields `undefined`, not an instance of C" + ); + assert!( + !promoted.contains_key(&2), + "and the unlicensed read voids the array, so the producer goes too" + ); +} + +/// **E5.** A loop bounded by something OTHER than this array's length proves +/// nothing about this array's indices. +/// +/// Sabotage: stop comparing the condition's `.length` receiver against the +/// read's array root and this fails. +#[test] +fn a_loop_bounded_by_another_length_does_not_license_the_read() { + let c = class_c(); + let cs = [c]; + let classes = classes_of(&cs); + let stmts = vec![ + let_arr(1, "rows"), + let_arr(2, "other"), + push(1, new_c()), + push(2, new_c()), + bounded_loop(5, 2, vec![let_elem(6, "r", 1, 5), read_x(6)]), + ]; + assert!(!promote(&stmts, &classes).contains_key(&6)); +} + +/// **E5.** `i < notAnArray.length` bounds `i` by something that is not this +/// array at all. Exactly one array exists in this fixture, so the guard under +/// test — resolving the condition's `.length` receiver to the read's array +/// ROOT — is the only thing that can deny it. +/// +/// Sabotage: replace `self.root_of(*a)?` in `bounded_induction` with any +/// tracked root and this fails. +#[test] +fn a_loop_bounded_by_a_non_array_length_does_not_license_the_read() { + let c = class_c(); + let cs = [c]; + let classes = classes_of(&cs); + let stmts = vec![ + let_arr(1, "rows"), + push(1, new_c()), + Stmt::Let { + id: 8, + name: "other".to_string(), + ty: Type::Any, + mutable: false, + init: Some(Expr::String("xyz".to_string())), + }, + bounded_loop_cond( + 5, + Expr::Compare { + op: CompareOp::Lt, + left: Box::new(Expr::LocalGet(5)), + right: Box::new(Expr::PropertyGet { + object: Box::new(Expr::LocalGet(8)), + property: "length".to_string(), + byte_offset: 0, + }), + }, + vec![let_elem(6, "r", 1, 5), read_x(6)], + ), + ]; + assert!(!promote(&stmts, &classes).contains_key(&6)); +} + +/// **E5.** A constant bound is not the array's length — the array may be +/// shorter. +/// +/// Sabotage: accept any `Compare { Lt }` condition and this fails. +#[test] +fn a_constant_bound_does_not_license_the_read() { + let c = class_c(); + let cs = [c]; + let classes = classes_of(&cs); + let stmts = vec![ + let_arr(1, "rows"), + push(1, new_c()), + bounded_loop_cond( + 5, + Expr::Compare { + op: CompareOp::Lt, + left: Box::new(Expr::LocalGet(5)), + right: Box::new(Expr::Number(10.0)), + }, + vec![let_elem(6, "r", 1, 5), read_x(6)], + ), + ]; + assert!(!promote(&stmts, &classes).contains_key(&6)); +} + +/// **E5.** An induction variable reassigned in the body is no longer bounded +/// by the loop condition at the read. +/// +/// Sabotage: delete the `idx_writes == 2` check and this fails. +#[test] +fn an_index_reassigned_in_the_body_does_not_license_the_read() { + let c = class_c(); + let cs = [c]; + let classes = classes_of(&cs); + let stmts = vec![ + let_arr(1, "rows"), + push(1, new_c()), + bounded_loop( + 5, + 1, + vec![ + Stmt::Expr(Expr::LocalSet(5, Box::new(Expr::Number(999.0)))), + let_elem(6, "r", 1, 5), + read_x(6), + ], + ), + ]; + assert!(!promote(&stmts, &classes).contains_key(&6)); +} + +/// **Group integrity.** One member adding an undeclared property reshapes the +/// objects every other member reads, so the whole group is dropped — not just +/// the offender. +/// +/// Sabotage: delete the group-integrity filter at the end of +/// `collect_shape_proven_ptr_locals` and this fails: the producer keeps a +/// promotion whose objects the reader has just reshaped. +#[test] +fn one_member_failing_containment_voids_the_group() { + let c = class_c(); + let cs = [c]; + let classes = classes_of(&cs); + let stmts = vec![ + let_arr(1, "rows"), + let_c(2, "row"), + push(1, Expr::LocalGet(2)), + bounded_loop( + 5, + 1, + vec![ + let_elem(6, "r", 1, 5), + // `r.extra = 1` — not a declared field of C, so this is a + // shape transition on an object `row` also references. + Stmt::Expr(Expr::PropertySet { + object: Box::new(Expr::LocalGet(6)), + property: "extra".to_string(), + value: Box::new(Expr::Number(1.0)), + }), + ], + ), + ]; + let promoted = promote(&stmts, &classes); + assert!( + !promoted.contains_key(&6), + "the offending member is denied by rule 2" + ); + assert!( + !promoted.contains_key(&2), + "and so is every sibling — the objects it reads have been reshaped" + ); +} + +/// **Group integrity, the other direction.** A group whose members are all +/// contained keeps every one of them, so the filter is not simply "drop +/// everything". +#[test] +fn a_clean_group_keeps_every_member() { + let c = class_c(); + let cs = [c]; + let classes = classes_of(&cs); + let stmts = vec![ + let_arr(1, "rows"), + let_c(2, "row"), + store_x(2), + push(1, Expr::LocalGet(2)), + bounded_loop(5, 1, vec![let_elem(6, "r", 1, 5), read_x(6), store_x(6)]), + ]; + let promoted = promote(&stmts, &classes); + assert!(promoted.contains_key(&2), "producer"); + assert!(promoted.contains_key(&6), "reader"); +} + +/// **E3, the ELEMENT half.** `f(A[i])` hands an element to an opaque callee, +/// which can add a property to an object that a licensed `const s = A[k]` +/// reads guard-free. A read cannot transition a shape, but the REFERENCE it +/// produces can be used to. +/// +/// Sabotage: make `ArrayWalk`'s `Expr::IndexGet` arm skip `self.disq` and this +/// fails — with a wrong answer under an aliasing mutation, not a compile +/// error. +#[test] +fn an_element_passed_to_a_callee_denies_the_array() { + let c = class_c(); + let cs = [c]; + let classes = classes_of(&cs); + let stmts = vec![ + let_arr(1, "rows"), + let_c(2, "row"), + push(1, Expr::LocalGet(2)), + bounded_loop( + 5, + 1, + vec![ + let_elem(6, "r", 1, 5), + read_x(6), + // `f(rows[i])` — the element escapes, the array does not. + Stmt::Expr(Expr::Call { + callee: Box::new(Expr::FuncRef(9)), + args: vec![Expr::IndexGet { + object: Box::new(Expr::LocalGet(1)), + index: Box::new(Expr::LocalGet(5)), + }], + type_args: Vec::new(), + byte_offset: 0, + }), + ], + ), + ]; + let promoted = promote(&stmts, &classes); + assert!( + !promoted.contains_key(&6) && !promoted.contains_key(&2), + "an element handed to an opaque callee must void the whole group" + ); +} + +/// **E3, the ELEMENT half, unlicensed binding.** `const r = A[0]` binds an +/// element to a local at a site E5 does not license, so rule 2 never walks +/// `r` — and `r.extra = 1` would reshape an object the licensed members read. +/// +/// Sabotage: restore the old "an unbounded index read does not disqualify the +/// array" early return in `walk_stmt` and this fails. +#[test] +fn an_unlicensed_element_binding_denies_the_array() { + let c = class_c(); + let cs = [c]; + let classes = classes_of(&cs); + let stmts = vec![ + let_arr(1, "rows"), + let_c(2, "row"), + push(1, Expr::LocalGet(2)), + // `const stray = rows[0];` — literal index, no bounding loop. + Stmt::Let { + id: 7, + name: "stray".to_string(), + ty: Type::Named("C".to_string()), + mutable: false, + init: Some(Expr::IndexGet { + object: Box::new(Expr::LocalGet(1)), + index: Box::new(Expr::Number(0.0)), + }), + }, + Stmt::Expr(Expr::PropertySet { + object: Box::new(Expr::LocalGet(7)), + property: "extra".to_string(), + value: Box::new(Expr::Number(1.0)), + }), + bounded_loop(5, 1, vec![let_elem(6, "r", 1, 5), read_x(6)]), + ]; + let promoted = promote(&stmts, &classes); + assert!( + !promoted.contains_key(&6) && !promoted.contains_key(&2), + "an element bound outside a licensed site must void the whole group" + ); +} + +/// The array ALIAS path, directly. `for (const r of A)` does not desugar to +/// `A[__idx]` — it binds `const __arr_N = A` first and indexes THAT +/// (`lower/stmt_loops.rs`), so every claim this pass makes about the iterator +/// form runs through the alias edge. The gap test covers it end-to-end; this +/// covers it where a break would be attributable. +/// +/// Sabotage: drop the `collect_alias_edges` fixpoint in +/// `collect_element_shape_facts` and this fails while every direct-index test +/// stays green. +#[test] +fn reads_through_an_array_alias_are_licensed() { + let c = class_c(); + let cs = [c]; + let classes = classes_of(&cs); + let stmts = vec![ + let_arr(1, "rows"), + let_c(2, "row"), + push(1, Expr::LocalGet(2)), + // `const __arr = rows;` — the for-of desugar's alias binding. + Stmt::Let { + id: 3, + name: "__arr_3".to_string(), + ty: Type::Array(Box::new(Type::Named("C".to_string()))), + mutable: false, + init: Some(Expr::LocalGet(1)), + }, + // `for (let i = 0; i < __arr.length; i++) { const r = __arr[i]; … }` + bounded_loop(5, 3, vec![let_elem(6, "r", 3, 5), read_x(6)]), + ]; + let promoted = promote(&stmts, &classes); + assert!( + promoted.contains_key(&6), + "an element read through an array alias must be licensed — this is the \ + `for…of` form" + ); + assert!(promoted.contains_key(&2), "and the producer with it"); +} + +// ── CodeRabbit review reproducers (PR #7149) ─────────────────────────────── + +/// **CodeRabbit 🔴 #1** (`ptr_shape_elements.rs:710`, review of `816a5a3`): +/// a property store through `A[i]` was admitted for ANY property name, so +/// `rows[i].extra = 1` added an own property while licensed reads kept doing +/// guard-free fixed-offset loads on the transitioned object. +/// +/// The reviewer's reproducer, verbatim in HIR form. The hazard is real; the +/// mechanism it named (`element_access_is_admissible`) no longer exists — +/// the same commit the review was posted against deleted it, so a +/// `PropertySet` whose receiver is an `IndexGet` now reaches the +/// `Expr::IndexGet` arm and disqualifies the array outright. This test is the +/// standing proof of that, and it is what stops a future "admit declared-field +/// element stores" widening from re-introducing the hole silently. +#[test] +fn a_property_store_through_an_element_denies_the_array() { + let c = class_c(); + let cs = [c]; + let classes = classes_of(&cs); + let stmts = vec![ + let_arr(1, "rows"), + push(1, new_c()), + // `for (…) rows[i].extra = 1;` + bounded_loop( + 5, + 1, + vec![Stmt::Expr(Expr::PropertySet { + object: Box::new(Expr::IndexGet { + object: Box::new(Expr::LocalGet(1)), + index: Box::new(Expr::LocalGet(5)), + }), + property: "extra".to_string(), + value: Box::new(Expr::Number(1.0)), + })], + ), + // `for (…) { const r = rows[i]; use(r.x); }` + bounded_loop(8, 1, vec![let_elem(6, "r", 1, 8), read_x(6)]), + ]; + assert!( + !promote(&stmts, &classes).contains_key(&6), + "an undeclared-property store through an element must void the array" + ); + assert!( + elements(&stmts, &classes).is_empty(), + "and no element fact may survive it" + ); +} + +/// Same hazard through `A[i].f++`, which is a different HIR node +/// (`PropertyUpdate`) and therefore a different arm. +#[test] +fn a_property_update_through_an_element_denies_the_array() { + let c = class_c(); + let cs = [c]; + let classes = classes_of(&cs); + let stmts = vec![ + let_arr(1, "rows"), + push(1, new_c()), + bounded_loop( + 5, + 1, + vec![Stmt::Expr(Expr::PropertyUpdate { + object: Box::new(Expr::IndexGet { + object: Box::new(Expr::LocalGet(1)), + index: Box::new(Expr::LocalGet(5)), + }), + property: "extra".to_string(), + op: perry_hir::BinaryOp::Add, + prefix: false, + })], + ), + bounded_loop(8, 1, vec![let_elem(6, "r", 1, 8), read_x(6)]), + ]; + assert!(elements(&stmts, &classes).is_empty()); +} + +/// **CodeRabbit 🔴 #2** (`ptr_shape.rs:562`): group integrity removed only the +/// ids `group_members()` reports, but the insert loop above it gives every +/// ALIAS of a promoted root the same `PtrShapeLocal` fact. An alias holds the +/// same object, so it kept a guard-free proof of a shape a sibling had just +/// transitioned. +/// +/// The reviewer's reproducer. Sabotage: drop the alias closure from the +/// group-integrity filter in `collect_shape_proven_ptr_locals`. +#[test] +fn group_integrity_drops_the_aliases_of_a_dropped_member() { + let c = class_c(); + let cs = [c]; + let classes = classes_of(&cs); + let stmts = vec![ + let_arr(1, "rows"), + let_c(2, "row"), + // `const a = row;` — an alias, which the ptr_shape alias pre-pass + // tracks and hands the same fact to. + Stmt::Let { + id: 3, + name: "a".to_string(), + ty: Type::Named("C".to_string()), + mutable: false, + init: Some(Expr::LocalGet(2)), + }, + push(1, Expr::LocalGet(2)), + bounded_loop( + 5, + 1, + vec![ + let_elem(6, "r", 1, 5), + // Fails rule 2: an undeclared property on a group member. + Stmt::Expr(Expr::PropertySet { + object: Box::new(Expr::LocalGet(6)), + property: "extra".to_string(), + value: Box::new(Expr::Number(1.0)), + }), + ], + ), + read_x(3), + ]; + let promoted = promote(&stmts, &classes); + assert!(!promoted.contains_key(&6), "the offender"); + assert!(!promoted.contains_key(&2), "the group member"); + assert!( + !promoted.contains_key(&3), + "and its ALIAS, which holds the same object and would otherwise keep a guard-free proof of a shape that has been transitioned" + ); +} + +/// **CodeRabbit 🟠 #3** (`ptr_shape_elements.rs:727`): pushing a tracked array +/// into another array made the OUTER array `PushValue::Other` (disqualified) +/// but never disqualified the INNER one, because the arm skips `walk_expr` for +/// `Expr::LocalGet` values. The inner array stays reachable and mutable +/// through the outer one, and `outer[0][0] = new Other()` is an `IndexSet` on +/// an `IndexGet` — an expression this walk does not track. +/// +/// The reviewer's reproducer. Sabotage: delete the `disq(*v)` in the +/// `Expr::ArrayPush` arm. +#[test] +fn an_array_pushed_into_another_array_is_disqualified() { + let cs = [class_c(), class_d()]; + let classes = classes_of(&cs); + let stmts = vec![ + let_arr(1, "inner"), + push(1, new_c()), + let_arr_ty( + 2, + "outer", + Type::Array(Box::new(Type::Array(Box::new(Type::Named( + "C".to_string(), + ))))), + ), + // `outer.push(inner)` + push(2, Expr::LocalGet(1)), + // `outer[0][0] = new D();` — reaches `inner`'s element through an + // expression neither walk tracks. + Stmt::Expr(Expr::IndexSet { + object: Box::new(Expr::IndexGet { + object: Box::new(Expr::LocalGet(2)), + index: Box::new(Expr::Number(0.0)), + }), + index: Box::new(Expr::Number(0.0)), + value: Box::new(new_of("D")), + }), + bounded_loop(5, 1, vec![let_elem(6, "r", 1, 5), read_x(6)]), + ]; + let facts = elements(&stmts, &classes); + assert!( + facts.element_read_class(6).is_none(), + "an array stored as an element of another array is aliased through it" + ); + assert!(!promote(&stmts, &classes).contains_key(&6)); +} + +/// **CodeRabbit 🟡 #6** (`is_empty` covered only `arrays`). The other three +/// maps are kept consistent with `arrays` by construction today — `pushed` is +/// `retain`ed against it, `element_reads` only inserts for a proven root, and +/// an empty `arrays` returns `default()` — but that is an invariant no type +/// enforces, and `is_empty()` gates every consumer. Assert it directly so a +/// future edit that populates one map without the other is caught here rather +/// than by a wrong answer. +#[test] +fn is_empty_covers_every_fact_map() { + let c = class_c(); + let cs = [c]; + let classes = classes_of(&cs); + // A region with an array that fails E3 (call-argument escape): every map + // must come back empty together. + let denied = vec![ + let_arr(1, "rows"), + let_c(2, "row"), + push(1, Expr::LocalGet(2)), + Stmt::Expr(Expr::Call { + callee: Box::new(Expr::FuncRef(9)), + args: vec![Expr::LocalGet(1)], + type_args: Vec::new(), + byte_offset: 0, + }), + ]; + let facts = elements(&denied, &classes); + assert!(facts.is_empty()); + assert!( + facts.debug_all_maps_empty(), + "is_empty must imply ALL maps empty" + ); + + // And the converse: a proven region is not `is_empty`. + let proven = vec![ + let_arr(1, "rows"), + let_c(2, "row"), + push(1, Expr::LocalGet(2)), + ]; + let facts = elements(&proven, &classes); + assert!(!facts.is_empty()); + assert!(!facts.debug_all_maps_empty()); +} + +// ── GC rooting obligations ───────────────────────────────────────────────── + +/// **GC (#7019).** `collect_pointer_typed_locals` infers an `IndexGet`'s type +/// from the object's: a local declared `number[]` yields `Number`, which is +/// "definitely not a pointer", so the binding gets NO shadow slot. Perry does +/// not validate annotations, so a `number[]` that this pass proved holds `C` +/// instances would leave a promoted element in an unrooted alloca and an +/// evacuating minor would move the object without rewriting it. +/// +/// Sabotage: make `array_type_keeps_element_slot` return `true` and this +/// fails — with a silent wrong answer under `PERRY_GC_FORCE_EVACUATE`, not a +/// compile error. +#[test] +fn a_number_typed_array_annotation_denies_the_fact() { + let c = class_c(); + let cs = [c]; + let classes = classes_of(&cs); + let stmts = vec![ + let_arr_ty(1, "rows", Type::Array(Box::new(Type::Number))), + push(1, new_c()), + bounded_loop(5, 1, vec![let_elem(6, "r", 1, 5), read_x(6)]), + ]; + assert!(!promote(&stmts, &classes).contains_key(&6)); +} + +/// **GC (#7019), the binding's own annotation.** Same hazard one level down: +/// a `const r: number = a[i]` binding is dropped from the shadow stack by its +/// declared type alone. +/// +/// Sabotage: delete the `elem_let_ty` / `is_definitely_non_pointer_type` +/// check on the read site and this fails. +#[test] +fn a_number_typed_element_binding_denies_the_fact() { + let c = class_c(); + let cs = [c]; + let classes = classes_of(&cs); + let stmts = vec![ + let_arr(1, "rows"), + push(1, new_c()), + bounded_loop( + 5, + 1, + vec![let_elem_ty(6, "r", 1, 5, Type::Number), read_x(6)], + ), + ]; + assert!(!promote(&stmts, &classes).contains_key(&6)); +} + +// ── Rules 4 and 5 still apply ────────────────────────────────────────────── + +/// **E4/rule 5.** One `Object.defineProperty` anywhere in the module still +/// kills every `Ptr` promotion, elements included. +/// +/// Sabotage: drop the `has_shape_barrier_sites` bail in +/// `collect_element_shape_facts` and this test still passes (ptr_shape bails +/// too) — which is why the assertion is on the ELEMENT facts directly. +#[test] +fn the_module_barrier_still_denies_element_facts() { + let c = class_c(); + let cs = [c]; + let classes = classes_of(&cs); + let barrier = Function { + id: 3, + name: "b".to_string(), + type_params: Vec::new(), + params: Vec::new(), + return_type: Type::Any, + body: vec![Stmt::Expr(Expr::ObjectDefineProperty( + Box::new(Expr::LocalGet(90)), + Box::new(Expr::String("k".to_string())), + Box::new(Expr::Undefined), + ))], + is_async: false, + is_generator: false, + is_strict: false, + is_exported: false, + captures: Vec::new(), + decorators: Vec::new(), + was_plain_async: false, + was_unrolled: false, + }; + let facts = facts_for(&classes, vec![barrier]); + assert!(facts.has_shape_barrier_sites(), "fixture must be a barrier"); + let stmts = vec![ + let_arr(1, "rows"), + let_c(2, "row"), + push(1, Expr::LocalGet(2)), + ]; + assert!(elements_with(&stmts, &classes, &facts).is_empty()); +} + +/// **E4.** An element class Phase 3b would not admit on its own is not +/// admitted through an array either. +/// +/// Sabotage: delete the `chain_admissible` call and this fails. +#[test] +fn an_inadmissible_element_class_denies_the_array() { + let mut c = class_c(); + // NOT "x"/"y": those are declared fields, and a name that is both would + // let this pass on field/method ambiguity rather than on the accessor + // rule it is written for. + c.getters = vec![( + "derived".to_string(), + perry_hir::Function { + id: 50, + name: "derived".to_string(), + type_params: Vec::new(), + params: Vec::new(), + return_type: Type::Any, + body: Vec::new(), + is_async: false, + is_generator: false, + is_strict: false, + is_exported: false, + captures: Vec::new(), + decorators: Vec::new(), + was_plain_async: false, + was_unrolled: false, + }, + )]; + let cs = [c]; + let classes = classes_of(&cs); + let stmts = vec![ + let_arr(1, "rows"), + let_c(2, "row"), + push(1, Expr::LocalGet(2)), + ]; + assert!(elements(&stmts, &classes).is_empty()); +} + +/// The gate is a bisection knob for the whole of Phase 3b, and it must take +/// this analysis with it. +/// +/// Sabotage: delete the `ptr_shape_locals_enabled()` bail and this fails. +#[test] +fn the_env_gate_disables_element_facts() { + // `ptr_shape_locals_enabled` caches in a `OnceLock`, so this asserts the + // call is PRESENT rather than flipping the env (which a parallel test + // would race on): with the gate on, the same fixture is non-empty. + let c = class_c(); + let cs = [c]; + let classes = classes_of(&cs); + let stmts = vec![ + let_arr(1, "rows"), + let_c(2, "row"), + push(1, Expr::LocalGet(2)), + ]; + assert_eq!( + elements(&stmts, &classes).is_empty(), + !ptr_shape_locals_enabled(), + "element facts must track PERRY_PTR_SHAPE_LOCALS exactly" + ); +} diff --git a/crates/perry-codegen/src/collectors/ptr_shape_opt_report_tests.rs b/crates/perry-codegen/src/collectors/ptr_shape_opt_report_tests.rs index fcaef635c7..fcfb1d6350 100644 --- a/crates/perry-codegen/src/collectors/ptr_shape_opt_report_tests.rs +++ b/crates/perry-codegen/src/collectors/ptr_shape_opt_report_tests.rs @@ -103,6 +103,7 @@ fn run(stmts: &[Stmt], classes: &HashMap) -> HashMap {} _ => return None, } + // #7034 §3: a promoted local is normally unaliased, which is + // exactly the freshness this fact needs. An ELEMENT-GROUP member + // is the one promoted local that is aliased on purpose — the + // array holds it too. That array is region-local and cannot + // outlive the frame here (returning it instead would make the + // returns disagree, and every other escape disqualifies it), but + // the caller-side proof should not rest on a two-step argument + // when refusing costs nothing: no return-shape fact for a value + // that is also in an array. + if elements.is_group_member(*id) { + return None; + } } } Some(class_name.to_string()) diff --git a/crates/perry-codegen/src/collectors/ptr_shape_returns_tests.rs b/crates/perry-codegen/src/collectors/ptr_shape_returns_tests.rs index 02b0d889f2..b027eef098 100644 --- a/crates/perry-codegen/src/collectors/ptr_shape_returns_tests.rs +++ b/crates/perry-codegen/src/collectors/ptr_shape_returns_tests.rs @@ -141,6 +141,13 @@ fn promote( classes, facts, &HashSet::new(), + &super::super::ptr_shape_elements::collect_element_shape_facts( + stmts, + &HashSet::new(), + &HashMap::new(), + classes, + facts, + ), ) } diff --git a/scripts/compiler_output_harness/repsel_census.py b/scripts/compiler_output_harness/repsel_census.py index 61875730e1..7799bc8913 100644 --- a/scripts/compiler_output_harness/repsel_census.py +++ b/scripts/compiler_output_harness/repsel_census.py @@ -192,6 +192,13 @@ # Its value is the SITE coverage it provides (checked separately); the # count floor here just keeps it honest as a promotion too. "fixture_ptr_shape_sites": {"ptr-shape": 1, "ptr-shape-consumed": 1}, + # #7034 §3 (array-element escape). The other two `ptr_shape` fixtures never + # touch an array, and the 18 real workloads promote zero element locals, so + # without this entry `collectors/ptr_shape_elements.rs` could stop issuing + # facts entirely and every counter in the census would be unchanged -- + # CLAUDE.md failure mode 4, exactly. Three promotions: the pushed producer, + # the `rows[i]` binding, and the `for…of` binding. + "fixture_ptr_shape_elements": {"ptr-shape": 3, "ptr-shape-consumed": 3}, "fixture_ptr_numarray": {"ptr-numarray": 1}, "fixture_canonical_slots": { "canonical-i32": 1, diff --git a/test-files/test_gap_repsel_ptr_shape_elements.ts b/test-files/test_gap_repsel_ptr_shape_elements.ts new file mode 100644 index 0000000000..f3dc32b20d --- /dev/null +++ b/test-files/test_gap_repsel_ptr_shape_elements.ts @@ -0,0 +1,195 @@ +// Representation-selection Phase 3b, #7034 §3: array-element shape facts +// (RFC docs/representation-selection-rfc.md §5.5-§5.7, +// collectors/ptr_shape_elements.rs). +// +// Behavioural guard for the escape this opens: `rows.push(row)` no longer +// disqualifies `row`, and `const r = rows[i]` under an `i < rows.length` loop +// is rule-1 provenance. Every case must be BYTE-EXACT against Node — an +// optimization that changes an observable answer is a miscompile, and every +// case below is one the pre-#7034-§3 compiler left on the guarded protocol, +// so a divergence here is attributable. +// +// The promotions this file is *about* are asserted structurally elsewhere +// (`benchmarks/repsel_census/fixtures/fixture_ptr_shape_elements.ts` carries +// the census floor, and `perry --opt-report` lists the locals). A +// green run with zero promotions would be a vacuous pass (#7024/#7025). +// +// Covered: +// 1. producer side: a contained local whose only escape is the push, +// 2. reader side, indexed: `const s = rows[i]` under `i < rows.length`, +// 3. reader side, iterator: `for (const r of rows)`, which desugars to (2), +// 4. GC movement between the element read and the field reads — the +// tagged-at-rest slot must be re-derived and rewritten (RFC §5.6), +// 5. the numeric-field STAND-DOWN: a NaN / Infinity / -0 / string stored +// through one group member and read back through another, +// 6. arrays that must NOT be proven: one that escapes to a callee, one +// mutated with `pop`, one with mixed element classes, one read out of +// bounds — each still has to produce Node's answer. +// +// NOT covered, deliberately: `short[5].id` where the binding is annotated +// `Row`. Node throws `TypeError: Cannot read properties of undefined`; Perry +// prints `undefined` — on `main`, at the base commit, and with +// `PERRY_PTR_SHAPE_LOCALS=0`, so it is an unrelated pre-existing gap and NOT +// this pass's OOB hazard (which E5's in-bounds conjunct is what rules out). +// Asserting `typeof` instead keeps the out-of-bounds read exercised without +// making this file red for someone else's bug. + +class Row { + id: number; + bucket: string; + weight: number; + score: number; + constructor(id: number, bucket: string, weight: number) { + this.id = id; + this.bucket = bucket; + this.weight = weight; + this.score = 0; + } + rescore(f: number): number { + this.score = this.weight * f + (this.id % 7); + return this.score; + } +} + +const BUCKETS = ["alpha", "beta", "gamma", "delta"]; + +// 1 + 2 + 3: build, then consume both ways in the same function. +function buildAndFold(n: number): string { + const rows: Row[] = []; + for (let i = 0; i < n; i++) { + const row = new Row(i, BUCKETS[i % 4], (i % 97) * 0.5); + row.score = row.weight + 1; + rows.push(row); + } + let indexed = 0; + for (let i = 0; i < rows.length; i++) { + const s = rows[i]; + s.score = s.score + s.weight; + indexed = indexed + s.score + s.id; + } + let iterated = 0; + for (const r of rows) { + iterated = iterated + r.rescore(1.5) + r.weight; + } + return indexed.toFixed(4) + "/" + iterated.toFixed(4) + "/" + rows.length; +} + +// 4: force collections between the element read and its field reads. Every +// access must re-derive the pointer from the shadow-bound slot; a cached raw +// pointer is a stale-address read after an evacuating minor. +function churnBetweenAccesses(n: number): string { + const rows: Row[] = []; + for (let i = 0; i < n; i++) { + rows.push(new Row(i, BUCKETS[i % 4], i)); + } + let acc = 0; + for (let i = 0; i < rows.length; i++) { + const s = rows[i]; + acc = acc + s.id; + // Allocation between two reads of the SAME element local. + const junk: Row[] = []; + for (let k = 0; k < 200; k++) { + junk.push(new Row(k, "j", k)); + } + acc = acc + junk.length * 0 + s.weight; + acc = acc + s.score; + } + return acc.toFixed(4); +} + +// 5: the numeric stand-down. `weird` is written through the indexed member +// and read back through the `for…of` member; the group claims no numeric +// fields, so the read must go through the plain-finite check, not a bare +// `load double` claiming JsNumber. +function nonFiniteThroughTheGroup(): string { + const rows: Row[] = []; + for (let i = 0; i < 4; i++) { + rows.push(new Row(i, "b", i)); + } + for (let i = 0; i < rows.length; i++) { + const s = rows[i]; + if (i === 0) { + s.score = NaN; + } else if (i === 1) { + s.score = Infinity; + } else if (i === 2) { + s.score = -0; + } else { + s.weight = -Infinity; + } + } + let out = ""; + for (const r of rows) { + out = out + String(r.score) + "|" + String(1 / r.score) + "|" + String(r.weight) + ";"; + } + return out; +} + +// 6a: the array escapes to an opaque callee, which reshapes an element. The +// answer must still be Node's, which means the proof must NOT have fired. +function reshape(list: Row[]): number { + let t = 0; + for (let i = 0; i < list.length; i++) { + const any = list[i] as unknown as Record; + any["extra"] = i + 1; + t = t + any["extra"]; + } + return t; +} + +function escapingArray(): string { + const rows: Row[] = []; + for (let i = 0; i < 3; i++) { + rows.push(new Row(i, "e", i)); + } + const t = reshape(rows); + let seen = ""; + for (let i = 0; i < rows.length; i++) { + const s = rows[i]; + seen = seen + s.id + ":" + JSON.stringify(s) + ";"; + } + return t + "/" + seen; +} + +// 6b: `pop` makes the array non-dense; 6c: two classes in one array; +// 6d: an out-of-bounds read yields `undefined`, never an instance of Row. +class Other { + tag: string; + constructor(tag: string) { + this.tag = tag; + } +} + +function mutatedAndMixed(): string { + const popped: Row[] = []; + for (let i = 0; i < 4; i++) { + popped.push(new Row(i, "p", i)); + } + popped.pop(); + let a = 0; + for (let i = 0; i < popped.length; i++) { + a = a + popped[i].id; + } + + const mixed: object[] = []; + mixed.push(new Row(1, "m", 1)); + mixed.push(new Other("o")); + let b = ""; + for (let i = 0; i < mixed.length; i++) { + b = b + (mixed[i] as { constructor: { name: string } }).constructor.name + ","; + } + + const short: Row[] = []; + short.push(new Row(9, "s", 9)); + // Out of bounds: `undefined`, never an instance of Row. E5 is what stops + // this from reaching a guard-free fixed-offset load. + const missing = short[5]; + const oob = typeof missing; + return a + "/" + b + "/" + oob; +} + +console.log("build: " + buildAndFold(500)); +console.log("churn: " + churnBetweenAccesses(60)); +console.log("nonfinite: " + nonFiniteThroughTheGroup()); +console.log("escaping: " + escapingArray()); +console.log("mutated: " + mutatedAndMixed()); diff --git a/test-parity/gc_repsel_corpus.txt b/test-parity/gc_repsel_corpus.txt index a9b205bb3d..3b754996fb 100644 --- a/test-parity/gc_repsel_corpus.txt +++ b/test-parity/gc_repsel_corpus.txt @@ -48,6 +48,15 @@ test_gap_repsel_ptr_shape_barriers # is exactly the claim this phase's GC contract makes. test_gap_repsel_return_shape +# --- Phase 3b / #7034 §3: array-element shape facts ------------------------- +# `rows.push(row)` no longer disqualifies `row`, and `const r = rows[i]` under +# an `i < rows.length` loop is rule-1 provenance. Every element local is an +# ordinary shadow-bound NaN-boxed slot re-derived per access, so `churn` +# deliberately allocates 200 objects BETWEEN two field reads of the same +# element local: the evacuating arms are the ones that can catch a raw pointer +# cached across that safepoint. +test_gap_repsel_ptr_shape_elements + # --- Phase 4a / 4a.3: Ptr numeric arrays (#6915, #6916) ----------- test_gap_repsel_p4a_holes_axis test_gap_repsel_p4a_inline_tiers