less aggressive needless_borrows_for_generic_args

clippy_lints/src/needless_borrows_for_generic_args.rs

@@ -1,6 +1,6 @@
 use clippy_config::msrvs::{self, Msrv};
 use clippy_utils::diagnostics::span_lint_and_then;
-use clippy_utils::mir::{enclosing_mir, expr_local, local_assignments, used_exactly_once, PossibleBorrowerMap};
+use clippy_utils::mir::PossibleBorrowerMap;
 use clippy_utils::source::snippet_with_context;
 use clippy_utils::ty::{implements_trait, is_copy};
 use clippy_utils::{expr_use_ctxt, peel_n_hir_expr_refs, DefinedTy, ExprUseNode};
@@ -11,7 +11,6 @@ use rustc_hir::{Body, Expr, ExprKind, Mutability, Path, QPath};
 use rustc_index::bit_set::BitSet;
 use rustc_infer::infer::TyCtxtInferExt;
 use rustc_lint::{LateContext, LateLintPass};
-use rustc_middle::mir::{Rvalue, StatementKind};
 use rustc_middle::ty::{
     self, ClauseKind, EarlyBinder, FnSig, GenericArg, GenericArgKind, List, ParamTy, ProjectionPredicate, Ty,
 };
@@ -106,7 +105,6 @@ impl<'tcx> LateLintPass<'tcx> for NeedlessBorrowsForGenericArgs<'tcx> {
             }
             && let count = needless_borrow_count(
                 cx,
-                &mut self.possible_borrowers,
                 fn_id,
                 cx.typeck_results().node_args(hir_id),
                 i,
@@ -155,11 +153,9 @@ fn path_has_args(p: &QPath<'_>) -> bool {
 /// The following constraints will be checked:
 /// * The borrowed expression meets all the generic type's constraints.
 /// * The generic type appears only once in the functions signature.
-/// * The borrowed value will not be moved if it is used later in the function.
-#[expect(clippy::too_many_arguments)]
+/// * The borrowed value is Copy itself OR not a variable (created by a function call)
 fn needless_borrow_count<'tcx>(
     cx: &LateContext<'tcx>,
-    possible_borrowers: &mut Vec<(LocalDefId, PossibleBorrowerMap<'tcx, 'tcx>)>,
     fn_id: DefId,
     callee_args: &'tcx List<GenericArg<'tcx>>,
     arg_index: usize,
@@ -234,9 +230,9 @@ fn needless_borrow_count<'tcx>(
 
         let referent_ty = cx.typeck_results().expr_ty(referent);
 
-        if !is_copy(cx, referent_ty)
-            && (referent_ty.has_significant_drop(cx.tcx, cx.param_env)
-                || !referent_used_exactly_once(cx, possible_borrowers, reference))
+        if (!is_copy(cx, referent_ty) && !referent_ty.is_ref())
+            && let ExprKind::AddrOf(_, _, inner) = reference.kind
+            && !matches!(inner.kind, ExprKind::Call(..) | ExprKind::MethodCall(..))
         {
             return false;
         }
@@ -339,37 +335,6 @@ fn is_mixed_projection_predicate<'tcx>(
     }
 }
 
-fn referent_used_exactly_once<'tcx>(
-    cx: &LateContext<'tcx>,
-    possible_borrowers: &mut Vec<(LocalDefId, PossibleBorrowerMap<'tcx, 'tcx>)>,
-    reference: &Expr<'tcx>,
-) -> bool {
-    if let Some(mir) = enclosing_mir(cx.tcx, reference.hir_id)
-        && let Some(local) = expr_local(cx.tcx, reference)
-        && let [location] = *local_assignments(mir, local).as_slice()
-        && let block_data = &mir.basic_blocks[location.block]
-        && let Some(statement) = block_data.statements.get(location.statement_index)
-        && let StatementKind::Assign(box (_, Rvalue::Ref(_, _, place))) = statement.kind
-        && !place.is_indirect_first_projection()
-    {
-        let body_owner_local_def_id = cx.tcx.hir().enclosing_body_owner(reference.hir_id);
-        if possible_borrowers
-            .last()
-            .map_or(true, |&(local_def_id, _)| local_def_id != body_owner_local_def_id)
-        {
-            possible_borrowers.push((body_owner_local_def_id, PossibleBorrowerMap::new(cx, mir)));
-        }
-        let possible_borrower = &mut possible_borrowers.last_mut().unwrap().1;
-        // 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)
-            && used_exactly_once(mir, place.local).unwrap_or(false)
-    } else {
-        false
-    }
-}
-
 // Iteratively replaces `param_ty` with `new_ty` in `args`, and similarly for each resulting
 // projected type that is a type parameter. Returns `false` if replacing the types would have an
 // effect on the function signature beyond substituting `new_ty` for `param_ty`.

tests/ui/needless_borrows_for_generic_args.fixed

@@ -141,8 +141,8 @@ fn main() {
         let f = |arg| {
             let loc = "loc".to_owned();
             let _ = std::fs::write("x", &env); // Don't lint. In environment
-            let _ = std::fs::write("x", arg);
-            let _ = std::fs::write("x", loc);
+            let _ = std::fs::write("x", &arg);
+            let _ = std::fs::write("x", &loc);
         };
         let _ = std::fs::write("x", &env); // Don't lint. Borrowed by `f`
         f(arg);
@@ -158,13 +158,13 @@ fn main() {
         fn f(_: impl Debug) {}
 
         let x = X;
-        f(&x); // Don't lint. Has significant drop
+        f(&x); // Don't lint, not copy, passed by a reference to a variable
     }
     {
         fn f(_: impl AsRef<str>) {}
 
         let x = String::new();
-        f(x);
+        f(&x);
     }
     {
         fn f(_: impl AsRef<str>) {}
@@ -299,4 +299,38 @@ fn main() {
             check_str(&owner.0); // Don't lint. `owner` can't be partially moved because it impl Drop
         }
     }
+    {
+        #[derive(Debug)]
+        struct X(Vec<u8>);
+
+        fn f(_: impl Debug) {}
+
+        let x = X(vec![]);
+        f(&x); // Don't lint, makes x unavailable later
+    }
+    {
+        #[derive(Debug)]
+        struct X;
+
+        impl Drop for X {
+            fn drop(&mut self) {}
+        }
+
+        fn f(_: impl Debug) {}
+
+        #[derive(Debug)]
+        struct Y(X);
+
+        let y = Y(X);
+        f(&y); // Don't lint. Not copy, passed by a reference to value
+    }
+    {
+        fn f(_: impl AsRef<str>) {}
+        let x = String::new();
+        f(&x); // Don't lint, not a copy, makes it unavailable later
+        f(String::new()); // Lint, makes no difference
+        let y = "".to_owned();
+        f(&y); // Don't lint
+        f("".to_owned()); // Lint
+    }
 }

tests/ui/needless_borrows_for_generic_args.rs

@@ -158,7 +158,7 @@ fn main() {
         fn f(_: impl Debug) {}
 
         let x = X;
-        f(&x); // Don't lint. Has significant drop
+        f(&x); // Don't lint, not copy, passed by a reference to a variable
     }
     {
         fn f(_: impl AsRef<str>) {}
@@ -299,4 +299,38 @@ fn main() {
             check_str(&owner.0); // Don't lint. `owner` can't be partially moved because it impl Drop
         }
     }
+    {
+        #[derive(Debug)]
+        struct X(Vec<u8>);
+
+        fn f(_: impl Debug) {}
+
+        let x = X(vec![]);
+        f(&x); // Don't lint, makes x unavailable later
+    }
+    {
+        #[derive(Debug)]
+        struct X;
+
+        impl Drop for X {
+            fn drop(&mut self) {}
+        }
+
+        fn f(_: impl Debug) {}
+
+        #[derive(Debug)]
+        struct Y(X);
+
+        let y = Y(X);
+        f(&y); // Don't lint. Not copy, passed by a reference to value
+    }
+    {
+        fn f(_: impl AsRef<str>) {}
+        let x = String::new();
+        f(&x); // Don't lint, not a copy, makes it unavailable later
+        f(&String::new()); // Lint, makes no difference
+        let y = "".to_owned();
+        f(&y); // Don't lint
+        f(&"".to_owned()); // Lint
+    }
 }

tests/ui/needless_borrows_for_generic_args.stderr

@@ -50,28 +50,22 @@ LL |         let _ = Command::new("ls").args(&["-a", "-l"]).status().unwrap();
    |                                         ^^^^^^^^^^^^^ help: change this to: `["-a", "-l"]`
 
 error: the borrowed expression implements the required traits
-  --> tests/ui/needless_borrows_for_generic_args.rs:144:41
-   |
-LL |             let _ = std::fs::write("x", &arg);
-   |                                         ^^^^ help: change this to: `arg`
-
-error: the borrowed expression implements the required traits
-  --> tests/ui/needless_borrows_for_generic_args.rs:145:41
+  --> tests/ui/needless_borrows_for_generic_args.rs:247:13
    |
-LL |             let _ = std::fs::write("x", &loc);
-   |                                         ^^^^ help: change this to: `loc`
+LL |         foo(&a);
+   |             ^^ help: change this to: `a`
 
 error: the borrowed expression implements the required traits
-  --> tests/ui/needless_borrows_for_generic_args.rs:167:11
+  --> tests/ui/needless_borrows_for_generic_args.rs:331:11
    |
-LL |         f(&x);
-   |           ^^ help: change this to: `x`
+LL |         f(&String::new()); // Lint, makes no difference
+   |           ^^^^^^^^^^^^^^ help: change this to: `String::new()`
 
 error: the borrowed expression implements the required traits
-  --> tests/ui/needless_borrows_for_generic_args.rs:247:13
+  --> tests/ui/needless_borrows_for_generic_args.rs:334:11
    |
-LL |         foo(&a);
-   |             ^^ help: change this to: `a`
+LL |         f(&"".to_owned()); // Lint
+   |           ^^^^^^^^^^^^^^ help: change this to: `"".to_owned()`
 
-error: aborting due to 12 previous errors
+error: aborting due to 11 previous errors