diff --git a/clippy_lints/src/needless_borrows_for_generic_args.rs b/clippy_lints/src/needless_borrows_for_generic_args.rs index daf166bad90d8..71073716cf8ba 100644 --- a/clippy_lints/src/needless_borrows_for_generic_args.rs +++ b/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::PossibleBorrowerMap; +use clippy_utils::mir::{enclosing_mir, expr_local, local_assignments, used_exactly_once, 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,6 +11,7 @@ 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, ParamTy, ProjectionPredicate, Ty, }; @@ -105,6 +106,7 @@ 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, @@ -153,9 +155,14 @@ 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 is Copy itself OR not a variable (created by a function call) +/// * The borrowed value is: +/// - `Copy` itself, or +/// - the only use of a mutable reference, or +/// - not a variable (created by a function call) +#[expect(clippy::too_many_arguments)] fn needless_borrow_count<'tcx>( cx: &LateContext<'tcx>, + possible_borrowers: &mut Vec<(LocalDefId, PossibleBorrowerMap<'tcx, 'tcx>)>, fn_id: DefId, callee_args: ty::GenericArgsRef<'tcx>, arg_index: usize, @@ -230,9 +237,9 @@ fn needless_borrow_count<'tcx>( let referent_ty = cx.typeck_results().expr_ty(referent); - if (!is_copy(cx, referent_ty) && !referent_ty.is_ref()) - && let ExprKind::AddrOf(_, _, inner) = reference.kind - && !matches!(inner.kind, ExprKind::Call(..) | ExprKind::MethodCall(..)) + if !(is_copy(cx, referent_ty) + || referent_ty.is_ref() && referent_used_exactly_once(cx, possible_borrowers, reference) + || matches!(referent.kind, ExprKind::Call(..) | ExprKind::MethodCall(..))) { return false; } @@ -335,6 +342,37 @@ 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`. diff --git a/tests/ui/needless_borrows_for_generic_args.fixed b/tests/ui/needless_borrows_for_generic_args.fixed index 5478372cbe00f..593649b60bc71 100644 --- a/tests/ui/needless_borrows_for_generic_args.fixed +++ b/tests/ui/needless_borrows_for_generic_args.fixed @@ -333,4 +333,12 @@ fn main() { f(&y); // Don't lint f("".to_owned()); // Lint } + { + fn takes_writer(_: T) {} + + fn f(mut buffer: &mut Vec) { + takes_writer(&mut buffer); // Don't lint, would make buffer unavailable later + buffer.extend(b"\n"); + } + } } diff --git a/tests/ui/needless_borrows_for_generic_args.rs b/tests/ui/needless_borrows_for_generic_args.rs index 2643815d939b5..0c9b98d739d38 100644 --- a/tests/ui/needless_borrows_for_generic_args.rs +++ b/tests/ui/needless_borrows_for_generic_args.rs @@ -333,4 +333,12 @@ fn main() { f(&y); // Don't lint f(&"".to_owned()); // Lint } + { + fn takes_writer(_: T) {} + + fn f(mut buffer: &mut Vec) { + takes_writer(&mut buffer); // Don't lint, would make buffer unavailable later + buffer.extend(b"\n"); + } + } }