From 16cbdd0321f342093b71026dc1c5126606e4abe9 Mon Sep 17 00:00:00 2001 From: Michael Goulet Date: Thu, 18 Jan 2024 16:39:20 +0000 Subject: [PATCH 1/3] Allow desugaring async fn in trait to compatible, concrete future types --- compiler/rustc_hir_analysis/messages.ftl | 4 --- .../src/check/compare_impl_item.rs | 31 ------------------- compiler/rustc_hir_analysis/src/errors.rs | 11 ------- .../in-trait/async-example-desugared-boxed.rs | 2 +- .../async-example-desugared-boxed.stderr | 11 ------- .../async-example-desugared-manual.rs | 2 +- .../async-example-desugared-manual.stderr | 11 ------- .../async-await/in-trait/fn-not-async-err.rs | 2 +- .../in-trait/fn-not-async-err.stderr | 18 +++++++---- 9 files changed, 15 insertions(+), 77 deletions(-) delete mode 100644 tests/ui/async-await/in-trait/async-example-desugared-boxed.stderr delete mode 100644 tests/ui/async-await/in-trait/async-example-desugared-manual.stderr diff --git a/compiler/rustc_hir_analysis/messages.ftl b/compiler/rustc_hir_analysis/messages.ftl index 54d0fb6ffab..d0fb0e3fb76 100644 --- a/compiler/rustc_hir_analysis/messages.ftl +++ b/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}`) diff --git a/compiler/rustc_hir_analysis/src/check/compare_impl_item.rs b/compiler/rustc_hir_analysis/src/check/compare_impl_item.rs index 379c1154e5f..f9c1ed0e0e1 100644 --- a/compiler/rustc_hir_analysis/src/check/compare_impl_item.rs +++ b/compiler/rustc_hir_analysis/src/check/compare_impl_item.rs @@ -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> 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` diff --git a/compiler/rustc_hir_analysis/src/errors.rs b/compiler/rustc_hir_analysis/src/errors.rs index 4eba31e327f..8b8c0f7ff8d 100644 --- a/compiler/rustc_hir_analysis/src/errors.rs +++ b/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, - pub method_name: Symbol, -} - #[derive(Diagnostic)] #[diag(hir_analysis_drop_impl_on_wrong_item, code = E0120)] pub struct DropImplOnWrongItem { diff --git a/tests/ui/async-await/in-trait/async-example-desugared-boxed.rs b/tests/ui/async-await/in-trait/async-example-desugared-boxed.rs index c5a9841029e..4008e09998e 100644 --- a/tests/ui/async-await/in-trait/async-example-desugared-boxed.rs +++ b/tests/ui/async-await/in-trait/async-example-desugared-boxed.rs @@ -1,4 +1,5 @@ // edition: 2021 +// check-pass use std::future::Future; use std::pin::Pin; @@ -9,7 +10,6 @@ trait MyTrait { impl MyTrait for i32 { fn foo(&self) -> Pin + '_>> { - //~^ ERROR method `foo` should be async Box::pin(async { *self }) } } diff --git a/tests/ui/async-await/in-trait/async-example-desugared-boxed.stderr b/tests/ui/async-await/in-trait/async-example-desugared-boxed.stderr deleted file mode 100644 index 1462c694e16..00000000000 --- a/tests/ui/async-await/in-trait/async-example-desugared-boxed.stderr +++ /dev/null @@ -1,11 +0,0 @@ -error: method `foo` should be async because the method from the trait is async - --> $DIR/async-example-desugared-boxed.rs:11:5 - | -LL | async fn foo(&self) -> i32; - | --------------------------- required because the trait method is async -... -LL | fn foo(&self) -> Pin + '_>> { - | ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ - -error: aborting due to 1 previous error - diff --git a/tests/ui/async-await/in-trait/async-example-desugared-manual.rs b/tests/ui/async-await/in-trait/async-example-desugared-manual.rs index c287b9a5b84..75f4ba1d076 100644 --- a/tests/ui/async-await/in-trait/async-example-desugared-manual.rs +++ b/tests/ui/async-await/in-trait/async-example-desugared-manual.rs @@ -1,4 +1,5 @@ // edition: 2021 +// check-pass use std::future::Future; use std::task::Poll; @@ -17,7 +18,6 @@ impl Future for MyFuture { impl MyTrait for u32 { fn foo(&self) -> MyFuture { - //~^ ERROR method `foo` should be async MyFuture } } diff --git a/tests/ui/async-await/in-trait/async-example-desugared-manual.stderr b/tests/ui/async-await/in-trait/async-example-desugared-manual.stderr deleted file mode 100644 index a2f1060e36f..00000000000 --- a/tests/ui/async-await/in-trait/async-example-desugared-manual.stderr +++ /dev/null @@ -1,11 +0,0 @@ -error: method `foo` should be async because the method from the trait is async - --> $DIR/async-example-desugared-manual.rs:19:5 - | -LL | async fn foo(&self) -> i32; - | --------------------------- required because the trait method is async -... -LL | fn foo(&self) -> MyFuture { - | ^^^^^^^^^^^^^^^^^^^^^^^^^ - -error: aborting due to 1 previous error - diff --git a/tests/ui/async-await/in-trait/fn-not-async-err.rs b/tests/ui/async-await/in-trait/fn-not-async-err.rs index 60077a7e00c..6261ed1f1b7 100644 --- a/tests/ui/async-await/in-trait/fn-not-async-err.rs +++ b/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: `i32` is not a future *self } } diff --git a/tests/ui/async-await/in-trait/fn-not-async-err.stderr b/tests/ui/async-await/in-trait/fn-not-async-err.stderr index f75ccb65d15..dc46077d583 100644 --- a/tests/ui/async-await/in-trait/fn-not-async-err.stderr +++ b/tests/ui/async-await/in-trait/fn-not-async-err.stderr @@ -1,11 +1,17 @@ -error: method `foo` should be async because the method from the trait is async - --> $DIR/fn-not-async-err.rs:10:5 +error[E0277]: `i32` is not a future + --> $DIR/fn-not-async-err.rs:10:22 + | +LL | fn foo(&self) -> i32 { + | ^^^ `i32` is not a future + | + = help: the trait `Future` is not implemented for `i32` + = note: i32 must be a future or must implement `IntoFuture` to be awaited +note: required by a bound in `MyTrait::{opaque#0}` + --> $DIR/fn-not-async-err.rs:6:5 | LL | async fn foo(&self) -> i32; - | --------------------------- required because the trait method is async -... -LL | fn foo(&self) -> i32 { - | ^^^^^^^^^^^^^^^^^^^^ + | ^^^^^^^^^^^^^^^^^^^^^^^^^^^ required by this bound in `MyTrait::{opaque#0}` error: aborting due to 1 previous error +For more information about this error, try `rustc --explain E0277`. From e65abc0ea58f2f7dd6812fafa92c4f5b2a28af7b Mon Sep 17 00:00:00 2001 From: Michael Goulet Date: Thu, 18 Jan 2024 17:20:39 +0000 Subject: [PATCH 2/3] Make the error message better --- compiler/rustc_hir_analysis/messages.ftl | 3 + .../src/check/compare_impl_item.rs | 57 +++++++++++++++++-- compiler/rustc_hir_analysis/src/errors.rs | 10 ++++ .../async-await/in-trait/fn-not-async-err.rs | 2 +- .../in-trait/fn-not-async-err.stderr | 13 ++--- 5 files changed, 72 insertions(+), 13 deletions(-) diff --git a/compiler/rustc_hir_analysis/messages.ftl b/compiler/rustc_hir_analysis/messages.ftl index d0fb0e3fb76..d6f604c180b 100644 --- a/compiler/rustc_hir_analysis/messages.ftl +++ b/compiler/rustc_hir_analysis/messages.ftl @@ -206,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 diff --git a/compiler/rustc_hir_analysis/src/check/compare_impl_item.rs b/compiler/rustc_hir_analysis/src/check/compare_impl_item.rs index f9c1ed0e0e1..18037ea6991 100644 --- a/compiler/rustc_hir_analysis/src/check/compare_impl_item.rs +++ b/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; @@ -664,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 @@ -2217,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(()) +} diff --git a/compiler/rustc_hir_analysis/src/errors.rs b/compiler/rustc_hir_analysis/src/errors.rs index 8b8c0f7ff8d..bec53693d6c 100644 --- a/compiler/rustc_hir_analysis/src/errors.rs +++ b/compiler/rustc_hir_analysis/src/errors.rs @@ -1501,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, +} + #[derive(Diagnostic)] #[diag(hir_analysis_unused_generic_parameter)] pub(crate) struct UnusedGenericParameter { diff --git a/tests/ui/async-await/in-trait/fn-not-async-err.rs b/tests/ui/async-await/in-trait/fn-not-async-err.rs index 6261ed1f1b7..ecd5737cf3c 100644 --- a/tests/ui/async-await/in-trait/fn-not-async-err.rs +++ b/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: `i32` is not a future + //~^ ERROR: method should be `async` or return a future, but it is synchronous *self } } diff --git a/tests/ui/async-await/in-trait/fn-not-async-err.stderr b/tests/ui/async-await/in-trait/fn-not-async-err.stderr index dc46077d583..8260cd5271e 100644 --- a/tests/ui/async-await/in-trait/fn-not-async-err.stderr +++ b/tests/ui/async-await/in-trait/fn-not-async-err.stderr @@ -1,17 +1,14 @@ -error[E0277]: `i32` is not a future - --> $DIR/fn-not-async-err.rs:10:22 +error: method should be `async` or return a future, but it is synchronous + --> $DIR/fn-not-async-err.rs:10:5 | LL | fn foo(&self) -> i32 { - | ^^^ `i32` is not a future + | ^^^^^^^^^^^^^^^^^^^^ | - = help: the trait `Future` is not implemented for `i32` - = note: i32 must be a future or must implement `IntoFuture` to be awaited -note: required by a bound in `MyTrait::{opaque#0}` +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; - | ^^^^^^^^^^^^^^^^^^^^^^^^^^^ required by this bound in `MyTrait::{opaque#0}` + | ^^^^^^^^^^^^^^^^^^^^^^^^^^^ error: aborting due to 1 previous error -For more information about this error, try `rustc --explain E0277`. From 1a3214b774f47b2fb8dadf305939f97e20993fe2 Mon Sep 17 00:00:00 2001 From: Michael Goulet Date: Thu, 18 Jan 2024 17:28:37 +0000 Subject: [PATCH 3/3] Make sure refinement still works --- .../in-trait/async-example-desugared-boxed.rs | 5 ++++- .../async-example-desugared-boxed.stderr | 22 +++++++++++++++++++ .../async-example-desugared-manual.rs | 7 ++++-- .../async-example-desugared-manual.stderr | 22 +++++++++++++++++++ 4 files changed, 53 insertions(+), 3 deletions(-) create mode 100644 tests/ui/async-await/in-trait/async-example-desugared-boxed.stderr create mode 100644 tests/ui/async-await/in-trait/async-example-desugared-manual.stderr diff --git a/tests/ui/async-await/in-trait/async-example-desugared-boxed.rs b/tests/ui/async-await/in-trait/async-example-desugared-boxed.rs index 4008e09998e..69871d0dca0 100644 --- a/tests/ui/async-await/in-trait/async-example-desugared-boxed.rs +++ b/tests/ui/async-await/in-trait/async-example-desugared-boxed.rs @@ -4,12 +4,15 @@ 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 + '_>> { + //~^ WARN impl trait in impl method signature does not match trait method signature Box::pin(async { *self }) } } diff --git a/tests/ui/async-await/in-trait/async-example-desugared-boxed.stderr b/tests/ui/async-await/in-trait/async-example-desugared-boxed.stderr new file mode 100644 index 00000000000..54aba77cc05 --- /dev/null +++ b/tests/ui/async-await/in-trait/async-example-desugared-boxed.stderr @@ -0,0 +1,22 @@ +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; + | --------------------------- return type from trait method defined here +... +LL | fn foo(&self) -> Pin + '_>> { + | ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ + | + = 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 { + | ~~~~~~~~~~~~~~~~~~~~~~~~~ + +warning: 1 warning emitted + diff --git a/tests/ui/async-await/in-trait/async-example-desugared-manual.rs b/tests/ui/async-await/in-trait/async-example-desugared-manual.rs index 75f4ba1d076..c6e8f1ae906 100644 --- a/tests/ui/async-await/in-trait/async-example-desugared-manual.rs +++ b/tests/ui/async-await/in-trait/async-example-desugared-manual.rs @@ -4,11 +4,12 @@ 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 { @@ -17,7 +18,9 @@ impl Future for MyFuture { } impl MyTrait for u32 { + #[warn(refining_impl_trait)] fn foo(&self) -> MyFuture { + //~^ WARN impl trait in impl method signature does not match trait method signature MyFuture } } diff --git a/tests/ui/async-await/in-trait/async-example-desugared-manual.stderr b/tests/ui/async-await/in-trait/async-example-desugared-manual.stderr new file mode 100644 index 00000000000..d94afd92c56 --- /dev/null +++ b/tests/ui/async-await/in-trait/async-example-desugared-manual.stderr @@ -0,0 +1,22 @@ +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; + | --------------------------- 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 { + | ~~~~~~~~~~~~~~~~~~~~~~~~~ + +warning: 1 warning emitted +