Skip to content

Commit 5eed9c6

Browse files
committed
1 parent f9ca9d4 commit 5eed9c6

9 files changed

+109
-234
lines changed

clippy_lints/src/dereference.rs

+4-4
Original file line numberDiff line numberDiff line change
@@ -1282,10 +1282,10 @@ fn referent_used_exactly_once<'tcx>(
12821282
possible_borrowers.push((body_owner_local_def_id, PossibleBorrowerMap::new(cx, mir)));
12831283
}
12841284
let possible_borrower = &mut possible_borrowers.last_mut().unwrap().1;
1285-
// If `place.local` were not included here, the `copyable_iterator::warn` test would fail. The
1286-
// reason is that `PossibleBorrowerVisitor::visit_terminator` considers `place.local` a possible
1287-
// borrower of itself. See the comment in that method for an explanation as to why.
1288-
possible_borrower.at_most_borrowers(cx, &[local, place.local], place.local, location)
1285+
// If `only_borrowers` were used here, the `copyable_iterator::warn` test would fail. The reason is
1286+
// that `PossibleBorrowerVisitor::visit_terminator` considers `place.local` a possible borrower of
1287+
// itself. See the comment in that method for an explanation as to why.
1288+
possible_borrower.bounded_borrowers(&[local], &[local, place.local], place.local, location)
12891289
&& used_exactly_once(mir, place.local).unwrap_or(false)
12901290
} else {
12911291
false

clippy_lints/src/redundant_clone.rs

+2-2
Original file line numberDiff line numberDiff line change
@@ -131,7 +131,7 @@ impl<'tcx> LateLintPass<'tcx> for RedundantClone {
131131
// `res = clone(arg)` can be turned into `res = move arg;`
132132
// if `arg` is the only borrow of `cloned` at this point.
133133

134-
if cannot_move_out || !possible_borrower.at_most_borrowers(cx, &[arg], cloned, loc) {
134+
if cannot_move_out || !possible_borrower.only_borrowers(&[arg], cloned, loc) {
135135
continue;
136136
}
137137

@@ -178,7 +178,7 @@ impl<'tcx> LateLintPass<'tcx> for RedundantClone {
178178
// StorageDead(pred_arg);
179179
// res = to_path_buf(cloned);
180180
// ```
181-
if cannot_move_out || !possible_borrower.at_most_borrowers(cx, &[arg, cloned], local, loc) {
181+
if cannot_move_out || !possible_borrower.only_borrowers(&[arg, cloned], local, loc) {
182182
continue;
183183
}
184184

clippy_utils/src/mir/possible_borrower.rs

+99-172
Original file line numberDiff line numberDiff line change
@@ -1,137 +1,89 @@
1-
use super::possible_origin::PossibleOriginVisitor;
1+
use super::{possible_origin::PossibleOriginVisitor, transitive_relation::TransitiveRelation};
22
use crate::ty::is_copy;
3-
use rustc_data_structures::fx::{FxHashMap, FxIndexMap};
3+
use rustc_data_structures::fx::FxHashMap;
44
use rustc_index::bit_set::{BitSet, HybridBitSet};
55
use rustc_lint::LateContext;
6-
use rustc_middle::mir::{
7-
self, visit::Visitor as _, BasicBlock, Local, Location, Mutability, Statement, StatementKind, Terminator,
8-
};
9-
use rustc_middle::ty::{self, visit::TypeVisitor, TyCtxt};
10-
use rustc_mir_dataflow::{
11-
fmt::DebugWithContext, impls::MaybeStorageLive, lattice::JoinSemiLattice, Analysis, AnalysisDomain,
12-
CallReturnPlaces, ResultsCursor,
13-
};
14-
use std::borrow::Cow;
6+
use rustc_middle::mir::{self, visit::Visitor as _, Mutability};
7+
use rustc_middle::ty::{self, visit::TypeVisitor};
8+
use rustc_mir_dataflow::{impls::MaybeStorageLive, Analysis, ResultsCursor};
159
use std::ops::ControlFlow;
1610

1711
/// Collects the possible borrowers of each local.
1812
/// For example, `b = &a; c = &a;` will make `b` and (transitively) `c`
1913
/// possible borrowers of `a`.
2014
#[allow(clippy::module_name_repetitions)]
21-
struct PossibleBorrowerAnalysis<'b, 'tcx> {
22-
tcx: TyCtxt<'tcx>,
15+
struct PossibleBorrowerVisitor<'a, 'b, 'tcx> {
16+
possible_borrower: TransitiveRelation,
2317
body: &'b mir::Body<'tcx>,
18+
cx: &'a LateContext<'tcx>,
2419
possible_origin: FxHashMap<mir::Local, HybridBitSet<mir::Local>>,
2520
}
2621

27-
#[derive(Clone, Debug, Eq, PartialEq)]
28-
struct PossibleBorrowerState {
29-
map: FxIndexMap<Local, BitSet<Local>>,
30-
domain_size: usize,
31-
}
32-
33-
impl PossibleBorrowerState {
34-
fn new(domain_size: usize) -> Self {
22+
impl<'a, 'b, 'tcx> PossibleBorrowerVisitor<'a, 'b, 'tcx> {
23+
fn new(
24+
cx: &'a LateContext<'tcx>,
25+
body: &'b mir::Body<'tcx>,
26+
possible_origin: FxHashMap<mir::Local, HybridBitSet<mir::Local>>,
27+
) -> Self {
3528
Self {
36-
map: FxIndexMap::default(),
37-
domain_size,
29+
possible_borrower: TransitiveRelation::default(),
30+
cx,
31+
body,
32+
possible_origin,
3833
}
3934
}
4035

41-
#[allow(clippy::similar_names)]
42-
fn add(&mut self, borrowed: Local, borrower: Local) {
43-
self.map
44-
.entry(borrowed)
45-
.or_insert(BitSet::new_empty(self.domain_size))
46-
.insert(borrower);
47-
}
48-
}
49-
50-
impl<C> DebugWithContext<C> for PossibleBorrowerState {
51-
fn fmt_with(&self, _ctxt: &C, f: &mut std::fmt::Formatter<'_>) -> std::fmt::Result {
52-
<_ as std::fmt::Debug>::fmt(self, f)
53-
}
54-
fn fmt_diff_with(&self, _old: &Self, _ctxt: &C, _f: &mut std::fmt::Formatter<'_>) -> std::fmt::Result {
55-
unimplemented!()
56-
}
57-
}
36+
fn into_map(
37+
self,
38+
cx: &'a LateContext<'tcx>,
39+
maybe_live: ResultsCursor<'b, 'tcx, MaybeStorageLive>,
40+
) -> PossibleBorrowerMap<'b, 'tcx> {
41+
let mut map = FxHashMap::default();
42+
for row in (1..self.body.local_decls.len()).map(mir::Local::from_usize) {
43+
if is_copy(cx, self.body.local_decls[row].ty) {
44+
continue;
45+
}
5846

59-
impl JoinSemiLattice for PossibleBorrowerState {
60-
fn join(&mut self, other: &Self) -> bool {
61-
let mut changed = false;
62-
for (&borrowed, borrowers) in other.map.iter() {
47+
let mut borrowers = self.possible_borrower.reachable_from(row, self.body.local_decls.len());
48+
borrowers.remove(mir::Local::from_usize(0));
6349
if !borrowers.is_empty() {
64-
changed |= self
65-
.map
66-
.entry(borrowed)
67-
.or_insert(BitSet::new_empty(self.domain_size))
68-
.union(borrowers);
50+
map.insert(row, borrowers);
6951
}
7052
}
71-
changed
72-
}
73-
}
74-
75-
impl<'b, 'tcx> AnalysisDomain<'tcx> for PossibleBorrowerAnalysis<'b, 'tcx> {
76-
type Domain = PossibleBorrowerState;
77-
78-
const NAME: &'static str = "possible_borrower";
79-
80-
fn bottom_value(&self, body: &mir::Body<'tcx>) -> Self::Domain {
81-
PossibleBorrowerState::new(body.local_decls.len())
82-
}
83-
84-
fn initialize_start_block(&self, _body: &mir::Body<'tcx>, _entry_set: &mut Self::Domain) {}
85-
}
8653

87-
impl<'b, 'tcx> PossibleBorrowerAnalysis<'b, 'tcx> {
88-
fn new(
89-
tcx: TyCtxt<'tcx>,
90-
body: &'b mir::Body<'tcx>,
91-
possible_origin: FxHashMap<mir::Local, HybridBitSet<mir::Local>>,
92-
) -> Self {
93-
Self {
94-
tcx,
95-
body,
96-
possible_origin,
54+
let bs = BitSet::new_empty(self.body.local_decls.len());
55+
PossibleBorrowerMap {
56+
map,
57+
maybe_live,
58+
bitset: (bs.clone(), bs),
9759
}
9860
}
9961
}
10062

101-
impl<'b, 'tcx> Analysis<'tcx> for PossibleBorrowerAnalysis<'b, 'tcx> {
102-
fn apply_call_return_effect(
103-
&self,
104-
_state: &mut Self::Domain,
105-
_block: BasicBlock,
106-
_return_places: CallReturnPlaces<'_, 'tcx>,
107-
) {
108-
}
109-
110-
fn apply_statement_effect(&self, state: &mut Self::Domain, statement: &Statement<'tcx>, _location: Location) {
111-
if let StatementKind::Assign(box (place, rvalue)) = &statement.kind {
112-
let lhs = place.local;
113-
match rvalue {
114-
mir::Rvalue::Ref(_, _, borrowed) => {
115-
state.add(borrowed.local, lhs);
116-
},
117-
other => {
118-
if ContainsRegion
119-
.visit_ty(place.ty(&self.body.local_decls, self.tcx).ty)
120-
.is_continue()
121-
{
122-
return;
63+
impl<'a, 'b, 'tcx> mir::visit::Visitor<'tcx> for PossibleBorrowerVisitor<'a, 'b, 'tcx> {
64+
fn visit_assign(&mut self, place: &mir::Place<'tcx>, rvalue: &mir::Rvalue<'_>, _location: mir::Location) {
65+
let lhs = place.local;
66+
match rvalue {
67+
mir::Rvalue::Ref(_, _, borrowed) => {
68+
self.possible_borrower.add(borrowed.local, lhs);
69+
},
70+
other => {
71+
if ContainsRegion
72+
.visit_ty(place.ty(&self.body.local_decls, self.cx.tcx).ty)
73+
.is_continue()
74+
{
75+
return;
76+
}
77+
rvalue_locals(other, |rhs| {
78+
if lhs != rhs {
79+
self.possible_borrower.add(rhs, lhs);
12380
}
124-
rvalue_locals(other, |rhs| {
125-
if lhs != rhs {
126-
state.add(rhs, lhs);
127-
}
128-
});
129-
},
130-
}
81+
});
82+
},
13183
}
13284
}
13385

134-
fn apply_terminator_effect(&self, state: &mut Self::Domain, terminator: &Terminator<'tcx>, _location: Location) {
86+
fn visit_terminator(&mut self, terminator: &mir::Terminator<'_>, _loc: mir::Location) {
13587
if let mir::TerminatorKind::Call {
13688
args,
13789
destination: mir::Place { local: dest, .. },
@@ -171,10 +123,10 @@ impl<'b, 'tcx> Analysis<'tcx> for PossibleBorrowerAnalysis<'b, 'tcx> {
171123

172124
for y in mutable_variables {
173125
for x in &immutable_borrowers {
174-
state.add(*x, y);
126+
self.possible_borrower.add(*x, y);
175127
}
176128
for x in &mutable_borrowers {
177-
state.add(*x, y);
129+
self.possible_borrower.add(*x, y);
178130
}
179131
}
180132
}
@@ -210,98 +162,73 @@ fn rvalue_locals(rvalue: &mir::Rvalue<'_>, mut visit: impl FnMut(mir::Local)) {
210162
}
211163
}
212164

213-
/// Result of `PossibleBorrowerAnalysis`.
165+
/// Result of `PossibleBorrowerVisitor`.
214166
#[allow(clippy::module_name_repetitions)]
215167
pub struct PossibleBorrowerMap<'b, 'tcx> {
216-
body: &'b mir::Body<'tcx>,
217-
possible_borrower: ResultsCursor<'b, 'tcx, PossibleBorrowerAnalysis<'b, 'tcx>>,
218-
maybe_live: ResultsCursor<'b, 'tcx, MaybeStorageLive<'b>>,
219-
pushed: BitSet<Local>,
220-
stack: Vec<Local>,
168+
/// Mapping `Local -> its possible borrowers`
169+
pub map: FxHashMap<mir::Local, HybridBitSet<mir::Local>>,
170+
maybe_live: ResultsCursor<'b, 'tcx, MaybeStorageLive>,
171+
// Caches to avoid allocation of `BitSet` on every query
172+
pub bitset: (BitSet<mir::Local>, BitSet<mir::Local>),
221173
}
222174

223-
impl<'b, 'tcx> PossibleBorrowerMap<'b, 'tcx> {
224-
pub fn new(cx: &LateContext<'tcx>, mir: &'b mir::Body<'tcx>) -> Self {
175+
impl<'a, 'b, 'tcx> PossibleBorrowerMap<'b, 'tcx> {
176+
pub fn new(cx: &'a LateContext<'tcx>, mir: &'b mir::Body<'tcx>) -> Self {
225177
let possible_origin = {
226178
let mut vis = PossibleOriginVisitor::new(mir);
227179
vis.visit_body(mir);
228180
vis.into_map(cx)
229181
};
230-
let possible_borrower = PossibleBorrowerAnalysis::new(cx.tcx, mir, possible_origin)
182+
let maybe_storage_live_result = MaybeStorageLive::new(BitSet::new_empty(mir.local_decls.len()))
231183
.into_engine(cx.tcx, mir)
232-
.pass_name("possible_borrower")
184+
.pass_name("redundant_clone")
233185
.iterate_to_fixpoint()
234186
.into_results_cursor(mir);
235-
let maybe_live = MaybeStorageLive::new(Cow::Owned(BitSet::new_empty(mir.local_decls.len())))
236-
.into_engine(cx.tcx, mir)
237-
.pass_name("possible_borrower")
238-
.iterate_to_fixpoint()
239-
.into_results_cursor(mir);
240-
PossibleBorrowerMap {
241-
body: mir,
242-
possible_borrower,
243-
maybe_live,
244-
pushed: BitSet::new_empty(mir.local_decls.len()),
245-
stack: Vec::with_capacity(mir.local_decls.len()),
246-
}
187+
let mut vis = PossibleBorrowerVisitor::new(cx, mir, possible_origin);
188+
vis.visit_body(mir);
189+
vis.into_map(cx, maybe_storage_live_result)
247190
}
248191

249-
/// Returns true if the set of borrowers of `borrowed` living at `at` includes no more than
250-
/// `borrowers`.
251-
/// Notes:
252-
/// 1. It would be nice if `PossibleBorrowerMap` could store `cx` so that `at_most_borrowers`
253-
/// would not require it to be passed in. But a `PossibleBorrowerMap` is stored in `LintPass`
254-
/// `Dereferencing`, which outlives any `LateContext`.
255-
/// 2. In all current uses of `at_most_borrowers`, `borrowers` is a slice of at most two
256-
/// elements. Thus, `borrowers.contains(...)` is effectively a constant-time operation. If
257-
/// `at_most_borrowers`'s uses were to expand beyond this, its implementation might have to be
258-
/// adjusted.
259-
pub fn at_most_borrowers(
192+
/// Returns true if the set of borrowers of `borrowed` living at `at` matches with `borrowers`.
193+
pub fn only_borrowers(&mut self, borrowers: &[mir::Local], borrowed: mir::Local, at: mir::Location) -> bool {
194+
self.bounded_borrowers(borrowers, borrowers, borrowed, at)
195+
}
196+
197+
/// Returns true if the set of borrowers of `borrowed` living at `at` includes at least `below`
198+
/// but no more than `above`.
199+
pub fn bounded_borrowers(
260200
&mut self,
261-
cx: &LateContext<'tcx>,
262-
borrowers: &[mir::Local],
201+
below: &[mir::Local],
202+
above: &[mir::Local],
263203
borrowed: mir::Local,
264204
at: mir::Location,
265205
) -> bool {
266-
if is_copy(cx, self.body.local_decls[borrowed].ty) {
267-
return true;
268-
}
269-
270-
self.possible_borrower.seek_before_primary_effect(at);
271-
self.maybe_live.seek_before_primary_effect(at);
272-
273-
let possible_borrower = &self.possible_borrower.get().map;
274-
let maybe_live = &self.maybe_live;
275-
276-
self.pushed.clear();
277-
self.stack.clear();
206+
self.maybe_live.seek_after_primary_effect(at);
278207

279-
if let Some(borrowers) = possible_borrower.get(&borrowed) {
280-
for b in borrowers.iter() {
281-
if self.pushed.insert(b) {
282-
self.stack.push(b);
283-
}
208+
self.bitset.0.clear();
209+
let maybe_live = &mut self.maybe_live;
210+
if let Some(bitset) = self.map.get(&borrowed) {
211+
for b in bitset.iter().filter(move |b| maybe_live.contains(*b)) {
212+
self.bitset.0.insert(b);
284213
}
285214
} else {
286-
// Nothing borrows `borrowed` at `at`.
287-
return true;
215+
return false;
288216
}
289217

290-
while let Some(borrower) = self.stack.pop() {
291-
if maybe_live.contains(borrower) && !borrowers.contains(&borrower) {
292-
return false;
293-
}
218+
self.bitset.1.clear();
219+
for b in below {
220+
self.bitset.1.insert(*b);
221+
}
294222

295-
if let Some(borrowers) = possible_borrower.get(&borrower) {
296-
for b in borrowers.iter() {
297-
if self.pushed.insert(b) {
298-
self.stack.push(b);
299-
}
300-
}
301-
}
223+
if !self.bitset.0.superset(&self.bitset.1) {
224+
return false;
225+
}
226+
227+
for b in above {
228+
self.bitset.0.remove(*b);
302229
}
303230

304-
true
231+
self.bitset.0.is_empty()
305232
}
306233

307234
pub fn local_is_alive_at(&mut self, local: mir::Local, at: mir::Location) -> bool {

0 commit comments

Comments
 (0)