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 in closure in async #11380

Closed
mkroening opened this issue Aug 22, 2023 · 7 comments · Fixed by #11492
Closed

needless_pass_by_ref_mut false positive in closure in async #11380

mkroening opened this issue Aug 22, 2023 · 7 comments · Fixed by #11492
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

Comments

@mkroening
Copy link

mkroening commented Aug 22, 2023

Summary

Another variant of #11179 and #11299.

Found here: hermit-os/kernel/src/fd/socket/tcp.rs#L69-L94

Lint Name

needless_pass_by_ref_mut

Reproducer

I tried this code:

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

I saw this happen:

warning: this argument is a mutable reference, but not used mutably
 --> foo.rs:1:17
  |
1 | async fn foo(x: &mut 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 needless_pass_by_ref_mut.

Version

rustc 1.74.0-nightly (ef85656a1 2023-08-21)
binary: rustc
commit-hash: ef85656a10657ba5e4f7fe2931a4ca6293138d51
commit-date: 2023-08-21
host: aarch64-apple-darwin
release: 1.74.0-nightly
LLVM version: 17.0.0

Additional Labels

No response

@mkroening mkroening 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 Aug 22, 2023
@mkroening
Copy link
Author

@GuillaumeGomez, would this be important to fix before the lint lands in stable? I am not sure about the release process, but it sounded to me like it would be relevant for beta: #11314 (comment)

@GuillaumeGomez
Copy link
Member

It'd be better indeed. I started looking at it but I'm a bit under the water currently, too many "more pressing" things to do. Hopefully I should be able to get back to it this week-end or next week.

@fasterthanlime
Copy link

Caught this in real-world code yesterday:

https://github.com/hapsoc/hring/blob/a2ffb643447b1e4bcdb673f3837b67cf3e02a19c/crates/hring/src/h1/client.rs#L34

(Not a showstopper, just thought I'd add a data point!)

@GuillaumeGomez
Copy link
Member

I really need to find time to fix this...

@fasterthanlime
Copy link

I really need to find time to fix this...

@GuillaumeGomez is this a good first issue? Is there a place one could get guidance to fix this?

@GuillaumeGomez
Copy link
Member

Honestly I don't think it's a good first issue at all. You can see the code here. If you want to start contributing on clippy, I'll be more than happy to help getting started but in this case, the problem is that the code generated by rustc for async is quite tricky to handle, especially variables being passed through which change IDs etc. Lots of fun. :3

@GuillaumeGomez
Copy link
Member

I opened #11492 which fixes this issue.

@bors bors closed this as completed in ece3878 Sep 20, 2023
nickjiang2378 pushed a commit to nickjiang2378/hydroflow that referenced this issue Jan 24, 2024
nickjiang2378 pushed a commit to nickjiang2378/hydroflow that referenced this issue Jan 25, 2024
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
Projects
None yet
3 participants