rust-lang / rust-lang/rust-clippy
assigning_clones false positive for Option<T> where T uses default clone_from impl
Nobody has claimed this yet.
- Dominant language
- Rust
- Stars
- 13.5k
- Forks
- 2.2k
- Avg merge
- 2d 10h
- Merged PRs (30d)
- 32
Description
Summary
Motivation
I have a type, Id, which is really cheap to copy, yet unfortunately cannot implement Copy: it uses just a u64 in-memory representation in almost all cases, and a reference-counted heap-allocated representation in the remaining cases. It also gets used quite frequently, and I want to make it easy to use and pass around. I mean, I'm almost tempted to let heap-allocated instances leak just so that I can implement Copy.
It's also common to want to copy an Option<Id> around. Queue Clippy's assigning_clones recommending usage of clone_from instead, despite the fact that Id does not implement clone_from (or have anything to gain if it did, since the
heap-allocated representations are not mutable).
Besides which, usages of Id are not particularly performance sensitive. Remember premature optimisation is the root of all evil? There may be cases where this lint significantly improves code, but it's not here.
Expectation
Either that Clippy can recognise the false positive (Option impls clone_from but T doesn't) or that this lint can be disabled with an annotation on the type.
Lint Name
assigning_clones
Reproducer
I tried this code:
mod inner {
#[derive(Clone, Debug, PartialEq, Eq)]
pub struct Id(u64);
impl From<u64> for Id {
fn from(n: u64) -> Self {
Id(n)
}
}
}
use inner::Id;
fn main() {
let a = Some(Id::from(123));
let mut b = None;
assert!(a != b);
b = a.clone();
assert_eq!(a, b);
dbg!(b);
}
I saw this happen:
$ cargo clippy
Checking assigning_clones v0.1.0 (/home/dhardy/projects/small/assigning_clones)
warning: assigning the result of `Clone::clone()` may be inefficient
--> src/main.rs:17:5
|
17 | b = a.clone();
| ^^^^^^^^^^^^^ help: use `clone_from()`: `b.clone_from(&a)`
|
= help: for further information visit https://rust-lang.github.io/rust-clippy/master/index.html#assigning_clones
= note: `#[warn(clippy::assigning_clones)]` on by default
warning: `assigning_clones` (bin "assigning_clones") generated 1 warning
Finished `dev` profile [unoptimized + debuginfo] target(s) in 0.04s
I expected to see this happen: no warning.
Version
rustc 1.79.0-nightly (244da22fa 2024-04-23)
binary: rustc
commit-hash: 244da22fabd9fa677bbd0ac601a88e5ca6917526
commit-date: 2024-04-23
host: x86_64-unknown-linux-gnu
release: 1.79.0-nightly
LLVM version: 18.1.4
Additional Labels
No response
Contributor guide
First steps
- Read the whole issue, then the project's contributing guide.
- Comment on the issue to say you are picking it up — it saves two people doing the same work.
- Fork the repository and make your change on a branch.
- Open a pull request that references the issue number.
Research direction
Start with the assigning_clones reproducer in src/main.rs and inspect the lint's handling of Option where T only uses the default clone_from implementation. Compare the lint's behavior with the reported Rust version, then ensure the false positive is avoided or that the requested type-level opt-out is supported.
Written by the indexing model from the issue text.
Assessment
- Tech stack
- rust
- Domain
- tooling
- Issue type
- Bug
- Difficulty
- 4/5
- Estimated time
- 3-5 days
- Activity status
- Stale
- Clarity
- Mostly clear
- Newbie friendliness
- 45/100