Skip to content

Commit 809a9e8

Browse files
committed
Merge branch 'simp-place-conflict'
2 parents cc4f8b0 + b49aba8 commit 809a9e8

File tree

5 files changed

+89
-91
lines changed

5 files changed

+89
-91
lines changed

compiler/rustc_borrowck/src/invalidation.rs

Lines changed: 1 addition & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -351,15 +351,14 @@ impl<'cx, 'tcx> InvalidationGenerator<'cx, 'tcx> {
351351
let tcx = self.tcx;
352352
let body = self.body;
353353
let borrow_set = self.borrow_set;
354-
let indices = self.borrow_set.indices();
355354
each_borrow_involving_path(
356355
self,
357356
tcx,
358357
body,
359358
location,
360359
(sd, place),
361360
borrow_set,
362-
indices,
361+
|_| true,
363362
|this, borrow_index, borrow| {
364363
match (rw, borrow.kind) {
365364
// Obviously an activation is compatible with its own

compiler/rustc_borrowck/src/lib.rs

Lines changed: 10 additions & 7 deletions
Original file line numberDiff line numberDiff line change
@@ -23,7 +23,7 @@ use rustc_errors::{Diagnostic, DiagnosticBuilder, DiagnosticMessage, Subdiagnost
2323
use rustc_fluent_macro::fluent_messages;
2424
use rustc_hir as hir;
2525
use rustc_hir::def_id::LocalDefId;
26-
use rustc_index::bit_set::ChunkedBitSet;
26+
use rustc_index::bit_set::{BitSet, ChunkedBitSet};
2727
use rustc_index::IndexVec;
2828
use rustc_infer::infer::{
2929
DefiningAnchor, InferCtxt, NllRegionVariableOrigin, RegionVariableOrigin, TyCtxtInferExt,
@@ -35,7 +35,6 @@ use rustc_session::lint::builtin::UNUSED_MUT;
3535
use rustc_span::{Span, Symbol};
3636
use rustc_target::abi::FieldIdx;
3737

38-
use either::Either;
3938
use smallvec::SmallVec;
4039
use std::cell::OnceCell;
4140
use std::cell::RefCell;
@@ -1000,12 +999,16 @@ impl<'cx, 'tcx> MirBorrowckCtxt<'cx, 'tcx> {
1000999
let borrow_set = self.borrow_set.clone();
10011000

10021001
// Use polonius output if it has been enabled.
1003-
let polonius_output = self.polonius_output.clone();
1004-
let borrows_in_scope = if let Some(polonius) = &polonius_output {
1002+
let mut polonius_output;
1003+
let borrows_in_scope = if let Some(polonius) = &self.polonius_output {
10051004
let location = self.location_table.start_index(location);
1006-
Either::Left(polonius.errors_at(location).iter().copied())
1005+
polonius_output = BitSet::new_empty(borrow_set.len());
1006+
for &idx in polonius.errors_at(location) {
1007+
polonius_output.insert(idx);
1008+
}
1009+
&polonius_output
10071010
} else {
1008-
Either::Right(flow_state.borrows.iter())
1011+
&flow_state.borrows
10091012
};
10101013

10111014
each_borrow_involving_path(
@@ -1015,7 +1018,7 @@ impl<'cx, 'tcx> MirBorrowckCtxt<'cx, 'tcx> {
10151018
location,
10161019
(sd, place_span.0),
10171020
&borrow_set,
1018-
borrows_in_scope,
1021+
|borrow_index| borrows_in_scope.contains(borrow_index),
10191022
|this, borrow_index, borrow| match (rw, borrow.kind) {
10201023
// Obviously an activation is compatible with its own
10211024
// reservation (or even prior activating uses of same

compiler/rustc_borrowck/src/path_utils.rs

Lines changed: 9 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -33,20 +33,24 @@ pub(super) fn each_borrow_involving_path<'tcx, F, I, S>(
3333
_location: Location,
3434
access_place: (AccessDepth, Place<'tcx>),
3535
borrow_set: &BorrowSet<'tcx>,
36-
candidates: I,
36+
is_candidate: I,
3737
mut op: F,
3838
) where
3939
F: FnMut(&mut S, BorrowIndex, &BorrowData<'tcx>) -> Control,
40-
I: Iterator<Item = BorrowIndex>,
40+
I: Fn(BorrowIndex) -> bool,
4141
{
4242
let (access, place) = access_place;
4343

44-
// FIXME: analogous code in check_loans first maps `place` to
45-
// its base_path.
44+
// The number of candidates can be large, but borrows for different locals cannot conflict with
45+
// each other, so we restrict the working set a priori.
46+
let Some(borrows_for_place_base) = borrow_set.local_map.get(&place.local) else { return };
4647

4748
// check for loan restricting path P being used. Accounts for
4849
// borrows of P, P.a.b, etc.
49-
for i in candidates {
50+
for &i in borrows_for_place_base {
51+
if !is_candidate(i) {
52+
continue;
53+
}
5054
let borrowed = &borrow_set[i];
5155

5256
if places_conflict::borrow_conflicts_with_place(

compiler/rustc_borrowck/src/places_conflict.rs

Lines changed: 66 additions & 75 deletions
Original file line numberDiff line numberDiff line change
@@ -1,3 +1,55 @@
1+
//! The borrowck rules for proving disjointness are applied from the "root" of the
2+
//! borrow forwards, iterating over "similar" projections in lockstep until
3+
//! we can prove overlap one way or another. Essentially, we treat `Overlap` as
4+
//! a monoid and report a conflict if the product ends up not being `Disjoint`.
5+
//!
6+
//! At each step, if we didn't run out of borrow or place, we know that our elements
7+
//! have the same type, and that they only overlap if they are the identical.
8+
//!
9+
//! For example, if we are comparing these:
10+
//! ```text
11+
//! BORROW: (*x1[2].y).z.a
12+
//! ACCESS: (*x1[i].y).w.b
13+
//! ```
14+
//!
15+
//! Then our steps are:
16+
//! ```text
17+
//! x1 | x1 -- places are the same
18+
//! x1[2] | x1[i] -- equal or disjoint (disjoint if indexes differ)
19+
//! x1[2].y | x1[i].y -- equal or disjoint
20+
//! *x1[2].y | *x1[i].y -- equal or disjoint
21+
//! (*x1[2].y).z | (*x1[i].y).w -- we are disjoint and don't need to check more!
22+
//! ```
23+
//!
24+
//! Because `zip` does potentially bad things to the iterator inside, this loop
25+
//! also handles the case where the access might be a *prefix* of the borrow, e.g.
26+
//!
27+
//! ```text
28+
//! BORROW: (*x1[2].y).z.a
29+
//! ACCESS: x1[i].y
30+
//! ```
31+
//!
32+
//! Then our steps are:
33+
//! ```text
34+
//! x1 | x1 -- places are the same
35+
//! x1[2] | x1[i] -- equal or disjoint (disjoint if indexes differ)
36+
//! x1[2].y | x1[i].y -- equal or disjoint
37+
//! ```
38+
//!
39+
//! -- here we run out of access - the borrow can access a part of it. If this
40+
//! is a full deep access, then we *know* the borrow conflicts with it. However,
41+
//! if the access is shallow, then we can proceed:
42+
//!
43+
//! ```text
44+
//! x1[2].y | (*x1[i].y) -- a deref! the access can't get past this, so we
45+
//! are disjoint
46+
//! ```
47+
//!
48+
//! Our invariant is, that at each step of the iteration:
49+
//! - If we didn't run out of access to match, our borrow and access are comparable
50+
//! and either equal or disjoint.
51+
//! - If we did run out of access, the borrow can access a part of it.
52+
153
#![deny(rustc::untranslatable_diagnostic)]
254
#![deny(rustc::diagnostic_outside_of_impl)]
355
use crate::ArtificialField;
@@ -46,7 +98,7 @@ pub(crate) fn places_conflict<'tcx>(
4698
/// access depth. The `bias` parameter is used to determine how the unknowable (comparing runtime
4799
/// array indices, for example) should be interpreted - this depends on what the caller wants in
48100
/// order to make the conservative choice and preserve soundness.
49-
#[instrument(level = "debug", skip(tcx, body))]
101+
#[inline]
50102
pub(super) fn borrow_conflicts_with_place<'tcx>(
51103
tcx: TyCtxt<'tcx>,
52104
body: &Body<'tcx>,
@@ -56,15 +108,24 @@ pub(super) fn borrow_conflicts_with_place<'tcx>(
56108
access: AccessDepth,
57109
bias: PlaceConflictBias,
58110
) -> bool {
111+
let borrow_local = borrow_place.local;
112+
let access_local = access_place.local;
113+
114+
if borrow_local != access_local {
115+
// We have proven the borrow disjoint - further projections will remain disjoint.
116+
return false;
117+
}
118+
59119
// This Local/Local case is handled by the more general code below, but
60120
// it's so common that it's a speed win to check for it first.
61-
if let Some(l1) = borrow_place.as_local() && let Some(l2) = access_place.as_local() {
62-
return l1 == l2;
121+
if borrow_place.projection.is_empty() && access_place.projection.is_empty() {
122+
return true;
63123
}
64124

65125
place_components_conflict(tcx, body, borrow_place, borrow_kind, access_place, access, bias)
66126
}
67127

128+
#[instrument(level = "debug", skip(tcx, body))]
68129
fn place_components_conflict<'tcx>(
69130
tcx: TyCtxt<'tcx>,
70131
body: &Body<'tcx>,
@@ -74,65 +135,10 @@ fn place_components_conflict<'tcx>(
74135
access: AccessDepth,
75136
bias: PlaceConflictBias,
76137
) -> bool {
77-
// The borrowck rules for proving disjointness are applied from the "root" of the
78-
// borrow forwards, iterating over "similar" projections in lockstep until
79-
// we can prove overlap one way or another. Essentially, we treat `Overlap` as
80-
// a monoid and report a conflict if the product ends up not being `Disjoint`.
81-
//
82-
// At each step, if we didn't run out of borrow or place, we know that our elements
83-
// have the same type, and that they only overlap if they are the identical.
84-
//
85-
// For example, if we are comparing these:
86-
// BORROW: (*x1[2].y).z.a
87-
// ACCESS: (*x1[i].y).w.b
88-
//
89-
// Then our steps are:
90-
// x1 | x1 -- places are the same
91-
// x1[2] | x1[i] -- equal or disjoint (disjoint if indexes differ)
92-
// x1[2].y | x1[i].y -- equal or disjoint
93-
// *x1[2].y | *x1[i].y -- equal or disjoint
94-
// (*x1[2].y).z | (*x1[i].y).w -- we are disjoint and don't need to check more!
95-
//
96-
// Because `zip` does potentially bad things to the iterator inside, this loop
97-
// also handles the case where the access might be a *prefix* of the borrow, e.g.
98-
//
99-
// BORROW: (*x1[2].y).z.a
100-
// ACCESS: x1[i].y
101-
//
102-
// Then our steps are:
103-
// x1 | x1 -- places are the same
104-
// x1[2] | x1[i] -- equal or disjoint (disjoint if indexes differ)
105-
// x1[2].y | x1[i].y -- equal or disjoint
106-
//
107-
// -- here we run out of access - the borrow can access a part of it. If this
108-
// is a full deep access, then we *know* the borrow conflicts with it. However,
109-
// if the access is shallow, then we can proceed:
110-
//
111-
// x1[2].y | (*x1[i].y) -- a deref! the access can't get past this, so we
112-
// are disjoint
113-
//
114-
// Our invariant is, that at each step of the iteration:
115-
// - If we didn't run out of access to match, our borrow and access are comparable
116-
// and either equal or disjoint.
117-
// - If we did run out of access, the borrow can access a part of it.
118-
119138
let borrow_local = borrow_place.local;
120139
let access_local = access_place.local;
121-
122-
match place_base_conflict(borrow_local, access_local) {
123-
Overlap::Arbitrary => {
124-
bug!("Two base can't return Arbitrary");
125-
}
126-
Overlap::EqualOrDisjoint => {
127-
// This is the recursive case - proceed to the next element.
128-
}
129-
Overlap::Disjoint => {
130-
// We have proven the borrow disjoint - further
131-
// projections will remain disjoint.
132-
debug!("borrow_conflicts_with_place: disjoint");
133-
return false;
134-
}
135-
}
140+
// borrow_conflicts_with_place should have checked that.
141+
assert_eq!(borrow_local, access_local);
136142

137143
// loop invariant: borrow_c is always either equal to access_c or disjoint from it.
138144
for (i, (borrow_c, &access_c)) in
@@ -287,21 +293,6 @@ fn place_components_conflict<'tcx>(
287293
}
288294
}
289295

290-
// Given that the bases of `elem1` and `elem2` are always either equal
291-
// or disjoint (and have the same type!), return the overlap situation
292-
// between `elem1` and `elem2`.
293-
fn place_base_conflict(l1: Local, l2: Local) -> Overlap {
294-
if l1 == l2 {
295-
// the same local - base case, equal
296-
debug!("place_element_conflict: DISJOINT-OR-EQ-LOCAL");
297-
Overlap::EqualOrDisjoint
298-
} else {
299-
// different locals - base case, disjoint
300-
debug!("place_element_conflict: DISJOINT-LOCAL");
301-
Overlap::Disjoint
302-
}
303-
}
304-
305296
// Given that the bases of `elem1` and `elem2` are always either equal
306297
// or disjoint (and have the same type!), return the overlap situation
307298
// between `elem1` and `elem2`.

compiler/rustc_borrowck/src/region_infer/mod.rs

Lines changed: 3 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -246,9 +246,10 @@ pub enum ExtraConstraintInfo {
246246
}
247247

248248
#[instrument(skip(infcx, sccs), level = "debug")]
249+
#[inline(never)] // This is a debugging tool, we need to discard it from profiles.
249250
fn sccs_info<'cx, 'tcx>(
250251
infcx: &'cx BorrowckInferCtxt<'cx, 'tcx>,
251-
sccs: Rc<Sccs<RegionVid, ConstraintSccIndex>>,
252+
sccs: &Sccs<RegionVid, ConstraintSccIndex>,
252253
) {
253254
use crate::renumber::RegionCtxt;
254255

@@ -348,7 +349,7 @@ impl<'tcx> RegionInferenceContext<'tcx> {
348349
let constraint_sccs = Rc::new(constraints.compute_sccs(&constraint_graph, fr_static));
349350

350351
if cfg!(debug_assertions) {
351-
sccs_info(_infcx, constraint_sccs.clone());
352+
sccs_info(_infcx, &*constraint_sccs);
352353
}
353354

354355
let mut scc_values =

0 commit comments

Comments
 (0)