Auto merge of #116184 - compiler-errors:afit-lint, r=tmandry

Add `async_fn_in_trait` lint

cc https://github.com/rust-lang/rust/pull/115822#issuecomment-1731168465

Mostly unsure what the messaging should be. Feedback required.

r? `@tmandry`
This commit is contained in:
bors 2023-10-05 01:14:25 +00:00
commit b781645332
33 changed files with 308 additions and 60 deletions

View File

@ -5,6 +5,10 @@ lint_array_into_iter =
.use_explicit_into_iter_suggestion =
or use `IntoIterator::into_iter(..)` instead of `.into_iter()` to explicitly iterate by value
lint_async_fn_in_trait = use of `async fn` in public traits is discouraged as auto trait bounds cannot be specified
.note = you can suppress this lint if you plan to use the trait only in your own code, or do not care about auto traits like `Send` on the `Future`
.suggestion = you can alternatively desugar to a normal `fn` that returns `impl Future` and add any desired bounds such as `Send`
lint_atomic_ordering_fence = memory fences cannot have `Relaxed` ordering
.help = consider using ordering modes `Acquire`, `Release`, `AcqRel` or `SeqCst`

View File

@ -0,0 +1,128 @@
use crate::lints::AsyncFnInTraitDiag;
use crate::LateContext;
use crate::LateLintPass;
use rustc_hir as hir;
use rustc_trait_selection::traits::error_reporting::suggestions::suggest_desugaring_async_fn_to_impl_future_in_trait;
declare_lint! {
/// The `async_fn_in_trait` lint detects use of `async fn` in the
/// definition of a publicly-reachable trait.
///
/// ### Example
///
/// ```rust
/// # #![feature(async_fn_in_trait)]
/// pub trait Trait {
/// async fn method(&self);
/// }
/// # fn main() {}
/// ```
///
/// {{produces}}
///
/// ### Explanation
///
/// When `async fn` is used in a trait definition, the trait does not
/// promise that the opaque [`Future`] returned by the associated function
/// or method will implement any [auto traits] such as [`Send`]. This may
/// be surprising and may make the associated functions or methods on the
/// trait less useful than intended. On traits exposed publicly from a
/// crate, this may affect downstream crates whose authors cannot alter
/// the trait definition.
///
/// For example, this code is invalid:
///
/// ```rust,compile_fail
/// # #![feature(async_fn_in_trait)]
/// pub trait Trait {
/// async fn method(&self) {}
/// }
///
/// fn test<T: Trait>(x: T) {
/// fn spawn<T: Send>(_: T) {}
/// spawn(x.method()); // Not OK.
/// }
/// ```
///
/// This lint exists to warn authors of publicly-reachable traits that
/// they may want to consider desugaring the `async fn` to a normal `fn`
/// that returns an opaque `impl Future<..> + Send` type.
///
/// For example, instead of:
///
/// ```rust
/// # #![feature(async_fn_in_trait)]
/// pub trait Trait {
/// async fn method(&self) {}
/// }
/// ```
///
/// The author of the trait may want to write:
///
///
/// ```rust
/// # #![feature(return_position_impl_trait_in_trait)]
/// use core::future::Future;
/// pub trait Trait {
/// fn method(&self) -> impl Future<Output = ()> + Send { async {} }
/// }
/// ```
///
/// This still allows the use of `async fn` within impls of the trait.
/// However, it also means that the trait will never be compatible with
/// impls where the returned [`Future`] of the method does not implement
/// `Send`.
///
/// Conversely, if the trait is used only locally, if it is never used in
/// generic functions, or if it is only used in single-threaded contexts
/// that do not care whether the returned [`Future`] implements [`Send`],
/// then the lint may be suppressed.
///
/// [`Future`]: https://doc.rust-lang.org/core/future/trait.Future.html
/// [`Send`]: https://doc.rust-lang.org/core/marker/trait.Send.html
/// [auto traits]: https://doc.rust-lang.org/reference/special-types-and-traits.html#auto-traits
pub ASYNC_FN_IN_TRAIT,
Warn,
"use of `async fn` in definition of a publicly-reachable trait"
}
declare_lint_pass!(
/// Lint for use of `async fn` in the definition of a publicly-reachable
/// trait.
AsyncFnInTrait => [ASYNC_FN_IN_TRAIT]
);
impl<'tcx> LateLintPass<'tcx> for AsyncFnInTrait {
fn check_trait_item(&mut self, cx: &LateContext<'tcx>, item: &'tcx hir::TraitItem<'tcx>) {
if let hir::TraitItemKind::Fn(sig, body) = item.kind
&& let hir::IsAsync::Async(async_span) = sig.header.asyncness
{
// RTN can be used to bound `async fn` in traits in a better way than "always"
if cx.tcx.features().return_type_notation {
return;
}
// Only need to think about library implications of reachable traits
if !cx.tcx.effective_visibilities(()).is_reachable(item.owner_id.def_id) {
return;
}
let hir::FnRetTy::Return(hir::Ty { kind: hir::TyKind::OpaqueDef(def, ..), .. }) =
sig.decl.output
else {
// This should never happen, but let's not ICE.
return;
};
let sugg = suggest_desugaring_async_fn_to_impl_future_in_trait(
cx.tcx,
sig,
body,
def.owner_id.def_id,
" + Send",
);
cx.tcx.emit_spanned_lint(ASYNC_FN_IN_TRAIT, item.hir_id(), async_span, AsyncFnInTraitDiag {
sugg
});
}
}
}

View File

@ -50,6 +50,7 @@ extern crate rustc_session;
extern crate tracing;
mod array_into_iter;
mod async_fn_in_trait;
pub mod builtin;
mod context;
mod deref_into_dyn_supertrait;
@ -96,6 +97,7 @@ use rustc_session::lint::builtin::{
};
use array_into_iter::ArrayIntoIter;
use async_fn_in_trait::AsyncFnInTrait;
use builtin::*;
use deref_into_dyn_supertrait::*;
use drop_forget_useless::*;
@ -234,6 +236,7 @@ late_lint_methods!(
MapUnitFn: MapUnitFn,
MissingDebugImplementations: MissingDebugImplementations,
MissingDoc: MissingDoc,
AsyncFnInTrait: AsyncFnInTrait,
]
]
);

View File

@ -1818,3 +1818,24 @@ pub struct UnusedAllocationDiag;
#[derive(LintDiagnostic)]
#[diag(lint_unused_allocation_mut)]
pub struct UnusedAllocationMutDiag;
pub struct AsyncFnInTraitDiag {
pub sugg: Option<Vec<(Span, String)>>,
}
impl<'a> DecorateLint<'a, ()> for AsyncFnInTraitDiag {
fn decorate_lint<'b>(
self,
diag: &'b mut rustc_errors::DiagnosticBuilder<'a, ()>,
) -> &'b mut rustc_errors::DiagnosticBuilder<'a, ()> {
diag.note(fluent::lint_note);
if let Some(sugg) = self.sugg {
diag.multipart_suggestion(fluent::lint_suggestion, sugg, Applicability::MaybeIncorrect);
}
diag
}
fn msg(&self) -> rustc_errors::DiagnosticMessage {
fluent::lint_async_fn_in_trait
}
}

View File

@ -4000,14 +4000,6 @@ impl<'tcx> TypeErrCtxtExt<'tcx> for TypeErrCtxt<'_, 'tcx> {
// ... whose signature is `async` (i.e. this is an AFIT)
let (sig, body) = item.expect_fn();
let hir::IsAsync::Async(async_span) = sig.header.asyncness else {
return;
};
let Ok(async_span) =
self.tcx.sess.source_map().span_extend_while(async_span, |c| c.is_whitespace())
else {
return;
};
let hir::FnRetTy::Return(hir::Ty { kind: hir::TyKind::OpaqueDef(def, ..), .. }) =
sig.decl.output
else {
@ -4021,55 +4013,17 @@ impl<'tcx> TypeErrCtxtExt<'tcx> for TypeErrCtxt<'_, 'tcx> {
return;
}
let future = self.tcx.hir().item(*def).expect_opaque_ty();
let Some(hir::GenericBound::LangItemTrait(_, _, _, generics)) = future.bounds.get(0) else {
// `async fn` should always lower to a lang item bound... but don't ICE.
return;
};
let Some(hir::TypeBindingKind::Equality { term: hir::Term::Ty(future_output_ty) }) =
generics.bindings.get(0).map(|binding| binding.kind)
else {
// Also should never happen.
let Some(sugg) = suggest_desugaring_async_fn_to_impl_future_in_trait(
self.tcx,
*sig,
*body,
opaque_def_id.expect_local(),
&format!(" + {auto_trait}"),
) else {
return;
};
let function_name = self.tcx.def_path_str(fn_def_id);
let mut sugg = if future_output_ty.span.is_empty() {
vec![
(async_span, String::new()),
(
future_output_ty.span,
format!(" -> impl std::future::Future<Output = ()> + {auto_trait}"),
),
]
} else {
vec![
(
future_output_ty.span.shrink_to_lo(),
"impl std::future::Future<Output = ".to_owned(),
),
(future_output_ty.span.shrink_to_hi(), format!("> + {auto_trait}")),
(async_span, String::new()),
]
};
// If there's a body, we also need to wrap it in `async {}`
if let hir::TraitFn::Provided(body) = body {
let body = self.tcx.hir().body(*body);
let body_span = body.value.span;
let body_span_without_braces =
body_span.with_lo(body_span.lo() + BytePos(1)).with_hi(body_span.hi() - BytePos(1));
if body_span_without_braces.is_empty() {
sugg.push((body_span_without_braces, " async {} ".to_owned()));
} else {
sugg.extend([
(body_span_without_braces.shrink_to_lo(), "async {".to_owned()),
(body_span_without_braces.shrink_to_hi(), "} ".to_owned()),
]);
}
}
err.multipart_suggestion(
format!(
"`{auto_trait}` can be made part of the associated future's \
@ -4321,3 +4275,65 @@ impl<'tcx> TypeFolder<TyCtxt<'tcx>> for ReplaceImplTraitFolder<'tcx> {
self.tcx
}
}
pub fn suggest_desugaring_async_fn_to_impl_future_in_trait<'tcx>(
tcx: TyCtxt<'tcx>,
sig: hir::FnSig<'tcx>,
body: hir::TraitFn<'tcx>,
opaque_def_id: LocalDefId,
add_bounds: &str,
) -> Option<Vec<(Span, String)>> {
let hir::IsAsync::Async(async_span) = sig.header.asyncness else {
return None;
};
let Ok(async_span) = tcx.sess.source_map().span_extend_while(async_span, |c| c.is_whitespace())
else {
return None;
};
let future = tcx.hir().get_by_def_id(opaque_def_id).expect_item().expect_opaque_ty();
let Some(hir::GenericBound::LangItemTrait(_, _, _, generics)) = future.bounds.get(0) else {
// `async fn` should always lower to a lang item bound... but don't ICE.
return None;
};
let Some(hir::TypeBindingKind::Equality { term: hir::Term::Ty(future_output_ty) }) =
generics.bindings.get(0).map(|binding| binding.kind)
else {
// Also should never happen.
return None;
};
let mut sugg = if future_output_ty.span.is_empty() {
vec![
(async_span, String::new()),
(
future_output_ty.span,
format!(" -> impl std::future::Future<Output = ()>{add_bounds}"),
),
]
} else {
vec![
(future_output_ty.span.shrink_to_lo(), "impl std::future::Future<Output = ".to_owned()),
(future_output_ty.span.shrink_to_hi(), format!(">{add_bounds}")),
(async_span, String::new()),
]
};
// If there's a body, we also need to wrap it in `async {}`
if let hir::TraitFn::Provided(body) = body {
let body = tcx.hir().body(body);
let body_span = body.value.span;
let body_span_without_braces =
body_span.with_lo(body_span.lo() + BytePos(1)).with_hi(body_span.hi() - BytePos(1));
if body_span_without_braces.is_empty() {
sugg.push((body_span_without_braces, " async {} ".to_owned()));
} else {
sugg.extend([
(body_span_without_braces.shrink_to_lo(), "async {".to_owned()),
(body_span_without_braces.shrink_to_hi(), "} ".to_owned()),
]);
}
}
Some(sugg)
}

View File

@ -9,12 +9,14 @@ use std::fmt::Debug;
trait MyTrait<'a, 'b, T> where Self: 'a, T: Debug + Sized + 'b {
type MyAssoc;
#[allow(async_fn_in_trait)]
async fn foo(&'a self, key: &'b T) -> Self::MyAssoc;
}
impl<'a, 'b, T: Debug + Sized + 'b, U: 'a> MyTrait<'a, 'b, T> for U {
type MyAssoc = (&'a U, &'b T);
#[allow(async_fn_in_trait)]
async fn foo(&'a self, key: &'b T) -> (&'a U, &'b T) {
(self, key)
}

View File

@ -6,10 +6,12 @@
use std::future::Future;
trait AsyncTrait {
#[allow(async_fn_in_trait)]
async fn default_impl() {
assert!(false);
}
#[allow(async_fn_in_trait)]
async fn call_default_impl() {
Self::default_impl().await
}

View File

@ -10,6 +10,7 @@ use std::pin::Pin;
use std::task::Poll;
pub trait MyTrait {
#[allow(async_fn_in_trait)]
async fn foo(&self) -> i32;
}

View File

@ -8,6 +8,7 @@
use std::future::Future;
trait MyTrait {
#[allow(async_fn_in_trait)]
async fn foo(&self) -> i32;
}

View File

@ -5,7 +5,10 @@
#![allow(incomplete_features)]
trait MyTrait {
#[allow(async_fn_in_trait)]
async fn foo(&self) -> i32;
#[allow(async_fn_in_trait)]
async fn bar(&self) -> i32;
}

View File

@ -7,6 +7,7 @@
use std::fmt::Debug;
trait MyTrait<'a, 'b, T> {
#[allow(async_fn_in_trait)]
async fn foo(&'a self, key: &'b T) -> (&'a Self, &'b T) where T: Debug + Sized;
}

View File

@ -5,6 +5,7 @@
#![allow(incomplete_features)]
trait MyTrait<'a, 'b, T> {
#[allow(async_fn_in_trait)]
async fn foo(&'a self, key: &'b T) -> (&'a Self, &'b T);
}

View File

@ -5,6 +5,7 @@
#![allow(incomplete_features)]
pub trait Foo {
#[allow(async_fn_in_trait)]
async fn foo(&mut self);
}

View File

@ -5,6 +5,7 @@
#![allow(incomplete_features)]
pub trait Foo {
#[allow(async_fn_in_trait)]
async fn foo(&mut self);
}

View File

@ -7,6 +7,8 @@
trait TcpStack {
type Connection<'a>: Sized where Self: 'a;
fn connect<'a>(&'a self) -> Self::Connection<'a>;
#[allow(async_fn_in_trait)]
async fn async_connect<'a>(&'a self) -> Self::Connection<'a>;
}

View File

@ -10,6 +10,8 @@ async fn yield_now() {}
trait AsyncIterator {
type Item;
#[allow(async_fn_in_trait)]
async fn next(&mut self) -> Option<Self::Item>;
}

View File

@ -6,5 +6,6 @@
#![allow(incomplete_features)]
trait T {
#[allow(async_fn_in_trait)]
async fn foo();
}

View File

@ -5,6 +5,7 @@
#![allow(incomplete_features)]
pub trait SpiDevice {
#[allow(async_fn_in_trait)]
async fn transaction<F, R>(&mut self);
}

View File

@ -8,6 +8,7 @@ use std::future::Future;
pub trait Pool {
type Conn;
#[allow(async_fn_in_trait)]
async fn async_callback<'a, F: FnOnce(&'a Self::Conn) -> Fut, Fut: Future<Output = ()>>(
&'a self,
callback: F,

View File

@ -9,6 +9,7 @@ use std::future::Future;
use std::marker::PhantomData;
trait Lockable<K, V> {
#[allow(async_fn_in_trait)]
async fn lock_all_entries(&self) -> impl Future<Output = Guard<'_>>;
}

View File

@ -11,6 +11,7 @@
pub struct SharedState {}
pub trait State {
#[allow(async_fn_in_trait)]
async fn execute(self, shared_state: &SharedState);
}

View File

@ -0,0 +1,23 @@
// edition: 2021
#![feature(async_fn_in_trait)]
#![deny(async_fn_in_trait)]
pub trait Foo {
async fn not_send();
//~^ ERROR use of `async fn` in public traits is discouraged
}
mod private {
pub trait FooUnreachable {
async fn not_send();
// No warning
}
}
pub(crate) trait FooCrate {
async fn not_send();
// No warning
}
fn main() {}

View File

@ -0,0 +1,20 @@
error: use of `async fn` in public traits is discouraged as auto trait bounds cannot be specified
--> $DIR/warn.rs:7:5
|
LL | async fn not_send();
| ^^^^^
|
= note: you can suppress this lint if you plan to use the trait only in your own code, or do not care about auto traits like `Send` on the `Future`
note: the lint level is defined here
--> $DIR/warn.rs:4:9
|
LL | #![deny(async_fn_in_trait)]
| ^^^^^^^^^^^^^^^^^
help: you can alternatively desugar to a normal `fn` that returns `impl Future` and add any desired bounds such as `Send`
|
LL - async fn not_send();
LL + fn not_send() -> impl std::future::Future<Output = ()> + Send;
|
error: aborting due to previous error

View File

@ -1,5 +1,5 @@
error[E0658]: return type notation is experimental
--> $DIR/feature-gate-return_type_notation.rs:14:17
--> $DIR/feature-gate-return_type_notation.rs:15:17
|
LL | fn foo<T: Trait<m(): Send>>() {}
| ^^^^^^^^^
@ -8,7 +8,7 @@ LL | fn foo<T: Trait<m(): Send>>() {}
= help: add `#![feature(return_type_notation)]` to the crate attributes to enable
error: parenthesized generic arguments cannot be used in associated type constraints
--> $DIR/feature-gate-return_type_notation.rs:14:17
--> $DIR/feature-gate-return_type_notation.rs:15:17
|
LL | fn foo<T: Trait<m(): Send>>() {}
| ^--
@ -16,7 +16,7 @@ LL | fn foo<T: Trait<m(): Send>>() {}
| help: remove these parentheses
error[E0220]: associated type `m` not found for `Trait`
--> $DIR/feature-gate-return_type_notation.rs:14:17
--> $DIR/feature-gate-return_type_notation.rs:15:17
|
LL | fn foo<T: Trait<m(): Send>>() {}
| ^ associated type `m` not found

View File

@ -1,5 +1,5 @@
warning: return type notation is experimental
--> $DIR/feature-gate-return_type_notation.rs:14:17
--> $DIR/feature-gate-return_type_notation.rs:15:17
|
LL | fn foo<T: Trait<m(): Send>>() {}
| ^^^^^^^^^

View File

@ -7,6 +7,7 @@
#![feature(async_fn_in_trait)]
trait Trait {
#[allow(async_fn_in_trait)]
async fn m();
}

View File

@ -9,6 +9,7 @@ trait AsyncLendingIterator {
where
Self: 'a;
#[allow(async_fn_in_trait)]
async fn next(&mut self) -> Option<Self::Item<'_>>;
}

View File

@ -7,6 +7,7 @@
use std::fmt::Debug;
trait Foo {
#[allow(async_fn_in_trait)]
async fn baz(&self) -> impl Debug {
""
}

View File

@ -7,6 +7,7 @@
use std::fmt::Debug;
trait Foo {
#[allow(async_fn_in_trait)]
async fn baz(&self) -> &str {
""
}

View File

@ -5,6 +5,7 @@
#![allow(incomplete_features)]
pub trait Foo {
#[allow(async_fn_in_trait)]
async fn bar<'a: 'a>(&'a mut self);
}

View File

@ -4,12 +4,15 @@
#![feature(async_fn_in_trait, return_position_impl_trait_in_trait)]
trait Trait {
#[allow(async_fn_in_trait)]
async fn foo();
#[allow(async_fn_in_trait)]
async fn bar() -> i32;
fn test(&self) -> impl Sized + '_;
#[allow(async_fn_in_trait)]
async fn baz(&self) -> &i32;
}

View File

@ -4,12 +4,15 @@
#![feature(async_fn_in_trait, return_position_impl_trait_in_trait)]
trait Trait {
#[allow(async_fn_in_trait)]
async fn foo();
#[allow(async_fn_in_trait)]
async fn bar() -> i32;
fn test(&self) -> impl Sized + '_;
#[allow(async_fn_in_trait)]
async fn baz(&self) -> &i32;
}

View File

@ -1,15 +1,15 @@
error[E0046]: not all trait items implemented, missing: `foo`, `bar`, `test`, `baz`
--> $DIR/suggest-missing-item.rs:18:1
--> $DIR/suggest-missing-item.rs:21:1
|
LL | async fn foo();
| --------------- `foo` from trait
LL |
...
LL | async fn bar() -> i32;
| ---------------------- `bar` from trait
LL |
LL | fn test(&self) -> impl Sized + '_;
| ---------------------------------- `test` from trait
LL |
...
LL | async fn baz(&self) -> &i32;
| ---------------------------- `baz` from trait
...