[assigning_clones]: bail out when the source borrows from target

This commit is contained in:
y21 2024-05-02 23:38:31 +02:00
parent dff9164ac5
commit 20d1bb1867
3 changed files with 122 additions and 1 deletions

View File

@ -1,16 +1,18 @@
use clippy_config::msrvs::{self, Msrv};
use clippy_utils::diagnostics::span_lint_and_then;
use clippy_utils::macros::HirNode;
use clippy_utils::mir::{enclosing_mir, PossibleBorrowerMap};
use clippy_utils::sugg::Sugg;
use clippy_utils::{is_trait_method, local_is_initialized, path_to_local};
use rustc_errors::Applicability;
use rustc_hir::{self as hir, Expr, ExprKind};
use rustc_lint::{LateContext, LateLintPass};
use rustc_middle::mir;
use rustc_middle::ty::{self, Instance, Mutability};
use rustc_session::impl_lint_pass;
use rustc_span::def_id::DefId;
use rustc_span::symbol::sym;
use rustc_span::{ExpnKind, SyntaxContext};
use rustc_span::{ExpnKind, Span, SyntaxContext};
declare_clippy_lint! {
/// ### What it does
@ -144,6 +146,7 @@ fn extract_call<'tcx>(cx: &LateContext<'tcx>, expr: &'tcx Expr<'tcx>) -> Option<
};
Some(CallCandidate {
span: expr.span,
target,
kind,
method_def_id: resolved_method.def_id(),
@ -215,6 +218,10 @@ fn is_ok_to_suggest<'tcx>(cx: &LateContext<'tcx>, lhs: &Expr<'tcx>, call: &CallC
return false;
};
if clone_source_borrows_from_dest(cx, lhs, call.span) {
return false;
}
// Now take a look if the impl block defines an implementation for the method that we're interested
// in. If not, then we're using a default implementation, which is not interesting, so we will
// not suggest the lint.
@ -222,6 +229,61 @@ fn is_ok_to_suggest<'tcx>(cx: &LateContext<'tcx>, lhs: &Expr<'tcx>, call: &CallC
implemented_fns.contains_key(&provided_fn.def_id)
}
/// Checks if the data being cloned borrows from the place that is being assigned to:
///
/// ```
/// let mut s = String::new();
/// let s2 = &s;
/// s = s2.to_owned();
/// ```
///
/// This cannot be written `s2.clone_into(&mut s)` because it has conflicting borrows.
fn clone_source_borrows_from_dest(cx: &LateContext<'_>, lhs: &Expr<'_>, call_span: Span) -> bool {
let Some(mir) = enclosing_mir(cx.tcx, lhs.hir_id) else {
return false;
};
let PossibleBorrowerMap { map: borrow_map, .. } = PossibleBorrowerMap::new(cx, mir);
// The operation `dest = src.to_owned()` in MIR is split up across 3 blocks.
// For the doc example above that would be roughly:
//
// bb0:
// s2 = &s
// s_temp = ToOwned::to_owned(move s2) -> bb1
//
// bb1:
// drop(s) -> bb2 // drop the old string
//
// bb2:
// s = s_temp
for bb in mir.basic_blocks.iter() {
let terminator = bb.terminator();
// Look for the to_owned/clone call.
if terminator.source_info.span == call_span
&& let mir::TerminatorKind::Call {
args,
target: Some(drop_bb),
..
} = &terminator.kind
&& let [source] = &**args
&& let mir::Operand::Move(source) = &source.node
// Block 2 only has the `drop()` terminator from to the assignment
&& let drop_bb = &mir.basic_blocks[*drop_bb]
&& let mir::TerminatorKind::Drop { target: assign_bb, .. } = drop_bb.terminator().kind
// Block 3 has the final assignment to the original local
&& let assign_bb = &mir.basic_blocks[assign_bb]
&& let [assignment, ..] = &*assign_bb.statements
&& let mir::StatementKind::Assign(box (borrowed, _)) = &assignment.kind
&& let Some(borrowers) = borrow_map.get(&borrowed.local)
&& borrowers.contains(source.local)
{
return true;
}
}
false
}
fn suggest<'tcx>(
cx: &LateContext<'tcx>,
ctxt: SyntaxContext,
@ -255,6 +317,7 @@ enum TargetTrait {
#[derive(Debug)]
struct CallCandidate<'tcx> {
span: Span,
target: TargetTrait,
kind: CallKind<'tcx>,
// DefId of the called method from an impl block that implements the target trait

View File

@ -272,3 +272,32 @@ impl<'a> Add for &'a mut HasCloneFrom {
self
}
}
mod borrowck_conflicts {
//! Cases where clone_into and friends cannot be used because the src/dest have conflicting
//! borrows.
use std::path::PathBuf;
fn issue12444(mut name: String) {
let parts = name.split(", ").collect::<Vec<_>>();
let first = *parts.first().unwrap();
name = first.to_owned();
}
fn issue12444_simple() {
let mut s = String::new();
let s2 = &s;
s = s2.to_owned();
}
fn issue12460(mut name: String) {
if let Some(stripped_name) = name.strip_prefix("baz-") {
name = stripped_name.to_owned();
}
}
fn issue12749() {
let mut path = PathBuf::from("/a/b/c");
path = path.components().as_path().to_owned();
}
}

View File

@ -272,3 +272,32 @@ impl<'a> Add for &'a mut HasCloneFrom {
self
}
}
mod borrowck_conflicts {
//! Cases where clone_into and friends cannot be used because the src/dest have conflicting
//! borrows.
use std::path::PathBuf;
fn issue12444(mut name: String) {
let parts = name.split(", ").collect::<Vec<_>>();
let first = *parts.first().unwrap();
name = first.to_owned();
}
fn issue12444_simple() {
let mut s = String::new();
let s2 = &s;
s = s2.to_owned();
}
fn issue12460(mut name: String) {
if let Some(stripped_name) = name.strip_prefix("baz-") {
name = stripped_name.to_owned();
}
}
fn issue12749() {
let mut path = PathBuf::from("/a/b/c");
path = path.components().as_path().to_owned();
}
}