From eac757fb07d1f80e235d4d6578923cf31c778724 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Ralph=20K=C3=BCpper?= Date: Sat, 1 Aug 2026 09:12:25 +0200 Subject: [PATCH 1/6] perf(repsel): stop charging Perry's own cjs_wrap preamble to the Ptr report MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit The `cjs_wrap` template emits `const __cjs_module = { exports: {} }` followed by `var module = __cjs_module` into every wrapped CommonJS module. Rule 1 seeds the record as a `Ptr` candidate; the alias on the next line denies it under rule 2 as a "bare reference". Over 195 real `__esModule` dependency modules that pair — plus the `{}` inside it, reported as an unbound constructor-argument allocation — is 379 of 1231 candidates, 31 % of the whole report, and it was the evidence behind #7139's "containment is the wall" reading and #7152's rule-1 headline. The reference is not exemptable: `module` is a reassignable `var` the preamble stores into `require.main` and that CJS bodies write `module.exports = X` through, so the record genuinely escapes for its whole life. What is wrong is that it was ever a candidate. `collectors/cjs_scaffolding.rs` now recognises the template's record — R1 a `const __cjs_module` bound to an `__AnonShape_…` allocation, R2 whose literal is exactly `{ exports: {} }`, R3 uniquely in the region, R4 aliased by the `var module` binding that denies it — and drops it at the seed, together with the object literals of the four allocating preamble statements behind it (the two `defineProperty` sites #7139 already recognises, reusing that predicate verbatim, plus `require.cache` and `require.extensions`). R4 is the soundness argument: it IS the denial, so a region carrying it cannot promote its record and the returned facts are bit-identical. Verified on emitted LLVM IR over 49 dependency modules with a same-compiler control run per module, and behaviourally against Node 26.5.1 on a CJS dependency fixture. Denials: rule-2 bare reference 140 -> 4, rule-1 constructor argument 196 -> 8, rule-5 187 -> 132, candidates 1231 -> 914. Selected/consumed unchanged at 3/11 in both arms: nothing is promoted, by construction. --- .../src/collectors/cjs_scaffolding.rs | 754 +++++++++++++++++- crates/perry-codegen/src/collectors/mod.rs | 1 + .../perry-codegen/src/collectors/ptr_shape.rs | 39 +- .../src/collectors/ptr_shape_report.rs | 82 +- crates/perry-codegen/src/lib.rs | 16 + .../compile/cjs_wrap/preamble_canary_tests.rs | 87 ++ 6 files changed, 932 insertions(+), 47 deletions(-) diff --git a/crates/perry-codegen/src/collectors/cjs_scaffolding.rs b/crates/perry-codegen/src/collectors/cjs_scaffolding.rs index 506688dbe7..97021c2488 100644 --- a/crates/perry-codegen/src/collectors/cjs_scaffolding.rs +++ b/crates/perry-codegen/src/collectors/cjs_scaffolding.rs @@ -71,6 +71,73 @@ //! identity of the mutated object, not about which analysis consults the //! flag, so it carries over unchanged: `exports` and `require` are likewise //! never `Ptr` locals nor proven-`this` receivers. +//! +//! --- +//! +//! # The preamble's own ALLOCATIONS (#7152) +//! +//! Everything above is about the rule-5 barrier. [`preamble_in_region`] is +//! about a different self-inflicted wound in the same template: the objects +//! the preamble *allocates* are counted as `Ptr` candidates and then +//! denied, in every CommonJS module, and on real dependency code they are the +//! majority of the report. +//! +//! Measured over 195 `__esModule` CJS modules from `scriptc/node_modules`: +//! 136 of the 140 rule-2 "bare reference" denials, 55 of the 187 rule-5 +//! denials, and 188 of the 194 "constructor argument" unbound-allocation +//! denials are one of these two statements — 379 rows, ~2 per module, 31 % of +//! every `Ptr` candidate in the corpus. #7139 read its share of them as +//! "containment is the wall in dependency JS" and scheduled #7149 on it; #7152 +//! re-measured and put the wall at rule 1. Both readings were partly about +//! Perry's own scaffolding. +//! +//! ## What is recognised +//! +//! One **region** (a lowered statement list — the `cjs_wrap` IIFE's body, or +//! module init on the flat path) is a CommonJS preamble when its top level +//! carries all four of: +//! +//! * **R1** `Stmt::Let` named `__cjs_module`, `mutable: false`, initialized by +//! an `Expr::New` of an `__AnonShape_…` class (an object literal); +//! * **R2** that literal is exactly `{ exports: {} }` — one field whose value +//! is an argument-less `__AnonShape_…` allocation; +//! * **R3** exactly one top-level statement satisfying R1+R2, so "the record" +//! is unambiguous; +//! * **R4** the same top level binds `var module = __cjs_module` — +//! `Stmt::Let { mutable: true, init: LocalGet() }`. +//! +//! Then, and only then, three things stop being reported: the record local +//! itself, the `{}` inside it, and the object literals of the preamble +//! statements [`CjsPreamble::stmt_allocates_only_scaffolding`] names. +//! +//! ## Why it is sound +//! +//! **This is a candidate SUPPRESSION, not a proof relaxation** — the opposite +//! direction from the barrier exemption above. Dropping a candidate can only +//! remove facts, never add one, so it cannot make codegen unsound; the entire +//! obligation is to show it never removes a fact that would otherwise exist. +//! +//! **R4 discharges that obligation outright.** It is not a heuristic about +//! the template, it is the denial itself: a `Stmt::Let` whose init is a bare +//! `Expr::LocalGet` of the record, with `mutable: true` so the alias pre-pass +//! in `ptr_shape.rs` refuses to track it, walks into `UseWalk`'s `LocalGet` +//! arm under the default escape context and disqualifies the record with +//! `ESC_BARE_REFERENCE` on every path. A region satisfying R4 therefore +//! *cannot* promote its record, so removing it from the candidate set leaves +//! the returned `HashMap` bit-identical. R1-R3 only make the recognition +//! unambiguous; they carry no soundness weight, and if any of them drifts the +//! candidate simply reappears in the report. +//! +//! And the escape R4 names is not incidental to the template — it is load +//! bearing. `var module` is what CommonJS bodies write `module.exports = X` +//! through, and the preamble goes on to store it into `require.main`. The +//! record is genuinely, permanently escaped; there is no narrowing of rule 2 +//! that could promote it, which is why this is a suppression and not an +//! exemption. +//! +//! The allocation-site half is **report-only** in the strongest sense: +//! `ptr_shape_report::unbound_new_sites` is called exclusively under +//! `opt_report::enabled()`, and its output feeds nothing but the report. use std::collections::HashSet; @@ -205,6 +272,244 @@ fn init_is_never_a_seed(init: Option<&Expr>) -> bool { ) } +// ── #7152: the preamble's own allocations, region by region ──────────────── + +/// The `cjs_wrap` module record's binding name. Perry writes it; no +/// transpiler emits it, and a user writing it is not a reason to promote. +const RECORD_BINDING: &str = "__cjs_module"; +/// The `var module = __cjs_module;` alias — R4, the denial itself. +const MODULE_BINDING: &str = "module"; +/// Object literals lower to `Expr::New` of a synthesised class with this +/// prefix (`perry-hir`'s anon-shape naming). +const ANON_SHAPE_PREFIX: &str = "__AnonShape_"; +/// The two `require` properties the preamble installs with an object-literal +/// value. `require.resolve` / `require.resolve.paths` take closures, which +/// `unbound_new_sites` does not descend into, so they need no arm here. +const REQUIRE_LITERAL_KEYS: [&str; 2] = ["cache", "extensions"]; + +/// Perry's `cjs_wrap` preamble as it appears in ONE lowered region. +/// +/// The record local and the scaffolding bindings live in ONE `Option` on +/// purpose. "Recognition failed" then has no representation in which anything +/// could still be suppressed — the `Default` is not a flag every reader has to +/// remember to test, it is the absence of the data they would need. Every +/// recognition failure lands there. +#[derive(Debug, Default)] +pub(super) struct CjsPreamble { + /// `(the `const __cjs_module = { exports: {} }` local, the region's + /// `exports` / `require` bindings resolved by the same whitelist [`collect`] + /// applies module-wide)` — `Some` only when R1-R4 all hold. + recognised: Option<(u32, CjsScaffolding)>, +} + +impl CjsPreamble { + /// Is `id` the recognised CommonJS module record? `ptr_shape.rs` drops it + /// from the `Ptr` candidate set — see the module doc for why that + /// cannot change a fact. + pub(super) fn is_module_record(&self, id: u32) -> bool { + matches!(self.recognised, Some((record, _)) if record == id) + } + + /// Does this statement allocate **only** preamble scaffolding, so that + /// `ptr_shape_report::unbound_new_sites` should not walk it? + /// + /// Four shapes, all from the one template, all gated on the record having + /// been recognised: + /// + /// 1. `const __cjs_module = { exports: {} };` — the inner `{}` is + /// `module.exports`, reported today as an unbound allocation in + /// *constructor-argument* position. It is the single most common + /// `Ptr` denial in dependency JS. + /// 2/3. The two `defineProperty` sites #7139 already recognises. Their + /// descriptor is a literal allocated in the call; the target check is + /// [`CjsScaffolding::exempts_shape_barrier`] verbatim, so the two + /// exemptions can never disagree about what "scaffolding" means. + /// 4. `require.cache = {}` and `require.extensions = { … }`. + /// + /// Nothing else. A `Stmt::Expr` of anything else, and any statement in a + /// region whose record was not recognised, is walked exactly as before. + pub(super) fn stmt_allocates_only_scaffolding(&self, stmt: &Stmt) -> bool { + let Some((record, scaffolding)) = &self.recognised else { + return false; + }; + match stmt { + Stmt::Let { id, .. } => id == record, + Stmt::Expr(expr) => match expr { + Expr::ObjectDefineProperty(..) => scaffolding.exempts_shape_barrier(expr), + Expr::PutValueSet { target, key, .. } => { + matches!( + (target.as_ref(), key.as_ref()), + (Expr::LocalGet(id), Expr::String(k)) + if scaffolding.require.contains(id) + && REQUIRE_LITERAL_KEYS.contains(&k.as_str()) + ) + } + _ => false, + }, + _ => false, + } + } +} + +/// Recognise the `cjs_wrap` preamble in one lowered region. +/// +/// Runs for **every** region of every module, so the first thing it does is a +/// single pass over the region's top-level statements looking for R1/R2. Only +/// `cjs_wrap` output binds `__cjs_module` to that literal, so on ordinary +/// TypeScript this returns `Default` after one `Vec` scan. +pub(super) fn preamble_in_region(stmts: &[Stmt]) -> CjsPreamble { + // R1 + R2 (`record_binding`), on top-level bindings of R1's name. + let records: Vec = stmts + .iter() + .filter(|stmt| matches!(stmt, Stmt::Let { name, .. } if name == RECORD_BINDING)) + .filter_map(record_binding) + .collect(); + // R3: two records in one region means "the record" is ambiguous, so + // recognise neither and let both stay candidates. + let [record] = records[..] else { + return CjsPreamble::default(); + }; + // R4: the bare reference that denies it. Without this the suppression + // would be a claim about the template; with it, it is a claim about this + // region's own statements. + if !binds_module_alias(stmts, record) { + return CjsPreamble::default(); + } + let mut acc = Acc::default(); + note_stmt_root(stmts, &mut acc); + CjsPreamble { + recognised: Some(( + record, + CjsScaffolding { + exports: acc.exports.difference(&acc.disqualified).copied().collect(), + require: acc.require.difference(&acc.disqualified).copied().collect(), + }, + )), + } +} + +/// What the `Ptr` report suppresses as `cjs_wrap` scaffolding in one +/// module. See [`census`]. +#[derive(Debug, Default, Clone, Copy, PartialEq, Eq)] +pub struct CjsPreambleCensus { + /// Regions whose CommonJS module record was recognised (R1-R4) and + /// therefore dropped from `Ptr` candidacy. + pub module_records: usize, + /// Top-level preamble statements, summed over those regions, whose object + /// literals `unbound_new_sites` no longer walks. + pub preamble_alloc_stmts: usize, +} + +/// Count what [`preamble_in_region`] recognises across every lowering region +/// of `module`. +/// +/// Exists **solely** for the `perry` crate's `cjs_wrap` template canary +/// (`commands/compile/cjs_wrap/preamble_canary_tests.rs`), the same coupling +/// [`crate::module_has_ptr_shape_barrier`] exists for: the template is in +/// `perry`, the recogniser is here, and a template edit would otherwise +/// silently un-recognise the preamble with no test going red. Nothing in the +/// compile pipeline calls it. +pub fn census(module: &Module) -> CjsPreambleCensus { + let mut out = CjsPreambleCensus::default(); + let mut roots: Vec<&[Stmt]> = vec![&module.init]; + for function in &module.functions { + roots.push(&function.body); + } + for class in &module.classes { + if let Some(ctor) = &class.constructor { + roots.push(&ctor.body); + } + for method in class + .methods + .iter() + .chain(class.static_methods.iter()) + .chain(class.getters.iter().map(|(_, f)| f)) + .chain(class.setters.iter().map(|(_, f)| f)) + .chain(class.computed_members.iter().map(|m| &m.function)) + { + roots.push(&method.body); + } + } + for root in roots { + note_region(root, &mut out); + // Recurses through `Expr::Closure`, so this reaches every closure body + // at any nesting depth exactly once — including the `cjs_wrap` IIFE, + // which is where the preamble actually lives. + for_each_expr_in_stmts(root, &mut |expr| { + if let Expr::Closure { body, .. } = expr { + note_region(body, &mut out); + } + }); + } + out +} + +/// Recognise one region and fold it into the census. +fn note_region(stmts: &[Stmt], out: &mut CjsPreambleCensus) { + let preamble = preamble_in_region(stmts); + if preamble.recognised.is_none() { + return; + } + out.module_records += 1; + out.preamble_alloc_stmts += stmts + .iter() + .filter(|s| preamble.stmt_allocates_only_scaffolding(s)) + .count(); +} + +/// R1 + R2 for a statement the caller has already matched on R1's binding +/// NAME. The name is checked exactly once, in [`preamble_in_region`]: two +/// enforcement points for one conjunct is how a sabotage hole gets in — either +/// one can be deleted with every test still green. +fn record_binding(stmt: &Stmt) -> Option { + let Stmt::Let { + id, + mutable: false, + init: Some(Expr::New { + class_name, args, .. + }), + .. + } = stmt + else { + return None; + }; + if !class_name.starts_with(ANON_SHAPE_PREFIX) { + return None; + } + // Exactly one field, whose value is an argument-less object literal. A + // record with more fields, or a non-literal field value, is not the + // template's `{ exports: {} }` and keeps its candidacy. + let [Expr::New { + class_name: inner, + args: inner_args, + .. + }] = args.as_slice() + else { + return None; + }; + (inner.starts_with(ANON_SHAPE_PREFIX) && inner_args.is_empty()).then_some(*id) +} + +/// R4: `var module = __cjs_module;` at the region's top level. +/// +/// `mutable: true` is required, not incidental: a `const` alias WOULD be +/// tracked by `ptr_shape.rs`'s alias pre-pass, and the record would then be +/// denied for a different reason (or, in principle, not at all) — so the +/// fact-neutrality argument would no longer hold. +fn binds_module_alias(stmts: &[Stmt], record: u32) -> bool { + stmts.iter().any(|stmt| { + matches!( + stmt, + Stmt::Let { + name, + mutable: true, + init: Some(Expr::LocalGet(src)), + .. + } if name == MODULE_BINDING && *src == record + ) + }) +} + /// A statement list plus every closure body reachable from it. fn note_stmt_root(stmts: &[Stmt], acc: &mut Acc) { for_each_stmt(stmts, &mut |stmt| acc.note_let(stmt)); @@ -295,6 +600,7 @@ mod tests { const REQUIRE_ID: u32 = 10; const CJS_MODULE_ID: u32 = 12; const EXPORTS_ID: u32 = 7; + const MODULE_ID: u32 = 6; const POINT_ID: u32 = 42; fn closure(func_id: u32, body: Vec) -> Expr { @@ -325,6 +631,18 @@ mod tests { } } + /// `const = ;` — `mutable: false`, which is what the template + /// emits for `require` and `__cjs_module` and what R1 requires. + fn let_const(id: u32, name: &str, init: Expr) -> Stmt { + Stmt::Let { + id, + name: name.to_string(), + ty: Type::Any, + mutable: false, + init: Some(init), + } + } + fn anon_shape(class_name: &str, args: Vec) -> Expr { Expr::New { class_name: class_name.to_string(), @@ -348,20 +666,28 @@ mod tests { )) } - /// The `cjs_wrap` preamble, verbatim in HIR shape: a `require` closure, the - /// `__cjs_module` record, and `exports` read out of it. `extra` is appended - /// to the CommonJS body inside the IIFE. - fn cjs_module(extra: Vec) -> perry_hir::Module { - let mut body = vec![ - let_stmt(REQUIRE_ID, "require", closure(7, Vec::new())), - let_stmt( - CJS_MODULE_ID, - "__cjs_module", - anon_shape( - "__AnonShape_module", - vec![anon_shape("__AnonShape_exports", Vec::new())], - ), + /// `const __cjs_module = { exports: {} };` — R1 + R2, exactly as the + /// template lowers (checked against `--print-hir` on a wrapped module). + fn record_stmt() -> Stmt { + let_const( + CJS_MODULE_ID, + "__cjs_module", + anon_shape( + "__AnonShape_module", + vec![anon_shape("__AnonShape_exports", Vec::new())], ), + ) + } + + /// The `cjs_wrap` preamble as ONE lowered region, verbatim in HIR shape: + /// the `require` closure, the `__cjs_module` record, the `var module` + /// alias that denies it (R4), and `var exports = __cjs_module.exports`. + /// `extra` is appended as the CommonJS body. + fn cjs_region(extra: Vec) -> Vec { + let mut body = vec![ + let_const(REQUIRE_ID, "require", closure(7, Vec::new())), + record_stmt(), + let_stmt(MODULE_ID, "module", Expr::LocalGet(CJS_MODULE_ID)), let_stmt( EXPORTS_ID, "exports", @@ -373,12 +699,17 @@ mod tests { ), ]; body.extend(extra); + body + } + + /// The same region wrapped in the IIFE the wrap emits, as a whole module. + fn cjs_module(extra: Vec) -> perry_hir::Module { let mut module = perry_hir::Module::new("node_modules/dep/index.js"); module.init.push(let_stmt( 0, "_cjs", Expr::Call { - callee: Box::new(closure(2, body)), + callee: Box::new(closure(2, cjs_region(extra))), args: Vec::new(), type_args: Vec::new(), byte_offset: 0, @@ -653,4 +984,399 @@ mod tests { ))))); assert!(!promotes_with(sites)); } + + // ── #7152: the preamble's own allocations ────────────────────────────── + + /// `require.cache = {}` / `require.extensions = { … }`. + fn require_install(key: &str, value: Expr) -> Stmt { + Stmt::Expr(Expr::PutValueSet { + target: Box::new(Expr::LocalGet(REQUIRE_ID)), + key: Box::new(Expr::String(key.to_string())), + value: Box::new(value), + receiver: Box::new(Expr::LocalGet(REQUIRE_ID)), + strict: true, + }) + } + + /// Every ALLOCATING statement the preamble emits after the record itself, + /// in template order. + fn preamble_alloc_stmts() -> Vec { + vec![ + define_property( + Expr::LocalGet(REQUIRE_ID), + Expr::String(REQUIRE_KEY.to_string()), + ), + require_install("cache", anon_shape("__AnonShape_cache", Vec::new())), + require_install( + "extensions", + anon_shape("__AnonShape_ext", vec![closure(12, Vec::new())]), + ), + define_property( + Expr::LocalGet(EXPORTS_ID), + Expr::String(EXPORTS_KEY.to_string()), + ), + ] + } + + /// A user allocation in statement position — never suppressed, and the + /// anti-vacuity control for every suppression assertion below. + fn user_alloc_stmt() -> Stmt { + Stmt::Expr(anon_shape("__AnonShape_user", Vec::new())) + } + + fn recognises(region: &[Stmt]) -> bool { + preamble_in_region(region).is_module_record(CJS_MODULE_ID) + } + + /// Swap the region's record `Let` for `replacement`. + fn region_with_record(replacement: Stmt) -> Vec { + let mut region = cjs_region(Vec::new()); + let at = region + .iter() + .position(|s| matches!(s, Stmt::Let { id, .. } if *id == CJS_MODULE_ID)) + .expect("the fixture binds the record"); + region[at] = replacement; + region + } + + #[test] + fn the_wrap_preamble_record_is_recognised() { + assert!(recognises(&cjs_region(Vec::new()))); + } + + /// **Anti-vacuity for everything in this section.** The record IS a rule-1 + /// provenance seed: without the suppression it becomes a `Ptr` + /// candidate, is denied, and is reported. If this ever goes red the + /// suppression is a no-op and the tests below prove nothing. + #[test] + fn the_record_is_a_ptr_shape_seed_in_the_first_place() { + let mut seeds = HashMap::new(); + super::super::find_new_candidates( + &cjs_region(Vec::new()), + &HashSet::new(), + &HashMap::new(), + &mut seeds, + ); + assert!( + seeds.contains_key(&CJS_MODULE_ID), + "the record stopped being a rule-1 seed; the #7152 suppression now \ + removes nothing and every assertion in this section is vacuous" + ); + } + + /// **The fact-neutrality argument, tested.** R4's statement shape IS the + /// rule-2 denial: a `var` alias of a local that otherwise promotes denies + /// it. So a region carrying that alias can never promote its record, and + /// dropping the record from candidacy cannot lose a promotion. + #[test] + fn a_var_alias_denies_a_local_that_otherwise_promotes() { + let point = point_class(); + let classes = HashMap::from([("Point".to_string(), &point)]); + let facts = collect_module_dispatch_facts(&perry_hir::Module::new("m.ts")); + let proven = |body: &[Stmt]| { + !collect_shape_proven_ptr_locals( + body, + &HashSet::new(), + &HashMap::new(), + &classes, + &facts, + &HashSet::new(), + &crate::collectors::ptr_shape_elements::ElementShapeFacts::default(), + ) + .is_empty() + }; + // Control: the same body without the alias DOES promote. + assert!(proven(&promotable_body())); + let mut aliased = promotable_body(); + aliased.insert(1, let_stmt(MODULE_ID, "module", Expr::LocalGet(POINT_ID))); + assert!( + !proven(&aliased), + "a `var m = p` alias no longer denies `p`. R4 is then not the \ + denial it is documented to be, and the #7152 suppression could \ + be dropping a value that would have been promoted." + ); + } + + // ---- sabotage: one red set per conjunct ---- + + /// R1: `mutable: false`. A reassignable record is not the template's. + #[test] + fn a_mutable_record_binding_is_not_recognised() { + assert!(!recognises(®ion_with_record(let_stmt( + CJS_MODULE_ID, + "__cjs_module", + anon_shape( + "__AnonShape_module", + vec![anon_shape("__AnonShape_exports", Vec::new())], + ), + )))); + } + + /// R1: the binding name. Only `cjs_wrap` writes this one. + #[test] + fn a_differently_named_record_binding_is_not_recognised() { + assert!(!recognises(®ion_with_record(let_const( + CJS_MODULE_ID, + "__cjs_modul", + anon_shape( + "__AnonShape_module", + vec![anon_shape("__AnonShape_exports", Vec::new())], + ), + )))); + } + + /// R1: an object literal, not a user class. `new Wrapper({})` is a value + /// with a constructor that can do anything. + #[test] + fn a_record_of_a_declared_class_is_not_recognised() { + assert!(!recognises(®ion_with_record(let_const( + CJS_MODULE_ID, + "__cjs_module", + anon_shape( + "Wrapper", + vec![anon_shape("__AnonShape_exports", Vec::new())] + ), + )))); + } + + /// R2: exactly one field. `{ exports: {}, id: {} }` is not the template. + #[test] + fn a_record_literal_with_a_second_field_is_not_recognised() { + assert!(!recognises(®ion_with_record(let_const( + CJS_MODULE_ID, + "__cjs_module", + anon_shape( + "__AnonShape_module", + vec![ + anon_shape("__AnonShape_exports", Vec::new()), + anon_shape("__AnonShape_extra", Vec::new()), + ], + ), + )))); + } + + /// R2: the field's value is an EMPTY literal. `{ exports: { a: 1 } }` + /// carries state the suppression makes no claim about. + #[test] + fn a_record_whose_exports_literal_is_not_empty_is_not_recognised() { + assert!(!recognises(®ion_with_record(let_const( + CJS_MODULE_ID, + "__cjs_module", + anon_shape( + "__AnonShape_module", + vec![anon_shape("__AnonShape_exports", vec![Expr::Number(1.0)])], + ), + )))); + } + + /// R2: the field's value is an allocation at all. + #[test] + fn a_record_whose_exports_field_is_not_an_allocation_is_not_recognised() { + assert!(!recognises(®ion_with_record(let_const( + CJS_MODULE_ID, + "__cjs_module", + anon_shape("__AnonShape_module", vec![Expr::Undefined]), + )))); + } + + /// R3: two top-level bindings of the name — "the record" is ambiguous, so + /// neither is recognised and both keep their candidacy. + #[test] + fn two_record_bindings_in_one_region_are_ambiguous() { + let mut region = cjs_region(Vec::new()); + region.push(let_const( + CJS_MODULE_ID + 100, + "__cjs_module", + anon_shape( + "__AnonShape_module", + vec![anon_shape("__AnonShape_exports", Vec::new())], + ), + )); + assert!(!recognises(®ion)); + } + + /// R4: no alias at all. Without the escape that denies it, the record is + /// a candidate like any other and must stay in the report. + #[test] + fn without_the_module_alias_the_record_is_not_recognised() { + let region: Vec = cjs_region(Vec::new()) + .into_iter() + .filter(|s| !matches!(s, Stmt::Let { id, .. } if *id == MODULE_ID)) + .collect(); + assert!(!recognises(®ion)); + } + + /// R4: `mutable: true`. A `const` alias is TRACKED by `ptr_shape.rs`'s + /// alias pre-pass rather than treated as an escape, so it does not + /// discharge the fact-neutrality obligation. + #[test] + fn a_const_module_alias_does_not_satisfy_r4() { + let mut region = cjs_region(Vec::new()); + let at = region + .iter() + .position(|s| matches!(s, Stmt::Let { id, .. } if *id == MODULE_ID)) + .expect("the fixture binds the alias"); + region[at] = let_const(MODULE_ID, "module", Expr::LocalGet(CJS_MODULE_ID)); + assert!(!recognises(®ion)); + } + + /// R4: the alias must be of THIS record. + #[test] + fn a_module_alias_of_another_local_does_not_satisfy_r4() { + let mut region = cjs_region(Vec::new()); + let at = region + .iter() + .position(|s| matches!(s, Stmt::Let { id, .. } if *id == MODULE_ID)) + .expect("the fixture binds the alias"); + region[at] = let_stmt(MODULE_ID, "module", Expr::LocalGet(REQUIRE_ID)); + assert!(!recognises(®ion)); + } + + // ---- the allocation-site suppression ---- + + #[test] + fn the_preamble_allocation_statements_are_suppressed() { + let region = cjs_region(preamble_alloc_stmts()); + let preamble = preamble_in_region(®ion); + let suppressed: Vec = region + .iter() + .map(|s| preamble.stmt_allocates_only_scaffolding(s)) + .collect(); + // record `Let` + the four preamble statements; `require`, the alias + // and the `exports` read allocate nothing and are irrelevant either + // way, but must not be claimed. + assert_eq!( + suppressed.iter().filter(|b| **b).count(), + 5, + "{suppressed:?}" + ); + assert!(preamble.stmt_allocates_only_scaffolding(&record_stmt())); + for stmt in preamble_alloc_stmts() { + assert!( + preamble.stmt_allocates_only_scaffolding(&stmt), + "not suppressed: {stmt:?}" + ); + } + } + + #[test] + fn user_allocations_are_never_suppressed() { + let region = cjs_region(preamble_alloc_stmts()); + let preamble = preamble_in_region(®ion); + let kept = [ + user_alloc_stmt(), + // A `defineProperty` whose target is neither scaffolding binding. + define_property(Expr::LocalGet(77), Expr::String(EXPORTS_KEY.to_string())), + // The right target, a key the preamble does not install. + require_install("main", anon_shape("__AnonShape_user", Vec::new())), + // The right key on the wrong object. + Stmt::Expr(Expr::PutValueSet { + target: Box::new(Expr::LocalGet(77)), + key: Box::new(Expr::String("cache".to_string())), + value: Box::new(anon_shape("__AnonShape_user", Vec::new())), + receiver: Box::new(Expr::LocalGet(77)), + strict: true, + }), + // A user `let` of an object literal — a real rule-1 candidate. + let_const(55, "row", anon_shape("__AnonShape_row", Vec::new())), + ]; + for stmt in kept { + assert!( + !preamble.stmt_allocates_only_scaffolding(&stmt), + "wrongly suppressed: {stmt:?}" + ); + } + } + + /// Nothing is suppressed in a region whose record was not recognised — + /// the whole exemption is gated on R1-R4, one instance at a time. + #[test] + fn an_unrecognised_region_suppresses_nothing() { + let region: Vec = cjs_region(preamble_alloc_stmts()) + .into_iter() + .filter(|s| !matches!(s, Stmt::Let { id, .. } if *id == MODULE_ID)) + .collect(); + let preamble = preamble_in_region(®ion); + for stmt in ®ion { + assert!( + !preamble.stmt_allocates_only_scaffolding(stmt), + "suppressed without a recognised record: {stmt:?}" + ); + } + } + + /// End to end through the report walk that consumes this: the scaffolding + /// allocations disappear from `unbound_new_sites` and the user's does not. + #[test] + fn the_report_walk_drops_the_scaffolding_allocations_only() { + let region = cjs_region({ + let mut body = preamble_alloc_stmts(); + body.push(user_alloc_stmt()); + body + }); + let base = + super::super::ptr_shape_report::unbound_new_sites(®ion, &CjsPreamble::default()); + let contexts: Vec<&str> = base.iter().map(|s| s.context).collect(); + assert!( + contexts.contains(&"constructor argument"), + "the `{{ exports: {{}} }}` inner literal is no longer reported \ + unsuppressed; this test's premise is gone: {contexts:?}" + ); + assert!(base.len() >= 6, "{contexts:?}"); + + let fixed = super::super::ptr_shape_report::unbound_new_sites( + ®ion, + &preamble_in_region(®ion), + ); + assert_eq!( + fixed.len(), + 1, + "{:?}", + fixed.iter().map(|s| s.context).collect::>() + ); + assert_eq!(fixed[0].context, "statement"); + } + + /// The suppression itself, at the one place it is applied: the record is + /// not a rule-1 candidate, a user record in the same region still is, and + /// with the recogniser unarmed the record comes straight back. + #[test] + fn the_record_is_not_a_ptr_shape_candidate() { + let region = cjs_region(vec![let_const( + 55, + "row", + anon_shape("__AnonShape_row", Vec::new()), + )]); + let seeds = |p: &CjsPreamble| { + super::super::ptr_shape_report::candidate_seeds( + ®ion, + &HashSet::new(), + &HashMap::new(), + p, + ) + }; + let fixed = seeds(&preamble_in_region(®ion)); + assert!(!fixed.contains_key(&CJS_MODULE_ID)); + assert!( + fixed.contains_key(&55), + "a user object literal in the same region must still be a candidate" + ); + assert!(seeds(&CjsPreamble::default()).contains_key(&CJS_MODULE_ID)); + } + + /// The census the `perry` crate's template canary reads. + #[test] + fn the_census_counts_one_preamble_per_wrapped_module() { + let c = census(&cjs_module(preamble_alloc_stmts())); + assert_eq!(c.module_records, 1); + assert_eq!(c.preamble_alloc_stmts, 5); + // An ordinary module has none. + let mut plain = perry_hir::Module::new("m.ts"); + plain.init.push(let_const( + 55, + "row", + anon_shape("__AnonShape_row", Vec::new()), + )); + assert_eq!(census(&plain), CjsPreambleCensus::default()); + } } diff --git a/crates/perry-codegen/src/collectors/mod.rs b/crates/perry-codegen/src/collectors/mod.rs index 7d9abe9970..70d7e4ac62 100644 --- a/crates/perry-codegen/src/collectors/mod.rs +++ b/crates/perry-codegen/src/collectors/mod.rs @@ -6,6 +6,7 @@ //! hub — public-API shape (`crate::collectors::*`) is preserved. mod cjs_scaffolding; +pub use cjs_scaffolding::{census as cjs_preamble_census, CjsPreambleCensus}; mod clamp_detect; mod class_accessors; mod closures; diff --git a/crates/perry-codegen/src/collectors/ptr_shape.rs b/crates/perry-codegen/src/collectors/ptr_shape.rs index 305048c630..abc1ec8f1f 100644 --- a/crates/perry-codegen/src/collectors/ptr_shape.rs +++ b/crates/perry-codegen/src/collectors/ptr_shape.rs @@ -30,6 +30,11 @@ //! carrying a **return-shape fact** is provenance of the same strength — //! that fact certifies the callee hands back a freshly allocated, unaliased //! `C` on every return path (`collectors/ptr_shape_returns.rs`). +//! **One seed is suppressed** (#7152, `collectors/cjs_scaffolding.rs`): +//! `cjs_wrap`'s own `const __cjs_module = { exports: {} }`, and only in a +//! region that also binds the `var module = __cjs_module` alias that denies +//! it under rule 2 — so it removes a value this walk would disqualify +//! anyway. It was 31 % of all candidates on real dependency JS. //! 2. **Containment**: every use of the local is a declared-chain field //! read/write/update or a vetted method call. Any other use — reassignment, //! closure capture, call argument, array/object element, throw, @@ -125,6 +130,7 @@ use std::collections::{HashMap, HashSet}; use perry_hir::{Class, Expr, Stmt}; +use super::cjs_scaffolding::CjsPreamble; use super::ptr_shape_elements::ElementShapeFacts; use super::ptr_shape_report as report; use super::ptr_shape_report::ShapeDenial; @@ -260,6 +266,7 @@ fn report_early_bail( stmts: &[Stmt], boxed_vars: &HashSet, module_globals: &HashMap, + preamble: &CjsPreamble, denial: ShapeDenial, ) { if !opt_report::enabled() { @@ -267,11 +274,11 @@ fn report_early_bail( } let names = report::local_names(stmts); let depths = report::loop_depths(stmts); - let seeds = report::candidate_seeds(stmts, boxed_vars, module_globals); + let seeds = report::candidate_seeds(stmts, boxed_vars, module_globals, preamble); for (id, class_name) in &seeds { report::deny_local(*id, &names, &depths, Some(class_name), denial); } - for site in report::unbound_new_sites(stmts) { + for site in report::unbound_new_sites(stmts, preamble) { report::deny_alloc_site(&site); } } @@ -289,12 +296,19 @@ pub(crate) fn collect_shape_proven_ptr_locals( not_bigint_locals: &HashSet, element_facts: &ElementShapeFacts, ) -> HashMap { - if !ptr_shape_locals_enabled() { - report_early_bail(stmts, boxed_vars, module_globals, report::GATE_DISABLED); - return HashMap::new(); - } - if module_dispatch.has_shape_barrier_sites() { - report_early_bail(stmts, boxed_vars, module_globals, report::MODULE_BARRIER); + // #7152: Perry's own `cjs_wrap` preamble, recognised once for this region. + // One scan of the top-level statement list on anything else, then a + // `Default` that suppresses nothing. See `cjs_scaffolding.rs`. + let preamble = super::cjs_scaffolding::preamble_in_region(stmts); + let bail = if !ptr_shape_locals_enabled() { + Some(report::GATE_DISABLED) + } else if module_dispatch.has_shape_barrier_sites() { + Some(report::MODULE_BARRIER) + } else { + None + }; + if let Some(denial) = bail { + report_early_bail(stmts, boxed_vars, module_globals, &preamble, denial); return HashMap::new(); } // `--opt-report` (#6952): binding names and loop depths for the values @@ -308,16 +322,17 @@ pub(crate) fn collect_shape_proven_ptr_locals( if opt_report::enabled() { // Allocations that are never bound to a local — rule 1 can never see // them, and on real code they are the majority (#7034 §4). - for site in report::unbound_new_sites(stmts) { + for site in report::unbound_new_sites(stmts, &preamble) { report::deny_alloc_site(&site); } } // Pass 1: `Stmt::Let { init: New }` candidates, same seed as scalar // replacement (excludes boxed and module-global locals — which also // excludes async/generator bodies, whose locals are boxed by the - // async-to-generator transform). - let mut candidates: HashMap = HashMap::new(); - super::find_new_candidates(stmts, boxed_vars, module_globals, &mut candidates); + // async-to-generator transform), minus #7152's CommonJS module record. + // Shared with `report_early_bail` so the collector and the report can + // never disagree about what a candidate is. + let mut candidates = report::candidate_seeds(stmts, boxed_vars, module_globals, &preamble); // #7034 §4: `const r = producer(...)` where `producer` carries a // return-shape fact is provenance of `new`-strength (module doc, rule 1). let return_seeded = super::ptr_shape_returns::find_return_shape_candidates( diff --git a/crates/perry-codegen/src/collectors/ptr_shape_report.rs b/crates/perry-codegen/src/collectors/ptr_shape_report.rs index a231d477c4..628fbae0c8 100644 --- a/crates/perry-codegen/src/collectors/ptr_shape_report.rs +++ b/crates/perry-codegen/src/collectors/ptr_shape_report.rs @@ -6,9 +6,12 @@ //! sites) that let those `continue` sites record `(value, reason)` instead of //! dropping the information on the floor. //! -//! **Everything here runs only when [`crate::opt_report::enabled`] is true.** -//! The collector's returned facts are untouched either way: recording happens -//! *next to* the `continue`, never instead of it. +//! **Everything here runs only when [`crate::opt_report::enabled`] is true**, +//! with one deliberate exception: [`candidate_seeds`] is the collector's own +//! rule-1 seeding, shared so that the report and the proof can never disagree +//! about what a candidate is. The collector's returned facts are untouched +//! either way: recording happens *next to* the `continue`, never instead of +//! it. //! //! ## On the actionability tiers //! @@ -29,6 +32,7 @@ use std::collections::{HashMap, HashSet}; use perry_hir::{Class, Expr, Stmt}; +use super::cjs_scaffolding::CjsPreamble; use crate::opt_report::{self, Analysis, Denial, Position, Tier}; /// A named denial: the rule as the collector numbers it, a human expansion, @@ -471,14 +475,30 @@ pub(super) struct NewSite { /// are never bound to a local. **Does not descend into closure bodies** — /// those are lowered as their own regions and reported under their own /// function name. -pub(super) fn unbound_new_sites(stmts: &[Stmt]) -> Vec { +/// +/// `preamble` (#7152) suppresses the object literals Perry's own `cjs_wrap` +/// preamble allocates. On the dependency corpus that is the whole +/// "constructor argument" bucket — `{ exports: {} }`, once per CommonJS +/// module — plus the descriptor / `require.cache` / `require.extensions` +/// literals behind it. Recognition is per region and fails to `Default`, in +/// which case nothing is suppressed. +pub(super) fn unbound_new_sites(stmts: &[Stmt], preamble: &CjsPreamble) -> Vec { let mut out = Vec::new(); - scan_stmts(stmts, 0, "statement", &mut out); + scan_stmts(stmts, 0, "statement", preamble, &mut out); out } -fn scan_stmts(stmts: &[Stmt], depth: u32, ctx: &'static str, out: &mut Vec) { +fn scan_stmts( + stmts: &[Stmt], + depth: u32, + ctx: &'static str, + preamble: &CjsPreamble, + out: &mut Vec, +) { for s in stmts { + if preamble.stmt_allocates_only_scaffolding(s) { + continue; + } match s { // The Let init IS the provenance site rule 1 accepts; skip it and // scan only its arguments. @@ -505,14 +525,14 @@ fn scan_stmts(stmts: &[Stmt], depth: u32, ctx: &'static str, out: &mut Vec { scan_expr(condition, depth, "condition", out); - scan_stmts(then_branch, depth, ctx, out); + scan_stmts(then_branch, depth, ctx, preamble, out); if let Some(eb) = else_branch { - scan_stmts(eb, depth, ctx, out); + scan_stmts(eb, depth, ctx, preamble, out); } } Stmt::While { condition, body } | Stmt::DoWhile { body, condition } => { scan_expr(condition, depth + 1, "condition", out); - scan_stmts(body, depth + 1, ctx, out); + scan_stmts(body, depth + 1, ctx, preamble, out); } Stmt::For { init, @@ -521,7 +541,13 @@ fn scan_stmts(stmts: &[Stmt], depth: u32, ctx: &'static str, out: &mut Vec { if let Some(init) = init { - scan_stmts(std::slice::from_ref(init.as_ref()), depth + 1, ctx, out); + scan_stmts( + std::slice::from_ref(init.as_ref()), + depth + 1, + ctx, + preamble, + out, + ); } if let Some(c) = condition { scan_expr(c, depth + 1, "condition", out); @@ -529,19 +555,19 @@ fn scan_stmts(stmts: &[Stmt], depth: u32, ctx: &'static str, out: &mut Vec { - scan_stmts(body, depth, ctx, out); + scan_stmts(body, depth, ctx, preamble, out); if let Some(c) = catch { - scan_stmts(&c.body, depth, ctx, out); + scan_stmts(&c.body, depth, ctx, preamble, out); } if let Some(f) = finally { - scan_stmts(f, depth, ctx, out); + scan_stmts(f, depth, ctx, preamble, out); } } Stmt::Switch { @@ -553,12 +579,16 @@ fn scan_stmts(stmts: &[Stmt], depth: u32, ctx: &'static str, out: &mut Vec { - scan_stmts(std::slice::from_ref(body.as_ref()), depth, ctx, out) - } + Stmt::Labeled { body, .. } => scan_stmts( + std::slice::from_ref(body.as_ref()), + depth, + ctx, + preamble, + out, + ), _ => {} } } @@ -651,16 +681,26 @@ pub(super) fn admission_cause( None } -/// Enumerate the `Stmt::Let`-bound allocation candidates in a region without -/// running the proof — used on the early-bail paths (gate off, module -/// barrier) so the report can still name what *would* have been considered. +/// The rule-1 candidate seeds of a region. +/// +/// Used by the collector itself and, on the early-bail paths (gate off, module +/// barrier), by the report so it can still name what *would* have been +/// considered. One function, so a value cannot be a candidate for one and not +/// the other. pub(super) fn candidate_seeds( stmts: &[Stmt], boxed_vars: &HashSet, module_globals: &HashMap, + preamble: &CjsPreamble, ) -> HashMap { let mut out = HashMap::new(); super::find_new_candidates(stmts, boxed_vars, module_globals, &mut out); + // #7152: mirror the collector's own filter. Without it a module whose + // rule-5 barrier is armed reports `__cjs_module` under rule 5 while an + // unarmed one reports it under rule 2, and the same scaffolding value + // moves between buckets depending on what the package source happens to + // do. + out.retain(|id, _| !preamble.is_module_record(*id)); out } diff --git a/crates/perry-codegen/src/lib.rs b/crates/perry-codegen/src/lib.rs index 56d5a4b1f5..6d87dde4fe 100644 --- a/crates/perry-codegen/src/lib.rs +++ b/crates/perry-codegen/src/lib.rs @@ -40,6 +40,7 @@ pub use codegen::{ compile_module, resolve_target_triple, AppMetadata, CompileOptions, FpContractMode, ImportedClass, NamespaceEntry, NamespaceEntryKind, }; +pub use collectors::CjsPreambleCensus; /// The shadow-stack field offsets generated code bakes into its inline root /// stores (#7088). @@ -123,3 +124,18 @@ pub fn iter_native_method_signatures() -> impl Iterator pub fn module_has_ptr_shape_barrier(hir: &perry_hir::Module) -> bool { collectors::collect_module_dispatch_facts(hir).has_shape_barrier_sites() } + +/// #7152 template-change canary: what the `Ptr` report suppresses in +/// `hir` as Perry's own `cjs_wrap` scaffolding. +/// +/// Public for exactly the reason [`module_has_ptr_shape_barrier`] is, and with +/// the same failure mode: rename `__cjs_module`, drop the +/// `var module = __cjs_module` alias, or change the `{ exports: {} }` literal, +/// and `collectors::cjs_scaffolding`'s recogniser stops firing. Nothing breaks +/// — the report just goes back to attributing Perry's scaffolding to the +/// user's code, in every CommonJS module, with no symptom. +/// +/// Not part of any codegen contract; nothing in the compile pipeline calls it. +pub fn cjs_preamble_census(hir: &perry_hir::Module) -> CjsPreambleCensus { + collectors::cjs_preamble_census(hir) +} diff --git a/crates/perry/src/commands/compile/cjs_wrap/preamble_canary_tests.rs b/crates/perry/src/commands/compile/cjs_wrap/preamble_canary_tests.rs index 10b29e894b..b9c9482370 100644 --- a/crates/perry/src/commands/compile/cjs_wrap/preamble_canary_tests.rs +++ b/crates/perry/src/commands/compile/cjs_wrap/preamble_canary_tests.rs @@ -18,6 +18,14 @@ //! //! This canary closes that. It runs the real template through the real //! recogniser: wrap → parse → lower → `module_has_ptr_shape_barrier`. +//! +//! #7152 adds the same coupling for the preamble's own ALLOCATIONS. The +//! recogniser there keys on the `__cjs_module` binding name, the +//! `{ exports: {} }` literal, and the `var module = __cjs_module` alias that +//! denies it — three more things this file's template controls and nothing +//! else checks. Its failure mode is quieter still: the report silently goes +//! back to charging Perry's own scaffolding to the user, which is what #7139 +//! and #7149 both misread as evidence about dependency code. use std::path::Path; @@ -100,3 +108,82 @@ fn the_canary_chain_still_reports_a_genuine_barrier() { exemption is far wider than #7139 intended)" ); } + +// ── #7152: the preamble's own allocations ────────────────────────────────── + +/// The record `Let` plus the four allocating preamble statements behind it: +/// `defineProperty(require, 'name', …)`, `require.cache = {}`, +/// `require.extensions = { … }`, and the transpiler's +/// `defineProperty(exports, "__esModule", …)`. +const EXPECTED_PREAMBLE_ALLOC_STMTS: usize = 5; + +/// The #7152 half of the canary. Red means `wrap.rs` and +/// `perry-codegen/src/collectors/cjs_scaffolding.rs` disagree about what the +/// preamble looks like — fix one or the other, do not delete this test. +#[test] +fn the_cjs_preamble_is_still_recognised_as_scaffolding_allocation() { + let path = Path::new("/tmp/perry-canary/node_modules/dep/index.js"); + let wrapped = wrap_commonjs_for_target(CJS_FIXTURE, path, None); + + // Anti-vacuity on the template, one assertion per recogniser conjunct, so + // a template edit names the conjunct it broke rather than failing as an + // opaque count mismatch. + for (needle, conjunct) in [ + ( + "const __cjs_module = { exports: {} };", + "R1/R2 (the record and its `{ exports: {} }` literal)", + ), + ( + "var module = __cjs_module;", + "R4 (the alias that denies the record)", + ), + ("require.cache = {}", "the `require.cache` allocation"), + ( + "require.extensions = {", + "the `require.extensions` allocation", + ), + ] { + assert!( + wrapped.contains(needle), + "the CJS preamble no longer emits `{needle}`.\n\ + That is conjunct {conjunct} of the #7152 recogniser in \ + perry-codegen/src/collectors/cjs_scaffolding.rs. Either update the \ + recogniser to match the new template, or — if the statement is \ + gone for good — delete its arm there and lower \ + EXPECTED_PREAMBLE_ALLOC_STMTS here." + ); + } + + let hir = wrap_and_lower(CJS_FIXTURE); + let census = perry_codegen::cjs_preamble_census(&hir); + assert_eq!( + census.module_records, 1, + "the `Ptr` report no longer recognises `const __cjs_module = \ + {{ exports: {{}} }}` as Perry's own scaffolding. Every CommonJS module \ + is now back to reporting it as a denied user candidate (#7152)." + ); + assert_eq!( + census.preamble_alloc_stmts, EXPECTED_PREAMBLE_ALLOC_STMTS, + "the number of recognised preamble allocation statements changed. \ + Compare `cjs_preamble` in wrap.rs against \ + `CjsPreamble::stmt_allocates_only_scaffolding`." + ); +} + +/// Positive control: the recogniser is per module, not a constant. A module +/// that was never `cjs_wrap`ped has no preamble at all — without this, the +/// assertions above would pass just as well against a recogniser that counted +/// every module as scaffolding. +#[test] +fn a_module_that_was_never_cjs_wrapped_has_no_preamble() { + let ast = perry_parser::parse_typescript( + "class Point { x = 0; }\nexport const p = new Point();\n", + "plain.ts", + ) + .expect("fixture must parse"); + let hir = perry_hir::lower_module(&ast, "plain", "/tmp/perry-canary/plain.ts") + .expect("fixture must lower"); + let census = perry_codegen::cjs_preamble_census(&hir); + assert_eq!(census.module_records, 0); + assert_eq!(census.preamble_alloc_stmts, 0); +} From 6166b2ac5baae62e1f5f3bb77df8f7e9b2290ac7 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Ralph=20K=C3=BCpper?= Date: Sat, 1 Aug 2026 09:14:25 +0200 Subject: [PATCH 2/6] docs(changelog): 7171 fragment --- ...171-cjs-preamble-ptr-shape-report-noise.md | 139 ++++++++++++++++++ 1 file changed, 139 insertions(+) create mode 100644 changelog.d/7171-cjs-preamble-ptr-shape-report-noise.md diff --git a/changelog.d/7171-cjs-preamble-ptr-shape-report-noise.md b/changelog.d/7171-cjs-preamble-ptr-shape-report-noise.md new file mode 100644 index 0000000000..afbf7c6100 --- /dev/null +++ b/changelog.d/7171-cjs-preamble-ptr-shape-report-noise.md @@ -0,0 +1,139 @@ +`--opt-report`'s `Ptr` section stops charging Perry's own CommonJS +wrapper to the user's code. Over 195 real `__esModule` dependency modules from `scriptc/node_modules`, +**31 % of all `Ptr` candidates (379 of 1231) were two statements the +`cjs_wrap` template emits itself** — and they were the evidence behind two +scheduling decisions that turned out to be about scaffolding rather than about +dependency code. + +| denial bucket | base | after | +|---|---:|---:| +| rule 2 — bare reference | 140 | **4** | +| rule 1 — unbound alloc, constructor argument | 196 | **8** | +| rule 5 — module barrier | 187 | **132** | +| rule 1 — unbound alloc, all positions | 885 | **759** | +| total `ptr-shape` candidates | 1231 | **914** | +| selected / consumed | 3 / 11 | **3 / 11** | + +Every removed denial is named: 136 of the 140 rule-2 bare references were the +local `__cjs_module`, as were 55 of the rule-5 denials, and 188 of the +constructor-argument allocations were the `{}` inside +`const __cjs_module = { exports: {} }` — 379 rows over 195 modules, almost +exactly two per module. Zero `__cjs_module` rows remain in any bucket. The net +drop is 317 rather than 379 because removing them **un-masks 62 real user +rows** (see below). Selections and consumptions are unchanged, which is the +point: nothing here promotes anything. + +## The shape + +`cjs_wrap`'s preamble emits, into every wrapped module: + +```js +const __cjs_module = { exports: {} }; +var module = __cjs_module; // ← the rule-2 "bare reference" +… +require.main = module; +``` + +`__cjs_module` is a `Stmt::Let` of an object literal, so rule 1 seeds it as a +candidate; the next statement aliases it into a `var`, so rule 2 denies it. +The inner `{}` is an allocation in constructor-argument position, so rule 1 +denies that too. Both fire once per CommonJS module, in every CommonJS module. + +**The reference is not exemptable, and that is a finding, not a limitation.** +`module` is a reassignable `var` that the preamble goes on to store into +`require.main`, and that CommonJS bodies write `module.exports = X` through. +The record genuinely escapes; no narrowing of rule 2 could promote it. What is +wrong is that it was ever a *candidate* — promotion would buy nothing (a +one-field record, read three times at module-init, loop depth 0) and can never +be proven. So it is suppressed at the seed, not exempted at the rule. + +## The recogniser (`collectors/cjs_scaffolding.rs`, extends #7139) + +A region is a CommonJS preamble when its top level carries **R1** a +`mutable: false` `Let` named `__cjs_module` initialized by an `__AnonShape_…` +allocation, **R2** whose literal is exactly `{ exports: {} }`, **R3** uniquely +in the region, and **R4** aliased by a `mutable: true` `Let` named `module` +whose init is a bare `LocalGet` of it. + +**R4 is the soundness argument, not a heuristic.** It *is* the denial: that +statement shape walks into `UseWalk`'s `LocalGet` arm under the default escape +context and disqualifies the record on every path, and `mutable: true` keeps +the alias pre-pass from tracking it. A region satisfying R4 cannot promote its +record, so removing it from the candidate set leaves the returned facts +bit-identical. R1-R3 only make the recognition unambiguous. Dropping a +candidate can only remove facts, never add one — the opposite direction from +#7139's barrier exemption, which relaxed a proof. + +Recognised, the region also stops reporting the object literals of the four +preamble statements behind the record: the two `defineProperty` sites #7139 +already recognises (same predicate, so the two exemptions cannot disagree about +what scaffolding is) plus `require.cache = {}` and `require.extensions = { … }`. +That half is report-only in the strongest sense — `unbound_new_sites` runs +only under `opt_report::enabled()`. + +## Why the whole preamble and not just the record + +`--opt-report` dedups allocation-site rows per function on +`(module, function, name, position, rule)`, and every object literal renders as +`object literal { ... }`. So each wrapped module contributes exactly **one** +alloc-site row, and which context it reports is whichever preamble literal the +walk reaches first. Suppressing only the record would have moved the row from +`constructor argument` to `statement` and changed the total by zero. The same +dedup is why `statement` rises 273 → 320 here: with the scaffolding row gone, +a *user* allocation that was being masked by it surfaces. Removing 317 +scaffolding rows made 47 real ones visible. + +## Evidence + +- **Codegen-neutral, measured**: emitted LLVM IR compared base vs fix over 49 + dependency modules, each with a same-compiler control run — 47 identical, 0 + different, 2 excluded because their own control run differs (HashMap-ordered + `perry_method___` emission when one method name is shared across + classes; pre-existing, #7131 family). The one nondeterminism that IS + normalised is HashMap-ordered `js_register_function_name` string constants, + 16 of ~12 900 lines; nothing else is. Object-file hashes are unusable here — + the `--no-link` temp output path lands in the Mach-O debug records, so the + same compiler run twice differs by ~2 KB. +- **Behaviour**: a CJS dependency fixture (`__esModule`, class, record, + 1000-iteration loop) compiled and run by both arms — byte-identical output, + byte-exact against Node 26.5.1. Report on the same fixture: 5 candidates / + 2 denied → 3 candidates / 0 denied, with the *same three* selections. +- **Sabotage, 16 rows, each with a named red set**: every conjunct deleted or + weakened in turn (R1 mutability / name / literal-ness, R2 arity / emptiness, + R3 uniqueness, R4 presence / mutability / identity, the `require`-key + whitelist, the candidate filter, the alloc-site skip) plus four template + edits in `wrap.rs`. Control green in all 17 runs. The first pass had **four + green holes**; all four were fixed in the code, not the tests — the binding + name and the uniqueness count each had two enforcement points that masked one + another, and the "is a record recognised" gate was a removable `if` that no + test could kill. The record and its scaffolding bindings now live in one + `Option<(u32, CjsScaffolding)>`, so "unrecognised but still suppressing" has + no representation. +- **Template canary** (`cjs_wrap/preamble_canary_tests.rs`, extending #7139's): + runs the real template through the real recogniser, with one anti-vacuity + assertion per conjunct naming the conjunct it broke, and a negative control + that a never-wrapped module recognises nothing. +- `cargo test -p perry-codegen --lib`: 472 passed (30 new). Census green, no + floor moved. + +## What this does not do + +Nothing is promoted. Conversion to selected/consumed is **zero**, by +construction: every suppressed value was denied in the base arm too, which is +exactly what R4 asserts. + +The dependency-JS wall stays where #7152 put it — rule 1, allocations never +bound to a local — but its size needs restating. In this corpus rule 1 goes +885 → 759: 188 base rows were the `{ exports: {} }` literal and 62 previously +masked user rows appeared. #7152's corpus is a different 180-module sample, so +its `506` is not directly comparable; by the same per-module rate, on the order +of 180 of those 506 are the same scaffolding literal, and the residual user +rule-1 population there is smaller than 506 but larger than `506 − 180`. Anyone +re-running that measurement should re-derive it against this compiler rather +than subtracting. + +`gc_repsel_matrix.sh --arms all --pressure 8` was started and aborted during its +compile phase: a sibling agent was running the identical 21-arm matrix +concurrently and host free disk had fallen below the campaign floor. For a +change that emits identical code the IR comparison above is the stronger +statement, but the matrix columns are unmeasured and are recorded as such. From 8845154fc80be7546d323bab99766c8bf442772d Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Ralph=20K=C3=BCpper?= Date: Sat, 1 Aug 2026 09:15:51 +0200 Subject: [PATCH 3/6] style(codegen): move the cjs preamble census re-export to the public block --- crates/perry-codegen/src/collectors/mod.rs | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/crates/perry-codegen/src/collectors/mod.rs b/crates/perry-codegen/src/collectors/mod.rs index 70d7e4ac62..0a27941c89 100644 --- a/crates/perry-codegen/src/collectors/mod.rs +++ b/crates/perry-codegen/src/collectors/mod.rs @@ -6,7 +6,6 @@ //! hub — public-API shape (`crate::collectors::*`) is preserved. mod cjs_scaffolding; -pub use cjs_scaffolding::{census as cjs_preamble_census, CjsPreambleCensus}; mod clamp_detect; mod class_accessors; mod closures; @@ -44,6 +43,7 @@ mod this_as_value; mod uppercase_strings; // Public re-exports for the visible API (`pub fn emit_i64_function` etc.). +pub use cjs_scaffolding::{census as cjs_preamble_census, CjsPreambleCensus}; pub use clamp_detect::{ detect_clamp3, detect_clamp_u8, is_integer_specializable, returns_i32_identity_arg, returns_integer, From 692d9ffb39c9164be89c1dfb571f0e018fc16ebd Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Ralph=20K=C3=BCpper?= Date: Sat, 1 Aug 2026 09:18:12 +0200 Subject: [PATCH 4/6] docs(repsel): say why the preamble recogniser is not gated on --opt-report --- crates/perry-codegen/src/collectors/cjs_scaffolding.rs | 8 ++++++++ 1 file changed, 8 insertions(+) diff --git a/crates/perry-codegen/src/collectors/cjs_scaffolding.rs b/crates/perry-codegen/src/collectors/cjs_scaffolding.rs index 97021c2488..9fe1855c37 100644 --- a/crates/perry-codegen/src/collectors/cjs_scaffolding.rs +++ b/crates/perry-codegen/src/collectors/cjs_scaffolding.rs @@ -357,6 +357,14 @@ impl CjsPreamble { /// single pass over the region's top-level statements looking for R1/R2. Only /// `cjs_wrap` output binds `__cjs_module` to that literal, so on ordinary /// TypeScript this returns `Default` after one `Vec` scan. +/// +/// Deliberately NOT gated on [`crate::opt_report::enabled`], even though the +/// only observable effect is on the report. Gating it would make the candidate +/// set differ between a reporting build and an ordinary one — the facts would +/// still be identical (that is R4's argument), but "the report describes a +/// different compile than the one you ran" is the exact confusion this whole +/// report exists to remove. The cost of not gating it is one string compare per +/// top-level statement per region. pub(super) fn preamble_in_region(stmts: &[Stmt]) -> CjsPreamble { // R1 + R2 (`record_binding`), on top-level bindings of R1's name. let records: Vec = stmts From 313294eafb21a1e1b9e9efa00ddedb47029fb9da Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Ralph=20K=C3=BCpper?= Date: Sat, 1 Aug 2026 09:19:38 +0200 Subject: [PATCH 5/6] docs(changelog): rewrap 7171 fragment intro --- .../7171-cjs-preamble-ptr-shape-report-noise.md | 10 +++++----- 1 file changed, 5 insertions(+), 5 deletions(-) diff --git a/changelog.d/7171-cjs-preamble-ptr-shape-report-noise.md b/changelog.d/7171-cjs-preamble-ptr-shape-report-noise.md index afbf7c6100..edc57b8ac7 100644 --- a/changelog.d/7171-cjs-preamble-ptr-shape-report-noise.md +++ b/changelog.d/7171-cjs-preamble-ptr-shape-report-noise.md @@ -1,9 +1,9 @@ `--opt-report`'s `Ptr` section stops charging Perry's own CommonJS -wrapper to the user's code. Over 195 real `__esModule` dependency modules from `scriptc/node_modules`, -**31 % of all `Ptr` candidates (379 of 1231) were two statements the -`cjs_wrap` template emits itself** — and they were the evidence behind two -scheduling decisions that turned out to be about scaffolding rather than about -dependency code. +wrapper to the user's code. Over 195 real `__esModule` dependency modules from +`scriptc/node_modules`, **31 % of all `Ptr` candidates (379 of 1231) +were two statements the `cjs_wrap` template emits itself** — and they were the +evidence behind two scheduling decisions that turned out to be about +scaffolding rather than about dependency code. | denial bucket | base | after | |---|---:|---:| From 698c6041037cf07c939c724e1cf80d9addebdf9c Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Ralph=20K=C3=BCpper?= Date: Sat, 1 Aug 2026 09:26:18 +0200 Subject: [PATCH 6/6] docs(changelog): record the pinned baseline for the 7171 measurement --- changelog.d/7171-cjs-preamble-ptr-shape-report-noise.md | 6 ++++++ 1 file changed, 6 insertions(+) diff --git a/changelog.d/7171-cjs-preamble-ptr-shape-report-noise.md b/changelog.d/7171-cjs-preamble-ptr-shape-report-noise.md index edc57b8ac7..da74ca6e7c 100644 --- a/changelog.d/7171-cjs-preamble-ptr-shape-report-noise.md +++ b/changelog.d/7171-cjs-preamble-ptr-shape-report-noise.md @@ -137,3 +137,9 @@ compile phase: a sibling agent was running the identical 21-arm matrix concurrently and host free disk had fallen below the campaign floor. For a change that emits identical code the IR comparison above is the stronger statement, but the matrix columns are unmeasured and are recorded as such. + +Both arms of the table are pinned at `df7214b0d`, differing only by this patch. +A re-measurement on the rebased HEAD keeps the scaffolding result exactly (rule-2 +bare reference 4, zero `__cjs_module` rows) but moves the other buckets, because +an unrelated `main` change made `rollup/dist/shared/index.js` compile where it +had failed in both original arms. Different baseline, not a different result.