Auto merge of #11900 - Enselic:needless-borrow-drop, r=Manishearth

needless_borrows_for_generic_args: Handle when field operand impl Drop

Before this fix, the lint had a false positive, namely when a reference was taken to a field when the field operand implements a custom Drop. The compiler will refuse to partially move a type that implements Drop, because that would put the type in a weird state.

## False Positive Example (Fixed)

```rs
struct CustomDrop(String);

impl Drop for CustomDrop {
    fn drop(&mut self) {}
}

fn check_str<P: AsRef<str>>(_to: P) {}

fn test() {
    let owner = CustomDrop(String::default());
    check_str(&owner.0); // Don't lint. `owner` can't be partially moved because it impl Drop
}
```

changelog: [`needless_borrows_for_generic_args`]: Handle when field operand impl Drop
This commit is contained in:
bors 2023-12-05 19:10:09 +00:00
commit 4a56563154
3 changed files with 40 additions and 2 deletions

View File

@ -2,7 +2,7 @@
use clippy_utils::diagnostics::span_lint_and_then; 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::{enclosing_mir, expr_local, local_assignments, used_exactly_once, PossibleBorrowerMap};
use clippy_utils::source::snippet_with_context; use clippy_utils::source::snippet_with_context;
use clippy_utils::ty::is_copy; use clippy_utils::ty::{implements_trait, is_copy};
use clippy_utils::{expr_use_ctxt, peel_n_hir_expr_refs, DefinedTy, ExprUseNode}; use clippy_utils::{expr_use_ctxt, peel_n_hir_expr_refs, DefinedTy, ExprUseNode};
use rustc_errors::Applicability; use rustc_errors::Applicability;
use rustc_hir::def::{DefKind, Res}; use rustc_hir::def::{DefKind, Res};
@ -169,6 +169,7 @@ fn needless_borrow_count<'tcx>(
) -> usize { ) -> usize {
let destruct_trait_def_id = cx.tcx.lang_items().destruct_trait(); let destruct_trait_def_id = cx.tcx.lang_items().destruct_trait();
let sized_trait_def_id = cx.tcx.lang_items().sized_trait(); let sized_trait_def_id = cx.tcx.lang_items().sized_trait();
let drop_trait_def_id = cx.tcx.lang_items().drop_trait();
let fn_sig = cx.tcx.fn_sig(fn_id).instantiate_identity().skip_binder(); let fn_sig = cx.tcx.fn_sig(fn_id).instantiate_identity().skip_binder();
let predicates = cx.tcx.param_env(fn_id).caller_bounds(); let predicates = cx.tcx.param_env(fn_id).caller_bounds();
@ -223,7 +224,14 @@ fn needless_borrow_count<'tcx>(
// elements are modified each time `check_referent` is called. // elements are modified each time `check_referent` is called.
let mut args_with_referent_ty = callee_args.to_vec(); let mut args_with_referent_ty = callee_args.to_vec();
let mut check_reference_and_referent = |reference, referent| { let mut check_reference_and_referent = |reference: &Expr<'tcx>, referent: &Expr<'tcx>| {
if let ExprKind::Field(base, _) = &referent.kind {
let base_ty = cx.typeck_results().expr_ty(base);
if drop_trait_def_id.map_or(false, |id| implements_trait(cx, base_ty, id, &[])) {
return false;
}
}
let referent_ty = cx.typeck_results().expr_ty(referent); let referent_ty = cx.typeck_results().expr_ty(referent);
if !is_copy(cx, referent_ty) if !is_copy(cx, referent_ty)

View File

@ -284,4 +284,19 @@ fn main() {
{ {
} }
} }
// address of field when operand impl Drop
{
struct CustomDrop(String);
impl Drop for CustomDrop {
fn drop(&mut self) {}
}
fn check_str<P: AsRef<str>>(_to: P) {}
fn test() {
let owner = CustomDrop(String::default());
check_str(&owner.0); // Don't lint. `owner` can't be partially moved because it impl Drop
}
}
} }

View File

@ -284,4 +284,19 @@ pub const fn compute<T>(_: T)
{ {
} }
} }
// address of field when operand impl Drop
{
struct CustomDrop(String);
impl Drop for CustomDrop {
fn drop(&mut self) {}
}
fn check_str<P: AsRef<str>>(_to: P) {}
fn test() {
let owner = CustomDrop(String::default());
check_str(&owner.0); // Don't lint. `owner` can't be partially moved because it impl Drop
}
}
} }