diff --git a/majit/majit-metainterp/src/recorder.rs b/majit/majit-metainterp/src/recorder.rs index 45c720ac833..7e21e6e1820 100644 --- a/majit/majit-metainterp/src/recorder.rs +++ b/majit/majit-metainterp/src/recorder.rs @@ -81,7 +81,12 @@ pub struct SnapshotFrame { /// between the two. pub pc: u32, /// Forward-carried Python instruction PC for this JitCode position. - /// `u32::MAX` is the no-snapshot sentinel paired with `pc == -1`. + /// + /// Unlike the `i32`-typed twins this type carries no no-snapshot sentinel: + /// both fields here are `u32`, no writer stamps one, and "no snapshot" is + /// the absence of a frame rather than a value inside one. The `-1` sentinel + /// belongs to `resume::SnapshotFrame` and `resumedata::RebuiltFrame`, whose + /// signed fields can represent it. /// /// Upstream derives the Python-level position where it needs one; this /// carries it, because the resume decoder that reads it back for diff --git a/pyre/gate-triage.md b/pyre/gate-triage.md index d04242c5cef..0a8c4d59492 100644 --- a/pyre/gate-triage.md +++ b/pyre/gate-triage.md @@ -369,7 +369,7 @@ call, and two nested inlined levels (depth 3). Instrumented 2026-07-26, both report the same cause — a ref color that is **live at the caller's post-call coordinate holds `ConcreteValue::Null`**: -``` +```text try/except caller: ref color=11 not concrete: Null result_color=Some(5) nlocals=3 depth=3 live_ref=[0,1,2,5,11] depth 3: ref color=2 not concrete: Null result_color=Some(0) nlocals=1 depth=1 live_ref=[0,1,2] ``` @@ -393,7 +393,7 @@ still named the CALLER while an inlined callee body ran. A `sys._getframe` that is *itself* the escaping residual therefore read the wrong frame at walk time, and the adopt committed that answer where legacy escape/replay discards it: -``` +```text _gf().f_code.co_name -> "main", not "leaf" _gf(1).f_code.co_name -> "", not "main" # one level too far up _gf(1).f_locals["k"] -> KeyError # same cause, seen through the argument @@ -846,13 +846,21 @@ 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 | 127 | +| (file, name) read pairs | 128 | | **live gates that were absent from this file** | **66** | | names here with no read site left (retire) | 51 | +```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' | xargs rg --no-filename -o 'env::var[_a-z]*\("(PYRE_[A-Z0-9_]+)"' -r '$1' | 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`. 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 diff --git a/pyre/pyrex/tests/gate_triage_complete.rs b/pyre/pyrex/tests/gate_triage_complete.rs index 25b034b3515..7ac3bf9269f 100644 --- a/pyre/pyrex/tests/gate_triage_complete.rs +++ b/pyre/pyrex/tests/gate_triage_complete.rs @@ -1,5 +1,5 @@ -//! Every `PYRE_*` environment gate read anywhere in the workspace must have an -//! entry in `pyre/gate-triage.md`. +//! Every `PYRE_*` environment gate read from a workspace member's Rust source +//! must have a live entry in `pyre/gate-triage.md`. //! //! 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 @@ -9,6 +9,12 @@ //! //! 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`. use std::collections::BTreeSet; use std::path::{Path, PathBuf}; @@ -71,27 +77,43 @@ fn collect_rs(dir: &Path, out: &mut Vec) { } } +/// The function names that read an environment variable in this tree. +/// +/// `env::var` (with its `_os` suffix) is the common one, but a search for it +/// alone misses the host seam: `host_os::var` on the sandbox path and +/// `host_seam::ops::getenv`, which takes a *byte* string. `PYRE_STDLIB` is read +/// through both seam forms in `pyre-interpreter/src/importing.rs` and stays +/// 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"]; + /// Gate names this text reads from the environment. /// -/// Matches the two read forms the tree uses — `env::var("NAME")` and -/// `env::var_os("NAME")` — rather than every mention of the name, so a gate +/// Matches the read forms above rather than every mention of the name, so a gate /// discussed in a comment or held in a Rust const does not count as live. That /// is the same distinction `gate-triage.md` §2 draws by hand. fn gates_read_by(text: &str) -> Vec<&str> { let mut found = Vec::new(); - for (at, _) in text.match_indices("env::var") { - let rest = &text[at + "env::var".len()..]; - let rest = rest.strip_prefix("_os").unwrap_or(rest); - let Some(rest) = rest.strip_prefix('(') else { - continue; - }; - let Some(rest) = rest.trim_start().strip_prefix('"') else { - continue; - }; - let Some(end) = rest.find('"') else { continue }; - let name = &rest[..end]; - if name.starts_with("PYRE_") { - found.push(name); + for form in READ_FORMS { + for (at, _) in text.match_indices(form) { + let rest = &text[at + form.len()..]; + // `env::var_os` is the same read wearing a suffix. + let rest = rest.strip_prefix("_os").unwrap_or(rest); + let Some(rest) = rest.strip_prefix('(') else { + continue; + }; + let rest = rest.trim_start(); + // `host_seam::ops::getenv` names its gate with a byte string. + let rest = rest.strip_prefix('b').unwrap_or(rest); + let Some(rest) = rest.strip_prefix('"') else { + continue; + }; + let Some(end) = rest.find('"') else { continue }; + let name = &rest[..end]; + if name.starts_with("PYRE_") { + found.push(name); + } } } found @@ -105,34 +127,65 @@ fn gates_read_by_matches_the_read_forms_and_nothing_else() { std::env::var( "PYRE_C", ); + host_os::var("PYRE_D").ok(); + crate::host_seam::ops::getenv(b"PYRE_E"); // PYRE_MENTIONED_IN_A_COMMENT const PYRE_CONST: &str = "PYRE_NOT_A_READ"; other::var("PYRE_NOT_ENV"); env::var("HOME"); "#; - assert_eq!(gates_read_by(sample), vec!["PYRE_A", "PYRE_B", "PYRE_C"]); + 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"]); } -/// Every `PYRE_*` token the triage document mentions. +/// Does this `##` heading introduce a section that records history? +/// +/// §1/§1b/§1c list gates whose readers were deleted, §2 lists names that were +/// never env vars, and §3 lists names with no read site. A name there is the +/// record of a gate that is *gone*, so it must not satisfy the brake — otherwise +/// re-introducing a reader for `PYRE_SINGLE_PASS` lands green on the strength of +/// its own retirement row, with neither its new polarity nor a retirement plan +/// written down. +fn is_history_heading(heading: &str) -> bool { + let lower = heading.to_ascii_lowercase(); + // "retired", not "retire": §4 is a *live* section whose heading says when + // its gates will go ("retire when the epic closes"). + lower.contains("retired") || lower.contains("dead") || lower.contains("not gates") +} + +/// Every `PYRE_*` token the triage document lists as a live gate. /// /// 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. +/// 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. fn gates_documented_in(triage: &str) -> BTreeSet<&str> { let mut found = BTreeSet::new(); - for (at, _) in triage.match_indices("PYRE_") { - // A name preceded by a name character is the tail of a longer token. - if triage[..at] - .chars() - .next_back() - .is_some_and(|c| c.is_ascii_alphanumeric() || c == '_') - { + let mut live = true; + for line in triage.lines() { + if let Some(heading) = line.strip_prefix("## ") { + live = !is_history_heading(heading); + } + if !live { continue; } - let end = triage[at..] - .find(|c: char| !(c.is_ascii_uppercase() || c.is_ascii_digit() || c == '_')) - .map_or(triage.len(), |off| at + off); - found.insert(&triage[at..end]); + 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]); + } } found } @@ -145,6 +198,21 @@ fn every_live_pyre_gate_has_a_gate_triage_entry() { .unwrap_or_else(|e| panic!("cannot read {}: {e}", triage_path.display())); let documented = gates_documented_in(&triage); + // Two anchors on the live/history split, because a heading reword would + // otherwise change what this test accepts without failing. One on each side: + // a retired name must not count, and a listed live name must. + assert!( + !documented.contains("PYRE_SINGLE_PASS"), + "PYRE_SINGLE_PASS is named only in a retirement section and has no read \ + site, so it must not count as documented — is_history_heading no longer \ + matches gate-triage.md's headings" + ); + 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); diff --git a/pyre/rework.md b/pyre/rework.md index bdd9a883346..4575d0436fb 100644 --- a/pyre/rework.md +++ b/pyre/rework.md @@ -3,8 +3,9 @@ **Status**: living record, companion to `design.md` (the charter). Where the charter states what pyre must be, this states where today's code violates it and what is left to do about it. **Findings are deleted as they close** — the history -of what was once wrong belongs in git, not here. Keep this document small enough -that it is worth re-reading. +of what was once wrong belongs in git, not here. The one exception is *Settled* +below: a closed finding leaves a one-line verdict there only where re-deriving it +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`. @@ -113,41 +114,43 @@ 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 — 66 of the 105 live `PYRE_*` gates are undocumented, and nothing stops a 67th +### F5 — three documented gates have no reader left, and the brake is Rust-only -**Measured 2026-08-07**, distinguishing the counts this has been confused between -before: +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 the environment | **105** | the gate population | -| (file, name) read pairs | 127 | read *sites*, not gates | -| names mentioned in `gate-triage.md` | 90 | of which only 39 are still read | -| **live gates absent from `gate-triage.md`** | **66** | the debt | - -``` -git ls-files '*.rs' | xargs rg --no-filename -o 'env::var[_a-z]*\("(PYRE_[A-Z0-9_]+)"' -r '$1' | sort -u +| 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 ``` -Earlier revisions of this document reported 119 and then 126 "distinct names". -Both were the (file, name) pair count: the command as previously written kept -rg's filename prefix, so `sort -u` counted sites. `--no-filename` is what makes it -a gate count. Say which of the four a number is. +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. The triage -table is a snapshot that is 63% empty, and nothing makes a new gate enter it at -birth — which is what the hygiene workstream asked for and never got. +**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. **List the 66.** Most are default-OFF diagnostics (`*_DIAG`, `*_AUDIT`, - `*_CENSUS`, `*_PROBE`), which `gate-triage.md`'s own polarity rule classifies - mechanically from the read expression. The default-ON ones are the removal - targets; the rest are book-keeping. -2. **Add the brake**: a check that fails when a `PYRE_*` env read has no entry in - `gate-triage.md`. Without it the table re-rots the moment it is filled, which - is how it got to 63% empty. -3. Retire the 51 documented names that no longer have a read site. +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 @@ -189,7 +192,10 @@ registration away, and that is gone. - **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: different crates, no shared surface. +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 +symbol — F4 is confined to `jit/codewriter.rs`, F3 to the root-walker +registration in `eval.rs` / `call_jit.rs` and `majit-gc/shadow_stack.rs`. Each item closes by the charter's instruments: N4 gates for every landing, N5 evidence for every default flip, N7 written rationale for every mechanism deleted @@ -232,8 +238,9 @@ or replaced. **And then its section here is deleted.** ## What falsifies this -- If the F4 census, once built, shows the unlisted set is already empty, F4 - closes on the spot and only the tracked residue remains. +- 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. - 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.