Auto merge of #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 #5765
This commit is contained in:
bors
commit
2eab060ab7
@ -4,8 +4,8 @@
|
||||
use rustc_errors::Applicability;
|
||||
use rustc_hir::intravisit::FnKind;
|
||||
use rustc_hir::{
|
||||
AsyncGeneratorKind, Block, Body, Expr, ExprKind, FnDecl, FnRetTy, GeneratorKind, GenericBound, HirId, IsAsync,
|
||||
ItemKind, TraitRef, Ty, TyKind, TypeBindingKind,
|
||||
AsyncGeneratorKind, Block, Body, Expr, ExprKind, FnDecl, FnRetTy, GeneratorKind, GenericArg, GenericBound, HirId,
|
||||
IsAsync, ItemKind, LifetimeName, TraitRef, Ty, TyKind, TypeBindingKind,
|
||||
};
|
||||
use rustc_lint::{LateContext, LateLintPass};
|
||||
use rustc_session::{declare_lint_pass, declare_tool_lint};
|
||||
@ -27,8 +27,6 @@
|
||||
/// ```
|
||||
/// Use instead:
|
||||
/// ```rust
|
||||
/// use std::future::Future;
|
||||
///
|
||||
/// async fn foo() -> i32 { 42 }
|
||||
/// ```
|
||||
pub MANUAL_ASYNC_FN,
|
||||
@ -53,8 +51,9 @@ fn check_fn(
|
||||
if let IsAsync::NotAsync = header.asyncness;
|
||||
// Check that this function returns `impl Future`
|
||||
if let FnRetTy::Return(ret_ty) = decl.output;
|
||||
if let Some(trait_ref) = future_trait_ref(cx, ret_ty);
|
||||
if let Some((trait_ref, output_lifetimes)) = future_trait_ref(cx, ret_ty);
|
||||
if let Some(output) = future_output_ty(trait_ref);
|
||||
if captures_all_lifetimes(decl.inputs, &output_lifetimes);
|
||||
// Check that the body of the function consists of one async block
|
||||
if let ExprKind::Block(block, _) = body.value.kind;
|
||||
if block.stmts.is_empty();
|
||||
@ -97,16 +96,35 @@ fn check_fn(
|
||||
}
|
||||
}
|
||||
|
||||
fn future_trait_ref<'tcx>(cx: &LateContext<'tcx>, ty: &'tcx Ty<'tcx>) -> Option<&'tcx TraitRef<'tcx>> {
|
||||
fn future_trait_ref<'tcx>(
|
||||
cx: &LateContext<'tcx>,
|
||||
ty: &'tcx Ty<'tcx>,
|
||||
) -> Option<(&'tcx TraitRef<'tcx>, Vec<LifetimeName>)> {
|
||||
if_chain! {
|
||||
if let TyKind::OpaqueDef(item_id, _) = ty.kind;
|
||||
if let TyKind::OpaqueDef(item_id, bounds) = ty.kind;
|
||||
let item = cx.tcx.hir().item(item_id.id);
|
||||
if let ItemKind::OpaqueTy(opaque) = &item.kind;
|
||||
if opaque.bounds.len() == 1;
|
||||
if let GenericBound::Trait(poly, _) = &opaque.bounds[0];
|
||||
if poly.trait_ref.trait_def_id() == cx.tcx.lang_items().future_trait();
|
||||
if let Some(trait_ref) = opaque.bounds.iter().find_map(|bound| {
|
||||
if let GenericBound::Trait(poly, _) = bound {
|
||||
Some(&poly.trait_ref)
|
||||
} else {
|
||||
None
|
||||
}
|
||||
});
|
||||
if trait_ref.trait_def_id() == cx.tcx.lang_items().future_trait();
|
||||
then {
|
||||
return Some(&poly.trait_ref);
|
||||
let output_lifetimes = bounds
|
||||
.iter()
|
||||
.filter_map(|bound| {
|
||||
if let GenericArg::Lifetime(lt) = bound {
|
||||
Some(lt.name)
|
||||
} else {
|
||||
None
|
||||
}
|
||||
})
|
||||
.collect();
|
||||
|
||||
return Some((trait_ref, output_lifetimes));
|
||||
}
|
||||
}
|
||||
|
||||
@ -129,6 +147,29 @@ fn future_output_ty<'tcx>(trait_ref: &'tcx TraitRef<'tcx>) -> Option<&'tcx Ty<'t
|
||||
None
|
||||
}
|
||||
|
||||
fn captures_all_lifetimes(inputs: &[Ty<'_>], output_lifetimes: &[LifetimeName]) -> bool {
|
||||
let input_lifetimes: Vec<LifetimeName> = inputs
|
||||
.iter()
|
||||
.filter_map(|ty| {
|
||||
if let TyKind::Rptr(lt, _) = ty.kind {
|
||||
Some(lt.name)
|
||||
} else {
|
||||
None
|
||||
}
|
||||
})
|
||||
.collect();
|
||||
|
||||
// The lint should trigger in one of these cases:
|
||||
// - There are no input lifetimes
|
||||
// - There's only one output lifetime bound using `+ '_`
|
||||
// - All input lifetimes are explicitly bound to the output
|
||||
input_lifetimes.is_empty()
|
||||
|| (output_lifetimes.len() == 1 && matches!(output_lifetimes[0], LifetimeName::Underscore))
|
||||
|| input_lifetimes
|
||||
.iter()
|
||||
.all(|in_lt| output_lifetimes.iter().any(|out_lt| in_lt == out_lt))
|
||||
}
|
||||
|
||||
fn desugared_async_block<'tcx>(cx: &LateContext<'tcx>, block: &'tcx Block<'tcx>) -> Option<&'tcx Body<'tcx>> {
|
||||
if_chain! {
|
||||
if let Some(block_expr) = block.expr;
|
||||
|
||||
@ -47,6 +47,7 @@ async fn not_good(x: &Mutex<u32>) -> u32 {
|
||||
first + second + third
|
||||
}
|
||||
|
||||
#[allow(clippy::manual_async_fn)]
|
||||
fn block_bad(x: &Mutex<u32>) -> impl std::future::Future<Output = u32> + '_ {
|
||||
async move {
|
||||
let guard = x.lock().unwrap();
|
||||
|
||||
@ -46,13 +46,13 @@ LL | | };
|
||||
| |_____^
|
||||
|
||||
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.
|
||||
--> $DIR/await_holding_lock.rs:52:13
|
||||
--> $DIR/await_holding_lock.rs:53:13
|
||||
|
|
||||
LL | let guard = x.lock().unwrap();
|
||||
| ^^^^^
|
||||
|
|
||||
note: these are all the await points this lock is held through
|
||||
--> $DIR/await_holding_lock.rs:52:9
|
||||
--> $DIR/await_holding_lock.rs:53:9
|
||||
|
|
||||
LL | / let guard = x.lock().unwrap();
|
||||
LL | | baz().await
|
||||
|
||||
@ -43,10 +43,6 @@ impl S {
|
||||
42
|
||||
}
|
||||
|
||||
async fn meth_fut(&self) -> i32 { 42 }
|
||||
|
||||
async fn empty_fut(&self) {}
|
||||
|
||||
// should be ignored
|
||||
fn not_fut(&self) -> i32 {
|
||||
42
|
||||
@ -64,4 +60,40 @@ impl S {
|
||||
}
|
||||
}
|
||||
|
||||
// Tests related to lifetime capture
|
||||
|
||||
async fn elided(_: &i32) -> i32 { 42 }
|
||||
|
||||
// should be ignored
|
||||
fn elided_not_bound(_: &i32) -> impl Future<Output = i32> {
|
||||
async { 42 }
|
||||
}
|
||||
|
||||
async fn explicit<'a, 'b>(_: &'a i32, _: &'b i32) -> i32 { 42 }
|
||||
|
||||
// should be ignored
|
||||
#[allow(clippy::needless_lifetimes)]
|
||||
fn explicit_not_bound<'a, 'b>(_: &'a i32, _: &'b i32) -> impl Future<Output = i32> {
|
||||
async { 42 }
|
||||
}
|
||||
|
||||
// should be ignored
|
||||
mod issue_5765 {
|
||||
use std::future::Future;
|
||||
|
||||
struct A;
|
||||
impl A {
|
||||
fn f(&self) -> impl Future<Output = ()> {
|
||||
async {}
|
||||
}
|
||||
}
|
||||
|
||||
fn test() {
|
||||
let _future = {
|
||||
let a = A;
|
||||
a.f()
|
||||
};
|
||||
}
|
||||
}
|
||||
|
||||
fn main() {}
|
||||
|
||||
@ -51,14 +51,6 @@ fn inh_fut() -> impl Future<Output = i32> {
|
||||
}
|
||||
}
|
||||
|
||||
fn meth_fut(&self) -> impl Future<Output = i32> {
|
||||
async { 42 }
|
||||
}
|
||||
|
||||
fn empty_fut(&self) -> impl Future<Output = ()> {
|
||||
async {}
|
||||
}
|
||||
|
||||
// should be ignored
|
||||
fn not_fut(&self) -> i32 {
|
||||
42
|
||||
@ -76,4 +68,44 @@ async fn already_async(&self) -> impl Future<Output = i32> {
|
||||
}
|
||||
}
|
||||
|
||||
// Tests related to lifetime capture
|
||||
|
||||
fn elided(_: &i32) -> impl Future<Output = i32> + '_ {
|
||||
async { 42 }
|
||||
}
|
||||
|
||||
// should be ignored
|
||||
fn elided_not_bound(_: &i32) -> impl Future<Output = i32> {
|
||||
async { 42 }
|
||||
}
|
||||
|
||||
fn explicit<'a, 'b>(_: &'a i32, _: &'b i32) -> impl Future<Output = i32> + 'a + 'b {
|
||||
async { 42 }
|
||||
}
|
||||
|
||||
// should be ignored
|
||||
#[allow(clippy::needless_lifetimes)]
|
||||
fn explicit_not_bound<'a, 'b>(_: &'a i32, _: &'b i32) -> impl Future<Output = i32> {
|
||||
async { 42 }
|
||||
}
|
||||
|
||||
// should be ignored
|
||||
mod issue_5765 {
|
||||
use std::future::Future;
|
||||
|
||||
struct A;
|
||||
impl A {
|
||||
fn f(&self) -> impl Future<Output = ()> {
|
||||
async {}
|
||||
}
|
||||
}
|
||||
|
||||
fn test() {
|
||||
let _future = {
|
||||
let a = A;
|
||||
a.f()
|
||||
};
|
||||
}
|
||||
}
|
||||
|
||||
fn main() {}
|
||||
|
||||
@ -65,34 +65,34 @@ LL | let c = 21;
|
||||
...
|
||||
|
||||
error: this function can be simplified using the `async fn` syntax
|
||||
--> $DIR/manual_async_fn.rs:54:5
|
||||
--> $DIR/manual_async_fn.rs:73:1
|
||||
|
|
||||
LL | fn meth_fut(&self) -> impl Future<Output = i32> {
|
||||
| ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^
|
||||
LL | fn elided(_: &i32) -> impl Future<Output = i32> + '_ {
|
||||
| ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^
|
||||
|
|
||||
help: make the function `async` and return the output of the future directly
|
||||
|
|
||||
LL | async fn meth_fut(&self) -> i32 {
|
||||
| ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^
|
||||
LL | async fn elided(_: &i32) -> i32 {
|
||||
| ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^
|
||||
help: move the body of the async block to the enclosing function
|
||||
|
|
||||
LL | fn meth_fut(&self) -> impl Future<Output = i32> { 42 }
|
||||
| ^^^^^^
|
||||
LL | fn elided(_: &i32) -> impl Future<Output = i32> + '_ { 42 }
|
||||
| ^^^^^^
|
||||
|
||||
error: this function can be simplified using the `async fn` syntax
|
||||
--> $DIR/manual_async_fn.rs:58:5
|
||||
--> $DIR/manual_async_fn.rs:82:1
|
||||
|
|
||||
LL | fn empty_fut(&self) -> impl Future<Output = ()> {
|
||||
| ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^
|
||||
LL | fn explicit<'a, 'b>(_: &'a i32, _: &'b i32) -> impl Future<Output = i32> + 'a + 'b {
|
||||
| ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^
|
||||
|
|
||||
help: make the function `async` and remove the return type
|
||||
help: make the function `async` and return the output of the future directly
|
||||
|
|
||||
LL | async fn empty_fut(&self) {
|
||||
| ^^^^^^^^^^^^^^^^^^^^^^^^^
|
||||
LL | async fn explicit<'a, 'b>(_: &'a i32, _: &'b i32) -> i32 {
|
||||
| ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^
|
||||
help: move the body of the async block to the enclosing function
|
||||
|
|
||||
LL | fn empty_fut(&self) -> impl Future<Output = ()> {}
|
||||
| ^^
|
||||
LL | fn explicit<'a, 'b>(_: &'a i32, _: &'b i32) -> impl Future<Output = i32> + 'a + 'b { 42 }
|
||||
| ^^^^^^
|
||||
|
||||
error: aborting due to 6 previous errors
|
||||
|
||||
|
||||
Loading…
Reference in New Issue
Block a user