From 306702ec55f367bae77ac222f6c634437479f756 Mon Sep 17 00:00:00 2001 From: Taylor Cramer Date: Wed, 20 Nov 2024 14:19:36 -0800 Subject: [PATCH] Add shorter and more direct error for dyn AsyncFn Fix #132713 --- .../src/error_codes/E0802.md | 37 ++++++++ compiler/rustc_error_codes/src/lib.rs | 1 + .../src/hir_ty_lowering/dyn_compatibility.rs | 15 ++-- .../src/hir_ty_lowering/errors.rs | 1 + compiler/rustc_middle/src/traits/mod.rs | 22 ++++- .../src/error_reporting/traits/mod.rs | 85 ++++++++++++++----- .../src/traits/dyn_compatibility.rs | 21 +++++ tests/ui/async-await/async-closures/dyn.rs | 39 +++++++++ .../ui/async-await/async-closures/dyn.stderr | 83 ++++++++++++++++++ tests/ui/async-await/async-fn/dyn-pos.rs | 5 +- tests/ui/async-await/async-fn/dyn-pos.stderr | 64 ++------------ 11 files changed, 282 insertions(+), 91 deletions(-) create mode 100644 compiler/rustc_error_codes/src/error_codes/E0802.md create mode 100644 tests/ui/async-await/async-closures/dyn.rs create mode 100644 tests/ui/async-await/async-closures/dyn.stderr diff --git a/compiler/rustc_error_codes/src/error_codes/E0802.md b/compiler/rustc_error_codes/src/error_codes/E0802.md new file mode 100644 index 0000000000000..8a0547b2306ac --- /dev/null +++ b/compiler/rustc_error_codes/src/error_codes/E0802.md @@ -0,0 +1,37 @@ +The `Async{Fn, FnMut, FnOnce}` traits are not yet dyn-compatible. + +Erroneous code example: + +```compile_fail,E0802,edition2018 +#![feature(async_closure)] +use core::ops::AsyncFn; + +async fn call_async_fn_twice(some_fn: &dyn AsyncFn()) { + some_fn().await; + some_fn().await; +} +``` + +One workaround to this issue is to use `impl Async...` instead: + +```edition2018 +#![feature(async_closure)] +use core::ops::AsyncFn; + +async fn call_async_fn_twice(some_fn: &impl AsyncFn()) { + some_fn().await; + some_fn().await; +} +``` + +This error indicates that you attempted to use `dyn AsyncFn`, +`dyn AsyncFnMut`, or `dyn AsyncFnOnce`. + +This is not yet possible because the `Async...` traits internally return +a concrete `Future` associated type. For dynamic callsites, it is impossible +to know the size of the returned `Future` object since different +`Async...` implementations may return differently-sized `Future` objects. + +This is analogous to the more general issue of creating a `dyn` type without +specifying associated types, e.g. `dyn Iterator` as opposed to +`dyn Iterator`. diff --git a/compiler/rustc_error_codes/src/lib.rs b/compiler/rustc_error_codes/src/lib.rs index 29f3277d3997e..0e4cb99b8aa0d 100644 --- a/compiler/rustc_error_codes/src/lib.rs +++ b/compiler/rustc_error_codes/src/lib.rs @@ -541,6 +541,7 @@ E0798: 0798, E0799: 0799, E0800: 0800, E0801: 0801, +E0802: 0802, ); ) } diff --git a/compiler/rustc_hir_analysis/src/hir_ty_lowering/dyn_compatibility.rs b/compiler/rustc_hir_analysis/src/hir_ty_lowering/dyn_compatibility.rs index 5e27ace4cbe4a..63233f21b60c3 100644 --- a/compiler/rustc_hir_analysis/src/hir_ty_lowering/dyn_compatibility.rs +++ b/compiler/rustc_hir_analysis/src/hir_ty_lowering/dyn_compatibility.rs @@ -103,17 +103,12 @@ impl<'tcx> dyn HirTyLowerer<'tcx> + '_ { // most importantly, that the supertraits don't contain `Self`, // to avoid ICEs. for item in ®ular_traits { - let violations = - hir_ty_lowering_dyn_compatibility_violations(tcx, item.trait_ref().def_id()); + let item_def_id = item.trait_ref().def_id(); + let violations = hir_ty_lowering_dyn_compatibility_violations(tcx, item_def_id); if !violations.is_empty() { - let reported = report_dyn_incompatibility( - tcx, - span, - Some(hir_id), - item.trait_ref().def_id(), - &violations, - ) - .emit(); + let reported = + report_dyn_incompatibility(tcx, span, Some(hir_id), item_def_id, &violations) + .emit(); return Ty::new_error(tcx, reported); } } diff --git a/compiler/rustc_hir_analysis/src/hir_ty_lowering/errors.rs b/compiler/rustc_hir_analysis/src/hir_ty_lowering/errors.rs index dd0f250a8e28b..c7f23fc83582e 100644 --- a/compiler/rustc_hir_analysis/src/hir_ty_lowering/errors.rs +++ b/compiler/rustc_hir_analysis/src/hir_ty_lowering/errors.rs @@ -733,6 +733,7 @@ impl<'tcx> dyn HirTyLowerer<'tcx> + '_ { (span, def_ids.into_iter().map(|did| tcx.associated_item(did)).collect()) }) .collect(); + let mut names: FxIndexMap> = Default::default(); let mut names_len = 0; diff --git a/compiler/rustc_middle/src/traits/mod.rs b/compiler/rustc_middle/src/traits/mod.rs index 09731d565b619..c47be25439683 100644 --- a/compiler/rustc_middle/src/traits/mod.rs +++ b/compiler/rustc_middle/src/traits/mod.rs @@ -712,6 +712,22 @@ pub enum DynCompatibilityViolation { /// GAT GAT(Symbol, Span), + + /// Async{Fn, FnMut, FnOnce} + /// + /// `fn_trait` is the name of the `AsyncFn...` trait, + AsyncFnTrait { + /// The `AsyncFn...` trait referenced. + /// + /// This is useful for better error reporting in cases where the + /// `dyn`-incompatible trait inherits from `Async...`. + // + // FIXME(cramertj): I'd love for this to be a DefId for proper comparison + // in the error reporting stage, but sadly this isn't possible because + // DefIds cannot be stored at this stage. Is there a better way to handle + // catching the supertrait case than string comparison? + fn_trait: Symbol, + }, } impl DynCompatibilityViolation { @@ -779,6 +795,9 @@ impl DynCompatibilityViolation { DynCompatibilityViolation::GAT(name, _) => { format!("it contains the generic associated type `{name}`").into() } + DynCompatibilityViolation::AsyncFnTrait { .. } => { + "`async` function traits are not yet dyn-compatible".into() + } } } @@ -786,7 +805,8 @@ impl DynCompatibilityViolation { match self { DynCompatibilityViolation::SizedSelf(_) | DynCompatibilityViolation::SupertraitSelf(_) - | DynCompatibilityViolation::SupertraitNonLifetimeBinder(..) => { + | DynCompatibilityViolation::SupertraitNonLifetimeBinder(..) + | DynCompatibilityViolation::AsyncFnTrait { .. } => { DynCompatibilityViolationSolution::None } DynCompatibilityViolation::Method( diff --git a/compiler/rustc_trait_selection/src/error_reporting/traits/mod.rs b/compiler/rustc_trait_selection/src/error_reporting/traits/mod.rs index b108a9352a53d..90851ddd1c262 100644 --- a/compiler/rustc_trait_selection/src/error_reporting/traits/mod.rs +++ b/compiler/rustc_trait_selection/src/error_reporting/traits/mod.rs @@ -7,7 +7,7 @@ pub mod suggestions; use std::{fmt, iter}; use rustc_data_structures::fx::{FxIndexMap, FxIndexSet}; -use rustc_errors::{Applicability, Diag, E0038, E0276, MultiSpan, struct_span_code_err}; +use rustc_errors::{Applicability, Diag, E0038, E0276, E0802, MultiSpan, struct_span_code_err}; use rustc_hir::def_id::{DefId, LocalDefId}; use rustc_hir::intravisit::Visitor; use rustc_hir::{self as hir, LangItem}; @@ -428,10 +428,46 @@ pub fn report_dyn_incompatibility<'tcx>( violations: &[DynCompatibilityViolation], ) -> Diag<'tcx> { let trait_str = tcx.def_path_str(trait_def_id); + + // Avoid errors diving into the details of the `AsyncFn` traits if this is + // a straightforward "`AsyncFn` is not yet dyn-compatible" error. + if let Some(async_fn_trait_kind) = tcx.async_fn_trait_kind_from_def_id(trait_def_id) { + let async_fn_trait_name = match async_fn_trait_kind { + ty::ClosureKind::Fn => "AsyncFn", + ty::ClosureKind::FnMut => "AsyncFnMut", + ty::ClosureKind::FnOnce => "AsyncFnOnce", + }; + + let mut err = struct_span_code_err!( + tcx.dcx(), + span, + E0802, + "the trait `{}` is not yet dyn-compatible", + async_fn_trait_name + ); + // Note: this check is quite imprecise. + // Comparing the DefIds or similar would be better, but we can't store + // DefIds in `DynCompatibilityViolation`. + if async_fn_trait_name == trait_str { + err.span_label(span, format!("`{async_fn_trait_name}` is not yet dyn compatible")); + } else { + let trait_str = tcx.def_path_str(trait_def_id); + err.span_label( + span, + format!( + "`{trait_str}` inherits from `{async_fn_trait_name}` which is not yet dyn-compatible'" + ), + ); + } + attempt_dyn_to_impl_suggestion(tcx, hir_id, &mut err); + return err; + } + let trait_span = tcx.hir().get_if_local(trait_def_id).and_then(|node| match node { hir::Node::Item(item) => Some(item.ident.span), _ => None, }); + let mut err = struct_span_code_err!( tcx.dcx(), span, @@ -441,24 +477,8 @@ pub fn report_dyn_incompatibility<'tcx>( ); err.span_label(span, format!("`{trait_str}` cannot be made into an object")); - if let Some(hir_id) = hir_id - && let hir::Node::Ty(ty) = tcx.hir_node(hir_id) - && let hir::TyKind::TraitObject([trait_ref, ..], ..) = ty.kind - { - let mut hir_id = hir_id; - while let hir::Node::Ty(ty) = tcx.parent_hir_node(hir_id) { - hir_id = ty.hir_id; - } - if tcx.parent_hir_node(hir_id).fn_sig().is_some() { - // Do not suggest `impl Trait` when dealing with things like super-traits. - err.span_suggestion_verbose( - ty.span.until(trait_ref.span), - "consider using an opaque type instead", - "impl ", - Applicability::MaybeIncorrect, - ); - } - } + attempt_dyn_to_impl_suggestion(tcx, hir_id, &mut err); + let mut reported_violations = FxIndexSet::default(); let mut multi_span = vec![]; let mut messages = vec![]; @@ -583,3 +603,30 @@ pub fn report_dyn_incompatibility<'tcx>( err } + +fn attempt_dyn_to_impl_suggestion(tcx: TyCtxt<'_>, hir_id: Option, err: &mut Diag<'_>) { + let Some(hir_id) = hir_id else { return }; + let hir::Node::Ty(ty) = tcx.hir_node(hir_id) else { return }; + let hir::TyKind::TraitObject([trait_ref, ..], ..) = ty.kind else { return }; + + // Only suggest converting `dyn` to `impl` if we're in a function signature. + // Get the top-most parent element which is a type. + let parent_ty_hir_id = tcx + .hir() + .parent_iter(hir_id) + .take_while(|(_id, node)| matches!(node, hir::Node::Ty(_))) + .last() + .map(|(id, _node)| id) + .unwrap_or(hir_id); + if tcx.parent_hir_node(parent_ty_hir_id).fn_sig().is_none() { + // Do not suggest `impl Trait` when dealing with things like super-traits. + return; + } + + err.span_suggestion_verbose( + ty.span.until(trait_ref.span), + "consider using an opaque type instead", + "impl ", + Applicability::MaybeIncorrect, + ); +} diff --git a/compiler/rustc_trait_selection/src/traits/dyn_compatibility.rs b/compiler/rustc_trait_selection/src/traits/dyn_compatibility.rs index c00246cfd7d0f..468aff2285313 100644 --- a/compiler/rustc_trait_selection/src/traits/dyn_compatibility.rs +++ b/compiler/rustc_trait_selection/src/traits/dyn_compatibility.rs @@ -39,6 +39,16 @@ pub fn hir_ty_lowering_dyn_compatibility_violations( trait_def_id: DefId, ) -> Vec { debug_assert!(tcx.generics_of(trait_def_id).has_self); + + // Check for `AsyncFn` traits first to avoid reporting various other + // errors about the details of why these traits aren't dyn-compatible. + for supertrait in tcx.supertrait_def_ids(trait_def_id) { + if tcx.async_fn_trait_kind_from_def_id(supertrait).is_some() { + let fn_trait = tcx.item_name(supertrait); + return vec![DynCompatibilityViolation::AsyncFnTrait { fn_trait }]; + } + } + tcx.supertrait_def_ids(trait_def_id) .map(|def_id| predicates_reference_self(tcx, def_id, true)) .filter(|spans| !spans.is_empty()) @@ -53,6 +63,17 @@ fn dyn_compatibility_violations( debug_assert!(tcx.generics_of(trait_def_id).has_self); debug!("dyn_compatibility_violations: {:?}", trait_def_id); + // Check for `AsyncFn` traits first to avoid reporting various other + // errors about the details of why these traits aren't dyn-compatible. + for supertrait in tcx.supertrait_def_ids(trait_def_id) { + if tcx.async_fn_trait_kind_from_def_id(supertrait).is_some() { + let fn_trait = tcx.item_name(supertrait); + return core::slice::from_ref( + tcx.arena.alloc(DynCompatibilityViolation::AsyncFnTrait { fn_trait }), + ); + } + } + tcx.arena.alloc_from_iter( tcx.supertrait_def_ids(trait_def_id) .flat_map(|def_id| dyn_compatibility_violations_for_trait(tcx, def_id)), diff --git a/tests/ui/async-await/async-closures/dyn.rs b/tests/ui/async-await/async-closures/dyn.rs new file mode 100644 index 0000000000000..3b26211a08baa --- /dev/null +++ b/tests/ui/async-await/async-closures/dyn.rs @@ -0,0 +1,39 @@ +// Test the diagnostic output (error descriptions) resulting from `dyn AsyncFn{,Mut,Once}`. +//@ edition:2018 + +#![feature(async_closure)] + +use core::ops::{AsyncFn, AsyncFnMut, AsyncFnOnce}; + +// --- Explicit `dyn` --- + +fn takes_async_fn(_: &dyn AsyncFn()) {} +//~^ ERROR the trait `AsyncFn` is not yet dyn-compatible + +fn takes_async_fn_mut(_: &mut dyn AsyncFnMut()) {} +//~^ ERROR the trait `AsyncFnMut` is not yet dyn-compatible + +fn takes_async_fn_once(_: Box) {} +//~^ ERROR the trait `AsyncFnOnce` is not yet dyn-compatible + +// --- Non-explicit `dyn` --- + +#[allow(bare_trait_objects)] +fn takes_async_fn_implicit_dyn(_: &AsyncFn()) {} +//~^ ERROR the trait `AsyncFn` is not yet dyn-compatible + +#[allow(bare_trait_objects)] +fn takes_async_fn_mut_implicit_dyn(_: &mut AsyncFnMut()) {} +//~^ ERROR the trait `AsyncFnMut` is not yet dyn-compatible + +#[allow(bare_trait_objects)] +fn takes_async_fn_once_implicit_dyn(_: Box) {} +//~^ ERROR the trait `AsyncFnOnce` is not yet dyn-compatible + +// --- Supertrait --- + +trait SubAsyncFn: AsyncFn() {} +fn takes_sub_async_fn(_: &dyn SubAsyncFn) {} +//~^ ERROR the trait `SubAsyncFn` cannot be made into an object + +fn main() {} diff --git a/tests/ui/async-await/async-closures/dyn.stderr b/tests/ui/async-await/async-closures/dyn.stderr new file mode 100644 index 0000000000000..6becce0a26f72 --- /dev/null +++ b/tests/ui/async-await/async-closures/dyn.stderr @@ -0,0 +1,83 @@ +error[E0802]: the trait `AsyncFn` is not yet dyn-compatible + --> $DIR/dyn.rs:10:23 + | +LL | fn takes_async_fn(_: &dyn AsyncFn()) {} + | ^^^^^^^^^^^^^ `AsyncFn` is not yet dyn compatible + | +help: consider using an opaque type instead + | +LL | fn takes_async_fn(_: &impl AsyncFn()) {} + | ~~~~ + +error[E0802]: the trait `AsyncFnMut` is not yet dyn-compatible + --> $DIR/dyn.rs:13:31 + | +LL | fn takes_async_fn_mut(_: &mut dyn AsyncFnMut()) {} + | ^^^^^^^^^^^^^^^^ `AsyncFnMut` is not yet dyn compatible + | +help: consider using an opaque type instead + | +LL | fn takes_async_fn_mut(_: &mut impl AsyncFnMut()) {} + | ~~~~ + +error[E0802]: the trait `AsyncFnOnce` is not yet dyn-compatible + --> $DIR/dyn.rs:16:31 + | +LL | fn takes_async_fn_once(_: Box) {} + | ^^^^^^^^^^^^^^^^^ `AsyncFnOnce` is not yet dyn compatible + | +help: consider using an opaque type instead + | +LL | fn takes_async_fn_once(_: Box) {} + | ~~~~ + +error[E0802]: the trait `AsyncFn` is not yet dyn-compatible + --> $DIR/dyn.rs:22:36 + | +LL | fn takes_async_fn_implicit_dyn(_: &AsyncFn()) {} + | ^^^^^^^^^ `AsyncFn` is not yet dyn compatible + | +help: consider using an opaque type instead + | +LL | fn takes_async_fn_implicit_dyn(_: &impl AsyncFn()) {} + | ++++ + +error[E0802]: the trait `AsyncFnMut` is not yet dyn-compatible + --> $DIR/dyn.rs:26:44 + | +LL | fn takes_async_fn_mut_implicit_dyn(_: &mut AsyncFnMut()) {} + | ^^^^^^^^^^^^ `AsyncFnMut` is not yet dyn compatible + | +help: consider using an opaque type instead + | +LL | fn takes_async_fn_mut_implicit_dyn(_: &mut impl AsyncFnMut()) {} + | ++++ + +error[E0802]: the trait `AsyncFnOnce` is not yet dyn-compatible + --> $DIR/dyn.rs:30:44 + | +LL | fn takes_async_fn_once_implicit_dyn(_: Box) {} + | ^^^^^^^^^^^^^ `AsyncFnOnce` is not yet dyn compatible + | +help: consider using an opaque type instead + | +LL | fn takes_async_fn_once_implicit_dyn(_: Box) {} + | ++++ + +error[E0038]: the trait `SubAsyncFn` cannot be made into an object + --> $DIR/dyn.rs:36:27 + | +LL | fn takes_sub_async_fn(_: &dyn SubAsyncFn) {} + | ^^^^^^^^^^^^^^ `SubAsyncFn` cannot be made into an object + | + = note: the trait cannot be made into an object because `async` function traits are not yet dyn-compatible + = note: for a trait to be "dyn-compatible" it needs to allow building a vtable to allow the call to be resolvable dynamically; for more information visit +help: consider using an opaque type instead + | +LL | fn takes_sub_async_fn(_: &impl SubAsyncFn) {} + | ~~~~ + +error: aborting due to 7 previous errors + +Some errors have detailed explanations: E0038, E0802. +For more information about an error, try `rustc --explain E0038`. diff --git a/tests/ui/async-await/async-fn/dyn-pos.rs b/tests/ui/async-await/async-fn/dyn-pos.rs index 772c7d15cfd49..91bf8ffddb628 100644 --- a/tests/ui/async-await/async-fn/dyn-pos.rs +++ b/tests/ui/async-await/async-fn/dyn-pos.rs @@ -3,9 +3,6 @@ #![feature(async_closure)] fn foo(x: &dyn async Fn()) {} -//~^ ERROR the trait `AsyncFn` cannot be made into an object -//~| ERROR the trait `AsyncFnMut` cannot be made into an object -//~| ERROR the trait `AsyncFnMut` cannot be made into an object -//~| ERROR the trait `AsyncFnMut` cannot be made into an object +//~^ ERROR the trait `AsyncFn` is not yet dyn-compatible fn main() {} diff --git a/tests/ui/async-await/async-fn/dyn-pos.stderr b/tests/ui/async-await/async-fn/dyn-pos.stderr index 78e915d49e78b..9961e7e79f0c1 100644 --- a/tests/ui/async-await/async-fn/dyn-pos.stderr +++ b/tests/ui/async-await/async-fn/dyn-pos.stderr @@ -1,64 +1,14 @@ -error[E0038]: the trait `AsyncFnMut` cannot be made into an object - --> $DIR/dyn-pos.rs:5:16 - | -LL | fn foo(x: &dyn async Fn()) {} - | ^^^^^^^^^^ `AsyncFnMut` cannot be made into an object - | -note: for a trait to be "dyn-compatible" it needs to allow building a vtable to allow the call to be resolvable dynamically; for more information visit - --> $SRC_DIR/core/src/ops/async_function.rs:LL:COL - | - = note: the trait cannot be made into an object because it contains the generic associated type `CallRefFuture` - = help: the following types implement the trait, consider defining an enum where each variant holds one of these types, implementing `AsyncFnMut` for this new enum and using it instead: - &F - &mut F - std::boxed::Box - -error[E0038]: the trait `AsyncFnMut` cannot be made into an object - --> $DIR/dyn-pos.rs:5:16 - | -LL | fn foo(x: &dyn async Fn()) {} - | ^^^^^^^^^^ `AsyncFnMut` cannot be made into an object - | -note: for a trait to be "dyn-compatible" it needs to allow building a vtable to allow the call to be resolvable dynamically; for more information visit - --> $SRC_DIR/core/src/ops/async_function.rs:LL:COL - | - = note: the trait cannot be made into an object because it contains the generic associated type `CallRefFuture` - = help: the following types implement the trait, consider defining an enum where each variant holds one of these types, implementing `AsyncFnMut` for this new enum and using it instead: - &F - &mut F - std::boxed::Box - = note: duplicate diagnostic emitted due to `-Z deduplicate-diagnostics=no` - -error[E0038]: the trait `AsyncFnMut` cannot be made into an object - --> $DIR/dyn-pos.rs:5:16 - | -LL | fn foo(x: &dyn async Fn()) {} - | ^^^^^^^^^^ `AsyncFnMut` cannot be made into an object - | -note: for a trait to be "dyn-compatible" it needs to allow building a vtable to allow the call to be resolvable dynamically; for more information visit - --> $SRC_DIR/core/src/ops/async_function.rs:LL:COL - | - = note: the trait cannot be made into an object because it contains the generic associated type `CallRefFuture` - = help: the following types implement the trait, consider defining an enum where each variant holds one of these types, implementing `AsyncFnMut` for this new enum and using it instead: - &F - &mut F - std::boxed::Box - = note: duplicate diagnostic emitted due to `-Z deduplicate-diagnostics=no` - -error[E0038]: the trait `AsyncFn` cannot be made into an object +error[E0802]: the trait `AsyncFn` is not yet dyn-compatible --> $DIR/dyn-pos.rs:5:12 | LL | fn foo(x: &dyn async Fn()) {} - | ^^^^^^^^^^^^^^ `AsyncFn` cannot be made into an object + | ^^^^^^^^^^^^^^ `AsyncFn` is not yet dyn compatible | -note: for a trait to be "dyn-compatible" it needs to allow building a vtable to allow the call to be resolvable dynamically; for more information visit - --> $SRC_DIR/core/src/ops/async_function.rs:LL:COL +help: consider using an opaque type instead | - = note: the trait cannot be made into an object because it contains the generic associated type `CallRefFuture` - = help: the following types implement the trait, consider defining an enum where each variant holds one of these types, implementing `AsyncFn` for this new enum and using it instead: - &F - std::boxed::Box +LL | fn foo(x: &impl async Fn()) {} + | ~~~~ -error: aborting due to 4 previous errors +error: aborting due to 1 previous error -For more information about this error, try `rustc --explain E0038`. +For more information about this error, try `rustc --explain E0802`.