Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

assigning_clones: Suggestion conflicts with borrow checker #12444

Open
adamreichold opened this issue Mar 9, 2024 · 2 comments · May be fixed by #12756
Open

assigning_clones: Suggestion conflicts with borrow checker #12444

adamreichold opened this issue Mar 9, 2024 · 2 comments · May be fixed by #12756
Assignees
Labels
C-bug Category: Clippy is not doing the correct thing I-suggestion-causes-error Issue: The suggestions provided by this Lint cause an ICE/error when applied

Comments

@adamreichold
Copy link
Contributor

adamreichold commented Mar 9, 2024

Summary

Nightly Clippy started suggesting usage of clone_into but applying the suggesting conflicts with the borrow checker because an immutable borrow of the target variable exists. Assigning works because the borrow ends after the clone is done and hence before the assignment.

Reproducer

I started with the warning:

warning: assigning the result of `ToOwned::to_owned()` may be inefficient
   --> harvester/src/uvp_verbund.rs:169:33
    |
169 | ...                   name = last.to_owned();
    |                       ^^^^^^^^^^^^^^^^^^^^^^ help: use `clone_into()`: `last.clone_into(&mut name)`
    |
    = help: for further information visit https://rust-lang.github.io/rust-clippy/master/index.html#assigning_clones

and after applying the suggestion got:

error[E0502]: cannot borrow `name` as mutable because it is also borrowed as immutable
   --> harvester/src/uvp_verbund.rs:169:49
    |
161 |                         let parts = name.split(", ").collect::<Vec<_>>();
    |                                     ---- immutable borrow occurs here
...
169 |                                 last.clone_into(&mut name);
    |                                      ---------- ^^^^^^^^^ mutable borrow occurs here
    |                                      |
    |                                      immutable borrow later used by call

The full code is available here.

Version

rustc 1.78.0-nightly (46b180ec2 2024-03-08)
binary: rustc
commit-hash: 46b180ec2452d388c5d9c14009442e2e0beb01d7
commit-date: 2024-03-08
host: x86_64-unknown-linux-gnu
release: 1.78.0-nightly
LLVM version: 18.1.0

Additional Labels

@rustbot label +I-suggestion-causes-error

@Kobzol
Copy link
Contributor

Kobzol commented Mar 12, 2024

This is a minimal reproducer:

fn foo(mut name: String) {
    let parts = name.split(", ").collect::<Vec<_>>();
    let first = *parts.first().unwrap();
    name = first.to_owned();
    //first.clone_into(&mut name);
}

I guess that the issue is that in first.to_owned(), the first reference is not alive once name is reassigned (?). While in the second case, both references need to live during the method call, which causes the issue.

I have no idea how to detect these situations though. @blyxyas any ideas? :)

zaeleus added a commit to zaeleus/noodles that referenced this issue May 2, 2024
This was added in Rust 1.78.0 / clippy 0.1.78 (9b00956 2024-04-29).

The suggestion doesn't work in this scenario, given it's not possible to
borrow values from the same slice both mutably and immutably. See
<rust-lang/rust-clippy#12444>.
@y21
Copy link
Member

y21 commented May 2, 2024

I think for this we should be able to use PossibleBorrowerMap. That can tell us that first borrows from name and we can safely bail out if so.
I've been looking into this issue for a bit so I can give this a try.
@rustbot claim

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
C-bug Category: Clippy is not doing the correct thing I-suggestion-causes-error Issue: The suggestions provided by this Lint cause an ICE/error when applied
Projects
None yet
4 participants