Skip to content

Commit

Permalink
Rollup merge of #100633 - estebank:must_use_async_fn_return, r=tmandry
Browse files Browse the repository at this point in the history
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 #78149.
  • Loading branch information
Dylan-DPC authored Nov 11, 2022
2 parents b7b7f27 + f57713b commit 978e7ab
Show file tree
Hide file tree
Showing 28 changed files with 159 additions and 105 deletions.
30 changes: 23 additions & 7 deletions 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 @@ -87,17 +87,33 @@ declare_lint_pass!(UnusedResults => [UNUSED_MUST_USE, UNUSED_RESULTS]);

impl<'tcx> LateLintPass<'tcx> for UnusedResults {
fn check_stmt(&mut self, cx: &LateContext<'_>, s: &hir::Stmt<'_>) {
let expr = match s.kind {
hir::StmtKind::Semi(ref expr) => &**expr,
_ => return,
};
let hir::StmtKind::Semi(expr) = s.kind else { return; };

if let hir::ExprKind::Ret(..) = expr.kind {
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(future_def_id, _) = ty.kind()
&& cx.tcx.ty_is_opaque_future(ty)
// FIXME: This also includes non-async fns that return `impl Future`.
&& let async_fn_def_id = cx.tcx.parent(*future_def_id)
&& check_must_use_def(
cx,
async_fn_def_id,
expr.span,
"output of 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, s.span, "", "", 1);
let type_permits_lack_of_use = check_must_use_ty(cx, ty, &expr, expr.span, "", "", 1);

let mut fn_warned = false;
let mut op_warned = false;
Expand All @@ -119,7 +135,7 @@ impl<'tcx> LateLintPass<'tcx> for UnusedResults {
_ => None,
};
if let Some(def_id) = maybe_def_id {
fn_warned = check_must_use_def(cx, def_id, s.span, "return value of ", "");
fn_warned = check_must_use_def(cx, def_id, expr.span, "return value of ", "");
} else if type_permits_lack_of_use {
// We don't warn about unused unit or uninhabited types.
// (See https://github.com/rust-lang/rust/issues/43806 for details.)
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
Original file line number Diff line number Diff line change
Expand Up @@ -16,7 +16,7 @@ LL | / || match out_ref {
LL | | Variant::A => (),
LL | | Variant::B => (),
LL | | };
| |______^
| |_____^
|
= note: closures are lazy and do nothing unless called
= note: `#[warn(unused_must_use)]` on by default
Expand All @@ -28,7 +28,7 @@ LL | / || match here.field {
LL | | Variant::A => (),
LL | | Variant::B => (),
LL | | };
| |______^
| |_____^
|
= note: closures are lazy and do nothing unless called

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -28,7 +28,7 @@ warning: unused `MustUseDeprecated` that must be used
--> $DIR/cfg-attr-multi-true.rs:19:5
|
LL | MustUseDeprecated::new();
| ^^^^^^^^^^^^^^^^^^^^^^^^^
| ^^^^^^^^^^^^^^^^^^^^^^^^
|
note: the lint level is defined here
--> $DIR/cfg-attr-multi-true.rs:7:9
Expand Down
4 changes: 2 additions & 2 deletions src/test/ui/generator/issue-52398.stderr
Original file line number Diff line number Diff line change
Expand Up @@ -4,7 +4,7 @@ warning: unused generator that must be used
LL | / move || {
LL | | A.test(yield);
LL | | };
| |______^
| |_____^
|
= note: generators are lazy and do nothing unless resumed
= note: `#[warn(unused_must_use)]` on by default
Expand All @@ -16,7 +16,7 @@ LL | / static move || {
LL | | yield *y.borrow();
LL | | return "Done";
LL | | };
| |______^
| |_____^
|
= note: generators are lazy and do nothing unless resumed

Expand Down
2 changes: 1 addition & 1 deletion src/test/ui/generator/issue-57084.stderr
Original file line number Diff line number Diff line change
Expand Up @@ -7,7 +7,7 @@ LL | | loop {
LL | | yield
LL | | }
LL | | };
| |______^
| |_____^
|
= note: generators are lazy and do nothing unless resumed
= note: `#[warn(unused_must_use)]` on by default
Expand Down
2 changes: 1 addition & 1 deletion src/test/ui/generator/match-bindings.stderr
Original file line number Diff line number Diff line change
Expand Up @@ -8,7 +8,7 @@ LL | | match Enum::A(String::new()) {
... |
LL | | }
LL | | };
| |______^
| |_____^
|
= note: generators are lazy and do nothing unless resumed
= note: `#[warn(unused_must_use)]` on by default
Expand Down
2 changes: 1 addition & 1 deletion src/test/ui/generator/reborrow-mut-upvar.stderr
Original file line number Diff line number Diff line change
Expand Up @@ -8,7 +8,7 @@ LL | | yield;
... |
LL | | *bar = 2;
LL | | };
| |______^
| |_____^
|
= note: generators are lazy and do nothing unless resumed
= note: `#[warn(unused_must_use)]` on by default
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -8,7 +8,7 @@ LL | | // and it should also find out that `a` is not live.
... |
LL | | let _ = &a;
LL | | };
| |__________^
| |_________^
|
= note: generators are lazy and do nothing unless resumed
= note: `#[warn(unused_must_use)]` on by default
Expand Down
2 changes: 1 addition & 1 deletion src/test/ui/generator/yield-in-args-rev.stderr
Original file line number Diff line number Diff line change
Expand Up @@ -5,7 +5,7 @@ LL | / || {
LL | | let b = true;
LL | | foo(yield, &b);
LL | | };
| |______^
| |_____^
|
= note: generators are lazy and do nothing unless resumed
= note: `#[warn(unused_must_use)]` on by default
Expand Down
2 changes: 1 addition & 1 deletion src/test/ui/generator/yield-in-box.stderr
Original file line number Diff line number Diff line change
Expand Up @@ -8,7 +8,7 @@ LL | | let _t = box (&x, yield 0, &y);
... |
LL | | }
LL | | };
| |______^
| |_____^
|
= note: generators are lazy and do nothing unless resumed
= note: `#[warn(unused_must_use)]` on by default
Expand Down
2 changes: 1 addition & 1 deletion src/test/ui/generator/yield-in-initializer.stderr
Original file line number Diff line number Diff line change
Expand Up @@ -8,7 +8,7 @@ LL | | // See https://github.com/rust-lang/rust/issues/52792
... |
LL | | }
LL | | };
| |______^
| |_____^
|
= note: generators are lazy and do nothing unless resumed
= note: `#[warn(unused_must_use)]` on by default
Expand Down
2 changes: 1 addition & 1 deletion src/test/ui/generator/yield-subtype.stderr
Original file line number Diff line number Diff line change
Expand Up @@ -5,7 +5,7 @@ LL | / || {
LL | | yield a;
LL | | yield b;
LL | | };
| |______^
| |_____^
|
= note: generators are lazy and do nothing unless resumed
= note: `#[warn(unused_must_use)]` on by default
Expand Down
2 changes: 1 addition & 1 deletion src/test/ui/issues/issue-1460.stderr
Original file line number Diff line number Diff line change
Expand Up @@ -2,7 +2,7 @@ warning: unused closure that must be used
--> $DIR/issue-1460.rs:6:5
|
LL | {|i: u32| if 1 == i { }};
| ^^^^^^^^^^^^^^^^^^^^^^^^^
| ^^^^^^^^^^^^^^^^^^^^^^^^
|
= note: closures are lazy and do nothing unless called
= note: `#[warn(unused_must_use)]` on by default
Expand Down
2 changes: 1 addition & 1 deletion src/test/ui/issues/issue-16256.stderr
Original file line number Diff line number Diff line change
Expand Up @@ -2,7 +2,7 @@ warning: unused closure that must be used
--> $DIR/issue-16256.rs:6:5
|
LL | |c: u8| buf.push(c);
| ^^^^^^^^^^^^^^^^^^^^
| ^^^^^^^^^^^^^^^^^^^
|
= note: closures are lazy and do nothing unless called
= note: `#[warn(unused_must_use)]` on by default
Expand Down
12 changes: 6 additions & 6 deletions src/test/ui/lint/fn_must_use.stderr
Original file line number Diff line number Diff line change
Expand Up @@ -2,7 +2,7 @@ warning: unused return value of `need_to_use_this_value` that must be used
--> $DIR/fn_must_use.rs:55:5
|
LL | need_to_use_this_value();
| ^^^^^^^^^^^^^^^^^^^^^^^^^
| ^^^^^^^^^^^^^^^^^^^^^^^^
|
= note: it's important
note: the lint level is defined here
Expand All @@ -15,33 +15,33 @@ warning: unused return value of `MyStruct::need_to_use_this_method_value` that m
--> $DIR/fn_must_use.rs:60:5
|
LL | m.need_to_use_this_method_value();
| ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^
| ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^

warning: unused return value of `EvenNature::is_even` that must be used
--> $DIR/fn_must_use.rs:61:5
|
LL | m.is_even(); // trait method!
| ^^^^^^^^^^^^
| ^^^^^^^^^^^
|
= note: no side effects

warning: unused return value of `MyStruct::need_to_use_this_associated_function_value` that must be used
--> $DIR/fn_must_use.rs:64:5
|
LL | MyStruct::need_to_use_this_associated_function_value();
| ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^
| ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^

warning: unused return value of `std::cmp::PartialEq::eq` that must be used
--> $DIR/fn_must_use.rs:70:5
|
LL | 2.eq(&3);
| ^^^^^^^^^
| ^^^^^^^^

warning: unused return value of `std::cmp::PartialEq::eq` that must be used
--> $DIR/fn_must_use.rs:71:5
|
LL | m.eq(&n);
| ^^^^^^^^^
| ^^^^^^^^

warning: unused comparison that must be used
--> $DIR/fn_must_use.rs:74:5
Expand Down
2 changes: 1 addition & 1 deletion src/test/ui/lint/unused/must-use-box-from-raw.stderr
Original file line number Diff line number Diff line change
Expand Up @@ -2,7 +2,7 @@ warning: unused return value of `Box::<T>::from_raw` that must be used
--> $DIR/must-use-box-from-raw.rs:8:5
|
LL | Box::from_raw(ptr);
| ^^^^^^^^^^^^^^^^^^^
| ^^^^^^^^^^^^^^^^^^
|
= note: call `drop(from_raw(ptr))` if you intend to drop the `Box`
note: the lint level is defined here
Expand Down
8 changes: 4 additions & 4 deletions src/test/ui/lint/unused/must_use-array.stderr
Original file line number Diff line number Diff line change
Expand Up @@ -2,7 +2,7 @@ error: unused array of `S` that must be used
--> $DIR/must_use-array.rs:39:5
|
LL | singleton();
| ^^^^^^^^^^^^
| ^^^^^^^^^^^
|
note: the lint level is defined here
--> $DIR/must_use-array.rs:1:9
Expand All @@ -14,7 +14,7 @@ error: unused array of `S` that must be used
--> $DIR/must_use-array.rs:40:5
|
LL | many();
| ^^^^^^^
| ^^^^^^

error: unused array of `S` in tuple element 0 that must be used
--> $DIR/must_use-array.rs:41:6
Expand All @@ -26,7 +26,7 @@ error: unused array of implementers of `T` that must be used
--> $DIR/must_use-array.rs:42:5
|
LL | array_of_impl_trait();
| ^^^^^^^^^^^^^^^^^^^^^^
| ^^^^^^^^^^^^^^^^^^^^^

error: unused array of boxed `T` trait objects in tuple element 1 that must be used
--> $DIR/must_use-array.rs:43:5
Expand All @@ -38,7 +38,7 @@ error: unused array of arrays of arrays of `S` that must be used
--> $DIR/must_use-array.rs:45:5
|
LL | array_of_arrays_of_arrays();
| ^^^^^^^^^^^^^^^^^^^^^^^^^^^^
| ^^^^^^^^^^^^^^^^^^^^^^^^^^^

error: aborting due to 6 previous errors

10 changes: 5 additions & 5 deletions src/test/ui/lint/unused/must_use-in-stdlib-traits.stderr
Original file line number Diff line number Diff line change
Expand Up @@ -2,7 +2,7 @@ error: unused implementer of `Iterator` that must be used
--> $DIR/must_use-in-stdlib-traits.rs:42:4
|
LL | iterator();
| ^^^^^^^^^^^
| ^^^^^^^^^^
|
= note: iterators are lazy and do nothing unless consumed
note: the lint level is defined here
Expand All @@ -15,31 +15,31 @@ error: unused implementer of `Future` that must be used
--> $DIR/must_use-in-stdlib-traits.rs:43:4
|
LL | future();
| ^^^^^^^^^
| ^^^^^^^^
|
= note: futures do nothing unless you `.await` or poll them

error: unused implementer of `FnOnce` that must be used
--> $DIR/must_use-in-stdlib-traits.rs:44:4
|
LL | square_fn_once();
| ^^^^^^^^^^^^^^^^^
| ^^^^^^^^^^^^^^^^
|
= note: closures are lazy and do nothing unless called

error: unused implementer of `FnMut` that must be used
--> $DIR/must_use-in-stdlib-traits.rs:45:4
|
LL | square_fn_mut();
| ^^^^^^^^^^^^^^^^
| ^^^^^^^^^^^^^^^
|
= note: closures are lazy and do nothing unless called

error: unused implementer of `Fn` that must be used
--> $DIR/must_use-in-stdlib-traits.rs:46:4
|
LL | square_fn();
| ^^^^^^^^^^^^
| ^^^^^^^^^^^
|
= note: closures are lazy and do nothing unless called

Expand Down
6 changes: 3 additions & 3 deletions src/test/ui/lint/unused/must_use-trait.stderr
Original file line number Diff line number Diff line change
Expand Up @@ -2,7 +2,7 @@ error: unused implementer of `Critical` that must be used
--> $DIR/must_use-trait.rs:33:5
|
LL | get_critical();
| ^^^^^^^^^^^^^^^
| ^^^^^^^^^^^^^^
|
note: the lint level is defined here
--> $DIR/must_use-trait.rs:1:9
Expand All @@ -14,13 +14,13 @@ error: unused boxed `Critical` trait object that must be used
--> $DIR/must_use-trait.rs:34:5
|
LL | get_boxed_critical();
| ^^^^^^^^^^^^^^^^^^^^^
| ^^^^^^^^^^^^^^^^^^^^

error: unused boxed boxed `Critical` trait object that must be used
--> $DIR/must_use-trait.rs:35:5
|
LL | get_nested_boxed_critical();
| ^^^^^^^^^^^^^^^^^^^^^^^^^^^^
| ^^^^^^^^^^^^^^^^^^^^^^^^^^^

error: unused boxed `Critical` trait object in tuple element 1 that must be used
--> $DIR/must_use-trait.rs:37:5
Expand Down
Loading

0 comments on commit 978e7ab

Please sign in to comment.