Skip to content

Commit

Permalink
check mut: from hir to mir
Browse files Browse the repository at this point in the history
just because less lines
  • Loading branch information
lengyijun committed Oct 28, 2023
1 parent 3045264 commit a748b06
Showing 1 changed file with 45 additions and 71 deletions.
116 changes: 45 additions & 71 deletions clippy_lints/src/explicit_reinitialization.rs
Original file line number Diff line number Diff line change
@@ -1,7 +1,6 @@
use clippy_utils::diagnostics::span_lint_and_sugg;
use clippy_utils::source::snippet_opt;
use clippy_utils::visitors::for_each_local_use_after_expr;
use clippy_utils::{fn_has_unsatisfiable_preds, get_parent_expr, is_from_proc_macro};
use clippy_utils::{fn_has_unsatisfiable_preds, is_from_proc_macro};
use rustc_data_structures::fx::FxHashSet;
use rustc_data_structures::graph::dominators::Dominators;
use rustc_data_structures::graph::iterate::DepthFirstSearch;
Expand All @@ -11,16 +10,14 @@ use rustc_hir::def::Res;
use rustc_hir::def_id::LocalDefId;
use rustc_hir::{
Closure, Expr, ExprKind, HirId, ImplItem, ImplItemKind, Item, ItemKind, Mutability, Node, Path, PathSegment, QPath,
StmtKind, TraitFn, TraitItem, TraitItemKind, UnOp,
StmtKind, TraitFn, TraitItem, TraitItemKind,
};
use rustc_lint::{LateContext, LateLintPass};
use rustc_middle::lint::in_external_macro;
use rustc_middle::mir::visit::{PlaceContext, Visitor};
use rustc_middle::mir::visit::{MutatingUseContext, PlaceContext, Visitor};
use rustc_middle::mir::{self, BasicBlock, Body, Local, Location, Place, Statement, Terminator};
use rustc_session::{declare_lint_pass, declare_tool_lint, Session};
use rustc_span::Span;
use std::collections::BTreeSet;
use std::ops::ControlFlow;

declare_clippy_lint! {
/// ### What it does
Expand Down Expand Up @@ -75,7 +72,7 @@ impl<'tcx> LateLintPass<'tcx> for ExplicitReinitialization {
None,
Path {
segments: [PathSegment { args: None, .. }],
res: Res::Local(local_id),
res: Res::Local(_),
..
},
)),
Expand All @@ -85,7 +82,6 @@ impl<'tcx> LateLintPass<'tcx> for ExplicitReinitialization {
right,
_,
),
hir_id: expr_id,
..
},
) = stmt.kind
Expand Down Expand Up @@ -118,65 +114,14 @@ impl<'tcx> LateLintPass<'tcx> for ExplicitReinitialization {

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

if dominate_all_usage(mir, dominators, local, start_location) {
// copy from `vec_init_then_push.rs`
let mut needs_mut = false;
let _ = for_each_local_use_after_expr(cx, *local_id, *expr_id, |e| {
let Some(parent) = get_parent_expr(cx, e) else {
return ControlFlow::Continue(());
};
let adjusted_ty = cx.typeck_results().expr_ty_adjusted(e);
let adjusted_mut = adjusted_ty.ref_mutability().unwrap_or(Mutability::Not);
needs_mut |= adjusted_mut == Mutability::Mut;
match parent.kind {
ExprKind::AddrOf(_, Mutability::Mut, _) => {
needs_mut = true;
return ControlFlow::Break(true);
},
ExprKind::Unary(UnOp::Deref, _) | ExprKind::Index(..) if !needs_mut => {
let mut last_place = parent;
while let Some(parent) = get_parent_expr(cx, last_place) {
if matches!(parent.kind, ExprKind::Unary(UnOp::Deref, _) | ExprKind::Field(..))
|| matches!(parent.kind, ExprKind::Index(e, _, _) if e.hir_id == last_place.hir_id)
{
last_place = parent;
} else {
break;
}
}
needs_mut |= cx.typeck_results().expr_ty_adjusted(last_place).ref_mutability()
== Some(Mutability::Mut)
|| get_parent_expr(cx, last_place)
.map_or(false, |e| matches!(e.kind, ExprKind::AddrOf(_, Mutability::Mut, _)));
},
ExprKind::MethodCall(_, recv, ..)
if recv.hir_id == e.hir_id
&& adjusted_mut == Mutability::Mut
&& !adjusted_ty.peel_refs().is_slice() =>
{
// No need to set `needs_mut` to true. The receiver will be either explicitly borrowed, or it
// will be implicitly borrowed via an adjustment. Both of these cases
// are already handled by this point.
return ControlFlow::Break(true);
},
ExprKind::Assign(lhs, ..) if e.hir_id == lhs.hir_id => {
needs_mut = true;
return ControlFlow::Break(false);
},
_ => (),
}
ControlFlow::Continue(())
});

let mut_str = if needs_mut { "mut " } else { "" };

if let Some(mutability) = dominate_all_usage(mir, dominators, local, location) {
span_lint_and_sugg(
cx,
EXPLICIT_REINITIALIZATION,
stmt.span,
"create a fresh variable is more explicit",
"create a fresh variable instead of reinitialization",
format!("let {mut_str}{snip}"),
format!("let {}{snip}", mutability.prefix_str()),
Applicability::MachineApplicable,
);
}
Expand Down Expand Up @@ -386,42 +331,71 @@ fn search_mir_by_span(
accurate_visitor.result
}

// None: doesn't dominate all usage
// Some(Mut): dominate all usage, and a mut usage found
// Some(Not); dominate all uagee, and no muge usage found
fn dominate_all_usage(
mir: &mir::Body<'_>,
dominators: &Dominators<BasicBlock>,
local: Local,
start_location: Location,
) -> bool {
) -> Option<Mutability> {
let mut dfs = DepthFirstSearch::new(&mir.basic_blocks);
for successor in mir.basic_blocks.successors(start_location.block) {
dfs.push_start_node(successor);
}
let reachable_bb: FxHashSet<BasicBlock> = dfs.collect();
find_usage(mir, local)

let mut res = Mutability::Not;

for (location, mutability) in find_usage(mir, local)
.into_iter()
.filter(|location| reachable_bb.contains(&location.block))
.filter(|location| !mir.basic_blocks[location.block].is_cleanup)
.all(|location| start_location.dominates(location, dominators))
.filter(|(location, _)| !mir.basic_blocks[location.block].is_cleanup)
{
if reachable_bb.contains(&location.block) {
if !start_location.dominates(location, dominators) {
return None;
}
if location != start_location && mutability == Mutability::Mut {
res = Mutability::Mut;
}
} else if location.block == start_location.block
&& location.statement_index > start_location.statement_index
&& mutability == Mutability::Mut
{
res = Mutability::Mut;
}
}
Some(res)
}

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

impl<'tcx> Visitor<'tcx> for AllLocalUsesVisitor {
fn visit_local(&mut self, local: Local, _context: PlaceContext, location: Location) {
fn visit_local(&mut self, local: Local, context: PlaceContext, location: Location) {
if local == self.for_local {
self.uses.insert(location);
match context {
PlaceContext::NonMutatingUse(_)
| PlaceContext::NonUse(_)
| PlaceContext::MutatingUse(MutatingUseContext::Drop) => {
self.uses.insert((location, Mutability::Not));
},
PlaceContext::MutatingUse(_) => {
self.uses.insert((location, Mutability::Mut));
},
}
}
}
}

let mut visitor = AllLocalUsesVisitor {
for_local: local,
uses: BTreeSet::default(),
uses: FxHashSet::default(),
};
visitor.visit_body(body);
visitor.uses
Expand Down

0 comments on commit a748b06

Please sign in to comment.