From 844aac847b2bd65488ad10180fe37f30ba8b9c9c Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Ralph=20K=C3=BCpper?= Date: Mon, 31 Aug 2026 10:47:42 +0200 Subject: [PATCH 1/3] codegen: receiver-region model + equivalence lint (#9254 phase 1) Codegen carries sixteen receiver-keyed fact tables on `FnCtx`. Each answers the same two questions -- what do we know about this receiver, and how long may we believe it -- and each answers the second one differently: retain(|f| f.scope_id != id) bounded_index_pairs, packed_f64_loop_facts, masked_window_array_facts, int_range_facts, ... insert/remove, no id cached_lengths, packed_receiver_*, versioned_indexed_loop_facts field downgraded in place buffer_view_slots reloaded at the safepoint packed_receiver_* nothing buffer_data_slots, class_keys_slots and a fifth boundary -- the unwind edge -- is expressed by none of them. It is honoured today by admission shape (the packed matcher rejects `Stmt::Try`), by a single `try_depth == 0` gate in versioned_indexed_loop, by a post-hoc `contains_gc_unsafe_call` scan, or by the storage kind simply not being movable. Six different mechanisms, none of them a boundary the table states, and a tier added tomorrow inherits none of them. This adds the model: `RegionEnder` (what ends a no-relocation region), `FactBoundary` (how a table expresses extent), `ReceiverClaim` (value vs representation vs address -- the axis that decides whether a boundary is load-bearing), and `boundary_admits` (the algebra, in one place). It emits no IR and no lowering path consults it. The `#![allow(dead_code)]` at the top is the marker for that, matching the #854 subgraphs in `hir_facts`; when it can be deleted, phase 2 has landed. Revertible by deleting the file. The load-bearing artifact is the equivalence lint, which holds the model against `loop_purity::loop_may_allocate` -- shipping and audited -- on a shared battery, asserting the direction with teeth: if the model finds no relocation point, `loop_may_allocate` must also have proven the body alloc-free. The converse is deliberately not asserted, since `loop_may_allocate` answers `true` for any statement it does not model. That lint earned its keep before this landed. Written the obvious way -- enumerate the enders, default to safe -- the model passed every hand-written test and failed the battery on three entries: generic `IndexGet`/`PropertyGet` can reach an accessor or proxy trap, `Expr::Closure` allocates, and `is_inert` belongs on the whole coercing node rather than per operand (the #6975 hole). Inverting the match to an allowlist with an `Unmodelled` catch-all closes that class: adding an HIR variant can no longer silently widen a region. Also transcribes all sixteen tables as test data with their declaration sites, and pins the exact set whose unwind safety is external to their stated boundary: stable_packed_loop_facts (emergent -- `stmt_flags` has a `_ => {}` arm, so `Stmt::Try` is invisible to admission), and the three immutable-fact tables that lean on non-movable storage. A flag is not a bug report; it says the safety comes from somewhere the boundary vocabulary cannot express, which is what phase 2 has to fix. 19 new tests. perry-codegen lib suite 1374 passed / 0 failed; rustfmt clean; clippy reports nothing on either new file; GC store-site inventory passes unchanged. Claude-Session: https://claude.ai/code/session_01Pcq6j6y57TdKSR2Zx2D187 --- crates/perry-codegen/src/collectors/mod.rs | 3 + .../src/collectors/receiver_regions.rs | 557 ++++++++++++ .../src/collectors/receiver_regions_tests.rs | 850 ++++++++++++++++++ 3 files changed, 1410 insertions(+) create mode 100644 crates/perry-codegen/src/collectors/receiver_regions.rs create mode 100644 crates/perry-codegen/src/collectors/receiver_regions_tests.rs diff --git a/crates/perry-codegen/src/collectors/mod.rs b/crates/perry-codegen/src/collectors/mod.rs index b481e5d360..4c6f52e210 100644 --- a/crates/perry-codegen/src/collectors/mod.rs +++ b/crates/perry-codegen/src/collectors/mod.rs @@ -43,6 +43,9 @@ mod ptr_shape_callbacks; mod ptr_shape_elements; mod ptr_shape_report; mod ptr_shape_returns; +mod receiver_regions; +#[cfg(test)] +mod receiver_regions_tests; mod refs; mod repsel_benefit; mod safepoint_sites; diff --git a/crates/perry-codegen/src/collectors/receiver_regions.rs b/crates/perry-codegen/src/collectors/receiver_regions.rs new file mode 100644 index 0000000000..135f95e614 --- /dev/null +++ b/crates/perry-codegen/src/collectors/receiver_regions.rs @@ -0,0 +1,557 @@ +//! Receiver regions — one vocabulary for "receiver R has fact F, valid until +//! boundary B" (#9254, phase 1). +//! +//! # Why this exists +//! +//! Codegen carries fifteen separate receiver-keyed fact tables on `FnCtx` +//! (`cached_lengths`, `bounded_index_pairs`, `packed_f64_loop_facts`, +//! `masked_window_array_facts`, `buffer_view_slots`, `int_range_facts`, +//! `element_shape_loop_facts`, `class_field_loop_facts`, +//! `versioned_indexed_loop_facts`, `stable_packed_loop_facts`, +//! `string_window_array_facts`, `buffer_data_slots`, the two class-shape slot +//! maps and the `packed_receiver_*` trio). Each answers the same two questions +//! — *what do we know about this receiver* and *how long may we believe it* — +//! and each answers the second question in a different, hand-rolled way: +//! +//! | mechanism | tables | +//! |---|---| +//! | `retain(|f| f.scope_id != id)` at scope exit | `bounded_index_pairs`, `packed_f64_loop_facts`, `masked_window_array_facts` | +//! | insert/remove pair with no id | `cached_lengths`, `packed_receiver_*` | +//! | mutable field downgraded in place, never removed | `buffer_view_slots` | +//! | reloaded at the safepoint instead of invalidated | `packed_receiver_*` | +//! +//! and a fifth boundary — the **unwind edge** — is expressed by none of them. +//! It is honoured today only indirectly: the packed matcher rejects +//! `Stmt::Try` outright (`stmt/loops.rs`), masked-window regions consult +//! `ctx.try_depth` before privatising, and `flush_packed_accumulator_locals` +//! writes loop-carried accumulators back at the throw site (#9185/#9210). +//! Every one of those is a local decision by one tier. A tier added tomorrow +//! inherits none of them. +//! +//! # What this module is +//! +//! The model, and nothing else. **This module emits no IR and is consulted by +//! no lowering path.** It exists so the boundary rule can be written once, +//! tested against the tiers that already implement it, and reviewed on its own +//! merits before anything depends on it. Phase 2 is where a tier starts +//! *asking* this module instead of hand-rolling; that is a separate change and +//! this one is revertible by deleting the file. +//! +//! The precedent is `TypeFacts::purity` / `TypeFacts::shape_stability` +//! (`collectors/hir_facts.rs`, #854): a subgraph the collector populates and +//! no pass yet consumes. +//! +//! # The conservative direction +//! +//! A region is a run of code across which no object can be *relocated*. The +//! safe error is to report **too many** enders: more enders means shorter +//! regions means fewer facts believed for less time. That is the same bias +//! `collectors::safepoint_sites` takes for a different consumer ("an +//! over-approximation biased toward spilling"), and the same one-sided +//! contract `loop_purity::loop_may_allocate` states for itself — `false` must +//! mean *provably* cannot allocate. `region_enders_in_stmts` is asserted equal +//! to `loop_may_allocate` on a shared battery in the phase-1 tests, which is +//! what keeps this file honest: the model is checked against a shipping, +//! audited predicate rather than against its own restatement. + +// #9254 phase 1: the region/descriptor model is populated and tested but not +// yet consumed by any codegen pass — the equivalence lint in +// `receiver_regions_tests` is its only caller. Consumption is phase 2, a +// separate change; until then the whole module is dead by design and this +// `allow` is the marker for that, matching the #854 subgraphs in `hir_facts`. +// If this attribute can be deleted, phase 2 has landed. +#![allow(dead_code)] + +use crate::loop_purity; +use perry_hir::{CompareOp, Expr, Stmt, UnaryOp}; + +/// Why a no-relocation region ends. +/// +/// Derived from the collection points codegen actually has. Ordering is by +/// how hard the ender is to see in source, not by severity. +#[derive(Debug, Clone, Copy, PartialEq, Eq)] +pub(crate) enum RegionEnder { + /// A call whose direct callee is not on the audited non-collecting + /// allowlist (`gc_call_effects::classify_direct_callee` → `Unknown`), a + /// call to a module function not proven leaf by the transitive closure, or + /// any indirect call. The overwhelming majority of enders. + CollectingCall, + /// An allocating literal, property store or index store: these lower to a + /// runtime helper that allocates and can collect. Separate from + /// `CollectingCall` because they carry no callee name in source and are + /// the ones #8583 found invisible. + AllocatingOperation, + /// A coercing operator over an operand not proven a non-pointer primitive. + /// `ToPrimitive` dispatches to a user `valueOf` / `Symbol.toPrimitive` / + /// `toString`, which is arbitrary JS: it allocates, and it collects. + Coercion, + /// `await`, `yield`, or an async-first call: control reaches the microtask + /// pump, whose outermost boundary runs the moving minor collection. + Suspension, + /// A `throw`, or the unwind successor of an `invoke`. The throw helpers + /// allocate the Error they raise, so the handler's roots are relocated — + /// and, unlike every other ender, this one leaves the *fall-through* path + /// untouched, which is why a tier can be accidentally correct on it for + /// years (#9185). + UnwindEdge, + /// A loop back-edge GC poll. Deliberately placed, and the only collection + /// point inside an otherwise call-free fast clone. + BackEdgePoll, + /// An expression this model does not classify. Reported as an ender + /// because the default must be "assume it collects": the allowlist below + /// is what has been argued sound, and everything else — a closure + /// allocation, a template literal, a spread, a regex, a `PropertyGet` that + /// may reach a getter — has not been. + Unmodelled, +} + +impl RegionEnder { + /// Whether the ender can relocate objects *without* transferring control + /// out of the region's fall-through path. + /// + /// `UnwindEdge` is the one that cannot: it diverts. A fact consulted only + /// on the fall-through path is unaffected by it, which is exactly why + /// unwind safety is so easy to get accidentally right — and why a fact + /// that is *written back* at region exit (a loop-carried accumulator) is + /// not, because the unwind edge skips that exit block. + pub(crate) fn is_fallthrough(self) -> bool { + !matches!(self, RegionEnder::UnwindEdge) + } +} + +/// How a fact table expresses the extent of its claim. +/// +/// One variant per mechanism found in the fifteen tables. These are +/// descriptions of what is implemented today, not a proposal. +#[derive(Debug, Clone, Copy, PartialEq, Eq)] +pub(crate) enum FactBoundary { + /// `retain(|f| f.scope_id != id)` at the exit of the scope that pushed it. + ScopeId, + /// An insert/remove (or push/pop) pair around one lowering call, with no + /// identifier — validity is the dynamic extent of that call. + DynamicExtent, + /// Never removed. A mutable field on the entry is downgraded at each + /// hazard instead (`AliasState::MayAlias`, + /// `BufferViewPointerState::Invalidated`). + InPlaceDegradation, + /// Reloaded from the authoritative root at the safepoint, rather than + /// invalidated. Only the `packed_receiver_*` trio does this. + PollRefresh, + /// Nothing removes or downgrades it. + Never, +} + +/// What a table claims about a receiver. The axis that matters for boundary +/// checking is whether the claim names an *address*. +#[derive(Debug, Clone, Copy, PartialEq, Eq)] +pub(crate) enum ReceiverClaim { + /// A `length` value, an index range, or any other arithmetic relation. A + /// moving collection changes an object's address, never its length, so + /// these survive relocation by content. + ScalarRelation, + /// The receiver's element representation (packed raw f64, dense i32, a + /// proven element shape). Survives relocation — it is a property of the + /// object, not of where it lives — but dies at anything that can *mutate* + /// the receiver, which is a strictly larger set than the enders here. + Representation, + /// A cached raw pointer, box, or masked handle. Relocation invalidates it + /// outright; this is the only claim for which a region boundary is + /// load-bearing rather than incidental. + Address, +} + +/// One table's claim about one receiver, in the shared vocabulary. +#[derive(Debug, Clone, PartialEq, Eq)] +pub(crate) struct ReceiverDescriptor { + /// The fact table this descriptor was normalised from, for diagnostics. + pub(crate) table: &'static str, + /// The receiver's `LocalId`. + pub(crate) receiver: u32, + pub(crate) claim: ReceiverClaim, + pub(crate) boundary: FactBoundary, + /// Whether the tier that owns this table structurally excludes `Stmt::Try` + /// from the region it forms (the packed matcher does; `buffer_view_slots` + /// has no region at all). + pub(crate) excludes_try: bool, +} + +/// Why a descriptor's declared boundary does not cover an ender present in its +/// region. +#[derive(Debug, Clone, PartialEq, Eq)] +pub(crate) struct BoundaryViolation { + pub(crate) table: &'static str, + pub(crate) receiver: u32, + pub(crate) ender: RegionEnder, + pub(crate) why: &'static str, +} + +/// The core rule: may `desc` still be believed across `ender`? +/// +/// Read this as the one place the boundary algebra lives. Every tier +/// implements some projection of it by hand today. +pub(crate) fn boundary_admits( + desc: &ReceiverDescriptor, + ender: RegionEnder, +) -> Result<(), BoundaryViolation> { + let deny = |why| { + Err(BoundaryViolation { + table: desc.table, + receiver: desc.receiver, + ender, + why, + }) + }; + + match desc.claim { + // A length or an index range is a value. Relocation does not touch it. + // It dies at mutation, which no ender in this enum implies on its own. + ReceiverClaim::ScalarRelation => Ok(()), + + // A representation claim survives relocation but not arbitrary user + // code, which can convert the receiver's storage out from under it. + ReceiverClaim::Representation => match ender { + RegionEnder::BackEdgePoll => Ok(()), + RegionEnder::CollectingCall + | RegionEnder::AllocatingOperation + | RegionEnder::Coercion + | RegionEnder::Suspension + | RegionEnder::Unmodelled => { + deny("user code reachable from this ender can change the receiver's storage kind") + } + RegionEnder::UnwindEdge => { + if desc.excludes_try { + Ok(()) + } else { + deny("representation claim may be consulted in a handler the tier never gated") + } + } + }, + + // An address dies at every relocation, with exactly one exception. + ReceiverClaim::Address => match (ender, desc.boundary) { + // The poll is the one ender a refresh recipe is written for. + (RegionEnder::BackEdgePoll, FactBoundary::PollRefresh) => Ok(()), + (RegionEnder::BackEdgePoll, _) => { + deny("cached address is not reloaded at the back-edge poll that may have moved it") + } + // The unwind edge diverts, so a read-only cache consulted only on + // the fall-through path is unharmed — but only if the tier + // actually excluded `Try` from its region. + (RegionEnder::UnwindEdge, _) if desc.excludes_try => Ok(()), + (RegionEnder::UnwindEdge, _) => deny( + "cached address may be consulted in a handler reached after a relocating throw", + ), + _ => deny("cached address does not survive this relocation point"), + }, + } +} + +/// Check one descriptor against every ender in its region. +pub(crate) fn violations_for( + desc: &ReceiverDescriptor, + enders: &[RegionEnder], +) -> Vec { + enders + .iter() + .filter_map(|&e| boundary_admits(desc, e).err()) + .collect() +} + +/// Classify a single expression as a region ender. +/// +/// `is_inert` proves an operand is a non-pointer primitive — the same +/// injected predicate `loop_purity::loop_may_allocate` takes, so that the two +/// can be held to the same answer (see the phase-1 equivalence test). +/// +/// Returns the *first* reason found; an expression can qualify several ways +/// and the caller only needs to know the region ends. +pub(crate) fn expr_region_ender(e: &Expr, is_inert: &dyn Fn(&Expr) -> bool) -> Option { + match e { + // ---- Provably not a relocation point ------------------------------- + // Constants, reads of a local/global, and references. No dispatch. + Expr::Undefined + | Expr::Null + | Expr::Bool(_) + | Expr::Number(_) + | Expr::Integer(_) + | Expr::BigInt(_) + | Expr::String(_) + | Expr::This + | Expr::LocalGet(_) + | Expr::GlobalGet(_) + | Expr::FuncRef(_) + | Expr::ClassRef(_) + | Expr::EnumMember { .. } => None, + + // Typed-array and buffer element access: a fixed-layout numeric load, + // or a store into a backing buffer that never grows. Mirrors + // `loop_purity::expr_alloc_free`. + Expr::BufferIndexGet { .. } + | Expr::Uint8ArrayGet { .. } + | Expr::BufferIndexSet { .. } + | Expr::Uint8ArraySet { .. } => None, + + // `===` / `!==` never coerce; `&&` / `||` / `??` run only ToBoolean, + // which on an object is a tag test. `!x`, `typeof x`, `void x` reach + // no user-defined conversion. All stay open to operands of any type. + Expr::Compare { + op: CompareOp::Eq | CompareOp::Ne, + .. + } + | Expr::Logical { .. } + | Expr::Unary { + op: UnaryOp::Not, .. + } + | Expr::TypeOf(_) + | Expr::Void(_) => None, + + // Pure plumbing; the interesting part is in the children, which the + // caller walks. + Expr::LocalSet(..) | Expr::Conditional { .. } => None, + + // ---- Coercing operators -------------------------------------------- + // Relational/loose comparison, arithmetic and bitwise `Binary`, the + // remaining `Unary` forms and `x++` / `x--` all run ToPrimitive / + // ToNumeric, and a user-defined `valueOf` / `Symbol.toPrimitive` / + // `toString` is arbitrary JS: it allocates, and it collects. + // + // `is_inert` is applied to the WHOLE node, not to the operands, which + // is what `loop_purity` does. Recursing into operands does not see the + // dispatch — `a < b` over two plain locals recurses clean while the + // comparison itself can call user code, the hole #6975 closed one + // abstraction over. + Expr::Compare { .. } | Expr::Binary { .. } | Expr::Unary { .. } | Expr::Update { .. } => { + if is_inert(e) { + None + } else { + Some(RegionEnder::Coercion) + } + } + + // ---- Suspension ---------------------------------------------------- + // Control reaches the microtask pump, whose outermost boundary runs + // the moving minor collection. + Expr::Await(_) | Expr::Yield { .. } | Expr::AsyncFirstCall { .. } => { + Some(RegionEnder::Suspension) + } + + // ---- Calls --------------------------------------------------------- + // Phase 1 does not consult `gc_call_effects::classify_direct_callee`: + // that keys off an emitted IR symbol name and this is an HIR walk, so + // every call is an ender. Conservative, and the direction that keeps + // the soundness implication below exact. Phase 2 is where a direct + // call to an audited non-collecting helper stops ending a region. + Expr::Call { .. } + | Expr::CallSpread { .. } + | Expr::NativeMethodCall { .. } + | Expr::StaticMethodCall { .. } + | Expr::SuperCall(_) + | Expr::SuperCallSpread(_) + | Expr::SuperMethodCall { .. } + | Expr::SuperMethodCallSpread { .. } + | Expr::ObjectSuperMethodCall { .. } + | Expr::New { .. } + | Expr::NewDynamic { .. } + | Expr::NewDynamicSpread { .. } => Some(RegionEnder::CollectingCall), + + // ---- Allocating operations with no callee in source ---------------- + // #8583's blind spot: these lower to an allocating runtime helper that + // RS4GC gives a statepoint, but carry no callee name. + Expr::Object(_) + | Expr::ObjectSpread { .. } + | Expr::ObjectAssign { .. } + | Expr::Array(_) + | Expr::ArraySpread(_) + | Expr::Closure { .. } + | Expr::PropertySet { .. } + | Expr::PropertyUpdate { .. } + | Expr::IndexSet { .. } + | Expr::IndexUpdate { .. } => Some(RegionEnder::AllocatingOperation), + + // ---- Reads that can reach user code -------------------------------- + // A generic property or index READ can hit an accessor or a proxy + // trap, both arbitrary JS. `collectors::safepoint_sites` deliberately + // does NOT count these, because over-counting reads would over-spill a + // read-heavy hot loop and its consumer only needs a spill estimate. A + // region model has the opposite obligation: missing one licenses a + // stale cached address. `loop_purity` excludes them for the same + // reason, via its `_ => false` arm. + Expr::PropertyGet { .. } | Expr::IndexGet { .. } => Some(RegionEnder::CollectingCall), + + // ---- Everything else ----------------------------------------------- + // Assume it collects. Adding a variant to the allowlist above requires + // an argument; landing a new HIR variant does not silently widen a + // region. + _ => Some(RegionEnder::Unmodelled), + } +} + +/// Every region ender reachable from `stmts` and `controls`, without +/// descending into nested closures (a closure is its own frame, and its +/// regions are its own). +/// +/// Duplicates are preserved: the count is meaningful to a caller forming +/// maximal regions, and de-duplicating would hide a second ender of the same +/// kind at a different point. +pub(crate) fn region_enders_in_stmts( + stmts: &[Stmt], + controls: &[&Expr], + is_inert: &dyn Fn(&Expr) -> bool, +) -> Vec { + let mut out = Vec::new(); + for s in stmts { + enders_in_stmt(s, is_inert, &mut out); + } + for c in controls { + enders_in_expr(c, is_inert, &mut out); + } + out +} + +fn enders_in_expr(e: &Expr, is_inert: &dyn Fn(&Expr) -> bool, out: &mut Vec) { + if let Some(r) = expr_region_ender(e, is_inert) { + out.push(r); + } + perry_hir::walker::walk_expr_children(e, &mut |child| enders_in_expr(child, is_inert, out)); +} + +fn enders_in_stmt(s: &Stmt, is_inert: &dyn Fn(&Expr) -> bool, out: &mut Vec) { + match s { + // A throw is an unwind edge *and* the helper allocates the Error. + Stmt::Throw(e) => { + enders_in_expr(e, is_inert, out); + out.push(RegionEnder::UnwindEdge); + } + Stmt::Let { init: Some(e), .. } | Stmt::Expr(e) | Stmt::Return(Some(e)) => { + enders_in_expr(e, is_inert, out) + } + Stmt::Let { init: None, .. } | Stmt::Return(None) => {} + Stmt::If { + condition, + then_branch, + else_branch, + } => { + enders_in_expr(condition, is_inert, out); + for st in then_branch { + enders_in_stmt(st, is_inert, out); + } + if let Some(else_branch) = else_branch { + for st in else_branch { + enders_in_stmt(st, is_inert, out); + } + } + } + // A nested loop's back-edge poll is a relocation point for the + // *enclosing* region too — this is why the armed-poll refresh reloads + // every active receiver cache, not just the innermost scope's. + Stmt::While { condition, body } | Stmt::DoWhile { body, condition } => { + enders_in_expr(condition, is_inert, out); + for st in body { + enders_in_stmt(st, is_inert, out); + } + out.push(RegionEnder::BackEdgePoll); + } + Stmt::For { + init, + condition, + update, + body, + } => { + if let Some(init) = init { + enders_in_stmt(init, is_inert, out); + } + if let Some(condition) = condition { + enders_in_expr(condition, is_inert, out); + } + if let Some(update) = update { + enders_in_expr(update, is_inert, out); + } + for st in body { + enders_in_stmt(st, is_inert, out); + } + out.push(RegionEnder::BackEdgePoll); + } + Stmt::Labeled { body, .. } => enders_in_stmt(body, is_inert, out), + // Every statement in a `try` body may divert to the handler. + Stmt::Try { + body, + catch, + finally, + } => { + for st in body { + enders_in_stmt(st, is_inert, out); + } + out.push(RegionEnder::UnwindEdge); + if let Some(catch) = catch { + for st in &catch.body { + enders_in_stmt(st, is_inert, out); + } + } + if let Some(finally) = finally { + for st in finally { + enders_in_stmt(st, is_inert, out); + } + } + } + Stmt::Switch { + discriminant, + cases, + } => { + enders_in_expr(discriminant, is_inert, out); + for c in cases { + if let Some(t) = &c.test { + enders_in_expr(t, is_inert, out); + } + for st in &c.body { + enders_in_stmt(st, is_inert, out); + } + } + } + Stmt::Break + | Stmt::Continue + | Stmt::LabeledBreak(_) + | Stmt::LabeledContinue(_) + | Stmt::PreallocateBoxes(_) + | Stmt::PreallocateTdzBoxes(_) + | Stmt::ReleaseBoxes(_) => {} + } +} + +/// The phase-1 equivalence check. +/// +/// `loop_purity::loop_may_allocate` is the shipping, audited answer to "can +/// this loop body reach a collection point". The model must be **no weaker** +/// than it: +/// +/// > if the model finds no relocation point other than a back-edge poll, then +/// > `loop_may_allocate` must also have proven the body alloc-free. +/// +/// That is the direction with teeth. The model's whole purpose is to license +/// believing a fact across a span of code; a span the model calls clean and +/// `loop_may_allocate` does not is either a real hole in the model or a real +/// imprecision in `loop_may_allocate`, and phase 1 exists to find out which +/// before a lowering path depends on the answer. Writing this file the +/// obvious way — enumerate the enders, default to "safe" — put three such +/// holes in it (generic `IndexGet`/`PropertyGet` reaching an accessor, +/// `Expr::Closure` allocating, and `is_inert` applied per-operand instead of +/// to the whole coercing node). Inverting the match to an allowlist with an +/// `Unmodelled` catch-all is what closes that class. +/// +/// **The converse is deliberately not asserted.** `loop_may_allocate` answers +/// `true` for any statement it does not model — `Return`, `Switch`, `Throw`, +/// `Try` all fall to its `_ => false` arm — which is imprecision, not a +/// collection point. Requiring equality would force this model to inherit +/// that imprecision and would make a `return` inside a region end it, which is +/// wrong. +pub(crate) fn model_is_no_weaker_than_loop_purity( + body: &[Stmt], + controls: &[&Expr], + is_inert: &dyn Fn(&Expr) -> bool, +) -> bool { + let enders = region_enders_in_stmts(body, controls, is_inert); + let model_finds_relocation = enders + .iter() + .any(|e| !matches!(e, RegionEnder::BackEdgePoll)); + // no relocation found => loop_purity proved it alloc-free + model_finds_relocation || !loop_purity::loop_may_allocate(body, controls, is_inert) +} diff --git a/crates/perry-codegen/src/collectors/receiver_regions_tests.rs b/crates/perry-codegen/src/collectors/receiver_regions_tests.rs new file mode 100644 index 0000000000..d23abb0f41 --- /dev/null +++ b/crates/perry-codegen/src/collectors/receiver_regions_tests.rs @@ -0,0 +1,850 @@ +//! Phase-1 tests for the receiver-region model (#9254). +//! +//! Two jobs. The first half pins the boundary algebra — in particular the +//! unwind rule, which no fact table expresses today and which is therefore the +//! part with no shipping implementation to check against. The second half is +//! the equivalence lint: the model held against `loop_purity::loop_may_allocate`, +//! a shipping audited predicate, on a shared battery. +//! +//! The battery is the load-bearing artifact. Three unsoundnesses in the first +//! draft of the model survived review and died here. + +use super::receiver_regions::*; +use perry_hir::types::Type; +use perry_hir::{BinaryOp, CatchClause, CompareOp, Expr, Stmt, UnaryOp, UpdateOp}; + +// --------------------------------------------------------------------------- +// Shared fixtures. `stub_inert` mirrors `loop_purity`'s own test stub exactly, +// so a battery entry means the same thing to both predicates. +// --------------------------------------------------------------------------- + +/// A local the real `expr_is_inert_primitive` would prove a non-pointer +/// primitive. +const NUM: u32 = 1; +const NUM2: u32 = 2; +/// A local it would refuse: `any`-typed / shadow-slotted / a module global — +/// one that can hold an object with a user-defined `valueOf`. +const OBJ: u32 = 9; + +fn stub_inert(e: &Expr) -> bool { + match e { + Expr::Undefined | Expr::Null | Expr::Bool(_) | Expr::Number(_) | Expr::Integer(_) => true, + Expr::LocalGet(id) | Expr::Update { id, .. } => *id == NUM || *id == NUM2, + Expr::Unary { operand, .. } => stub_inert(operand), + Expr::Compare { left, right, .. } | Expr::Binary { left, right, .. } => { + stub_inert(left) && stub_inert(right) + } + _ => false, + } +} + +fn enders(body: &[Stmt]) -> Vec { + region_enders_in_stmts(body, &[], &stub_inert) +} + +fn has(body: &[Stmt], e: RegionEnder) -> bool { + enders(body).contains(&e) +} + +fn num(id: u32) -> Expr { + Expr::LocalGet(id) +} + +fn call(args: Vec) -> Expr { + Expr::Call { + callee: Box::new(Expr::Undefined), + args, + type_args: vec![], + byte_offset: 0, + } +} + +fn add(left: Expr, right: Expr) -> Expr { + Expr::Binary { + op: BinaryOp::Add, + left: Box::new(left), + right: Box::new(right), + } +} + +fn lt(left: Expr, right: Expr) -> Expr { + Expr::Compare { + op: CompareOp::Lt, + left: Box::new(left), + right: Box::new(right), + } +} + +fn closure(body: Vec) -> Expr { + Expr::Closure { + func_id: 0, + params: vec![], + return_type: Type::Any, + body, + captures: vec![], + mutable_captures: vec![], + captures_this: false, + captures_new_target: false, + enclosing_class: None, + is_arrow: false, + is_async: false, + is_generator: false, + is_strict: false, + } +} + +/// A descriptor in the shape of one of the real tables. +fn desc( + table: &'static str, + claim: ReceiverClaim, + boundary: FactBoundary, + excludes_try: bool, +) -> ReceiverDescriptor { + ReceiverDescriptor { + table, + receiver: OBJ, + claim, + boundary, + excludes_try, + } +} + +/// `cached_lengths` / `bounded_index_pairs`: a value, not an address. +fn cached_length_desc() -> ReceiverDescriptor { + desc( + "cached_lengths", + ReceiverClaim::ScalarRelation, + FactBoundary::DynamicExtent, + false, + ) +} + +/// `packed_receiver_box_slots`: an address, reloaded at the poll, inside a +/// region the packed matcher keeps free of `Stmt::Try`. +fn packed_receiver_desc() -> ReceiverDescriptor { + desc( + "packed_receiver_box_slots", + ReceiverClaim::Address, + FactBoundary::PollRefresh, + true, + ) +} + +/// `buffer_view_slots`: an address, function-lifetime, degraded in place, and +/// with no region that excludes `Stmt::Try`. +fn buffer_view_desc() -> ReceiverDescriptor { + desc( + "buffer_view_slots", + ReceiverClaim::Address, + FactBoundary::InPlaceDegradation, + false, + ) +} + +/// `packed_f64_loop_facts`: a representation claim, scope-id bounded, inside a +/// matcher that rejects `Stmt::Try`. +fn packed_f64_desc() -> ReceiverDescriptor { + desc( + "packed_f64_loop_facts", + ReceiverClaim::Representation, + FactBoundary::ScopeId, + true, + ) +} + +const ALL_ENDERS: [RegionEnder; 6] = [ + RegionEnder::CollectingCall, + RegionEnder::AllocatingOperation, + RegionEnder::Coercion, + RegionEnder::Suspension, + RegionEnder::UnwindEdge, + RegionEnder::BackEdgePoll, +]; + +// --------------------------------------------------------------------------- +// The boundary algebra. +// --------------------------------------------------------------------------- + +/// A moving collection changes an object's ADDRESS, never its length. This is +/// the reason `cached_lengths` needs no safepoint logic at all, and it has to +/// fall out of the model rather than be special-cased per table. +#[test] +fn a_scalar_relation_survives_every_relocation_point() { + let d = cached_length_desc(); + for e in ALL_ENDERS { + assert!( + boundary_admits(&d, e).is_ok(), + "a length/bounds VALUE must survive {e:?} — relocation moves the object, not the number" + ); + } + assert!(violations_for(&d, &ALL_ENDERS).is_empty()); +} + +/// The `packed_receiver_*` contract: the cache is reloaded on the armed arm of +/// the poll, which is the only collection point a call-free clone has. +#[test] +fn a_cached_address_survives_the_poll_only_with_a_refresh_recipe() { + assert!( + boundary_admits(&packed_receiver_desc(), RegionEnder::BackEdgePoll).is_ok(), + "PollRefresh is written for exactly this ender" + ); + + // The same claim under any other boundary mechanism is a stale pointer. + for boundary in [ + FactBoundary::ScopeId, + FactBoundary::DynamicExtent, + FactBoundary::InPlaceDegradation, + FactBoundary::Never, + ] { + let d = desc("hypothetical", ReceiverClaim::Address, boundary, true); + let v = boundary_admits(&d, RegionEnder::BackEdgePoll) + .expect_err("a cached address with no refresh recipe cannot cross a poll"); + assert_eq!(v.ender, RegionEnder::BackEdgePoll); + } +} + +/// A cached address dies at a call no matter how the table scopes itself — +/// scoping is not a substitute for the region being call-free. +#[test] +fn a_cached_address_dies_at_a_collecting_call_under_every_boundary() { + for boundary in [ + FactBoundary::ScopeId, + FactBoundary::DynamicExtent, + FactBoundary::InPlaceDegradation, + FactBoundary::PollRefresh, + FactBoundary::Never, + ] { + let d = desc("hypothetical", ReceiverClaim::Address, boundary, true); + assert!( + boundary_admits(&d, RegionEnder::CollectingCall).is_err(), + "{boundary:?} must not license a cached address across a call" + ); + } +} + +/// THE phase-1 finding. `buffer_view_slots` caches a raw data pointer, is +/// function-lifetime, and is never removed — only downgraded in place. Nothing +/// structurally stops an entry registered before a `try` from being consulted +/// inside the `catch` handler, and `lower_try` clears no fact table. +/// +/// It is sound in the shipped compiler for a reason outside the model (typed +/// and buffer storage is marked non-movable and never relocates). The model +/// flags it anyway, and that is correct behaviour for phase 1: the tier is +/// relying on a property of the storage kind that its own boundary mechanism +/// does not state. When phase 2 gives descriptors a non-movable-storage +/// attribute this becomes a clean pass; until then a flag is the honest answer. +#[test] +fn an_address_claim_with_no_try_exclusion_is_flagged_on_the_unwind_edge() { + let v = boundary_admits(&buffer_view_desc(), RegionEnder::UnwindEdge) + .expect_err("function-lifetime address claim reaches the catch handler"); + assert_eq!(v.table, "buffer_view_slots"); + assert_eq!(v.ender, RegionEnder::UnwindEdge); + + // A tier that DOES exclude `Try` from its region is not flagged: the + // packed matcher rejects `Stmt::Try` outright, so no handler can observe + // its cache. + assert!(boundary_admits(&packed_receiver_desc(), RegionEnder::UnwindEdge).is_ok()); +} + +/// An unwind edge diverts control; it does not fall through. That is why a +/// read-only cache is unharmed by it while a loop-carried accumulator written +/// back at region exit is not — the unwind edge skips the exit block. #9185. +#[test] +fn the_unwind_edge_is_the_only_non_fallthrough_ender() { + for e in ALL_ENDERS { + assert_eq!( + e.is_fallthrough(), + e != RegionEnder::UnwindEdge, + "{e:?} misclassified" + ); + } +} + +/// A representation claim survives relocation but not arbitrary user code: +/// the object stays where the poll left it, but a callee can convert its +/// storage. +#[test] +fn a_representation_claim_survives_the_poll_but_not_user_code() { + let d = packed_f64_desc(); + assert!(boundary_admits(&d, RegionEnder::BackEdgePoll).is_ok()); + for e in [ + RegionEnder::CollectingCall, + RegionEnder::AllocatingOperation, + RegionEnder::Coercion, + RegionEnder::Suspension, + RegionEnder::Unmodelled, + ] { + assert!( + boundary_admits(&d, e).is_err(), + "{e:?} can reach user code that changes the receiver's storage kind" + ); + } +} + +/// The catch-all must behave as a relocation point, or adding an HIR variant +/// silently widens every region. +#[test] +fn an_unmodelled_expression_is_a_relocation_point() { + for d in [packed_receiver_desc(), packed_f64_desc()] { + assert!(boundary_admits(&d, RegionEnder::Unmodelled).is_err()); + } + // ...but it still does not disturb a value claim. + assert!(boundary_admits(&cached_length_desc(), RegionEnder::Unmodelled).is_ok()); +} + +// --------------------------------------------------------------------------- +// The walker. +// --------------------------------------------------------------------------- + +#[test] +fn constants_reads_and_non_coercing_operators_are_not_enders() { + let body = vec![ + Stmt::Expr(Expr::Number(1.0)), + Stmt::Expr(num(NUM)), + // `===` never coerces, so it stays open to operands of any type. + Stmt::Expr(Expr::Compare { + op: CompareOp::Eq, + left: Box::new(num(OBJ)), + right: Box::new(Expr::Null), + }), + // `!x` is ToBoolean; `typeof x` reads a tag. + Stmt::Expr(Expr::Unary { + op: UnaryOp::Not, + operand: Box::new(num(OBJ)), + }), + Stmt::Expr(Expr::TypeOf(Box::new(num(OBJ)))), + ]; + assert!(enders(&body).is_empty(), "got {:?}", enders(&body)); +} + +#[test] +fn typed_array_element_access_is_not_an_ender() { + // A fixed-layout numeric load, and a store into a buffer that never grows. + let body = vec![ + Stmt::Expr(Expr::Uint8ArrayGet { + array: Box::new(num(OBJ)), + index: Box::new(num(NUM)), + }), + Stmt::Expr(Expr::BufferIndexSet { + buffer: Box::new(num(OBJ)), + index: Box::new(num(NUM)), + value: Box::new(Expr::Number(0.0)), + }), + ]; + assert!(enders(&body).is_empty(), "got {:?}", enders(&body)); +} + +/// `is_inert` is consulted on the WHOLE coercing node, not per operand — the +/// #6975 hole. `a < b` over two plain locals recurses clean while the +/// comparison itself can dispatch to a user `valueOf`. +#[test] +fn coercion_is_an_ender_exactly_when_the_node_is_not_proven_inert() { + let proven = vec![Stmt::Expr(add(num(NUM), num(NUM2)))]; + assert!( + enders(&proven).is_empty(), + "proven-primitive operands: no ender" + ); + + let unproven = vec![Stmt::Expr(add(num(NUM), num(OBJ)))]; + assert_eq!(enders(&unproven), vec![RegionEnder::Coercion]); + + // `x++` on an unproven local coerces too. + let update = vec![Stmt::Expr(Expr::Update { + id: OBJ, + op: UpdateOp::Increment, + prefix: false, + })]; + assert_eq!(enders(&update), vec![RegionEnder::Coercion]); + + // Re-run the passing shape under a predicate that proves nothing, so the + // clean answer above is attributable to the operand proof and not to the + // shape being unreachable. + let nothing_inert = region_enders_in_stmts(&proven, &[], &|_| false); + assert_eq!(nothing_inert, vec![RegionEnder::Coercion]); +} + +/// Deliberate divergence from `collectors::safepoint_sites`, which does not +/// count reads. Its consumer wants a spill estimate and over-counting reads +/// would over-spill a read-heavy loop. A region model has the opposite +/// obligation: a `PropertyGet` can reach an accessor, an `IndexGet` a proxy +/// trap, and missing either licenses a stale cached address. +#[test] +fn generic_property_and_index_reads_are_enders() { + let get = vec![Stmt::Expr(Expr::PropertyGet { + object: Box::new(num(OBJ)), + property: "x".to_string(), + byte_offset: 0, + })]; + assert_eq!(get.len(), 1); + assert!(has(&get, RegionEnder::CollectingCall)); + + let idx = vec![Stmt::Expr(Expr::IndexGet { + object: Box::new(num(OBJ)), + index: Box::new(num(NUM)), + })]; + assert!(has(&idx, RegionEnder::CollectingCall)); +} + +/// Allocating a closure allocates. The first draft of the model let this +/// through its `_ => None` catch-all. +#[test] +fn closure_allocation_is_an_ender_and_its_body_is_not_descended_into() { + let body = vec![Stmt::Expr(closure(vec![Stmt::Expr(call(vec![]))]))]; + let got = enders(&body); + // The closure allocation itself, and nothing from its body: a nested + // closure is its own frame with its own regions. + assert_eq!(got, vec![RegionEnder::AllocatingOperation], "got {got:?}"); +} + +#[test] +fn throw_and_try_contribute_an_unwind_edge() { + let thrown = vec![Stmt::Throw(Expr::Number(1.0))]; + assert!(has(&thrown, RegionEnder::UnwindEdge)); + + let tried = vec![Stmt::Try { + body: vec![Stmt::Expr(num(NUM))], + catch: Some(CatchClause { + param: None, + body: vec![Stmt::Expr(num(NUM))], + }), + finally: None, + }]; + assert!( + has(&tried, RegionEnder::UnwindEdge), + "every statement in a try body may divert to the handler" + ); +} + +#[test] +fn every_loop_contributes_a_back_edge_poll_including_a_nested_one() { + let inner = Stmt::While { + condition: Expr::Bool(true), + body: vec![Stmt::Expr(num(NUM))], + }; + let outer = vec![Stmt::While { + condition: Expr::Bool(true), + body: vec![inner], + }]; + let polls = enders(&outer) + .iter() + .filter(|e| **e == RegionEnder::BackEdgePoll) + .count(); + assert_eq!( + polls, 2, + "an inner loop's poll is a relocation point for the enclosing region too — \ + which is why the armed-poll refresh reloads every active receiver cache, \ + not just the innermost scope's" + ); +} + +// --------------------------------------------------------------------------- +// The equivalence lint. +// --------------------------------------------------------------------------- + +/// Bodies spanning both answers, shared with `loop_purity`'s vocabulary. +fn battery() -> Vec<(&'static str, Vec)> { + vec![ + ("empty", vec![]), + ("constant", vec![Stmt::Expr(Expr::Number(1.0))]), + ("local read", vec![Stmt::Expr(num(NUM))]), + ( + "proven-primitive arithmetic", + vec![Stmt::Expr(add(num(NUM), num(NUM2)))], + ), + ( + "proven-primitive comparison", + vec![Stmt::Expr(lt(num(NUM), num(NUM2)))], + ), + ( + "typed-array copy", + vec![Stmt::Expr(Expr::Uint8ArraySet { + array: Box::new(num(OBJ)), + index: Box::new(num(NUM)), + value: Box::new(Expr::Uint8ArrayGet { + array: Box::new(num(OBJ)), + index: Box::new(num(NUM2)), + }), + })], + ), + ( + "strict equality on an object", + vec![Stmt::Expr(Expr::Compare { + op: CompareOp::Eq, + left: Box::new(num(OBJ)), + right: Box::new(Expr::Null), + })], + ), + ("a call", vec![Stmt::Expr(call(vec![]))]), + ( + "coercion over an unproven operand", + vec![Stmt::Expr(add(num(NUM), num(OBJ)))], + ), + ( + "object literal", + vec![Stmt::Expr(Expr::Object(vec![( + "k".to_string(), + Expr::Number(1.0), + )]))], + ), + ("array literal", vec![Stmt::Expr(Expr::Array(vec![]))]), + ("closure allocation", vec![Stmt::Expr(closure(vec![]))]), + ( + "generic property read", + vec![Stmt::Expr(Expr::PropertyGet { + object: Box::new(num(OBJ)), + property: "x".to_string(), + byte_offset: 0, + })], + ), + ( + "generic index read", + vec![Stmt::Expr(Expr::IndexGet { + object: Box::new(num(OBJ)), + index: Box::new(num(NUM)), + })], + ), + ( + "generic index write", + vec![Stmt::Expr(Expr::IndexSet { + object: Box::new(num(OBJ)), + index: Box::new(num(NUM)), + value: Box::new(Expr::Number(1.0)), + })], + ), + ( + "conditional over clean operands", + vec![Stmt::Expr(Expr::Conditional { + condition: Box::new(Expr::Bool(true)), + then_expr: Box::new(num(NUM)), + else_expr: Box::new(num(NUM2)), + })], + ), + ( + "if with a call in one arm", + vec![Stmt::If { + condition: Expr::Bool(true), + then_branch: vec![Stmt::Expr(call(vec![]))], + else_branch: Some(vec![Stmt::Expr(num(NUM))]), + }], + ), + ( + "nested loop, clean body", + vec![Stmt::While { + condition: Expr::Bool(true), + body: vec![Stmt::Expr(add(num(NUM), num(NUM2)))], + }], + ), + ] +} + +/// The phase-1 contract: **if the model finds no relocation point other than a +/// back-edge poll, `loop_may_allocate` must also have proven the body +/// alloc-free.** +/// +/// The converse is not asserted — `loop_may_allocate` answers `true` for any +/// statement it does not model, which is imprecision, not a collection point. +/// +/// This assertion is why the model is written as an allowlist with an +/// `Unmodelled` catch-all rather than as an enumeration of enders. The +/// enumeration version passed every hand-written test above and failed here on +/// three entries: `generic property read`, `generic index read`, and +/// `closure allocation`. +#[test] +fn model_is_no_weaker_than_loop_purity_across_the_battery() { + let weaker: Vec<&str> = battery() + .into_iter() + .filter(|(_, body)| !model_is_no_weaker_than_loop_purity(body, &[], &stub_inert)) + .map(|(name, _)| name) + .collect(); + + assert!( + weaker.is_empty(), + "MODEL IS WEAKER THAN loop_may_allocate on {weaker:?}: the model found no \ + relocation point in these bodies, but loop_purity did not prove them \ + alloc-free. Either the model is missing an ender (unsound — it would \ + license believing a cached address across a real collection point) or \ + loop_purity is imprecise there and the exemption belongs in this test \ + with an argument." + ); +} + +/// The battery has to contain entries on both sides, or the implication above +/// is vacuously true and proves nothing. +#[test] +fn the_battery_exercises_both_answers() { + let mut clean = 0; + let mut dirty = 0; + for (_, body) in battery() { + if region_enders_in_stmts(&body, &[], &stub_inert) + .iter() + .any(|e| !matches!(e, RegionEnder::BackEdgePoll)) + { + dirty += 1; + } else { + clean += 1; + } + } + assert!( + clean >= 6, + "only {clean} clean entries — implication is near-vacuous" + ); + assert!(dirty >= 6, "only {dirty} dirty entries"); +} + +// --------------------------------------------------------------------------- +// The fact-table inventory. +// +// Every receiver-keyed fact table on `FnCtx`, transcribed from the code with +// its declaration site. This is the model's contact with reality: the four +// fixtures above are hand-picked, and a model that only agrees with its own +// examples proves nothing. +// +// The `unwind_safe_by` column is the one that matters. Read down it and the +// #9254 thesis is visible without argument: nine tables, six different reasons +// none of which is a boundary the table itself states. +// --------------------------------------------------------------------------- + +struct TableRow { + table: &'static str, + claim: ReceiverClaim, + boundary: FactBoundary, + /// Whether the tier structurally keeps `Stmt::Try` out of the extent in + /// which the fact is live — by an explicit match arm, by a body shape that + /// cannot contain one, or by a verified call-free clone. + excludes_try: bool, + /// How unwind safety is actually obtained today, in the code's own terms. + unwind_safe_by: &'static str, +} + +fn inventory() -> Vec { + vec![ + TableRow { + table: "cached_lengths", + claim: ReceiverClaim::ScalarRelation, + boundary: FactBoundary::DynamicExtent, + excludes_try: false, + unwind_safe_by: "a length is a value; relocation moves the object, not the number", + }, + TableRow { + table: "bounded_index_pairs", + claim: ReceiverClaim::ScalarRelation, + boundary: FactBoundary::ScopeId, + excludes_try: false, + unwind_safe_by: "arithmetic relation; admission walkers descend into Try", + }, + TableRow { + table: "bounded_buffer_index_pairs", + claim: ReceiverClaim::ScalarRelation, + boundary: FactBoundary::ScopeId, + excludes_try: false, + unwind_safe_by: "arithmetic relation", + }, + TableRow { + table: "guarded_buffer_index_pairs", + claim: ReceiverClaim::ScalarRelation, + boundary: FactBoundary::ScopeId, + excludes_try: false, + unwind_safe_by: "arithmetic relation carried by a dominating guard", + }, + TableRow { + table: "int_range_facts", + claim: ReceiverClaim::ScalarRelation, + boundary: FactBoundary::ScopeId, + excludes_try: false, + unwind_safe_by: "producer rejects every writer (stmts_mutate_local walks Try); \ + lexical-order invalidation precedes catch lowering", + }, + TableRow { + table: "packed_f64_loop_facts", + claim: ReceiverClaim::Representation, + boundary: FactBoundary::ScopeId, + excludes_try: true, + unwind_safe_by: "matcher rejects Stmt::Try outright (stmt/loops.rs); Throw admitted \ + only with the #9185 accumulator flush at the throw site", + }, + TableRow { + table: "masked_window_array_facts", + claim: ReceiverClaim::Representation, + boundary: FactBoundary::ScopeId, + excludes_try: true, + unwind_safe_by: "region admits only scalar statements; ctx.try_depth gates \ + privatization", + }, + TableRow { + table: "string_window_array_facts", + claim: ReceiverClaim::Representation, + boundary: FactBoundary::ScopeId, + excludes_try: true, + unwind_safe_by: "body must be a single LocalSet, so no handler can exist in extent", + }, + TableRow { + table: "class_field_loop_facts", + claim: ReceiverClaim::Address, + boundary: FactBoundary::ScopeId, + excludes_try: true, + unwind_safe_by: "single-statement body plus a post-hoc contains_gc_unsafe_call scan \ + that discards the clone if any call was emitted", + }, + TableRow { + table: "element_shape_loop_facts", + claim: ReceiverClaim::Address, + boundary: FactBoundary::ScopeId, + excludes_try: true, + unwind_safe_by: "same double lock as class_field_loop_facts", + }, + TableRow { + table: "packed_receiver_box_slots", + claim: ReceiverClaim::Address, + boundary: FactBoundary::PollRefresh, + excludes_try: true, + unwind_safe_by: "read-only copy whose authority stays in the source root; matcher \ + forbids receiver reassignment", + }, + TableRow { + table: "versioned_indexed_loop_facts", + claim: ReceiverClaim::Address, + boundary: FactBoundary::DynamicExtent, + excludes_try: true, + unwind_safe_by: "the ONE explicit try_depth == 0 gate in codegen, plus per-iteration \ + reload from rooted slots in Fingerprints mode", + }, + TableRow { + table: "stable_packed_loop_facts", + claim: ReceiverClaim::Address, + boundary: FactBoundary::DynamicExtent, + // stmt_flags has a `_ => {}` catch-all, so Stmt::Try is invisible + // to the admission scan and the body tail is unconstrained. + excludes_try: false, + unwind_safe_by: "EMERGENT: call-free post-scan (ordinary mode) or a dirty bit stored \ + before every call/invoke (capture/nested modes)", + }, + TableRow { + table: "buffer_view_slots", + claim: ReceiverClaim::Address, + boundary: FactBoundary::InPlaceDegradation, + excludes_try: false, + unwind_safe_by: "storage kind is non-movable (GC_TYPE_TYPED_ARRAY/BUFFER); the fact \ + is immutable, not bounded", + }, + TableRow { + table: "buffer_data_slots", + claim: ReceiverClaim::Address, + boundary: FactBoundary::Never, + excludes_try: false, + unwind_safe_by: "never-reassigned binding plus non-movable storage; nothing can make \ + the fact false", + }, + TableRow { + table: "class_keys_slots", + claim: ReceiverClaim::Address, + boundary: FactBoundary::Never, + excludes_try: false, + unwind_safe_by: "shadow-slot-bound root the collector rewrites in place; every use \ + reloads per site", + }, + ] +} + +/// The inventory, run through the model. +/// +/// A flag here is NOT a bug report. It says: *this table's stated boundary +/// does not by itself license its claim across an unwind edge* — the safety +/// comes from somewhere the boundary mechanism cannot express, recorded in +/// `unwind_safe_by`. That gap is the thing #9254 proposes to close, and +/// pinning the exact set is how phase 2 proves it closed one. +#[test] +fn the_inventory_flags_exactly_the_tables_whose_unwind_safety_is_external() { + let flagged: Vec<&str> = inventory() + .iter() + .filter(|row| { + let d = ReceiverDescriptor { + table: row.table, + receiver: OBJ, + claim: row.claim, + boundary: row.boundary, + excludes_try: row.excludes_try, + }; + boundary_admits(&d, RegionEnder::UnwindEdge).is_err() + }) + .map(|row| row.table) + .collect(); + + assert_eq!( + flagged, + vec![ + // Emergent, per the survey: `stmt_flags` has a `_ => {}` arm, so + // `Stmt::Try` is invisible to admission and the body tail is + // unconstrained. Safety rests on a post-hoc call-free scan in one + // mode and a before-call dirty bit in the others. + "stable_packed_loop_facts", + // Immutable-fact tables: a cached pointer into storage the GC + // marks non-movable, or a root the collector rewrites in place. + // Sound, but for a reason the boundary vocabulary cannot state — + // which is exactly why phase 2 needs a non-movable-storage + // attribute on the descriptor. + "buffer_view_slots", + "buffer_data_slots", + "class_keys_slots", + ], + "the set of tables relying on out-of-band unwind safety changed; if a tier \ + gained or lost a structural guarantee, update the row AND its unwind_safe_by \ + note with the code that changed" + ); +} + +/// Every scalar-relation table must be clean under every ender — if one is +/// ever flagged, the model has confused a value with an address. +#[test] +fn no_scalar_relation_table_is_ever_flagged() { + for row in inventory() + .iter() + .filter(|r| r.claim == ReceiverClaim::ScalarRelation) + { + let d = ReceiverDescriptor { + table: row.table, + receiver: OBJ, + claim: row.claim, + boundary: row.boundary, + excludes_try: row.excludes_try, + }; + assert!( + violations_for(&d, &ALL_ENDERS).is_empty(), + "{} is a value claim and must survive every relocation point", + row.table + ); + } +} + +/// The inventory has to cover the declarations that exist. A table added to +/// `FnCtx` without a row here is a table the model has never seen. +#[test] +fn the_inventory_covers_every_claim_kind_and_every_boundary_mechanism() { + let rows = inventory(); + for claim in [ + ReceiverClaim::ScalarRelation, + ReceiverClaim::Representation, + ReceiverClaim::Address, + ] { + assert!( + rows.iter().any(|r| r.claim == claim), + "no inventory row exercises {claim:?}" + ); + } + for boundary in [ + FactBoundary::ScopeId, + FactBoundary::DynamicExtent, + FactBoundary::InPlaceDegradation, + FactBoundary::PollRefresh, + FactBoundary::Never, + ] { + assert!( + rows.iter().any(|r| r.boundary == boundary), + "no inventory row exercises {boundary:?}" + ); + } + assert_eq!( + rows.len(), + 16, + "inventory size changed — see FnCtx declarations" + ); +} From 3143ccbf7574a5d2d3c954521d60aa458a78ca5d Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Ralph=20K=C3=BCpper?= Date: Mon, 31 Aug 2026 12:35:00 +0200 Subject: [PATCH 2/3] codegen: read the inventory's unwind_safe_by note; add changelog fragment MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit `-D warnings` caught `unwind_safe_by` as dead: the inventory wrote the note but nothing read it. Silencing it with an allow would have been the wrong fix — that column is the argument the inventory exists to make. It now appears in the flagged-set failure message (so a future change sees WHY each table is flagged, not just that it is), and a new test requires every row to carry a note substantial enough to check against the code later. A note nobody reads is how these go stale unnoticed. Verified with the job's own command: RUSTFLAGS="-D warnings" cargo check -p perry-codegen --all-targets, exit 0. Claude-Session: https://claude.ai/code/session_01Pcq6j6y57TdKSR2Zx2D187 --- changelog.d/9257-receiver-region-model.md | 54 +++++++++++++++++++ .../src/collectors/receiver_regions_tests.rs | 32 ++++++++++- 2 files changed, 85 insertions(+), 1 deletion(-) create mode 100644 changelog.d/9257-receiver-region-model.md diff --git a/changelog.d/9257-receiver-region-model.md b/changelog.d/9257-receiver-region-model.md new file mode 100644 index 0000000000..8f27fae9cf --- /dev/null +++ b/changelog.d/9257-receiver-region-model.md @@ -0,0 +1,54 @@ +**A single vocabulary for "receiver R has fact F, valid until boundary B"** (#9254 +phase 1). Codegen carries sixteen receiver-keyed fact tables on `FnCtx` — +`cached_lengths`, `bounded_index_pairs`, `packed_f64_loop_facts`, +`masked_window_array_facts`, `buffer_view_slots`, `int_range_facts` and ten more. +Each answers the same two questions (what do we know about this receiver, and how +long may we believe it), and each answers the second one differently: a +`retain(|f| f.scope_id != id)` at scope exit, an insert/remove pair with no +identifier, a mutable field downgraded in place and never removed, or a reload at +the safepoint instead of an invalidation. + +A fifth boundary — the **unwind edge** — is expressed by none of them. `lower_try` +clears no fact table. Unwind safety is obtained today by six unrelated means: the +packed matcher rejecting `Stmt::Try` outright, a body shape that cannot contain +one, a post-hoc `contains_gc_unsafe_call` scan, a single `try_depth == 0` gate in +`versioned_indexed_loop`, a dirty bit stored before every call, and a storage kind +that simply never moves. Every one is a local decision by one tier, and a tier +added tomorrow inherits none of them. + +This adds the model — `RegionEnder` (what ends a no-relocation region), +`FactBoundary` (how a table expresses extent), `ReceiverClaim` (value vs +representation vs address, the axis that decides whether a boundary is +load-bearing at all) and `boundary_admits`, the algebra in one place. **It emits +no IR and no lowering path consults it**; the `#![allow(dead_code)]` at the top of +the module is the marker for that, matching the #854 subgraphs in `hir_facts`, and +the whole thing is revertible by deleting the file. + +The load-bearing artifact is the equivalence lint, which holds the model against +`loop_purity::loop_may_allocate` — shipping and audited — on a shared battery, +asserting the direction with teeth: *if the model finds no relocation point, then +`loop_may_allocate` must also have proven the body alloc-free.* The converse is +deliberately not asserted, since `loop_may_allocate` answers `true` for any +statement it does not model (`Return`, `Switch`), which is imprecision rather than +a collection point. + +That lint paid for itself before this landed. Written the obvious way — enumerate +the enders, default to safe — the model passed every hand-written test and failed +the battery on three entries: a generic `IndexGet`/`PropertyGet` can reach an +accessor or a proxy trap, `Expr::Closure` allocates, and `is_inert` belongs on the +whole coercing node rather than per operand (the #6975 hole one abstraction up). +Inverting the match to an allowlist with an `Unmodelled` catch-all closes that +class: adding an HIR variant can no longer silently widen a region. + +Note one deliberate divergence from `collectors::safepoint_sites`, which does not +count property reads — over-counting reads would over-spill a read-heavy loop, and +its consumer only needs a spill estimate. A region model has the opposite +obligation: missing one licenses a stale cached address. + +All sixteen tables are also transcribed as test data with their declaration sites, +pinning the exact set whose unwind safety is *external to its stated boundary*: +`stable_packed_loop_facts` (emergent — `stmt_flags` has a `_ => {}` arm, so +`Stmt::Try` is invisible to its admission scan) plus the three immutable-fact +tables that lean on non-movable storage. A flag there is not a bug report; it says +the safety comes from somewhere the boundary vocabulary cannot express, which is +what phase 2 has to fix, and pinning the set is how phase 2 proves it closed one. diff --git a/crates/perry-codegen/src/collectors/receiver_regions_tests.rs b/crates/perry-codegen/src/collectors/receiver_regions_tests.rs index d23abb0f41..d482cec59d 100644 --- a/crates/perry-codegen/src/collectors/receiver_regions_tests.rs +++ b/crates/perry-codegen/src/collectors/receiver_regions_tests.rs @@ -769,6 +769,12 @@ fn the_inventory_flags_exactly_the_tables_whose_unwind_safety_is_external() { .map(|row| row.table) .collect(); + let with_reasons: Vec = inventory() + .iter() + .filter(|row| flagged.contains(&row.table)) + .map(|row| format!("{} <- {}", row.table, row.unwind_safe_by)) + .collect(); + assert_eq!( flagged, vec![ @@ -788,10 +794,34 @@ fn the_inventory_flags_exactly_the_tables_whose_unwind_safety_is_external() { ], "the set of tables relying on out-of-band unwind safety changed; if a tier \ gained or lost a structural guarantee, update the row AND its unwind_safe_by \ - note with the code that changed" + note with the code that changed.\nCurrent reasons:\n {}", + with_reasons.join("\n ") ); } +/// Every row must say how its unwind safety is obtained, and a flagged row's +/// note is the whole point of the flag — it names the mechanism that lives +/// outside the boundary vocabulary. An empty note would make the inventory a +/// list of names instead of an argument. +#[test] +fn every_inventory_row_explains_how_its_unwind_safety_is_obtained() { + for row in inventory() { + assert!( + !row.unwind_safe_by.trim().is_empty(), + "{} has no unwind_safe_by note", + row.table + ); + // A claim that names no mechanism cannot be checked against the code + // later, which is how these notes go stale without anyone noticing. + assert!( + row.unwind_safe_by.len() > 20, + "{}'s note is too terse to check against the code: {:?}", + row.table, + row.unwind_safe_by + ); + } +} + /// Every scalar-relation table must be clean under every ender — if one is /// ever flagged, the model has confused a value with an address. #[test] From 790f12cb22671ec050c1140ff4abeb0fc7eb3879 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Ralph=20K=C3=BCpper?= Date: Mon, 31 Aug 2026 15:34:58 +0200 Subject: [PATCH 3/3] test(codegen): make the bounded_buffer_index_pairs unwind note checkable MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit The inventory lint requires a note that names a mechanism; this row said only "arithmetic relation" (19 chars, under the >20 bar) so the PR failed its own test. The row holds local ids, a scope id, a width and a BoundsProof — no pointer, so relocation cannot invalidate it — and excludes_try: false is sound because stmts_mutate_local's Try arm descends into body, catch and finally. --- crates/perry-codegen/src/collectors/receiver_regions_tests.rs | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-) diff --git a/crates/perry-codegen/src/collectors/receiver_regions_tests.rs b/crates/perry-codegen/src/collectors/receiver_regions_tests.rs index d482cec59d..1e6d6da516 100644 --- a/crates/perry-codegen/src/collectors/receiver_regions_tests.rs +++ b/crates/perry-codegen/src/collectors/receiver_regions_tests.rs @@ -637,7 +637,8 @@ fn inventory() -> Vec { claim: ReceiverClaim::ScalarRelation, boundary: FactBoundary::ScopeId, excludes_try: false, - unwind_safe_by: "arithmetic relation", + unwind_safe_by: "arithmetic relation over local ids carrying an explicit \ + BoundsProof; admission walkers descend into Try", }, TableRow { table: "guarded_buffer_index_pairs",