perf(repsel): exempt CommonJS module scaffolding from the Ptr<Shape> rule-5 barrier

[proggeramlug]

Narrows the Ptr<Shape> rule-5 module-wide barrier so that the
defineProperty calls every cjs_wrap-compiled CommonJS module carries stop
disabling shape promotion for the whole module. Closes the #7034 §2 barrier
narrowing item.

What was actually blocking

#7034 recorded a source-level scan: Object.defineProperty(exports, "__esModule", { value: true }) appears in 52 % of sampled dependency JS.
Reproducing that on scriptc/node_modules (1841 JS files): 718 have at least
one barrier-family site, and 403 of those 718 (56 %) have no barrier other
than that one line.

But exempting only that line would have recovered nothing, because Perry
arms the barrier itself. cjs_wrap's preamble emits

Object.defineProperty(require, 'name', { value: 'require', writable: false, … });

into every wrapped module — crates/perry/src/commands/compile/cjs_wrap/wrap.rs:841,
inside the shared cjs_preamble used by both the IIFE wrap and the flat emit.
Confirmed in HIR: a trivial CJS dependency lowers to
Expr(ObjectDefineProperty(LocalGet(10), String("name"), …)) before a single
line of package source runs. So shape_barrier_sites was true in 100 % of
the CommonJS dependency graph regardless of what the package did.

Both sites are recognised here. Nothing else is.

The predicate

An Expr::ObjectDefineProperty(target, key, desc) node is exempt iff

• target is Expr::LocalGet(id);
• the module binds id with a Stmt::Let named exports (resp. require);
• every Stmt::Let binding of id in the module has an initializer that
  is not a fresh allocation at all — PropertyGet
  (var exports = __cjs_module.exports), Closure
  (function require(specifier) {…}), Undefined, or absent;
• key is the string literal "__esModule" (resp. "name").

Any other target, any other key, a computed key, and every other barrier family
(delete, setPrototypeOf / __proto__ write, new Proxy, mutating
Reflect.*) keep the module-wide kill untouched.

Soundness

A defineProperty can only invalidate a Ptr<Shape> proof if the object it
mutates is the object the promoted local holds. Two independent facts rule
that out:

1. The target is not a candidate. Ptr<Shape> candidates are seeded by
   find_new_candidates from Stmt::Let { init: Some(Expr::New { .. }), .. }.
   The third clause above is deliberately a whitelist, not a blacklist of
   Expr::New: rule 1's seed set is a moving target — repsel row 1: scoping Ptr<Shape> beyond locals — measured, promotion is 0 on the motivating workload; build --opt-report (#6952) first #7034 §4 already landed
   return-shape facts (ModuleDispatchFacts::return_shape_class) under which a
   call to a proven function is a provenance seed — and a blacklist would
   silently widen this exemption the day such a seed is wired into the
   candidate search. A whitelist fails closed. It is checked on the HIR rather
   than on an expectation about the wrap template, so if cjs_wrap ever
   changes shape the exemption degrades to no exemption, never to an unsound
   one.
2. No promoted object can reach the target. Rule 2 (containment) admits a
   local only when every use is a declared-chain field read/write/update or a
   vetted method call; reassignment, aliasing, capture, and passing it as a
   call/constructor argument all disqualify. A promoted object therefore never
   flows into another binding, so it can never be the value of exports or
   require.

The descriptor argument is deliberately unconstrained: a descriptor is a plain
value, and if it contains an accessor closure the module walk descends into
that closure's body on its own, so a barrier inside a descriptor still arms
the flag.

shape_barrier_sites is also read by ptr_numarray and proven_this. The
argument is about the identity of the mutated object, not about which analysis
reads the flag, so it carries over unchanged.

Measurement — eligibility and promotion, separately

85 self-contained __esModule CJS leaves from scriptc/node_modules, each
compiled as a compilePackages dependency by both arms
(--opt-report=json --no-link, 85/85 compiled in both):

	base	fix
modules with a rule-5 denial	85	1
ptr-shape denials: rule 5 (module barrier)	117	14
ptr-shape denials: rule 1 (provenance)	154	154
ptr-shape denials: rule 2 (containment)	0	100
distinct ptr-shape selections	0	18 (in 2 modules)
ptr-shape consumptions	0	16 (in 1 module)

Eligibility recovered: 84 of 85 modules (98.8 %). The one holdout has a
genuine barrier of its own.

Promotion gained: small, and that is the finding. Of the ~103 candidates the
barrier stopped denying, 100 were immediately re-denied by rule 2
(containment). The 18 that got through, named:

• @eslint-community/regexpp/index.js — 15 proven-this receivers
  (BranchID.sibling, GroupSpecifiersAsES2018/2025.*, RegExpVisitor.visit*),
  16 consumption sites. This is the only module in the corpus that gained
  consumed promotions.
• typescript/vendor/vscode-jsonrpc/lib/common/linkedMap.js — 3 anon-shape
  record locals (result, the { value, done } iterator result in three
  closures), selected and consumed zero times (0 numeric fields proven).

So the honest one-liner is: eligibility recovered ~99 %, promotion gained 18
values in 2 of 85 modules, of which 16 consumptions all land on proven-this
and zero on a Ptr<Shape> local. Containment, not the barrier, is the wall in
minified dependency code — this redirects effort to Track D rather than to more
barrier work.

IR evidence for the claimed consumption

Object hashes and counters are not enough, so the call sites were checked.
A minimal CJS dependency (class Point, function compute(n) { const p = new Point(n, n+1); p.x = p.x + 1; return p.sum() + p.x + p.y; }), --trace llvm,
function perry_closure_…_17 (the compute body):

	base	fix
IR lines in the body	354	136
js_typed_feedback_class_field_get_guard	3	0
js_typed_feedback_class_field_set_guard	1	0
js_typed_feedback_record_fallback_call	3	0
js_object_get_field_by_name_f64	3	0
js_method_direct_shape_guard	1	0
js_native_call_method_by_id	1	0
method callee	…Point__sum	…Point__sum__pshape

The __pshape clone appears only in the fix arm — Phase 5a's proven-this
admission consumes the same flag, as expected.

Sabotage

New tests, run against the unfixed rule (&& !cjs.exempts_shape_barrier(expr)
removed): exactly the 3 exemption-dependent tests go red
(cjs_scaffolding_define_property_sites_do_not_arm_the_module_barrier,
each_scaffolding_site_is_exempt_on_its_own,
an_eligible_local_promotes_in_a_module_carrying_only_scaffolding_sites) and
the 8 negative/anti-vacuity tests stay green — i.e. they are not asserting
something the unfixed compiler already did. Restored, 11/11 pass.
the_same_local_is_denied_when_a_real_barrier_is_present sabotages the other
direction: one delete anywhere in the same module still denies the same
local, so the promotion test is not vacuous.

Template-change canary

The require / "name" arm is coupled to cjs_wrap's preamble template by
nothing but a matching binding name, initializer shape and property key. Rename
the local, change the key, or bind it through anything but a function
declaration and the exemption silently stops applying: nothing breaks, no test
fails, and the rule-5 barrier re-arms for 100 % of CommonJS modules. Since
that site is the one arming every module, silent drift costs the entire win with
no symptom — the "gate that cannot fail" shape CLAUDE.md documents.

cjs_wrap::preamble_canary_tests closes it by running the real template through
the real recogniser: wrap → parse → lower → module_has_ptr_shape_barrier,
asserting the barrier is not armed. Two anti-vacuity guards:

• the wrapped source must still contain a defineProperty(require, …) site,
  so the canary cannot pass trivially the day the template stops emitting it and
  leaves that arm of the recogniser as dead code nobody notices;
• a positive control asserts the same chain still reports a genuine delete, so
  an empty or failed lowering cannot make the canary pass for the wrong reason.

Sabotage-verified in four independent directions, positive control green in
all four:

perturbation	canary	which assertion fired
template: 'name' → 'displayName'	RED	barrier re-armed
template: defineProperty(require, …) block deleted	RED	"no longer emits … dead code"
recogniser: REQUIRE_KEY changed	RED	barrier re-armed
recogniser: EXPORTS_KEY changed	RED	barrier re-armed

perry_codegen::module_has_ptr_shape_barrier is public solely for this —
the recogniser is in perry-codegen, the template is in perry, and only
perry can see both. Same shape as iter_native_method_signatures, which
exists for perry-api-manifest's consistency test; nothing in the compile
pipeline calls it.

Adjacent question answered: does the barrier walk run before or after DCE?

After — but Perry has no dead-code elimination in a default build, so the
walk sees everything.

• collect_module_dispatch_facts is called at
  crates/perry-codegen/src/codegen/mod.rs:1385, on the fully-transformed HIR.
• The only two dead-code passes in the pipeline are both opt-in and default
  off (tree_shake: false, crates/perry/src/commands/compile/types.rs:1146;
  set only by PERRY_TREE_SHAKE or perry.experiments.treeShake,
  host_config.rs:44/:212):
  • reachability::tree_shake (compile/reachability.rs:45, called from
    run_pipeline.rs:375) — prunes whole modules, never intra-module code;
  • env_fold::fold_env_branches (compile/env_fold.rs:30, called from
    collect_modules.rs:779) — splices statically-false process.env if
    branches.
• There is no unreachable-branch or unreferenced-function pruner.
  collect_module_dispatch_facts (scalar_method_dispatch.rs:181-220) walks
  hir.init, every entry of hir.functions whether called or not, and
  every class member body.

Quantified before deciding: over the same corpus, of 1750 barrier sites in
718 files, 12 (0.7 %) sit inside a statically-dead if (if (false) /
if (0) / dead process.env.NODE_ENV guards), and zero files go from
barrier-armed to barrier-free if all of them are removed. Making the walk
DCE-aware would need an intra-module call graph and would buy nothing measurable
here, so this PR does not attempt it; the numbers are recorded rather than an
issue filed for a non-problem.

Validation

• cargo test -p perry-codegen --lib — 417 passed (11 new); cargo test -p perry --bin perry preamble_canary — 2 passed (the canary above).
• cargo test -p perry-hir -p perry-transform --lib — 266 + 54 passed.
• cargo clippy -p perry-codegen --all-targets — no warnings on the changed files.
• Census gate (compiler_output_regression.py census --gate) — Census OK;
  batch ptr-shape = 2 asserted programmatically against its floor of 2;
  benchmarks/repsel_census/baseline.json untouched.
• Behavioural A/B vs Node 26.5.1 (.node-version), byte-identical output
  on both arms and node:
  • the synthetic CJS probe — 16 / 16 / 16;
  • @eslint-community/regexpp (the module that gained all 16 consumptions),
    compiled as a compilePackages dependency and asked to parse
    /(?<y>a|b)+c[d-f]\\1/gu and validate (?<z>x)\\k<z> —
    RegExpLiteral true true (?<y>a|b)+c[d-f]\\1 + validated from node,
    base perry and fix perry alike.
• Both arms built -p perry -p perry-runtime-static -p perry-stdlib-static in
  separate CARGO_TARGET_DIRs; binary hashes verified distinct
  (b1b2e6d5… vs 51ebdde7…) before any A/B was believed.
• Emitted LLVM IR is byte-identical between the two arms on pure-TS
  workloads (benchmarks/app-patterns/kernels/batch.ts,
  test_gap_repsel_canonical_i32.ts, test_gap_repsel_gc_stress.ts) — codegen
  changes only where CJS scaffolding exists.
• CJS integration tests: issue_4872_barrel_default_reexports,
  issue_5257_require_adopt_no_default_namespace,
  issue_6585_cjs_class_forward_function — 4 passed.
• scripts/gc_repsel_matrix.sh --arms all --pressure 8 — FAIL=0
  (PASS=426, UNVER=119, XFAIL=1), byte-exact vs node 26.5.1 on 545/546 cells,
  with the requires=move arms live (25/26 copy-minor). Run twice, before and
  after the whitelist hardening; identical both times.
• gc-ratchet: all 8 probes in benchmarks/gc_ratchet/probes/ emit
  byte-identical LLVM IR under both arms (per-probe hashes, all 8 distinct from
  each other — so the hashing is reading real content, the perf(codegen): module-init / program-entry bodies select canonical i32/u32/Str (#7109) #7121 anti-vacuity
  precedent), i.e. 0/8 can move the collector measurement.
• scripts/addr_class_inventory.py — exit 0.
• scripts/check_file_size.sh — the 16 failures are pre-existing on main;
  none of the files in this PR are among them (1881 / 810 / 606 lines).

Not measured

• Only the 85 self-contained __esModule leaves of one corpus were
  A/B'd — modules with require() edges need their whole dependency tree
  present and were out of scope for this pass.
• No wall-clock benchmark: 19 selections in 2 dependency modules is below the
  noise floor of anything in benchmarks/, and claiming a speedup from it
  would be dishonest.

https://claude.ai/code/session_018ZFER8EEg8K7ez2n6oDrT9

[coderabbitai]

📝 Walkthrough

Walkthrough

The change adds HIR-based detection for two CommonJS scaffolding patterns. It excludes eligible sites from module-wide shape barriers, preserves other barriers, integrates the analysis into dispatch collection, and adds unit, promotion, and end-to-end validation.

Changes

CommonJS shape-barrier handling

Layer / File(s)	Summary
Scaffolding detection and binding collection crates/perry-codegen/src/collectors/cjs_scaffolding.rs, crates/perry-codegen/src/collectors/mod.rs	Adds CjsScaffolding to recognize valid exports.__esModule and require.name definitions. It validates bindings, initializers, literal keys, nested statements, class members, and closures.
Barrier classification integration crates/perry-codegen/src/collectors/scalar_method_dispatch.rs, crates/perry-codegen/src/collectors/ptr_shape.rs	Passes scaffolding facts through module expression collection. Exempt sites no longer arm the shape barrier, while prototype, freeze, NumArray, and unrelated defineProperty barriers remain active.
Barrier regression and promotion validation crates/perry-codegen/src/collectors/cjs_scaffolding.rs, changelog.d/7139-cjs-scaffolding-barrier-exemption.md	Adds tests for valid and invalid patterns, promotion behavior, module measurements, traversal timing, IR reductions, and byte-identical behavior checks.

Estimated code review effort: 3 (Moderate) | ~25 minutes

Sequence Diagram(s)

sequenceDiagram
  participant ModuleExpressions
  participant CjsScaffolding
  participant ScalarMethodDispatch
  participant PtrShapePromotion
  ModuleExpressions->>CjsScaffolding: Collect bindings and HIR expressions
  CjsScaffolding-->>ScalarMethodDispatch: Provide scaffolding facts
  ScalarMethodDispatch->>CjsScaffolding: Classify defineProperty expressions
  CjsScaffolding-->>ScalarMethodDispatch: Return exemption results
  ScalarMethodDispatch->>PtrShapePromotion: Record remaining barriers

Loading

Possibly related PRs

• PerryTS/perry#6911: Adds the shape-proven Ptr<Shape> promotion infrastructure extended by this change.

🚥 Pre-merge checks | ✅ 5

✅ Passed checks (5 passed)

Check name	Status	Explanation
Docstring Coverage	✅ Passed	No functions found in the changed files to evaluate docstring coverage. Skipping docstring coverage check.
Linked Issues check	✅ Passed	Check skipped because no linked issues were found for this pull request.
Out of Scope Changes check	✅ Passed	Check skipped because no linked issues were found for this pull request.
Title check	✅ Passed	The title clearly and concisely identifies the CommonJS scaffolding exemption from the Ptr rule-5 barrier.
Description check	✅ Passed	The description clearly covers the change, related issue, implementation details, tests, measurements, validation, and limitations.

✨ Finishing Touches

🧪 Generate unit tests (beta)

• Create PR with unit tests
• Commit unit tests in branch perf/7139-esmodule-barrier-exemption

---

Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out.

❤️ Share

• X
• Mastodon
• Reddit
• LinkedIn

_{Comment @coderabbitai help to get the list of available commands.}

[coderabbitai]

🧹 Nitpick comments (2)

crates/perry-codegen/src/collectors/scalar_method_dispatch.rs (1)

193-204: 📐 Maintainability & Code Quality | 🔵 Trivial | 🏗️ Heavy lift

Duplicated module-traversal structure with cjs_scaffolding::collect.

collect_module_dispatch_facts walks hir.init, every function body, and every class member body — the same structural walk that cjs_scaffolding::collect (crates/perry-codegen/src/collectors/cjs_scaffolding.rs, lines 121-163) independently re-implements to gather exports/require bindings. Two independent implementations of the same module walk must be kept in sync by hand whenever Module or Class gains new fields.

🤖 Prompt for AI Agents

Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.

In `@crates/perry-codegen/src/collectors/scalar_method_dispatch.rs` around lines
193 - 204, Refactor collect_module_dispatch_facts and cjs_scaffolding::collect
to share one module-body traversal helper covering hir.init, function bodies,
and class member bodies. Update both collectors to consume that shared traversal
so future Module or Class fields are handled consistently, without changing
their collector-specific behavior.

crates/perry-codegen/src/collectors/cjs_scaffolding.rs (1)

121-163: 📐 Maintainability & Code Quality | 🔵 Trivial | 🏗️ Heavy lift

Duplicated module-traversal structure with collect_module_dispatch_facts.

collect re-implements the same walk over module.init, every function body, every class constructor/method/getter/setter/computed-member body, and every field/computed-member key expression that collect_module_dispatch_facts in scalar_method_dispatch.rs already performs. The doc comment above (lines 116-120) even states this function "Mirrors … collect_module_dispatch_facts's coverage", which means any future change to Class or Module shape requires updating both traversals in lockstep, or the two collectors silently diverge.

Extract a shared "walk every function/method/constructor body and field/computed-member expression in a module" helper that both collect_module_dispatch_facts and collect can drive with different per-node callbacks. This removes the manual-mirroring requirement documented in the comment.

🤖 Prompt for AI Agents

Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.

In `@crates/perry-codegen/src/collectors/cjs_scaffolding.rs` around lines 121 -
163, Extract a shared module traversal helper covering module initialization,
function bodies, class constructors, methods/getters/setters/computed-member
bodies, and field/computed-member key expressions. Refactor both collect and
collect_module_dispatch_facts to use this helper with their respective per-node
callbacks, removing the duplicated traversal logic while preserving each
collector’s existing behavior.

🤖 Prompt for all review comments with AI agents

Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.

Nitpick comments:
In `@crates/perry-codegen/src/collectors/cjs_scaffolding.rs`:
- Around line 121-163: Extract a shared module traversal helper covering module
initialization, function bodies, class constructors,
methods/getters/setters/computed-member bodies, and field/computed-member key
expressions. Refactor both collect and collect_module_dispatch_facts to use this
helper with their respective per-node callbacks, removing the duplicated
traversal logic while preserving each collector’s existing behavior.

In `@crates/perry-codegen/src/collectors/scalar_method_dispatch.rs`:
- Around line 193-204: Refactor collect_module_dispatch_facts and
cjs_scaffolding::collect to share one module-body traversal helper covering
hir.init, function bodies, and class member bodies. Update both collectors to
consume that shared traversal so future Module or Class fields are handled
consistently, without changing their collector-specific behavior.

---

ℹ️ Review info

⚙️ Run configuration

Configuration used: defaults

Review profile: CHILL

Plan: Pro Plus

Run ID: 7fafd556-65cd-48e0-b4ee-56bb1af6a6ac

📥 Commits

Reviewing files that changed from the base of the PR and between a3b31c0 and c3be0f6.

📒 Files selected for processing (5)

• changelog.d/7139-cjs-scaffolding-barrier-exemption.md
• crates/perry-codegen/src/collectors/cjs_scaffolding.rs
• crates/perry-codegen/src/collectors/mod.rs
• crates/perry-codegen/src/collectors/ptr_shape.rs
• crates/perry-codegen/src/collectors/scalar_method_dispatch.rs