|
1 | 1 | use clippy_config::msrvs::{self, Msrv}; |
2 | 2 | use clippy_utils::diagnostics::span_lint_and_then; |
3 | 3 | use clippy_utils::macros::HirNode; |
| 4 | +use clippy_utils::mir::{enclosing_mir, PossibleBorrowerMap}; |
4 | 5 | use clippy_utils::sugg::Sugg; |
5 | 6 | use clippy_utils::{is_trait_method, local_is_initialized, path_to_local}; |
6 | 7 | use rustc_errors::Applicability; |
7 | 8 | use rustc_hir::{self as hir, Expr, ExprKind}; |
8 | 9 | use rustc_lint::{LateContext, LateLintPass}; |
| 10 | +use rustc_middle::mir; |
9 | 11 | use rustc_middle::ty::{self, Instance, Mutability}; |
10 | 12 | use rustc_session::impl_lint_pass; |
11 | 13 | use rustc_span::def_id::DefId; |
12 | 14 | use rustc_span::symbol::sym; |
13 | | -use rustc_span::{ExpnKind, SyntaxContext}; |
| 15 | +use rustc_span::{ExpnKind, Span, SyntaxContext}; |
14 | 16 |
|
15 | 17 | declare_clippy_lint! { |
16 | 18 | /// ### What it does |
@@ -144,6 +146,7 @@ fn extract_call<'tcx>(cx: &LateContext<'tcx>, expr: &'tcx Expr<'tcx>) -> Option< |
144 | 146 | }; |
145 | 147 |
|
146 | 148 | Some(CallCandidate { |
| 149 | + span: expr.span, |
147 | 150 | target, |
148 | 151 | kind, |
149 | 152 | method_def_id: resolved_method.def_id(), |
@@ -215,13 +218,85 @@ fn is_ok_to_suggest<'tcx>(cx: &LateContext<'tcx>, lhs: &Expr<'tcx>, call: &CallC |
215 | 218 | return false; |
216 | 219 | }; |
217 | 220 |
|
| 221 | + if clone_source_borrows_from_dest(cx, lhs, call.span) { |
| 222 | + return false; |
| 223 | + } |
| 224 | + |
218 | 225 | // Now take a look if the impl block defines an implementation for the method that we're interested |
219 | 226 | // in. If not, then we're using a default implementation, which is not interesting, so we will |
220 | 227 | // not suggest the lint. |
221 | 228 | let implemented_fns = cx.tcx.impl_item_implementor_ids(impl_block); |
222 | 229 | implemented_fns.contains_key(&provided_fn.def_id) |
223 | 230 | } |
224 | 231 |
|
| 232 | +/// Checks if the data being cloned borrows from the place that is being assigned to: |
| 233 | +/// |
| 234 | +/// ``` |
| 235 | +/// let mut s = String::new(); |
| 236 | +/// let s2 = &s; |
| 237 | +/// s = s2.to_owned(); |
| 238 | +/// ``` |
| 239 | +/// |
| 240 | +/// This cannot be written `s2.clone_into(&mut s)` because it has conflicting borrows. |
| 241 | +fn clone_source_borrows_from_dest(cx: &LateContext<'_>, lhs: &Expr<'_>, call_span: Span) -> bool { |
| 242 | + /// If this basic block only exists to drop a local as part of an assignment, returns its |
| 243 | + /// successor. Otherwise returns the basic block that was passed in. |
| 244 | + fn skip_drop_block(mir: &mir::Body<'_>, bb: mir::BasicBlock) -> mir::BasicBlock { |
| 245 | + if let mir::TerminatorKind::Drop { target, .. } = mir.basic_blocks[bb].terminator().kind { |
| 246 | + target |
| 247 | + } else { |
| 248 | + bb |
| 249 | + } |
| 250 | + } |
| 251 | + |
| 252 | + let Some(mir) = enclosing_mir(cx.tcx, lhs.hir_id) else { |
| 253 | + return false; |
| 254 | + }; |
| 255 | + let PossibleBorrowerMap { map: borrow_map, .. } = PossibleBorrowerMap::new(cx, mir); |
| 256 | + |
| 257 | + // The operation `dest = src.to_owned()` in MIR is split up across 3 blocks *if* the type has `Drop` |
| 258 | + // code. For types that don't, the second basic block is simply skipped. |
| 259 | + // For the doc example above that would be roughly: |
| 260 | + // |
| 261 | + // bb0: |
| 262 | + // s2 = &s |
| 263 | + // s_temp = ToOwned::to_owned(move s2) -> bb1 |
| 264 | + // |
| 265 | + // bb1: |
| 266 | + // drop(s) -> bb2 // drop the old string |
| 267 | + // |
| 268 | + // bb2: |
| 269 | + // s = s_temp |
| 270 | + for bb in mir.basic_blocks.iter() { |
| 271 | + let terminator = bb.terminator(); |
| 272 | + |
| 273 | + // Look for the to_owned/clone call. |
| 274 | + if terminator.source_info.span != call_span { |
| 275 | + continue; |
| 276 | + } |
| 277 | + |
| 278 | + if let mir::TerminatorKind::Call { ref args, target: Some(assign_bb), .. } = terminator.kind |
| 279 | + && let [source] = &**args |
| 280 | + && let mir::Operand::Move(source) = &source.node |
| 281 | + && let assign_bb = skip_drop_block(mir, assign_bb) |
| 282 | + // Skip any storage statements as they are just noise |
| 283 | + && let Some(assignment) = mir.basic_blocks[assign_bb].statements |
| 284 | + .iter() |
| 285 | + .find(|stmt| { |
| 286 | + !matches!(stmt.kind, mir::StatementKind::StorageDead(_) | mir::StatementKind::StorageLive(_)) |
| 287 | + }) |
| 288 | + && let mir::StatementKind::Assign(box (borrowed, _)) = &assignment.kind |
| 289 | + && let Some(borrowers) = borrow_map.get(&borrowed.local) |
| 290 | + && borrowers.contains(source.local) |
| 291 | + { |
| 292 | + return true; |
| 293 | + } |
| 294 | + |
| 295 | + return false; |
| 296 | + } |
| 297 | + false |
| 298 | +} |
| 299 | + |
225 | 300 | fn suggest<'tcx>( |
226 | 301 | cx: &LateContext<'tcx>, |
227 | 302 | ctxt: SyntaxContext, |
@@ -255,6 +330,7 @@ enum TargetTrait { |
255 | 330 |
|
256 | 331 | #[derive(Debug)] |
257 | 332 | struct CallCandidate<'tcx> { |
| 333 | + span: Span, |
258 | 334 | target: TargetTrait, |
259 | 335 | kind: CallKind<'tcx>, |
260 | 336 | // DefId of the called method from an impl block that implements the target trait |
|
0 commit comments