diff --git a/majit/majit-metainterp/src/pyjitpl.rs b/majit/majit-metainterp/src/pyjitpl.rs index 64c785c7684..abc34b779d4 100644 --- a/majit/majit-metainterp/src/pyjitpl.rs +++ b/majit/majit-metainterp/src/pyjitpl.rs @@ -514,7 +514,10 @@ fn snapshot_map_from_trace_snapshots( let mut size_map = Vec::new(); let mut vable_map = Vec::new(); let mut vref_map = Vec::new(); - let mut pc_map = Vec::new(); + // Not `pc_map`: that name belongs to the `-live-` marker table keyed by + // Python pc (`pc_map[py_pc]`, `pyre-jit/src/jit/codewriter.rs`). This is + // keyed by snapshot id and holds one `(jitcode_index, pc, py_pc)` per frame. + let mut frame_pcs_map = Vec::new(); // opencoder.py:603 _encode: trace snapshot recorder only emits Box // (live deadframe slot) and Const (compile-time pool) payloads. // TAGVIRTUAL belongs to resume numbering (resume.py:_number_boxes) @@ -575,9 +578,9 @@ fn snapshot_map_from_trace_snapshots( snapshot_insert(&mut size_map, id, frame_sizes); snapshot_insert(&mut vable_map, id, vable_boxes); snapshot_insert(&mut vref_map, id, vref_boxes); - snapshot_insert(&mut pc_map, id, frame_pcs); + snapshot_insert(&mut frame_pcs_map, id, frame_pcs); } - (box_map, size_map, vable_map, vref_map, pc_map) + (box_map, size_map, vable_map, vref_map, frame_pcs_map) } struct PreparedBridgeTrace { @@ -6025,7 +6028,7 @@ impl MetaInterp { snapshot_frame_size_map, mut snapshot_vable_map, mut snapshot_vref_map, - snapshot_pc_map, + snapshot_frame_pcs, ) = snapshot_map_from_trace_snapshots(&trace_snapshots, &mut constants); // history.py:220/261/307 — `Const{Int,Float,Ptr}.type` is an // intrinsic attribute on the Box itself, so no raw-u32 type @@ -6035,7 +6038,7 @@ impl MetaInterp { unroll_opt.snapshot_frame_sizes = snapshot_frame_size_map.clone(); unroll_opt.snapshot_vable_boxes = snapshot_vable_map.clone(); unroll_opt.snapshot_vref_boxes = snapshot_vref_map.clone(); - unroll_opt.snapshot_frame_pcs = snapshot_pc_map.clone(); + unroll_opt.snapshot_frame_pcs = snapshot_frame_pcs.clone(); // The original snapshot maps are re-cloned into `simple_opt` on the // InvalidLoop retry below, so they must stay rooted across the WHOLE // unroll. Each phase's `replace_compile_snapshot_roots` overwrites the @@ -6169,7 +6172,7 @@ impl MetaInterp { simple_opt.snapshot_frame_sizes = snapshot_frame_size_map; simple_opt.snapshot_vable_boxes = snapshot_vable_map; simple_opt.snapshot_vref_boxes = snapshot_vref_map; - simple_opt.snapshot_frame_pcs = snapshot_pc_map; + simple_opt.snapshot_frame_pcs = snapshot_frame_pcs; simple_opt.call_pure_results = call_pure_results.clone(); // Forward the recorder's operand pool — the retry path // uses the same upstream `Rc` allocations from @@ -8561,7 +8564,7 @@ impl MetaInterp { snapshot_frame_size_map, mut snapshot_vable_map, mut snapshot_vref_map, - snapshot_pc_map, + snapshot_frame_pcs, ) = snapshot_map_from_trace_snapshots(&trace_snapshots, &mut constants); self.compile_snapshot_refs = collect_snapshot_const_ptr_slots(&mut [ &mut snapshot_map, @@ -8580,7 +8583,7 @@ impl MetaInterp { optimizer.snapshot_frame_sizes = snapshot_frame_size_map; optimizer.snapshot_vable_boxes = snapshot_vable_map; optimizer.snapshot_vref_boxes = snapshot_vref_map; - optimizer.snapshot_frame_pcs = snapshot_pc_map; + optimizer.snapshot_frame_pcs = snapshot_frame_pcs; // InvalidLoop during optimization should abort the trace, not crash // the process. Matches compile_loop. @@ -9003,7 +9006,7 @@ impl MetaInterp { snapshot_frame_size_map, mut snapshot_vable_map, mut snapshot_vref_map, - snapshot_pc_map, + snapshot_frame_pcs, ) = snapshot_map_from_trace_snapshots(&trace_snapshots, &mut constants); self.compile_snapshot_refs = collect_snapshot_const_ptr_slots(&mut [ &mut snapshot_map, @@ -9014,7 +9017,7 @@ impl MetaInterp { optimizer.snapshot_frame_sizes = snapshot_frame_size_map; optimizer.snapshot_vable_boxes = snapshot_vable_map; optimizer.snapshot_vref_boxes = snapshot_vref_map; - optimizer.snapshot_frame_pcs = snapshot_pc_map; + optimizer.snapshot_frame_pcs = snapshot_frame_pcs; let optimize_start = Instant::now(); let optimize_result = optimizer.optimize_with_constants_and_inputs_oprc( diff --git a/pyre/gate-triage.md b/pyre/gate-triage.md index e8180b7fcd0..763b6f4a285 100644 --- a/pyre/gate-triage.md +++ b/pyre/gate-triage.md @@ -237,7 +237,7 @@ program point is not a safepoint, and the same bench is clean under the real allocation-driven stress. Force collections through the GC's own stress hook, never at a hand-picked instruction. -## §1c — Retired since the 2026-07-05 audit (10): reader already deleted by a closed epic +## §1c — Retired since the 2026-07-05 audit (11): reader already deleted by a closed epic Book-keeping only: these OFF-paths were deleted in source by the cited epics after the 2026-07-05 audit; this pass removes their stale registry rows. The @@ -255,6 +255,7 @@ after the 2026-07-05 audit; this pass removes their stale registry rows. The | PYRE_P2_FS_COMPILE | PR#374 (`9a97c47f6e9`) | stale §5 deferred entry removed | | PYRE_P2_AUTHORITATIVE | reader gone; attribution #374 per re-audit | stale §5 deferred entry removed | | PYRE_SAME_GREENKEY | PR#390 (`802b79ff8db`); follow-up `111bdb4eeb8` dropped the gate | stale §1b deferred mention and §5 list entry removed | +| PYRE_FBW_REC_UNROLL | PR#374 (`9a97c47f6e9`) deleted `fbw_unroll_bound()` | stale §5 config-switch entry removed 2026-08-08. The successor knob `PYRE_FBW_REC_UNROLL_DEPTH` was never listed here and its reader `fbw_max_rec_unroll_depth()` is gone too (PR#887, `e5546b2ed36`) — both names read from nothing | ## §1d — Parity verdicts for the default-OFF `PYRE_FBW_*` seams (2026-07-25) @@ -770,7 +771,7 @@ residual level under the walked frame and an inlined level under that, so the force has to reach two frames up. Per-frame vable binding, outer-locals materialization, and the `jit.virtual_ref` emit are therefore validatable now. -## §2 — Not gates (11): Rust identifiers, not env vars +## §2 — Not gates (12): Rust identifiers, not env vars The audit regex matched non-env identifiers. These are real code; **do not delete, do not count as gates.** @@ -778,6 +779,9 @@ delete, do not count as gates.** - `PYRE_STR_DESCR`, `PYRE_STR_BYTE_LEN_DESCR`, `PYRE_UNICODE_DESCR`, `PYRE_UNICODE_LEN_DESCR` — field-descriptor `const`s (`pyre-jit-trace/src/pyre_cpu.rs`) - `PYRE_CLASS_DESCRIPTOR` — macro-built identifier `W_{}_PYRE_CLASS_DESCRIPTOR` (`pyre-macros`) +- `PYRE_CLASS_DESCRIPTORS` — the whole-program `linkme` distributed slice the + macro registers into (`pyre-object/src/lltype.rs`); the only `PYRE_*` name that + appears outside Rust and Python source, and it is not an env var - `PYRE_PARAM_NAMES`, `PYRE_PARAM_REQUIRED` — macro `const __PYRE_PARAM_*` (`pyre-macros`) - `PYRE_JIT_GRAPH_MODULES` — compile-time `const &[&str]` module manifest (`generated.rs`) - `PYRE_REF_OPAQUE` — `OpaqueType::gc("PYRE_REF_OPAQUE")` type label (`annotator/builtin.rs`) @@ -875,12 +879,12 @@ Kept as-is; listed for completeness. the mirror is seeded and then read by nobody. Definition of done before re-evaluating: make the maintenance sites read the ACTIVE callee jitcode metadata (what the gate doc already asks for), then land a consumer. -- **Config / value / master switches (~18)** — tuning, paths, modes; keep: - `PYRE_FBW_REC_UNROLL`, `PYRE_WALKER_STORE_SUBSCR_FNADDR`, +- **Config / value / master switches (~17)** — tuning, paths, modes; keep: + `PYRE_WALKER_STORE_SUBSCR_FNADDR`, `PYRE_MIR_FRONTEND_LLBC`, `PYRE_WASM_ENGINE`, `_FUEL`, `_MODULE`, `_NO_CACHE`, `PYRE_GC_INTERP`, `PYRE_JIT`, `PYRE_NO_JIT`, `PYRE_STDLIB`, `PYRE_CHECK_PYPY3`, `PYRE_CHECK_PYTHON3`, `PYRE_SANDBOX_NO_SECCOMP`, - `PYRE_SHARED_BUILD`, `PYRE_SYNTH_PYPY`, `_PYRE`, `_PYTHON`. + `PYRE_SHARED_BUILD`, `PYRE_SYNTH_PYPY`, `PYRE_SYNTH_PYRE`, `PYRE_SYNTH_PYTHON`. - **Test harness (1)**: `PYRE_MIR_STRESS_LLBC`. ## §6 — The 66 gates the audits never listed (2026-08-07) @@ -893,22 +897,54 @@ with no entry here fails `cargo test`. The counts to quote, distinguished: | count | value | |---|---| -| distinct names read from the environment | **105** | -| (file, name) read pairs | 128 | +| distinct names read from the environment | **111** | +| — of those, read from Rust | 105 | +| — read only from the harness Python | 6 | +| (file, name) read pairs | 137 | | **live gates that were absent from this file** | **66** | -| names here with no read site left (retire) | 51 | +| names still listed live with no read site left (retire) | 0 | ```sh -git ls-files '*.rs' | xargs rg --no-filename -o \ - '(env::var[_a-z]*|host_os::var|getenv)\(b?"(PYRE_[A-Z0-9_]+)"' -r '$2' | sort -u +{ git ls-files '*.rs'; git ls-files 'pyre/**/*.py' 'scripts/*.py'; } \ + | xargs rg --no-filename -o \ + '(env::var[_a-z]*|host_os::var|getenv|environ\.get)\(b?"(PYRE_[A-Z0-9_]+)"' \ + -r '$2' | sort -u ``` `--no-filename` is what makes this count gates: without it rg prefixes each hit -and `sort -u` counts (file, name) pairs instead. The two seam forms matter for -the same reason — `host_os::var` and `host_seam::ops::getenv` (a *byte* string) -are how `importing.rs` reads `PYRE_STDLIB`, and a `std::env` search alone would -miss a sandbox- or wasm-only gate entirely. Neither seam form adds a name here; -both are `PYRE_STDLIB`, already read through `env::var` in `pyre-wasm-runner`. +and `sort -u` counts (file, name) pairs instead. Each read form is here because +something was hiding behind it: + +- `host_os::var` and `host_seam::ops::getenv` (a **byte** string) are how + `importing.rs` reads `PYRE_STDLIB`. Neither adds a name — that gate is also + read through `env::var` in `pyre-wasm-runner` — but a sandbox- or wasm-only + gate would have had no such cover. +- `environ.get` and `getenv` are the harness. Six gates are read from + `check.py`, `check_synthetic.py`, the `extra_tests` runners and + `scripts/llbc_extract.py` and from no Rust file at all, so every `*.rs` + census — including this section's first draft — missed all six. + +Only unambiguous reads count. The harness also *writes* into a child's +environment (`env[…] = …`, `env.pop(…)`), and writing a gate for a child is not +owning it: the child's read is what this file is about. A subscript cannot be +told from a read without parsing, and naming a fixture variable here would enter +it in the census as a documented gate — which is why the example above has no +name in it. + +**Spell every name in full at least once.** This file abbreviates runs of related +gates (`PYRE_WASM_ENGINE`, `_FUEL`, `_MODULE`), and the brake matches whole +tokens, so a name appearing *only* in that shorthand reads as undocumented. +`PYRE_SYNTH_PYRE` and `PYRE_SYNTH_PYTHON` were written `_PYRE`, `_PYTHON`, and +were the only two the widened census reported missing — they had been documented +all along. The shorthand is fine beside a full spelling; it is not fine alone. + +**A retirement row documents nothing, wherever it sits.** §1/§1b/§1c/§2/§3 are +history sections and no name in them counts. But §1d's heading reads *Parity +verdicts*, so that section reads live while its table marks +`PYRE_FBW_VABLE_SCALAR_CA` **RETIRED** — a mixed section, which section +granularity cannot express. So any row that says "retired" is skipped too, and +re-introducing a reader for a retired gate fails the brake rather than passing on +the strength of its own obituary. Polarity below follows this file's rule, with one correction it needed: an `is_none()` whose value *is* the enable flag means default **ON**, but an @@ -962,11 +998,11 @@ already-ON criterion. They are listed so they cannot be missed again. | bucket | count | |---|---| -| retired (§1 + §1b + §1c + §1d parity pass) | 5 + 4 + 10 + 1 | -| not gates (identifiers) | 11 | +| retired (§1 + §1b + §1c + §1d parity pass) | 5 + 4 + 11 + 1 | +| not gates (identifiers) | 12 | | dead (no read site) | 10 | | live default-ON, kept until epic closes | 9 (+ `PYRE_GC_INTERP`, wasm32-only) | | diagnostics (OFF) | ~34 | | default-OFF experiments (all keep — adoption targets) | 3 | -| config / value / master | ~18 | +| config / value / master | ~17 | | test harness | 1 | diff --git a/pyre/pyre-jit/src/jit/codewriter.rs b/pyre/pyre-jit/src/jit/codewriter.rs index 02abede0347..ee442aceb1d 100644 --- a/pyre/pyre-jit/src/jit/codewriter.rs +++ b/pyre/pyre-jit/src/jit/codewriter.rs @@ -13181,8 +13181,143 @@ impl CodeWriter { push_and_bump!(result_value.into(), py_pc); } - // Catch-all: unknown instruction. - _other => { + // No catch-all. The match is exhaustive over `Instruction`, so a + // variant added upstream fails to compile here instead of + // declining the loop with no record of which opcode it was. The + // three groups below are the opcodes this compiler cannot emit; + // each still declines, and each now says which one it was. + + // ---- Adaptive specializations ---- + // A specializing interpreter rewrites the generic opcode in place + // in the code object; the compiler never emits one. pyre's eval + // loop does not quicken, so the walk cannot meet one. + // `deoptimize()` is deliberately not called on them: a stream + // carrying a specialization this interpreter never wrote is + // corrupt, not something to fold back. A walk is no place to + // raise, so the answer is the same permanent decline the unported + // opcodes take. + Instruction::BinaryOpAddFloat + | Instruction::BinaryOpAddInt + | Instruction::BinaryOpAddUnicode + | Instruction::BinaryOpExtend + | Instruction::BinaryOpInplaceAddUnicode + | Instruction::BinaryOpMultiplyFloat + | Instruction::BinaryOpMultiplyInt + | Instruction::BinaryOpSubscrDict + | Instruction::BinaryOpSubscrGetitem + | Instruction::BinaryOpSubscrListInt + | Instruction::BinaryOpSubscrListSlice + | Instruction::BinaryOpSubscrStrInt + | Instruction::BinaryOpSubscrTupleInt + | Instruction::BinaryOpSubtractFloat + | Instruction::BinaryOpSubtractInt + | Instruction::CallAllocAndEnterInit + | Instruction::CallBoundMethodExactArgs + | Instruction::CallBoundMethodGeneral + | Instruction::CallBuiltinClass + | Instruction::CallBuiltinFast + | Instruction::CallBuiltinFastWithKeywords + | Instruction::CallBuiltinO + | Instruction::CallIsinstance + | Instruction::CallKwBoundMethod + | Instruction::CallKwNonPy + | Instruction::CallKwPy + | Instruction::CallLen + | Instruction::CallListAppend + | Instruction::CallMethodDescriptorFast + | Instruction::CallMethodDescriptorFastWithKeywords + | Instruction::CallMethodDescriptorNoargs + | Instruction::CallMethodDescriptorO + | Instruction::CallNonPyGeneral + | Instruction::CallPyExactArgs + | Instruction::CallPyGeneral + | Instruction::CallStr1 + | Instruction::CallTuple1 + | Instruction::CallType1 + | Instruction::CompareOpFloat + | Instruction::CompareOpInt + | Instruction::CompareOpStr + | Instruction::ContainsOpDict + | Instruction::ContainsOpSet + | Instruction::ForIterGen + | Instruction::ForIterList + | Instruction::ForIterRange + | Instruction::ForIterTuple + | Instruction::LoadAttrClass + | Instruction::LoadAttrClassWithMetaclassCheck + | Instruction::LoadAttrGetattributeOverridden + | Instruction::LoadAttrInstanceValue + | Instruction::LoadAttrMethodLazyDict + | Instruction::LoadAttrMethodNoDict + | Instruction::LoadAttrMethodWithValues + | Instruction::LoadAttrModule + | Instruction::LoadAttrNondescriptorNoDict + | Instruction::LoadAttrNondescriptorWithValues + | Instruction::LoadAttrProperty + | Instruction::LoadAttrSlot + | Instruction::LoadAttrWithHint + | Instruction::LoadConstImmortal + | Instruction::LoadConstMortal + | Instruction::LoadGlobalBuiltin + | Instruction::LoadGlobalModule + | Instruction::LoadSuperAttrAttr + | Instruction::LoadSuperAttrMethod + | Instruction::ResumeCheck + | Instruction::SendGen + | Instruction::StoreAttrInstanceValue + | Instruction::StoreAttrSlot + | Instruction::StoreAttrWithHint + | Instruction::StoreSubscrDict + | Instruction::StoreSubscrListInt + | Instruction::ToBoolAlwaysTrue + | Instruction::ToBoolBool + | Instruction::ToBoolInt + | Instruction::ToBoolList + | Instruction::ToBoolNone + | Instruction::ToBoolStr + | Instruction::UnpackSequenceList + | Instruction::UnpackSequenceTuple + | Instruction::UnpackSequenceTwoTuple => { + emit_abort_permanent!(py_pc); + } + + // ---- sys.monitoring instrumentation ---- + // Substituted at runtime while a monitoring tool is attached, and + // never produced by the static compiler. Nothing in the tree + // writes one. + Instruction::InstrumentedCall + | Instruction::InstrumentedCallFunctionEx + | Instruction::InstrumentedCallKw + | Instruction::InstrumentedEndAsyncFor + | Instruction::InstrumentedEndFor + | Instruction::InstrumentedEndSend + | Instruction::InstrumentedForIter + | Instruction::InstrumentedInstruction + | Instruction::InstrumentedJumpBackward + | Instruction::InstrumentedJumpForward + | Instruction::InstrumentedLine + | Instruction::InstrumentedLoadSuperAttr + | Instruction::InstrumentedNotTaken + | Instruction::InstrumentedPopIter + | Instruction::InstrumentedPopJumpIfFalse + | Instruction::InstrumentedPopJumpIfNone + | Instruction::InstrumentedPopJumpIfNotNone + | Instruction::InstrumentedPopJumpIfTrue + | Instruction::InstrumentedResume + | Instruction::InstrumentedReturnValue + | Instruction::InstrumentedYieldValue => { + emit_abort_permanent!(py_pc); + } + + // ---- Interpreter- and JIT-internal ---- + // `EnterExecutor`, `InterpreterExit` and the two `JumpBackward` + // forms belong to a tier-2 executor this interpreter does not + // have; `Reserved` is a hole in the opcode table. + Instruction::EnterExecutor + | Instruction::InterpreterExit + | Instruction::JumpBackwardJit + | Instruction::JumpBackwardNoJit + | Instruction::Reserved => { emit_abort_permanent!(py_pc); } } diff --git a/pyre/pyrex/tests/gate_triage_complete.rs b/pyre/pyrex/tests/gate_triage_complete.rs index 7ac3bf9269f..c0565d59669 100644 --- a/pyre/pyrex/tests/gate_triage_complete.rs +++ b/pyre/pyrex/tests/gate_triage_complete.rs @@ -1,5 +1,6 @@ -//! Every `PYRE_*` environment gate read from a workspace member's Rust source -//! must have a live entry in `pyre/gate-triage.md`. +//! `pyre/gate-triage.md` and the tree's `PYRE_*` environment gates must name the +//! same set — every gate read from a workspace member's Rust or from this +//! project's own Python has a live entry, and every live entry has a reader. //! //! The charter (§3.6) says a gate is a staging area, not a home, and //! `gate-triage.md` is the standing list of what to retire and when. That list @@ -10,11 +11,24 @@ //! Adding a gate therefore costs one row. The row is cheap; the alternative is //! another hand audit that goes stale the week after it lands. //! -//! **Scope: Rust only.** `PYRE_CHECK_PYPY3`, `PYRE_CHECK_PYTHON3`, -//! `PYRE_SHARED_BUILD` and `PYRE_SYNTH_PYPY` are live gates read from `check.py`, -//! `check_synthetic.py`, the CI workflows and `scripts/llbc_extract.py`, and -//! nothing here sees them. A Python- or YAML-only gate can still enter -//! undocumented; whether to widen this scan is tracked as F5 in `rework.md`. +//! Both directions, because they rot differently and only one of them is loud. A +//! gate with no row is a gate nobody will retire. A row with no reader is the +//! quieter half: it survives the deletion of the code it describes, and the next +//! sweep spends its time re-deriving that the name is already gone — +//! `PYRE_FBW_REC_UNROLL` sat in the config-switch list from 2026-07-06, when +//! PR#374 deleted `fbw_unroll_bound()`, until this test went looking. +//! +//! **Scope.** Rust sources in workspace member crates, and this project's own +//! Python under `pyre/` and `scripts/`. The harness reads six gates that no Rust +//! file reads — `check.py`, `check_synthetic.py`, the `extra_tests` runners and +//! `scripts/llbc_extract.py` — and two of them, `PYRE_SYNTH_PYRE` and +//! `PYRE_SYNTH_PYTHON`, were undocumented until this scan reached them. +//! +//! Shell and YAML are not scanned. Every `PYRE_*` in them is *set* for a child +//! process whose reader is Rust or Python, so the read side is already covered +//! here; the one remaining `PYRE_*` name outside both languages, +//! `PYRE_CLASS_DESCRIPTORS`, is a linkme distributed slice and not an env var at +//! all (`gate-triage.md` §2). use std::collections::BTreeSet; use std::path::{Path, PathBuf}; @@ -60,7 +74,16 @@ fn workspace_member_dirs(root: &Path) -> Vec { dirs } -fn collect_rs(dir: &Path, out: &mut Vec) { +/// Where this project's own Python lives. +/// +/// An allow-list rather than a tree walk, for two reasons the Rust side does not +/// have. The repository vendors the CPython and PyPy sources — `lib-python/`, +/// `pypy/`, `rpython/`, `lib_pypy/`, some 4600 `.py` files that are upstream's +/// and not ours — and untracked scratch directories sit at the repository root. +/// Neither is under these two. +const PYTHON_ROOTS: [&str; 2] = ["pyre", "scripts"]; + +fn collect_sources(dir: &Path, ext: &str, out: &mut Vec) { let Ok(entries) = std::fs::read_dir(dir) else { return; }; @@ -69,9 +92,9 @@ fn collect_rs(dir: &Path, out: &mut Vec) { if path.is_dir() { // A member crate's own `target/` from a standalone build. if entry.file_name() != "target" { - collect_rs(&path, out); + collect_sources(&path, ext, out); } - } else if path.extension().is_some_and(|e| e == "rs") { + } else if path.extension().is_some_and(|e| e == ext) { out.push(path); } } @@ -86,7 +109,16 @@ fn collect_rs(dir: &Path, out: &mut Vec) { /// visible to a `std::env` search only because unrelated `env::var` readers /// exist in `pyre-wasm-runner`. A wasm- or sandbox-only gate would have no such /// cover and would bypass the brake entirely. -const READ_FORMS: [&str; 3] = ["env::var", "host_os::var", "getenv"]; +/// +/// `environ.get` and `getenv` are the Python side. Only these two, because they +/// read unambiguously: the harness also writes gates into a child's environment +/// (`env["PYRE_PROBE"] = …`, `env.pop(…)`), and a subscript form cannot be told +/// from a read without parsing. Writing a gate for a child is not owning it — +/// the child's read is what this file asks about, and that read is in Rust. +/// +/// A form belonging to the other language costs nothing: `env::var(` cannot +/// occur in Python, nor `environ.get(` in Rust. +const READ_FORMS: [&str; 4] = ["env::var", "host_os::var", "getenv", "environ.get"]; /// Gate names this text reads from the environment. /// @@ -129,6 +161,10 @@ fn gates_read_by_matches_the_read_forms_and_nothing_else() { ); host_os::var("PYRE_D").ok(); crate::host_seam::ops::getenv(b"PYRE_E"); + os.environ.get("PYRE_F") + os.getenv("PYRE_G") + # a gate written into a child's environment is not a read of ours + env["PYRE_NOT_A_READ_EITHER"] = "1" // PYRE_MENTIONED_IN_A_COMMENT const PYRE_CONST: &str = "PYRE_NOT_A_READ"; other::var("PYRE_NOT_ENV"); @@ -137,7 +173,12 @@ fn gates_read_by_matches_the_read_forms_and_nothing_else() { let mut got = gates_read_by(sample); // Sorted: the scan groups by read form, so the order carries no meaning. got.sort_unstable(); - assert_eq!(got, vec!["PYRE_A", "PYRE_B", "PYRE_C", "PYRE_D", "PYRE_E"]); + assert_eq!( + got, + vec![ + "PYRE_A", "PYRE_B", "PYRE_C", "PYRE_D", "PYRE_E", "PYRE_F", "PYRE_G" + ] + ); } /// Does this `##` heading introduce a section that records history? @@ -155,39 +196,131 @@ fn is_history_heading(heading: &str) -> bool { lower.contains("retired") || lower.contains("dead") || lower.contains("not gates") } -/// Every `PYRE_*` token the triage document lists as a live gate. +/// The `PYRE_*` names this line spells out, in the order it spells them. /// /// Tokenized rather than substring-searched: `contains("PYRE_A")` is satisfied /// by a documented `PYRE_ANCHOR_STRICT`, so a new gate whose name is a prefix of -/// a listed one would slip through the brake unnoticed. Scoped to the live -/// sections for the reason in `is_history_heading`. `###` subsections inherit -/// their `##` parent, which is what keeps §6a–§6c live under §6. +/// a listed one would slip through the brake unnoticed. +fn pyre_names_in(line: &str) -> Vec<&str> { + let mut names = Vec::new(); + for (at, _) in line.match_indices("PYRE_") { + // A name preceded by a name character is the tail of a longer token. + if line[..at] + .chars() + .next_back() + .is_some_and(|c| c.is_ascii_alphanumeric() || c == '_') + { + continue; + } + let end = line[at..] + .find(|c: char| !(c.is_ascii_uppercase() || c.is_ascii_digit() || c == '_')) + .map_or(line.len(), |off| at + off); + names.push(&line[at..end]); + } + names +} + +/// Every `PYRE_*` name the triage document lists as a live gate. +/// +/// Scoped to the live sections for the reason in `is_history_heading` — `###` +/// subsections inherit their `##` parent, which is what keeps §6a–§6c live under +/// §6 — and then filtered by what the document records about each name. +/// +/// **Retirement is a property of the gate, not of the line.** A retirement line +/// retires its *subject*, and a name stays retired wherever else it is written: +/// §5's table records `PYRE_CARRIER_EXC_RESUME` retired with its reader deleted, +/// while §1e's prose — a live section — discusses the seam that gate used to +/// guard and names it three more times. Counting those mentions as documentation +/// made a gate with no reader look live. +/// +/// **Subject, not mention**: only the *first* name on a retirement line is +/// retired by it. §1c's row for `PYRE_AUTHORITATIVE` ends "`PYRE_PROBE_AUTHORITATIVE` +/// is separate and remains live" — the whole point of that clause is that the +/// second name is not the one being retired. +/// +/// "retired", not "retire": §4 is a live section whose gates are the ones that +/// "retire when the epic closes". fn gates_documented_in(triage: &str) -> BTreeSet<&str> { - let mut found = BTreeSet::new(); + let mut live_names: BTreeSet<&str> = BTreeSet::new(); + let mut retired_subjects: BTreeSet<&str> = BTreeSet::new(); let mut live = true; for line in triage.lines() { if let Some(heading) = line.strip_prefix("## ") { live = !is_history_heading(heading); } - if !live { + let names = pyre_names_in(line); + if line.to_ascii_lowercase().contains("retired") { + if let Some(&subject) = names.first() { + retired_subjects.insert(subject); + } + } else if live { + live_names.extend(names); + } + } + live_names.retain(|name| !retired_subjects.contains(name)); + live_names +} + +/// A documented token that is a wildcard's stem rather than a gate. +/// +/// This file writes `PYRE_*` and `PYRE_FBW_*` when it means a family, and the +/// token scan stops at the `*`, leaving a name ending in `_`. No gate is named +/// that way, so the trailing underscore is the whole test. +fn is_wildcard_stem(name: &str) -> bool { + name.ends_with('_') +} + +/// Every source file a gate can be read from. +fn source_files(root: &Path) -> Vec { + let mut sources = Vec::new(); + for member in workspace_member_dirs(root) { + collect_sources(&member, "rs", &mut sources); + } + assert!( + sources.len() > 100, + "found only {} .rs files across the workspace members — the walk is not \ + reaching the tree", + sources.len() + ); + + let rust_files = sources.len(); + for py_root in PYTHON_ROOTS { + let dir = root.join(py_root); + assert!( + dir.is_dir(), + "PYTHON_ROOTS names {py_root}, which is not a directory under {}", + root.display() + ); + collect_sources(&dir, "py", &mut sources); + } + assert!( + sources.len() - rust_files > 100, + "found only {} .py files under {PYTHON_ROOTS:?} — the walk is not \ + reaching the harness", + sources.len() - rust_files + ); + sources +} + +/// `(gate name, repo-relative file)` for every environment read in the tree. +fn read_sites(root: &Path) -> BTreeSet<(String, String)> { + // This file's own fixture spells out `env::var("PYRE_A")` and friends, which + // are test data rather than gates. + let self_path = root.join(file!()); + let mut sites = BTreeSet::new(); + for path in source_files(root) { + if path == self_path { continue; } - for (at, _) in line.match_indices("PYRE_") { - // A name preceded by a name character is the tail of a longer token. - if line[..at] - .chars() - .next_back() - .is_some_and(|c| c.is_ascii_alphanumeric() || c == '_') - { - continue; - } - let end = line[at..] - .find(|c: char| !(c.is_ascii_uppercase() || c.is_ascii_digit() || c == '_')) - .map_or(line.len(), |off| at + off); - found.insert(&line[at..end]); + let Ok(text) = std::fs::read_to_string(&path) else { + continue; + }; + for name in gates_read_by(&text) { + let rel = path.strip_prefix(root).unwrap_or(&path); + sites.insert((name.to_string(), rel.display().to_string())); } } - found + sites } #[test] @@ -207,41 +340,27 @@ fn every_live_pyre_gate_has_a_gate_triage_entry() { site, so it must not count as documented — is_history_heading no longer \ matches gate-triage.md's headings" ); + assert!( + !documented.contains("PYRE_FBW_VABLE_SCALAR_CA"), + "PYRE_FBW_VABLE_SCALAR_CA is marked RETIRED inside §1d, a section whose \ + heading reads live — the per-row retirement check is no longer catching it" + ); + assert!( + !documented.contains("PYRE_CARRIER_EXC_RESUME"), + "PYRE_CARRIER_EXC_RESUME is recorded retired in §5 and its reader is gone, \ + but §1e's live prose names it three times — a retired subject is retired \ + wherever else it is written, and this check is what enforces that" + ); assert!( documented.contains("PYRE_JD1"), "PYRE_JD1 is listed live in §6a but did not count as documented — \ is_history_heading is excluding a live section" ); - let mut sources = Vec::new(); - for member in workspace_member_dirs(&root) { - collect_rs(&member, &mut sources); - } - assert!( - sources.len() > 100, - "found only {} .rs files across the workspace members — the walk is not \ - reaching the tree", - sources.len() - ); - // This file's own fixture spells out `env::var("PYRE_A")` and friends, which - // are test data rather than gates. - let self_path = root.join(file!()); - - let mut missing: BTreeSet<(String, String)> = BTreeSet::new(); - for path in &sources { - if *path == self_path { - continue; - } - let Ok(text) = std::fs::read_to_string(path) else { - continue; - }; - for name in gates_read_by(&text) { - if !documented.contains(name) { - let rel = path.strip_prefix(&root).unwrap_or(path); - missing.insert((name.to_string(), rel.display().to_string())); - } - } - } + let missing: BTreeSet<(String, String)> = read_sites(&root) + .into_iter() + .filter(|(name, _)| !documented.contains(name.as_str())) + .collect(); if !missing.is_empty() { let listed = missing @@ -258,3 +377,35 @@ fn every_live_pyre_gate_has_a_gate_triage_entry() { ); } } + +#[test] +fn every_live_gate_triage_entry_still_has_a_reader() { + let root = repo_root(); + let triage_path = root.join("pyre/gate-triage.md"); + let triage = std::fs::read_to_string(&triage_path) + .unwrap_or_else(|e| panic!("cannot read {}: {e}", triage_path.display())); + + let read: BTreeSet = read_sites(&root) + .into_iter() + .map(|(name, _)| name) + .collect(); + let stale: Vec<&str> = gates_documented_in(&triage) + .into_iter() + .filter(|name| !is_wildcard_stem(name) && !read.contains(*name)) + .collect(); + + assert!( + stale.is_empty(), + "{} name(s) are listed live in pyre/gate-triage.md but nothing in the tree \ + reads them:\n{}\n\n\ + Deleting the row loses why the gate existed. Move each to a retirement \ + section (§1c) naming the change that removed its reader — that is what \ + stops the next reader of this name from passing on an obituary.", + stale.len(), + stale + .iter() + .map(|name| format!(" {name}")) + .collect::>() + .join("\n") + ); +} diff --git a/pyre/rework.md b/pyre/rework.md index 4575d0436fb..d17cd45ad64 100644 --- a/pyre/rework.md +++ b/pyre/rework.md @@ -8,54 +8,59 @@ below: a closed finding leaves a one-line verdict there only where re-deriving i is the live risk. Keep this document small enough that it is worth re-reading. Original audit: branch `pc-map`, 2026-07-05, against the charter's axioms A1–A7 -and norms N1–N7. Re-measured 2026-08-07 on `ec-wiring`. +and norms N1–N7. Re-measured 2026-08-08 on `ec-wiring`. The verdict has not changed shape: **the skeleton is right, the JIT spine is where the violations live.** The layer map of charter §1 is real in the tree and none of the anti-roadmap (§3.5) items have been rebuilt. What changed is the size -of the remainder — two of the original five findings are closed and deleted, and -thirteen of the fifteen tracked issues are closed. +of the remainder — three of the original five findings are closed and deleted, +and thirteen of the fifteen tracked issues are closed. --- ## Open findings -### F4 — one `_other` catch-all is the whole unlisted-cliff surface +### F4 — eight unported opcodes; the unlisted surface is gone -**Counted 2026-08-07.** Earlier revisions of this finding reported "~214 matches, -unchanged in scale" and left it unmeasurable behind a census that was never -built. Counting *emission sites* rather than mentions changes the picture: -the 219 textual matches across 20 files are almost all comments. The real surface -is **23 `emit_abort_permanent!` sites, all in `pyre-jit/src/jit/codewriter.rs`** -— 22 named-opcode arms plus one catch-all — and every named arm already carries -its reason inline. +**Counted 2026-08-07, re-measured 2026-08-08.** Earlier revisions reported "~214 +matches, unchanged in scale" and left the finding unmeasurable behind a census +that was never built. Counting *emission sites* rather than mentions changed the +picture: the textual matches across 20 files are almost all comments, and the +real surface is the `emit_abort_permanent!` sites in +`pyre-jit/src/jit/codewriter.rs` — now **25**, every one of them named. | class | n | opcodes | |---|---|---| | **genuine trace boundaries** — a trace records one continuous execution, so no residual can express the resume | 9 | `YieldValue`, `Send`, `EndSend`, `ReturnGenerator`, `GetYieldFromIter`, `GetAiter`, `GetAnext`, `EndAsyncFor`, `CleanupThrow` | | **narrow conditional shapes** inside an otherwise-lowered opcode | 5 | `Call` / `CallKw` (nargs past the backend dispatch ceiling), `LoadFastCheck`, `LoadLocals` (non-portal `is_locals`), `DeleteDeref` (compiler-normalized class-scope case) | | **unported opcodes** — the real coverage gap | 8 | `CheckEgMatch`, `BuildInterpolation`, `BuildTemplate`, `CallIntrinsic1`, `CallIntrinsic2`, `LoadSpecial`, `LoadFromDictOrDeref`, `SetupAnnotations` | -| **catch-all** `_other => emit_abort_permanent!(py_pc)` | 1 | whatever the match does not name | +| **not emittable by this compiler** — three classified arms where the catch-all was | 108 | 82 adaptive specializations, 21 `Instrumented*`, 5 interpreter/JIT-internal | **Violates.** A1 ("Rust can't be meta-traced is never a valid excuse") and charter §3.1's norm that every fallback is a census-tracked gap, never a silent hole. -**Where the violation actually lives.** Not in the 22 — those are listed by -construction, each beside its reason. It lives in the **`_other` arm**: an opcode -nobody has looked at declines the whole loop with no record of which opcode it -was. That single arm is the entire unlisted surface, which is why the finding -could never be closed by counting matches. - -**What is left.** - -1. **Make `_other` name its opcode.** A cliff that says which instruction caused - it stops being a silent hole, and the gap list below becomes self-maintaining - instead of needing a census run to rediscover. -2. **Port the 8.** `CALL_INTRINSIC_1 → HLOp` is the template; `CallIntrinsic1` - still aborts on the intrinsic kinds that arm does not cover. -3. **Record the 9 as boundaries, not gaps.** They are correct, and counting them - is most of what made the raw number look like a wall. +**What the catch-all turned out to be.** Not "an opcode nobody has looked at" — +`_other` covered exactly the 108 `Instruction` variants the dispatch never named, +and all 108 are opcodes this compiler cannot emit: the adaptive specializations a +quickening interpreter writes in place (pyre's eval loop does not quicken — +nothing calls `replace_op` outside a corruption test), the `sys.monitoring` +substitutions, and the tier-2 executor's internal forms. So the unlisted cliff +was never a live cliff. It was the *silence* that mattered: had one appeared, the +loop declined with no record of which opcode did it. + +Deleting `_other` and classifying all 108 makes the match exhaustive, so a +variant added upstream now fails to compile here instead of vanishing into a +catch-all — `majit-translate`'s `flowspace/flowcontext.rs` already classifies the +same three groups, and this brings the walker in line with it. `cargo check -p +pyre-jit` passes with no catch-all, and dropping a single pattern from the list +fails as `E0004: non-exhaustive patterns: Instruction::ToBoolBool not covered`, +which is what makes the coverage claim load-bearing rather than decorative. + +**What is left.** **Port the 8.** `CALL_INTRINSIC_1 → HLOp` is the template; +`CallIntrinsic1` still aborts on the intrinsic kinds that arm does not cover. The +9 boundaries are correct and counting them is most of what made the raw number +look like a wall. **Tracking.** gh#346 (two-phase coverage roadmap) and gh#373 (the cliff symptom) are closed; coverage work continues against the #346 line. @@ -114,44 +119,6 @@ nowhere to go belongs inside an existing kind, not in a new slot. runs), and the regrtest harness under moving collection. The real exit test is that the oldgen-nonmoving concession becomes deletable. -### F5 — three documented gates have no reader left, and the brake is Rust-only - -The 66 undocumented gates and the missing brake are both closed: `gate-triage.md` -§6 lists all 66, and `pyre/pyrex/tests/gate_triage_complete.rs` fails the build -when a `PYRE_*` read in a workspace member has no entry in a **live** section of -that file. **Measured 2026-08-07**, re-measured once §6 landed: - -| count | value | what it is | -|---|---|---| -| distinct names read from `*.rs` | **105** | the population the brake sees | -| (file, name) read pairs | 128 | read *sites*, not gates | -| named in `gate-triage.md`'s live sections | 113 | tokens; one is the `PYRE_FBW_*` fragment in §1d's heading | -| named in its retirement sections | 47 | already swept | -| **live-named with no reader anywhere** | **3** | the debt | - -```sh -git ls-files '*.rs' | xargs rg --no-filename -o \ - '(env::var[_a-z]*|host_os::var|getenv)\(b?"(PYRE_[A-Z0-9_]+)"' -r '$2' | sort -u -``` - -Say which of these a number is. Earlier revisions reported 119 and then 126 -"distinct names"; both were the (file, name) pair count, because the command as -written then kept rg's filename prefix and `sort -u` counted sites. - -**Violates.** Charter §3.6: a gate is a staging area, not a home. Three names are -staged in a file nobody swept. - -**What is left.** - -1. Retire `PYRE_FBW_REC_UNROLL`, `PYRE_FBW_VABLE_SCALAR_CA` and `PYRE_P2_DRAIN` - — named in §5/§1d, read from nothing in the tree. -2. Decide whether the brake scans beyond Rust. Four live gates - (`PYRE_CHECK_PYPY3`, `PYRE_CHECK_PYTHON3`, `PYRE_SHARED_BUILD`, - `PYRE_SYNTH_PYPY`) are read only by `check.py`, `check_synthetic.py`, the CI - workflows and `scripts/llbc_extract.py`. A `*.rs` census reads them as retire - targets and they are not; conversely a Python- or YAML-only gate added - tomorrow enters undocumented, which is the hole §6 was written to close. - ### Smaller open items - **One unproven resume coordinate.** `build_state_field_snapshot` stamps @@ -166,9 +133,6 @@ staged in a file nobody swept. (cross-check the decoded `py_pc` against `containing_py_pc_for_jitcode_pc_public(jitcode_index, pc)` over the corpus) before it is a finding rather than a suspicion. -- **A misleading survivor of the resume rework.** `pyjitpl.rs` still calls its - `SnapshotFramePcs` local `pc_map`, the name of the deleted translation table. - Rename it so the name stops implying a mechanism that no longer exists. - **Compilation cliffs** outside F4's census: nested-loop / cross-loop no-token walls (gh#152, gh#177) and the recursion / call-frame wall (gh#126, open). - **Phase C decision document** (gh#376, open): a C-extension strategy document, @@ -179,18 +143,16 @@ staged in a file nobody swept. ## Sequencing -**F4 > F3 > F5.** The charter's own §5 order, restored — it was inverted in an -earlier revision because the root-walker registry had a hard failure one -registration away, and that is gone. +**F4 > F3.** The charter's own §5 order, restored — it was inverted in an earlier +revision because the root-walker registry had a hard failure one registration +away, and that is gone. -- **F4 first**, and it is now small. Naming the opcode in the `_other` arm is a - one-site change that converts the only silent hole into a reported one; the - eight unported opcodes are then a list somebody can work through, and Phase A's - cliff-free exit criterion becomes checkable without any new instrument. +- **F4 first**, and it is now small: eight unported opcodes, a list somebody can + work through. The silent hole is closed, so Phase A's cliff-free exit criterion + is checkable without any new instrument — an opcode the walker declines names + itself, and one it has never seen fails the build. - **F3 next**: the deepest structural work, unblocked by one answerable question (the mid-major rescan above) rather than by a taxonomy. -- **F5 out of order whenever convenient** — it is cheap, it is the only item that - gets *worse* while ignored, and its brake is what keeps it closed. F4 and F3 are parallel-safe, though not for the reason an earlier revision gave: both touch `pyre-jit`, so "different crates" was wrong. They share no file and no @@ -218,6 +180,20 @@ or replaced. **And then its section here is deleted.** is the sole trace-time executor and observes a vable-force through the residual-call token protocol, which is the metainterp mechanism, not a second leg. +- *Gates staged in a file nobody swept.* The 66 undocumented gates, the missing + brake, the brake's Rust-only reach and the last reader-less entry are all + closed: 111 names read (105 from `*.rs`, 6 only from the harness) and the same + 111 documented live, once the two `PYRE_*` wildcard stems the token scan leaves + behind are set aside. + `pyre/pyrex/tests/gate_triage_complete.rs` now fails the build in **both** + directions — a read with no live entry, and a live entry with no reader. Say + which of these a number is: earlier revisions reported 119 and then 126 + "distinct names" and both were the (file, name) pair count, because the census + command kept rg's filename prefix and `sort -u` counted sites. + `gate-triage.md` §6 carries the command and the counting rules; the two that + cost the most to rediscover are that a retirement row documents nothing + wherever it sits, and that a name written only in the file's `_PYRE`, + `_PYTHON` run-on shorthand reads as undocumented to any whole-token census. **Deliberate adaptations, decided — keep.** @@ -238,9 +214,10 @@ or replaced. **And then its section here is deleted.** ## What falsifies this -- The F4 census is built, and the unlisted set is not empty: it is the one - `_other` catch-all. If naming that arm's opcode shows it never fires on the - corpus, F4 closes on the spot and only the tracked residue remains. +- The F4 census is built and the unlisted set is empty: the catch-all held only + opcodes this compiler cannot emit, and the match is exhaustive without it. What + remains of F4 is the eight unported opcodes, so if a corpus run shows one of + those eight never fires either, F4 closes on the spot. - If F3's class-(b) absorption measurably regresses minor-collection pause (prebuilt scanning cost), the registry survives *for that class only*, documented as the deliberate adaptation it currently is not.