Skip to content

Commit

Permalink
Rollup merge of rust-lang#136073 - compiler-errors:recursive-coro-alw…
Browse files Browse the repository at this point in the history
…ays, r=oli-obk

Always compute coroutine layout for eagerly emitting recursive layout errors

Detect recursive coroutine layouts even if we don't detect opaque type recursion in the new solver. This is for two reasons:
1. It helps us detect (bad) recursive async function calls in the new solver, which due to its approach to normalization causes us to not detect this via a recursive RPIT (since the opaques are more eagerly revealed in the opaque body).
    * Fixes rust-lang/trait-system-refactor-initiative#137.
2. It helps us detect (bad) recursive async functions behind AFITs. See the AFIT test that changed for the old solver too.
3. It also greatly simplifies the recursive impl trait check, since I can remove some jankness around how it handles coroutines.
  • Loading branch information
matthiaskrgr authored Feb 6, 2025
2 parents 5958825 + d0b0b02 commit 7ca2936
Show file tree
Hide file tree
Showing 12 changed files with 37 additions and 95 deletions.
29 changes: 4 additions & 25 deletions compiler/rustc_hir_analysis/src/check/check.rs
Original file line number Diff line number Diff line change
Expand Up @@ -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,
};
Expand Down Expand Up @@ -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(())
Expand Down
5 changes: 5 additions & 0 deletions compiler/rustc_interface/src/passes.rs
Original file line number Diff line number Diff line change
Expand Up @@ -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()),
);
}
});
});
Expand Down
58 changes: 2 additions & 56 deletions compiler/rustc_middle/src/ty/util.rs
Original file line number Diff line number Diff line change
Expand Up @@ -777,7 +777,6 @@ impl<'tcx> TyCtxt<'tcx> {
self,
def_id: DefId,
args: GenericArgsRef<'tcx>,
inspect_coroutine_fields: InspectCoroutineFields,
) -> Result<Ty<'tcx>, Ty<'tcx>> {
let mut visitor = OpaqueTypeExpander {
seen_opaque_tys: FxHashSet::default(),
Expand All @@ -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();
Expand Down Expand Up @@ -965,19 +962,11 @@ struct OpaqueTypeExpander<'tcx> {
primary_def_id: Option<DefId>,
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> {
Expand Down Expand Up @@ -1009,41 +998,6 @@ impl<'tcx> OpaqueTypeExpander<'tcx> {
None
}
}

fn expand_coroutine(&mut self, def_id: DefId, args: GenericArgsRef<'tcx>) -> Option<Ty<'tcx>> {
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<TyCtxt<'tcx>> for OpaqueTypeExpander<'tcx> {
Expand All @@ -1052,19 +1006,13 @@ impl<'tcx> TypeFolder<TyCtxt<'tcx>> 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> {
Expand Down Expand Up @@ -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)
}
Expand Down
Original file line number Diff line number Diff line change
@@ -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;
Expand Down
Original file line number Diff line number Diff line change
@@ -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) {
| ^^^^^^^^^^^^^^^^^^^^^
Expand Down
4 changes: 3 additions & 1 deletion tests/ui/force-inlining/deny-async.rs
Original file line number Diff line number Diff line change
@@ -1,6 +1,6 @@
//@ check-fail
//@ compile-flags: --crate-type=lib
//@ edition: 2021

#![allow(internal_features)]
#![feature(rustc_attrs)]

Expand All @@ -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
}
19 changes: 18 additions & 1 deletion tests/ui/force-inlining/deny-async.stderr
Original file line number Diff line number Diff line change
Expand Up @@ -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

Original file line number Diff line number Diff line change
@@ -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 || {
| ^^^^^^^
Expand Down
Original file line number Diff line number Diff line change
@@ -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 || {
| ^^^^^^^
Expand Down
3 changes: 0 additions & 3 deletions tests/ui/impl-trait/recursive-coroutine-indirect.rs
Original file line number Diff line number Diff line change
Expand Up @@ -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 {
Expand Down
Original file line number Diff line number Diff line change
@@ -1,5 +1,4 @@
//@ edition: 2021
//@ build-fail

#![feature(impl_trait_in_assoc_type)]

Expand Down
Original file line number Diff line number Diff line change
@@ -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) {
| ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^
Expand Down

0 comments on commit 7ca2936

Please sign in to comment.