Skip to content

Commit 6006861

Browse files
committed
1 parent 2171749 commit 6006861

File tree

9 files changed

+287
-163
lines changed

9 files changed

+287
-163
lines changed

clippy_lints/src/dereference.rs

Lines changed: 68 additions & 29 deletions
Original file line numberDiff line numberDiff line change
@@ -1,7 +1,5 @@
11
use clippy_utils::diagnostics::{span_lint_and_sugg, span_lint_hir_and_then};
2-
use clippy_utils::mir::{
3-
dropped_without_further_use, enclosing_mir, expr_local, local_assignments, PossibleBorrowerMap,
4-
};
2+
use clippy_utils::mir::{enclosing_mir, expr_local, local_assignments, used_exactly_once, PossibleBorrowerMap};
53
use clippy_utils::source::{snippet_with_applicability, snippet_with_context};
64
use clippy_utils::sugg::has_enclosing_paren;
75
use clippy_utils::ty::{expr_sig, is_copy, peel_mid_ty_refs, ty_sig, variant_of_res};
@@ -14,21 +12,24 @@ use rustc_data_structures::fx::FxIndexMap;
1412
use rustc_errors::Applicability;
1513
use rustc_hir::intravisit::{walk_ty, Visitor};
1614
use rustc_hir::{
17-
self as hir, def_id::DefId, BindingAnnotation, Body, BodyId, BorrowKind, Closure, Expr, ExprKind, FnRetTy,
18-
GenericArg, HirId, ImplItem, ImplItemKind, Item, ItemKind, Local, MatchSource, Mutability, Node, Pat, PatKind,
19-
Path, QPath, TraitItem, TraitItemKind, TyKind, UnOp,
15+
self as hir,
16+
def_id::{DefId, LocalDefId},
17+
BindingAnnotation, Body, BodyId, BorrowKind, Closure, Expr, ExprKind, FnRetTy, GenericArg, HirId, ImplItem,
18+
ImplItemKind, Item, ItemKind, Local, MatchSource, Mutability, Node, Pat, PatKind, Path, QPath, TraitItem,
19+
TraitItemKind, TyKind, UnOp,
2020
};
2121
use rustc_index::bit_set::BitSet;
2222
use rustc_infer::infer::TyCtxtInferExt;
23-
use rustc_lint::{LateContext, LateLintPass};
23+
use rustc_lint::{LateContext, LateLintPass, LintArray, LintPass};
24+
use rustc_lint_defs::lint_array;
2425
use rustc_middle::mir::{Rvalue, StatementKind};
2526
use rustc_middle::ty::adjustment::{Adjust, Adjustment, AutoBorrow, AutoBorrowMutability};
2627
use rustc_middle::ty::{
2728
self, subst::Subst, Binder, BoundVariableKind, EarlyBinder, FnSig, GenericArgKind, List, ParamTy, PredicateKind,
2829
ProjectionPredicate, Ty, TyCtxt, TypeVisitable, TypeckResults,
2930
};
3031
use rustc_semver::RustcVersion;
31-
use rustc_session::{declare_tool_lint, impl_lint_pass};
32+
use rustc_session::declare_tool_lint;
3233
use rustc_span::{symbol::sym, Span, Symbol, DUMMY_SP};
3334
use rustc_trait_selection::infer::InferCtxtExt as _;
3435
use rustc_trait_selection::traits::{query::evaluate_obligation::InferCtxtExt as _, Obligation, ObligationCause};
@@ -145,15 +146,27 @@ declare_clippy_lint! {
145146
"dereferencing when the compiler would automatically dereference"
146147
}
147148

148-
impl_lint_pass!(Dereferencing => [
149-
EXPLICIT_DEREF_METHODS,
150-
NEEDLESS_BORROW,
151-
REF_BINDING_TO_REFERENCE,
152-
EXPLICIT_AUTO_DEREF,
153-
]);
149+
#[expect(rustc::internal)]
150+
impl<'tcx> LintPass for Dereferencing<'tcx> {
151+
fn name(&self) -> &'static str {
152+
stringify!($ty)
153+
}
154+
}
155+
156+
impl<'tcx> Dereferencing<'tcx> {
157+
#[expect(dead_code)]
158+
pub fn get_lints() -> LintArray {
159+
lint_array!(
160+
EXPLICIT_DEREF_METHODS,
161+
NEEDLESS_BORROW,
162+
REF_BINDING_TO_REFERENCE,
163+
EXPLICIT_AUTO_DEREF,
164+
)
165+
}
166+
}
154167

155168
#[derive(Default)]
156-
pub struct Dereferencing {
169+
pub struct Dereferencing<'tcx> {
157170
state: Option<(State, StateData)>,
158171

159172
// While parsing a `deref` method call in ufcs form, the path to the function is itself an
@@ -174,11 +187,15 @@ pub struct Dereferencing {
174187
/// e.g. `m!(x) | Foo::Bar(ref x)`
175188
ref_locals: FxIndexMap<HirId, Option<RefPat>>,
176189

190+
/// Map from body owners to `PossibleBorrowerMap`s. Used by `needless_borrow_impl_arg_position`
191+
/// to determine when a borrowed expression can instead be moved.
192+
possible_borrowers: FxIndexMap<LocalDefId, PossibleBorrowerMap<'tcx, 'tcx>>,
193+
177194
// `IntoIterator` for arrays requires Rust 1.53.
178195
msrv: Option<RustcVersion>,
179196
}
180197

181-
impl Dereferencing {
198+
impl<'tcx> Dereferencing<'tcx> {
182199
#[must_use]
183200
pub fn new(msrv: Option<RustcVersion>) -> Self {
184201
Self {
@@ -245,7 +262,7 @@ struct RefPat {
245262
hir_id: HirId,
246263
}
247264

248-
impl<'tcx> LateLintPass<'tcx> for Dereferencing {
265+
impl<'tcx> LateLintPass<'tcx> for Dereferencing<'tcx> {
249266
#[expect(clippy::too_many_lines)]
250267
fn check_expr(&mut self, cx: &LateContext<'tcx>, expr: &'tcx Expr<'_>) {
251268
// Skip path expressions from deref calls. e.g. `Deref::deref(e)`
@@ -279,7 +296,7 @@ impl<'tcx> LateLintPass<'tcx> for Dereferencing {
279296
match (self.state.take(), kind) {
280297
(None, kind) => {
281298
let expr_ty = typeck.expr_ty(expr);
282-
let (position, adjustments) = walk_parents(cx, expr, self.msrv);
299+
let (position, adjustments) = walk_parents(cx, &mut self.possible_borrowers, expr, self.msrv);
283300

284301
match kind {
285302
RefOp::Deref => {
@@ -678,6 +695,7 @@ impl Position {
678695
#[expect(clippy::too_many_lines)]
679696
fn walk_parents<'tcx>(
680697
cx: &LateContext<'tcx>,
698+
possible_borrowers: &mut FxIndexMap<LocalDefId, PossibleBorrowerMap<'tcx, 'tcx>>,
681699
e: &'tcx Expr<'_>,
682700
msrv: Option<RustcVersion>,
683701
) -> (Position, &'tcx [Adjustment<'tcx>]) {
@@ -792,7 +810,16 @@ fn walk_parents<'tcx>(
792810
Some(hir_ty) => binding_ty_auto_deref_stability(cx, hir_ty, precedence, ty.bound_vars()),
793811
None => {
794812
if let ty::Param(param_ty) = ty.skip_binder().kind() {
795-
needless_borrow_impl_arg_position(cx, parent, i, *param_ty, e, precedence, msrv)
813+
needless_borrow_impl_arg_position(
814+
cx,
815+
possible_borrowers,
816+
parent,
817+
i,
818+
*param_ty,
819+
e,
820+
precedence,
821+
msrv,
822+
)
796823
} else {
797824
ty_auto_deref_stability(cx, cx.tcx.erase_late_bound_regions(ty), precedence)
798825
.position_for_arg()
@@ -840,7 +867,16 @@ fn walk_parents<'tcx>(
840867
args.iter().position(|arg| arg.hir_id == child_id).map(|i| {
841868
let ty = cx.tcx.fn_sig(id).skip_binder().inputs()[i + 1];
842869
if let ty::Param(param_ty) = ty.kind() {
843-
needless_borrow_impl_arg_position(cx, parent, i + 1, *param_ty, e, precedence, msrv)
870+
needless_borrow_impl_arg_position(
871+
cx,
872+
possible_borrowers,
873+
parent,
874+
i + 1,
875+
*param_ty,
876+
e,
877+
precedence,
878+
msrv,
879+
)
844880
} else {
845881
ty_auto_deref_stability(
846882
cx,
@@ -1004,8 +1040,10 @@ fn ty_contains_infer(ty: &hir::Ty<'_>) -> bool {
10041040
// If the conditions are met, returns `Some(Position::ImplArg(..))`; otherwise, returns `None`.
10051041
// The "is copyable" condition is to avoid the case where removing the `&` means `e` would have to
10061042
// be moved, but it cannot be.
1043+
#[expect(clippy::too_many_arguments)]
10071044
fn needless_borrow_impl_arg_position<'tcx>(
10081045
cx: &LateContext<'tcx>,
1046+
possible_borrowers: &mut FxIndexMap<LocalDefId, PossibleBorrowerMap<'tcx, 'tcx>>,
10091047
parent: &Expr<'tcx>,
10101048
arg_index: usize,
10111049
param_ty: ParamTy,
@@ -1064,8 +1102,6 @@ fn needless_borrow_impl_arg_position<'tcx>(
10641102
return Position::Other(precedence);
10651103
}
10661104

1067-
let mut possible_borrower = None;
1068-
10691105
// `substs_with_referent_ty` can be constructed outside of `check_referent` because the same
10701106
// elements are modified each time `check_referent` is called.
10711107
let mut substs_with_referent_ty = substs_with_expr_ty.to_vec();
@@ -1075,7 +1111,7 @@ fn needless_borrow_impl_arg_position<'tcx>(
10751111

10761112
if !is_copy(cx, referent_ty)
10771113
&& (referent_ty.has_significant_drop(cx.tcx, cx.param_env)
1078-
|| !referent_dropped_without_further_use(cx, &mut possible_borrower, reference))
1114+
|| !referent_used_exactly_once(cx, possible_borrowers, reference))
10791115
{
10801116
return false;
10811117
}
@@ -1146,9 +1182,9 @@ fn has_ref_mut_self_method(cx: &LateContext<'_>, trait_def_id: DefId) -> bool {
11461182
})
11471183
}
11481184

1149-
fn referent_dropped_without_further_use<'a, 'tcx>(
1185+
fn referent_used_exactly_once<'a, 'tcx>(
11501186
cx: &'a LateContext<'tcx>,
1151-
possible_borrower: &mut Option<PossibleBorrowerMap<'a, 'tcx>>,
1187+
possible_borrower: &mut FxIndexMap<LocalDefId, PossibleBorrowerMap<'tcx, 'tcx>>,
11521188
reference: &Expr<'tcx>,
11531189
) -> bool {
11541190
let mir = enclosing_mir(cx.tcx, reference.hir_id);
@@ -1158,12 +1194,15 @@ fn referent_dropped_without_further_use<'a, 'tcx>(
11581194
mir.basic_blocks[location.block].statements[location.statement_index].kind
11591195
&& !place.has_deref()
11601196
{
1161-
let possible_borrower = possible_borrower.get_or_insert_with(|| PossibleBorrowerMap::new(cx, mir));
1197+
let body_owner_local_def_id = cx.tcx.hir().enclosing_body_owner(reference.hir_id);
1198+
let possible_borrower = possible_borrower
1199+
.entry(body_owner_local_def_id)
1200+
.or_insert_with(|| PossibleBorrowerMap::new(cx, mir));
11621201
// If `only_borrowers` were used here, the `copyable_iterator::warn` test would fail. The reason is
11631202
// that `PossibleBorrowerVisitor::visit_terminator` considers `place.local` a possible borrower of
11641203
// itself. See the comment in that method for an explanation as to why.
11651204
possible_borrower.bounded_borrowers(&[local], &[local, place.local], place.local, location)
1166-
&& dropped_without_further_use(mir, place.local, location).unwrap_or(false)
1205+
&& used_exactly_once(mir, place.local).unwrap_or(false)
11671206
} else {
11681207
false
11691208
}
@@ -1453,8 +1492,8 @@ fn report<'tcx>(cx: &LateContext<'tcx>, expr: &'tcx Expr<'_>, state: State, data
14531492
}
14541493
}
14551494

1456-
impl Dereferencing {
1457-
fn check_local_usage<'tcx>(&mut self, cx: &LateContext<'tcx>, e: &Expr<'tcx>, local: HirId) {
1495+
impl<'tcx> Dereferencing<'tcx> {
1496+
fn check_local_usage(&mut self, cx: &LateContext<'tcx>, e: &Expr<'tcx>, local: HirId) {
14581497
if let Some(outer_pat) = self.ref_locals.get_mut(&local) {
14591498
if let Some(pat) = outer_pat {
14601499
// Check for auto-deref

clippy_lints/src/lib.rs

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -37,6 +37,7 @@ extern crate rustc_index;
3737
extern crate rustc_infer;
3838
extern crate rustc_lexer;
3939
extern crate rustc_lint;
40+
extern crate rustc_lint_defs;
4041
extern crate rustc_middle;
4142
extern crate rustc_parse;
4243
extern crate rustc_parse_format;

clippy_lints/src/redundant_clone.rs

Lines changed: 14 additions & 16 deletions
Original file line numberDiff line numberDiff line change
@@ -355,34 +355,32 @@ struct CloneUsage {
355355
}
356356

357357
fn visit_clone_usage(cloned: mir::Local, clone: mir::Local, mir: &mir::Body<'_>, bb: mir::BasicBlock) -> CloneUsage {
358-
if let Some(
359-
&[
360-
LocalUsage {
361-
local_use_loc: cloned_use_loc,
362-
local_consume_or_mutate_loc: cloned_consume_or_mutate_loc,
363-
},
364-
LocalUsage {
365-
local_use_loc: _,
366-
local_consume_or_mutate_loc: clone_consume_or_mutate_loc,
367-
},
368-
],
369-
) = visit_local_usage(
358+
if let Some((
359+
LocalUsage {
360+
local_use_locs: cloned_use_locs,
361+
local_consume_or_mutate_locs: cloned_consume_or_mutate_locs,
362+
},
363+
LocalUsage {
364+
local_use_locs: _,
365+
local_consume_or_mutate_locs: clone_consume_or_mutate_locs,
366+
},
367+
)) = visit_local_usage(
370368
&[cloned, clone],
371369
mir,
372370
mir::Location {
373371
block: bb,
374372
statement_index: mir.basic_blocks[bb].statements.len(),
375373
},
376374
)
377-
.as_deref()
375+
.map(|mut vec| (vec.remove(0), vec.remove(0)))
378376
{
379377
CloneUsage {
380-
cloned_used: cloned_use_loc.is_some(),
381-
cloned_consume_or_mutate_loc,
378+
cloned_used: !cloned_use_locs.is_empty(),
379+
cloned_consume_or_mutate_loc: cloned_consume_or_mutate_locs.first().copied(),
382380
// Consider non-temporary clones consumed.
383381
// TODO: Actually check for mutation of non-temporaries.
384382
clone_consumed_or_mutated: mir.local_kind(clone) != mir::LocalKind::Temp
385-
|| clone_consume_or_mutate_loc.is_some(),
383+
|| !clone_consume_or_mutate_locs.is_empty(),
386384
}
387385
} else {
388386
CloneUsage {

0 commit comments

Comments
 (0)