Skip to content

Commit

Permalink
Unrolled build for rust-lang#134980
Browse files Browse the repository at this point in the history 
Rollup merge of rust-lang#134980 - lqd:polonius-next-episode-7, r=jackh726

Location-sensitive polonius prototype: endgame

This PR sets up the naive location-sensitive analysis end-to-end, and replaces the location-insensitive analysis. It's roughly all the in-progress work I wanted to land for the prototype, modulo cleanups I still want to do after the holidays, or the polonius debugger, and so on.

Here, we traverse the localized constraint graph, have to deal with kills and time-traveling (👌), and record that as loan liveness for the existing scope and active loans computations.

Then the near future looks like this, especially if the 2025h1 project goal is accepted:
- gradually bringing it up to completion
- analyzing and fixing the few remaining test failures
- going over the *numerous* fixmes in this prototype (one of which is similar to a hang on one test's millions and millions of constraints)
- trying to see how to lower the impact of the lack of NLL liveness optimization on diagnostics, and their categorization of local variables and temporaries (the vast majority of blessed expectations differences), as well as the couple ICEs trying to find an NLL constraint to blame for errors.
- dealing with the theoretical weakness around kills, conflating reachability for the two TCS, etc that is described ad nauseam in the code.
- switching the compare mode to the in-tree implementation, and blessing the diagnostics
- apart from the hang, it's not catastrophically slower on our test suite, so then we can try to enable it on CI
- checking crater, maybe trying to make it faster :3, etc.

I've tried to gradually introduce this PR's work over 4 commits, because it's kind of subtle/annoying, and Niko/I are not completely convinced yet. That one comment explaining the situation is maybe 30% of the PR 😓. Who knew that spacetime reachability and time-traveling could be mind bending.

I kinda found this late and the impact on this part of the computation was a bit unexpected to us. A bit more care/thought will be needed here. I've described my plan in the comments though. In any case, I believe we have the current implementation is a conservative approximation that shouldn't result in unsoundness but false positives at worst. So it feels fine for now.

r? ``@jackh726``

---

Fixes rust-lang#127628 -- which was a assertion triggered for a difference in loan computation between NLLs and the location-insensitive analysis. That doesn't exist anymore so I've removed this crash test.
  • Loading branch information
@rust-timer
rust-timer authored Jan 17, 2025
2 parents 0c2c096 + 8ac045d commit ceb58f7
Show file tree
Hide file tree
Showing 9 changed files with 386 additions and 185 deletions.
101 changes: 45 additions & 56 deletions compiler/rustc_borrowck/src/dataflow.rs
View file
Original file line number Diff line number Diff line change
Expand Up @@ -187,19 +187,28 @@ struct OutOfScopePrecomputer<'a, 'tcx> {
borrows_out_of_scope_at_location: FxIndexMap<Location, Vec<BorrowIndex>>,
}

impl<'a, 'tcx> OutOfScopePrecomputer<'a, 'tcx> {
fn new(body: &'a Body<'tcx>, regioncx: &'a RegionInferenceContext<'tcx>) -> Self {
OutOfScopePrecomputer {
impl<'tcx> OutOfScopePrecomputer<'_, 'tcx> {
fn compute(
body: &Body<'tcx>,
regioncx: &RegionInferenceContext<'tcx>,
borrow_set: &BorrowSet<'tcx>,
) -> FxIndexMap<Location, Vec<BorrowIndex>> {
let mut prec = OutOfScopePrecomputer {
visited: DenseBitSet::new_empty(body.basic_blocks.len()),
visit_stack: vec![],
body,
regioncx,
borrows_out_of_scope_at_location: FxIndexMap::default(),
};
for (borrow_index, borrow_data) in borrow_set.iter_enumerated() {
let borrow_region = borrow_data.region;
let location = borrow_data.reserve_location;
prec.precompute_borrows_out_of_scope(borrow_index, borrow_region, location);
}

prec.borrows_out_of_scope_at_location
}
}

impl<'tcx> OutOfScopePrecomputer<'_, 'tcx> {
fn precompute_borrows_out_of_scope(
&mut self,
borrow_index: BorrowIndex,
Expand Down Expand Up @@ -280,15 +289,7 @@ pub fn calculate_borrows_out_of_scope_at_location<'tcx>(
regioncx: &RegionInferenceContext<'tcx>,
borrow_set: &BorrowSet<'tcx>,
) -> FxIndexMap<Location, Vec<BorrowIndex>> {
let mut prec = OutOfScopePrecomputer::new(body, regioncx);
for (borrow_index, borrow_data) in borrow_set.iter_enumerated() {
let borrow_region = borrow_data.region;
let location = borrow_data.reserve_location;

prec.precompute_borrows_out_of_scope(borrow_index, borrow_region, location);
}

prec.borrows_out_of_scope_at_location
OutOfScopePrecomputer::compute(body, regioncx, borrow_set)
}

struct PoloniusOutOfScopePrecomputer<'a, 'tcx> {
Expand All @@ -300,19 +301,30 @@ struct PoloniusOutOfScopePrecomputer<'a, 'tcx> {
loans_out_of_scope_at_location: FxIndexMap<Location, Vec<BorrowIndex>>,
}

impl<'a, 'tcx> PoloniusOutOfScopePrecomputer<'a, 'tcx> {
fn new(body: &'a Body<'tcx>, regioncx: &'a RegionInferenceContext<'tcx>) -> Self {
Self {
impl<'tcx> PoloniusOutOfScopePrecomputer<'_, 'tcx> {
fn compute(
body: &Body<'tcx>,
regioncx: &RegionInferenceContext<'tcx>,
borrow_set: &BorrowSet<'tcx>,
) -> FxIndexMap<Location, Vec<BorrowIndex>> {
// The in-tree polonius analysis computes loans going out of scope using the
// set-of-loans model.
let mut prec = PoloniusOutOfScopePrecomputer {
visited: DenseBitSet::new_empty(body.basic_blocks.len()),
visit_stack: vec![],
body,
regioncx,
loans_out_of_scope_at_location: FxIndexMap::default(),
};
for (loan_idx, loan_data) in borrow_set.iter_enumerated() {
let issuing_region = loan_data.region;
let loan_issued_at = loan_data.reserve_location;
prec.precompute_loans_out_of_scope(loan_idx, issuing_region, loan_issued_at);
}

prec.loans_out_of_scope_at_location
}
}

impl<'tcx> PoloniusOutOfScopePrecomputer<'_, 'tcx> {
/// Loans are in scope while they are live: whether they are contained within any live region.
/// In the location-insensitive analysis, a loan will be contained in a region if the issuing
/// region can reach it in the subset graph. So this is a reachability problem.
Expand All @@ -325,10 +337,17 @@ impl<'tcx> PoloniusOutOfScopePrecomputer<'_, 'tcx> {
let sccs = self.regioncx.constraint_sccs();
let universal_regions = self.regioncx.universal_regions();

// The loop below was useful for the location-insensitive analysis but shouldn't be
// impactful in the location-sensitive case. It seems that it does, however, as without it a
// handful of tests fail. That likely means some liveness or outlives data related to choice
// regions is missing
// FIXME: investigate the impact of loans traversing applied member constraints and why some
// tests fail otherwise.
//
// We first handle the cases where the loan doesn't go out of scope, depending on the
// issuing region's successors.
for successor in graph::depth_first_search(&self.regioncx.region_graph(), issuing_region) {
// 1. Via applied member constraints
// Via applied member constraints
//
// The issuing region can flow into the choice regions, and they are either:
// - placeholders or free regions themselves,
Expand All @@ -346,14 +365,6 @@ impl<'tcx> PoloniusOutOfScopePrecomputer<'_, 'tcx> {
return;
}
}

// 2. Via regions that are live at all points: placeholders and free regions.
//
// If the issuing region outlives such a region, its loan escapes the function and
// cannot go out of scope. We can early return.
if self.regioncx.is_region_live_at_all_points(successor) {
return;
}
}

let first_block = loan_issued_at.block;
Expand Down Expand Up @@ -461,34 +472,12 @@ impl<'a, 'tcx> Borrows<'a, 'tcx> {
regioncx: &RegionInferenceContext<'tcx>,
borrow_set: &'a BorrowSet<'tcx>,
) -> Self {
let mut borrows_out_of_scope_at_location =
calculate_borrows_out_of_scope_at_location(body, regioncx, borrow_set);

// The in-tree polonius analysis computes loans going out of scope using the set-of-loans
// model, and makes sure they're identical to the existing computation of the set-of-points
// model.
if tcx.sess.opts.unstable_opts.polonius.is_next_enabled() {
let mut polonius_prec = PoloniusOutOfScopePrecomputer::new(body, regioncx);
for (loan_idx, loan_data) in borrow_set.iter_enumerated() {
let issuing_region = loan_data.region;
let loan_issued_at = loan_data.reserve_location;

polonius_prec.precompute_loans_out_of_scope(
loan_idx,
issuing_region,
loan_issued_at,
);
}

assert_eq!(
borrows_out_of_scope_at_location, polonius_prec.loans_out_of_scope_at_location,
"polonius loan scopes differ from NLL borrow scopes, for body {:?}",
body.span,
);

borrows_out_of_scope_at_location = polonius_prec.loans_out_of_scope_at_location;
}

let borrows_out_of_scope_at_location =
if !tcx.sess.opts.unstable_opts.polonius.is_next_enabled() {
calculate_borrows_out_of_scope_at_location(body, regioncx, borrow_set)
} else {
PoloniusOutOfScopePrecomputer::compute(body, regioncx, borrow_set)
};
Borrows { tcx, body, borrow_set, borrows_out_of_scope_at_location }
}

Expand Down
10 changes: 5 additions & 5 deletions compiler/rustc_borrowck/src/nll.rs
View file
Original file line number Diff line number Diff line change
Expand Up @@ -103,7 +103,7 @@ pub(crate) fn compute_regions<'a, 'tcx>(
constraints,
universal_region_relations,
opaque_type_values,
mut polonius_context,
polonius_context,
} = type_check::type_check(
infcx,
body,
Expand Down Expand Up @@ -142,10 +142,10 @@ pub(crate) fn compute_regions<'a, 'tcx>(
location_map,
);

// If requested for `-Zpolonius=next`, convert NLL constraints to localized outlives
// constraints.
let localized_outlives_constraints = polonius_context.as_mut().map(|polonius_context| {
polonius_context.create_localized_constraints(infcx.tcx, &regioncx, body)
// If requested for `-Zpolonius=next`, convert NLL constraints to localized outlives constraints
// and use them to compute loan liveness.
let localized_outlives_constraints = polonius_context.as_ref().map(|polonius_context| {
polonius_context.compute_loan_liveness(infcx.tcx, &mut regioncx, body, borrow_set)
});

// If requested: dump NLL facts, and run legacy polonius analysis.
Expand Down
Loading

0 comments on commit ceb58f7

Please sign in to comment.