clippy: Removes redundant async blocks

[brooksprumo]

Problem

Clippy throws warnings on the next nightly version (1.70.0), which blocks upgrading our nightly Rust (see #31381).

Here's an example:

warning: this async expression only awaits a single future
    --> rpc/src/rpc.rs:3522:22
     |
3522 |             Box::pin(async move { meta.get_signature_statuses(signatures, config).await })
     |                      ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ help: you can reduce it to: `meta.get_signature_statuses(signatures, config)`
     |
     = help: for further information visit https://rust-lang.github.io/rust-clippy/master/index.html#redundant_async_block
     = note: `#[warn(clippy::redundant_async_block)]` on by default

Summary of Changes

Remove redundant async away blocks

[codecov]

Codecov Report

Merging #31526 (61f3fe2) into master (775639c) will decrease coverage by 0.1%.
The diff coverage is 85.7%.

@@            Coverage Diff            @@
##           master   #31526     +/-   ##
=========================================
- Coverage    81.4%    81.3%   -0.1%     
=========================================
  Files         731      731             
  Lines      205088   205084      -4     
=========================================
- Hits       166944   166914     -30     
- Misses      38144    38170     +26     

[CriesofCarrots]

Out of curiosity, why is the example in the description not one of the instances fixed in this PR?

[brooksprumo]

Out of curiosity, why is the example in the description not one of the instances fixed in this PR?

Heh, good eye! Here's the output from running clippy --fix:

warning: `solana-rpc` (lib) generated 13 warnings (run `cargo clippy --fix --lib -p solana-rpc` to apply 13 suggestions)
warning: failed to automatically apply fixes suggested by rustc to crate `solana_rpc`

after fixes were automatically applied the compiler reported errors within these files:

  * rpc/src/rpc.rs

This likely indicates a bug in either rustc or cargo itself,
and we would appreciate a bug report! You're likely to see 
a number of compiler warnings after this message which cargo
attempted to fix but failed. If you could open an issue at
https://github.com/rust-lang/rust/issues
quoting the full output of this command we'd be very appreciative!
Note that you may be able to make some more progress in the near-term
fixing code with the `--broken-code` flag

There were a few files like this too. So looks like rpc.rs didn't end up modified (on disk) due to the bug and thus not added to this commit. I used that one as the example in the PR description since it was at the bottom of the output and one of the first I saw.

Do you think I should manually add the changes from rpc.rs, or better to handle them separately?

[brooksprumo]

Does this change affect behavior? I see we grab an async semaphore at the top of the function; does not running the send_data_async in an async block break anything?

[alessandrod]

No it does not

async fn send_data_async(
    connection: Arc<NonblockingQuicConnection>,
    buffer: Vec<u8>,
) -> TransportResult<()> {

Since send_data_async is an async fn, when you call it what happens is that it builds its return future and returns it. It only builds it tho, it does not poll it. Futures don't make progress until they're polled, so effectively the future will start executing only inside the task it's spawned on.

It would be different if send_data_async built the future manually, eg:

fn send_data_async(
    connection: Arc<NonblockingQuicConnection>,
    buffer: Vec<u8>,
) -> SendDataFuture {
    do_something_before_creating_the_future();
    SendDataFuture {
       ...
    }
}

In this case do_something_before_creating_the_future does execute before the resulting future is polled. So before landing this we need to double check if we're creating any manual futures, and if we're doing actual work (eg locking things) before returning them.

[brooksprumo]

Thanks for the explanation!

I went through and audited all functions that were inside the async { .await } blocks, and they all were async fn (i.e. not creating any manual futures. So I think that means this PR is ok.

[brooksprumo]

I plan on merging this PR tomorrow unless there are any objections.

[CriesofCarrots]

Do you think I should manually add the changes from rpc.rs, or better to handle them separately?

Either is fine with me.

[ilya-bobyr]

I've made a similar fix here: #30808
There were cases that clippy flagged incorrectly.
It seems to be missing implicitly captured values (maybe only some?) in the analysis.
In particular, in rpc.rs.
I had to apply exceptions in a few cases.
In my PR, I could not enable clippy exceptions, as they are only supported starting at 1.69.
But as we are now on 1.69, it should be possible.

I wonder if after applying this PR you actually see all the Clippy warnings about redundant async/await really disappearing.
Just as Tyera pointed out, the warning you used in the PR description is not addressed in the patch. Is it gone after applying this patch?

[brooksprumo]

I wonder if after applying this PR you actually see all the Clippy warnings about redundant async/await really disappearing. Just as Tyera pointed out, the warning you used in the PR description is not addressed in the patch. Is it gone after applying this patch?

I would expect the same warnings to still exist in rpc.rs, since they are not addressed in this PR.