Skip to content
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
3 changes: 3 additions & 0 deletions changelog.d/7870-ptr-shape-prefilter-report.md
Original file line number Diff line number Diff line change
@@ -0,0 +1,3 @@
**`--opt-report` now reports boxed and module-global `Ptr<Shape>` candidates instead of silently omitting them** (#7112).

These `let`-bound object allocations are rejected before the containment walk, so the report previously could not distinguish them from values the analysis never examined. Reporting builds now record the provenance denial while ordinary builds keep the existing single scan.
67 changes: 67 additions & 0 deletions crates/perry-codegen/src/collectors/ptr_shape_opt_report_tests.rs
Original file line number Diff line number Diff line change
Expand Up @@ -280,6 +280,73 @@ fn module_barrier_still_enumerates_the_candidates_it_killed() {
assert_eq!(e.rule.as_deref(), Some("rule 5 (module-wide barrier)"));
}

/// #7112: the provenance scan filters boxed and module-global bindings before
/// the containment walk. They are still real `let o = new C()` shape
/// candidates, so an empty report must not make them look unexamined.
#[test]
fn provenance_prefilters_report_boxed_and_module_global_new_bindings() {
let c = class_with_fields("C", &["x"]);
let mut classes = HashMap::new();
classes.insert("C".to_string(), &c);
let stmts = vec![let_c(1, "boxed"), let_c(2, "module_global")];
let boxed = HashSet::from([1]);
let module_globals = HashMap::from([(2, "module_global".to_string())]);

let session = Session::start();
let facts = collect_shape_proven_ptr_locals(
&stmts,
&boxed,
&module_globals,
&classes,
&clean_dispatch(),
&HashSet::new(),
&crate::collectors::ptr_shape_elements::ElementShapeFacts::default(),
);
let entries = session.entries();

assert!(facts.is_empty(), "neither filtered binding may be promoted");
assert_eq!(entries.len(), 2, "each filtered candidate is reported once");
let boxed_entry = entries
.iter()
.find(|e| e.name == "boxed")
.expect("the boxed candidate must be reported");
assert_eq!(boxed_entry.outcome, Outcome::Denied);
assert_eq!(boxed_entry.position, Position::Local);
assert_eq!(boxed_entry.rule.as_deref(), Some("rule 1 (provenance)"));
assert_eq!(
boxed_entry.tier,
Some(crate::opt_report::Tier::CompilerLimitation)
);
assert!(
boxed_entry
.reason
.as_deref()
.unwrap_or("")
.contains("boxed"),
"the denial must name the storage prefilter: {boxed_entry:?}"
);

let global_entry = entries
.iter()
.find(|e| e.name == "module_global")
.expect("the module-global candidate must be reported");
assert_eq!(global_entry.outcome, Outcome::Denied);
assert_eq!(global_entry.position, Position::Local);
assert_eq!(global_entry.rule.as_deref(), Some("rule 1 (provenance)"));
assert_eq!(
global_entry.tier,
Some(crate::opt_report::Tier::CompilerLimitation)
);
assert!(
global_entry
.reason
.as_deref()
.unwrap_or("")
.contains("module-global"),
"the denial must name the storage prefilter: {global_entry:?}"
);
}

/// The `.map(x => ({...}))` idiom: an allocation never bound to a local.
/// Rule 1 can never see it, so it must be reported as an allocation site
/// rather than silently omitted.
Expand Down
51 changes: 51 additions & 0 deletions crates/perry-codegen/src/collectors/ptr_shape_report.rs
Original file line number Diff line number Diff line change
Expand Up @@ -126,6 +126,28 @@ pub(super) const ALIAS_NOT_SINGLE_LET: ShapeDenial = ShapeDenial {
issue: None,
};

/// #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.
pub(super) const BOXED_BINDING: ShapeDenial = ShapeDenial {
rule: RULE1,
reason: "the allocation's binding is boxed (captured, async/generator, or \
otherwise cell-backed), so the function-local slot proof cannot \
anchor to it.",
tier: Tier::CompilerLimitation,
issue: None,
};

/// #7112: module globals are outside `Ptr<Shape>`'s function-local
/// containment region and are filtered before any per-candidate denial runs.
pub(super) const MODULE_GLOBAL_BINDING: ShapeDenial = ShapeDenial {
rule: RULE1,
reason: "the allocation is stored in a module-global binding, outside the \
function-local containment region this analysis proves.",
tier: Tier::CompilerLimitation,
issue: None,
};

// ── Class admission ────────────────────────────────────────────────────────

pub(super) const ADMIT_ACCESSOR: ShapeDenial = ShapeDenial {
Expand Down Expand Up @@ -869,6 +891,35 @@ pub(super) fn candidate_seeds(
// moves between buckets depending on what the package source happens to
// do.
out.retain(|id, _| !preamble.is_module_record(*id));

// #7112: the proof's scan above must stay exactly as cheap as it was for
// ordinary builds. Only an enabled report re-scans without the two storage
// filters, then records the candidates that disappeared before the
// containment walk could attach a denial. A direct `Let { init: New }` is
// a real shape candidate; unlike an arbitrary non-object local, silence
// here cannot honestly mean "not applicable".
if opt_report::enabled() && !suppressed() {
let mut unfiltered = HashMap::new();
super::find_new_candidates(stmts, &HashSet::new(), &HashMap::new(), &mut unfiltered);
unfiltered.retain(|id, _| !preamble.is_module_record(*id));
let names = local_names(stmts);
let depths = loop_depths(stmts);
for (id, class_name) in unfiltered {
if out.contains_key(&id) {
continue;
}
let denial = if boxed_vars.contains(&id) {
BOXED_BINDING
} else if module_globals.contains_key(&id) {
MODULE_GLOBAL_BINDING
} else {
// `find_new_candidates` currently has exactly these two
// filters; an included candidate has no prefilter denial.
continue;
};
deny_local(id, &names, &depths, Some(&class_name), denial);
}
}
out
}

Expand Down
Loading