needless_pass_by_value false positive when value used in async move block

[findepi]

Summary

needless_pass_by_value lint is triggered when a value is moved into a function and referenced in async move block.
If not for the async, it would be correct to flag this as needless_pass_by_value.

Lint Name

needless_pass_by_value

Reproducer

I tried this code:

use std::future::Future;

struct MyStruct {
    a: i32,
    b: i32,
}

fn main() {
    let my = MyStruct { a: 1, b: 2 };
    let _ = async_use(my);
}

fn async_use(my: MyStruct) -> impl Future<Output=i32> {
    async move {
        println!("a: {}, b: {}", my.a, my.b);
        10i32
    }
}

i run cargo clippy -- -Aclippy::all -Wclippy::needless_pass_by_value -D warnings

I saw this happen:

warning: this argument is passed by value, but not consumed in the function body
  --> src/main.rs:13:18
   |
13 | fn async_use(my: MyStruct) -> impl Future<Output=i32> {
   |                  ^^^^^^^^ help: consider taking a reference instead: `&MyStruct`
   |
help: consider marking this type as `Copy`
  --> src/main.rs:3:1
   |
3  | struct MyStruct {
   | ^^^^^^^^^^^^^^^
   = help: for further information visit https://rust-lang.github.io/rust-clippy/master/index.html#needless_pass_by_value
   = note: `-D clippy::needless-pass-by-value` implied by `-D warnings`
   = help: to override `-D warnings` add `#[allow(clippy::needless_pass_by_value)]`

however, if i follow the advice and make async_use take the reference, cargo build fails with

error[E0700]: hidden type for `impl Future<Output = i32>` captures lifetime that does not appear in bounds
  --> src/main.rs:14:5
   |
13 |   fn async_use(my: &MyStruct) -> impl Future<Output=i32> {
   |                    ---------     ----------------------- opaque type defined here
   |                    |
   |                    hidden type `{async block@src/main.rs:14:5: 17:6}` captures the anonymous lifetime defined here
14 | /     async move {
15 | |         println!("a: {}, b: {}", my.a, my.b);
16 | |         10i32
17 | |     }
   | |_____^
   |
help: to declare that `impl Future<Output = i32>` captures `'_`, you can add an explicit `'_` lifetime bound
   |
13 | fn async_use(my: &MyStruct) -> impl Future<Output=i32> + '_ {
   |                                                        ++++

For more information about this error, try `rustc --explain E0700`.

Version

rustc 1.80.1 (3f5fd8dd4 2024-08-06)
binary: rustc
commit-hash: 3f5fd8dd41153bc5fdca9427e9e05be2c767ba23
commit-date: 2024-08-06
host: aarch64-apple-darwin
release: 1.80.1
LLVM version: 18.1.7

Additional Labels

No response

[samueltardieu]

Did you apply both parts of the suggestion?

consider taking a reference instead: &MyStruct
consider marking this type as Copy

When I do, I don't get an error:

use std::future::Future;

#[derive(Clone, Copy)]
struct MyStruct {
    a: i32,
    b: i32,
}

fn main() {
    let my = MyStruct { a: 1, b: 2 };
    let _ = async_use(&my);
}

fn async_use(my: &MyStruct) -> impl Future<Output=i32> + '_ {
    async move {
        println!("a: {}, b: {}", my.a, my.b);
        10i32
    }
}

[findepi]

Yes, i noticed the hint about Copy but i considered it a hint. I didn't want to opt-in into implicit copying.

Also, without async move there is no needless_pass_by_value related warning. I couldn't understand why would async move require "less moving" and thus trigger needless_pass_by_value.

[samueltardieu]

Also, without async move there is no needless_pass_by_value
related warning. I couldn't understand why would async move require
"less moving" and thus trigger needless_pass_by_value.

What do you mean by "without async move"? What would be the code?

[findepi]

I was wrong in my last message.
Testing again this i see what is in the initial description:

If not for the async, it would be correct to flag this as needless_pass_by_value.

With async, the clippy suggests to use reference and make the struct Copy. Using reference requires lifetime hint '_ (as in #13321 (comment)), but then I can delete Copy trait.

If using Copy and capturing reference lifetime with '_ have now downsides and I should just follow the advise, please close this as not a false positive. Thanks!

@samueltardieu what happens if you modify your example from #13321 (comment) and remove #[derive(Clone, Copy)]? For me it still compiles, so maybe the suggestion to add Copy trait wasn't strictly needed?

[samueltardieu]

Yes, it does work without Copy. Maybe you want to open a separate issue to question the usefulness of this message.

Maybe you should also close this issue (I can't do it myself).

[findepi]

Sure, I can close. I didn't do that, because I still can't explain why it's not a false-negative.

Consider slightly more complex example

use std::future::Future;

struct MyStruct {
    a: i32,
    b: i32,
}

#[tokio::main]
async fn main() {
    println!("result {}", run().await);
}

fn run() -> impl Future<Output = i32> {
    let my = MyStruct { a: 1, b: 2 };
    async_use(my)
}

fn async_use(my: MyStruct) -> impl Future<Output = i32> {
    async move {
        println!("a: {}, b: {}", my.a, my.b);
        10i32
    }
}

To make clippy happy i will have to make my type Copy this time, since there is no lifetime such as '_ that would make this work. However, if i make my type Copy, there is no need for a reference, so the very first suggestion to use pass-by-reference isn't best. Also, I believe Copy has downsides, that's maybe why not everything is Copy by defauilt.

What I am getting at is -- I would expect that the code either conforms with clippy triggering no warnings (in particular needless_pass_by_value), or can be improved and made to conform with clippy. If this is not the case, I view this as a false-positive.

please let me know if this makes sense

[Jarcho]

The suggestion to implement Copy is an alternative to adding the reference, not a second part to the original suggestion.

This is a false positive on the lint since it returns the input by value meaning it's consumed by the function. You could do fn async_use<'a>(my: &'a MyStruct) -> impl 'a + Future<Output=i32>, but that isn't the same function.

[samueltardieu]

The suggestion to implement Copy is an alternative to adding the reference, not a second part to the original suggestion.

Independently as this FP, I'll open a PR to add a "or" before "consider marking this type as Copy"