Skip to content

Commit

Permalink
Consider #[must_use] annotation on async fn as also affecting the…
Browse files Browse the repository at this point in the history
… `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 #78149.
  • Loading branch information
estebank committed Nov 11, 2022
1 parent 50bb7a4 commit 243496e
Show file tree
Hide file tree
Showing 4 changed files with 93 additions and 43 deletions.
14 changes: 13 additions & 1 deletion compiler/rustc_lint/src/unused.rs
Original file line number Diff line number Diff line change
Expand Up @@ -9,7 +9,7 @@ use rustc_hir::def::{DefKind, Res};
use rustc_hir::def_id::DefId;
use rustc_infer::traits::util::elaborate_predicates_with_span;
use rustc_middle::ty::adjustment;
use rustc_middle::ty::{self, Ty};
use rustc_middle::ty::{self, DefIdTree, Ty};
use rustc_span::symbol::Symbol;
use rustc_span::symbol::{kw, sym};
use rustc_span::{BytePos, Span};
Expand Down Expand Up @@ -93,6 +93,18 @@ impl<'tcx> LateLintPass<'tcx> for UnusedResults {
return;
}

if let hir::ExprKind::Match(await_expr, _arms, hir::MatchSource::AwaitDesugar) = expr.kind
&& let ty = cx.typeck_results().expr_ty(&await_expr)
&& let ty::Opaque(def_id, _) = ty.kind()
&& cx.tcx.ty_is_opaque_future(ty)
&& let parent = cx.tcx.parent(*def_id)
&& check_must_use_def(cx, parent, expr.span, "awaited future returned by ", "")
{
// We have a bare `foo().await;` on an opaque type from an async function that was
// annotated with `#[must_use]`.
return;
}

let ty = cx.typeck_results().expr_ty(&expr);
let type_permits_lack_of_use = check_must_use_ty(cx, ty, &expr, expr.span, "", "", 1);

Expand Down
14 changes: 2 additions & 12 deletions compiler/rustc_passes/src/check_attr.rs
Original file line number Diff line number Diff line change
Expand Up @@ -139,7 +139,7 @@ impl CheckAttrVisitor<'_> {
sym::collapse_debuginfo => self.check_collapse_debuginfo(attr, span, target),
sym::const_trait => self.check_const_trait(attr, span, target),
sym::must_not_suspend => self.check_must_not_suspend(&attr, span, target),
sym::must_use => self.check_must_use(hir_id, &attr, span, target),
sym::must_use => self.check_must_use(hir_id, &attr, target),
sym::rustc_pass_by_value => self.check_pass_by_value(&attr, span, target),
sym::rustc_allow_incoherent_impl => {
self.check_allow_incoherent_impl(&attr, span, target)
Expand Down Expand Up @@ -1163,17 +1163,7 @@ impl CheckAttrVisitor<'_> {
}

/// Warns against some misuses of `#[must_use]`
fn check_must_use(&self, hir_id: HirId, attr: &Attribute, span: Span, target: Target) -> bool {
let node = self.tcx.hir().get(hir_id);
if let Some(kind) = node.fn_kind() && let rustc_hir::IsAsync::Async = kind.asyncness() {
self.tcx.emit_spanned_lint(
UNUSED_ATTRIBUTES,
hir_id,
attr.span,
errors::MustUseAsync { span }
);
}

fn check_must_use(&self, hir_id: HirId, attr: &Attribute, target: Target) -> bool {
if !matches!(
target,
Target::Fn
Expand Down
29 changes: 24 additions & 5 deletions src/test/ui/lint/unused/unused-async.rs
Original file line number Diff line number Diff line change
@@ -1,24 +1,43 @@
// edition:2018
// run-pass
#![allow(dead_code)]
#![deny(unused_must_use)]


#[must_use]
//~^ WARNING `must_use`
async fn test() -> i32 {
async fn foo() -> i32 {
1
}

#[must_use]
fn bar() -> impl std::future::Future<Output=i32> {
async {
42
}
}

async fn baz() -> i32 {
0
}

struct Wowee {}

impl Wowee {
#[must_use]
//~^ WARNING `must_use`
async fn test_method() -> i32 {
1
}
}

async fn test() {
foo(); //~ ERROR unused return value of `foo` that must be used
//~^ ERROR unused implementer of `Future` that must be used
foo().await; //~ ERROR unused awaited future returned by `foo` that must be used
bar(); //~ ERROR unused return value of `bar` that must be used
//~^ ERROR unused implementer of `Future` that must be used
bar().await; //~ ERROR unused awaited future returned by `bar` that must be used
baz(); //~ ERROR unused implementer of `Future` that must be used
baz().await; // ok
}

/* FIXME(guswynn) update this test when async-fn-in-traits works
trait Doer {
#[must_use]
Expand Down
79 changes: 54 additions & 25 deletions src/test/ui/lint/unused/unused-async.stderr
Original file line number Diff line number Diff line change
@@ -1,26 +1,55 @@
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 `Future`s returned by async functions are already `must_use`
|
= note: `#[warn(unused_attributes)]` on by default

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

warning: 2 warnings emitted
error: unused implementer of `Future` that must be used
--> $DIR/unused-async.rs:31:5
|
LL | foo();
| ^^^^^
|
note: the lint level is defined here
--> $DIR/unused-async.rs:2:9
|
LL | #![deny(unused_must_use)]
| ^^^^^^^^^^^^^^^
= note: futures do nothing unless you `.await` or poll them

error: unused return value of `foo` that must be used
--> $DIR/unused-async.rs:31:5
|
LL | foo();
| ^^^^^

error: unused awaited future returned by `foo` that must be used
--> $DIR/unused-async.rs:33:5
|
LL | foo().await;
| ^^^^^^^^^^^

error: unused implementer of `Future` that must be used
--> $DIR/unused-async.rs:34:5
|
LL | bar();
| ^^^^^
|
= note: futures do nothing unless you `.await` or poll them

error: unused return value of `bar` that must be used
--> $DIR/unused-async.rs:34:5
|
LL | bar();
| ^^^^^

error: unused awaited future returned by `bar` that must be used
--> $DIR/unused-async.rs:36:5
|
LL | bar().await;
| ^^^^^^^^^^^

error: unused implementer of `Future` that must be used
--> $DIR/unused-async.rs:37:5
|
LL | baz();
| ^^^^^
|
= note: futures do nothing unless you `.await` or poll them

error: aborting due to 7 previous errors

0 comments on commit 243496e

Please sign in to comment.