Skip to content

Commit 3ca0895

Browse files
committed
Add redundant_clone lint
1 parent 4a7601b commit 3ca0895

File tree

5 files changed

+447
-0
lines changed

5 files changed

+447
-0
lines changed

clippy_lints/src/lib.rs

Lines changed: 3 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -179,6 +179,7 @@ pub mod ptr;
179179
pub mod ptr_offset_with_cast;
180180
pub mod question_mark;
181181
pub mod ranges;
182+
pub mod redundant_clone;
182183
pub mod redundant_field_names;
183184
pub mod redundant_pattern_matching;
184185
pub mod reference;
@@ -452,6 +453,7 @@ pub fn register_plugins(reg: &mut rustc_plugin::Registry<'_>, conf: &Conf) {
452453
reg.register_late_lint_pass(box indexing_slicing::IndexingSlicing);
453454
reg.register_late_lint_pass(box non_copy_const::NonCopyConst);
454455
reg.register_late_lint_pass(box ptr_offset_with_cast::Pass);
456+
reg.register_late_lint_pass(box redundant_clone::RedundantClone);
455457

456458
reg.register_lint_group("clippy::restriction", Some("clippy_restriction"), vec![
457459
arithmetic::FLOAT_ARITHMETIC,
@@ -981,6 +983,7 @@ pub fn register_plugins(reg: &mut rustc_plugin::Registry<'_>, conf: &Conf) {
981983
fallible_impl_from::FALLIBLE_IMPL_FROM,
982984
mutex_atomic::MUTEX_INTEGER,
983985
needless_borrow::NEEDLESS_BORROW,
986+
redundant_clone::REDUNDANT_CLONE,
984987
unwrap::PANICKING_UNWRAP,
985988
unwrap::UNNECESSARY_UNWRAP,
986989
]);

clippy_lints/src/redundant_clone.rs

Lines changed: 281 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,281 @@
1+
// Copyright 2018 The Rust Project Developers. See the COPYRIGHT
2+
// file at the top-level directory of this distribution.
3+
//
4+
// Licensed under the Apache License, Version 2.0 <LICENSE-APACHE or
5+
// http://www.apache.org/licenses/LICENSE-2.0> or the MIT license
6+
// <LICENSE-MIT or http://opensource.org/licenses/MIT>, at your
7+
// option. This file may not be copied, modified, or distributed
8+
// except according to those terms.
9+
10+
use crate::rustc::hir::intravisit::FnKind;
11+
use crate::rustc::hir::{def_id, Body, FnDecl};
12+
use crate::rustc::lint::{LateContext, LateLintPass, LintArray, LintPass};
13+
use crate::rustc::mir::{
14+
self, traversal,
15+
visit::{PlaceContext, Visitor},
16+
TerminatorKind,
17+
};
18+
use crate::rustc::ty;
19+
use crate::rustc::{declare_tool_lint, lint_array};
20+
use crate::rustc_errors::Applicability;
21+
use crate::syntax::{
22+
ast::NodeId,
23+
source_map::{BytePos, Span},
24+
};
25+
use crate::utils::{
26+
in_macro, is_copy, match_def_path, match_type, paths, snippet_opt, span_lint, span_lint_and_then,
27+
walk_ptrs_ty_depth,
28+
};
29+
use if_chain::if_chain;
30+
use std::convert::TryFrom;
31+
32+
/// **What it does:** Checks for a redudant `clone()` (and its relatives) which clones an owned
33+
/// value that is going to be dropped without further use.
34+
///
35+
/// **Why is this bad?** It is not always possible for the compiler to eliminate useless
36+
/// allocations and deallocations generated by redundant `clone()`s.
37+
///
38+
/// **Known problems:**
39+
///
40+
/// * Suggestions made by this lint could require NLL to be enabled.
41+
/// * False-positive if there is a borrow preventing the value from moving out.
42+
///
43+
/// ```rust
44+
/// let x = String::new();
45+
///
46+
/// let y = &x;
47+
///
48+
/// foo(x.clone()); // This lint suggests to remove this `clone()`
49+
/// ```
50+
///
51+
/// **Example:**
52+
/// ```rust
53+
/// {
54+
/// let x = Foo::new();
55+
/// call(x.clone());
56+
/// call(x.clone()); // this can just pass `x`
57+
/// }
58+
///
59+
/// ["lorem", "ipsum"].join(" ").to_string()
60+
///
61+
/// Path::new("/a/b").join("c").to_path_buf()
62+
/// ```
63+
declare_clippy_lint! {
64+
pub REDUNDANT_CLONE,
65+
nursery,
66+
"`clone()` of an owned value that is going to be dropped immediately"
67+
}
68+
69+
pub struct RedundantClone;
70+
71+
impl LintPass for RedundantClone {
72+
fn get_lints(&self) -> LintArray {
73+
lint_array!(REDUNDANT_CLONE)
74+
}
75+
}
76+
77+
impl<'a, 'tcx> LateLintPass<'a, 'tcx> for RedundantClone {
78+
fn check_fn(
79+
&mut self,
80+
cx: &LateContext<'a, 'tcx>,
81+
_: FnKind<'tcx>,
82+
_: &'tcx FnDecl,
83+
body: &'tcx Body,
84+
_: Span,
85+
_: NodeId,
86+
) {
87+
let def_id = cx.tcx.hir.body_owner_def_id(body.id());
88+
let mir = cx.tcx.optimized_mir(def_id);
89+
90+
// Looks for `call(&T)` where `T: !Copy`
91+
let call = |kind: &mir::TerminatorKind<'tcx>| -> Option<(def_id::DefId, mir::Local, ty::Ty<'tcx>)> {
92+
if_chain! {
93+
if let TerminatorKind::Call { func, args, .. } = kind;
94+
if args.len() == 1;
95+
if let mir::Operand::Move(mir::Place::Local(local)) = &args[0];
96+
if let ty::FnDef(def_id, _) = func.ty(&*mir, cx.tcx).sty;
97+
if let (inner_ty, 1) = walk_ptrs_ty_depth(args[0].ty(&*mir, cx.tcx));
98+
if !is_copy(cx, inner_ty);
99+
then {
100+
Some((def_id, *local, inner_ty))
101+
} else {
102+
None
103+
}
104+
}
105+
};
106+
107+
for (bb, bbdata) in mir.basic_blocks().iter_enumerated() {
108+
let terminator = if let Some(terminator) = &bbdata.terminator {
109+
terminator
110+
} else {
111+
continue;
112+
};
113+
114+
// Give up on loops
115+
if terminator.successors().any(|s| *s == bb) {
116+
continue;
117+
}
118+
119+
let (fn_def_id, arg, arg_ty) = if let Some(t) = call(&terminator.kind) {
120+
t
121+
} else {
122+
continue;
123+
};
124+
125+
let from_borrow = match_def_path(cx.tcx, fn_def_id, &paths::CLONE_TRAIT_METHOD)
126+
|| match_def_path(cx.tcx, fn_def_id, &paths::TO_OWNED_METHOD)
127+
|| (match_def_path(cx.tcx, fn_def_id, &paths::TO_STRING_METHOD)
128+
&& match_type(cx, arg_ty, &paths::STRING));
129+
130+
let from_deref = !from_borrow
131+
&& (match_def_path(cx.tcx, fn_def_id, &paths::PATH_TO_PATH_BUF)
132+
|| match_def_path(cx.tcx, fn_def_id, &paths::OS_STR_TO_OS_STRING));
133+
134+
if !from_borrow && !from_deref {
135+
continue;
136+
}
137+
138+
// _1 in MIR `{ _2 = &_1; clone(move _2); }` or `{ _2 = _1; to_path_buf(_2); }
139+
let cloned = if let Some(referent) = bbdata
140+
.statements
141+
.iter()
142+
.rev()
143+
.filter_map(|stmt| {
144+
if let mir::StatementKind::Assign(mir::Place::Local(local), v) = &stmt.kind {
145+
if *local == arg {
146+
if from_deref {
147+
// `r` is already a reference.
148+
if let mir::Rvalue::Use(mir::Operand::Copy(mir::Place::Local(r))) = **v {
149+
return Some(r);
150+
}
151+
} else if let mir::Rvalue::Ref(_, _, mir::Place::Local(r)) = **v {
152+
return Some(r);
153+
}
154+
}
155+
}
156+
157+
None
158+
})
159+
.next()
160+
{
161+
referent
162+
} else {
163+
continue;
164+
};
165+
166+
// _1 in MIR `{ _2 = &_1; _3 = deref(move _2); } -> { _4 = _3; to_path_buf(move _4); }`
167+
let referent = if from_deref {
168+
let ps = mir.predecessors_for(bb);
169+
let pred_arg = if_chain! {
170+
if ps.len() == 1;
171+
if let Some(pred_terminator) = &mir[ps[0]].terminator;
172+
if let mir::TerminatorKind::Call { destination: Some((res, _)), .. } = &pred_terminator.kind;
173+
if *res == mir::Place::Local(cloned);
174+
if let Some((pred_fn_def_id, pred_arg, pred_arg_ty)) = call(&pred_terminator.kind);
175+
if match_def_path(cx.tcx, pred_fn_def_id, &paths::DEREF_TRAIT_METHOD);
176+
if match_type(cx, pred_arg_ty, &paths::PATH_BUF)
177+
|| match_type(cx, pred_arg_ty, &paths::OS_STRING);
178+
then {
179+
pred_arg
180+
} else {
181+
continue;
182+
}
183+
};
184+
185+
if let Some(referent) = mir[ps[0]]
186+
.statements
187+
.iter()
188+
.rev()
189+
.filter_map(|stmt| {
190+
if let mir::StatementKind::Assign(mir::Place::Local(l), v) = &stmt.kind {
191+
if *l == pred_arg {
192+
if let mir::Rvalue::Ref(_, _, mir::Place::Local(referent)) = **v {
193+
return Some(referent);
194+
}
195+
}
196+
}
197+
198+
None
199+
})
200+
.next()
201+
{
202+
referent
203+
} else {
204+
continue;
205+
}
206+
} else {
207+
cloned
208+
};
209+
210+
let used_later = traversal::ReversePostorder::new(&mir, bb).skip(1).any(|(tbb, tdata)| {
211+
if let Some(term) = &tdata.terminator {
212+
// Give up on loops
213+
if term.successors().any(|s| *s == bb) {
214+
return true;
215+
}
216+
}
217+
218+
let mut vis = LocalUseVisitor {
219+
local: referent,
220+
used_other_than_drop: false,
221+
};
222+
vis.visit_basic_block_data(tbb, tdata);
223+
vis.used_other_than_drop
224+
});
225+
226+
if !used_later {
227+
let span = terminator.source_info.span;
228+
if_chain! {
229+
if !in_macro(span);
230+
if let Some(snip) = snippet_opt(cx, span);
231+
if let Some(dot) = snip.rfind('.');
232+
then {
233+
let sugg_span = span.with_lo(
234+
span.lo() + BytePos(u32::try_from(dot).unwrap())
235+
);
236+
237+
span_lint_and_then(cx, REDUNDANT_CLONE, sugg_span, "redundant clone", |db| {
238+
db.span_suggestion_with_applicability(
239+
sugg_span,
240+
"remove this",
241+
String::new(),
242+
Applicability::MaybeIncorrect,
243+
);
244+
db.span_note(
245+
span.with_hi(span.lo() + BytePos(u32::try_from(dot).unwrap())),
246+
"this value is dropped without further use",
247+
);
248+
});
249+
} else {
250+
span_lint(cx, REDUNDANT_CLONE, span, "redundant clone");
251+
}
252+
}
253+
}
254+
}
255+
}
256+
}
257+
258+
struct LocalUseVisitor {
259+
local: mir::Local,
260+
used_other_than_drop: bool,
261+
}
262+
263+
impl<'tcx> mir::visit::Visitor<'tcx> for LocalUseVisitor {
264+
fn visit_statement(&mut self, block: mir::BasicBlock, statement: &mir::Statement<'tcx>, location: mir::Location) {
265+
// Once flagged, skip remaining statements
266+
if !self.used_other_than_drop {
267+
self.super_statement(block, statement, location);
268+
}
269+
}
270+
271+
fn visit_local(&mut self, local: &mir::Local, ctx: PlaceContext<'tcx>, _: mir::Location) {
272+
match ctx {
273+
PlaceContext::Drop | PlaceContext::StorageDead => return,
274+
_ => {},
275+
}
276+
277+
if *local == self.local {
278+
self.used_other_than_drop = true;
279+
}
280+
}
281+
}

clippy_lints/src/utils/paths.rs

Lines changed: 8 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -23,6 +23,7 @@ pub const BTREEMAP: [&str; 5] = ["alloc", "collections", "btree", "map", "BTreeM
2323
pub const BTREEMAP_ENTRY: [&str; 5] = ["alloc", "collections", "btree", "map", "Entry"];
2424
pub const BTREESET: [&str; 5] = ["alloc", "collections", "btree", "set", "BTreeSet"];
2525
pub const CLONE_TRAIT: [&str; 3] = ["core", "clone", "Clone"];
26+
pub const CLONE_TRAIT_METHOD: [&str; 4] = ["core", "clone", "Clone", "clone"];
2627
pub const CMP_MAX: [&str; 3] = ["core", "cmp", "max"];
2728
pub const CMP_MIN: [&str; 3] = ["core", "cmp", "min"];
2829
pub const COW: [&str; 3] = ["alloc", "borrow", "Cow"];
@@ -31,6 +32,7 @@ pub const C_VOID: [&str; 3] = ["core", "ffi", "c_void"];
3132
pub const C_VOID_LIBC: [&str; 2] = ["libc", "c_void"];
3233
pub const DEFAULT_TRAIT: [&str; 3] = ["core", "default", "Default"];
3334
pub const DEFAULT_TRAIT_METHOD: [&str; 4] = ["core", "default", "Default", "default"];
35+
pub const DEREF_TRAIT_METHOD: [&str; 5] = ["core", "ops", "deref", "Deref", "deref"];
3436
pub const DISPLAY_FMT_METHOD: [&str; 4] = ["core", "fmt", "Display", "fmt"];
3537
pub const DOUBLE_ENDED_ITERATOR: [&str; 4] = ["core", "iter", "traits", "DoubleEndedIterator"];
3638
pub const DROP: [&str; 3] = ["core", "mem", "drop"];
@@ -67,7 +69,11 @@ pub const OPTION: [&str; 3] = ["core", "option", "Option"];
6769
pub const OPTION_NONE: [&str; 4] = ["core", "option", "Option", "None"];
6870
pub const OPTION_SOME: [&str; 4] = ["core", "option", "Option", "Some"];
6971
pub const ORD: [&str; 3] = ["core", "cmp", "Ord"];
72+
pub const OS_STRING: [&str; 4] = ["std", "ffi", "os_str", "OsString"];
73+
pub const OS_STR_TO_OS_STRING: [&str; 5] = ["std", "ffi", "os_str", "OsStr", "to_os_string"];
7074
pub const PARTIAL_ORD: [&str; 3] = ["core", "cmp", "PartialOrd"];
75+
pub const PATH_BUF: [&str; 3] = ["std", "path", "PathBuf"];
76+
pub const PATH_TO_PATH_BUF: [&str; 4] = ["std", "path", "Path", "to_path_buf"];
7177
pub const PTR_NULL: [&str; 2] = ["ptr", "null"];
7278
pub const PTR_NULL_MUT: [&str; 2] = ["ptr", "null_mut"];
7379
pub const RANGE: [&str; 3] = ["core", "ops", "Range"];
@@ -100,7 +106,9 @@ pub const SLICE_INTO_VEC: [&str; 4] = ["alloc", "slice", "<impl [T]>", "into_vec
100106
pub const SLICE_ITER: [&str; 3] = ["core", "slice", "Iter"];
101107
pub const STRING: [&str; 3] = ["alloc", "string", "String"];
102108
pub const TO_OWNED: [&str; 3] = ["alloc", "borrow", "ToOwned"];
109+
pub const TO_OWNED_METHOD: [&str; 4] = ["alloc", "borrow", "ToOwned", "to_owned"];
103110
pub const TO_STRING: [&str; 3] = ["alloc", "string", "ToString"];
111+
pub const TO_STRING_METHOD: [&str; 4] = ["alloc", "string", "ToString", "to_string"];
104112
pub const TRANSMUTE: [&str; 4] = ["core", "intrinsics", "", "transmute"];
105113
pub const TRY_INTO_RESULT: [&str; 4] = ["std", "ops", "Try", "into_result"];
106114
pub const UNINIT: [&str; 4] = ["core", "intrinsics", "", "uninit"];

tests/ui/redundant_clone.rs

Lines changed: 44 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,44 @@
1+
// Copyright 2018 The Rust Project Developers. See the COPYRIGHT
2+
// file at the top-level directory of this distribution.
3+
//
4+
// Licensed under the Apache License, Version 2.0 <LICENSE-APACHE or
5+
// http://www.apache.org/licenses/LICENSE-2.0> or the MIT license
6+
// <LICENSE-MIT or http://opensource.org/licenses/MIT>, at your
7+
// option. This file may not be copied, modified, or distributed
8+
// except according to those terms.
9+
10+
#![warn(clippy::redundant_clone)]
11+
12+
use std::path::Path;
13+
use std::ffi::OsString;
14+
15+
fn main() {
16+
let _ = ["lorem", "ipsum"].join(" ").to_string();
17+
18+
let s = String::from("foo");
19+
let _ = s.clone();
20+
21+
let s = String::from("foo");
22+
let _ = s.to_string();
23+
24+
let s = String::from("foo");
25+
let _ = s.to_owned();
26+
27+
let _ = Path::new("/a/b/").join("c").to_owned();
28+
29+
let _ = Path::new("/a/b/").join("c").to_path_buf();
30+
31+
let _ = OsString::new().to_owned();
32+
33+
let _ = OsString::new().to_os_string();
34+
}
35+
36+
#[derive(Clone)]
37+
struct Alpha;
38+
fn double(a: Alpha) -> (Alpha, Alpha) {
39+
if true {
40+
(a.clone(), a.clone())
41+
} else {
42+
(Alpha, a)
43+
}
44+
}

0 commit comments

Comments
 (0)