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

#[must_use] on async fns works on returned Future instead of the awaited value #78149

Closed
ghost opened this issue Oct 20, 2020 · 10 comments · Fixed by #100633
Closed

#[must_use] on async fns works on returned Future instead of the awaited value #78149

ghost opened this issue Oct 20, 2020 · 10 comments · Fixed by #100633
Assignees
Labels
A-async-await Area: Async & Await A-attributes Area: #[attributes(..)] A-lint Area: Lints (warnings about flaws in source code) such as unused_mut. AsyncAwait-Polish Async-await issues that are part of the "polish" area AsyncAwait-Triaged Async-await issues that have been triaged during a working group meeting. C-bug Category: This is a bug. P-medium Medium priority T-compiler Relevant to the compiler team, which will review and decide on the PR/issue.

Comments

@ghost
Copy link

ghost commented Oct 20, 2020

I tried this code:

#[must_use]
async fn foo() -> i32 {
    std::future::pending().await
}

pub fn with_warnings() {
    foo();
}

pub async fn without_warning() {
    foo().await;
}

I expected the compiler to warn about the unused return value in without_warning(), but the compiler emitted two warnings in with_warnings():

warning: unused implementer of `Future` that must be used
 --> src/lib.rs:7:5
  |
7 |     foo();
  |     ^^^^^^
  |
  = note: `#[warn(unused_must_use)]` on by default
  = note: futures do nothing unless you `.await` or poll them

warning: unused return value of `foo` that must be used
 --> src/lib.rs:7:5
  |
7 |     foo();
  |     ^^^^^^

Playground (tried using nightly 2020-10-18 b1496c6)

As the real return values of async fns are actually impl Futures, you may consider this as the expected behavior (I don't agree, and impl Futures are already #[must_use]). If that is the case, there should be a way to make the compiler warn on unused .awaited values (when type of the .awaited value is not #[must_use]).

@rustbot modify labels: A-async-await A-lint -A-diagnostics

@ghost ghost added the C-bug Category: This is a bug. label Oct 20, 2020
@rustbot rustbot added A-async-await Area: Async & Await A-diagnostics Area: Messages for errors, warnings, and lints labels Oct 20, 2020
@tmandry
Copy link
Contributor

tmandry commented Oct 20, 2020

I think this is indeed a bug, or at the very least unexpected behavior. It might be challenging to fix, since I don't know of a way to express the desired desugaring.

@tmandry tmandry added this to On deck in wg-async work via automation Oct 20, 2020
@tmandry tmandry added I-prioritize Indicates that prioritization has been requested for this issue A-attributes Area: #[attributes(..)] AsyncAwait-Triaged Async-await issues that have been triaged during a working group meeting. labels Oct 20, 2020
@nikomatsakis
Copy link
Contributor

I agree that this will be challenging. The first problem as you said is that, in the desugaring, the #[must_use] is being applied to the function that returns the impl Future, and we don't have a clear place where it would be applied to mean "the value awaited by this future". So that's something we'd have to teach the lint somehow.

Second problem then would be that the lint would have to have the idea of a "must use future" (let's call it), and it would have to track when that future is awaited and figure out that the resulting value is itself #[must_use].

I'm pondering if there is a generalization that might make this easier and less special case, not clear.

@tmandry
Copy link
Contributor

tmandry commented Oct 20, 2020

In the short term we could make a lint to warn people who use the attribute on async fns that it won't do anything.

@camelid
Copy link
Member

camelid commented Oct 20, 2020

Assigning P-medium and removing I-prioritize as discussed in the prioritization working group.

@camelid camelid added P-medium Medium priority and removed I-prioritize Indicates that prioritization has been requested for this issue labels Oct 20, 2020
@JohnTitor JohnTitor added the T-compiler Relevant to the compiler team, which will review and decide on the PR/issue. label Oct 21, 2020
@rustbot rustbot added A-lint Area: Lints (warnings about flaws in source code) such as unused_mut. and removed A-diagnostics Area: Messages for errors, warnings, and lints labels Oct 21, 2020
@jinpan
Copy link

jinpan commented Jul 11, 2021

A hacky workaround for this is to wrap the return type inside a Result:

async fn bar() -> Result<(), i32> {
    std::future::pending().await
}

pub async fn async_with_warnings() {
    bar().await;
}

and the compiler will emit warnings for the unused result.

Playground link

@guswynn
Copy link
Contributor

guswynn commented Sep 30, 2021

@rustbot claim

At least "In the short term we could make a lint to warn people who use the attribute on async fns that it won't do anything." part

@eholk
Copy link
Contributor

eholk commented Oct 5, 2021

@rustbot label AsyncAwait-Polish

@rustbot
Copy link
Collaborator

rustbot commented Oct 5, 2021

Error: Label AsyncAwait-Polish can only be set by Rust team members

Please let @rust-lang/release know if you're having trouble with this bot.

@tmandry
Copy link
Contributor

tmandry commented Oct 6, 2021

@rustbot label AsyncAwait-Polish

@rustbot rustbot added the AsyncAwait-Polish Async-await issues that are part of the "polish" area label Oct 6, 2021
matthiaskrgr added a commit to matthiaskrgr/rust that referenced this issue Nov 13, 2021
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)
bors added a commit to rust-lang-ci/rust that referenced this issue Nov 13, 2021
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)
matthiaskrgr added a commit to matthiaskrgr/rust that referenced this issue Nov 15, 2021
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)
matthiaskrgr added a commit to matthiaskrgr/rust that referenced this issue Nov 16, 2021
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)
matthiaskrgr added a commit to matthiaskrgr/rust that referenced this issue Nov 16, 2021
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)
matthiaskrgr added a commit to matthiaskrgr/rust that referenced this issue Nov 16, 2021
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)
matthiaskrgr added a commit to matthiaskrgr/rust that referenced this issue Nov 17, 2021
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)
matthiaskrgr added a commit to matthiaskrgr/rust that referenced this issue Nov 17, 2021
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)
@ijackson
Copy link
Contributor

A similar issue arises with fehler, withoutboats/fehler#45. In fact, it would be nice in general to be able to say, for a function which returns Result<Wombat,SomeError>, that the Wombat must be used. #[must_use] doesn't work there either, because it just demands use of the Result.

Another way to look at this is that both Result and Future have syntactic sugar for unpacking and implementing control flow effects, which makes them both special in that their contained type is somehow the "underlying" "real" return type of the function.

tylerwhall added a commit to tylerwhall/rmp-futures that referenced this issue Apr 21, 2022
Unfortunately these don't work as they're applied to the future and not
the value returned by the future.

rust-lang/rust#78149
estebank added a commit to estebank/rust that referenced this issue Aug 16, 2022
… `Future::Output`

No longer lint against `#[must_use] async fn foo()`.

When encountering a statement that awaits on a `Future`, check if the
`Future`'s parent item is annotated with `#[must_use]` and emit a lint
if so. This effectively makes `must_use` an annotation on the
`Future::Output` instead of only the `Future` itself.

Fix rust-lang#78149.
Manishearth added a commit to Manishearth/rust that referenced this issue Nov 11, 2022
… r=tmandry

Consider `#[must_use]` annotation on `async fn` as also affecting the `Future::Output`

No longer lint against `#[must_use] async fn foo()`.

When encountering a statement that awaits on a `Future`, check if the
`Future`'s parent item is annotated with `#[must_use]` and emit a lint
if so. This effectively makes `must_use` an annotation on the
`Future::Output` instead of only the `Future` itself.

Fix rust-lang#78149.
Manishearth added a commit to Manishearth/rust that referenced this issue Nov 11, 2022
… r=tmandry

Consider `#[must_use]` annotation on `async fn` as also affecting the `Future::Output`

No longer lint against `#[must_use] async fn foo()`.

When encountering a statement that awaits on a `Future`, check if the
`Future`'s parent item is annotated with `#[must_use]` and emit a lint
if so. This effectively makes `must_use` an annotation on the
`Future::Output` instead of only the `Future` itself.

Fix rust-lang#78149.
Dylan-DPC added a commit to Dylan-DPC/rust that referenced this issue Nov 11, 2022
… r=tmandry

Consider `#[must_use]` annotation on `async fn` as also affecting the `Future::Output`

No longer lint against `#[must_use] async fn foo()`.

When encountering a statement that awaits on a `Future`, check if the
`Future`'s parent item is annotated with `#[must_use]` and emit a lint
if so. This effectively makes `must_use` an annotation on the
`Future::Output` instead of only the `Future` itself.

Fix rust-lang#78149.
Dylan-DPC added a commit to Dylan-DPC/rust that referenced this issue Nov 11, 2022
… r=tmandry

Consider `#[must_use]` annotation on `async fn` as also affecting the `Future::Output`

No longer lint against `#[must_use] async fn foo()`.

When encountering a statement that awaits on a `Future`, check if the
`Future`'s parent item is annotated with `#[must_use]` and emit a lint
if so. This effectively makes `must_use` an annotation on the
`Future::Output` instead of only the `Future` itself.

Fix rust-lang#78149.
@bors bors closed this as completed in 243496e Nov 11, 2022
wg-async work automation moved this from On deck to Done Nov 11, 2022
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
A-async-await Area: Async & Await A-attributes Area: #[attributes(..)] A-lint Area: Lints (warnings about flaws in source code) such as unused_mut. AsyncAwait-Polish Async-await issues that are part of the "polish" area AsyncAwait-Triaged Async-await issues that have been triaged during a working group meeting. C-bug Category: This is a bug. P-medium Medium priority T-compiler Relevant to the compiler team, which will review and decide on the PR/issue.
Projects
Status: Done
Development

Successfully merging a pull request may close this issue.

9 participants