rust-lang / rust-lang/rust-clippy

assigning_clones false positive for Option<T> where T uses default clone_from impl

Open
#12,709 3 comments 0 reactions 0 assignees View on GitHub

Nobody has claimed this yet.

C-bug I-false-positive
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

Open the contributing guide

First steps

  1. Read the whole issue, then the project's contributing guide.
  2. Comment on the issue to say you are picking it up — it saves two people doing the same work.
  3. Fork the repository and make your change on a branch.
  4. 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

Get new issues in your inbox

A short digest of beginner-friendly GitHub issues.