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