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

needless_pass_by_ref_mut false positive #11179

Closed
iamsauravsharma opened this issue Jul 18, 2023 · 6 comments · Fixed by #11184
Closed

needless_pass_by_ref_mut false positive #11179

iamsauravsharma opened this issue Jul 18, 2023 · 6 comments · Fixed by #11184
Labels
C-bug Category: Clippy is not doing the correct thing I-false-positive Issue: The lint was triggered on code it shouldn't have I-suggestion-causes-error Issue: The suggestions provided by this Lint cause an ICE/error when applied

Comments

@iamsauravsharma
Copy link

Summary

I got

error: this argument is a mutable reference, but not used mutably
  --> src/migrator/any.rs:22:17
   |
22 |     connection: &mut <Any as sqlx::Database>::Connection,
   |                 ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ help: consider changing to: `&<Any as sqlx::Database>::Connection`
   |
   = help: for further information visit https://rust-lang.github.io/rust-clippy/master/index.html#needless_pass_by_ref_mut
note: the lint level is defined here
  --> src/lib.rs:3:9
   |
3  | #![deny(clippy::all)]
   |         ^^^^^^^^^^^
   = note: `#[deny(clippy::needless_pass_by_ref_mut)]` implied by `#[deny(clippy::all)]`

even when connection is used as mutable https://github.com/iamsauravsharma/sqlx_migrator/blob/master/src/migrator/any.rs#L36
GitHub actions error https://github.com/iamsauravsharma/sqlx_migrator/actions/runs/5583309753/jobs/10203493083

Lint Name

needless_pass_by_ref_mut

Reproducer

  1. clone repo at https://github.com/iamsauravsharma/sqlx_migrator
  2. run cargo clippy --features any,postgres you will see clippy error
error: this argument is a mutable reference, but not used mutably
  --> src/migrator/any.rs:22:17
   |
22 |     connection: &mut <Any as sqlx::Database>::Connection,
   |                 ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ help: consider changing to: `&<Any as sqlx::Database>::Connection`
   |
   = help: for further information visit https://rust-lang.github.io/rust-clippy/master/index.html#needless_pass_by_ref_mut

Expects no error since connection is used mutably and using as suggestion doesn't compile code

Version

rustc 1.73.0-nightly (da6b55cc5 2023-07-17)
binary: rustc
commit-hash: da6b55cc5eaf76ed6acb7dc2f7d611e32af7c9a7
commit-date: 2023-07-17
host: aarch64-apple-darwin
release: 1.73.0-nightly
LLVM version: 16.0.5

Additional Labels

No response

@iamsauravsharma iamsauravsharma added C-bug Category: Clippy is not doing the correct thing I-false-positive Issue: The lint was triggered on code it shouldn't have labels Jul 18, 2023
@Centri3
Copy link
Member

Centri3 commented Jul 18, 2023

cc @GuillaumeGomez

@y21
Copy link
Member

y21 commented Jul 18, 2023

Minimal reproducible example:

async fn f(x: &mut i32) {
  *x += 1;
}

Looks like it doesn't work correctly in async fns.

@iamsauravsharma
Copy link
Author

iamsauravsharma commented Jul 18, 2023

Also, It works properly if you add extra parameter which are not stored in heap? (not sure)

// no issue
async fn _f(x: &mut i32, _p: i32) {
    *x += 1;
}

and doesn't work with heap based parameter? (not sure)

async fn _g(x: &mut i32, _p: String) {
    *x += 1;
}

async fn _h(x: &mut i32, _p: Vec<i32>, _q: Box<u64>) {
    *x += 1;
}

@GuillaumeGomez
Copy link
Member

Thanks! Taking a look.

@GuillaumeGomez
Copy link
Member

I opened #11184 to fix this issue.

@taiki-e
Copy link
Member

taiki-e commented Jul 22, 2023

@rustbot label +I-suggestion-causes-error

@rustbot rustbot added the I-suggestion-causes-error Issue: The suggestions provided by this Lint cause an ICE/error when applied label Jul 22, 2023
bors added a commit that referenced this issue Jul 22, 2023
… r=llogiq

Fix async functions handling for `needless_pass_by_ref_mut` lint

Fixes #11179.

The problem with async is that "internals" are actually inside a closure from the `ExprUseVisitor` point of view, meaning we need to actually run the check on the closures' body as well.

r? `@llogiq`
@bors bors closed this as completed in 43577d5 Jul 23, 2023
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-false-positive Issue: The lint was triggered on code it shouldn't have I-suggestion-causes-error Issue: The suggestions provided by this Lint cause an ICE/error when applied
Projects
None yet
Development

Successfully merging a pull request may close this issue.

6 participants