Skip to content

Commit

Permalink
Auto merge of #11900 - Enselic:needless-borrow-drop, r=Manishearth
Browse files Browse the repository at this point in the history
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
  • Loading branch information
bors committed Dec 2, 2023
2 parents da27c97 + 512f302 commit 6289c30
Show file tree
Hide file tree
Showing 3 changed files with 40 additions and 2 deletions.
12 changes: 10 additions & 2 deletions clippy_lints/src/needless_borrows_for_generic_args.rs
Original file line number Diff line number Diff line change
Expand Up @@ -2,7 +2,7 @@ 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::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 rustc_errors::Applicability;
use rustc_hir::def::{DefKind, Res};
Expand Down Expand Up @@ -169,6 +169,7 @@ fn needless_borrow_count<'tcx>(
) -> usize {
let destruct_trait_def_id = cx.tcx.lang_items().destruct_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 predicates = cx.tcx.param_env(fn_id).caller_bounds();
Expand Down Expand Up @@ -223,7 +224,14 @@ fn needless_borrow_count<'tcx>(
// elements are modified each time `check_referent` is called.
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);

if !is_copy(cx, referent_ty)
Expand Down
15 changes: 15 additions & 0 deletions tests/ui/needless_borrows_for_generic_args.fixed
Original file line number Diff line number Diff line change
Expand Up @@ -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
}
}
}
15 changes: 15 additions & 0 deletions tests/ui/needless_borrows_for_generic_args.rs
Original file line number Diff line number Diff line change
Expand Up @@ -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
}
}
}

0 comments on commit 6289c30

Please sign in to comment.