From 755b2da841b7dad218bd34708a592a2bef0f4b21 Mon Sep 17 00:00:00 2001 From: Michael Goulet Date: Wed, 8 Nov 2023 05:56:32 +0000 Subject: [PATCH 1/7] Value recovery can take the whole CycleError --- compiler/rustc_middle/src/query/plumbing.rs | 2 +- compiler/rustc_middle/src/values.rs | 48 +++++++++++++------ compiler/rustc_query_impl/src/lib.rs | 6 +-- .../rustc_query_system/src/query/config.rs | 4 +- compiler/rustc_query_system/src/query/job.rs | 2 +- compiler/rustc_query_system/src/query/mod.rs | 4 +- .../rustc_query_system/src/query/plumbing.rs | 8 ++-- compiler/rustc_query_system/src/values.rs | 11 +++-- 8 files changed, 54 insertions(+), 31 deletions(-) diff --git a/compiler/rustc_middle/src/query/plumbing.rs b/compiler/rustc_middle/src/query/plumbing.rs index d2ff1e3c0944..414d4c8d9496 100644 --- a/compiler/rustc_middle/src/query/plumbing.rs +++ b/compiler/rustc_middle/src/query/plumbing.rs @@ -53,7 +53,7 @@ pub struct DynamicQuery<'tcx, C: QueryCache> { fn(tcx: TyCtxt<'tcx>, key: &C::Key, index: SerializedDepNodeIndex) -> bool, pub hash_result: HashResult, pub value_from_cycle_error: - fn(tcx: TyCtxt<'tcx>, cycle: &[QueryInfo], guar: ErrorGuaranteed) -> C::Value, + fn(tcx: TyCtxt<'tcx>, cycle_error: &CycleError, guar: ErrorGuaranteed) -> C::Value, pub format_value: fn(&C::Value) -> String, } diff --git a/compiler/rustc_middle/src/values.rs b/compiler/rustc_middle/src/values.rs index b3c05a36a138..4ee97dac4444 100644 --- a/compiler/rustc_middle/src/values.rs +++ b/compiler/rustc_middle/src/values.rs @@ -6,7 +6,7 @@ use rustc_hir as hir; use rustc_hir::def::{DefKind, Res}; use rustc_middle::ty::Representability; use rustc_middle::ty::{self, Ty, TyCtxt}; -use rustc_query_system::query::QueryInfo; +use rustc_query_system::query::CycleError; use rustc_query_system::Value; use rustc_span::def_id::LocalDefId; use rustc_span::{ErrorGuaranteed, Span}; @@ -14,7 +14,7 @@ use rustc_span::{ErrorGuaranteed, Span}; use std::fmt::Write; impl<'tcx> Value> for Ty<'_> { - fn from_cycle_error(tcx: TyCtxt<'tcx>, _: &[QueryInfo], guar: ErrorGuaranteed) -> Self { + fn from_cycle_error(tcx: TyCtxt<'tcx>, _: &CycleError, guar: ErrorGuaranteed) -> Self { // SAFETY: This is never called when `Self` is not `Ty<'tcx>`. // FIXME: Represent the above fact in the trait system somehow. unsafe { std::mem::transmute::, Ty<'_>>(Ty::new_error(tcx, guar)) } @@ -22,13 +22,13 @@ impl<'tcx> Value> for Ty<'_> { } impl<'tcx> Value> for Result>, CyclePlaceholder> { - fn from_cycle_error(_tcx: TyCtxt<'tcx>, _: &[QueryInfo], guar: ErrorGuaranteed) -> Self { + fn from_cycle_error(_tcx: TyCtxt<'tcx>, _: &CycleError, guar: ErrorGuaranteed) -> Self { Err(CyclePlaceholder(guar)) } } impl<'tcx> Value> for ty::SymbolName<'_> { - fn from_cycle_error(tcx: TyCtxt<'tcx>, _: &[QueryInfo], _guar: ErrorGuaranteed) -> Self { + fn from_cycle_error(tcx: TyCtxt<'tcx>, _: &CycleError, _guar: ErrorGuaranteed) -> Self { // SAFETY: This is never called when `Self` is not `SymbolName<'tcx>`. // FIXME: Represent the above fact in the trait system somehow. unsafe { @@ -40,10 +40,14 @@ impl<'tcx> Value> for ty::SymbolName<'_> { } impl<'tcx> Value> for ty::Binder<'_, ty::FnSig<'_>> { - fn from_cycle_error(tcx: TyCtxt<'tcx>, stack: &[QueryInfo], guar: ErrorGuaranteed) -> Self { + fn from_cycle_error( + tcx: TyCtxt<'tcx>, + cycle_error: &CycleError, + guar: ErrorGuaranteed, + ) -> Self { let err = Ty::new_error(tcx, guar); - let arity = if let Some(frame) = stack.get(0) + let arity = if let Some(frame) = cycle_error.cycle.get(0) && frame.query.dep_kind == dep_kinds::fn_sig && let Some(def_id) = frame.query.def_id && let Some(node) = tcx.hir().get_if_local(def_id) @@ -70,10 +74,14 @@ impl<'tcx> Value> for ty::Binder<'_, ty::FnSig<'_>> { } impl<'tcx> Value> for Representability { - fn from_cycle_error(tcx: TyCtxt<'tcx>, cycle: &[QueryInfo], _guar: ErrorGuaranteed) -> Self { + fn from_cycle_error( + tcx: TyCtxt<'tcx>, + cycle_error: &CycleError, + _guar: ErrorGuaranteed, + ) -> Self { let mut item_and_field_ids = Vec::new(); let mut representable_ids = FxHashSet::default(); - for info in cycle { + for info in &cycle_error.cycle { if info.query.dep_kind == dep_kinds::representability && let Some(field_id) = info.query.def_id && let Some(field_id) = field_id.as_local() @@ -87,7 +95,7 @@ impl<'tcx> Value> for Representability { item_and_field_ids.push((item_id.expect_local(), field_id)); } } - for info in cycle { + for info in &cycle_error.cycle { if info.query.dep_kind == dep_kinds::representability_adt_ty && let Some(def_id) = info.query.ty_adt_id && let Some(def_id) = def_id.as_local() @@ -102,19 +110,31 @@ impl<'tcx> Value> for Representability { } impl<'tcx> Value> for ty::EarlyBinder> { - fn from_cycle_error(tcx: TyCtxt<'tcx>, cycle: &[QueryInfo], guar: ErrorGuaranteed) -> Self { - ty::EarlyBinder::bind(Ty::from_cycle_error(tcx, cycle, guar)) + fn from_cycle_error( + tcx: TyCtxt<'tcx>, + cycle_error: &CycleError, + guar: ErrorGuaranteed, + ) -> Self { + ty::EarlyBinder::bind(Ty::from_cycle_error(tcx, cycle_error, guar)) } } impl<'tcx> Value> for ty::EarlyBinder>> { - fn from_cycle_error(tcx: TyCtxt<'tcx>, cycle: &[QueryInfo], guar: ErrorGuaranteed) -> Self { - ty::EarlyBinder::bind(ty::Binder::from_cycle_error(tcx, cycle, guar)) + fn from_cycle_error( + tcx: TyCtxt<'tcx>, + cycle_error: &CycleError, + guar: ErrorGuaranteed, + ) -> Self { + ty::EarlyBinder::bind(ty::Binder::from_cycle_error(tcx, cycle_error, guar)) } } impl<'tcx, T> Value> for Result> { - fn from_cycle_error(_tcx: TyCtxt<'tcx>, _cycle: &[QueryInfo], guar: ErrorGuaranteed) -> Self { + fn from_cycle_error( + _tcx: TyCtxt<'tcx>, + _cycle_error: &CycleError, + guar: ErrorGuaranteed, + ) -> Self { // tcx.arena.alloc cannot be used because we are not allowed to use &'tcx LayoutError under // min_specialization. Since this is an error path anyways, leaking doesn't matter (and really, // tcx.arena.alloc is pretty much equal to leaking). diff --git a/compiler/rustc_query_impl/src/lib.rs b/compiler/rustc_query_impl/src/lib.rs index d5883f528199..b5e8ac4018df 100644 --- a/compiler/rustc_query_impl/src/lib.rs +++ b/compiler/rustc_query_impl/src/lib.rs @@ -35,7 +35,7 @@ use rustc_middle::ty::TyCtxt; use rustc_query_system::dep_graph::SerializedDepNodeIndex; use rustc_query_system::ich::StableHashingContext; use rustc_query_system::query::{ - get_query_incr, get_query_non_incr, HashResult, QueryCache, QueryConfig, QueryInfo, QueryMap, + get_query_incr, get_query_non_incr, CycleError, HashResult, QueryCache, QueryConfig, QueryMap, QueryMode, QueryState, }; use rustc_query_system::HandleCycleError; @@ -144,10 +144,10 @@ where fn value_from_cycle_error( self, tcx: TyCtxt<'tcx>, - cycle: &[QueryInfo], + cycle_error: &CycleError, guar: ErrorGuaranteed, ) -> Self::Value { - (self.dynamic.value_from_cycle_error)(tcx, cycle, guar) + (self.dynamic.value_from_cycle_error)(tcx, cycle_error, guar) } #[inline(always)] diff --git a/compiler/rustc_query_system/src/query/config.rs b/compiler/rustc_query_system/src/query/config.rs index c025fac2631a..958d9fdb52ae 100644 --- a/compiler/rustc_query_system/src/query/config.rs +++ b/compiler/rustc_query_system/src/query/config.rs @@ -5,7 +5,7 @@ use crate::error::HandleCycleError; use crate::ich::StableHashingContext; use crate::query::caches::QueryCache; use crate::query::DepNodeIndex; -use crate::query::{QueryContext, QueryInfo, QueryState}; +use crate::query::{CycleError, QueryContext, QueryState}; use rustc_data_structures::fingerprint::Fingerprint; use rustc_span::ErrorGuaranteed; @@ -57,7 +57,7 @@ pub trait QueryConfig: Copy { fn value_from_cycle_error( self, tcx: Qcx::DepContext, - cycle: &[QueryInfo], + cycle_error: &CycleError, guar: ErrorGuaranteed, ) -> Self::Value; diff --git a/compiler/rustc_query_system/src/query/job.rs b/compiler/rustc_query_system/src/query/job.rs index 982b9ee94da9..3ef9de7da74b 100644 --- a/compiler/rustc_query_system/src/query/job.rs +++ b/compiler/rustc_query_system/src/query/job.rs @@ -556,7 +556,7 @@ pub fn deadlock(query_map: QueryMap, registry: &rayon_core::Registry) { #[inline(never)] #[cold] -pub(crate) fn report_cycle<'a>( +pub fn report_cycle<'a>( sess: &'a Session, CycleError { usage, cycle: stack }: &CycleError, ) -> DiagnosticBuilder<'a> { diff --git a/compiler/rustc_query_system/src/query/mod.rs b/compiler/rustc_query_system/src/query/mod.rs index 96a0c7a033aa..ce6a6d6cb1b6 100644 --- a/compiler/rustc_query_system/src/query/mod.rs +++ b/compiler/rustc_query_system/src/query/mod.rs @@ -4,7 +4,9 @@ pub use self::plumbing::*; mod job; #[cfg(parallel_compiler)] pub use self::job::deadlock; -pub use self::job::{print_query_stack, QueryInfo, QueryJob, QueryJobId, QueryJobInfo, QueryMap}; +pub use self::job::{ + print_query_stack, report_cycle, QueryInfo, QueryJob, QueryJobId, QueryJobInfo, QueryMap, +}; mod caches; pub use self::caches::{ diff --git a/compiler/rustc_query_system/src/query/plumbing.rs b/compiler/rustc_query_system/src/query/plumbing.rs index c5715d938595..c5a0cc753a8f 100644 --- a/compiler/rustc_query_system/src/query/plumbing.rs +++ b/compiler/rustc_query_system/src/query/plumbing.rs @@ -134,7 +134,7 @@ where match query.handle_cycle_error() { Error => { let guar = error.emit(); - query.value_from_cycle_error(*qcx.dep_context(), &cycle_error.cycle, guar) + query.value_from_cycle_error(*qcx.dep_context(), cycle_error, guar) } Fatal => { error.emit(); @@ -143,7 +143,7 @@ where } DelayBug => { let guar = error.delay_as_bug(); - query.value_from_cycle_error(*qcx.dep_context(), &cycle_error.cycle, guar) + query.value_from_cycle_error(*qcx.dep_context(), cycle_error, guar) } Stash => { let guar = if let Some(root) = cycle_error.cycle.first() @@ -154,7 +154,7 @@ where } else { error.emit() }; - query.value_from_cycle_error(*qcx.dep_context(), &cycle_error.cycle, guar) + query.value_from_cycle_error(*qcx.dep_context(), cycle_error, guar) } } } @@ -211,7 +211,7 @@ where } #[derive(Clone, Debug)] -pub(crate) struct CycleError { +pub struct CycleError { /// The query and related span that uses the cycle. pub usage: Option<(Span, QueryStackFrame)>, pub cycle: Vec, diff --git a/compiler/rustc_query_system/src/values.rs b/compiler/rustc_query_system/src/values.rs index 4f1c182cdb8e..133904f59af1 100644 --- a/compiler/rustc_query_system/src/values.rs +++ b/compiler/rustc_query_system/src/values.rs @@ -1,20 +1,21 @@ use rustc_span::ErrorGuaranteed; use crate::dep_graph::DepContext; -use crate::query::QueryInfo; +use crate::query::CycleError; pub trait Value: Sized { - fn from_cycle_error(tcx: Tcx, cycle: &[QueryInfo], guar: ErrorGuaranteed) -> Self; + fn from_cycle_error(tcx: Tcx, cycle_error: &CycleError, guar: ErrorGuaranteed) -> Self; } impl Value for T { - default fn from_cycle_error(tcx: Tcx, cycle: &[QueryInfo], _guar: ErrorGuaranteed) -> T { + default fn from_cycle_error(tcx: Tcx, cycle_error: &CycleError, _guar: ErrorGuaranteed) -> T { tcx.sess().dcx().abort_if_errors(); // Ideally we would use `bug!` here. But bug! is only defined in rustc_middle, and it's // non-trivial to define it earlier. panic!( - "<{} as Value>::from_cycle_error called without errors: {cycle:#?}", - std::any::type_name::() + "<{} as Value>::from_cycle_error called without errors: {:#?}", + std::any::type_name::(), + cycle_error.cycle, ); } } From dfb9f5df2c9f3c60f2541b3804cb6be1b5c66e63 Mon Sep 17 00:00:00 2001 From: Michael Goulet Date: Sun, 12 Nov 2023 18:57:24 +0000 Subject: [PATCH 2/7] Rustdoc and Clippy stop misusing Key for Ty -> (adt) DefId --- src/librustdoc/html/render/print_item.rs | 3 +-- src/tools/clippy/clippy_lints/src/copies.rs | 3 +-- src/tools/clippy/clippy_lints/src/methods/drain_collect.rs | 7 +++---- .../clippy/clippy_lints/src/methods/redundant_as_str.rs | 7 +------ src/tools/clippy/clippy_lints/src/mut_key.rs | 3 +-- src/tools/clippy/clippy_lints/src/non_copy_const.rs | 3 +-- .../src/transmute/transmute_int_to_non_zero.rs | 5 ++--- 7 files changed, 10 insertions(+), 21 deletions(-) diff --git a/src/librustdoc/html/render/print_item.rs b/src/librustdoc/html/render/print_item.rs index 5ca623f01f1f..3b91fbdcb29d 100644 --- a/src/librustdoc/html/render/print_item.rs +++ b/src/librustdoc/html/render/print_item.rs @@ -4,7 +4,6 @@ use rustc_hir as hir; use rustc_hir::def::CtorKind; use rustc_hir::def_id::DefId; use rustc_index::IndexVec; -use rustc_middle::query::Key; use rustc_middle::ty::{self, TyCtxt}; use rustc_span::hygiene::MacroKind; use rustc_span::symbol::{kw, sym, Symbol}; @@ -1259,7 +1258,7 @@ fn item_type_alias(w: &mut Buffer, cx: &mut Context<'_>, it: &clean::Item, t: &c clean::TypeAliasInnerType::Enum { variants, is_non_exhaustive } => { let variants_iter = || variants.iter().filter(|i| !i.is_stripped()); let ty = cx.tcx().type_of(it.def_id().unwrap()).instantiate_identity(); - let enum_def_id = ty.ty_adt_id().unwrap(); + let enum_def_id = ty.ty_adt_def().unwrap().did(); wrap_item(w, |w| { let variants_len = variants.len(); diff --git a/src/tools/clippy/clippy_lints/src/copies.rs b/src/tools/clippy/clippy_lints/src/copies.rs index d91af76f5e0d..bd07c19a2d81 100644 --- a/src/tools/clippy/clippy_lints/src/copies.rs +++ b/src/tools/clippy/clippy_lints/src/copies.rs @@ -12,7 +12,6 @@ use rustc_errors::Applicability; use rustc_hir::def_id::DefIdSet; use rustc_hir::{intravisit, BinOpKind, Block, Expr, ExprKind, HirId, HirIdSet, Stmt, StmtKind}; use rustc_lint::{LateContext, LateLintPass}; -use rustc_middle::query::Key; use rustc_session::impl_lint_pass; use rustc_span::hygiene::walk_chain; use rustc_span::source_map::SourceMap; @@ -574,7 +573,7 @@ fn method_caller_is_mutable(cx: &LateContext<'_>, caller_expr: &Expr<'_>, ignore let caller_ty = cx.typeck_results().expr_ty(caller_expr); // Check if given type has inner mutability and was not set to ignored by the configuration let is_inner_mut_ty = is_interior_mut_ty(cx, caller_ty) - && !matches!(caller_ty.ty_adt_id(), Some(adt_id) if ignored_ty_ids.contains(&adt_id)); + && !matches!(caller_ty.ty_adt_def(), Some(adt) if ignored_ty_ids.contains(&adt.did())); is_inner_mut_ty || caller_ty.is_mutable_ptr() diff --git a/src/tools/clippy/clippy_lints/src/methods/drain_collect.rs b/src/tools/clippy/clippy_lints/src/methods/drain_collect.rs index 6a82d8f756ad..3a8ca37610a9 100644 --- a/src/tools/clippy/clippy_lints/src/methods/drain_collect.rs +++ b/src/tools/clippy/clippy_lints/src/methods/drain_collect.rs @@ -6,7 +6,6 @@ use clippy_utils::ty::is_type_lang_item; use rustc_errors::Applicability; use rustc_hir::{Expr, ExprKind, LangItem, Path, QPath}; use rustc_lint::LateContext; -use rustc_middle::query::Key; use rustc_middle::ty; use rustc_middle::ty::Ty; use rustc_span::{sym, Symbol}; @@ -18,10 +17,10 @@ use rustc_span::{sym, Symbol}; /// `vec![1,2].drain(..).collect::>()` /// ^^^^^^^^^ ^^^^^^^^^^ false fn types_match_diagnostic_item(cx: &LateContext<'_>, expr: Ty<'_>, recv: Ty<'_>, sym: Symbol) -> bool { - if let Some(expr_adt_did) = expr.ty_adt_id() - && let Some(recv_adt_did) = recv.ty_adt_id() + if let Some(expr_adt) = expr.ty_adt_def() + && let Some(recv_adt) = recv.ty_adt_def() { - cx.tcx.is_diagnostic_item(sym, expr_adt_did) && cx.tcx.is_diagnostic_item(sym, recv_adt_did) + cx.tcx.is_diagnostic_item(sym, expr_adt.did()) && cx.tcx.is_diagnostic_item(sym, recv_adt.did()) } else { false } diff --git a/src/tools/clippy/clippy_lints/src/methods/redundant_as_str.rs b/src/tools/clippy/clippy_lints/src/methods/redundant_as_str.rs index 98cd6afc2b79..2a2feedd2b49 100644 --- a/src/tools/clippy/clippy_lints/src/methods/redundant_as_str.rs +++ b/src/tools/clippy/clippy_lints/src/methods/redundant_as_str.rs @@ -4,7 +4,6 @@ use clippy_utils::source::snippet_with_applicability; use rustc_errors::Applicability; use rustc_hir::Expr; use rustc_lint::LateContext; -use rustc_middle::query::Key; use rustc_span::Span; pub(super) fn check( @@ -14,11 +13,7 @@ pub(super) fn check( as_str_span: Span, other_method_span: Span, ) { - if cx - .tcx - .lang_items() - .string() - .is_some_and(|id| Some(id) == cx.typeck_results().expr_ty(recv).ty_adt_id()) + if cx.typeck_results().expr_ty(recv).ty_adt_def().is_some_and(|adt| Some(adt.did()) == cx.tcx.lang_items().string()) { let mut applicability = Applicability::MachineApplicable; span_lint_and_sugg( diff --git a/src/tools/clippy/clippy_lints/src/mut_key.rs b/src/tools/clippy/clippy_lints/src/mut_key.rs index 04d2ced6abf8..c32025fcbb6a 100644 --- a/src/tools/clippy/clippy_lints/src/mut_key.rs +++ b/src/tools/clippy/clippy_lints/src/mut_key.rs @@ -4,7 +4,6 @@ use clippy_utils::{def_path_def_ids, trait_ref_of_method}; use rustc_data_structures::fx::FxHashSet; use rustc_hir as hir; use rustc_lint::{LateContext, LateLintPass}; -use rustc_middle::query::Key; use rustc_middle::ty::{Adt, Ty}; use rustc_session::impl_lint_pass; use rustc_span::def_id::LocalDefId; @@ -166,7 +165,7 @@ impl MutableKeyType { // Determines if a type contains interior mutability which would affect its implementation of // [`Hash`] or [`Ord`]. if is_interior_mut_ty(cx, subst_ty) - && !matches!(subst_ty.ty_adt_id(), Some(adt_id) if self.ignore_mut_def_ids.contains(&adt_id)) + && !matches!(subst_ty.ty_adt_def(), Some(adt) if self.ignore_mut_def_ids.contains(&adt.did())) { span_lint(cx, MUTABLE_KEY_TYPE, span, "mutable key type"); } diff --git a/src/tools/clippy/clippy_lints/src/non_copy_const.rs b/src/tools/clippy/clippy_lints/src/non_copy_const.rs index 4013cb34561a..f8365deebd46 100644 --- a/src/tools/clippy/clippy_lints/src/non_copy_const.rs +++ b/src/tools/clippy/clippy_lints/src/non_copy_const.rs @@ -15,7 +15,6 @@ use rustc_hir::{ }; use rustc_lint::{LateContext, LateLintPass, Lint}; use rustc_middle::mir::interpret::{ErrorHandled, EvalToValTreeResult, GlobalId}; -use rustc_middle::query::Key; use rustc_middle::ty::adjustment::Adjust; use rustc_middle::ty::{self, Ty, TyCtxt}; use rustc_session::impl_lint_pass; @@ -188,7 +187,7 @@ impl NonCopyConst { } fn is_ty_ignored(&self, ty: Ty<'_>) -> bool { - matches!(ty.ty_adt_id(), Some(adt_id) if self.ignore_mut_def_ids.contains(&adt_id)) + matches!(ty.ty_adt_def(), Some(adt) if self.ignore_mut_def_ids.contains(&adt.did())) } fn is_unfrozen<'tcx>(cx: &LateContext<'tcx>, ty: Ty<'tcx>) -> bool { diff --git a/src/tools/clippy/clippy_lints/src/transmute/transmute_int_to_non_zero.rs b/src/tools/clippy/clippy_lints/src/transmute/transmute_int_to_non_zero.rs index c0d0d2b93dc0..5df645491ff8 100644 --- a/src/tools/clippy/clippy_lints/src/transmute/transmute_int_to_non_zero.rs +++ b/src/tools/clippy/clippy_lints/src/transmute/transmute_int_to_non_zero.rs @@ -4,7 +4,6 @@ use clippy_utils::sugg; use rustc_errors::Applicability; use rustc_hir::Expr; use rustc_lint::LateContext; -use rustc_middle::query::Key; use rustc_middle::ty::{self, Ty}; use rustc_span::symbol::sym; @@ -17,10 +16,10 @@ pub(super) fn check<'tcx>( to_ty: Ty<'tcx>, arg: &'tcx Expr<'_>, ) -> bool { - let (ty::Int(_) | ty::Uint(_), Some(to_ty_id)) = (&from_ty.kind(), to_ty.ty_adt_id()) else { + let (ty::Int(_) | ty::Uint(_), Some(to_ty_adt)) = (&from_ty.kind(), to_ty.ty_adt_def()) else { return false; }; - let Some(to_type_sym) = cx.tcx.get_diagnostic_name(to_ty_id) else { + let Some(to_type_sym) = cx.tcx.get_diagnostic_name(to_ty_adt.did()) else { return false; }; From 82a22154815b70e5701064bef59d3334f5bc1cf7 Mon Sep 17 00:00:00 2001 From: Michael Goulet Date: Wed, 8 Nov 2023 06:56:06 +0000 Subject: [PATCH 3/7] Don't check for recursion in generator witness fields --- .../src/error_codes/E0733.md | 16 ++++++++- compiler/rustc_hir/src/hir.rs | 6 ++++ .../rustc_hir_analysis/src/check/check.rs | 25 ++++--------- compiler/rustc_middle/src/query/keys.rs | 13 ++++--- compiler/rustc_middle/src/query/mod.rs | 2 ++ compiler/rustc_middle/src/ty/util.rs | 13 +------ compiler/rustc_middle/src/values.rs | 36 ++++++++++++++++--- compiler/rustc_query_impl/src/plumbing.rs | 4 +-- compiler/rustc_query_system/src/query/mod.rs | 7 ++-- .../src/traits/query/normalize.rs | 13 +++---- ...utually-recursive-async-impl-trait-type.rs | 2 +- ...lly-recursive-async-impl-trait-type.stderr | 11 +----- .../recursive-coroutine.current.stderr | 12 ------- .../recursive-coroutine.next.stderr | 12 ------- tests/ui/impl-trait/recursive-coroutine.rs | 5 +-- .../recursive-impl-trait-type-indirect.rs | 2 +- .../recursive-impl-trait-type-indirect.stderr | 17 ++++----- .../indirect-recursion-issue-112047.rs | 3 +- .../indirect-recursion-issue-112047.stderr | 27 ++++---------- 19 files changed, 102 insertions(+), 124 deletions(-) delete mode 100644 tests/ui/impl-trait/recursive-coroutine.current.stderr delete mode 100644 tests/ui/impl-trait/recursive-coroutine.next.stderr diff --git a/compiler/rustc_error_codes/src/error_codes/E0733.md b/compiler/rustc_error_codes/src/error_codes/E0733.md index 051b75148e50..cceb0880350e 100644 --- a/compiler/rustc_error_codes/src/error_codes/E0733.md +++ b/compiler/rustc_error_codes/src/error_codes/E0733.md @@ -13,7 +13,7 @@ async fn foo(n: usize) { To perform async recursion, the `async fn` needs to be desugared such that the `Future` is explicit in the return type: -```edition2018,compile_fail,E0720 +```edition2018,compile_fail,E0733 use std::future::Future; fn foo_desugared(n: usize) -> impl Future { async move { @@ -41,4 +41,18 @@ fn foo_recursive(n: usize) -> Pin>> { The `Box<...>` ensures that the result is of known size, and the pin is required to keep it in the same place in memory. +Alternatively, the recursive call-site can be boxed: + +```edition2018 +use std::future::Future; +use std::pin::Pin; +fn foo_recursive(n: usize) -> impl Future { + async move { + if n > 0 { + Box::pin(foo_recursive(n - 1)).await; + } + } +} +``` + [`async`]: https://doc.rust-lang.org/std/keyword.async.html diff --git a/compiler/rustc_hir/src/hir.rs b/compiler/rustc_hir/src/hir.rs index e88b876534e5..cadf54c76a3f 100644 --- a/compiler/rustc_hir/src/hir.rs +++ b/compiler/rustc_hir/src/hir.rs @@ -1361,6 +1361,12 @@ impl CoroutineKind { } } +impl CoroutineKind { + pub fn is_fn_like(self) -> bool { + matches!(self, CoroutineKind::Desugared(_, CoroutineSource::Fn)) + } +} + impl fmt::Display for CoroutineKind { fn fmt(&self, f: &mut fmt::Formatter<'_>) -> fmt::Result { match self { diff --git a/compiler/rustc_hir_analysis/src/check/check.rs b/compiler/rustc_hir_analysis/src/check/check.rs index 4b26a469eb56..12430b1465c3 100644 --- a/compiler/rustc_hir_analysis/src/check/check.rs +++ b/compiler/rustc_hir_analysis/src/check/check.rs @@ -213,13 +213,12 @@ fn check_opaque(tcx: TyCtxt<'_>, def_id: LocalDefId) { return; } - let args = GenericArgs::identity_for_item(tcx, item.owner_id); let span = tcx.def_span(item.owner_id.def_id); if tcx.type_of(item.owner_id.def_id).instantiate_identity().references_error() { return; } - if check_opaque_for_cycles(tcx, item.owner_id.def_id, args, span, origin).is_err() { + if check_opaque_for_cycles(tcx, item.owner_id.def_id, span).is_err() { return; } @@ -230,16 +229,16 @@ fn check_opaque(tcx: TyCtxt<'_>, def_id: LocalDefId) { pub(super) fn check_opaque_for_cycles<'tcx>( tcx: TyCtxt<'tcx>, def_id: LocalDefId, - args: GenericArgsRef<'tcx>, span: Span, - origin: &hir::OpaqueTyOrigin, ) -> Result<(), ErrorGuaranteed> { + let args = GenericArgs::identity_for_item(tcx, def_id); if tcx.try_expand_impl_trait_type(def_id.to_def_id(), args).is_err() { - let reported = match origin { - hir::OpaqueTyOrigin::AsyncFn(..) => async_opaque_type_cycle_error(tcx, span), - _ => opaque_type_cycle_error(tcx, def_id, span), - }; + let reported = opaque_type_cycle_error(tcx, def_id, span); Err(reported) + } else if let Err(&LayoutError::Cycle(guar)) = + tcx.layout_of(tcx.param_env(def_id).and(Ty::new_opaque(tcx, def_id.to_def_id(), args))) + { + Err(guar) } else { Ok(()) } @@ -1300,16 +1299,6 @@ pub(super) fn check_type_params_are_used<'tcx>( } } -fn async_opaque_type_cycle_error(tcx: TyCtxt<'_>, span: Span) -> ErrorGuaranteed { - struct_span_err!(tcx.dcx(), span, E0733, "recursion in an `async fn` requires boxing") - .span_label_mv(span, "recursive `async fn`") - .note_mv("a recursive `async fn` must be rewritten to return a boxed `dyn Future`") - .note_mv( - "consider using the `async_recursion` crate: https://crates.io/crates/async_recursion", - ) - .emit() -} - /// Emit an error for recursive opaque types. /// /// If this is a return `impl Trait`, find the item's return expressions and point at them. For diff --git a/compiler/rustc_middle/src/query/keys.rs b/compiler/rustc_middle/src/query/keys.rs index 113763450529..945f17d5df2a 100644 --- a/compiler/rustc_middle/src/query/keys.rs +++ b/compiler/rustc_middle/src/query/keys.rs @@ -40,7 +40,7 @@ pub trait Key: Sized { None } - fn ty_adt_id(&self) -> Option { + fn ty_def_id(&self) -> Option { None } } @@ -406,9 +406,10 @@ impl<'tcx> Key for Ty<'tcx> { DUMMY_SP } - fn ty_adt_id(&self) -> Option { - match self.kind() { + fn ty_def_id(&self) -> Option { + match *self.kind() { ty::Adt(adt, _) => Some(adt.did()), + ty::Coroutine(def_id, ..) => Some(def_id), _ => None, } } @@ -452,6 +453,10 @@ impl<'tcx, T: Key> Key for ty::ParamEnvAnd<'tcx, T> { fn default_span(&self, tcx: TyCtxt<'_>) -> Span { self.value.default_span(tcx) } + + fn ty_def_id(&self) -> Option { + self.value.ty_def_id() + } } impl Key for Symbol { @@ -550,7 +555,7 @@ impl<'tcx> Key for (ValidityRequirement, ty::ParamEnvAnd<'tcx, Ty<'tcx>>) { DUMMY_SP } - fn ty_adt_id(&self) -> Option { + fn ty_def_id(&self) -> Option { match self.1.value.kind() { ty::Adt(adt, _) => Some(adt.did()), _ => None, diff --git a/compiler/rustc_middle/src/query/mod.rs b/compiler/rustc_middle/src/query/mod.rs index 7d5abaceb20a..0e3b9984423f 100644 --- a/compiler/rustc_middle/src/query/mod.rs +++ b/compiler/rustc_middle/src/query/mod.rs @@ -1387,6 +1387,8 @@ rustc_queries! { ) -> Result, &'tcx ty::layout::LayoutError<'tcx>> { depth_limit desc { "computing layout of `{}`", key.value } + // we emit our own error during query cycle handling + cycle_delay_bug } /// Compute a `FnAbi` suitable for indirect calls, i.e. to `fn` pointers. diff --git a/compiler/rustc_middle/src/ty/util.rs b/compiler/rustc_middle/src/ty/util.rs index b9c75bd205b2..74dba41647bd 100644 --- a/compiler/rustc_middle/src/ty/util.rs +++ b/compiler/rustc_middle/src/ty/util.rs @@ -896,18 +896,7 @@ impl<'tcx> OpaqueTypeExpander<'tcx> { } let args = args.fold_with(self); if !self.check_recursion || self.seen_opaque_tys.insert(def_id) { - let expanded_ty = match self.expanded_cache.get(&(def_id, args)) { - Some(expanded_ty) => *expanded_ty, - None => { - for bty in self.tcx.coroutine_hidden_types(def_id) { - let hidden_ty = bty.instantiate(self.tcx, args); - self.fold_ty(hidden_ty); - } - let expanded_ty = Ty::new_coroutine_witness(self.tcx, def_id, args); - self.expanded_cache.insert((def_id, args), expanded_ty); - expanded_ty - } - }; + let expanded_ty = Ty::new_coroutine_witness(self.tcx, def_id, args); if self.check_recursion { self.seen_opaque_tys.remove(&def_id); } diff --git a/compiler/rustc_middle/src/values.rs b/compiler/rustc_middle/src/values.rs index 4ee97dac4444..0179829cc46a 100644 --- a/compiler/rustc_middle/src/values.rs +++ b/compiler/rustc_middle/src/values.rs @@ -6,7 +6,7 @@ use rustc_hir as hir; use rustc_hir::def::{DefKind, Res}; use rustc_middle::ty::Representability; use rustc_middle::ty::{self, Ty, TyCtxt}; -use rustc_query_system::query::CycleError; +use rustc_query_system::query::{report_cycle, CycleError}; use rustc_query_system::Value; use rustc_span::def_id::LocalDefId; use rustc_span::{ErrorGuaranteed, Span}; @@ -97,7 +97,7 @@ impl<'tcx> Value> for Representability { } for info in &cycle_error.cycle { if info.query.dep_kind == dep_kinds::representability_adt_ty - && let Some(def_id) = info.query.ty_adt_id + && let Some(def_id) = info.query.ty_def_id && let Some(def_id) = def_id.as_local() && !item_and_field_ids.iter().any(|&(id, _)| id == def_id) { @@ -131,10 +131,36 @@ impl<'tcx> Value> for ty::EarlyBinder> impl<'tcx, T> Value> for Result> { fn from_cycle_error( - _tcx: TyCtxt<'tcx>, - _cycle_error: &CycleError, - guar: ErrorGuaranteed, + tcx: TyCtxt<'tcx>, + cycle_error: &CycleError, + _guar: ErrorGuaranteed, ) -> Self { + let guar = if cycle_error.cycle[0].query.dep_kind == dep_kinds::layout_of + && let Some(def_id) = cycle_error.cycle[0].query.ty_def_id + && let Some(def_id) = def_id.as_local() + && matches!(tcx.def_kind(def_id), DefKind::Closure) + && let Some(coroutine_kind) = tcx.coroutine_kind(def_id) + { + // FIXME: `def_span` for an fn-like coroutine will point to the fn's body + // due to interactions between the desugaring into a closure expr and the + // def_span code. I'm not motivated to fix it, because I tried and it was + // not working, so just hack around it by grabbing the parent fn's span. + let span = if coroutine_kind.is_fn_like() { + tcx.def_span(tcx.local_parent(def_id)) + } else { + tcx.def_span(def_id) + }; + struct_span_err!(tcx.sess.dcx(), span, E0733, "recursion in an `async fn` requires boxing") + .span_label(span, "recursive `async fn`") + .note("a recursive `async fn` must be rewritten to return a boxed `dyn Future`") + .note( + "consider using the `async_recursion` crate: https://crates.io/crates/async_recursion", + ) + .emit() + } else { + report_cycle(tcx.sess, cycle_error).emit() + }; + // tcx.arena.alloc cannot be used because we are not allowed to use &'tcx LayoutError under // min_specialization. Since this is an error path anyways, leaking doesn't matter (and really, // tcx.arena.alloc is pretty much equal to leaking). diff --git a/compiler/rustc_query_impl/src/plumbing.rs b/compiler/rustc_query_impl/src/plumbing.rs index f131a0f75932..7e0fbf3d76cb 100644 --- a/compiler/rustc_query_impl/src/plumbing.rs +++ b/compiler/rustc_query_impl/src/plumbing.rs @@ -342,9 +342,9 @@ pub(crate) fn create_query_frame< hasher.finish::() }) }; - let ty_adt_id = key.ty_adt_id(); + let ty_def_id = key.ty_def_id(); - QueryStackFrame::new(description, span, def_id, def_kind, kind, ty_adt_id, hash) + QueryStackFrame::new(description, span, def_id, def_kind, kind, ty_def_id, hash) } pub(crate) fn encode_query_results<'a, 'tcx, Q>( diff --git a/compiler/rustc_query_system/src/query/mod.rs b/compiler/rustc_query_system/src/query/mod.rs index ce6a6d6cb1b6..9ff04c4e910d 100644 --- a/compiler/rustc_query_system/src/query/mod.rs +++ b/compiler/rustc_query_system/src/query/mod.rs @@ -35,7 +35,8 @@ pub struct QueryStackFrame { span: Option, pub def_id: Option, pub def_kind: Option, - pub ty_adt_id: Option, + /// A def-id that is extracted from a `Ty` in a query key + pub ty_def_id: Option, pub dep_kind: DepKind, /// This hash is used to deterministically pick /// a query to remove cycles in the parallel compiler. @@ -51,7 +52,7 @@ impl QueryStackFrame { def_id: Option, def_kind: Option, dep_kind: DepKind, - ty_adt_id: Option, + ty_def_id: Option, _hash: impl FnOnce() -> Hash64, ) -> Self { Self { @@ -59,7 +60,7 @@ impl QueryStackFrame { span, def_id, def_kind, - ty_adt_id, + ty_def_id, dep_kind, #[cfg(parallel_compiler)] hash: _hash(), diff --git a/compiler/rustc_trait_selection/src/traits/query/normalize.rs b/compiler/rustc_trait_selection/src/traits/query/normalize.rs index e8867187a408..425c4fbe9c50 100644 --- a/compiler/rustc_trait_selection/src/traits/query/normalize.rs +++ b/compiler/rustc_trait_selection/src/traits/query/normalize.rs @@ -239,16 +239,13 @@ impl<'cx, 'tcx> FallibleTypeFolder> for QueryNormalizer<'cx, 'tcx> } let generic_ty = self.interner().type_of(data.def_id); - let concrete_ty = generic_ty.instantiate(self.interner(), args); + let mut concrete_ty = generic_ty.instantiate(self.interner(), args); self.anon_depth += 1; if concrete_ty == ty { - bug!( - "infinite recursion generic_ty: {:#?}, args: {:#?}, \ - concrete_ty: {:#?}, ty: {:#?}", - generic_ty, - args, - concrete_ty, - ty + concrete_ty = Ty::new_error_with_message( + self.interner(), + DUMMY_SP, + "recursive opaque type", ); } let folded_ty = ensure_sufficient_stack(|| self.try_fold_ty(concrete_ty)); diff --git a/tests/ui/async-await/mutually-recursive-async-impl-trait-type.rs b/tests/ui/async-await/mutually-recursive-async-impl-trait-type.rs index bb2a61f03ce1..d38ba1a569bb 100644 --- a/tests/ui/async-await/mutually-recursive-async-impl-trait-type.rs +++ b/tests/ui/async-await/mutually-recursive-async-impl-trait-type.rs @@ -6,7 +6,7 @@ async fn rec_1() { //~ ERROR recursion in an `async fn` rec_2().await; } -async fn rec_2() { //~ ERROR recursion in an `async fn` +async fn rec_2() { rec_1().await; } diff --git a/tests/ui/async-await/mutually-recursive-async-impl-trait-type.stderr b/tests/ui/async-await/mutually-recursive-async-impl-trait-type.stderr index 9442609e8058..dd53075be602 100644 --- a/tests/ui/async-await/mutually-recursive-async-impl-trait-type.stderr +++ b/tests/ui/async-await/mutually-recursive-async-impl-trait-type.stderr @@ -7,15 +7,6 @@ LL | async fn rec_1() { = note: a recursive `async fn` must be rewritten to return a boxed `dyn Future` = note: consider using the `async_recursion` crate: https://crates.io/crates/async_recursion -error[E0733]: recursion in an `async fn` requires boxing - --> $DIR/mutually-recursive-async-impl-trait-type.rs:9:1 - | -LL | async fn rec_2() { - | ^^^^^^^^^^^^^^^^ recursive `async fn` - | - = note: a recursive `async fn` must be rewritten to return a boxed `dyn Future` - = note: consider using the `async_recursion` crate: https://crates.io/crates/async_recursion - -error: aborting due to 2 previous errors +error: aborting due to 1 previous error For more information about this error, try `rustc --explain E0733`. diff --git a/tests/ui/impl-trait/recursive-coroutine.current.stderr b/tests/ui/impl-trait/recursive-coroutine.current.stderr deleted file mode 100644 index e838634ed087..000000000000 --- a/tests/ui/impl-trait/recursive-coroutine.current.stderr +++ /dev/null @@ -1,12 +0,0 @@ -error[E0720]: cannot resolve opaque type - --> $DIR/recursive-coroutine.rs:7:13 - | -LL | fn foo() -> impl Coroutine { - | ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ recursive opaque type -... -LL | let mut gen = Box::pin(foo()); - | ------- coroutine captures itself here - -error: aborting due to 1 previous error - -For more information about this error, try `rustc --explain E0720`. diff --git a/tests/ui/impl-trait/recursive-coroutine.next.stderr b/tests/ui/impl-trait/recursive-coroutine.next.stderr deleted file mode 100644 index e838634ed087..000000000000 --- a/tests/ui/impl-trait/recursive-coroutine.next.stderr +++ /dev/null @@ -1,12 +0,0 @@ -error[E0720]: cannot resolve opaque type - --> $DIR/recursive-coroutine.rs:7:13 - | -LL | fn foo() -> impl Coroutine { - | ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ recursive opaque type -... -LL | let mut gen = Box::pin(foo()); - | ------- coroutine captures itself here - -error: aborting due to 1 previous error - -For more information about this error, try `rustc --explain E0720`. diff --git a/tests/ui/impl-trait/recursive-coroutine.rs b/tests/ui/impl-trait/recursive-coroutine.rs index b82fe134a406..b9291f07e213 100644 --- a/tests/ui/impl-trait/recursive-coroutine.rs +++ b/tests/ui/impl-trait/recursive-coroutine.rs @@ -1,3 +1,4 @@ +// check-pass // revisions: current next //[next] compile-flags: -Znext-solver #![feature(coroutines, coroutine_trait)] @@ -5,12 +6,8 @@ use std::ops::{Coroutine, CoroutineState}; fn foo() -> impl Coroutine { - //~^ ERROR cannot resolve opaque type - //~| NOTE recursive opaque type - //~| NOTE in this expansion of desugaring of || { let mut gen = Box::pin(foo()); - //~^ NOTE coroutine captures itself here let mut r = gen.as_mut().resume(()); while let CoroutineState::Yielded(v) = r { yield v; diff --git a/tests/ui/impl-trait/recursive-impl-trait-type-indirect.rs b/tests/ui/impl-trait/recursive-impl-trait-type-indirect.rs index 8331eec906e1..a6bca107b1ec 100644 --- a/tests/ui/impl-trait/recursive-impl-trait-type-indirect.rs +++ b/tests/ui/impl-trait/recursive-impl-trait-type-indirect.rs @@ -70,8 +70,8 @@ fn substs_change() -> impl Sized { } fn coroutine_hold() -> impl Sized { - //~^ ERROR move || { + //~^ ERROR let x = coroutine_hold(); yield; x; diff --git a/tests/ui/impl-trait/recursive-impl-trait-type-indirect.stderr b/tests/ui/impl-trait/recursive-impl-trait-type-indirect.stderr index 8e9aa8ad0a69..0dabba264689 100644 --- a/tests/ui/impl-trait/recursive-impl-trait-type-indirect.stderr +++ b/tests/ui/impl-trait/recursive-impl-trait-type-indirect.stderr @@ -109,14 +109,14 @@ LL | LL | (substs_change::<&T>(),) | ------------------------ returning here with type `(impl Sized,)` -error[E0720]: cannot resolve opaque type - --> $DIR/recursive-impl-trait-type-indirect.rs:72:24 +error[E0733]: recursion in an `async fn` requires boxing + --> $DIR/recursive-impl-trait-type-indirect.rs:73:5 | -LL | fn coroutine_hold() -> impl Sized { - | ^^^^^^^^^^ recursive opaque type -... -LL | let x = coroutine_hold(); - | - coroutine captures itself here +LL | move || { + | ^^^^^^^ recursive `async fn` + | + = note: a recursive `async fn` must be rewritten to return a boxed `dyn Future` + = note: consider using the `async_recursion` crate: https://crates.io/crates/async_recursion error[E0720]: cannot resolve opaque type --> $DIR/recursive-impl-trait-type-indirect.rs:86:26 @@ -144,4 +144,5 @@ LL | mutual_recursion() error: aborting due to 14 previous errors -For more information about this error, try `rustc --explain E0720`. +Some errors have detailed explanations: E0720, E0733. +For more information about an error, try `rustc --explain E0720`. diff --git a/tests/ui/type-alias-impl-trait/indirect-recursion-issue-112047.rs b/tests/ui/type-alias-impl-trait/indirect-recursion-issue-112047.rs index 6a2ee761e191..3637f416c7b5 100644 --- a/tests/ui/type-alias-impl-trait/indirect-recursion-issue-112047.rs +++ b/tests/ui/type-alias-impl-trait/indirect-recursion-issue-112047.rs @@ -1,5 +1,4 @@ // edition: 2021 -// build-fail #![feature(impl_trait_in_assoc_type)] @@ -20,7 +19,7 @@ impl Recur for () { fn recur(self) -> Self::Recur { async move { recur(self).await; } - //~^ ERROR cycle detected when computing layout of + //~^ ERROR recursion in an `async fn` requires boxing } } diff --git a/tests/ui/type-alias-impl-trait/indirect-recursion-issue-112047.stderr b/tests/ui/type-alias-impl-trait/indirect-recursion-issue-112047.stderr index 11d9cd0af081..aa352b326c64 100644 --- a/tests/ui/type-alias-impl-trait/indirect-recursion-issue-112047.stderr +++ b/tests/ui/type-alias-impl-trait/indirect-recursion-issue-112047.stderr @@ -1,27 +1,12 @@ -error[E0391]: cycle detected when computing layout of `{async block@$DIR/indirect-recursion-issue-112047.rs:22:9: 22:42}` - --> $DIR/indirect-recursion-issue-112047.rs:22:22 +error[E0733]: recursion in an `async fn` requires boxing + --> $DIR/indirect-recursion-issue-112047.rs:21:9 | LL | async move { recur(self).await; } - | ^^^^^^^^^^^^^^^^^ + | ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ recursive `async fn` | - = note: ...which requires computing layout of `core::mem::maybe_uninit::MaybeUninit<{async fn body@$DIR/indirect-recursion-issue-112047.rs:14:31: 16:2}>`... - = note: ...which requires computing layout of `core::mem::manually_drop::ManuallyDrop<{async fn body@$DIR/indirect-recursion-issue-112047.rs:14:31: 16:2}>`... -note: ...which requires computing layout of `{async fn body@$DIR/indirect-recursion-issue-112047.rs:14:31: 16:2}`... - --> $DIR/indirect-recursion-issue-112047.rs:15:5 - | -LL | t.recur().await; - | ^^^^^^^^^^^^^^^ - = note: ...which requires computing layout of `core::mem::maybe_uninit::MaybeUninit<<() as Recur>::Recur>`... - = note: ...which requires computing layout of `core::mem::maybe_uninit::MaybeUninit<{async block@$DIR/indirect-recursion-issue-112047.rs:22:9: 22:42}>`... - = note: ...which requires computing layout of `core::mem::manually_drop::ManuallyDrop<{async block@$DIR/indirect-recursion-issue-112047.rs:22:9: 22:42}>`... - = note: ...which again requires computing layout of `{async block@$DIR/indirect-recursion-issue-112047.rs:22:9: 22:42}`, completing the cycle -note: cycle used when elaborating drops for `::recur` - --> $DIR/indirect-recursion-issue-112047.rs:21:5 - | -LL | fn recur(self) -> Self::Recur { - | ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ - = note: see https://rustc-dev-guide.rust-lang.org/overview.html#queries and https://rustc-dev-guide.rust-lang.org/query.html for more information + = note: a recursive `async fn` must be rewritten to return a boxed `dyn Future` + = note: consider using the `async_recursion` crate: https://crates.io/crates/async_recursion error: aborting due to 1 previous error -For more information about this error, try `rustc --explain E0391`. +For more information about this error, try `rustc --explain E0733`. From 199af7cef0cd31e201307769d974631847cf98fd Mon Sep 17 00:00:00 2001 From: Michael Goulet Date: Wed, 8 Nov 2023 07:19:01 +0000 Subject: [PATCH 4/7] Point out source of recursion --- compiler/rustc_middle/src/ty/util.rs | 45 +++++++++++--- compiler/rustc_middle/src/values.rs | 58 ++++++++++++++++--- .../in-trait/async-recursive-generic.rs | 2 +- .../in-trait/async-recursive-generic.stderr | 9 ++- .../async-await/in-trait/async-recursive.rs | 2 +- .../in-trait/async-recursive.stderr | 9 ++- ...utually-recursive-async-impl-trait-type.rs | 2 +- ...lly-recursive-async-impl-trait-type.stderr | 15 ++++- .../recursive-async-impl-trait-type.rs | 2 +- .../recursive-async-impl-trait-type.stderr | 9 ++- .../recursive-impl-trait-type-indirect.stderr | 10 ++-- .../indirect-recursion-issue-112047.rs | 2 +- .../indirect-recursion-issue-112047.stderr | 15 ++++- 13 files changed, 139 insertions(+), 41 deletions(-) diff --git a/compiler/rustc_middle/src/ty/util.rs b/compiler/rustc_middle/src/ty/util.rs index 74dba41647bd..f11a24543f3c 100644 --- a/compiler/rustc_middle/src/ty/util.rs +++ b/compiler/rustc_middle/src/ty/util.rs @@ -729,16 +729,43 @@ impl<'tcx> TyCtxt<'tcx> { DefKind::AssocFn if self.associated_item(def_id).fn_has_self_parameter => "method", DefKind::Closure if let Some(coroutine_kind) = self.coroutine_kind(def_id) => { match coroutine_kind { - hir::CoroutineKind::Desugared(hir::CoroutineDesugaring::Async, _) => { - "async closure" - } - hir::CoroutineKind::Desugared(hir::CoroutineDesugaring::AsyncGen, _) => { - "async gen closure" - } + hir::CoroutineKind::Desugared( + hir::CoroutineDesugaring::Async, + hir::CoroutineSource::Fn, + ) => "async fn", + hir::CoroutineKind::Desugared( + hir::CoroutineDesugaring::Async, + hir::CoroutineSource::Block, + ) => "async block", + hir::CoroutineKind::Desugared( + hir::CoroutineDesugaring::Async, + hir::CoroutineSource::Closure, + ) => "async closure", + hir::CoroutineKind::Desugared( + hir::CoroutineDesugaring::AsyncGen, + hir::CoroutineSource::Fn, + ) => "async gen fn", + hir::CoroutineKind::Desugared( + hir::CoroutineDesugaring::AsyncGen, + hir::CoroutineSource::Block, + ) => "async gen block", + hir::CoroutineKind::Desugared( + hir::CoroutineDesugaring::AsyncGen, + hir::CoroutineSource::Closure, + ) => "async gen closure", + hir::CoroutineKind::Desugared( + hir::CoroutineDesugaring::Gen, + hir::CoroutineSource::Fn, + ) => "gen fn", + hir::CoroutineKind::Desugared( + hir::CoroutineDesugaring::Gen, + hir::CoroutineSource::Block, + ) => "gen block", + hir::CoroutineKind::Desugared( + hir::CoroutineDesugaring::Gen, + hir::CoroutineSource::Closure, + ) => "gen closure", hir::CoroutineKind::Coroutine(_) => "coroutine", - hir::CoroutineKind::Desugared(hir::CoroutineDesugaring::Gen, _) => { - "gen closure" - } } } _ => def_kind.descr(def_id), diff --git a/compiler/rustc_middle/src/values.rs b/compiler/rustc_middle/src/values.rs index 0179829cc46a..3e3aa821b4ec 100644 --- a/compiler/rustc_middle/src/values.rs +++ b/compiler/rustc_middle/src/values.rs @@ -138,7 +138,8 @@ impl<'tcx, T> Value> for Result> let guar = if cycle_error.cycle[0].query.dep_kind == dep_kinds::layout_of && let Some(def_id) = cycle_error.cycle[0].query.ty_def_id && let Some(def_id) = def_id.as_local() - && matches!(tcx.def_kind(def_id), DefKind::Closure) + && let def_kind = tcx.def_kind(def_id) + && matches!(def_kind, DefKind::Closure) && let Some(coroutine_kind) = tcx.coroutine_kind(def_id) { // FIXME: `def_span` for an fn-like coroutine will point to the fn's body @@ -150,13 +151,56 @@ impl<'tcx, T> Value> for Result> } else { tcx.def_span(def_id) }; - struct_span_err!(tcx.sess.dcx(), span, E0733, "recursion in an `async fn` requires boxing") - .span_label(span, "recursive `async fn`") - .note("a recursive `async fn` must be rewritten to return a boxed `dyn Future`") - .note( + let mut diag = struct_span_err!( + tcx.sess.dcx(), + span, + E0733, + "recursion in {} {} requires boxing", + tcx.def_kind_descr_article(def_kind, def_id.to_def_id()), + tcx.def_kind_descr(def_kind, def_id.to_def_id()), + ); + for (i, frame) in cycle_error.cycle.iter().enumerate() { + if frame.query.dep_kind != dep_kinds::layout_of { + continue; + } + let Some(frame_def_id) = frame.query.ty_def_id else { + continue; + }; + let Some(frame_coroutine_kind) = tcx.coroutine_kind(frame_def_id) else { + continue; + }; + let frame_span = frame + .query + .default_span(cycle_error.cycle[(i + 1) % cycle_error.cycle.len()].span); + if frame_span.is_dummy() { + continue; + } + if i == 0 { + diag.span_label(frame_span, "recursive call here"); + } else { + let coroutine_span = if frame_coroutine_kind.is_fn_like() { + tcx.def_span(tcx.parent(frame_def_id)) + } else { + tcx.def_span(frame_def_id) + }; + let mut multispan = MultiSpan::from_span(coroutine_span); + multispan.push_span_label(frame_span, "...leading to this recursive call"); + diag.span_note( + multispan, + format!("which leads to this {}", tcx.def_descr(frame_def_id)), + ); + } + } + if matches!( + coroutine_kind, + hir::CoroutineKind::Desugared(hir::CoroutineDesugaring::Async, _) + ) { + diag.note("a recursive `async fn` call must introduce indirection such as `Box::pin` to avoid an infinitely sized future"); + diag.note( "consider using the `async_recursion` crate: https://crates.io/crates/async_recursion", - ) - .emit() + ); + } + diag.emit() } else { report_cycle(tcx.sess, cycle_error).emit() }; diff --git a/tests/ui/async-await/in-trait/async-recursive-generic.rs b/tests/ui/async-await/in-trait/async-recursive-generic.rs index c6031ce28d1d..33eb2b2de131 100644 --- a/tests/ui/async-await/in-trait/async-recursive-generic.rs +++ b/tests/ui/async-await/in-trait/async-recursive-generic.rs @@ -6,7 +6,7 @@ trait MyTrait { impl MyTrait for T where T: Copy { async fn foo_recursive(&self, n: usize) -> T { - //~^ ERROR recursion in an `async fn` requires boxing + //~^ ERROR recursion in an async fn requires boxing if n > 0 { self.foo_recursive(n - 1).await } else { diff --git a/tests/ui/async-await/in-trait/async-recursive-generic.stderr b/tests/ui/async-await/in-trait/async-recursive-generic.stderr index 11489c18ad40..37de274565d3 100644 --- a/tests/ui/async-await/in-trait/async-recursive-generic.stderr +++ b/tests/ui/async-await/in-trait/async-recursive-generic.stderr @@ -1,10 +1,13 @@ -error[E0733]: recursion in an `async fn` requires boxing +error[E0733]: recursion in an async fn requires boxing --> $DIR/async-recursive-generic.rs:8:5 | LL | async fn foo_recursive(&self, n: usize) -> T { - | ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ recursive `async fn` + | ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ +... +LL | self.foo_recursive(n - 1).await + | ------------------------------- recursive call here | - = note: a recursive `async fn` must be rewritten to return a boxed `dyn Future` + = note: a recursive `async fn` call must introduce indirection such as `Box::pin` to avoid an infinitely sized future = note: consider using the `async_recursion` crate: https://crates.io/crates/async_recursion error: aborting due to 1 previous error diff --git a/tests/ui/async-await/in-trait/async-recursive.rs b/tests/ui/async-await/in-trait/async-recursive.rs index 09f1ffe499e3..2534c43413ed 100644 --- a/tests/ui/async-await/in-trait/async-recursive.rs +++ b/tests/ui/async-await/in-trait/async-recursive.rs @@ -6,7 +6,7 @@ trait MyTrait { impl MyTrait for i32 { async fn foo_recursive(&self, n: usize) -> i32 { - //~^ ERROR recursion in an `async fn` requires boxing + //~^ ERROR recursion in an async fn requires boxing if n > 0 { self.foo_recursive(n - 1).await } else { diff --git a/tests/ui/async-await/in-trait/async-recursive.stderr b/tests/ui/async-await/in-trait/async-recursive.stderr index 587962857269..6b99c516c3b5 100644 --- a/tests/ui/async-await/in-trait/async-recursive.stderr +++ b/tests/ui/async-await/in-trait/async-recursive.stderr @@ -1,10 +1,13 @@ -error[E0733]: recursion in an `async fn` requires boxing +error[E0733]: recursion in an async fn requires boxing --> $DIR/async-recursive.rs:8:5 | LL | async fn foo_recursive(&self, n: usize) -> i32 { - | ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ recursive `async fn` + | ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ +... +LL | self.foo_recursive(n - 1).await + | ------------------------------- recursive call here | - = note: a recursive `async fn` must be rewritten to return a boxed `dyn Future` + = note: a recursive `async fn` call must introduce indirection such as `Box::pin` to avoid an infinitely sized future = note: consider using the `async_recursion` crate: https://crates.io/crates/async_recursion error: aborting due to 1 previous error diff --git a/tests/ui/async-await/mutually-recursive-async-impl-trait-type.rs b/tests/ui/async-await/mutually-recursive-async-impl-trait-type.rs index d38ba1a569bb..fedc814b0418 100644 --- a/tests/ui/async-await/mutually-recursive-async-impl-trait-type.rs +++ b/tests/ui/async-await/mutually-recursive-async-impl-trait-type.rs @@ -2,7 +2,7 @@ // Test that impl trait does not allow creating recursive types that are // otherwise forbidden when using `async` and `await`. -async fn rec_1() { //~ ERROR recursion in an `async fn` +async fn rec_1() { //~ ERROR recursion in an async fn rec_2().await; } diff --git a/tests/ui/async-await/mutually-recursive-async-impl-trait-type.stderr b/tests/ui/async-await/mutually-recursive-async-impl-trait-type.stderr index dd53075be602..3f2ee4150dae 100644 --- a/tests/ui/async-await/mutually-recursive-async-impl-trait-type.stderr +++ b/tests/ui/async-await/mutually-recursive-async-impl-trait-type.stderr @@ -1,10 +1,19 @@ -error[E0733]: recursion in an `async fn` requires boxing +error[E0733]: recursion in an async fn requires boxing --> $DIR/mutually-recursive-async-impl-trait-type.rs:5:1 | LL | async fn rec_1() { - | ^^^^^^^^^^^^^^^^ recursive `async fn` + | ^^^^^^^^^^^^^^^^ +LL | rec_2().await; + | ------------- recursive call here | - = note: a recursive `async fn` must be rewritten to return a boxed `dyn Future` +note: which leads to this async fn + --> $DIR/mutually-recursive-async-impl-trait-type.rs:9:1 + | +LL | async fn rec_2() { + | ^^^^^^^^^^^^^^^^ +LL | rec_1().await; + | ------------- ...leading to this recursive call + = note: a recursive `async fn` call must introduce indirection such as `Box::pin` to avoid an infinitely sized future = note: consider using the `async_recursion` crate: https://crates.io/crates/async_recursion error: aborting due to 1 previous error diff --git a/tests/ui/async-await/recursive-async-impl-trait-type.rs b/tests/ui/async-await/recursive-async-impl-trait-type.rs index edc4cb8ac5df..9351ee53f075 100644 --- a/tests/ui/async-await/recursive-async-impl-trait-type.rs +++ b/tests/ui/async-await/recursive-async-impl-trait-type.rs @@ -3,7 +3,7 @@ // otherwise forbidden when using `async` and `await`. async fn recursive_async_function() -> () { - //~^ ERROR recursion in an `async fn` requires boxing + //~^ ERROR recursion in an async fn requires boxing recursive_async_function().await; } diff --git a/tests/ui/async-await/recursive-async-impl-trait-type.stderr b/tests/ui/async-await/recursive-async-impl-trait-type.stderr index 969258f84ed6..cec92c54f01f 100644 --- a/tests/ui/async-await/recursive-async-impl-trait-type.stderr +++ b/tests/ui/async-await/recursive-async-impl-trait-type.stderr @@ -1,10 +1,13 @@ -error[E0733]: recursion in an `async fn` requires boxing +error[E0733]: recursion in an async fn requires boxing --> $DIR/recursive-async-impl-trait-type.rs:5:1 | LL | async fn recursive_async_function() -> () { - | ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ recursive `async fn` + | ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ +LL | +LL | recursive_async_function().await; + | -------------------------------- recursive call here | - = note: a recursive `async fn` must be rewritten to return a boxed `dyn Future` + = note: a recursive `async fn` call must introduce indirection such as `Box::pin` to avoid an infinitely sized future = note: consider using the `async_recursion` crate: https://crates.io/crates/async_recursion error: aborting due to 1 previous error diff --git a/tests/ui/impl-trait/recursive-impl-trait-type-indirect.stderr b/tests/ui/impl-trait/recursive-impl-trait-type-indirect.stderr index 0dabba264689..16a9012958e9 100644 --- a/tests/ui/impl-trait/recursive-impl-trait-type-indirect.stderr +++ b/tests/ui/impl-trait/recursive-impl-trait-type-indirect.stderr @@ -109,14 +109,14 @@ LL | LL | (substs_change::<&T>(),) | ------------------------ returning here with type `(impl Sized,)` -error[E0733]: recursion in an `async fn` requires boxing +error[E0733]: recursion in a coroutine requires boxing --> $DIR/recursive-impl-trait-type-indirect.rs:73:5 | LL | move || { - | ^^^^^^^ recursive `async fn` - | - = note: a recursive `async fn` must be rewritten to return a boxed `dyn Future` - = note: consider using the `async_recursion` crate: https://crates.io/crates/async_recursion + | ^^^^^^^ +LL | +LL | let x = coroutine_hold(); + | - recursive call here error[E0720]: cannot resolve opaque type --> $DIR/recursive-impl-trait-type-indirect.rs:86:26 diff --git a/tests/ui/type-alias-impl-trait/indirect-recursion-issue-112047.rs b/tests/ui/type-alias-impl-trait/indirect-recursion-issue-112047.rs index 3637f416c7b5..7a2ea881b643 100644 --- a/tests/ui/type-alias-impl-trait/indirect-recursion-issue-112047.rs +++ b/tests/ui/type-alias-impl-trait/indirect-recursion-issue-112047.rs @@ -19,7 +19,7 @@ impl Recur for () { fn recur(self) -> Self::Recur { async move { recur(self).await; } - //~^ ERROR recursion in an `async fn` requires boxing + //~^ ERROR recursion in an async block requires boxing } } diff --git a/tests/ui/type-alias-impl-trait/indirect-recursion-issue-112047.stderr b/tests/ui/type-alias-impl-trait/indirect-recursion-issue-112047.stderr index aa352b326c64..5427ebe92ad9 100644 --- a/tests/ui/type-alias-impl-trait/indirect-recursion-issue-112047.stderr +++ b/tests/ui/type-alias-impl-trait/indirect-recursion-issue-112047.stderr @@ -1,10 +1,19 @@ -error[E0733]: recursion in an `async fn` requires boxing +error[E0733]: recursion in an async block requires boxing --> $DIR/indirect-recursion-issue-112047.rs:21:9 | LL | async move { recur(self).await; } - | ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ recursive `async fn` + | ^^^^^^^^^^^^^-----------------^^^ + | | + | recursive call here | - = note: a recursive `async fn` must be rewritten to return a boxed `dyn Future` +note: which leads to this async fn + --> $DIR/indirect-recursion-issue-112047.rs:13:1 + | +LL | async fn recur(t: impl Recur) { + | ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ +LL | t.recur().await; + | --------------- ...leading to this recursive call + = note: a recursive `async fn` call must introduce indirection such as `Box::pin` to avoid an infinitely sized future = note: consider using the `async_recursion` crate: https://crates.io/crates/async_recursion error: aborting due to 1 previous error From fa2ff51ace4f1c415cbe24687828058bcf74d200 Mon Sep 17 00:00:00 2001 From: Michael Goulet Date: Mon, 13 Nov 2023 02:08:14 +0000 Subject: [PATCH 5/7] Only compute layout of opaque if coroutine is the cause of an opaque cycle --- .../rustc_hir_analysis/src/check/check.rs | 35 ++++++++++++++----- compiler/rustc_middle/src/ty/util.rs | 25 ++++++++++++- .../indirect-recursion-issue-112047.rs | 1 + .../indirect-recursion-issue-112047.stderr | 4 +-- 4 files changed, 53 insertions(+), 12 deletions(-) diff --git a/compiler/rustc_hir_analysis/src/check/check.rs b/compiler/rustc_hir_analysis/src/check/check.rs index 12430b1465c3..7f23c04ce2df 100644 --- a/compiler/rustc_hir_analysis/src/check/check.rs +++ b/compiler/rustc_hir_analysis/src/check/check.rs @@ -17,7 +17,7 @@ use rustc_middle::middle::stability::EvalResult; use rustc_middle::traits::{DefiningAnchor, ObligationCauseCode}; use rustc_middle::ty::fold::BottomUpFolder; use rustc_middle::ty::layout::{LayoutError, MAX_SIMD_LANES}; -use rustc_middle::ty::util::{Discr, IntTypeExt}; +use rustc_middle::ty::util::{Discr, InspectCoroutineFields, IntTypeExt}; use rustc_middle::ty::GenericArgKind; use rustc_middle::ty::{ AdtDef, ParamEnv, RegionKind, TypeSuperVisitable, TypeVisitable, TypeVisitableExt, @@ -232,16 +232,33 @@ pub(super) fn check_opaque_for_cycles<'tcx>( span: Span, ) -> Result<(), ErrorGuaranteed> { let args = GenericArgs::identity_for_item(tcx, def_id); - if tcx.try_expand_impl_trait_type(def_id.to_def_id(), args).is_err() { - let reported = opaque_type_cycle_error(tcx, def_id, span); - Err(reported) - } else if let Err(&LayoutError::Cycle(guar)) = - tcx.layout_of(tcx.param_env(def_id).and(Ty::new_opaque(tcx, def_id.to_def_id(), args))) + + // First, try to look at any opaque expansion cycles, considering coroutine fields + // (even though these aren't necessarily true errors). + if tcx + .try_expand_impl_trait_type(def_id.to_def_id(), args, InspectCoroutineFields::Yes) + .is_err() { - Err(guar) - } else { - Ok(()) + // Look for true opaque expansion cycles, but ignore coroutines. + // This will give us any true errors. Coroutines are only problematic + // if they cause layout computation errors. + if tcx + .try_expand_impl_trait_type(def_id.to_def_id(), args, InspectCoroutineFields::No) + .is_err() + { + let reported = opaque_type_cycle_error(tcx, def_id, span); + return Err(reported); + } + + // And also look for cycle errors in the layout of coroutines. + if let Err(&LayoutError::Cycle(guar)) = + tcx.layout_of(tcx.param_env(def_id).and(Ty::new_opaque(tcx, def_id.to_def_id(), args))) + { + return Err(guar); + } } + + Ok(()) } /// Check that the concrete type behind `impl Trait` actually implements `Trait`. diff --git a/compiler/rustc_middle/src/ty/util.rs b/compiler/rustc_middle/src/ty/util.rs index f11a24543f3c..d9f2034bba53 100644 --- a/compiler/rustc_middle/src/ty/util.rs +++ b/compiler/rustc_middle/src/ty/util.rs @@ -702,6 +702,7 @@ impl<'tcx> TyCtxt<'tcx> { self, def_id: DefId, args: GenericArgsRef<'tcx>, + inspect_coroutine_fields: InspectCoroutineFields, ) -> Result, Ty<'tcx>> { let mut visitor = OpaqueTypeExpander { seen_opaque_tys: FxHashSet::default(), @@ -712,6 +713,7 @@ impl<'tcx> TyCtxt<'tcx> { check_recursion: true, expand_coroutines: true, tcx: self, + inspect_coroutine_fields, }; let expanded_type = visitor.expand_opaque_ty(def_id, args).unwrap(); @@ -885,6 +887,13 @@ struct OpaqueTypeExpander<'tcx> { /// recursion, and 'false' otherwise to avoid unnecessary work. check_recursion: bool, tcx: TyCtxt<'tcx>, + inspect_coroutine_fields: InspectCoroutineFields, +} + +#[derive(Copy, Clone, PartialEq, Eq, Debug)] +pub enum InspectCoroutineFields { + No, + Yes, } impl<'tcx> OpaqueTypeExpander<'tcx> { @@ -923,7 +932,20 @@ impl<'tcx> OpaqueTypeExpander<'tcx> { } let args = args.fold_with(self); if !self.check_recursion || self.seen_opaque_tys.insert(def_id) { - let expanded_ty = Ty::new_coroutine_witness(self.tcx, def_id, args); + let expanded_ty = match self.expanded_cache.get(&(def_id, args)) { + Some(expanded_ty) => *expanded_ty, + None => { + if matches!(self.inspect_coroutine_fields, InspectCoroutineFields::Yes) { + for bty in self.tcx.coroutine_hidden_types(def_id) { + let hidden_ty = bty.instantiate(self.tcx, args); + self.fold_ty(hidden_ty); + } + } + let expanded_ty = Ty::new_coroutine_witness(self.tcx, def_id, args); + self.expanded_cache.insert((def_id, args), expanded_ty); + expanded_ty + } + }; if self.check_recursion { self.seen_opaque_tys.remove(&def_id); } @@ -1495,6 +1517,7 @@ pub fn reveal_opaque_types_in_bounds<'tcx>( check_recursion: false, expand_coroutines: false, tcx, + inspect_coroutine_fields: InspectCoroutineFields::No, }; val.fold_with(&mut visitor) } diff --git a/tests/ui/type-alias-impl-trait/indirect-recursion-issue-112047.rs b/tests/ui/type-alias-impl-trait/indirect-recursion-issue-112047.rs index 7a2ea881b643..e7b23d5f8a1c 100644 --- a/tests/ui/type-alias-impl-trait/indirect-recursion-issue-112047.rs +++ b/tests/ui/type-alias-impl-trait/indirect-recursion-issue-112047.rs @@ -1,4 +1,5 @@ // edition: 2021 +// build-fail #![feature(impl_trait_in_assoc_type)] diff --git a/tests/ui/type-alias-impl-trait/indirect-recursion-issue-112047.stderr b/tests/ui/type-alias-impl-trait/indirect-recursion-issue-112047.stderr index 5427ebe92ad9..2e75f676971c 100644 --- a/tests/ui/type-alias-impl-trait/indirect-recursion-issue-112047.stderr +++ b/tests/ui/type-alias-impl-trait/indirect-recursion-issue-112047.stderr @@ -1,5 +1,5 @@ error[E0733]: recursion in an async block requires boxing - --> $DIR/indirect-recursion-issue-112047.rs:21:9 + --> $DIR/indirect-recursion-issue-112047.rs:22:9 | LL | async move { recur(self).await; } | ^^^^^^^^^^^^^-----------------^^^ @@ -7,7 +7,7 @@ LL | async move { recur(self).await; } | recursive call here | note: which leads to this async fn - --> $DIR/indirect-recursion-issue-112047.rs:13:1 + --> $DIR/indirect-recursion-issue-112047.rs:14:1 | LL | async fn recur(t: impl Recur) { | ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ From 841184bcae3000335c26bfae24f8be44beb81759 Mon Sep 17 00:00:00 2001 From: Michael Goulet Date: Fri, 24 Nov 2023 15:05:11 +0000 Subject: [PATCH 6/7] Make cycle error more resilient to where it starts Also don't recomment recursive_async crate anymore Co-authored-by: lcnr --- compiler/rustc_middle/src/values.rs | 131 +++++++++--------- .../in-trait/async-recursive-generic.stderr | 1 - .../in-trait/async-recursive.stderr | 1 - .../indirect-recursion-issue-112047.rs | 4 +- .../indirect-recursion-issue-112047.stderr | 15 +- ...lly-recursive-async-impl-trait-type.stderr | 1 - .../recursive-async-impl-trait-type.stderr | 1 - ...outine.rs => recursive-coroutine-boxed.rs} | 0 ...ecursive-coroutine-indirect.current.stderr | 11 ++ .../recursive-coroutine-indirect.next.stderr | 11 ++ .../recursive-coroutine-indirect.rs | 13 ++ .../recursive-impl-trait-type-indirect.rs | 10 -- .../recursive-impl-trait-type-indirect.stderr | 50 +++---- .../indirect-recursion-issue-112047.stderr | 1 - 14 files changed, 134 insertions(+), 116 deletions(-) rename tests/ui/impl-trait/{recursive-coroutine.rs => recursive-coroutine-boxed.rs} (100%) create mode 100644 tests/ui/impl-trait/recursive-coroutine-indirect.current.stderr create mode 100644 tests/ui/impl-trait/recursive-coroutine-indirect.next.stderr create mode 100644 tests/ui/impl-trait/recursive-coroutine-indirect.rs diff --git a/compiler/rustc_middle/src/values.rs b/compiler/rustc_middle/src/values.rs index 3e3aa821b4ec..77e4ed06eaa2 100644 --- a/compiler/rustc_middle/src/values.rs +++ b/compiler/rustc_middle/src/values.rs @@ -11,6 +11,7 @@ use rustc_query_system::Value; use rustc_span::def_id::LocalDefId; use rustc_span::{ErrorGuaranteed, Span}; +use std::collections::VecDeque; use std::fmt::Write; impl<'tcx> Value> for Ty<'_> { @@ -135,73 +136,79 @@ impl<'tcx, T> Value> for Result> cycle_error: &CycleError, _guar: ErrorGuaranteed, ) -> Self { - let guar = if cycle_error.cycle[0].query.dep_kind == dep_kinds::layout_of - && let Some(def_id) = cycle_error.cycle[0].query.ty_def_id - && let Some(def_id) = def_id.as_local() - && let def_kind = tcx.def_kind(def_id) - && matches!(def_kind, DefKind::Closure) - && let Some(coroutine_kind) = tcx.coroutine_kind(def_id) - { - // FIXME: `def_span` for an fn-like coroutine will point to the fn's body - // due to interactions between the desugaring into a closure expr and the - // def_span code. I'm not motivated to fix it, because I tried and it was - // not working, so just hack around it by grabbing the parent fn's span. - let span = if coroutine_kind.is_fn_like() { - tcx.def_span(tcx.local_parent(def_id)) - } else { - tcx.def_span(def_id) - }; - let mut diag = struct_span_err!( - tcx.sess.dcx(), - span, - E0733, - "recursion in {} {} requires boxing", - tcx.def_kind_descr_article(def_kind, def_id.to_def_id()), - tcx.def_kind_descr(def_kind, def_id.to_def_id()), - ); - for (i, frame) in cycle_error.cycle.iter().enumerate() { - if frame.query.dep_kind != dep_kinds::layout_of { - continue; - } - let Some(frame_def_id) = frame.query.ty_def_id else { - continue; - }; - let Some(frame_coroutine_kind) = tcx.coroutine_kind(frame_def_id) else { - continue; - }; - let frame_span = frame - .query - .default_span(cycle_error.cycle[(i + 1) % cycle_error.cycle.len()].span); - if frame_span.is_dummy() { - continue; - } - if i == 0 { - diag.span_label(frame_span, "recursive call here"); - } else { - let coroutine_span = if frame_coroutine_kind.is_fn_like() { - tcx.def_span(tcx.parent(frame_def_id)) + let mut cycle: VecDeque<_> = cycle_error.cycle.iter().collect(); + + let guar = 'search: { + for _ in 0..cycle.len() { + if cycle[0].query.dep_kind == dep_kinds::layout_of + && let Some(def_id) = cycle[0].query.ty_def_id + && let Some(def_id) = def_id.as_local() + && let def_kind = tcx.def_kind(def_id) + && matches!(def_kind, DefKind::Closure) + && let Some(coroutine_kind) = tcx.coroutine_kind(def_id) + { + // FIXME: `def_span` for an fn-like coroutine will point to the fn's body + // due to interactions between the desugaring into a closure expr and the + // def_span code. I'm not motivated to fix it, because I tried and it was + // not working, so just hack around it by grabbing the parent fn's span. + let span = if coroutine_kind.is_fn_like() { + tcx.def_span(tcx.local_parent(def_id)) } else { - tcx.def_span(frame_def_id) + tcx.def_span(def_id) }; - let mut multispan = MultiSpan::from_span(coroutine_span); - multispan.push_span_label(frame_span, "...leading to this recursive call"); - diag.span_note( - multispan, - format!("which leads to this {}", tcx.def_descr(frame_def_id)), + let mut diag = struct_span_err!( + tcx.sess.dcx(), + span, + E0733, + "recursion in {} {} requires boxing", + tcx.def_kind_descr_article(def_kind, def_id.to_def_id()), + tcx.def_kind_descr(def_kind, def_id.to_def_id()), ); + for (i, frame) in cycle.iter().enumerate() { + if frame.query.dep_kind != dep_kinds::layout_of { + continue; + } + let Some(frame_def_id) = frame.query.ty_def_id else { + continue; + }; + let Some(frame_coroutine_kind) = tcx.coroutine_kind(frame_def_id) else { + continue; + }; + let frame_span = + frame.query.default_span(cycle[(i + 1) % cycle.len()].span); + if frame_span.is_dummy() { + continue; + } + if i == 0 { + diag.span_label(frame_span, "recursive call here"); + } else { + let coroutine_span: Span = if frame_coroutine_kind.is_fn_like() { + tcx.def_span(tcx.parent(frame_def_id)) + } else { + tcx.def_span(frame_def_id) + }; + let mut multispan = MultiSpan::from_span(coroutine_span); + multispan + .push_span_label(frame_span, "...leading to this recursive call"); + diag.span_note( + multispan, + format!("which leads to this {}", tcx.def_descr(frame_def_id)), + ); + } + } + // FIXME: We could report a structured suggestion if we had + // enough info here... Maybe we can use a hacky HIR walker. + if matches!( + coroutine_kind, + hir::CoroutineKind::Desugared(hir::CoroutineDesugaring::Async, _) + ) { + diag.note("a recursive `async fn` call must introduce indirection such as `Box::pin` to avoid an infinitely sized future"); + } + break 'search diag.emit(); + } else { + cycle.rotate_left(1); } } - if matches!( - coroutine_kind, - hir::CoroutineKind::Desugared(hir::CoroutineDesugaring::Async, _) - ) { - diag.note("a recursive `async fn` call must introduce indirection such as `Box::pin` to avoid an infinitely sized future"); - diag.note( - "consider using the `async_recursion` crate: https://crates.io/crates/async_recursion", - ); - } - diag.emit() - } else { report_cycle(tcx.sess, cycle_error).emit() }; diff --git a/tests/ui/async-await/in-trait/async-recursive-generic.stderr b/tests/ui/async-await/in-trait/async-recursive-generic.stderr index 37de274565d3..d085747bc4bf 100644 --- a/tests/ui/async-await/in-trait/async-recursive-generic.stderr +++ b/tests/ui/async-await/in-trait/async-recursive-generic.stderr @@ -8,7 +8,6 @@ LL | self.foo_recursive(n - 1).await | ------------------------------- recursive call here | = note: a recursive `async fn` call must introduce indirection such as `Box::pin` to avoid an infinitely sized future - = note: consider using the `async_recursion` crate: https://crates.io/crates/async_recursion error: aborting due to 1 previous error diff --git a/tests/ui/async-await/in-trait/async-recursive.stderr b/tests/ui/async-await/in-trait/async-recursive.stderr index 6b99c516c3b5..25ebc6e77c4f 100644 --- a/tests/ui/async-await/in-trait/async-recursive.stderr +++ b/tests/ui/async-await/in-trait/async-recursive.stderr @@ -8,7 +8,6 @@ LL | self.foo_recursive(n - 1).await | ------------------------------- recursive call here | = note: a recursive `async fn` call must introduce indirection such as `Box::pin` to avoid an infinitely sized future - = note: consider using the `async_recursion` crate: https://crates.io/crates/async_recursion error: aborting due to 1 previous error diff --git a/tests/ui/async-await/in-trait/indirect-recursion-issue-112047.rs b/tests/ui/async-await/in-trait/indirect-recursion-issue-112047.rs index 8443cbcf4ac8..4b615343a05f 100644 --- a/tests/ui/async-await/in-trait/indirect-recursion-issue-112047.rs +++ b/tests/ui/async-await/in-trait/indirect-recursion-issue-112047.rs @@ -1,6 +1,7 @@ // edition: 2021 + +// Test doesn't fail until monomorphization time, unfortunately. // build-fail -//~^^ ERROR cycle detected when computing layout of fn main() { let _ = async { @@ -31,6 +32,7 @@ where C: First, { async fn second(self) { + //~^ ERROR recursion in an async fn requires boxing self.first().await.second().await; } } diff --git a/tests/ui/async-await/in-trait/indirect-recursion-issue-112047.stderr b/tests/ui/async-await/in-trait/indirect-recursion-issue-112047.stderr index 8e573b512ad4..8126c6e13942 100644 --- a/tests/ui/async-await/in-trait/indirect-recursion-issue-112047.stderr +++ b/tests/ui/async-await/in-trait/indirect-recursion-issue-112047.stderr @@ -1,12 +1,11 @@ -error[E0391]: cycle detected when computing layout of `core::mem::maybe_uninit::MaybeUninit<{async fn body@$DIR/indirect-recursion-issue-112047.rs:33:27: 35:6}>` +error[E0733]: recursion in an async fn requires boxing + --> $DIR/indirect-recursion-issue-112047.rs:34:5 | - = note: ...which requires computing layout of `core::mem::manually_drop::ManuallyDrop<{async fn body@$DIR/indirect-recursion-issue-112047.rs:33:27: 35:6}>`... - = note: ...which requires computing layout of `{async fn body@$DIR/indirect-recursion-issue-112047.rs:33:27: 35:6}`... - = note: ...which requires computing layout of `core::mem::maybe_uninit::MaybeUninit<<::Second as Second>::{opaque#0}>`... - = note: ...which again requires computing layout of `core::mem::maybe_uninit::MaybeUninit<{async fn body@$DIR/indirect-recursion-issue-112047.rs:33:27: 35:6}>`, completing the cycle - = note: cycle used when computing layout of `{async block@$DIR/indirect-recursion-issue-112047.rs:6:13: 8:6}` - = note: see https://rustc-dev-guide.rust-lang.org/overview.html#queries and https://rustc-dev-guide.rust-lang.org/query.html for more information +LL | async fn second(self) { + | ^^^^^^^^^^^^^^^^^^^^^ + | + = note: a recursive `async fn` call must introduce indirection such as `Box::pin` to avoid an infinitely sized future error: aborting due to 1 previous error -For more information about this error, try `rustc --explain E0391`. +For more information about this error, try `rustc --explain E0733`. diff --git a/tests/ui/async-await/mutually-recursive-async-impl-trait-type.stderr b/tests/ui/async-await/mutually-recursive-async-impl-trait-type.stderr index 3f2ee4150dae..52fb41be1fbc 100644 --- a/tests/ui/async-await/mutually-recursive-async-impl-trait-type.stderr +++ b/tests/ui/async-await/mutually-recursive-async-impl-trait-type.stderr @@ -14,7 +14,6 @@ LL | async fn rec_2() { LL | rec_1().await; | ------------- ...leading to this recursive call = note: a recursive `async fn` call must introduce indirection such as `Box::pin` to avoid an infinitely sized future - = note: consider using the `async_recursion` crate: https://crates.io/crates/async_recursion error: aborting due to 1 previous error diff --git a/tests/ui/async-await/recursive-async-impl-trait-type.stderr b/tests/ui/async-await/recursive-async-impl-trait-type.stderr index cec92c54f01f..5475469335fb 100644 --- a/tests/ui/async-await/recursive-async-impl-trait-type.stderr +++ b/tests/ui/async-await/recursive-async-impl-trait-type.stderr @@ -8,7 +8,6 @@ LL | recursive_async_function().await; | -------------------------------- recursive call here | = note: a recursive `async fn` call must introduce indirection such as `Box::pin` to avoid an infinitely sized future - = note: consider using the `async_recursion` crate: https://crates.io/crates/async_recursion error: aborting due to 1 previous error diff --git a/tests/ui/impl-trait/recursive-coroutine.rs b/tests/ui/impl-trait/recursive-coroutine-boxed.rs similarity index 100% rename from tests/ui/impl-trait/recursive-coroutine.rs rename to tests/ui/impl-trait/recursive-coroutine-boxed.rs diff --git a/tests/ui/impl-trait/recursive-coroutine-indirect.current.stderr b/tests/ui/impl-trait/recursive-coroutine-indirect.current.stderr new file mode 100644 index 000000000000..11b3c4ef0078 --- /dev/null +++ b/tests/ui/impl-trait/recursive-coroutine-indirect.current.stderr @@ -0,0 +1,11 @@ +error[E0733]: recursion in a coroutine requires boxing + --> $DIR/recursive-coroutine-indirect.rs:6:5 + | +LL | move || { + | ^^^^^^^ +LL | let x = coroutine_hold(); + | - recursive call here + +error: aborting due to 1 previous error + +For more information about this error, try `rustc --explain E0733`. diff --git a/tests/ui/impl-trait/recursive-coroutine-indirect.next.stderr b/tests/ui/impl-trait/recursive-coroutine-indirect.next.stderr new file mode 100644 index 000000000000..11b3c4ef0078 --- /dev/null +++ b/tests/ui/impl-trait/recursive-coroutine-indirect.next.stderr @@ -0,0 +1,11 @@ +error[E0733]: recursion in a coroutine requires boxing + --> $DIR/recursive-coroutine-indirect.rs:6:5 + | +LL | move || { + | ^^^^^^^ +LL | let x = coroutine_hold(); + | - recursive call here + +error: aborting due to 1 previous error + +For more information about this error, try `rustc --explain E0733`. diff --git a/tests/ui/impl-trait/recursive-coroutine-indirect.rs b/tests/ui/impl-trait/recursive-coroutine-indirect.rs new file mode 100644 index 000000000000..4f8d4d330505 --- /dev/null +++ b/tests/ui/impl-trait/recursive-coroutine-indirect.rs @@ -0,0 +1,13 @@ +// revisions: current next +//[next] compile-flags: -Znext-solver +#![feature(coroutines)] +#![allow(unconditional_recursion)] +fn coroutine_hold() -> impl Sized { + move || { //~ ERROR recursion in a coroutine requires boxing + let x = coroutine_hold(); + yield; + x; + } +} + +fn main() {} diff --git a/tests/ui/impl-trait/recursive-impl-trait-type-indirect.rs b/tests/ui/impl-trait/recursive-impl-trait-type-indirect.rs index a6bca107b1ec..432f80a1763e 100644 --- a/tests/ui/impl-trait/recursive-impl-trait-type-indirect.rs +++ b/tests/ui/impl-trait/recursive-impl-trait-type-indirect.rs @@ -1,6 +1,5 @@ // Test that impl trait does not allow creating recursive types that are // otherwise forbidden. - #![feature(coroutines)] #![allow(unconditional_recursion)] @@ -69,15 +68,6 @@ fn substs_change() -> impl Sized { (substs_change::<&T>(),) } -fn coroutine_hold() -> impl Sized { - move || { - //~^ ERROR - let x = coroutine_hold(); - yield; - x; - } -} - fn use_fn_ptr() -> impl Sized { // OK, error already reported fn_ptr() diff --git a/tests/ui/impl-trait/recursive-impl-trait-type-indirect.stderr b/tests/ui/impl-trait/recursive-impl-trait-type-indirect.stderr index 16a9012958e9..d5b8c531fd6e 100644 --- a/tests/ui/impl-trait/recursive-impl-trait-type-indirect.stderr +++ b/tests/ui/impl-trait/recursive-impl-trait-type-indirect.stderr @@ -1,5 +1,5 @@ error[E0720]: cannot resolve opaque type - --> $DIR/recursive-impl-trait-type-indirect.rs:7:22 + --> $DIR/recursive-impl-trait-type-indirect.rs:6:22 | LL | fn option(i: i32) -> impl Sized { | ^^^^^^^^^^ recursive opaque type @@ -10,7 +10,7 @@ LL | if i < 0 { None } else { Some((option(i - 1), i)) } | returning here with type `Option<(impl Sized, i32)>` error[E0720]: cannot resolve opaque type - --> $DIR/recursive-impl-trait-type-indirect.rs:12:15 + --> $DIR/recursive-impl-trait-type-indirect.rs:11:15 | LL | fn tuple() -> impl Sized { | ^^^^^^^^^^ recursive opaque type @@ -19,7 +19,7 @@ LL | (tuple(),) | ---------- returning here with type `(impl Sized,)` error[E0720]: cannot resolve opaque type - --> $DIR/recursive-impl-trait-type-indirect.rs:17:15 + --> $DIR/recursive-impl-trait-type-indirect.rs:16:15 | LL | fn array() -> impl Sized { | ^^^^^^^^^^ recursive opaque type @@ -28,7 +28,7 @@ LL | [array()] | --------- returning here with type `[impl Sized; 1]` error[E0720]: cannot resolve opaque type - --> $DIR/recursive-impl-trait-type-indirect.rs:22:13 + --> $DIR/recursive-impl-trait-type-indirect.rs:21:13 | LL | fn ptr() -> impl Sized { | ^^^^^^^^^^ recursive opaque type @@ -37,7 +37,7 @@ LL | &ptr() as *const _ | ------------------ returning here with type `*const impl Sized` error[E0720]: cannot resolve opaque type - --> $DIR/recursive-impl-trait-type-indirect.rs:27:16 + --> $DIR/recursive-impl-trait-type-indirect.rs:26:16 | LL | fn fn_ptr() -> impl Sized { | ^^^^^^^^^^ recursive opaque type @@ -46,7 +46,7 @@ LL | fn_ptr as fn() -> _ | ------------------- returning here with type `fn() -> impl Sized` error[E0720]: cannot resolve opaque type - --> $DIR/recursive-impl-trait-type-indirect.rs:32:25 + --> $DIR/recursive-impl-trait-type-indirect.rs:31:25 | LL | fn closure_capture() -> impl Sized { | ^^^^^^^^^^ recursive opaque type @@ -55,10 +55,10 @@ LL | / move || { LL | | x; | | - closure captures itself here LL | | } - | |_____- returning here with type `{closure@$DIR/recursive-impl-trait-type-indirect.rs:35:5: 35:12}` + | |_____- returning here with type `{closure@$DIR/recursive-impl-trait-type-indirect.rs:34:5: 34:12}` error[E0720]: cannot resolve opaque type - --> $DIR/recursive-impl-trait-type-indirect.rs:40:29 + --> $DIR/recursive-impl-trait-type-indirect.rs:39:29 | LL | fn closure_ref_capture() -> impl Sized { | ^^^^^^^^^^ recursive opaque type @@ -67,28 +67,28 @@ LL | / move || { LL | | &x; | | - closure captures itself here LL | | } - | |_____- returning here with type `{closure@$DIR/recursive-impl-trait-type-indirect.rs:43:5: 43:12}` + | |_____- returning here with type `{closure@$DIR/recursive-impl-trait-type-indirect.rs:42:5: 42:12}` error[E0720]: cannot resolve opaque type - --> $DIR/recursive-impl-trait-type-indirect.rs:48:21 + --> $DIR/recursive-impl-trait-type-indirect.rs:47:21 | LL | fn closure_sig() -> impl Sized { | ^^^^^^^^^^ recursive opaque type LL | LL | || closure_sig() - | ---------------- returning here with type `{closure@$DIR/recursive-impl-trait-type-indirect.rs:50:5: 50:7}` + | ---------------- returning here with type `{closure@$DIR/recursive-impl-trait-type-indirect.rs:49:5: 49:7}` error[E0720]: cannot resolve opaque type - --> $DIR/recursive-impl-trait-type-indirect.rs:53:23 + --> $DIR/recursive-impl-trait-type-indirect.rs:52:23 | LL | fn coroutine_sig() -> impl Sized { | ^^^^^^^^^^ recursive opaque type LL | LL | || coroutine_sig() - | ------------------ returning here with type `{closure@$DIR/recursive-impl-trait-type-indirect.rs:55:5: 55:7}` + | ------------------ returning here with type `{closure@$DIR/recursive-impl-trait-type-indirect.rs:54:5: 54:7}` error[E0720]: cannot resolve opaque type - --> $DIR/recursive-impl-trait-type-indirect.rs:58:27 + --> $DIR/recursive-impl-trait-type-indirect.rs:57:27 | LL | fn coroutine_capture() -> impl Sized { | ^^^^^^^^^^ recursive opaque type @@ -98,10 +98,10 @@ LL | | yield; LL | | x; | | - coroutine captures itself here LL | | } - | |_____- returning here with type `{coroutine@$DIR/recursive-impl-trait-type-indirect.rs:61:5: 61:12}` + | |_____- returning here with type `{coroutine@$DIR/recursive-impl-trait-type-indirect.rs:60:5: 60:12}` error[E0720]: cannot resolve opaque type - --> $DIR/recursive-impl-trait-type-indirect.rs:67:35 + --> $DIR/recursive-impl-trait-type-indirect.rs:66:35 | LL | fn substs_change() -> impl Sized { | ^^^^^^^^^^ recursive opaque type @@ -109,17 +109,8 @@ LL | LL | (substs_change::<&T>(),) | ------------------------ returning here with type `(impl Sized,)` -error[E0733]: recursion in a coroutine requires boxing - --> $DIR/recursive-impl-trait-type-indirect.rs:73:5 - | -LL | move || { - | ^^^^^^^ -LL | -LL | let x = coroutine_hold(); - | - recursive call here - error[E0720]: cannot resolve opaque type - --> $DIR/recursive-impl-trait-type-indirect.rs:86:26 + --> $DIR/recursive-impl-trait-type-indirect.rs:76:26 | LL | fn mutual_recursion() -> impl Sync { | ^^^^^^^^^ recursive opaque type @@ -131,7 +122,7 @@ LL | fn mutual_recursion_b() -> impl Sized { | ---------- returning this opaque type `impl Sized` error[E0720]: cannot resolve opaque type - --> $DIR/recursive-impl-trait-type-indirect.rs:91:28 + --> $DIR/recursive-impl-trait-type-indirect.rs:81:28 | LL | fn mutual_recursion() -> impl Sync { | --------- returning this opaque type `impl Sync` @@ -142,7 +133,6 @@ LL | LL | mutual_recursion() | ------------------ returning here with type `impl Sync` -error: aborting due to 14 previous errors +error: aborting due to 13 previous errors -Some errors have detailed explanations: E0720, E0733. -For more information about an error, try `rustc --explain E0720`. +For more information about this error, try `rustc --explain E0720`. diff --git a/tests/ui/type-alias-impl-trait/indirect-recursion-issue-112047.stderr b/tests/ui/type-alias-impl-trait/indirect-recursion-issue-112047.stderr index 2e75f676971c..b62186103c7c 100644 --- a/tests/ui/type-alias-impl-trait/indirect-recursion-issue-112047.stderr +++ b/tests/ui/type-alias-impl-trait/indirect-recursion-issue-112047.stderr @@ -14,7 +14,6 @@ LL | async fn recur(t: impl Recur) { LL | t.recur().await; | --------------- ...leading to this recursive call = note: a recursive `async fn` call must introduce indirection such as `Box::pin` to avoid an infinitely sized future - = note: consider using the `async_recursion` crate: https://crates.io/crates/async_recursion error: aborting due to 1 previous error From 9a756034a981b0aa15598f990381203dffe96a1c Mon Sep 17 00:00:00 2001 From: Michael Goulet Date: Mon, 8 Jan 2024 14:31:25 +0000 Subject: [PATCH 7/7] Last nits --- .../src/error_codes/E0733.md | 38 +++++------------- compiler/rustc_middle/src/values.rs | 40 ++++++++++++++----- 2 files changed, 41 insertions(+), 37 deletions(-) diff --git a/compiler/rustc_error_codes/src/error_codes/E0733.md b/compiler/rustc_error_codes/src/error_codes/E0733.md index cceb0880350e..42c01975dd8d 100644 --- a/compiler/rustc_error_codes/src/error_codes/E0733.md +++ b/compiler/rustc_error_codes/src/error_codes/E0733.md @@ -10,48 +10,30 @@ async fn foo(n: usize) { } ``` -To perform async recursion, the `async fn` needs to be desugared such that the -`Future` is explicit in the return type: - -```edition2018,compile_fail,E0733 -use std::future::Future; -fn foo_desugared(n: usize) -> impl Future { - async move { - if n > 0 { - foo_desugared(n - 1).await; - } - } -} -``` - -Finally, the future is wrapped in a pinned box: +The recursive invocation can be boxed: ```edition2018 -use std::future::Future; -use std::pin::Pin; -fn foo_recursive(n: usize) -> Pin>> { - Box::pin(async move { - if n > 0 { - foo_recursive(n - 1).await; - } - }) +async fn foo(n: usize) { + if n > 0 { + Box::pin(foo(n - 1)).await; + } } ``` The `Box<...>` ensures that the result is of known size, and the pin is required to keep it in the same place in memory. -Alternatively, the recursive call-site can be boxed: +Alternatively, the body can be boxed: ```edition2018 use std::future::Future; use std::pin::Pin; -fn foo_recursive(n: usize) -> impl Future { - async move { +fn foo(n: usize) -> Pin>> { + Box::pin(async move { if n > 0 { - Box::pin(foo_recursive(n - 1)).await; + foo(n - 1).await; } - } + }) } ``` diff --git a/compiler/rustc_middle/src/values.rs b/compiler/rustc_middle/src/values.rs index 77e4ed06eaa2..27d04dbe3314 100644 --- a/compiler/rustc_middle/src/values.rs +++ b/compiler/rustc_middle/src/values.rs @@ -13,6 +13,7 @@ use rustc_span::{ErrorGuaranteed, Span}; use std::collections::VecDeque; use std::fmt::Write; +use std::ops::ControlFlow; impl<'tcx> Value> for Ty<'_> { fn from_cycle_error(tcx: TyCtxt<'tcx>, _: &CycleError, guar: ErrorGuaranteed) -> Self { @@ -130,16 +131,34 @@ impl<'tcx> Value> for ty::EarlyBinder> } } +// Take a cycle of `Q` and try `try_cycle` on every permutation, falling back to `otherwise`. +fn search_for_cycle_permutation( + cycle: &[Q], + try_cycle: impl Fn(&mut VecDeque<&Q>) -> ControlFlow, + otherwise: impl FnOnce() -> T, +) -> T { + let mut cycle: VecDeque<_> = cycle.iter().collect(); + for _ in 0..cycle.len() { + match try_cycle(&mut cycle) { + ControlFlow::Continue(_) => { + cycle.rotate_left(1); + } + ControlFlow::Break(t) => return t, + } + } + + otherwise() +} + impl<'tcx, T> Value> for Result> { fn from_cycle_error( tcx: TyCtxt<'tcx>, cycle_error: &CycleError, _guar: ErrorGuaranteed, ) -> Self { - let mut cycle: VecDeque<_> = cycle_error.cycle.iter().collect(); - - let guar = 'search: { - for _ in 0..cycle.len() { + let diag = search_for_cycle_permutation( + &cycle_error.cycle, + |cycle| { if cycle[0].query.dep_kind == dep_kinds::layout_of && let Some(def_id) = cycle[0].query.ty_def_id && let Some(def_id) = def_id.as_local() @@ -204,13 +223,16 @@ impl<'tcx, T> Value> for Result> ) { diag.note("a recursive `async fn` call must introduce indirection such as `Box::pin` to avoid an infinitely sized future"); } - break 'search diag.emit(); + + ControlFlow::Break(diag) } else { - cycle.rotate_left(1); + ControlFlow::Continue(()) } - } - report_cycle(tcx.sess, cycle_error).emit() - }; + }, + || report_cycle(tcx.sess, cycle_error), + ); + + let guar = diag.emit(); // tcx.arena.alloc cannot be used because we are not allowed to use &'tcx LayoutError under // min_specialization. Since this is an error path anyways, leaking doesn't matter (and really,