Skip to content

Commit 2eab060

Browse files
borsbors
committed
Auto merge of rust-lang#5859 - ebroto:5765_manual_async_fn_fp, r=yaahc
manual_async_fn: take input lifetimes into account The anonymous future returned from an `async fn` captures all input lifetimes. This was not being taken into account. See https://github.com/rust-lang/rfcs/blob/master/text/2394-async_await.md#lifetime-capture-in-the-anonymous-future changelog: Take input lifetimes into account in [`manual_async_fn`]. Fixes rust-lang#5765
2 parents 3d7e3fd + e336fe8 commit 2eab060

6 files changed

+146
-40
lines changed

‎clippy_lints/src/manual_async_fn.rs

+52-11
Original file line numberDiff line numberDiff line change
@@ -4,8 +4,8 @@ use if_chain::if_chain;
44
use rustc_errors::Applicability;
55
use rustc_hir::intravisit::FnKind;
66
use rustc_hir::{
7-
AsyncGeneratorKind, Block, Body, Expr, ExprKind, FnDecl, FnRetTy, GeneratorKind, GenericBound, HirId, IsAsync,
8-
ItemKind, TraitRef, Ty, TyKind, TypeBindingKind,
7+
AsyncGeneratorKind, Block, Body, Expr, ExprKind, FnDecl, FnRetTy, GeneratorKind, GenericArg, GenericBound, HirId,
8+
IsAsync, ItemKind, LifetimeName, TraitRef, Ty, TyKind, TypeBindingKind,
99
};
1010
use rustc_lint::{LateContext, LateLintPass};
1111
use rustc_session::{declare_lint_pass, declare_tool_lint};
@@ -27,8 +27,6 @@ declare_clippy_lint! {
2727
/// ```
2828
/// Use instead:
2929
/// ```rust
30-
/// use std::future::Future;
31-
///
3230
/// async fn foo() -> i32 { 42 }
3331
/// ```
3432
pub MANUAL_ASYNC_FN,
@@ -53,8 +51,9 @@ impl<'tcx> LateLintPass<'tcx> for ManualAsyncFn {
5351
if let IsAsync::NotAsync = header.asyncness;
5452
// Check that this function returns `impl Future`
5553
if let FnRetTy::Return(ret_ty) = decl.output;
56-
if let Some(trait_ref) = future_trait_ref(cx, ret_ty);
54+
if let Some((trait_ref, output_lifetimes)) = future_trait_ref(cx, ret_ty);
5755
if let Some(output) = future_output_ty(trait_ref);
56+
if captures_all_lifetimes(decl.inputs, &output_lifetimes);
5857
// Check that the body of the function consists of one async block
5958
if let ExprKind::Block(block, _) = body.value.kind;
6059
if block.stmts.is_empty();
@@ -97,16 +96,35 @@ impl<'tcx> LateLintPass<'tcx> for ManualAsyncFn {
9796
}
9897
}
9998

100-
fn future_trait_ref<'tcx>(cx: &LateContext<'tcx>, ty: &'tcx Ty<'tcx>) -> Option<&'tcx TraitRef<'tcx>> {
99+
fn future_trait_ref<'tcx>(
100+
cx: &LateContext<'tcx>,
101+
ty: &'tcx Ty<'tcx>,
102+
) -> Option<(&'tcx TraitRef<'tcx>, Vec<LifetimeName>)> {
101103
if_chain! {
102-
if let TyKind::OpaqueDef(item_id, _) = ty.kind;
104+
if let TyKind::OpaqueDef(item_id, bounds) = ty.kind;
103105
let item = cx.tcx.hir().item(item_id.id);
104106
if let ItemKind::OpaqueTy(opaque) = &item.kind;
105-
if opaque.bounds.len() == 1;
106-
if let GenericBound::Trait(poly, _) = &opaque.bounds[0];
107-
if poly.trait_ref.trait_def_id() == cx.tcx.lang_items().future_trait();
107+
if let Some(trait_ref) = opaque.bounds.iter().find_map(|bound| {
108+
if let GenericBound::Trait(poly, _) = bound {
109+
Some(&poly.trait_ref)
110+
} else {
111+
None
112+
}
113+
});
114+
if trait_ref.trait_def_id() == cx.tcx.lang_items().future_trait();
108115
then {
109-
return Some(&poly.trait_ref);
116+
let output_lifetimes = bounds
117+
.iter()
118+
.filter_map(|bound| {
119+
if let GenericArg::Lifetime(lt) = bound {
120+
Some(lt.name)
121+
} else {
122+
None
123+
}
124+
})
125+
.collect();
126+
127+
return Some((trait_ref, output_lifetimes));
110128
}
111129
}
112130

@@ -129,6 +147,29 @@ fn future_output_ty<'tcx>(trait_ref: &'tcx TraitRef<'tcx>) -> Option<&'tcx Ty<'t
129147
None
130148
}
131149

150+
fn captures_all_lifetimes(inputs: &[Ty<'_>], output_lifetimes: &[LifetimeName]) -> bool {
151+
let input_lifetimes: Vec<LifetimeName> = inputs
152+
.iter()
153+
.filter_map(|ty| {
154+
if let TyKind::Rptr(lt, _) = ty.kind {
155+
Some(lt.name)
156+
} else {
157+
None
158+
}
159+
})
160+
.collect();
161+
162+
// The lint should trigger in one of these cases:
163+
// - There are no input lifetimes
164+
// - There's only one output lifetime bound using `+ '_`
165+
// - All input lifetimes are explicitly bound to the output
166+
input_lifetimes.is_empty()
167+
|| (output_lifetimes.len() == 1 && matches!(output_lifetimes[0], LifetimeName::Underscore))
168+
|| input_lifetimes
169+
.iter()
170+
.all(|in_lt| output_lifetimes.iter().any(|out_lt| in_lt == out_lt))
171+
}
172+
132173
fn desugared_async_block<'tcx>(cx: &LateContext<'tcx>, block: &'tcx Block<'tcx>) -> Option<&'tcx Body<'tcx>> {
133174
if_chain! {
134175
if let Some(block_expr) = block.expr;

‎tests/ui/await_holding_lock.rs

+1
Original file line numberDiff line numberDiff line change
@@ -47,6 +47,7 @@ async fn not_good(x: &Mutex<u32>) -> u32 {
4747
first + second + third
4848
}
4949

50+
#[allow(clippy::manual_async_fn)]
5051
fn block_bad(x: &Mutex<u32>) -> impl std::future::Future<Output = u32> + '_ {
5152
async move {
5253
let guard = x.lock().unwrap();

‎tests/ui/await_holding_lock.stderr

+2-2
Original file line numberDiff line numberDiff line change
@@ -46,13 +46,13 @@ LL | | };
4646
| |_____^
4747

4848
error: this MutexGuard is held across an 'await' point. Consider using an async-aware Mutex type or ensuring the MutexGuard is dropped before calling await.
49-
--> $DIR/await_holding_lock.rs:52:13
49+
--> $DIR/await_holding_lock.rs:53:13
5050
|
5151
LL | let guard = x.lock().unwrap();
5252
| ^^^^^
5353
|
5454
note: these are all the await points this lock is held through
55-
--> $DIR/await_holding_lock.rs:52:9
55+
--> $DIR/await_holding_lock.rs:53:9
5656
|
5757
LL | / let guard = x.lock().unwrap();
5858
LL | | baz().await

‎tests/ui/manual_async_fn.fixed

+36-4
Original file line numberDiff line numberDiff line change
@@ -43,10 +43,6 @@ impl S {
4343
42
4444
}
4545

46-
async fn meth_fut(&self) -> i32 { 42 }
47-
48-
async fn empty_fut(&self) {}
49-
5046
// should be ignored
5147
fn not_fut(&self) -> i32 {
5248
42
@@ -64,4 +60,40 @@ impl S {
6460
}
6561
}
6662

63+
// Tests related to lifetime capture
64+
65+
async fn elided(_: &i32) -> i32 { 42 }
66+
67+
// should be ignored
68+
fn elided_not_bound(_: &i32) -> impl Future<Output = i32> {
69+
async { 42 }
70+
}
71+
72+
async fn explicit<'a, 'b>(_: &'a i32, _: &'b i32) -> i32 { 42 }
73+
74+
// should be ignored
75+
#[allow(clippy::needless_lifetimes)]
76+
fn explicit_not_bound<'a, 'b>(_: &'a i32, _: &'b i32) -> impl Future<Output = i32> {
77+
async { 42 }
78+
}
79+
80+
// should be ignored
81+
mod issue_5765 {
82+
use std::future::Future;
83+
84+
struct A;
85+
impl A {
86+
fn f(&self) -> impl Future<Output = ()> {
87+
async {}
88+
}
89+
}
90+
91+
fn test() {
92+
let _future = {
93+
let a = A;
94+
a.f()
95+
};
96+
}
97+
}
98+
6799
fn main() {}

‎tests/ui/manual_async_fn.rs

+40-8
Original file line numberDiff line numberDiff line change
@@ -51,14 +51,6 @@ impl S {
5151
}
5252
}
5353

54-
fn meth_fut(&self) -> impl Future<Output = i32> {
55-
async { 42 }
56-
}
57-
58-
fn empty_fut(&self) -> impl Future<Output = ()> {
59-
async {}
60-
}
61-
6254
// should be ignored
6355
fn not_fut(&self) -> i32 {
6456
42
@@ -76,4 +68,44 @@ impl S {
7668
}
7769
}
7870

71+
// Tests related to lifetime capture
72+
73+
fn elided(_: &i32) -> impl Future<Output = i32> + '_ {
74+
async { 42 }
75+
}
76+
77+
// should be ignored
78+
fn elided_not_bound(_: &i32) -> impl Future<Output = i32> {
79+
async { 42 }
80+
}
81+
82+
fn explicit<'a, 'b>(_: &'a i32, _: &'b i32) -> impl Future<Output = i32> + 'a + 'b {
83+
async { 42 }
84+
}
85+
86+
// should be ignored
87+
#[allow(clippy::needless_lifetimes)]
88+
fn explicit_not_bound<'a, 'b>(_: &'a i32, _: &'b i32) -> impl Future<Output = i32> {
89+
async { 42 }
90+
}
91+
92+
// should be ignored
93+
mod issue_5765 {
94+
use std::future::Future;
95+
96+
struct A;
97+
impl A {
98+
fn f(&self) -> impl Future<Output = ()> {
99+
async {}
100+
}
101+
}
102+
103+
fn test() {
104+
let _future = {
105+
let a = A;
106+
a.f()
107+
};
108+
}
109+
}
110+
79111
fn main() {}

‎tests/ui/manual_async_fn.stderr

+15-15
Original file line numberDiff line numberDiff line change
@@ -65,34 +65,34 @@ LL | let c = 21;
6565
...
6666

6767
error: this function can be simplified using the `async fn` syntax
68-
--> $DIR/manual_async_fn.rs:54:5
68+
--> $DIR/manual_async_fn.rs:73:1
6969
|
70-
LL | fn meth_fut(&self) -> impl Future<Output = i32> {
71-
| ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^
70+
LL | fn elided(_: &i32) -> impl Future<Output = i32> + '_ {
71+
| ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^
7272
|
7373
help: make the function `async` and return the output of the future directly
7474
|
75-
LL | async fn meth_fut(&self) -> i32 {
76-
| ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^
75+
LL | async fn elided(_: &i32) -> i32 {
76+
| ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^
7777
help: move the body of the async block to the enclosing function
7878
|
79-
LL | fn meth_fut(&self) -> impl Future<Output = i32> { 42 }
80-
| ^^^^^^
79+
LL | fn elided(_: &i32) -> impl Future<Output = i32> + '_ { 42 }
80+
| ^^^^^^
8181

8282
error: this function can be simplified using the `async fn` syntax
83-
--> $DIR/manual_async_fn.rs:58:5
83+
--> $DIR/manual_async_fn.rs:82:1
8484
|
85-
LL | fn empty_fut(&self) -> impl Future<Output = ()> {
86-
| ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^
85+
LL | fn explicit<'a, 'b>(_: &'a i32, _: &'b i32) -> impl Future<Output = i32> + 'a + 'b {
86+
| ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^
8787
|
88-
help: make the function `async` and remove the return type
88+
help: make the function `async` and return the output of the future directly
8989
|
90-
LL | async fn empty_fut(&self) {
91-
| ^^^^^^^^^^^^^^^^^^^^^^^^^
90+
LL | async fn explicit<'a, 'b>(_: &'a i32, _: &'b i32) -> i32 {
91+
| ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^
9292
help: move the body of the async block to the enclosing function
9393
|
94-
LL | fn empty_fut(&self) -> impl Future<Output = ()> {}
95-
| ^^
94+
LL | fn explicit<'a, 'b>(_: &'a i32, _: &'b i32) -> impl Future<Output = i32> + 'a + 'b { 42 }
95+
| ^^^^^^
9696

9797
error: aborting due to 6 previous errors
9898

0 commit comments

Comments
 (0)