From ec79720c1e908728924c9634aac36fdc1fecd425 Mon Sep 17 00:00:00 2001 From: Michael Goulet Date: Tue, 26 Sep 2023 20:20:21 +0000 Subject: [PATCH 1/6] Add async_fn_in_trait lint --- compiler/rustc_lint/messages.ftl | 4 + compiler/rustc_lint/src/async_fn_in_trait.rs | 58 +++++++++ compiler/rustc_lint/src/lib.rs | 3 + compiler/rustc_lint/src/lints.rs | 21 +++ .../src/traits/error_reporting/suggestions.rs | 122 ++++++++++-------- tests/ui/async-await/in-trait/warn.rs | 11 ++ tests/ui/async-await/in-trait/warn.stderr | 20 +++ 7 files changed, 186 insertions(+), 53 deletions(-) create mode 100644 compiler/rustc_lint/src/async_fn_in_trait.rs create mode 100644 tests/ui/async-await/in-trait/warn.rs create mode 100644 tests/ui/async-await/in-trait/warn.stderr diff --git a/compiler/rustc_lint/messages.ftl b/compiler/rustc_lint/messages.ftl index 7377c6e2f35a2..ad6c381a58135 100644 --- a/compiler/rustc_lint/messages.ftl +++ b/compiler/rustc_lint/messages.ftl @@ -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 = usage of `async fn` in trait is discouraged because they do not automatically have auto trait bounds + .note = you can suppress this lint if you plan to use the trait locally, for concrete types, or do not care about auto traits like `Send` on the future + .suggestion = you can alternatively desugar the `async fn` and any add additional traits such as `Send` to the signature + lint_atomic_ordering_fence = memory fences cannot have `Relaxed` ordering .help = consider using ordering modes `Acquire`, `Release`, `AcqRel` or `SeqCst` diff --git a/compiler/rustc_lint/src/async_fn_in_trait.rs b/compiler/rustc_lint/src/async_fn_in_trait.rs new file mode 100644 index 0000000000000..0ad129d43cd7a --- /dev/null +++ b/compiler/rustc_lint/src/async_fn_in_trait.rs @@ -0,0 +1,58 @@ +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! { + /// TODO + /// + /// ### Example + /// + /// ```rust + /// fn foo() {} + /// ``` + /// + /// {{produces}} + /// + /// ### Explanation + /// + /// TODO + pub ASYNC_FN_IN_TRAIT, + Warn, + "TODO" +} + +declare_lint_pass!( + // TODO: + 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 + { + if cx.tcx.features().return_type_notation { + 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 + }); + } + } +} diff --git a/compiler/rustc_lint/src/lib.rs b/compiler/rustc_lint/src/lib.rs index 72c103f2d4a1e..af2132fb8997a 100644 --- a/compiler/rustc_lint/src/lib.rs +++ b/compiler/rustc_lint/src/lib.rs @@ -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, ] ] ); diff --git a/compiler/rustc_lint/src/lints.rs b/compiler/rustc_lint/src/lints.rs index c091c260a470e..12694aa0bedb3 100644 --- a/compiler/rustc_lint/src/lints.rs +++ b/compiler/rustc_lint/src/lints.rs @@ -1818,3 +1818,24 @@ pub struct UnusedAllocationDiag; #[derive(LintDiagnostic)] #[diag(lint_unused_allocation_mut)] pub struct UnusedAllocationMutDiag; + +pub struct AsyncFnInTraitDiag { + pub sugg: Option>, +} + +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 + } +} diff --git a/compiler/rustc_trait_selection/src/traits/error_reporting/suggestions.rs b/compiler/rustc_trait_selection/src/traits/error_reporting/suggestions.rs index 15f2ba809a4a6..b7c7350128026 100644 --- a/compiler/rustc_trait_selection/src/traits/error_reporting/suggestions.rs +++ b/compiler/rustc_trait_selection/src/traits/error_reporting/suggestions.rs @@ -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 + {auto_trait}"), - ), - ] - } else { - vec![ - ( - future_output_ty.span.shrink_to_lo(), - "impl std::future::Future + {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> 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> { + 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{add_bounds}"), + ), + ] + } else { + vec![ + (future_output_ty.span.shrink_to_lo(), "impl std::future::Future{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) +} diff --git a/tests/ui/async-await/in-trait/warn.rs b/tests/ui/async-await/in-trait/warn.rs new file mode 100644 index 0000000000000..defbdcffccd7f --- /dev/null +++ b/tests/ui/async-await/in-trait/warn.rs @@ -0,0 +1,11 @@ +// edition: 2021 + +#![feature(async_fn_in_trait)] +#![deny(async_fn_in_trait)] + +trait Foo { + async fn not_send(); + //~^ ERROR +} + +fn main() {} diff --git a/tests/ui/async-await/in-trait/warn.stderr b/tests/ui/async-await/in-trait/warn.stderr new file mode 100644 index 0000000000000..3680bd393b177 --- /dev/null +++ b/tests/ui/async-await/in-trait/warn.stderr @@ -0,0 +1,20 @@ +error: usage of `async fn` in trait is discouraged because they do not automatically have auto trait bounds + --> $DIR/warn.rs:7:5 + | +LL | async fn not_send(); + | ^^^^^ + | + = note: you can suppress this lint if you plan to use the trait locally, for concrete types, 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 the `async fn` and any add additional traits such as `Send` to the signature + | +LL - async fn not_send(); +LL + fn not_send() -> impl std::future::Future + Send; + | + +error: aborting due to previous error + From 28d58f6524a10406c60c170c5e9eae70c8a7b76d Mon Sep 17 00:00:00 2001 From: Michael Goulet Date: Tue, 26 Sep 2023 20:20:25 +0000 Subject: [PATCH 2/6] Bless tests --- tests/ui/async-await/in-trait/async-associated-types.rs | 2 ++ .../ui/async-await/in-trait/async-default-fn-overridden.rs | 2 ++ .../async-await/in-trait/async-example-desugared-extra.rs | 1 + tests/ui/async-await/in-trait/async-example-desugared.rs | 1 + tests/ui/async-await/in-trait/async-example.rs | 3 +++ tests/ui/async-await/in-trait/async-lifetimes-and-bounds.rs | 1 + tests/ui/async-await/in-trait/async-lifetimes.rs | 1 + tests/ui/async-await/in-trait/early-bound-1.rs | 1 + tests/ui/async-await/in-trait/early-bound-2.rs | 1 + tests/ui/async-await/in-trait/implied-bounds.rs | 2 ++ tests/ui/async-await/in-trait/issue-102138.rs | 2 ++ tests/ui/async-await/in-trait/issue-102219.rs | 1 + tests/ui/async-await/in-trait/issue-102310.rs | 1 + tests/ui/async-await/in-trait/issue-104678.rs | 1 + tests/ui/async-await/in-trait/nested-rpit.rs | 1 + .../in-trait/normalize-opaque-with-bound-vars.rs | 1 + .../feature-gate-return_type_notation.cfg.stderr | 6 +++--- .../feature-gate-return_type_notation.no.stderr | 2 +- tests/ui/feature-gates/feature-gate-return_type_notation.rs | 1 + tests/ui/impl-trait/in-trait/assumed-wf-bounds-in-impl.rs | 1 + tests/ui/impl-trait/in-trait/default-body-with-rpit.rs | 1 + tests/ui/impl-trait/in-trait/default-body.rs | 1 + tests/ui/impl-trait/in-trait/early.rs | 1 + tests/ui/impl-trait/in-trait/suggest-missing-item.fixed | 3 +++ tests/ui/impl-trait/in-trait/suggest-missing-item.rs | 3 +++ tests/ui/impl-trait/in-trait/suggest-missing-item.stderr | 6 +++--- 26 files changed, 40 insertions(+), 7 deletions(-) diff --git a/tests/ui/async-await/in-trait/async-associated-types.rs b/tests/ui/async-await/in-trait/async-associated-types.rs index 3e2739a164f17..14f18811c1e23 100644 --- a/tests/ui/async-await/in-trait/async-associated-types.rs +++ b/tests/ui/async-await/in-trait/async-associated-types.rs @@ -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) } diff --git a/tests/ui/async-await/in-trait/async-default-fn-overridden.rs b/tests/ui/async-await/in-trait/async-default-fn-overridden.rs index 06413fe6f8277..8143f0bca0315 100644 --- a/tests/ui/async-await/in-trait/async-default-fn-overridden.rs +++ b/tests/ui/async-await/in-trait/async-default-fn-overridden.rs @@ -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 } diff --git a/tests/ui/async-await/in-trait/async-example-desugared-extra.rs b/tests/ui/async-await/in-trait/async-example-desugared-extra.rs index 3505690f1ec7a..5d5aa817b4cb1 100644 --- a/tests/ui/async-await/in-trait/async-example-desugared-extra.rs +++ b/tests/ui/async-await/in-trait/async-example-desugared-extra.rs @@ -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; } diff --git a/tests/ui/async-await/in-trait/async-example-desugared.rs b/tests/ui/async-await/in-trait/async-example-desugared.rs index 0a5023176fef7..7987645c97b22 100644 --- a/tests/ui/async-await/in-trait/async-example-desugared.rs +++ b/tests/ui/async-await/in-trait/async-example-desugared.rs @@ -8,6 +8,7 @@ use std::future::Future; trait MyTrait { + #[allow(async_fn_in_trait)] async fn foo(&self) -> i32; } diff --git a/tests/ui/async-await/in-trait/async-example.rs b/tests/ui/async-await/in-trait/async-example.rs index abf94ef7450fc..8c80c21eabec4 100644 --- a/tests/ui/async-await/in-trait/async-example.rs +++ b/tests/ui/async-await/in-trait/async-example.rs @@ -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; } diff --git a/tests/ui/async-await/in-trait/async-lifetimes-and-bounds.rs b/tests/ui/async-await/in-trait/async-lifetimes-and-bounds.rs index d5481d277e40a..96cda4e35da78 100644 --- a/tests/ui/async-await/in-trait/async-lifetimes-and-bounds.rs +++ b/tests/ui/async-await/in-trait/async-lifetimes-and-bounds.rs @@ -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; } diff --git a/tests/ui/async-await/in-trait/async-lifetimes.rs b/tests/ui/async-await/in-trait/async-lifetimes.rs index f298e45d2390b..4b0264bc8d017 100644 --- a/tests/ui/async-await/in-trait/async-lifetimes.rs +++ b/tests/ui/async-await/in-trait/async-lifetimes.rs @@ -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); } diff --git a/tests/ui/async-await/in-trait/early-bound-1.rs b/tests/ui/async-await/in-trait/early-bound-1.rs index 6b3b142014bd3..bc410cc29546d 100644 --- a/tests/ui/async-await/in-trait/early-bound-1.rs +++ b/tests/ui/async-await/in-trait/early-bound-1.rs @@ -5,6 +5,7 @@ #![allow(incomplete_features)] pub trait Foo { + #[allow(async_fn_in_trait)] async fn foo(&mut self); } diff --git a/tests/ui/async-await/in-trait/early-bound-2.rs b/tests/ui/async-await/in-trait/early-bound-2.rs index 270443229b054..1974b1d9f7a4f 100644 --- a/tests/ui/async-await/in-trait/early-bound-2.rs +++ b/tests/ui/async-await/in-trait/early-bound-2.rs @@ -5,6 +5,7 @@ #![allow(incomplete_features)] pub trait Foo { + #[allow(async_fn_in_trait)] async fn foo(&mut self); } diff --git a/tests/ui/async-await/in-trait/implied-bounds.rs b/tests/ui/async-await/in-trait/implied-bounds.rs index 52bceb3cc5cd6..40eebad86c27c 100644 --- a/tests/ui/async-await/in-trait/implied-bounds.rs +++ b/tests/ui/async-await/in-trait/implied-bounds.rs @@ -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>; } diff --git a/tests/ui/async-await/in-trait/issue-102138.rs b/tests/ui/async-await/in-trait/issue-102138.rs index f61b34ed99e00..3d9cef0210faa 100644 --- a/tests/ui/async-await/in-trait/issue-102138.rs +++ b/tests/ui/async-await/in-trait/issue-102138.rs @@ -10,6 +10,8 @@ async fn yield_now() {} trait AsyncIterator { type Item; + + #[allow(async_fn_in_trait)] async fn next(&mut self) -> Option; } diff --git a/tests/ui/async-await/in-trait/issue-102219.rs b/tests/ui/async-await/in-trait/issue-102219.rs index 9a35f6515cb1a..4a23e4be4f710 100644 --- a/tests/ui/async-await/in-trait/issue-102219.rs +++ b/tests/ui/async-await/in-trait/issue-102219.rs @@ -6,5 +6,6 @@ #![allow(incomplete_features)] trait T { + #[allow(async_fn_in_trait)] async fn foo(); } diff --git a/tests/ui/async-await/in-trait/issue-102310.rs b/tests/ui/async-await/in-trait/issue-102310.rs index 49c3e9feeb4c4..327d432a6a6d1 100644 --- a/tests/ui/async-await/in-trait/issue-102310.rs +++ b/tests/ui/async-await/in-trait/issue-102310.rs @@ -5,6 +5,7 @@ #![allow(incomplete_features)] pub trait SpiDevice { + #[allow(async_fn_in_trait)] async fn transaction(&mut self); } diff --git a/tests/ui/async-await/in-trait/issue-104678.rs b/tests/ui/async-await/in-trait/issue-104678.rs index e396df4e5d119..0a33470750544 100644 --- a/tests/ui/async-await/in-trait/issue-104678.rs +++ b/tests/ui/async-await/in-trait/issue-104678.rs @@ -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>( &'a self, callback: F, diff --git a/tests/ui/async-await/in-trait/nested-rpit.rs b/tests/ui/async-await/in-trait/nested-rpit.rs index 9cdc23bbc7809..8c43e1b07e297 100644 --- a/tests/ui/async-await/in-trait/nested-rpit.rs +++ b/tests/ui/async-await/in-trait/nested-rpit.rs @@ -9,6 +9,7 @@ use std::future::Future; use std::marker::PhantomData; trait Lockable { + #[allow(async_fn_in_trait)] async fn lock_all_entries(&self) -> impl Future>; } diff --git a/tests/ui/async-await/in-trait/normalize-opaque-with-bound-vars.rs b/tests/ui/async-await/in-trait/normalize-opaque-with-bound-vars.rs index c4008f2b7e7ad..f8fe0d1bde88e 100644 --- a/tests/ui/async-await/in-trait/normalize-opaque-with-bound-vars.rs +++ b/tests/ui/async-await/in-trait/normalize-opaque-with-bound-vars.rs @@ -11,6 +11,7 @@ pub struct SharedState {} pub trait State { + #[allow(async_fn_in_trait)] async fn execute(self, shared_state: &SharedState); } diff --git a/tests/ui/feature-gates/feature-gate-return_type_notation.cfg.stderr b/tests/ui/feature-gates/feature-gate-return_type_notation.cfg.stderr index 1bdb2574eadc3..f6230b7646358 100644 --- a/tests/ui/feature-gates/feature-gate-return_type_notation.cfg.stderr +++ b/tests/ui/feature-gates/feature-gate-return_type_notation.cfg.stderr @@ -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>() {} | ^^^^^^^^^ @@ -8,7 +8,7 @@ LL | fn foo>() {} = 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>() {} | ^-- @@ -16,7 +16,7 @@ LL | fn foo>() {} | 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>() {} | ^ associated type `m` not found diff --git a/tests/ui/feature-gates/feature-gate-return_type_notation.no.stderr b/tests/ui/feature-gates/feature-gate-return_type_notation.no.stderr index dd6ebb6103862..c7f52d7cddc20 100644 --- a/tests/ui/feature-gates/feature-gate-return_type_notation.no.stderr +++ b/tests/ui/feature-gates/feature-gate-return_type_notation.no.stderr @@ -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>() {} | ^^^^^^^^^ diff --git a/tests/ui/feature-gates/feature-gate-return_type_notation.rs b/tests/ui/feature-gates/feature-gate-return_type_notation.rs index ae12495b5dc68..c0c285cef3cb1 100644 --- a/tests/ui/feature-gates/feature-gate-return_type_notation.rs +++ b/tests/ui/feature-gates/feature-gate-return_type_notation.rs @@ -7,6 +7,7 @@ #![feature(async_fn_in_trait)] trait Trait { + #[allow(async_fn_in_trait)] async fn m(); } diff --git a/tests/ui/impl-trait/in-trait/assumed-wf-bounds-in-impl.rs b/tests/ui/impl-trait/in-trait/assumed-wf-bounds-in-impl.rs index 2a61c5cc8dffa..5de9c01e3e01f 100644 --- a/tests/ui/impl-trait/in-trait/assumed-wf-bounds-in-impl.rs +++ b/tests/ui/impl-trait/in-trait/assumed-wf-bounds-in-impl.rs @@ -9,6 +9,7 @@ trait AsyncLendingIterator { where Self: 'a; + #[allow(async_fn_in_trait)] async fn next(&mut self) -> Option>; } diff --git a/tests/ui/impl-trait/in-trait/default-body-with-rpit.rs b/tests/ui/impl-trait/in-trait/default-body-with-rpit.rs index 25133214dc624..9c60cf4e72adb 100644 --- a/tests/ui/impl-trait/in-trait/default-body-with-rpit.rs +++ b/tests/ui/impl-trait/in-trait/default-body-with-rpit.rs @@ -7,6 +7,7 @@ use std::fmt::Debug; trait Foo { + #[allow(async_fn_in_trait)] async fn baz(&self) -> impl Debug { "" } diff --git a/tests/ui/impl-trait/in-trait/default-body.rs b/tests/ui/impl-trait/in-trait/default-body.rs index b0baf5bb10dd2..d3ea9fbeabc34 100644 --- a/tests/ui/impl-trait/in-trait/default-body.rs +++ b/tests/ui/impl-trait/in-trait/default-body.rs @@ -7,6 +7,7 @@ use std::fmt::Debug; trait Foo { + #[allow(async_fn_in_trait)] async fn baz(&self) -> &str { "" } diff --git a/tests/ui/impl-trait/in-trait/early.rs b/tests/ui/impl-trait/in-trait/early.rs index 9c1c2b5033904..bb5718b49344c 100644 --- a/tests/ui/impl-trait/in-trait/early.rs +++ b/tests/ui/impl-trait/in-trait/early.rs @@ -5,6 +5,7 @@ #![allow(incomplete_features)] pub trait Foo { + #[allow(async_fn_in_trait)] async fn bar<'a: 'a>(&'a mut self); } diff --git a/tests/ui/impl-trait/in-trait/suggest-missing-item.fixed b/tests/ui/impl-trait/in-trait/suggest-missing-item.fixed index d9f775a6c8464..58d83384a238c 100644 --- a/tests/ui/impl-trait/in-trait/suggest-missing-item.fixed +++ b/tests/ui/impl-trait/in-trait/suggest-missing-item.fixed @@ -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; } diff --git a/tests/ui/impl-trait/in-trait/suggest-missing-item.rs b/tests/ui/impl-trait/in-trait/suggest-missing-item.rs index 26979b5149b06..c27229806e130 100644 --- a/tests/ui/impl-trait/in-trait/suggest-missing-item.rs +++ b/tests/ui/impl-trait/in-trait/suggest-missing-item.rs @@ -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; } diff --git a/tests/ui/impl-trait/in-trait/suggest-missing-item.stderr b/tests/ui/impl-trait/in-trait/suggest-missing-item.stderr index 44f98896eb384..29f6bad86dc82 100644 --- a/tests/ui/impl-trait/in-trait/suggest-missing-item.stderr +++ b/tests/ui/impl-trait/in-trait/suggest-missing-item.stderr @@ -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 ... From afea0b4eab6f5e91edf70102194d77c8f8f20479 Mon Sep 17 00:00:00 2001 From: Travis Cross Date: Fri, 29 Sep 2023 20:56:49 +0000 Subject: [PATCH 3/6] Fill in prose to describe the `async_fn_in_trait` lint We're stabilizing `async fn` in trait (AFIT), but we have some reservations about how people might use this in the definitions of publicly-visible traits, so we're going to lint about that. This is a bit of an odd lint for `rustc`. We normally don't lint just to have people confirm that they understand how Rust works. But in this one exceptional case, this seems like the right thing to do as compared to the other plausible alternatives. In this commit, we describe the nature of this odd lint. --- compiler/rustc_lint/messages.ftl | 6 +- compiler/rustc_lint/src/async_fn_in_trait.rs | 70 ++++++++++++++++++-- tests/ui/async-await/in-trait/warn.stderr | 6 +- 3 files changed, 71 insertions(+), 11 deletions(-) diff --git a/compiler/rustc_lint/messages.ftl b/compiler/rustc_lint/messages.ftl index ad6c381a58135..060cc77c70ddb 100644 --- a/compiler/rustc_lint/messages.ftl +++ b/compiler/rustc_lint/messages.ftl @@ -5,9 +5,9 @@ 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 = usage of `async fn` in trait is discouraged because they do not automatically have auto trait bounds - .note = you can suppress this lint if you plan to use the trait locally, for concrete types, or do not care about auto traits like `Send` on the future - .suggestion = you can alternatively desugar the `async fn` and any add additional traits such as `Send` to the signature +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 locally, for concrete types, 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` diff --git a/compiler/rustc_lint/src/async_fn_in_trait.rs b/compiler/rustc_lint/src/async_fn_in_trait.rs index 0ad129d43cd7a..1329ba3d519c8 100644 --- a/compiler/rustc_lint/src/async_fn_in_trait.rs +++ b/compiler/rustc_lint/src/async_fn_in_trait.rs @@ -5,26 +5,86 @@ use rustc_hir as hir; use rustc_trait_selection::traits::error_reporting::suggestions::suggest_desugaring_async_fn_to_impl_future_in_trait; declare_lint! { - /// TODO + /// The `async_fn_in_trait` lint detects use of `async fn` in the + /// definition of a publicly-reachable trait. /// /// ### Example /// /// ```rust - /// fn foo() {} + /// # #![feature(async_fn_in_trait)] + /// pub trait Trait { + /// async fn method(&self); + /// } + /// # fn main() {} /// ``` /// /// {{produces}} /// /// ### Explanation /// - /// TODO + /// 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(x: T) { + /// fn is_send(_: T) {} + /// is_send(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 + Send { async {} } + /// } + /// ``` + /// + /// Conversely, if the trait is used only locally, if only concrete types + /// that implement the trait are used, or if the trait author otherwise + /// does not care that the trait will not promise that the returned + /// [`Future`] implements any [auto traits] such as [`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, - "TODO" + "use of `async fn` in definition of a publicly-reachable trait" } declare_lint_pass!( - // TODO: + /// Lint for use of `async fn` in the definition of a publicly-reachable + /// trait. AsyncFnInTrait => [ASYNC_FN_IN_TRAIT] ); diff --git a/tests/ui/async-await/in-trait/warn.stderr b/tests/ui/async-await/in-trait/warn.stderr index 3680bd393b177..0e9c9e2e28d18 100644 --- a/tests/ui/async-await/in-trait/warn.stderr +++ b/tests/ui/async-await/in-trait/warn.stderr @@ -1,16 +1,16 @@ -error: usage of `async fn` in trait is discouraged because they do not automatically have auto trait bounds +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 locally, for concrete types, or do not care about auto traits like `Send` on the future + = note: you can suppress this lint if you plan to use the trait locally, for concrete types, 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 the `async fn` and any add additional traits such as `Send` to the signature +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 + Send; From 90dfa24415d4f29e1970bf215458ccefec60f85f Mon Sep 17 00:00:00 2001 From: Michael Goulet Date: Sat, 30 Sep 2023 19:28:40 +0000 Subject: [PATCH 4/6] Only reachable traits --- compiler/rustc_lint/src/async_fn_in_trait.rs | 6 ++++++ tests/ui/async-await/in-trait/warn.rs | 16 ++++++++++++++-- 2 files changed, 20 insertions(+), 2 deletions(-) diff --git a/compiler/rustc_lint/src/async_fn_in_trait.rs b/compiler/rustc_lint/src/async_fn_in_trait.rs index 1329ba3d519c8..9214e04bb8b60 100644 --- a/compiler/rustc_lint/src/async_fn_in_trait.rs +++ b/compiler/rustc_lint/src/async_fn_in_trait.rs @@ -93,10 +93,16 @@ impl<'tcx> LateLintPass<'tcx> for AsyncFnInTrait { 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 { diff --git a/tests/ui/async-await/in-trait/warn.rs b/tests/ui/async-await/in-trait/warn.rs index defbdcffccd7f..4f981c31f5c06 100644 --- a/tests/ui/async-await/in-trait/warn.rs +++ b/tests/ui/async-await/in-trait/warn.rs @@ -3,9 +3,21 @@ #![feature(async_fn_in_trait)] #![deny(async_fn_in_trait)] -trait Foo { +pub trait Foo { async fn not_send(); - //~^ ERROR + //~^ 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() {} From c373d206cd9ed7beec89c72e61d76ad61d6d35c1 Mon Sep 17 00:00:00 2001 From: Michael Goulet Date: Tue, 3 Oct 2023 00:51:13 +0000 Subject: [PATCH 5/6] Address review nits --- compiler/rustc_lint/messages.ftl | 2 +- compiler/rustc_lint/src/async_fn_in_trait.rs | 13 ++++++------- tests/ui/async-await/in-trait/warn.stderr | 2 +- 3 files changed, 8 insertions(+), 9 deletions(-) diff --git a/compiler/rustc_lint/messages.ftl b/compiler/rustc_lint/messages.ftl index 060cc77c70ddb..4cabb5c326663 100644 --- a/compiler/rustc_lint/messages.ftl +++ b/compiler/rustc_lint/messages.ftl @@ -6,7 +6,7 @@ lint_array_into_iter = 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 locally, for concrete types, or do not care about auto traits like `Send` on the `Future` + .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 diff --git a/compiler/rustc_lint/src/async_fn_in_trait.rs b/compiler/rustc_lint/src/async_fn_in_trait.rs index 9214e04bb8b60..e0572b8ed43b1 100644 --- a/compiler/rustc_lint/src/async_fn_in_trait.rs +++ b/compiler/rustc_lint/src/async_fn_in_trait.rs @@ -39,8 +39,8 @@ declare_lint! { /// } /// /// fn test(x: T) { - /// fn is_send(_: T) {} - /// is_send(x.method()); // Not OK. + /// fn spawn(_: T) {} + /// spawn(x.method()); // Not OK. /// } /// ``` /// @@ -68,11 +68,10 @@ declare_lint! { /// } /// ``` /// - /// Conversely, if the trait is used only locally, if only concrete types - /// that implement the trait are used, or if the trait author otherwise - /// does not care that the trait will not promise that the returned - /// [`Future`] implements any [auto traits] such as [`Send`], then the - /// lint may be suppressed. + /// 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 [auto traits] + /// such as [`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 diff --git a/tests/ui/async-await/in-trait/warn.stderr b/tests/ui/async-await/in-trait/warn.stderr index 0e9c9e2e28d18..eac41a6e92425 100644 --- a/tests/ui/async-await/in-trait/warn.stderr +++ b/tests/ui/async-await/in-trait/warn.stderr @@ -4,7 +4,7 @@ error: use of `async fn` in public traits is discouraged as auto trait bounds ca LL | async fn not_send(); | ^^^^^ | - = note: you can suppress this lint if you plan to use the trait locally, for concrete types, or do not care about auto traits like `Send` on the `Future` + = 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 | From 2f5249019e94f99920dae6df832bde633b151eb4 Mon Sep 17 00:00:00 2001 From: Tyler Mandry Date: Wed, 4 Oct 2023 18:20:05 -0400 Subject: [PATCH 6/6] Apply suggestions from code review Co-authored-by: Travis Cross --- compiler/rustc_lint/src/async_fn_in_trait.rs | 9 +++++++-- 1 file changed, 7 insertions(+), 2 deletions(-) diff --git a/compiler/rustc_lint/src/async_fn_in_trait.rs b/compiler/rustc_lint/src/async_fn_in_trait.rs index e0572b8ed43b1..ff4c81e2fc9b1 100644 --- a/compiler/rustc_lint/src/async_fn_in_trait.rs +++ b/compiler/rustc_lint/src/async_fn_in_trait.rs @@ -68,10 +68,15 @@ declare_lint! { /// } /// ``` /// + /// 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 [auto traits] - /// such as [`Send`], then the lint may be suppressed. + /// 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