repsel: one safety gate and one write walker for both numeric proofs, plus the #7770 super-chain test and bench numbers

changelog.d/7774-element-group-numeric-proof.md

@@ -1,4 +1,4 @@
-**repsel: element-group members now claim numeric fields — proven reads drop `js_number_coerce` (#7770, PR #7774).**
+**repsel: element-group members now claim numeric fields (~10-12% on the target read loop) — proven reads drop `js_number_coerce` (#7770, PR #7774).**
 
 A `Ptr<Shape>`-proven `const r = a[i]` (or a producer pushed into an
 element-shape-proven array) stood down to zero numeric fields, so every
@@ -26,7 +26,14 @@ byte-identical vs Node 26.5.1 across sibling/push-site/method poison
 channels, NaN / Infinity / −0 payloads, and `null`/`{}`/`1n`/`true` stores
 (`test-files/test_gap_repsel_element_group_numeric.ts`), and a
 `PERRY_GC_ZEAL=1 PERRY_GC_PROTECT_FROMSPACE=1` run relocated 4014 objects
-under the bare loads with a clean verdict. Pass 4 moved wholesale into the
+under the bare loads with a clean verdict. On the pinned quiet mini
+(interleaved, best-of-15, two runs) the issue's read loop goes 101/102 ms ->
+91/90 ms while `batch.ts`, `suite/04_array_read` and `suite/09_method_calls`
+are unchanged; the A/B's subject is verified live (base arm emits 4
+`js_number_coerce` sites and 12 checked-load diamonds on the benchmarked
+source, branch arm zero) -- note that benchmarking this needs the array and
+its read loop in ONE function, since an array crossing a function boundary is
+the #7766 shape this change does not address. Pass 4 moved wholesale into the
 `ptr_shape_numeric.rs` child module for the 2000-line gate. A neighbouring
 PRE-EXISTING divergence found during validation (`o.x + 1` coercing where
 Node concatenates, plus an evaporating any-laundered add) is filed as #7773.

changelog.d/7788-shared-this-flow-gate.md

@@ -0,0 +1,19 @@
+**repsel: one Pass-3 safety gate and one write walker for both numeric proofs (#7788, follow-up to #7770/#7774).**
+
+No behavior change. `prove_group_numeric_fields` had grown an independent copy
+of the `'cand` loop's obligation set (`ctor_chain_safe`, `prototype_is_stable`,
+field/method ambiguity, per-method `method_safe`); that verdict licenses a bare
+unchecked `load double`, so two copies drifting is a miscompile rather than a
+missed optimization. Extracted as `chain_this_flow_verdict`, the single
+implementation both callers share. Likewise
+`collect_numeric_by_construction_locals`'s hand-rolled write collector is gone:
+`not_bigint_locals::collect_writes` now records a no-init `Let` as `None` (fine
+for the non-BigInt fixpoint, fatal for the numeric one) and serves both.
+
+Adds the coverage gap the review found: the `super(...)` parameter-resolution
+path became reachable through the group MEET in #7770 and had no group-scope
+test (every fixture was `extends: None`), so a wrong index there would have
+granted an unsound claim silently. `super_chain_params_resolve_under_the_group_meet`
+covers both directions. Also skips the group proof's this-flow walk when no
+chain field is raw-f64-declared, and computes `group_members()` once per region
+instead of twice.

crates/perry-codegen/src/collectors/not_bigint_locals.rs

@@ -51,7 +51,7 @@ pub fn collect_not_bigint_locals(
     // into closure bodies so a `LocalSet` to an ENCLOSING local inside a
     // closure is captured (LocalIds are unique per function, so the write is
     // recorded against the enclosing id).
-    let mut writes: HashMap<u32, Vec<&Expr>> = HashMap::new();
+    let mut writes: HashMap<u32, Vec<Option<&Expr>>> = HashMap::new();
     let mut candidates: HashSet<u32> = HashSet::new();
     collect_writes(stmts, &mut writes, &mut candidates);
 
@@ -70,8 +70,11 @@ pub fn collect_not_bigint_locals(
                 // A declared-but-never-assigned `let x;` is `undefined` — a
                 // non-BigInt — so no writes means the local stays.
                 .map(|ws| {
-                    ws.iter()
-                        .all(|rhs| expr_not_bigint(rhs, &types, &not_bigint, &numeric_locals))
+                    ws.iter().all(|rhs| match rhs {
+                        // A `let x;` binding is `undefined` — a non-BigInt.
+                        None => true,
+                        Some(rhs) => expr_not_bigint(rhs, &types, &not_bigint, &numeric_locals),
+                    })
                 })
                 .unwrap_or(true);
             if !all_ok {
@@ -217,18 +220,25 @@ fn index_receiver_is_numeric(object: &Expr, types: &HashMap<u32, HirType>) -> bo
 }
 
 /// Record every write (Let init + `LocalSet` rhs) per local, gather the set of
-/// analyzed candidate ids, and descend into closure bodies.
-fn collect_writes<'a>(
+/// `Let`-bound candidate ids, and descend into closure bodies. A `Let` with no
+/// initializer records `None` (the binding is `undefined` until assigned).
+///
+/// Shared with `ptr_shape/ptr_shape_numeric.rs`'s numeric-by-construction
+/// fixpoint (#7770) — the two analyses differ only in how they judge a write
+/// (`undefined` is a fine non-BigInt and a fatal non-number), so ONE walker
+/// keeps them from drifting by a `Stmt` variant, the bug class
+/// `ptr_shape_elements.rs`'s doc warns about.
+pub(super) fn collect_writes<'a>(
     stmts: &'a [Stmt],
-    writes: &mut HashMap<u32, Vec<&'a Expr>>,
+    writes: &mut HashMap<u32, Vec<Option<&'a Expr>>>,
     candidates: &mut HashSet<u32>,
 ) {
     for s in stmts {
         match s {
             Stmt::Let { id, init, .. } => {
                 candidates.insert(*id);
+                writes.entry(*id).or_default().push(init.as_ref());
                 if let Some(e) = init {
-                    writes.entry(*id).or_default().push(e);
                     collect_writes_expr(e, writes, candidates);
                 }
             }
@@ -314,11 +324,11 @@ fn collect_writes<'a>(
 
 fn collect_writes_expr<'a>(
     e: &'a Expr,
-    writes: &mut HashMap<u32, Vec<&'a Expr>>,
+    writes: &mut HashMap<u32, Vec<Option<&'a Expr>>>,
     candidates: &mut HashSet<u32>,
 ) {
     if let Expr::LocalSet(id, rhs) = e {
-        writes.entry(*id).or_default().push(rhs.as_ref());
+        writes.entry(*id).or_default().push(Some(rhs.as_ref()));
     }
     // Closure bodies are statements, not expression children, so descend
     // explicitly. A write to an enclosing local inside the closure body targets

crates/perry-codegen/src/collectors/ptr_shape.rs

@@ -451,10 +451,12 @@ pub(crate) fn collect_shape_proven_ptr_locals(
     // claim is honest even though member verdicts are not in yet: group
     // integrity below drops EVERY member's fact when any member fails, and a
     // dropped fact takes its claim with it.
+    let groups = element_facts.group_members();
     let group_numeric = prove_group_numeric_fields(
         classes,
         module_dispatch,
         element_facts,
+        &groups,
         &roots,
         &field_stores,
         &method_calls,
@@ -499,46 +501,28 @@ pub(crate) fn collect_shape_proven_ptr_locals(
         let chain = chain_classes(classes, class_name);
         let fields = chain_field_names(&chain);
         let methods = chain_method_map(&chain);
+        let called = method_calls.get(id);
         // Constructor chain + field initializers must not leak `this`. All
         // methods called on the local must be `this`-flow safe and the module
-        // must prove the method table stable.
-        let mut analysis = ThisFlowAnalysis {
-            chain: &chain,
-            fields: &fields,
-            methods: &methods,
-            visited: HashSet::new(),
-            store_records: Vec::new(),
-            super_call_args: HashMap::new(),
-            internally_invoked: HashSet::new(),
-            allow_this_in_store_values: false,
-        };
-        if !analysis.ctor_chain_safe() {
-            deny(id, class_name, report::THIS_ESCAPE);
-            continue;
-        }
-        let called = method_calls.get(id);
-        if let Some(called) = called {
-            if !module_dispatch.prototype_is_stable(classes, class_name) {
-                deny(id, class_name, report::UNSTABLE_PROTOTYPE);
-                continue;
-            }
-            for m in called.keys() {
-                if fields.contains(m.as_str()) {
-                    // A name that is both a field and a method is ambiguous
-                    // under own-property shadowing — bail.
-                    deny(id, class_name, report::FIELD_METHOD_AMBIGUITY);
-                    continue 'cand;
-                }
-                let Some((owner, func)) = methods.get(m.as_str()) else {
-                    deny(id, class_name, report::ESC_UNRESOLVED_METHOD);
-                    continue 'cand;
-                };
-                if !analysis.method_safe(owner, func) {
-                    deny(id, class_name, report::METHOD_THIS_ESCAPE);
-                    continue 'cand;
-                }
+        // must prove the method table stable. ONE implementation, shared with
+        // the group-scope numeric proof (#7770) — this gate licenses a bare
+        // unchecked `load double`, so two drifting copies would be a
+        // miscompile waiting to happen, not a missed optimization.
+        let mut analysis = match chain_this_flow_verdict(
+            classes,
+            module_dispatch,
+            class_name,
+            &chain,
+            &fields,
+            &methods,
+            called,
+        ) {
+            Ok(analysis) => analysis,
+            Err(why) => {
+                deny(id, class_name, why);
+                continue 'cand;
             }
-        }
+        };
         let store_records = std::mem::take(&mut analysis.store_records);
         let super_call_args = std::mem::take(&mut analysis.super_call_args);
         let internally_invoked = std::mem::take(&mut analysis.internally_invoked);
@@ -612,7 +596,7 @@ pub(crate) fn collect_shape_proven_ptr_locals(
     // group is therefore all-or-nothing. Dropping is the conservative
     // direction and needs no fixpoint: removing members never admits one.
     if !element_facts.is_empty() {
-        for (_, members) in element_facts.group_members() {
+        for members in groups.values() {
             if members.iter().any(|m| !out.contains_key(m)) {
                 // The insert loop above gives every ALIAS of a promoted root
                 // the same fact, because an alias holds the same object. The
@@ -1338,6 +1322,61 @@ struct ThisStoreRecord<'a> {
     context: Option<(String, String, Vec<u32>)>,
 }
 
+/// Pass-3 safety verdict for one class chain plus the methods invoked on the
+/// value: `this`-flow containment of the constructor chain, prototype
+/// stability when any method is called, field/method name-ambiguity, and
+/// per-method `this`-flow safety. Returns the analysis (holding the store
+/// records, `super(...)` argument lists, and internally-invoked set the
+/// numeric proof consumes) or the FIRST failed obligation.
+///
+/// This is the single implementation behind both the per-candidate `'cand`
+/// loop and the group-scope numeric proof (#7770,
+/// `ptr_shape_numeric.rs::prove_group_numeric_fields`). The verdict licenses
+/// a bare unchecked `load double`; keeping the two callers on one function
+/// is what makes "tighten an obligation" a one-place change.
+fn chain_this_flow_verdict<'a, 'b>(
+    classes: &HashMap<String, &'a Class>,
+    module_dispatch: &ModuleDispatchFacts,
+    class_name: &str,
+    chain: &'b [&'a Class],
+    fields: &'b HashSet<String>,
+    methods: &'b HashMap<String, (String, &'a perry_hir::Function)>,
+    called: Option<&HashMap<String, Vec<&'a [Expr]>>>,
+) -> Result<ThisFlowAnalysis<'a, 'b>, ShapeDenial> {
+    let mut analysis = ThisFlowAnalysis {
+        chain,
+        fields,
+        methods,
+        visited: HashSet::new(),
+        store_records: Vec::new(),
+        super_call_args: HashMap::new(),
+        internally_invoked: HashSet::new(),
+        allow_this_in_store_values: false,
+    };
+    if !analysis.ctor_chain_safe() {
+        return Err(report::THIS_ESCAPE);
+    }
+    if let Some(called) = called {
+        if !called.is_empty() && !module_dispatch.prototype_is_stable(classes, class_name) {
+            return Err(report::UNSTABLE_PROTOTYPE);
+        }
+        for m in called.keys() {
+            if fields.contains(m.as_str()) {
+                // A name that is both a field and a method is ambiguous
+                // under own-property shadowing — bail.
+                return Err(report::FIELD_METHOD_AMBIGUITY);
+            }
+            let Some((owner, func)) = methods.get(m.as_str()) else {
+                return Err(report::ESC_UNRESOLVED_METHOD);
+            };
+            if !analysis.method_safe(owner, func) {
+                return Err(report::METHOD_THIS_ESCAPE);
+            }
+        }
+    }
+    Ok(analysis)
+}
+
 pub(super) struct ThisFlowAnalysis<'a, 'b> {
     chain: &'b [&'a Class],
     fields: &'b HashSet<String>,

crates/perry-codegen/src/collectors/ptr_shape_group_numeric_tests.rs

@@ -18,6 +18,9 @@ use perry_hir::{BinaryOp, ClassField, CompareOp, Function, Module, Param, Update
 /// the tests use, mirroring the module-wide id allocator.
 const CTOR_PX: u32 = 100;
 const CTOR_PY: u32 = 101;
+/// `Base`'s ctor param and `D`'s super-feeding param (the subclass tests).
+const CTOR_PZ: u32 = 102;
+const CTOR_PZ2: u32 = 103;
 /// Method parameter id for `setX(v)`.
 const METH_PV: u32 = 110;
 
@@ -501,6 +504,117 @@ fn method_site_string_arg_drops_the_field() {
     );
 }
 
+/// `class Base { z; constructor(z) { this.z = z } }`.
+fn class_base() -> Class {
+    let mut b = class_p();
+    b.name = "Base".to_string();
+    b.fields = vec![field("z")];
+    b.constructor = Some(Function {
+        id: 902,
+        name: "constructor".to_string(),
+        type_params: Vec::new(),
+        params: vec![num_param(CTOR_PZ, "z")],
+        return_type: Type::Void,
+        body: vec![this_store("z", Expr::LocalGet(CTOR_PZ))],
+        is_async: false,
+        is_generator: false,
+        is_strict: true,
+        is_exported: false,
+        captures: Vec::new(),
+        decorators: Vec::new(),
+        was_plain_async: false,
+        was_unrolled: false,
+    });
+    b
+}
+
+/// `class D extends Base { x; constructor(x, z) { super(z); this.x = x } }`.
+fn class_d_extends_base() -> Class {
+    let mut d = class_p();
+    d.id = 1;
+    d.name = "D".to_string();
+    d.extends_name = Some("Base".to_string());
+    d.fields = vec![field("x")];
+    d.constructor = Some(Function {
+        id: 903,
+        name: "constructor".to_string(),
+        type_params: Vec::new(),
+        params: vec![num_param(CTOR_PX, "x"), num_param(CTOR_PZ2, "z")],
+        return_type: Type::Void,
+        body: vec![
+            Stmt::Expr(Expr::SuperCall(vec![Expr::LocalGet(CTOR_PZ2)])),
+            this_store("x", Expr::LocalGet(CTOR_PX)),
+        ],
+        is_async: false,
+        is_generator: false,
+        is_strict: true,
+        is_exported: false,
+        captures: Vec::new(),
+        decorators: Vec::new(),
+        was_plain_async: false,
+        was_unrolled: false,
+    });
+    d
+}
+
+/// The super()-argument resolution path under the group MEET — the one place
+/// a parent-constructor parameter environment is derived from MULTIPLE
+/// provenance `new`s. A wrong index or an unresolved caller env here would
+/// grant an unsound claim on `z` (a bare raw load), so both directions get a
+/// red test: all-numeric sites prove BOTH the derived and the inherited
+/// field; one string at the super-feeding position drops exactly `z`,
+/// group-wide, while `x` survives.
+#[test]
+fn super_chain_params_resolve_under_the_group_meet() {
+    let cs = [class_base(), class_d_extends_base()];
+    let classes = classes_of(&cs);
+    let read_loop = |idx: u32, r: u32| {
+        counted_loop(
+            idx,
+            arr_len(1),
+            vec![let_elem(r, 1, idx, "D"), read_field(r, "z")],
+        )
+    };
+    let stmts_ok = vec![
+        let_arr(1, "D"),
+        counted_loop(
+            2,
+            Expr::Number(4.0),
+            vec![push(
+                1,
+                new_of("D", vec![Expr::LocalGet(2), counter_plus_one(2)]),
+            )],
+        ),
+        read_loop(5, 6),
+    ];
+    let promoted_ok = promote(&stmts_ok, &classes);
+    assert_eq!(
+        promoted_ok.get(&6).expect("reader promotes").numeric_fields,
+        names(&["x", "z"]),
+        "super(z) must resolve Base's parameter through the caller env"
+    );
+
+    let stmts_poison = vec![
+        let_arr(1, "D"),
+        push(1, new_of("D", vec![Expr::Number(1.0), Expr::Number(2.0)])),
+        push(
+            1,
+            new_of("D", vec![Expr::Number(3.0), Expr::String("s".to_string())]),
+        ),
+        read_loop(5, 6),
+    ];
+    let promoted_poison = promote(&stmts_poison, &classes);
+    assert_eq!(
+        promoted_poison
+            .get(&6)
+            .expect("reader promotes")
+            .numeric_fields,
+        names(&["x"]),
+        "one string at the super-feeding position must drop `z` for the whole \
+         group and leave `x` standing"
+    );
+}
+
 /// The group claim dies with the group: an undeclared-property store on one
 /// member removes every member's FACT, claim included.
 #[test]