warn on must_use use on async fn's #89610

guswynn · 2021-10-06T20:24:58Z

As referenced in #78149

This only works on async fn's for now, I can also look into if I can get Box<dyn Future> and impl Future working at this level (hir)

rust-highfive · 2021-10-06T20:25:32Z

r? @wesleywiser

(rust-highfive has picked a reviewer for you, use r? to override)

compiler/rustc_passes/src/check_attr.rs

wesleywiser · 2021-10-13T02:59:28Z

cc @eholk since this is an async-polish item

Before r+ing, I need to double check if we need to get the ok from the lang team before adding new warnings

eholk · 2021-10-18T21:12:03Z

Looks good to me!

wesleywiser · 2021-10-19T15:35:11Z

Nominating for lang team discussion. This PR adds a new warning that detects #[must_use] attributes applied to async fns which currently does nothing.

warning: `must_use` attribute on `async` functions applies to the anonymous Future returned by the function, not the value within.
  --> $DIR/unused-async.rs:5:1
   |
LL |   #[must_use]
   |   ^^^^^^^^^^^
LL |
LL | / async fn test() -> i32 {
LL | |     1
LL | | }
   | |_- this attribute does nothing, the Futures returned by async functions are already `must_use`

Mark-Simulacrum · 2021-10-19T15:45:08Z

I suspect this might just be an oversight in this PR, but it seems like this should be an unused_attributes lint, not an unconditional warning (and a custom message is still fine).

If so, then I suspect it need not receive T-lang approval... though that is a little murky these days :)

Mark-Simulacrum · 2021-10-19T17:49:29Z

T-lang discussed this in today's meeting and agreed that this can move ahead. We may eventually want to make the must_use have behavior related to return of the .await'ing the resulting future, but that's not happening immediately (if ever), so we can move ahead here in the meantime.

We briefly discussed my concern around unused_attributes and generally felt positive about its use here, I think.

guswynn · 2021-10-29T22:23:27Z

@wesleywiser what review is outstanding?

wesleywiser · 2021-11-04T16:37:57Z

@guswynn I believe this is still waiting on Mark's feedback to be addressed:

it seems like this should be an unused_attributes lint, not an unconditional warning (and a custom message is still fine).

guswynn · 2021-11-08T21:27:45Z

@wesleywiser sorry about the delay, was on a break, this should now be ready!

wesleywiser

Looks great, thanks @guswynn!

wesleywiser · 2021-11-13T01:19:18Z

@bors r+

bors · 2021-11-13T01:19:20Z

📌 Commit 958de5a has been approved by wesleywiser

warn on must_use use on async fn's As referenced in rust-lang#78149 This only works on `async` fn's for now, I can also look into if I can get `Box<dyn Future>` and `impl Future` working at this level (hir)

compiler/rustc_passes/src/check_attr.rs

src/test/ui/lint/unused/unused-async.stderr

Co-authored-by: Yuki Okushi <jtitor@2k36.org>

wesleywiser · 2021-11-16T18:31:25Z

Blessed tests.

@bors r+

bors · 2021-11-16T18:31:26Z

📌 Commit 83ce771 has been approved by wesleywiser

warn on must_use use on async fn's As referenced in rust-lang#78149 This only works on `async` fn's for now, I can also look into if I can get `Box<dyn Future>` and `impl Future` working at this level (hir)

…askrgr Rollup of 8 pull requests Successful merges: - rust-lang#89610 (warn on must_use use on async fn's) - rust-lang#90667 (Improve diagnostics when a static lifetime is expected) - rust-lang#90687 (Permit const panics in stable const contexts in stdlib) - rust-lang#90772 (Add Vec::retain_mut) - rust-lang#90861 (Print escaped string if char literal has multiple characters, but only one printable character) - rust-lang#90884 (Fix span for non-satisfied trivial trait bounds) - rust-lang#90900 (Remove workaround for the forward progress handling in LLVM) - rust-lang#90901 (Improve ManuallyDrop suggestion) Failed merges: r? `@ghost` `@rustbot` modify labels: rollup

guswynn · 2021-11-23T04:04:22Z

@wesleywiser you are a legend for blessing those tests, sorry this fell off my radar!

wesleywiser · 2021-11-23T17:56:52Z

@guswynn Thanks for working on it!! 🎉

The Future type that an async function implicitly returns is already implicitly marked as must_use, and additional must_use annotations on async functions are, therefore, redundant. These unneeded annotations generate a warning, which is an error in our build, with this change to the rust compiler rust-lang/rust#89610 Bug: 88999 Change-Id: I84685942c30eeaf98ad9597285d289d5bcbc12d3 Reviewed-on: https://fuchsia-review.googlesource.com/c/fuchsia/+/608581 Reviewed-by: Abdulla Kamar <abdulla@google.com> Fuchsia-Auto-Submit: Adrian Danis <adanis@google.com> Commit-Queue: Auto-Submit <auto-submit@fuchsia-infra.iam.gserviceaccount.com>

rust-highfive assigned wesleywiser Oct 6, 2021

rust-highfive added the S-waiting-on-review Status: Awaiting review from the assignee but also interested parties. label Oct 6, 2021

wesleywiser reviewed Oct 13, 2021

View reviewed changes

compiler/rustc_passes/src/check_attr.rs Outdated Show resolved Hide resolved

wesleywiser added the AsyncAwait-Polish Async-await issues that are part of the "polish" area label Oct 13, 2021

apiraino added the T-compiler Relevant to the compiler team, which will review and decide on the PR/issue. label Oct 14, 2021

wesleywiser added I-nominated T-lang Relevant to the language team, which will review and decide on the PR/issue. and removed T-compiler Relevant to the compiler team, which will review and decide on the PR/issue. labels Oct 19, 2021

Mark-Simulacrum removed the I-nominated label Oct 19, 2021

wesleywiser added S-waiting-on-author Status: This is awaiting some action (such as code changes or more information) from the author. and removed S-waiting-on-review Status: Awaiting review from the assignee but also interested parties. labels Oct 25, 2021

This comment has been minimized.

Sign in to view

warn on must_use use on async fn's

958de5a

guswynn force-pushed the must_use_future branch from f61d916 to 958de5a Compare November 8, 2021 22:41

wesleywiser approved these changes Nov 13, 2021

View reviewed changes

bors added S-waiting-on-bors Status: Waiting on bors to run and complete tests. Bors will change the label on completion. and removed S-waiting-on-author Status: This is awaiting some action (such as code changes or more information) from the author. labels Nov 13, 2021

matthiaskrgr mentioned this pull request Nov 13, 2021

Rollup of 5 pull requests #90858

Closed

matthiaskrgr mentioned this pull request Nov 15, 2021

Rollup of 13 pull requests #90926

Closed

JohnTitor reviewed Nov 16, 2021

View reviewed changes

compiler/rustc_passes/src/check_attr.rs Outdated Show resolved Hide resolved

wesleywiser reviewed Nov 16, 2021

View reviewed changes

src/test/ui/lint/unused/unused-async.stderr Outdated Show resolved Hide resolved

src/test/ui/lint/unused/unused-async.stderr Outdated Show resolved Hide resolved

Update compiler/rustc_passes/src/check_attr.rs

83ce771

Co-authored-by: Yuki Okushi <jtitor@2k36.org>

wesleywiser force-pushed the must_use_future branch from e75a0ad to 83ce771 Compare November 16, 2021 16:44

matthiaskrgr mentioned this pull request Nov 16, 2021

Rollup of 10 pull requests #90963

Closed

matthiaskrgr mentioned this pull request Nov 16, 2021

Rollup of 10 pull requests #90965

Closed

matthiaskrgr mentioned this pull request Nov 16, 2021

Rollup of 6 pull requests #90967

Closed

matthiaskrgr mentioned this pull request Nov 17, 2021

Rollup of 7 pull requests #90975

Closed

matthiaskrgr mentioned this pull request Nov 17, 2021

Rollup of 8 pull requests #90984

Merged

bors merged commit 0734282 into rust-lang:master Nov 17, 2021

rustbot added this to the 1.58.0 milestone Nov 17, 2021

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

warn on must_use use on async fn's #89610

warn on must_use use on async fn's #89610

guswynn commented Oct 6, 2021

rust-highfive commented Oct 6, 2021

wesleywiser commented Oct 13, 2021 •

edited

Loading

eholk commented Oct 18, 2021

wesleywiser commented Oct 19, 2021

Mark-Simulacrum commented Oct 19, 2021

Mark-Simulacrum commented Oct 19, 2021

guswynn commented Oct 29, 2021

wesleywiser commented Nov 4, 2021

guswynn commented Nov 8, 2021

This comment has been minimized.

wesleywiser left a comment

wesleywiser commented Nov 13, 2021

bors commented Nov 13, 2021

wesleywiser commented Nov 16, 2021

bors commented Nov 16, 2021

guswynn commented Nov 23, 2021

wesleywiser commented Nov 23, 2021

warn on must_use use on async fn's #89610

warn on must_use use on async fn's #89610

Conversation

guswynn commented Oct 6, 2021

rust-highfive commented Oct 6, 2021

wesleywiser commented Oct 13, 2021 • edited Loading

eholk commented Oct 18, 2021

wesleywiser commented Oct 19, 2021

Mark-Simulacrum commented Oct 19, 2021

Mark-Simulacrum commented Oct 19, 2021

guswynn commented Oct 29, 2021

wesleywiser commented Nov 4, 2021

guswynn commented Nov 8, 2021

This comment has been minimized.

wesleywiser left a comment

Choose a reason for hiding this comment

wesleywiser commented Nov 13, 2021

bors commented Nov 13, 2021

wesleywiser commented Nov 16, 2021

bors commented Nov 16, 2021

guswynn commented Nov 23, 2021

wesleywiser commented Nov 23, 2021

wesleywiser commented Oct 13, 2021 •

edited

Loading