Skip to content

Commit

Permalink
Add shorter and more direct error for dyn AsyncFn
Browse files Browse the repository at this point in the history
  • Loading branch information
cramertj committed Nov 20, 2024
1 parent 03ee484 commit 1e17c14
Show file tree
Hide file tree
Showing 14 changed files with 290 additions and 92 deletions.
30 changes: 30 additions & 0 deletions compiler/rustc_error_codes/src/error_codes/E0802.md
Original file line number Diff line number Diff line change
@@ -0,0 +1,30 @@
The `Async{Fn, FnMut, FnOnce}` traits are not yet `dyn`-compatible.
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<Item = SomeItemType>`.

Erroneous code example:

```compile_fail,E0802
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:

```
async fn call_async_fn_twice(some_fn: &impl AsyncFn()) {
some_fn().await;
some_fn().await;
}
```
1 change: 1 addition & 0 deletions compiler/rustc_error_codes/src/lib.rs
Original file line number Diff line number Diff line change
Expand Up @@ -541,6 +541,7 @@ E0798: 0798,
E0799: 0799,
E0800: 0800,
E0801: 0801,
E0802: 0802,
);
)
}
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -103,17 +103,12 @@ impl<'tcx> dyn HirTyLowerer<'tcx> + '_ {
// most importantly, that the supertraits don't contain `Self`,
// to avoid ICEs.
for item in &regular_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);
}
}
Expand Down
1 change: 1 addition & 0 deletions compiler/rustc_hir_analysis/src/hir_ty_lowering/errors.rs
Original file line number Diff line number Diff line change
Expand Up @@ -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<String, Vec<Symbol>> = Default::default();
let mut names_len = 0;

Expand Down
22 changes: 21 additions & 1 deletion compiler/rustc_middle/src/traits/mod.rs
Original file line number Diff line number Diff line change
Expand Up @@ -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 {
Expand Down Expand Up @@ -779,14 +795,18 @@ 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()
}
}
}

pub fn solution(&self) -> DynCompatibilityViolationSolution {
match self {
DynCompatibilityViolation::SizedSelf(_)
| DynCompatibilityViolation::SupertraitSelf(_)
| DynCompatibilityViolation::SupertraitNonLifetimeBinder(..) => {
| DynCompatibilityViolation::SupertraitNonLifetimeBinder(..)
| DynCompatibilityViolation::AsyncFnTrait { .. } => {
DynCompatibilityViolationSolution::None
}
DynCompatibilityViolation::Method(
Expand Down
4 changes: 4 additions & 0 deletions compiler/rustc_middle/src/ty/context.rs
Original file line number Diff line number Diff line change
Expand Up @@ -560,6 +560,10 @@ impl<'tcx> Interner for TyCtxt<'tcx> {
self.trait_is_alias(trait_def_id)
}

fn trait_is_async_fn(self, trait_def_id: DefId) -> bool {
self.trait_is_async_fn(trait_def_id)
}

fn trait_is_dyn_compatible(self, trait_def_id: DefId) -> bool {
self.is_dyn_compatible(trait_def_id)
}
Expand Down
11 changes: 11 additions & 0 deletions compiler/rustc_middle/src/ty/mod.rs
Original file line number Diff line number Diff line change
Expand Up @@ -1830,6 +1830,17 @@ impl<'tcx> TyCtxt<'tcx> {
self.trait_def(trait_def_id).has_auto_impl
}

/// Returns `true` if this is an `AsyncFn`, `AsyncFnMut`, or `AsyncFnOnce` trait.
pub fn trait_is_async_fn(self, trait_def_id: DefId) -> bool {
let lang_items = self.lang_items();
[
lang_items.async_fn_trait(),
lang_items.async_fn_mut_trait(),
lang_items.async_fn_once_trait(),
]
.contains(&Some(trait_def_id))
}

/// Returns `true` if this is coinductive, either because it is
/// an auto trait or because it has the `#[rustc_coinductive]` attribute.
pub fn trait_is_coinductive(self, trait_def_id: DefId) -> bool {
Expand Down
84 changes: 64 additions & 20 deletions compiler/rustc_trait_selection/src/error_reporting/traits/mod.rs
Original file line number Diff line number Diff line change
Expand Up @@ -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};
Expand All @@ -17,7 +17,7 @@ use rustc_infer::traits::{
};
use rustc_middle::ty::print::{PrintTraitRefExt as _, with_no_trimmed_paths};
use rustc_middle::ty::{self, Ty, TyCtxt};
use rustc_span::{ErrorGuaranteed, ExpnKind, Span};
use rustc_span::{ErrorGuaranteed, ExpnKind, Span, Symbol};
use tracing::{info, instrument};

pub use self::overflow::*;
Expand Down Expand Up @@ -428,10 +428,50 @@ 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 tcx.trait_is_async_fn(trait_def_id) {
let fn_trait: Symbol = violations
.iter()
.find_map(|violation| {
// Try to find the original trait that caused the violation.
match *violation {
DynCompatibilityViolation::AsyncFnTrait { fn_trait } => Some(fn_trait),
_ => None,
}
})
.expect("AsyncFn traits should report a corresponding DynCompatibilityViolation");
let mut err = struct_span_code_err!(
tcx.dcx(),
span,
E0802,
"the trait `{}` is not yet `dyn`-compatible",
fn_trait
);
// Note: this check is quite imprecise.
// Comparing the DefIds or similar would be better, but we can't store
// DefIds in `DynCompatibilityViolation`.
if fn_trait.as_str() == trait_str {
err.span_label(span, format!("`{fn_trait}` 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 `{fn_trait}` 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,
Expand All @@ -441,24 +481,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![];
Expand Down Expand Up @@ -583,3 +607,23 @@ pub fn report_dyn_incompatibility<'tcx>(

err
}

fn attempt_dyn_to_impl_suggestion(tcx: TyCtxt<'_>, hir_id: Option<hir::HirId>, 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 };
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_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,
);
}
21 changes: 21 additions & 0 deletions compiler/rustc_trait_selection/src/traits/dyn_compatibility.rs
Original file line number Diff line number Diff line change
Expand Up @@ -39,6 +39,16 @@ pub fn hir_ty_lowering_dyn_compatibility_violations(
trait_def_id: DefId,
) -> Vec<DynCompatibilityViolation> {
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 object-safe.
for supertrait in tcx.supertrait_def_ids(trait_def_id) {
if tcx.trait_is_async_fn(supertrait) {
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())
Expand All @@ -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 object-safe.
for supertrait in tcx.supertrait_def_ids(trait_def_id) {
if tcx.trait_is_async_fn(supertrait) {
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)),
Expand Down
2 changes: 2 additions & 0 deletions compiler/rustc_type_ir/src/interner.rs
Original file line number Diff line number Diff line change
Expand Up @@ -262,6 +262,8 @@ pub trait Interner:

fn trait_is_alias(self, trait_def_id: Self::DefId) -> bool;

fn trait_is_async_fn(self, trait_def_id: Self::DefId) -> bool;

fn trait_is_dyn_compatible(self, trait_def_id: Self::DefId) -> bool;

fn trait_is_fundamental(self, def_id: Self::DefId) -> bool;
Expand Down
39 changes: 39 additions & 0 deletions tests/ui/async-await/async-closures/dyn.rs
Original file line number Diff line number Diff line change
@@ -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<dyn AsyncFnOnce()>) {}
//~^ 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<AsyncFnOnce()>) {}
//~^ 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() {}
Loading

0 comments on commit 1e17c14

Please sign in to comment.