Skip to content

Conversation

samueltardieu
Copy link
Member

@samueltardieu samueltardieu commented Mar 26, 2023

This lets us detect more complex situations: async { x.await } is simplified into x if:

  • x is an expression without side-effect
  • or x is an async block itself

In both cases, no part of the async expression can be part of a macro expansion.

Fixes #10509.
Fixes #10525.

changelog: [redundant_async_block] Do not lint expressions with side effects.

@rustbot
Copy link
Collaborator

rustbot commented Mar 26, 2023

r? @Jarcho

(rustbot has picked a reviewer for you, use r? to override)

@rustbot rustbot added the S-waiting-on-review Status: Awaiting review from the assignee but also interested parties label Mar 26, 2023
Copy link
Contributor

@Jarcho Jarcho left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Sorry for the wait. Just one suggested improvement and then a couple of nits.

@samueltardieu samueltardieu force-pushed the redundant-async-block branch 3 times, most recently from 44caf1b to 66b9d30 Compare April 3, 2023 07:18
@Jarcho
Copy link
Contributor

Jarcho commented Apr 5, 2023

Just needs an in_eternal_macro check and then this is good. Can you check to see if it still fixes #10525?

This lets us detect more complex situations: `async { x.await }` is
simplified into `x` if:

- `x` is an expression without side-effect
- or `x` is an async block itself

In both cases, no part of the `async` expression can be part of a macro
expansion.
@samueltardieu samueltardieu force-pushed the redundant-async-block branch from 66b9d30 to 2891d8f Compare April 5, 2023 08:12
@samueltardieu
Copy link
Member Author

Just needs an in_eternal_macro check and then this is good. Can you check to see if it still fixes #10525?

Done for in_external_macro(). I confirm that it still fixes #10525, which was already mitigated by #10490.

@Jarcho
Copy link
Contributor

Jarcho commented Apr 5, 2023

Thanks for fixing the lint. @bors r+

@bors
Copy link
Contributor

bors commented Apr 5, 2023

📌 Commit 2891d8f has been approved by Jarcho

It is now in the queue for this repository.

@bors
Copy link
Contributor

bors commented Apr 5, 2023

⌛ Testing commit 2891d8f with merge 6a4e7d4...

bors added a commit that referenced this pull request Apr 5, 2023
Make redundant_async_block a more complete late pass

This lets us detect more complex situations: `async { x.await }` is simplified into `x` if:

- `x` is an expression without side-effect
- or `x` is an `async` block itself

In both cases, no part of the `async` expression can be part of a macro expansion.

Fixes #10509.
Fixes #10525.

changelog: [`redundant_async_block`] Do not lint expressions with side effects.
@bors
Copy link
Contributor

bors commented Apr 5, 2023

💔 Test failed - checks-action_test

@Jarcho
Copy link
Contributor

Jarcho commented Apr 5, 2023

@bors retry

@bors
Copy link
Contributor

bors commented Apr 5, 2023

⌛ Testing commit 2891d8f with merge 26e245d...

@bors
Copy link
Contributor

bors commented Apr 5, 2023

☀️ Test successful - checks-action_dev_test, checks-action_remark_test, checks-action_test
Approved by: Jarcho
Pushing 26e245d to master...

1 similar comment
@bors
Copy link
Contributor

bors commented Apr 5, 2023

☀️ Test successful - checks-action_dev_test, checks-action_remark_test, checks-action_test
Approved by: Jarcho
Pushing 26e245d to master...

@bors bors merged commit 26e245d into rust-lang:master Apr 5, 2023
@samueltardieu samueltardieu deleted the redundant-async-block branch November 29, 2023 22:41
gnoliyil pushed a commit to gnoliyil/fuchsia that referenced this pull request Jan 27, 2024
This reverts commit 2ca919d5593244ee9ff97cc0805de16c04e3351a.

Reason for revert: The lint was fixed by rust-lang/rust-clippy#10554
Fixed: 123778

Original change's description:
> [rust] Add #![allow(clippy::redundant_async_block)]
>
> This CL adds attributes to allow the new redundant_async_block clippy
> lint to unblock the toolchain roll. It only adds it to affected crates.
> It also adds #![allow(unknown_lints)] which can be removed as soon as
> the toolchain rolls.
>
> We are not fixing the lints right now because there are false positives:
>
> rust-lang/rust-clippy#10482
> rust-lang/rust-clippy#10509
>
> Once those are resolved, we can remove these attributes and fix the
> occurrences or add more narrowly scoped attributes.
>
> Test: fx clippy --all --raw # tested with old and new toolchain
> Bug: 123528
> Bug: 123778
> Change-Id: If874cae8621b9469b9fde0cb6220250710c8848c
> Reviewed-on: https://fuchsia-review.googlesource.com/c/fuchsia/+/820449
> Reviewed-by: Dan Johnson <[email protected]>
> Reviewed-by: Tyler Mandry <[email protected]>
> Commit-Queue: Mitchell Kember <[email protected]>

Bug: 123528
Bug: 123778
Change-Id: I3385740f5b8d5205919a5c2a41c1eb584791fa88
Reviewed-on: https://fuchsia-review.googlesource.com/c/fuchsia/+/834979
Reviewed-by: Mitchell Kember <[email protected]>
Reviewed-by: Tyler Mandry <[email protected]>
Reviewed-by: Dan Johnson <[email protected]>
Commit-Queue: Joseph Ryan <[email protected]>
ephemeralriggs pushed a commit to google/omaha-client that referenced this pull request Feb 19, 2024
This reverts commit 4531398.

Reason for revert: The lint was fixed by rust-lang/rust-clippy#10554
Fixed: 123778

Original change's description:
> [rust] Add #![allow(clippy::redundant_async_block)]
>
> This CL adds attributes to allow the new redundant_async_block clippy
> lint to unblock the toolchain roll. It only adds it to affected crates.
> It also adds #![allow(unknown_lints)] which can be removed as soon as
> the toolchain rolls.
>
> We are not fixing the lints right now because there are false positives:
>
> rust-lang/rust-clippy#10482
> rust-lang/rust-clippy#10509
>
> Once those are resolved, we can remove these attributes and fix the
> occurrences or add more narrowly scoped attributes.
>
> Test: fx clippy --all --raw # tested with old and new toolchain
> Bug: 123528
> Bug: 123778
> Change-Id: If874cae8621b9469b9fde0cb6220250710c8848c
> Reviewed-on: https://fuchsia-review.googlesource.com/c/fuchsia/+/820449
> Reviewed-by: Dan Johnson <[email protected]>
> Reviewed-by: Tyler Mandry <[email protected]>
> Commit-Queue: Mitchell Kember <[email protected]>

Bug: 123528
Bug: 123778
Change-Id: I3385740f5b8d5205919a5c2a41c1eb584791fa88
Reviewed-on: https://fuchsia-review.googlesource.com/c/fuchsia/+/834979
Reviewed-by: Mitchell Kember <[email protected]>
Reviewed-by: Tyler Mandry <[email protected]>
Reviewed-by: Dan Johnson <[email protected]>
Commit-Queue: Joseph Ryan <[email protected]>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
S-waiting-on-review Status: Awaiting review from the assignee but also interested parties
Projects
None yet
Development

Successfully merging this pull request may close these issues.

redundant_async_block creates invalid code in async tests redundant_async_block can subtly change behavior
4 participants