Skip to content

Commit bebd568

Browse files
committed
check mut: from hir to mir
just because less lines
1 parent cdd073f commit bebd568

File tree

1 file changed

+45
-71
lines changed

1 file changed

+45
-71
lines changed

clippy_lints/src/explicit_reinitialization.rs

Lines changed: 45 additions & 71 deletions
Original file line numberDiff line numberDiff line change
@@ -1,7 +1,6 @@
11
use clippy_utils::diagnostics::span_lint_and_sugg;
22
use clippy_utils::source::snippet_opt;
3-
use clippy_utils::visitors::for_each_local_use_after_expr;
4-
use clippy_utils::{fn_has_unsatisfiable_preds, get_parent_expr, is_from_proc_macro};
3+
use clippy_utils::{fn_has_unsatisfiable_preds, is_from_proc_macro};
54
use rustc_data_structures::fx::FxHashSet;
65
use rustc_data_structures::graph::dominators::Dominators;
76
use rustc_data_structures::graph::iterate::DepthFirstSearch;
@@ -11,16 +10,14 @@ use rustc_hir::def::Res;
1110
use rustc_hir::def_id::LocalDefId;
1211
use rustc_hir::{
1312
Closure, Expr, ExprKind, HirId, ImplItem, ImplItemKind, Item, ItemKind, Mutability, Node, Path, PathSegment, QPath,
14-
StmtKind, TraitFn, TraitItem, TraitItemKind, UnOp,
13+
StmtKind, TraitFn, TraitItem, TraitItemKind,
1514
};
1615
use rustc_lint::{LateContext, LateLintPass};
1716
use rustc_middle::lint::in_external_macro;
18-
use rustc_middle::mir::visit::{PlaceContext, Visitor};
17+
use rustc_middle::mir::visit::{MutatingUseContext, PlaceContext, Visitor};
1918
use rustc_middle::mir::{self, BasicBlock, Body, Local, Location, Place, Statement, Terminator};
2019
use rustc_session::{declare_lint_pass, declare_tool_lint, Session};
2120
use rustc_span::Span;
22-
use std::collections::BTreeSet;
23-
use std::ops::ControlFlow;
2421

2522
declare_clippy_lint! {
2623
/// ### What it does
@@ -84,7 +81,7 @@ impl<'tcx> LateLintPass<'tcx> for ExplicitReinitialization {
8481
None,
8582
Path {
8683
segments: [PathSegment { args: None, .. }],
87-
res: Res::Local(local_id),
84+
res: Res::Local(_),
8885
..
8986
},
9087
)),
@@ -94,7 +91,6 @@ impl<'tcx> LateLintPass<'tcx> for ExplicitReinitialization {
9491
right,
9592
_,
9693
),
97-
hir_id: expr_id,
9894
..
9995
},
10096
) = stmt.kind
@@ -127,65 +123,14 @@ impl<'tcx> LateLintPass<'tcx> for ExplicitReinitialization {
127123

128124
assert!(start_location.dominates(location, dominators));
129125

130-
if dominate_all_usage(mir, dominators, local, start_location) {
131-
// copy from `vec_init_then_push.rs`
132-
let mut needs_mut = false;
133-
let _ = for_each_local_use_after_expr(cx, *local_id, *expr_id, |e| {
134-
let Some(parent) = get_parent_expr(cx, e) else {
135-
return ControlFlow::Continue(());
136-
};
137-
let adjusted_ty = cx.typeck_results().expr_ty_adjusted(e);
138-
let adjusted_mut = adjusted_ty.ref_mutability().unwrap_or(Mutability::Not);
139-
needs_mut |= adjusted_mut == Mutability::Mut;
140-
match parent.kind {
141-
ExprKind::AddrOf(_, Mutability::Mut, _) => {
142-
needs_mut = true;
143-
return ControlFlow::Break(true);
144-
},
145-
ExprKind::Unary(UnOp::Deref, _) | ExprKind::Index(..) if !needs_mut => {
146-
let mut last_place = parent;
147-
while let Some(parent) = get_parent_expr(cx, last_place) {
148-
if matches!(parent.kind, ExprKind::Unary(UnOp::Deref, _) | ExprKind::Field(..))
149-
|| matches!(parent.kind, ExprKind::Index(e, _, _) if e.hir_id == last_place.hir_id)
150-
{
151-
last_place = parent;
152-
} else {
153-
break;
154-
}
155-
}
156-
needs_mut |= cx.typeck_results().expr_ty_adjusted(last_place).ref_mutability()
157-
== Some(Mutability::Mut)
158-
|| get_parent_expr(cx, last_place)
159-
.map_or(false, |e| matches!(e.kind, ExprKind::AddrOf(_, Mutability::Mut, _)));
160-
},
161-
ExprKind::MethodCall(_, recv, ..)
162-
if recv.hir_id == e.hir_id
163-
&& adjusted_mut == Mutability::Mut
164-
&& !adjusted_ty.peel_refs().is_slice() =>
165-
{
166-
// No need to set `needs_mut` to true. The receiver will be either explicitly borrowed, or it
167-
// will be implicitly borrowed via an adjustment. Both of these cases
168-
// are already handled by this point.
169-
return ControlFlow::Break(true);
170-
},
171-
ExprKind::Assign(lhs, ..) if e.hir_id == lhs.hir_id => {
172-
needs_mut = true;
173-
return ControlFlow::Break(false);
174-
},
175-
_ => (),
176-
}
177-
ControlFlow::Continue(())
178-
});
179-
180-
let mut_str = if needs_mut { "mut " } else { "" };
181-
126+
if let Some(mutability) = dominate_all_usage(mir, dominators, local, location) {
182127
span_lint_and_sugg(
183128
cx,
184129
EXPLICIT_REINITIALIZATION,
185130
stmt.span,
186131
"create a fresh variable is more explicit",
187132
"create a fresh variable instead of reinitialization",
188-
format!("let {mut_str}{snip}"),
133+
format!("let {}{snip}", mutability.prefix_str()),
189134
Applicability::MachineApplicable,
190135
);
191136
}
@@ -395,42 +340,71 @@ fn search_mir_by_span(
395340
accurate_visitor.result
396341
}
397342

343+
// None: doesn't dominate all usage
344+
// Some(Mut): dominate all usage, and a mut usage found
345+
// Some(Not); dominate all uagee, and no muge usage found
398346
fn dominate_all_usage(
399347
mir: &mir::Body<'_>,
400348
dominators: &Dominators<BasicBlock>,
401349
local: Local,
402350
start_location: Location,
403-
) -> bool {
351+
) -> Option<Mutability> {
404352
let mut dfs = DepthFirstSearch::new(&mir.basic_blocks);
405353
for successor in mir.basic_blocks.successors(start_location.block) {
406354
dfs.push_start_node(successor);
407355
}
408356
let reachable_bb: FxHashSet<BasicBlock> = dfs.collect();
409-
find_usage(mir, local)
357+
358+
let mut res = Mutability::Not;
359+
360+
for (location, mutability) in find_usage(mir, local)
410361
.into_iter()
411-
.filter(|location| reachable_bb.contains(&location.block))
412-
.filter(|location| !mir.basic_blocks[location.block].is_cleanup)
413-
.all(|location| start_location.dominates(location, dominators))
362+
.filter(|(location, _)| !mir.basic_blocks[location.block].is_cleanup)
363+
{
364+
if reachable_bb.contains(&location.block) {
365+
if !start_location.dominates(location, dominators) {
366+
return None;
367+
}
368+
if location != start_location && mutability == Mutability::Mut {
369+
res = Mutability::Mut;
370+
}
371+
} else if location.block == start_location.block
372+
&& location.statement_index > start_location.statement_index
373+
&& mutability == Mutability::Mut
374+
{
375+
res = Mutability::Mut;
376+
}
377+
}
378+
Some(res)
414379
}
415380

416381
// copy from https://doc.rust-lang.org/nightly/nightly-rustc/src/rustc_borrowck/diagnostics/find_all_local_uses.rs.html#1-29
417-
fn find_usage(body: &Body<'_>, local: Local) -> BTreeSet<Location> {
382+
fn find_usage(body: &Body<'_>, local: Local) -> FxHashSet<(Location, Mutability)> {
418383
struct AllLocalUsesVisitor {
419384
for_local: Local,
420-
uses: BTreeSet<Location>,
385+
uses: FxHashSet<(Location, Mutability)>,
421386
}
422387

423388
impl<'tcx> Visitor<'tcx> for AllLocalUsesVisitor {
424-
fn visit_local(&mut self, local: Local, _context: PlaceContext, location: Location) {
389+
fn visit_local(&mut self, local: Local, context: PlaceContext, location: Location) {
425390
if local == self.for_local {
426-
self.uses.insert(location);
391+
match context {
392+
PlaceContext::NonMutatingUse(_)
393+
| PlaceContext::NonUse(_)
394+
| PlaceContext::MutatingUse(MutatingUseContext::Drop) => {
395+
self.uses.insert((location, Mutability::Not));
396+
},
397+
PlaceContext::MutatingUse(_) => {
398+
self.uses.insert((location, Mutability::Mut));
399+
},
400+
}
427401
}
428402
}
429403
}
430404

431405
let mut visitor = AllLocalUsesVisitor {
432406
for_local: local,
433-
uses: BTreeSet::default(),
407+
uses: FxHashSet::default(),
434408
};
435409
visitor.visit_body(body);
436410
visitor.uses

0 commit comments

Comments
 (0)