Skip to content

Commit e80ca2f

Browse files
committed
Auto merge of rust-lang#12615 - Kobzol:fix-recursive-clone-from, r=blyxyas
Do not suggest `assigning_clones` in `Clone` impl This PR modifies `assigning_clones` to detect situations where the `clone` call is inside a `Clone` impl, and avoids suggesting the lint in such situations. r? `@blyxyas` Fixes: rust-lang/rust-clippy#12600 changelog: Do not invoke `assigning_clones` inside `Clone` impl
2 parents f9f854f + 571118f commit e80ca2f

File tree

4 files changed

+49
-6
lines changed

4 files changed

+49
-6
lines changed

clippy_lints/src/assigning_clones.rs

+17
Original file line numberDiff line numberDiff line change
@@ -181,6 +181,23 @@ fn is_ok_to_suggest<'tcx>(cx: &LateContext<'tcx>, lhs: &Expr<'tcx>, call: &CallC
181181
return false;
182182
}
183183

184+
// If the call expression is inside an impl block that contains the method invoked by the
185+
// call expression, we bail out to avoid suggesting something that could result in endless
186+
// recursion.
187+
if let Some(local_block_id) = impl_block.as_local()
188+
&& let Some(block) = cx.tcx.hir_node_by_def_id(local_block_id).as_owner()
189+
{
190+
let impl_block_owner = block.def_id();
191+
if cx
192+
.tcx
193+
.hir()
194+
.parent_id_iter(lhs.hir_id)
195+
.any(|parent| parent.owner == impl_block_owner)
196+
{
197+
return false;
198+
}
199+
}
200+
184201
// Find the function for which we want to check that it is implemented.
185202
let provided_fn = match call.target {
186203
TargetTrait::Clone => cx.tcx.get_diagnostic_item(sym::Clone).and_then(|clone| {

tests/ui/assigning_clones.fixed

+13
Original file line numberDiff line numberDiff line change
@@ -153,6 +153,19 @@ fn clone_inside_macro() {
153153
clone_inside!(a, b);
154154
}
155155

156+
// Make sure that we don't suggest the lint when we call clone inside a Clone impl
157+
// https://github.com/rust-lang/rust-clippy/issues/12600
158+
pub struct AvoidRecursiveCloneFrom;
159+
160+
impl Clone for AvoidRecursiveCloneFrom {
161+
fn clone(&self) -> Self {
162+
Self
163+
}
164+
fn clone_from(&mut self, source: &Self) {
165+
*self = source.clone();
166+
}
167+
}
168+
156169
// ToOwned
157170
fn owned_method_mut_ref(mut_string: &mut String, ref_str: &str) {
158171
ref_str.clone_into(mut_string);

tests/ui/assigning_clones.rs

+13
Original file line numberDiff line numberDiff line change
@@ -153,6 +153,19 @@ fn clone_inside_macro() {
153153
clone_inside!(a, b);
154154
}
155155

156+
// Make sure that we don't suggest the lint when we call clone inside a Clone impl
157+
// https://github.com/rust-lang/rust-clippy/issues/12600
158+
pub struct AvoidRecursiveCloneFrom;
159+
160+
impl Clone for AvoidRecursiveCloneFrom {
161+
fn clone(&self) -> Self {
162+
Self
163+
}
164+
fn clone_from(&mut self, source: &Self) {
165+
*self = source.clone();
166+
}
167+
}
168+
156169
// ToOwned
157170
fn owned_method_mut_ref(mut_string: &mut String, ref_str: &str) {
158171
*mut_string = ref_str.to_owned();

tests/ui/assigning_clones.stderr

+6-6
Original file line numberDiff line numberDiff line change
@@ -86,37 +86,37 @@ LL | a = c.to_owned();
8686
| ^^^^^^^^^^^^^^^^ help: use `clone_into()`: `c.clone_into(&mut a)`
8787

8888
error: assigning the result of `ToOwned::to_owned()` may be inefficient
89-
--> tests/ui/assigning_clones.rs:158:5
89+
--> tests/ui/assigning_clones.rs:171:5
9090
|
9191
LL | *mut_string = ref_str.to_owned();
9292
| ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ help: use `clone_into()`: `ref_str.clone_into(mut_string)`
9393

9494
error: assigning the result of `ToOwned::to_owned()` may be inefficient
95-
--> tests/ui/assigning_clones.rs:162:5
95+
--> tests/ui/assigning_clones.rs:175:5
9696
|
9797
LL | mut_string = ref_str.to_owned();
9898
| ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ help: use `clone_into()`: `ref_str.clone_into(&mut mut_string)`
9999

100100
error: assigning the result of `ToOwned::to_owned()` may be inefficient
101-
--> tests/ui/assigning_clones.rs:183:5
101+
--> tests/ui/assigning_clones.rs:196:5
102102
|
103103
LL | **mut_box_string = ref_str.to_owned();
104104
| ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ help: use `clone_into()`: `ref_str.clone_into(&mut (*mut_box_string))`
105105

106106
error: assigning the result of `ToOwned::to_owned()` may be inefficient
107-
--> tests/ui/assigning_clones.rs:187:5
107+
--> tests/ui/assigning_clones.rs:200:5
108108
|
109109
LL | **mut_box_string = ref_str.to_owned();
110110
| ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ help: use `clone_into()`: `ref_str.clone_into(&mut (*mut_box_string))`
111111

112112
error: assigning the result of `ToOwned::to_owned()` may be inefficient
113-
--> tests/ui/assigning_clones.rs:191:5
113+
--> tests/ui/assigning_clones.rs:204:5
114114
|
115115
LL | *mut_thing = ToOwned::to_owned(ref_str);
116116
| ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ help: use `clone_into()`: `ToOwned::clone_into(ref_str, mut_thing)`
117117

118118
error: assigning the result of `ToOwned::to_owned()` may be inefficient
119-
--> tests/ui/assigning_clones.rs:195:5
119+
--> tests/ui/assigning_clones.rs:208:5
120120
|
121121
LL | mut_thing = ToOwned::to_owned(ref_str);
122122
| ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ help: use `clone_into()`: `ToOwned::clone_into(ref_str, &mut mut_thing)`

0 commit comments

Comments
 (0)