Address #9386 (review)

Author: smoelius

clippy_lints/src/dereference.rs

@@ -1,7 +1,5 @@
 use clippy_utils::diagnostics::{span_lint_and_sugg, span_lint_hir_and_then};
-use clippy_utils::mir::{
-    dropped_without_further_use, enclosing_mir, expr_local, local_assignments, PossibleBorrowerMap,
-};
+use clippy_utils::mir::{enclosing_mir, expr_local, local_assignments, used_exactly_once, PossibleBorrowerMap};
 use clippy_utils::source::{snippet_with_applicability, snippet_with_context};
 use clippy_utils::sugg::has_enclosing_paren;
 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;
 use rustc_errors::Applicability;
 use rustc_hir::intravisit::{walk_ty, Visitor};
 use rustc_hir::{
-    self as hir, def_id::DefId, BindingAnnotation, Body, BodyId, BorrowKind, Closure, Expr, ExprKind, FnRetTy,
-    GenericArg, HirId, ImplItem, ImplItemKind, Item, ItemKind, Local, MatchSource, Mutability, Node, Pat, PatKind,
-    Path, QPath, TraitItem, TraitItemKind, TyKind, UnOp,
+    self as hir,
+    def_id::{DefId, LocalDefId},
+    BindingAnnotation, Body, BodyId, BorrowKind, Closure, Expr, ExprKind, FnRetTy, GenericArg, HirId, ImplItem,
+    ImplItemKind, Item, ItemKind, Local, MatchSource, Mutability, Node, Pat, PatKind, Path, QPath, TraitItem,
+    TraitItemKind, TyKind, UnOp,
 };
 use rustc_index::bit_set::BitSet;
 use rustc_infer::infer::TyCtxtInferExt;
-use rustc_lint::{LateContext, LateLintPass};
+use rustc_lint::{LateContext, LateLintPass, LintArray, LintPass};
+use rustc_lint_defs::lint_array;
 use rustc_middle::mir::{Rvalue, StatementKind};
 use rustc_middle::ty::adjustment::{Adjust, Adjustment, AutoBorrow, AutoBorrowMutability};
 use rustc_middle::ty::{
     self, Binder, BoundVariableKind, EarlyBinder, FnSig, GenericArgKind, List, ParamTy, PredicateKind,
     ProjectionPredicate, Ty, TyCtxt, TypeVisitable, TypeckResults,
 };
 use rustc_semver::RustcVersion;
-use rustc_session::{declare_tool_lint, impl_lint_pass};
+use rustc_session::declare_tool_lint;
 use rustc_span::{symbol::sym, Span, Symbol, DUMMY_SP};
 use rustc_trait_selection::infer::InferCtxtExt as _;
 use rustc_trait_selection::traits::{query::evaluate_obligation::InferCtxtExt as _, Obligation, ObligationCause};
@@ -145,15 +146,27 @@ declare_clippy_lint! {
     "dereferencing when the compiler would automatically dereference"
 }
 
-impl_lint_pass!(Dereferencing => [
-    EXPLICIT_DEREF_METHODS,
-    NEEDLESS_BORROW,
-    REF_BINDING_TO_REFERENCE,
-    EXPLICIT_AUTO_DEREF,
-]);
+#[expect(rustc::internal)]
+impl<'tcx> LintPass for Dereferencing<'tcx> {
+    fn name(&self) -> &'static str {
+        stringify!($ty)
+    }
+}
+
+impl<'tcx> Dereferencing<'tcx> {
+    #[expect(dead_code)]
+    pub fn get_lints() -> LintArray {
+        lint_array!(
+            EXPLICIT_DEREF_METHODS,
+            NEEDLESS_BORROW,
+            REF_BINDING_TO_REFERENCE,
+            EXPLICIT_AUTO_DEREF,
+        )
+    }
+}
 
 #[derive(Default)]
-pub struct Dereferencing {
+pub struct Dereferencing<'tcx> {
     state: Option<(State, StateData)>,
 
     // While parsing a `deref` method call in ufcs form, the path to the function is itself an
@@ -174,11 +187,15 @@ pub struct Dereferencing {
     /// e.g. `m!(x) | Foo::Bar(ref x)`
     ref_locals: FxIndexMap<HirId, Option<RefPat>>,
 
+    /// Map from body owners to `PossibleBorrowerMap`s. Used by `needless_borrow_impl_arg_position`
+    /// to determine when a borrowed expression can instead be moved.
+    possible_borrowers: FxIndexMap<LocalDefId, PossibleBorrowerMap<'tcx, 'tcx>>,
+
     // `IntoIterator` for arrays requires Rust 1.53.
     msrv: Option<RustcVersion>,
 }
 
-impl Dereferencing {
+impl<'tcx> Dereferencing<'tcx> {
     #[must_use]
     pub fn new(msrv: Option<RustcVersion>) -> Self {
         Self {
@@ -248,7 +265,7 @@ struct RefPat {
     hir_id: HirId,
 }
 
-impl<'tcx> LateLintPass<'tcx> for Dereferencing {
+impl<'tcx> LateLintPass<'tcx> for Dereferencing<'tcx> {
     #[expect(clippy::too_many_lines)]
     fn check_expr(&mut self, cx: &LateContext<'tcx>, expr: &'tcx Expr<'_>) {
         // Skip path expressions from deref calls. e.g. `Deref::deref(e)`
@@ -282,7 +299,7 @@ impl<'tcx> LateLintPass<'tcx> for Dereferencing {
         match (self.state.take(), kind) {
             (None, kind) => {
                 let expr_ty = typeck.expr_ty(expr);
-                let (position, adjustments) = walk_parents(cx, expr, self.msrv);
+                let (position, adjustments) = walk_parents(cx, &mut self.possible_borrowers, expr, self.msrv);
                 match kind {
                     RefOp::Deref => {
                         if let Position::FieldAccess {
@@ -686,6 +703,7 @@ impl Position {
 #[expect(clippy::too_many_lines)]
 fn walk_parents<'tcx>(
     cx: &LateContext<'tcx>,
+    possible_borrowers: &mut FxIndexMap<LocalDefId, PossibleBorrowerMap<'tcx, 'tcx>>,
     e: &'tcx Expr<'_>,
     msrv: Option<RustcVersion>,
 ) -> (Position, &'tcx [Adjustment<'tcx>]) {
@@ -800,7 +818,16 @@ fn walk_parents<'tcx>(
                             Some(hir_ty) => binding_ty_auto_deref_stability(cx, hir_ty, precedence, ty.bound_vars()),
                             None => {
                                 if let ty::Param(param_ty) = ty.skip_binder().kind() {
-                                    needless_borrow_impl_arg_position(cx, parent, i, *param_ty, e, precedence, msrv)
+                                    needless_borrow_impl_arg_position(
+                                        cx,
+                                        possible_borrowers,
+                                        parent,
+                                        i,
+                                        *param_ty,
+                                        e,
+                                        precedence,
+                                        msrv,
+                                    )
                                 } else {
                                     ty_auto_deref_stability(cx, cx.tcx.erase_late_bound_regions(ty), precedence)
                                         .position_for_arg()
@@ -848,7 +875,16 @@ fn walk_parents<'tcx>(
                     args.iter().position(|arg| arg.hir_id == child_id).map(|i| {
                         let ty = cx.tcx.fn_sig(id).skip_binder().inputs()[i + 1];
                         if let ty::Param(param_ty) = ty.kind() {
-                            needless_borrow_impl_arg_position(cx, parent, i + 1, *param_ty, e, precedence, msrv)
+                            needless_borrow_impl_arg_position(
+                                cx,
+                                possible_borrowers,
+                                parent,
+                                i + 1,
+                                *param_ty,
+                                e,
+                                precedence,
+                                msrv,
+                            )
                         } else {
                             ty_auto_deref_stability(
                                 cx,
@@ -1022,8 +1058,10 @@ fn ty_contains_infer(ty: &hir::Ty<'_>) -> bool {
 // If the conditions are met, returns `Some(Position::ImplArg(..))`; otherwise, returns `None`.
 //   The "is copyable" condition is to avoid the case where removing the `&` means `e` would have to
 // be moved, but it cannot be.
+#[expect(clippy::too_many_arguments)]
 fn needless_borrow_impl_arg_position<'tcx>(
     cx: &LateContext<'tcx>,
+    possible_borrowers: &mut FxIndexMap<LocalDefId, PossibleBorrowerMap<'tcx, 'tcx>>,
     parent: &Expr<'tcx>,
     arg_index: usize,
     param_ty: ParamTy,
@@ -1082,8 +1120,6 @@ fn needless_borrow_impl_arg_position<'tcx>(
         return Position::Other(precedence);
     }
 
-    let mut possible_borrower = None;
-
     // `substs_with_referent_ty` can be constructed outside of `check_referent` because the same
     // elements are modified each time `check_referent` is called.
     let mut substs_with_referent_ty = substs_with_expr_ty.to_vec();
@@ -1093,7 +1129,7 @@ fn needless_borrow_impl_arg_position<'tcx>(
 
         if !is_copy(cx, referent_ty)
             && (referent_ty.has_significant_drop(cx.tcx, cx.param_env)
-                || !referent_dropped_without_further_use(cx, &mut possible_borrower, reference))
+                || !referent_used_exactly_once(cx, possible_borrowers, reference))
         {
             return false;
         }
@@ -1164,9 +1200,9 @@ fn has_ref_mut_self_method(cx: &LateContext<'_>, trait_def_id: DefId) -> bool {
         })
 }
 
-fn referent_dropped_without_further_use<'a, 'tcx>(
+fn referent_used_exactly_once<'a, 'tcx>(
     cx: &'a LateContext<'tcx>,
-    possible_borrower: &mut Option<PossibleBorrowerMap<'a, 'tcx>>,
+    possible_borrower: &mut FxIndexMap<LocalDefId, PossibleBorrowerMap<'tcx, 'tcx>>,
     reference: &Expr<'tcx>,
 ) -> bool {
     let mir = enclosing_mir(cx.tcx, reference.hir_id);
@@ -1176,12 +1212,15 @@ fn referent_dropped_without_further_use<'a, 'tcx>(
             mir.basic_blocks[location.block].statements[location.statement_index].kind
         && !place.has_deref()
     {
-        let possible_borrower = possible_borrower.get_or_insert_with(|| PossibleBorrowerMap::new(cx, mir));
+        let body_owner_local_def_id = cx.tcx.hir().enclosing_body_owner(reference.hir_id);
+        let possible_borrower = possible_borrower
+            .entry(body_owner_local_def_id)
+            .or_insert_with(|| PossibleBorrowerMap::new(cx, mir));
         // If `only_borrowers` were used here, the `copyable_iterator::warn` test would fail. The reason is
         // that `PossibleBorrowerVisitor::visit_terminator` considers `place.local` a possible borrower of
         // itself. See the comment in that method for an explanation as to why.
         possible_borrower.bounded_borrowers(&[local], &[local, place.local], place.local, location)
-            && dropped_without_further_use(mir, place.local, location).unwrap_or(false)
+            && used_exactly_once(mir, place.local).unwrap_or(false)
     } else {
         false
     }
@@ -1471,8 +1510,8 @@ fn report<'tcx>(cx: &LateContext<'tcx>, expr: &'tcx Expr<'_>, state: State, data
     }
 }
 
-impl Dereferencing {
-    fn check_local_usage<'tcx>(&mut self, cx: &LateContext<'tcx>, e: &Expr<'tcx>, local: HirId) {
+impl<'tcx> Dereferencing<'tcx> {
+    fn check_local_usage(&mut self, cx: &LateContext<'tcx>, e: &Expr<'tcx>, local: HirId) {
         if let Some(outer_pat) = self.ref_locals.get_mut(&local) {
             if let Some(pat) = outer_pat {
                 // Check for auto-deref

clippy_lints/src/lib.rs

@@ -37,6 +37,7 @@ extern crate rustc_index;
 extern crate rustc_infer;
 extern crate rustc_lexer;
 extern crate rustc_lint;
+extern crate rustc_lint_defs;
 extern crate rustc_middle;
 extern crate rustc_parse;
 extern crate rustc_session;

clippy_lints/src/redundant_clone.rs

@@ -355,34 +355,32 @@ struct CloneUsage {
 }
 
 fn visit_clone_usage(cloned: mir::Local, clone: mir::Local, mir: &mir::Body<'_>, bb: mir::BasicBlock) -> CloneUsage {
-    if let Some(
-        &[
-            LocalUsage {
-                local_use_loc: cloned_use_loc,
-                local_consume_or_mutate_loc: cloned_consume_or_mutate_loc,
-            },
-            LocalUsage {
-                local_use_loc: _,
-                local_consume_or_mutate_loc: clone_consume_or_mutate_loc,
-            },
-        ],
-    ) = visit_local_usage(
+    if let Some((
+        LocalUsage {
+            local_use_locs: cloned_use_locs,
+            local_consume_or_mutate_locs: cloned_consume_or_mutate_locs,
+        },
+        LocalUsage {
+            local_use_locs: _,
+            local_consume_or_mutate_locs: clone_consume_or_mutate_locs,
+        },
+    )) = visit_local_usage(
         &[cloned, clone],
         mir,
         mir::Location {
             block: bb,
             statement_index: mir.basic_blocks[bb].statements.len(),
         },
     )
-    .as_deref()
+    .map(|mut vec| (vec.remove(0), vec.remove(0)))
     {
         CloneUsage {
-            cloned_used: cloned_use_loc.is_some(),
-            cloned_consume_or_mutate_loc,
+            cloned_used: !cloned_use_locs.is_empty(),
+            cloned_consume_or_mutate_loc: cloned_consume_or_mutate_locs.first().copied(),
             // Consider non-temporary clones consumed.
             // TODO: Actually check for mutation of non-temporaries.
             clone_consumed_or_mutated: mir.local_kind(clone) != mir::LocalKind::Temp
-                || clone_consume_or_mutate_loc.is_some(),
+                || !clone_consume_or_mutate_locs.is_empty(),
         }
     } else {
         CloneUsage {