Rollup merge of rust-lang#120103 - compiler-errors:concrete-afits, r=oli-obk

Make it so that async-fn-in-trait is compatible with a concrete future in implementation

There's no technical reason why an AFIT like `async fn foo()` cannot be satisfied with an implementation signature like `fn foo() -> Pin<Box<dyn Future<Output = ()> + 'static>>`.

We rejected this previously because we were uncertain about how AFITs worked with refinement, but I don't believe this needs to be a restriction any longer.

r? oli-obk

Author: matthiaskrgr

compiler/rustc_hir_analysis/messages.ftl

@@ -33,10 +33,6 @@ hir_analysis_associated_type_trait_uninferred_generic_params = cannot use the as
 
 hir_analysis_associated_type_trait_uninferred_generic_params_multipart_suggestion = use a fully qualified path with explicit lifetimes
 
-hir_analysis_async_trait_impl_should_be_async =
-    method `{$method_name}` should be async because the method from the trait is async
-    .trait_item_label = required because the trait method is async
-
 hir_analysis_auto_deref_reached_recursion_limit = reached the recursion limit while auto-dereferencing `{$ty}`
     .label = deref recursion limit reached
     .help = consider increasing the recursion limit by adding a `#![recursion_limit = "{$suggested_limit}"]` attribute to your crate (`{$crate_name}`)
@@ -210,6 +206,9 @@ hir_analysis_manual_implementation =
     .label = manual implementations of `{$trait_name}` are experimental
     .help = add `#![feature(unboxed_closures)]` to the crate attributes to enable
 
+hir_analysis_method_should_return_future = method should be `async` or return a future, but it is synchronous
+    .note = this method is `async` so it expects a future to be returned
+
 hir_analysis_missing_one_of_trait_item = not all trait items implemented, missing one of: `{$missing_items_msg}`
     .label = missing one of `{$missing_items_msg}` in implementation
     .note = required because of this annotation

compiler/rustc_hir_analysis/src/check/compare_impl_item.rs

@@ -1,5 +1,5 @@
 use super::potentially_plural_count;
-use crate::errors::LifetimesOrBoundsMismatchOnTrait;
+use crate::errors::{LifetimesOrBoundsMismatchOnTrait, MethodShouldReturnFuture};
 use hir::def_id::{DefId, DefIdMap, LocalDefId};
 use rustc_data_structures::fx::{FxHashMap, FxHashSet, FxIndexSet};
 use rustc_errors::{codes::*, pluralize, struct_span_code_err, Applicability, ErrorGuaranteed};
@@ -10,7 +10,7 @@ use rustc_hir::{GenericParamKind, ImplItemKind};
 use rustc_infer::infer::outlives::env::OutlivesEnvironment;
 use rustc_infer::infer::type_variable::{TypeVariableOrigin, TypeVariableOriginKind};
 use rustc_infer::infer::{self, InferCtxt, TyCtxtInferExt};
-use rustc_infer::traits::util;
+use rustc_infer::traits::{util, FulfillmentError};
 use rustc_middle::ty::error::{ExpectedFound, TypeError};
 use rustc_middle::ty::fold::BottomUpFolder;
 use rustc_middle::ty::util::ExplicitSelf;
@@ -74,7 +74,6 @@ fn check_method_is_structurally_compatible<'tcx>(
     compare_generic_param_kinds(tcx, impl_m, trait_m, delay)?;
     compare_number_of_method_arguments(tcx, impl_m, trait_m, delay)?;
     compare_synthetic_generics(tcx, impl_m, trait_m, delay)?;
-    compare_asyncness(tcx, impl_m, trait_m, delay)?;
     check_region_bounds_on_impl_item(tcx, impl_m, trait_m, delay)?;
     Ok(())
 }
@@ -414,36 +413,6 @@ impl<'tcx> TypeFolder<TyCtxt<'tcx>> for RemapLateBound<'_, 'tcx> {
     }
 }
 
-fn compare_asyncness<'tcx>(
-    tcx: TyCtxt<'tcx>,
-    impl_m: ty::AssocItem,
-    trait_m: ty::AssocItem,
-    delay: bool,
-) -> Result<(), ErrorGuaranteed> {
-    if tcx.asyncness(trait_m.def_id).is_async() {
-        match tcx.fn_sig(impl_m.def_id).skip_binder().skip_binder().output().kind() {
-            ty::Alias(ty::Opaque, ..) => {
-                // allow both `async fn foo()` and `fn foo() -> impl Future`
-            }
-            ty::Error(_) => {
-                // We don't know if it's ok, but at least it's already an error.
-            }
-            _ => {
-                return Err(tcx
-                    .dcx()
-                    .create_err(crate::errors::AsyncTraitImplShouldBeAsync {
-                        span: tcx.def_span(impl_m.def_id),
-                        method_name: trait_m.name,
-                        trait_item_span: tcx.hir().span_if_local(trait_m.def_id),
-                    })
-                    .emit_unless(delay));
-            }
-        };
-    }
-
-    Ok(())
-}
-
 /// Given a method def-id in an impl, compare the method signature of the impl
 /// against the trait that it's implementing. In doing so, infer the hidden types
 /// that this method's signature provides to satisfy each return-position `impl Trait`
@@ -695,8 +664,13 @@ pub(super) fn collect_return_position_impl_trait_in_trait_tys<'tcx>(
     // RPITs.
     let errors = ocx.select_all_or_error();
     if !errors.is_empty() {
-        let reported = infcx.err_ctxt().report_fulfillment_errors(errors);
-        return Err(reported);
+        if let Err(guar) = try_report_async_mismatch(tcx, infcx, &errors, trait_m, impl_m, impl_sig)
+        {
+            return Err(guar);
+        }
+
+        let guar = infcx.err_ctxt().report_fulfillment_errors(errors);
+        return Err(guar);
     }
 
     // Finally, resolve all regions. This catches wily misuses of
@@ -2248,3 +2222,47 @@ fn assoc_item_kind_str(impl_item: &ty::AssocItem) -> &'static str {
         ty::AssocKind::Type => "type",
     }
 }
+
+/// Manually check here that `async fn foo()` wasn't matched against `fn foo()`,
+/// and extract a better error if so.
+fn try_report_async_mismatch<'tcx>(
+    tcx: TyCtxt<'tcx>,
+    infcx: &InferCtxt<'tcx>,
+    errors: &[FulfillmentError<'tcx>],
+    trait_m: ty::AssocItem,
+    impl_m: ty::AssocItem,
+    impl_sig: ty::FnSig<'tcx>,
+) -> Result<(), ErrorGuaranteed> {
+    if !tcx.asyncness(trait_m.def_id).is_async() {
+        return Ok(());
+    }
+
+    let ty::Alias(ty::Projection, ty::AliasTy { def_id: async_future_def_id, .. }) =
+        *tcx.fn_sig(trait_m.def_id).skip_binder().skip_binder().output().kind()
+    else {
+        bug!("expected `async fn` to return an RPITIT");
+    };
+
+    for error in errors {
+        if let traits::BindingObligation(def_id, _) = *error.root_obligation.cause.code()
+            && def_id == async_future_def_id
+            && let Some(proj) = error.root_obligation.predicate.to_opt_poly_projection_pred()
+            && let Some(proj) = proj.no_bound_vars()
+            && infcx.can_eq(
+                error.root_obligation.param_env,
+                proj.term.ty().unwrap(),
+                impl_sig.output(),
+            )
+        {
+            // FIXME: We should suggest making the fn `async`, but extracting
+            // the right span is a bit difficult.
+            return Err(tcx.sess.dcx().emit_err(MethodShouldReturnFuture {
+                span: tcx.def_span(impl_m.def_id),
+                method_name: trait_m.name,
+                trait_item_span: tcx.hir().span_if_local(trait_m.def_id),
+            }));
+        }
+    }
+
+    Ok(())
+}

compiler/rustc_hir_analysis/src/errors.rs

@@ -166,17 +166,6 @@ pub struct LifetimesOrBoundsMismatchOnTrait {
     pub ident: Ident,
 }
 
-#[derive(Diagnostic)]
-#[diag(hir_analysis_async_trait_impl_should_be_async)]
-pub struct AsyncTraitImplShouldBeAsync {
-    #[primary_span]
-    // #[label]
-    pub span: Span,
-    #[label(hir_analysis_trait_item_label)]
-    pub trait_item_span: Option<Span>,
-    pub method_name: Symbol,
-}
-
 #[derive(Diagnostic)]
 #[diag(hir_analysis_drop_impl_on_wrong_item, code = E0120)]
 pub struct DropImplOnWrongItem {
@@ -1512,6 +1501,16 @@ pub struct NotSupportedDelegation<'a> {
     pub callee_span: Span,
 }
 
+#[derive(Diagnostic)]
+#[diag(hir_analysis_method_should_return_future)]
+pub struct MethodShouldReturnFuture {
+    #[primary_span]
+    pub span: Span,
+    pub method_name: Symbol,
+    #[note]
+    pub trait_item_span: Option<Span>,
+}
+
 #[derive(Diagnostic)]
 #[diag(hir_analysis_unused_generic_parameter)]
 pub(crate) struct UnusedGenericParameter {

tests/ui/async-await/in-trait/async-example-desugared-boxed.rs

@@ -1,15 +1,18 @@
 // edition: 2021
+// check-pass
 
 use std::future::Future;
 use std::pin::Pin;
 
-trait MyTrait {
+#[allow(async_fn_in_trait)]
+pub trait MyTrait {
     async fn foo(&self) -> i32;
 }
 
 impl MyTrait for i32 {
+    #[warn(refining_impl_trait)]
     fn foo(&self) -> Pin<Box<dyn Future<Output = i32> + '_>> {
-        //~^ ERROR method `foo` should be async
+        //~^ WARN impl trait in impl method signature does not match trait method signature
         Box::pin(async { *self })
     }
 }

tests/ui/async-await/in-trait/async-example-desugared-boxed.stderr

@@ -1,11 +1,22 @@
-error: method `foo` should be async because the method from the trait is async
-  --> $DIR/async-example-desugared-boxed.rs:11:5
+warning: impl trait in impl method signature does not match trait method signature
+  --> $DIR/async-example-desugared-boxed.rs:14:22
    |
 LL |     async fn foo(&self) -> i32;
-   |     --------------------------- required because the trait method is async
+   |     --------------------------- return type from trait method defined here
 ...
 LL |     fn foo(&self) -> Pin<Box<dyn Future<Output = i32> + '_>> {
-   |     ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^
+   |                      ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^
+   |
+   = note: add `#[allow(refining_impl_trait)]` if it is intended for this to be part of the public API of this crate
+note: the lint level is defined here
+  --> $DIR/async-example-desugared-boxed.rs:13:12
+   |
+LL |     #[warn(refining_impl_trait)]
+   |            ^^^^^^^^^^^^^^^^^^^
+help: replace the return type so that it matches the trait
+   |
+LL |     fn foo(&self) -> impl Future<Output = i32> {
+   |                      ~~~~~~~~~~~~~~~~~~~~~~~~~
 
-error: aborting due to 1 previous error
+warning: 1 warning emitted
 

tests/ui/async-await/in-trait/async-example-desugared-manual.rs

@@ -1,13 +1,15 @@
 // edition: 2021
+// check-pass
 
 use std::future::Future;
 use std::task::Poll;
 
-trait MyTrait {
+#[allow(async_fn_in_trait)]
+pub trait MyTrait {
     async fn foo(&self) -> i32;
 }
 
-struct MyFuture;
+pub struct MyFuture;
 impl Future for MyFuture {
     type Output = i32;
     fn poll(self: std::pin::Pin<&mut Self>, _: &mut std::task::Context<'_>) -> Poll<Self::Output> {
@@ -16,8 +18,9 @@ impl Future for MyFuture {
 }
 
 impl MyTrait for u32 {
+    #[warn(refining_impl_trait)]
     fn foo(&self) -> MyFuture {
-        //~^ ERROR method `foo` should be async
+        //~^ WARN impl trait in impl method signature does not match trait method signature
         MyFuture
     }
 }

tests/ui/async-await/in-trait/async-example-desugared-manual.stderr

@@ -1,11 +1,22 @@
-error: method `foo` should be async because the method from the trait is async
-  --> $DIR/async-example-desugared-manual.rs:19:5
+warning: impl trait in impl method signature does not match trait method signature
+  --> $DIR/async-example-desugared-manual.rs:22:22
    |
 LL |     async fn foo(&self) -> i32;
-   |     --------------------------- required because the trait method is async
+   |     --------------------------- return type from trait method defined here
 ...
 LL |     fn foo(&self) -> MyFuture {
-   |     ^^^^^^^^^^^^^^^^^^^^^^^^^
+   |                      ^^^^^^^^
+   |
+   = note: add `#[allow(refining_impl_trait)]` if it is intended for this to be part of the public API of this crate
+note: the lint level is defined here
+  --> $DIR/async-example-desugared-manual.rs:21:12
+   |
+LL |     #[warn(refining_impl_trait)]
+   |            ^^^^^^^^^^^^^^^^^^^
+help: replace the return type so that it matches the trait
+   |
+LL |     fn foo(&self) -> impl Future<Output = i32> {
+   |                      ~~~~~~~~~~~~~~~~~~~~~~~~~
 
-error: aborting due to 1 previous error
+warning: 1 warning emitted
 

tests/ui/async-await/in-trait/fn-not-async-err.rs

@@ -8,7 +8,7 @@ trait MyTrait {
 
 impl MyTrait for i32 {
     fn foo(&self) -> i32 {
-        //~^ ERROR: method `foo` should be async
+        //~^ ERROR: method should be `async` or return a future, but it is synchronous
         *self
     }
 }

tests/ui/async-await/in-trait/fn-not-async-err.stderr

@@ -1,11 +1,14 @@
-error: method `foo` should be async because the method from the trait is async
+error: method should be `async` or return a future, but it is synchronous
   --> $DIR/fn-not-async-err.rs:10:5
    |
-LL |     async fn foo(&self) -> i32;
-   |     --------------------------- required because the trait method is async
-...
 LL |     fn foo(&self) -> i32 {
    |     ^^^^^^^^^^^^^^^^^^^^
+   |
+note: this method is `async` so it expects a future to be returned
+  --> $DIR/fn-not-async-err.rs:6:5
+   |
+LL |     async fn foo(&self) -> i32;
+   |     ^^^^^^^^^^^^^^^^^^^^^^^^^^^
 
 error: aborting due to 1 previous error