perf(repsel): admit method return-shape producers (#7170 R2)

changelog.d/8029-method-return-shapes.md

@@ -0,0 +1,11 @@
+### Representation selection: preserve fresh shapes returned by methods (#7170 R2)
+
+Native compilation now propagates proven fresh return shapes through instance
+method calls when the receiver has an exact contained shape and its prototype
+dispatch is stable. Results can use guard-free fixed-offset field access just
+like values returned by direct function calls.
+
+The proof stays fail-closed for decorated classes, mutable prototype dispatch,
+unproven or escaping receivers, and modules containing shape barriers. The
+optimization report also classifies allocations served by method return-shape
+facts separately from the remaining rule-1 provenance wall.

crates/perry-codegen/src/codegen/method.rs

@@ -371,9 +371,12 @@ pub(super) fn compile_method(
     let flat_const_ids: std::collections::HashSet<u32> =
         cross_module.flat_const_arrays.keys().copied().collect();
     // `--opt-report` (#6952) attribution scope; no-op when off.
-    let _opt_report_scope = crate::opt_report::enter_region(
+    let _opt_report_scope = crate::opt_report::enter_method_region(
         &format!("{}.{}", class.name, method.name),
-        crate::opt_report::RegionKind::Method,
+        cross_module
+            .module_dispatch
+            .return_shape_method_class(&class.name, &method.name, method.id)
+            .is_some(),
     );
     let native_facts = crate::collectors::collect_native_region_fact_graph(
         &method.body,

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

@@ -339,13 +339,15 @@ pub(crate) fn collect_shape_proven_ptr_locals(
     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(
+    let return_seeds = super::ptr_shape_returns::find_return_shape_candidates(
         stmts,
         boxed_vars,
         module_globals,
+        classes,
         module_dispatch,
         &mut candidates,
     );
+    let return_seeded = &return_seeds.seeded;
     // #7034 §3: `const r = A[i]` at an in-bounds site on an element-shape-
     // proven local array is provenance of `new C(...)` strength (module doc,
     // rule 2's array-element exception). The seeds are already filtered for
@@ -418,7 +420,7 @@ pub(crate) fn collect_shape_proven_ptr_locals(
         const_local_inits: HashMap::new(),
         disq_reasons: HashMap::new(),
         escape_ctx: report::ESC_BARE_REFERENCE,
-        return_seeded: &return_seeded,
+        return_seeded,
         element_seeded: &element_seeded,
         element_facts,
         in_closure: false,
@@ -628,6 +630,42 @@ pub(crate) fn collect_shape_proven_ptr_locals(
             }
         }
     }
+    // #7170 R2: a dynamic method call names one implementation only while
+    // its receiver keeps the exact shape/containment fact that licensed the
+    // resolution. Candidate discovery runs before the full use, constructor,
+    // and method-body proofs, so enforce that dependency after every ordinary
+    // rejection (including element-group all-or-nothing). Iterate to a
+    // fixpoint for chains such as `a.makeB().makeC()` represented by bound
+    // intermediate locals.
+    loop {
+        let doomed_roots: Vec<u32> = return_seeds
+            .method_receivers
+            .iter()
+            .filter_map(|(result, receiver)| {
+                (out.contains_key(result) && !out.contains_key(receiver)).then_some(*result)
+            })
+            .collect();
+        if doomed_roots.is_empty() {
+            break;
+        }
+        for root in doomed_roots {
+            let doomed: Vec<u32> = roots
+                .iter()
+                .filter_map(|(member, member_root)| (*member_root == root).then_some(*member))
+                .collect();
+            for member in doomed {
+                if out.remove(&member).is_some() {
+                    report::deny_local(
+                        member,
+                        &names,
+                        &depths,
+                        candidates.get(&member).map(String::as_str),
+                        report::RETURN_METHOD_RECEIVER_UNPROVEN,
+                    );
+                }
+            }
+        }
+    }
     out
 }
 
@@ -737,6 +775,21 @@ pub(super) fn chain_field_names(chain: &[&Class]) -> HashSet<String> {
     out
 }
 
+/// Whether one class in the resolved chain declares the same instance method
+/// name more than once. Overrides in different classes are intentional and
+/// remain resolvable by the prototype chain; duplicate declarations within a
+/// single class are different because JavaScript selects the last declaration
+/// while several Perry symbol/collector paths still select the first.
+pub(super) fn chain_has_duplicate_method_names(chain: &[&Class]) -> bool {
+    chain.iter().any(|class| {
+        let mut names = HashSet::new();
+        class
+            .methods
+            .iter()
+            .any(|method| !names.insert(method.name.as_str()))
+    })
+}
+
 /// name -> (owning class name, method function), first (most-derived) wins —
 /// matching JS prototype-chain resolution for an exact-class instance.
 pub(super) fn chain_method_map<'a>(
@@ -895,13 +948,12 @@ impl<'a> UseWalk<'a> {
                     // is the CALL. It records no `new_args` — the constructor
                     // ran in the callee, so the numeric-field proof stands
                     // down for these candidates entirely (see the `'cand`
-                    // loop). The argument expressions are ordinary values;
-                    // walk them so OTHER candidates passed there still escape.
+                    // loop). Walk the complete call so OTHER candidates passed
+                    // as arguments still escape and a tracked method receiver
+                    // records the call for pass 3's `this`-flow audit.
                     if self.return_seeded.contains(id) {
-                        if let Some(Expr::Call { args, .. }) = init.as_ref() {
-                            for a in args {
-                                self.with_ctx(report::ESC_CALL_ARGUMENT, |w| w.walk_expr(a));
-                            }
+                        if let Some(call @ Expr::Call { .. }) = init.as_ref() {
+                            self.walk_expr(call);
                             return;
                         }
                     }

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

@@ -520,6 +520,32 @@ fn a_return_in_a_return_shape_producer_is_reported_as_served() {
     assert_eq!(rows[0].tier, Some(crate::opt_report::Tier::Served));
 }
 
+/// #7170 R2 extends the same report correction to instance-method producers.
+/// A served allocation must not reappear in the rule-1 wall merely because
+/// codegen labels its region `method` rather than `function`.
+#[test]
+fn a_return_in_a_method_shape_producer_is_reported_as_served() {
+    let c = class_with_fields("C", &["x"]);
+    let mut classes = HashMap::new();
+    classes.insert("C".to_string(), &c);
+    let stmts = vec![Stmt::Return(Some(new_c()))];
+
+    let session = Session::start();
+    let guard = crate::opt_report::enter_method_region("Factory.make", true);
+    let _ = run(&stmts, &classes);
+    drop(guard);
+    let entries = session.entries();
+
+    let rows = alloc_rows(&entries);
+    assert_eq!(rows.len(), 1);
+    assert_eq!(
+        rows[0].rule.as_deref(),
+        Some("rule 1 (provenance) — already served by return-shape")
+    );
+    assert_eq!(rows[0].tier, Some(crate::opt_report::Tier::Served));
+    assert_eq!(rows[0].region, crate::opt_report::RegionKind::Method);
+}
+
 /// The same site in a function WITHOUT a return-shape fact stays an ordinary
 /// rule-1 denial. This is the anti-vacuity half: a classifier that answers
 /// "served" unconditionally passes the test above and fails this one.

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

@@ -126,6 +126,15 @@ pub(super) const ALIAS_NOT_SINGLE_LET: ShapeDenial = ShapeDenial {
     issue: None,
 };
 
+pub(super) const RETURN_METHOD_RECEIVER_UNPROVEN: ShapeDenial = ShapeDenial {
+    rule: RULE1,
+    reason: "initialized by a fresh-returning method call, but the receiver's \
+             exact shape or contained dispatch proof did not survive the full \
+             region analysis.",
+    tier: Tier::Fixable,
+    issue: Some("#7170 R2"),
+};
+
 /// #7112: `find_new_candidates` excludes cell-backed locals before the
 /// containment walk, so without an entry they look indistinguishable from
 /// values the analysis never considered.

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

@@ -66,6 +66,13 @@
 //! `return flag && new C()`, or an expression with a non-fresh/disagreeing
 //! result — yields no fact.
 //!
+//! #7170 R2 also applies the same producer proof to declared instance methods.
+//! A method result is a caller-side seed only when the receiver is itself an
+//! exact shape-proven local, the method name resolves unambiguously on that
+//! class chain, and the module-wide prototype-stability proof holds. The
+//! result fact depends on the receiver fact: if the receiver later fails the
+//! full containment/`this`-flow proof, the result is removed too.
+//!
 //! ## Why the producer must not fall off its end
 //!
 //! `function f(x) { if (x) return new C(); }` returns `undefined` on the other
@@ -102,7 +109,10 @@ use std::collections::{HashMap, HashSet};
 use perry_hir::types::Type;
 use perry_hir::{Class, Expr, LogicalOp, Module, Stmt};
 
-use super::ptr_shape::{chain_admissible, ptr_shape_locals_enabled};
+use super::ptr_shape::{
+    chain_admissible, chain_classes, chain_field_names, chain_has_duplicate_method_names,
+    chain_method_map, ptr_shape_locals_enabled,
+};
 use super::ptr_shape_report as report;
 use super::ModuleDispatchFacts;
 
@@ -248,6 +258,90 @@ pub(crate) fn collect_return_shape_functions(
     out
 }
 
+/// Declared instance methods whose body returns one fresh exact class on every
+/// path, keyed by the method implementation that owns the body.
+///
+/// The method name is resolved separately from the receiver's exact class at
+/// the call site. Keying the producer fact by `(owner class, method name,
+/// FuncId)` prevents equal numeric ids in unrelated declarations (including
+/// accessors) from aliasing. A key claimed by two bodies is dropped entirely:
+/// transformed HIR can duplicate ids, and attributing either body by walk
+/// order would make a guard-free load depend on an arbitrary choice.
+pub(crate) fn collect_return_shape_methods(
+    facts: &ModuleDispatchFacts,
+    hir: &Module,
+) -> HashMap<(String, String, u32), String> {
+    let mut out = HashMap::new();
+    if !ptr_shape_locals_enabled() || facts.has_shape_barrier_sites() {
+        return out;
+    }
+    let classes: HashMap<String, &Class> = hir
+        .classes
+        .iter()
+        .map(|class| (class.name.clone(), class))
+        .collect();
+    let mut claims: HashMap<(String, String, u32), usize> = HashMap::new();
+    let mut proven = Vec::new();
+
+    for class in &hir.classes {
+        if class_has_legacy_decorators(class) {
+            continue;
+        }
+        for method in &class.methods {
+            let key = (class.name.clone(), method.name.clone(), method.id);
+            *claims.entry(key.clone()).or_insert(0) += 1;
+            let view = ProducerBody {
+                boxed_or_resumable: method.is_async
+                    || method.is_generator
+                    || method.was_plain_async,
+                return_type: &method.return_type,
+                body: &method.body,
+            };
+            if let Some(class_name) = producer_return_class(&view, &classes, facts) {
+                proven.push((key, class_name));
+            }
+        }
+    }
+    for (key, class_name) in proven {
+        if claims.get(&key) == Some(&1) {
+            out.insert(key, class_name);
+        }
+    }
+    out
+}
+
+/// Legacy decorators execute arbitrary user code at class-definition time.
+/// A decorator imported from another module can rewrite a prototype without
+/// leaving a `.prototype` expression in this module for rule 4 to observe, so
+/// decorated classes cannot participate in static method-return resolution.
+fn class_has_legacy_decorators(class: &Class) -> bool {
+    let function_has_decorators = |function: &perry_hir::Function| {
+        !function.decorators.is_empty()
+            || function
+                .params
+                .iter()
+                .any(|param| !param.decorators.is_empty())
+    };
+    !class.decorators.is_empty()
+        || class
+            .fields
+            .iter()
+            .chain(class.static_fields.iter())
+            .any(|field| !field.decorators.is_empty())
+        || class
+            .constructor
+            .as_ref()
+            .is_some_and(|constructor| function_has_decorators(constructor))
+        || class
+            .methods
+            .iter()
+            .chain(class.static_methods.iter())
+            .chain(class.getters.iter().map(|(_, function)| function))
+            .chain(class.setters.iter().map(|(_, function)| function))
+            .chain(class.computed_members.iter().map(|member| &member.function))
+            .any(function_has_decorators)
+}
+
 /// Export name -> anonymous-record class for source functions whose final HIR
 /// carries a return-shape fact.
 ///
@@ -723,14 +817,24 @@ fn walk_stmts<'a>(stmts: &'a [Stmt], f: &mut impl FnMut(&'a Stmt)) {
 /// Mirrors `find_new_candidates`' shape exactly — same exclusions (boxed,
 /// module-global), same nesting, and no descent into closure bodies (each is
 /// its own region).
+pub(super) struct ReturnShapeSeeds {
+    pub(super) seeded: HashSet<u32>,
+    /// Result local -> exact receiver local whose final shape proof licenses
+    /// the method dispatch. The parent collector enforces these dependencies
+    /// after all ordinary candidate and element-group rejections have run.
+    pub(super) method_receivers: HashMap<u32, u32>,
+}
+
 pub(crate) fn find_return_shape_candidates(
     stmts: &[Stmt],
     boxed_vars: &HashSet<u32>,
     module_globals: &HashMap<u32, String>,
+    classes: &HashMap<String, &Class>,
     module_dispatch: &ModuleDispatchFacts,
     candidates: &mut HashMap<u32, String>,
-) -> HashSet<u32> {
+) -> ReturnShapeSeeds {
     let mut seeded = HashSet::new();
+    let mut method_receivers = HashMap::new();
     walk_stmts(stmts, &mut |s| {
         let Stmt::Let {
             id,
@@ -743,17 +847,62 @@ pub(crate) fn find_return_shape_candidates(
         if boxed_vars.contains(id) || module_globals.contains_key(id) {
             return;
         }
+        let mut method_receiver = None;
         let class_name = match callee.as_ref() {
-            Expr::ExternFuncRef { name, .. } => module_dispatch.imported_return_shape_class(name),
+            Expr::ExternFuncRef { name, .. } => module_dispatch
+                .imported_return_shape_class(name)
+                .map(str::to_string),
+            Expr::PropertyGet {
+                object, property, ..
+            } => {
+                let Expr::LocalGet(receiver_id) = object.as_ref() else {
+                    return;
+                };
+                let Some(receiver_class) = candidates.get(receiver_id).cloned() else {
+                    return;
+                };
+                if !module_dispatch.prototype_is_stable(classes, &receiver_class) {
+                    return;
+                }
+                let chain = chain_classes(classes, &receiver_class);
+                if chain.iter().any(|class| class_has_legacy_decorators(class)) {
+                    return;
+                }
+                if chain_has_duplicate_method_names(&chain) {
+                    return;
+                }
+                if chain_field_names(&chain).contains(property) {
+                    return;
+                }
+                let methods = chain_method_map(&chain);
+                let Some((owner_class, method)) = methods.get(property) else {
+                    return;
+                };
+                let Some(class_name) = module_dispatch
+                    .return_shape_method_class(owner_class, property, method.id)
+                    .map(str::to_string)
+                else {
+                    return;
+                };
+                method_receiver = Some(*receiver_id);
+                Some(class_name)
+            }
             _ => callee_names_one_function(callee, module_dispatch)
-                .and_then(|func_id| module_dispatch.return_shape_class(func_id)),
+                .and_then(|func_id| module_dispatch.return_shape_class(func_id))
+                .map(str::to_string),
         };
         if let Some(class_name) = class_name {
-            candidates.insert(*id, class_name.to_string());
+            candidates.insert(*id, class_name);
             seeded.insert(*id);
+            if let Some(receiver_id) = method_receiver {
+                method_receivers.insert(*id, receiver_id);
+            }
         }
     });
-    seeded
+    ReturnShapeSeeds {
+        seeded,
+        method_receivers,
+    }
 }
 
 /// The one statically-known function a callee expression names, or `None`.