Skip to content

needless_pass_by_ref_mut false positive because of type change #11182

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

Closed
smoelius opened this issue Jul 18, 2023 · 2 comments · Fixed by #11207
Closed

needless_pass_by_ref_mut false positive because of type change #11182

smoelius opened this issue Jul 18, 2023 · 2 comments · Fixed by #11207
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

@smoelius
Copy link
Contributor

smoelius commented Jul 18, 2023

Summary

A function may require an &mut argument so that it has a particular type.

Lint Name

needless_pass_by_ref_mut

Reproducer

I tried this code (note that foo needs to have the &mut argument to be passed to bar):

fn foo(y: &mut i32) -> i32 {
    12 + *y
}

fn bar(_: impl Fn(&mut i32) -> i32) {}

fn main() {
    bar(foo);
}

I saw this happen:

warning: this argument is a mutable reference, but not used mutably
 --> src/main.rs:1:11
  |
1 | fn foo(y: &mut i32) -> i32 {
  |           ^^^^^^^^ help: consider changing to: `&i32`
  |
  = help: for further information visit https://rust-lang.github.io/rust-clippy/master/index.html#needless_pass_by_ref_mut
  = note: `#[warn(clippy::needless_pass_by_ref_mut)]` on by default

I expected to see this happen: no warning.

Version

rustc 1.73.0-nightly (da6b55cc5 2023-07-17)
binary: rustc
commit-hash: da6b55cc5eaf76ed6acb7dc2f7d611e32af7c9a7
commit-date: 2023-07-17
host: x86_64-unknown-linux-gnu
release: 1.73.0-nightly
LLVM version: 16.0.5

Additional Labels

No response

@smoelius smoelius 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 19, 2023

cc @GuillaumeGomez

This would require some work, but should be possible to fix. My lint single_call_fn does something similar, use a visitor to see where the function's accessed and suppress the warning if it's passed to a function that expects a closure taking &mut.

Note that this would not cover cases where it's first assigned to a local however; you'd need to visit local usage too then, but that's likely rare enough that it's not really too big of a deal (perhaps it could just be noted in "Known issues")

You'd likely want to store the function LocalDefIds in the lint's struct then have a check_crate_post to get both the usage and to do the linting

(PS: I think this will need span_lint_hir_and_then! I need to fix that on my lint...)

@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 25, 2023
[`needless_pass_by_ref_mut`]: Do not lint if passed as a fn-like argument

Fixes #11182 and also fixes #11199 (though this is kind of a duplicate)

There's likely a case or two I've missed, so this likely needs a bit more work but it seems to work fine with the tests I've added.

PS, the diff for the test is useless because it iterates over a hashmap before linting. Seems to work fine but we could maybe change this for consistency's sake

changelog: [`needless_pass_by_ref_mut`]: No longer lints if the function is passed as a fn-like argument
@bors bors closed this as completed in 43b0e11 Jul 25, 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.

4 participants