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

Can't use swap or replace as suggested by manual_swap #981

Closed
mcarton opened this issue Jun 5, 2016 · 2 comments · Fixed by #4478
Closed

Can't use swap or replace as suggested by manual_swap #981

mcarton opened this issue Jun 5, 2016 · 2 comments · Fixed by #4478
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 L-suggestion Lint: Improving, adding or fixing lint suggestions

Comments

@mcarton
Copy link
Member

mcarton commented Jun 5, 2016

On the following:

fn main() {
    let mut foo = [1, 2];
    let temp = foo[0];
    foo[0] = foo[1];
    foo[1] = temp;
}

Clippy suggests std::mem::swap(&mut foo[0], &mut foo[1]);, but this would mutably borrow foo twice. The correct suggestion for slices would be foo.swap(0, 1). For other types, we should check that the suggestion would not mutably borrow something twice somehow.

@mcarton mcarton added the C-bug Category: Clippy is not doing the correct thing label Jun 5, 2016
@mcarton
Copy link
Member Author

mcarton commented Jun 5, 2016

Oh wait, actually the full suggestion is: → fixed in #982

let std::mem::swap(&mut foo[0], &mut foo[1]);
^^^

@Manishearth I there a trick with compiletest-rs to check that there is nothing else before/after the suggestion? → fixed in #983

@thomcc
Copy link
Member

thomcc commented Aug 15, 2019

I have the related issue where clippy wants me to use mem::swap instead of my manual swap. It's a field through a slice, so it's the same kind of problem:

let id = tris[a].id;
tris[a].id = tris[b].id;
tris[b].id = id;

@flip1995 flip1995 added the I-suggestion-causes-error Issue: The suggestions provided by this Lint cause an ICE/error when applied label Aug 15, 2019
@phansch phansch added the L-suggestion Lint: Improving, adding or fixing lint suggestions label Aug 24, 2019
bors added a commit that referenced this issue Sep 11, 2019
Fix incorrect swap suggestion

Clippy suggests using swap on fields belonging to the same owner causing two mutable borrows of the owner.

Disclosure: This is my first time working with clippy and rusts HIR. I'm very grateful for assistance and suggestions to improve the PR.

fixes #981
changelog: none
bors added a commit that referenced this issue Sep 11, 2019
Fix incorrect swap suggestion

Clippy suggests using swap on fields belonging to the same owner causing two mutable borrows of the owner.

Disclosure: This is my first time working with clippy and rusts HIR. I'm very grateful for assistance and suggestions to improve the PR.

fixes #981
changelog: Fix false positive in `manual_swap` lint
bors added a commit that referenced this issue Sep 11, 2019
Fix incorrect swap suggestion

Clippy suggests using swap on fields belonging to the same owner causing two mutable borrows of the owner.

Disclosure: This is my first time working with clippy and rusts HIR. I'm very grateful for assistance and suggestions to improve the PR.

fixes #981
changelog: Fix false positive in `manual_swap` lint
@bors bors closed this as completed in 8bc1ded Sep 11, 2019
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 L-suggestion Lint: Improving, adding or fixing lint suggestions
Projects
None yet
Development

Successfully merging a pull request may close this issue.

4 participants