From d0b0b028a68eaee609b235a8cb5627466b2d79fb Mon Sep 17 00:00:00 2001 From: Michael Goulet Date: Mon, 27 Jan 2025 19:15:37 +0000 Subject: [PATCH] Eagerly detect coroutine recursion pre-mono when possible --- .../rustc_hir_analysis/src/check/check.rs | 29 ++-------- compiler/rustc_interface/src/passes.rs | 5 ++ compiler/rustc_middle/src/ty/util.rs | 58 +------------------ .../indirect-recursion-issue-112047.rs | 3 - .../indirect-recursion-issue-112047.stderr | 2 +- tests/ui/force-inlining/deny-async.rs | 4 +- tests/ui/force-inlining/deny-async.stderr | 19 +++++- ...ecursive-coroutine-indirect.current.stderr | 2 +- .../recursive-coroutine-indirect.next.stderr | 2 +- .../recursive-coroutine-indirect.rs | 3 - .../indirect-recursion-issue-112047.rs | 1 - .../indirect-recursion-issue-112047.stderr | 4 +- 12 files changed, 37 insertions(+), 95 deletions(-) diff --git a/compiler/rustc_hir_analysis/src/check/check.rs b/compiler/rustc_hir_analysis/src/check/check.rs index 6c534d58a7da8..9ed56d7bde51e 100644 --- a/compiler/rustc_hir_analysis/src/check/check.rs +++ b/compiler/rustc_hir_analysis/src/check/check.rs @@ -19,7 +19,7 @@ use rustc_middle::span_bug; use rustc_middle::ty::error::TypeErrorToStringExt; use rustc_middle::ty::fold::{BottomUpFolder, fold_regions}; use rustc_middle::ty::layout::{LayoutError, MAX_SIMD_LANES}; -use rustc_middle::ty::util::{Discr, InspectCoroutineFields, IntTypeExt}; +use rustc_middle::ty::util::{Discr, IntTypeExt}; use rustc_middle::ty::{ AdtDef, GenericArgKind, RegionKind, TypeSuperVisitable, TypeVisitable, TypeVisitableExt, }; @@ -257,30 +257,9 @@ pub(super) fn check_opaque_for_cycles<'tcx>( // 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() - { - // 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); - return Err(reported); - } - - // And also look for cycle errors in the layout of coroutines. - if let Err(&LayoutError::Cycle(guar)) = - tcx.layout_of( - ty::TypingEnv::post_analysis(tcx, def_id.to_def_id()) - .as_query_input(Ty::new_opaque(tcx, def_id.to_def_id(), args)), - ) - { - return Err(guar); - } + if tcx.try_expand_impl_trait_type(def_id.to_def_id(), args).is_err() { + let reported = opaque_type_cycle_error(tcx, def_id); + return Err(reported); } Ok(()) diff --git a/compiler/rustc_interface/src/passes.rs b/compiler/rustc_interface/src/passes.rs index 8a121c5a8655e..9c5f30012e732 100644 --- a/compiler/rustc_interface/src/passes.rs +++ b/compiler/rustc_interface/src/passes.rs @@ -909,6 +909,11 @@ fn run_required_analyses(tcx: TyCtxt<'_>) { tcx.ensure_ok().check_coroutine_obligations( tcx.typeck_root_def_id(def_id.to_def_id()).expect_local(), ); + // Eagerly check the unsubstituted layout for cycles. + tcx.ensure_ok().layout_of( + ty::TypingEnv::post_analysis(tcx, def_id.to_def_id()) + .as_query_input(tcx.type_of(def_id).instantiate_identity()), + ); } }); }); diff --git a/compiler/rustc_middle/src/ty/util.rs b/compiler/rustc_middle/src/ty/util.rs index 318bd0c7ec020..6fd0fb9a0a4f0 100644 --- a/compiler/rustc_middle/src/ty/util.rs +++ b/compiler/rustc_middle/src/ty/util.rs @@ -777,7 +777,6 @@ 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(), @@ -786,9 +785,7 @@ impl<'tcx> TyCtxt<'tcx> { found_recursion: false, found_any_recursion: false, check_recursion: true, - expand_coroutines: true, tcx: self, - inspect_coroutine_fields, }; let expanded_type = visitor.expand_opaque_ty(def_id, args).unwrap(); @@ -965,19 +962,11 @@ struct OpaqueTypeExpander<'tcx> { primary_def_id: Option, found_recursion: bool, found_any_recursion: bool, - expand_coroutines: bool, /// Whether or not to check for recursive opaque types. /// This is `true` when we're explicitly checking for opaque type /// 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> { @@ -1009,41 +998,6 @@ impl<'tcx> OpaqueTypeExpander<'tcx> { None } } - - fn expand_coroutine(&mut self, def_id: DefId, args: GenericArgsRef<'tcx>) -> Option> { - if self.found_any_recursion { - return None; - } - 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 => { - if matches!(self.inspect_coroutine_fields, InspectCoroutineFields::Yes) { - for bty in self.tcx.bound_coroutine_hidden_types(def_id) { - let hidden_ty = self.tcx.instantiate_bound_regions_with_erased( - 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); - } - Some(expanded_ty) - } else { - // If another opaque type that we contain is recursive, then it - // will report the error, so we don't have to. - self.found_any_recursion = true; - self.found_recursion = def_id == *self.primary_def_id.as_ref().unwrap(); - None - } - } } impl<'tcx> TypeFolder> for OpaqueTypeExpander<'tcx> { @@ -1052,19 +1006,13 @@ impl<'tcx> TypeFolder> for OpaqueTypeExpander<'tcx> { } fn fold_ty(&mut self, t: Ty<'tcx>) -> Ty<'tcx> { - let mut t = if let ty::Alias(ty::Opaque, ty::AliasTy { def_id, args, .. }) = *t.kind() { + if let ty::Alias(ty::Opaque, ty::AliasTy { def_id, args, .. }) = *t.kind() { self.expand_opaque_ty(def_id, args).unwrap_or(t) - } else if t.has_opaque_types() || t.has_coroutines() { + } else if t.has_opaque_types() { t.super_fold_with(self) } else { t - }; - if self.expand_coroutines { - if let ty::CoroutineWitness(def_id, args) = *t.kind() { - t = self.expand_coroutine(def_id, args).unwrap_or(t); - } } - t } fn fold_predicate(&mut self, p: ty::Predicate<'tcx>) -> ty::Predicate<'tcx> { @@ -1753,9 +1701,7 @@ pub fn reveal_opaque_types_in_bounds<'tcx>( found_recursion: false, found_any_recursion: false, check_recursion: false, - expand_coroutines: false, tcx, - inspect_coroutine_fields: InspectCoroutineFields::No, }; val.fold_with(&mut visitor) } 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 aa8667a00ca30..b25d8a34aca1c 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,8 +1,5 @@ //@ edition: 2021 -// Test doesn't fail until monomorphization time, unfortunately. -//@ build-fail - fn main() { let _ = async { A.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 8126c6e13942e..4ca6ef8981984 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,5 +1,5 @@ error[E0733]: recursion in an async fn requires boxing - --> $DIR/indirect-recursion-issue-112047.rs:34:5 + --> $DIR/indirect-recursion-issue-112047.rs:31:5 | LL | async fn second(self) { | ^^^^^^^^^^^^^^^^^^^^^ diff --git a/tests/ui/force-inlining/deny-async.rs b/tests/ui/force-inlining/deny-async.rs index bdf6271bd2a11..7c95a53c26fde 100644 --- a/tests/ui/force-inlining/deny-async.rs +++ b/tests/ui/force-inlining/deny-async.rs @@ -1,6 +1,6 @@ -//@ check-fail //@ compile-flags: --crate-type=lib //@ edition: 2021 + #![allow(internal_features)] #![feature(rustc_attrs)] @@ -20,5 +20,7 @@ pub fn callee_justified() { async fn async_caller() { callee(); + //~^ ERROR `callee` could not be inlined callee_justified(); + //~^ ERROR `callee_justified` could not be inlined } diff --git a/tests/ui/force-inlining/deny-async.stderr b/tests/ui/force-inlining/deny-async.stderr index 302ca4190710b..d6516ed875c21 100644 --- a/tests/ui/force-inlining/deny-async.stderr +++ b/tests/ui/force-inlining/deny-async.stderr @@ -20,5 +20,22 @@ LL | pub fn callee_justified() { | = note: incompatible due to: #[rustc_no_mir_inline] -error: aborting due to 2 previous errors +error: `callee` could not be inlined into `async_caller::{closure#0}` but is required to be inlined + --> $DIR/deny-async.rs:22:5 + | +LL | callee(); + | ^^^^^^^^ ...`callee` called here + | + = note: could not be inlined due to: #[rustc_no_mir_inline] + +error: `callee_justified` could not be inlined into `async_caller::{closure#0}` but is required to be inlined + --> $DIR/deny-async.rs:24:5 + | +LL | callee_justified(); + | ^^^^^^^^^^^^^^^^^^ ...`callee_justified` called here + | + = note: could not be inlined due to: #[rustc_no_mir_inline] + = note: `callee_justified` is required to be inlined to: the test requires it + +error: aborting due to 4 previous errors diff --git a/tests/ui/impl-trait/recursive-coroutine-indirect.current.stderr b/tests/ui/impl-trait/recursive-coroutine-indirect.current.stderr index 9814187e179ef..7f3691123ee87 100644 --- a/tests/ui/impl-trait/recursive-coroutine-indirect.current.stderr +++ b/tests/ui/impl-trait/recursive-coroutine-indirect.current.stderr @@ -1,5 +1,5 @@ error[E0733]: recursion in a coroutine requires boxing - --> $DIR/recursive-coroutine-indirect.rs:11:18 + --> $DIR/recursive-coroutine-indirect.rs:8:18 | LL | #[coroutine] move || { | ^^^^^^^ diff --git a/tests/ui/impl-trait/recursive-coroutine-indirect.next.stderr b/tests/ui/impl-trait/recursive-coroutine-indirect.next.stderr index 9814187e179ef..7f3691123ee87 100644 --- a/tests/ui/impl-trait/recursive-coroutine-indirect.next.stderr +++ b/tests/ui/impl-trait/recursive-coroutine-indirect.next.stderr @@ -1,5 +1,5 @@ error[E0733]: recursion in a coroutine requires boxing - --> $DIR/recursive-coroutine-indirect.rs:11:18 + --> $DIR/recursive-coroutine-indirect.rs:8:18 | LL | #[coroutine] move || { | ^^^^^^^ diff --git a/tests/ui/impl-trait/recursive-coroutine-indirect.rs b/tests/ui/impl-trait/recursive-coroutine-indirect.rs index cec2176049b87..f0727ad374270 100644 --- a/tests/ui/impl-trait/recursive-coroutine-indirect.rs +++ b/tests/ui/impl-trait/recursive-coroutine-indirect.rs @@ -2,9 +2,6 @@ //@ ignore-compare-mode-next-solver (explicit revisions) //@[next] compile-flags: -Znext-solver -//@[next] build-fail -// Deeply normalizing writeback results of opaques makes this into a post-mono error :( - #![feature(coroutines)] #![allow(unconditional_recursion)] fn coroutine_hold() -> impl Sized { 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 1aa64810d1908..38dd6dd0af209 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)] 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 6cbffaaed4d35..304ed12e944c6 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,11 +1,11 @@ error[E0733]: recursion in an async block requires boxing - --> $DIR/indirect-recursion-issue-112047.rs:22:9 + --> $DIR/indirect-recursion-issue-112047.rs:21:9 | LL | async move { recur(self).await; } | ^^^^^^^^^^ ----------------- recursive call here | note: which leads to this async fn - --> $DIR/indirect-recursion-issue-112047.rs:14:1 + --> $DIR/indirect-recursion-issue-112047.rs:13:1 | LL | async fn recur(t: impl Recur) { | ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^