Skip to content

Commit 866710c

Browse files
committedAug 1, 2023
Auto merge of rust-lang#111753 - cjgillot:simp-place-conflict, r=compiler-errors
Only consider places with the same local in each_borrow_involving_path. This avoids having a busy loop that repeatedly checks for equality of locals.
2 parents b484c87 + b798939 commit 866710c

File tree

4 files changed

+87
-90
lines changed

4 files changed

+87
-90
lines changed
 

‎compiler/rustc_borrowck/src/invalidation.rs

+1-2
Original file line numberDiff line numberDiff line change
@@ -353,15 +353,14 @@ impl<'cx, 'tcx> InvalidationGenerator<'cx, 'tcx> {
353353
let tcx = self.tcx;
354354
let body = self.body;
355355
let borrow_set = self.borrow_set;
356-
let indices = self.borrow_set.indices();
357356
each_borrow_involving_path(
358357
self,
359358
tcx,
360359
body,
361360
location,
362361
(sd, place),
363362
borrow_set,
364-
indices,
363+
|_| true,
365364
|this, borrow_index, borrow| {
366365
match (rw, borrow.kind) {
367366
// Obviously an activation is compatible with its own

‎compiler/rustc_borrowck/src/lib.rs

+10-7
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::{IndexSlice, IndexVec};
2828
use rustc_infer::infer::{
2929
InferCtxt, NllRegionVariableOrigin, RegionVariableOrigin, TyCtxtInferExt,
@@ -42,7 +42,6 @@ use rustc_session::lint::builtin::UNUSED_MUT;
4242
use rustc_span::{Span, Symbol};
4343
use rustc_target::abi::FieldIdx;
4444

45-
use either::Either;
4645
use smallvec::SmallVec;
4746
use std::cell::RefCell;
4847
use std::collections::BTreeMap;
@@ -1035,12 +1034,16 @@ impl<'cx, 'tcx> MirBorrowckCtxt<'cx, 'tcx> {
10351034
let borrow_set = self.borrow_set.clone();
10361035

10371036
// Use polonius output if it has been enabled.
1038-
let polonius_output = self.polonius_output.clone();
1039-
let borrows_in_scope = if let Some(polonius) = &polonius_output {
1037+
let mut polonius_output;
1038+
let borrows_in_scope = if let Some(polonius) = &self.polonius_output {
10401039
let location = self.location_table.start_index(location);
1041-
Either::Left(polonius.errors_at(location).iter().copied())
1040+
polonius_output = BitSet::new_empty(borrow_set.len());
1041+
for &idx in polonius.errors_at(location) {
1042+
polonius_output.insert(idx);
1043+
}
1044+
&polonius_output
10421045
} else {
1043-
Either::Right(flow_state.borrows.iter())
1046+
&flow_state.borrows
10441047
};
10451048

10461049
each_borrow_involving_path(
@@ -1050,7 +1053,7 @@ impl<'cx, 'tcx> MirBorrowckCtxt<'cx, 'tcx> {
10501053
location,
10511054
(sd, place_span.0),
10521055
&borrow_set,
1053-
borrows_in_scope,
1056+
|borrow_index| borrows_in_scope.contains(borrow_index),
10541057
|this, borrow_index, borrow| match (rw, borrow.kind) {
10551058
// Obviously an activation is compatible with its own
10561059
// reservation (or even prior activating uses of same

‎compiler/rustc_borrowck/src/path_utils.rs

+9-5
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

+67-76
Original file line numberDiff line numberDiff line change
@@ -1,11 +1,63 @@
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;
456
use crate::Overlap;
557
use crate::{AccessDepth, Deep, Shallow};
658
use rustc_hir as hir;
759
use rustc_middle::mir::{
8-
Body, BorrowKind, Local, MutBorrowKind, Place, PlaceElem, PlaceRef, ProjectionElem,
60+
Body, BorrowKind, MutBorrowKind, Place, PlaceElem, PlaceRef, ProjectionElem,
961
};
1062
use rustc_middle::ty::{self, TyCtxt};
1163
use std::cmp::max;
@@ -48,7 +100,7 @@ pub fn places_conflict<'tcx>(
48100
/// access depth. The `bias` parameter is used to determine how the unknowable (comparing runtime
49101
/// array indices, for example) should be interpreted - this depends on what the caller wants in
50102
/// order to make the conservative choice and preserve soundness.
51-
#[instrument(level = "debug", skip(tcx, body))]
103+
#[inline]
52104
pub(super) fn borrow_conflicts_with_place<'tcx>(
53105
tcx: TyCtxt<'tcx>,
54106
body: &Body<'tcx>,
@@ -58,15 +110,24 @@ pub(super) fn borrow_conflicts_with_place<'tcx>(
58110
access: AccessDepth,
59111
bias: PlaceConflictBias,
60112
) -> bool {
113+
let borrow_local = borrow_place.local;
114+
let access_local = access_place.local;
115+
116+
if borrow_local != access_local {
117+
// We have proven the borrow disjoint - further projections will remain disjoint.
118+
return false;
119+
}
120+
61121
// This Local/Local case is handled by the more general code below, but
62122
// it's so common that it's a speed win to check for it first.
63-
if let Some(l1) = borrow_place.as_local() && let Some(l2) = access_place.as_local() {
64-
return l1 == l2;
123+
if borrow_place.projection.is_empty() && access_place.projection.is_empty() {
124+
return true;
65125
}
66126

67127
place_components_conflict(tcx, body, borrow_place, borrow_kind, access_place, access, bias)
68128
}
69129

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

139145
// loop invariant: borrow_c is always either equal to access_c or disjoint from it.
140146
for ((borrow_place, borrow_c), &access_c) in
@@ -277,21 +283,6 @@ fn place_components_conflict<'tcx>(
277283
}
278284
}
279285

280-
// Given that the bases of `elem1` and `elem2` are always either equal
281-
// or disjoint (and have the same type!), return the overlap situation
282-
// between `elem1` and `elem2`.
283-
fn place_base_conflict(l1: Local, l2: Local) -> Overlap {
284-
if l1 == l2 {
285-
// the same local - base case, equal
286-
debug!("place_element_conflict: DISJOINT-OR-EQ-LOCAL");
287-
Overlap::EqualOrDisjoint
288-
} else {
289-
// different locals - base case, disjoint
290-
debug!("place_element_conflict: DISJOINT-LOCAL");
291-
Overlap::Disjoint
292-
}
293-
}
294-
295286
// Given that the bases of `elem1` and `elem2` are always either equal
296287
// or disjoint (and have the same type!), return the overlap situation
297288
// between `elem1` and `elem2`.

0 commit comments

Comments
 (0)
Please sign in to comment.