Skip to content

Commit eb12bac

Browse files
authored
perf: skip redundant clone analysis for clone-free functions (#17486)
On master, `redundant_clone` requests optimized MIR for every function. It turns out this is fairly expensive. This PR adds a pre-filter that scans each function’s HIR first and skips MIR analysis _unless_ the body contains a `.clone()`, `.to_owned()`, `.to_string()`, `.to_path_buf()`, or `.to_os_string()` call. (It's just a pre-filter so false positives are acceptable.) Benchmarking on my M4, incremental Clippy checks in `codex-rs` went from 4.19 s to 3.98 s (~5% improvement). A more contrived workload (4,096 functions) went from 220 ms to 178 ms (~19% improvement). changelog: none
2 parents db59539 + 7c5a095 commit eb12bac

4 files changed

Lines changed: 91 additions & 4 deletions

File tree

clippy_lints/src/redundant_clone.rs

Lines changed: 22 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -3,16 +3,18 @@ use clippy_utils::mir::{LocalUsage, PossibleBorrowerMap, visit_local_usage};
33
use clippy_utils::res::MaybeDef as _;
44
use clippy_utils::source::SpanExt as _;
55
use clippy_utils::ty::{has_drop, is_copy, peel_and_count_ty_refs};
6+
use clippy_utils::visitors::for_each_expr_without_closures;
67
use clippy_utils::{fn_has_unsatisfiable_clauses, sym};
78
use rustc_errors::Applicability;
89
use rustc_hir::attrs::lang_items::LangItem;
910
use rustc_hir::intravisit::FnKind;
10-
use rustc_hir::{Body, FnDecl, def_id};
11+
use rustc_hir::{Body, ExprKind, FnDecl, def_id};
1112
use rustc_lint::{LateContext, LateLintPass, declare_lint_pass};
1213
use rustc_middle::mir;
1314
use rustc_middle::ty::{self, Ty};
1415
use rustc_span::def_id::LocalDefId;
1516
use rustc_span::{BytePos, Span};
17+
use std::ops::ControlFlow;
1618

1719
declare_clippy_lint! {
1820
/// ### What it does
@@ -60,12 +62,13 @@ impl<'tcx> LateLintPass<'tcx> for RedundantClone {
6062
cx: &LateContext<'tcx>,
6163
_: FnKind<'tcx>,
6264
_: &'tcx FnDecl<'_>,
63-
_: &'tcx Body<'_>,
65+
body: &'tcx Body<'_>,
6466
_: Span,
6567
def_id: LocalDefId,
6668
) {
69+
// Closures receive their own `check_fn` callback and are checked separately.
6770
// Building MIR for `fn`s with unsatisfiable clauses results in ICE.
68-
if fn_has_unsatisfiable_clauses(cx, def_id.to_def_id()) {
71+
if !contains_clone_like_call(body) || fn_has_unsatisfiable_clauses(cx, def_id.to_def_id()) {
6972
return;
7073
}
7174

@@ -244,6 +247,22 @@ impl<'tcx> LateLintPass<'tcx> for RedundantClone {
244247
}
245248
}
246249

250+
fn contains_clone_like_call(body: &Body<'_>) -> bool {
251+
for_each_expr_without_closures(body, |expr| {
252+
if let ExprKind::MethodCall(method, ..) = expr.kind
253+
&& matches!(
254+
method.ident.name,
255+
sym::clone | sym::to_owned | sym::to_string | sym::to_path_buf | sym::to_os_string
256+
)
257+
{
258+
ControlFlow::Break(())
259+
} else {
260+
ControlFlow::Continue(())
261+
}
262+
})
263+
.is_some()
264+
}
265+
247266
/// If `kind` is `y = func(x: &T)` where `T: !Copy`, returns `(DefId of func, x, T, y)`.
248267
fn is_call_with_ref_arg<'tcx>(
249268
cx: &LateContext<'tcx>,

tests/ui/redundant_clone.fixed

Lines changed: 22 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -291,3 +291,25 @@ mod issue13900 {
291291
}
292292
}
293293
}
294+
295+
mod hir_prefilter {
296+
fn nested_closure() {
297+
let _ = || {
298+
let value = String::from("closure");
299+
let _: String = value;
300+
//~^ redundant_clone
301+
};
302+
}
303+
304+
macro_rules! identity {
305+
($expression:expr) => {
306+
$expression
307+
};
308+
}
309+
310+
fn macro_expression() {
311+
let value = String::from("macro");
312+
let _ = identity!(value);
313+
//~^ redundant_clone
314+
}
315+
}

tests/ui/redundant_clone.rs

Lines changed: 22 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -291,3 +291,25 @@ mod issue13900 {
291291
}
292292
}
293293
}
294+
295+
mod hir_prefilter {
296+
fn nested_closure() {
297+
let _ = || {
298+
let value = String::from("closure");
299+
let _: String = value.clone();
300+
//~^ redundant_clone
301+
};
302+
}
303+
304+
macro_rules! identity {
305+
($expression:expr) => {
306+
$expression
307+
};
308+
}
309+
310+
fn macro_expression() {
311+
let value = String::from("macro");
312+
let _ = identity!(value.clone());
313+
//~^ redundant_clone
314+
}
315+
}

tests/ui/redundant_clone.stderr

Lines changed: 25 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -180,5 +180,29 @@ note: this value is dropped without further use
180180
LL | foo(&x.clone(), move || {
181181
| ^
182182

183-
error: aborting due to 15 previous errors
183+
error: redundant clone
184+
--> tests/ui/redundant_clone.rs:299:34
185+
|
186+
LL | let _: String = value.clone();
187+
| ^^^^^^^^ help: remove this
188+
|
189+
note: this value is dropped without further use
190+
--> tests/ui/redundant_clone.rs:299:29
191+
|
192+
LL | let _: String = value.clone();
193+
| ^^^^^
194+
195+
error: redundant clone
196+
--> tests/ui/redundant_clone.rs:312:32
197+
|
198+
LL | let _ = identity!(value.clone());
199+
| ^^^^^^^^ help: remove this
200+
|
201+
note: this value is dropped without further use
202+
--> tests/ui/redundant_clone.rs:312:27
203+
|
204+
LL | let _ = identity!(value.clone());
205+
| ^^^^^
206+
207+
error: aborting due to 17 previous errors
184208

0 commit comments

Comments
 (0)