Skip to content

Commit

Permalink
Auto merge of #116184 - compiler-errors:afit-lint, r=tmandry
Browse files Browse the repository at this point in the history
Add `async_fn_in_trait` lint

cc #115822 (comment)

Mostly unsure what the messaging should be. Feedback required.

r? `@tmandry`
  • Loading branch information
bors committed Oct 5, 2023
2 parents afe67fa + 2f52490 commit b781645
Show file tree
Hide file tree
Showing 33 changed files with 308 additions and 60 deletions.
4 changes: 4 additions & 0 deletions compiler/rustc_lint/messages.ftl
Original file line number Diff line number Diff line change
Expand Up @@ -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`
Expand Down
128 changes: 128 additions & 0 deletions compiler/rustc_lint/src/async_fn_in_trait.rs
Original file line number Diff line number Diff line change
@@ -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
});
}
}
}
3 changes: 3 additions & 0 deletions compiler/rustc_lint/src/lib.rs
Original file line number Diff line number Diff line change
Expand Up @@ -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;
Expand Down Expand Up @@ -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::*;
Expand Down Expand Up @@ -234,6 +236,7 @@ late_lint_methods!(
MapUnitFn: MapUnitFn,
MissingDebugImplementations: MissingDebugImplementations,
MissingDoc: MissingDoc,
AsyncFnInTrait: AsyncFnInTrait,
]
]
);
Expand Down
21 changes: 21 additions & 0 deletions compiler/rustc_lint/src/lints.rs
Original file line number Diff line number Diff line change
Expand Up @@ -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
}
}
Original file line number Diff line number Diff line change
Expand Up @@ -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 {
Expand All @@ -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 \
Expand Down Expand Up @@ -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)
}
2 changes: 2 additions & 0 deletions tests/ui/async-await/in-trait/async-associated-types.rs
Original file line number Diff line number Diff line change
Expand Up @@ -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)
}
Expand Down
2 changes: 2 additions & 0 deletions tests/ui/async-await/in-trait/async-default-fn-overridden.rs
Original file line number Diff line number Diff line change
Expand Up @@ -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
}
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -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;
}

Expand Down
1 change: 1 addition & 0 deletions tests/ui/async-await/in-trait/async-example-desugared.rs
Original file line number Diff line number Diff line change
Expand Up @@ -8,6 +8,7 @@
use std::future::Future;

trait MyTrait {
#[allow(async_fn_in_trait)]
async fn foo(&self) -> i32;
}

Expand Down
3 changes: 3 additions & 0 deletions tests/ui/async-await/in-trait/async-example.rs
Original file line number Diff line number Diff line change
Expand Up @@ -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;
}

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -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;
}

Expand Down
Loading

0 comments on commit b781645

Please sign in to comment.