assigning_clones: Wrong suggestion with deref

@Kobzol

Summary

assigning_clones suggests code that fails to compile when the original code has deref.

Mentioning @Kobzol, who implemented this lint in #12077.

Reproducer

I tried this code:

#![allow(dead_code)]
#![warn(clippy::assigning_clones)]

use std::task::{Context, Waker};

fn f(mut prev: Box<Waker>, cx: &mut Context<'_>) -> Box<Waker> {
    *prev = cx.waker().clone();
    prev
}

I expected to see this happen: lint suggests code that can compile

Instead, this happened:

warning: assigning the result of `Clone::clone()` may be inefficient
 --> src/lib.rs:7:5
  |
7 |     *prev = cx.waker().clone();
  |     ^^^^^^^^^^^^^^^^^^^^^^^^^^ help: use `clone_from()`: `prev.clone_from(cx.waker())`
  |
  = help: for further information visit https://rust-lang.github.io/rust-clippy/master/index.html#assigning_clones

Suggested code prev.clone_from(cx.waker()) fails to compile because deref (*prev) is dropped:

#![allow(dead_code)]
#![warn(clippy::assigning_clones)]

use std::task::{Context, Waker};

fn f(mut prev: Box<Waker>, cx: &mut Context<'_>) -> Box<Waker> {
    prev.clone_from(cx.waker());
    prev
}

error[E0308]: mismatched types
   --> src/lib.rs:7:21
    |
7   |     prev.clone_from(cx.waker());
    |          ---------- ^^^^^^^^^^ expected `&Box<Waker>`, found `&Waker`
    |          |
    |          arguments to this method are incorrect
    |
    = note: expected reference `&std::boxed::Box<std::task::Waker>`
               found reference `&std::task::Waker`

The correct code is (*prev).clone_from(cx.waker()); (preserve deref).

#![allow(dead_code)]
#![warn(clippy::assigning_clones)]

use std::task::{Context, Waker};

fn f(mut prev: Box<Waker>, cx: &mut Context<'_>) -> Box<Waker> {
    (*prev).clone_from(cx.waker());
    prev
}

Version

rustc 1.78.0-nightly (9c3ad802d 2024-03-07)
binary: rustc
commit-hash: 9c3ad802d9b9633d60d3a74668eb1be819212d34
commit-date: 2024-03-07
host: aarch64-apple-darwin
release: 1.78.0-nightly
LLVM version: 18.1.0

Additional Labels

@rustbot label +I-suggestion-causes-error

I'm seeing a similar issue but where the variable is being assigned to itself. Not sure if this counts as a separate issue.

Example code (taken from one of my projects where it locates the root of the repo)

use std::{env, error::Error, fmt::Display};

#[derive(Debug)]
struct NoJustfile;

impl Display for NoJustfile {
    fn fmt(&self, f: &mut std::fmt::Formatter<'_>) -> std::fmt::Result {
        write!(f, "{self:?}")
    }
}
impl Error for NoJustfile {}

fn main() -> Result<(), Box<dyn Error>> {
    let mut path_to_search = env::current_dir()?;

    while !path_to_search.join("justfile").exists() {
        path_to_search = path_to_search
            .parent()
            .ok_or_else(|| Box::new(NoJustfile))?
            .to_owned();
    }

    println!("{}", path_to_search.display());

    Ok(())
}

you get the suggestion:

    Checking test-crate v0.1.0 (/tmp/test-crate)
warning: assigning the result of `ToOwned::to_owned()` may be inefficient
  --> src/main.rs:17:9
   |
17 | /         path_to_search = path_to_search
18 | |             .parent()
19 | |             .ok_or_else(|| Box::new(NoJustfile))?
20 | |             .to_owned();
   | |_______________________^
   |
   = 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
help: use `clone_into()`
   |
17 ~         path_to_search
18 +             .parent()
19 ~             .ok_or_else(|| Box::new(NoJustfile))?.clone_into(&mut path_to_search);
   |

warning: `test-crate` (bin "test-crate") generated 1 warning (run `cargo clippy --fix --bin "test-crate"` to apply 1 suggestion)
    Finished `dev` profile [unoptimized + debuginfo] target(s) in 0.07s

but if you do that you get:

error[E0502]: cannot borrow `path_to_search` as mutable because it is also borrowed as immutable
  --> src/main.rs:20:25
   |
17 |         path_to_search
   |         -------------- immutable borrow occurs here
...
20 |             .clone_into(&mut path_to_search);
   |              ---------- ^^^^^^^^^^^^^^^^^^^ mutable borrow occurs here
   |              |
   |              immutable borrow later used by call

For more information about this error, try `rustc --explain E0502`.
error: could not compile `test-crate` (bin "test-crate") due to 1 previous error

Version:

rustc 1.78.0-nightly (9c3ad802d 2024-03-07)
binary: rustc
commit-hash: 9c3ad802d9b9633d60d3a74668eb1be819212d34
commit-date: 2024-03-07
host: x86_64-unknown-linux-gnu
release: 1.78.0-nightly
LLVM version: 18.1.0

Thanks for the report, I'll take a look.

@taiki-e

#12473 should hopefully fix the original issue from @taiki-e.

The issue from @gwilymk is related to a different problem, and has been found in multiple other issues (see the linked PR).

Auto merge of #12473 - Kobzol:assigning-clones-deref, r=Alexendoo

Navigation Menu

Repository navigation

assigning_clones: Wrong suggestion with deref #12437

Summary

Reproducer

Version

Additional Labels

Metadata

Assignees

Labels

Type

Projects

Milestone

Relationships

Development

assigning_clones: Wrong suggestion with deref #12437

Description

Summary

Reproducer

Version

Additional Labels

Activity

gwilymk commented on Mar 8, 2024

Kobzol commented on Mar 10, 2024

Kobzol commented on Mar 12, 2024

Metadata

Metadata

Assignees

Labels

Type

Projects

Milestone

Relationships

Development

Issue actions