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

changelog.d/8015-logical-return-shapes.md

@@ -0,0 +1,13 @@
+### Representation selection: prove fresh logical return shapes (#7170 R2)
+
+Functions and CJS-wrapped closure producers that return `&&` / `||`
+expressions can now issue a `Ptr<Shape>` return fact when every value that can
+escape the complete short-circuit expression is a fresh allocation of the same
+admissible class. This includes nested fallback forms such as
+`(flag && new C()) || new C()`; caller bindings reuse the existing guard-free
+fixed-offset field-access path with no new pointer position or ABI change.
+
+Primitive or unknown escape paths, disagreeing reachable classes, and nullish
+coalescing remain fail-closed. `--opt-report` marks only allocations that can
+actually become the logical result as served, leaving consumed short-circuit
+operands out of that population.

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

@@ -745,9 +745,10 @@ fn a_region_lowered_twice_still_collapses_and_says_how_much() {
 // producer, `Tier::Served` with it.
 //
 // The return-shape fact originally covered only the function's direct RETURN
-// VALUE. #7170 R2 additionally consumes fresh, agreeing conditional result
-// arms. It still says nothing about a conditional's condition, a `&&`, an
-// `await` or a member base that happens to sit inside the returned expression.
+// VALUE. #7170 R2 additionally consumes fresh, agreeing conditional and
+// logical result allocations. It still says nothing about a conditional's
+// condition, a consumed short-circuit operand, an `await` or a member base
+// that happens to sit inside the returned expression.
 //
 // R0 separated the syntactic bucket from the servedness bit precisely so R2
 // could make that distinction. These tests force the producer flag on so the
@@ -839,9 +840,12 @@ fn a_conditional_condition_is_not_a_return_shape_source() {
     );
 }
 
-/// `return flag && new C()` — a binary operand.
+/// `return true && new C()` — the right operand is the only value this
+/// expression can return, so the logical-return proof consumes it. It keeps
+/// the honest operand-position label rather than masquerading as a bare
+/// return.
 #[test]
-fn a_logical_operand_under_a_return_is_not_a_return_position() {
+fn a_logical_result_operand_is_reported_as_served_by_return_shape() {
     let c = c_classes();
     let mut classes = HashMap::new();
     classes.insert("C".to_string(), &c);
@@ -855,7 +859,48 @@ fn a_logical_operand_under_a_return_is_not_a_return_position() {
     let rows = alloc_rows(&entries);
     assert_eq!(rows.len(), 1);
     assert_ne!(rows[0].alloc_context.as_deref(), Some("return"));
-    assert_ne!(rows[0].tier, Some(crate::opt_report::Tier::Served));
+    assert_eq!(rows[0].tier, Some(crate::opt_report::Tier::Served));
+}
+
+/// A fresh left operand is always truthy. In `new C() || new D()` it is the
+/// returned value and the right allocation is consumed by short-circuiting.
+/// Marking both served would put a structurally unreachable site back into the
+/// population R0 corrected.
+#[test]
+fn a_consumed_logical_operand_is_not_a_return_shape_source() {
+    let c = c_classes();
+    let d = class_with_fields("D", &["x"]);
+    let mut classes = HashMap::new();
+    classes.insert("C".to_string(), &c);
+    classes.insert("D".to_string(), &d);
+    let new_d = Expr::New {
+        class_name: "D".to_string(),
+        args: Vec::new(),
+        type_args: Vec::new(),
+        byte_offset: 0,
+        cap_args_appended: 0,
+    };
+    let stmts = vec![Stmt::Return(Some(Expr::Logical {
+        op: perry_hir::LogicalOp::Or,
+        left: Box::new(new_c()),
+        right: Box::new(new_d),
+    }))];
+
+    let entries = run_as_producer(&stmts, &classes);
+    let rows = alloc_rows(&entries);
+    assert_eq!(rows.len(), 2);
+    assert_eq!(
+        rows.iter()
+            .filter(|e| e.tier == Some(crate::opt_report::Tier::Served))
+            .count(),
+        1,
+        "only the allocation that can become the logical result is served"
+    );
+    let served = rows
+        .iter()
+        .find(|e| e.tier == Some(crate::opt_report::Tier::Served))
+        .expect("one served allocation");
+    assert!(served.name.contains("C"));
 }
 
 /// `return await new C()` — the awaited operand is not the returned value

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

@@ -549,10 +549,10 @@ fn walk_lets(stmts: &[Stmt], depth: u32, f: &mut impl FnMut(u32, &str, u32)) {
 /// The allocation IS the function's return value: `return new C(...)`.
 const RETURN: &str = "return";
 /// The allocation sits *inside* a returned expression but is not the returned
-/// value — a conditional arm, a `&&` operand, an awaited operand, a member
-/// access base. #7170 R2 consumes conditional *result* arms as return-shape
-/// sources, but they remain in this syntactic bucket rather than being
-/// mislabeled as direct returns.
+/// value — a conditional arm, a logical operand, an awaited operand, a member
+/// access base. #7170 R2 consumes conditional and logical *result* allocations
+/// as return-shape sources, but they remain in this syntactic bucket rather
+/// than being mislabeled as direct returns.
 ///
 /// Split out in review of #7176. `RETURN` was set once, at
 /// `Stmt::Return(Some(e))`, and `scan_expr` propagates its context unchanged
@@ -601,9 +601,10 @@ pub(super) struct NewSite {
     /// see [`crate::opt_report::Entry::alloc_ordinal`].
     pub ordinal: u32,
     /// This allocation is one of the fresh values a return-shape fact proves:
-    /// either the direct expression of a `Stmt::Return`, or a result arm of a
-    /// returned conditional (#7170 R2). An allocation in the condition, a
-    /// constructor argument, or another nested operand is not a source.
+    /// either the direct expression of a `Stmt::Return`, or a possible result
+    /// allocation of a returned conditional / logical expression (#7170 R2).
+    /// An allocation in a condition, a constructor argument, or a consumed
+    /// short-circuit operand is not a source.
     ///
     /// Set only by [`scan_return`] and its result-arm walker. Deriving
     /// servedness from the context label instead would be wrong: conditional
@@ -747,49 +748,42 @@ fn scan_stmts(
 /// Scan the expression of a `Stmt::Return`.
 ///
 /// The direct expression of a `return` is the function's return value. #7170
-/// R2 additionally proves the result arms of a conditional when every leaf is
-/// a fresh allocation of one class. The condition and every non-result nested
-/// expression remain ordinary operands.
+/// R2 additionally proves possible result allocations of conditional and
+/// short-circuiting logical expressions. Conditions and every non-result
+/// nested expression remain ordinary operands.
 ///
 /// This is the only entry into the served-source classification.
 fn scan_return(e: &Expr, depth: u32, out: &mut Vec<NewSite>) {
-    match e {
-        Expr::New {
-            class_name,
-            args,
-            byte_offset,
-            ..
-        } => {
-            push_new_site(out, class_name, RETURN, depth, *byte_offset, true);
-            let arg_ctx = arg_context(class_name);
-            for a in args {
-                scan_expr(a, depth, arg_ctx, out);
-            }
-        }
-        Expr::Conditional {
-            condition,
-            then_expr,
-            else_expr,
-        } => {
-            scan_expr(condition, depth, RETURNED_OPERAND, out);
-            scan_conditional_result(then_expr, depth, out);
-            scan_conditional_result(else_expr, depth, out);
-        }
-        _ => scan_expr(e, depth, RETURNED_OPERAND, out),
-    }
+    let sources = super::ptr_shape_returns::possible_return_shape_new_sources(e);
+    scan_return_result(e, depth, true, &sources, out);
 }
 
-/// Scan one result arm of a returned conditional. Nested conditionals keep
-/// their result leaves in the source set, but their conditions do not.
-fn scan_conditional_result(e: &Expr, depth: u32, out: &mut Vec<NewSite>) {
+/// Scan a returned expression while preserving which nested allocations can
+/// actually become its result. `direct` controls only the position label; the
+/// source bit comes from the producer proof's own outcome analysis.
+fn scan_return_result(
+    e: &Expr,
+    depth: u32,
+    direct: bool,
+    sources: &[&Expr],
+    out: &mut Vec<NewSite>,
+) {
     match e {
         Expr::New {
             class_name,
             args,
             byte_offset,
             ..
         } => {
-            push_new_site(out, class_name, RETURNED_OPERAND, depth, *byte_offset, true);
+            let is_source = sources.iter().any(|source| std::ptr::eq(*source, e));
+            push_new_site(
+                out,
+                class_name,
+                if direct { RETURN } else { RETURNED_OPERAND },
+                depth,
+                *byte_offset,
+                is_source,
+            );
             let arg_ctx = arg_context(class_name);
             for a in args {
                 scan_expr(a, depth, arg_ctx, out);
@@ -801,8 +795,12 @@ fn scan_conditional_result(e: &Expr, depth: u32, out: &mut Vec<NewSite>) {
             else_expr,
         } => {
             scan_expr(condition, depth, RETURNED_OPERAND, out);
-            scan_conditional_result(then_expr, depth, out);
-            scan_conditional_result(else_expr, depth, out);
+            scan_return_result(then_expr, depth, false, sources, out);
+            scan_return_result(else_expr, depth, false, sources, out);
+        }
+        Expr::Logical { left, right, .. } => {
+            scan_return_result(left, depth, false, sources, out);
+            scan_return_result(right, depth, false, sources, out);
         }
         _ => scan_expr(e, depth, RETURNED_OPERAND, out),
     }