Rollup merge of #153317 - nnethercote:abort-after-infinite-errors, r=oli-obk

Abort after `representability` errors

Doing so results in better error messages and makes the code a bit simpler. Details in individual commits.

r? @oli-obk

Author: JonathanBrouwer

compiler/rustc_hir_analysis/src/check/wfcheck.rs

@@ -997,7 +997,7 @@ fn check_type_defn<'tcx>(
     item: &hir::Item<'tcx>,
     all_sized: bool,
 ) -> Result<(), ErrorGuaranteed> {
-    let _ = tcx.representability(item.owner_id.def_id);
+    let _ = tcx.check_representability(item.owner_id.def_id);
     let adt_def = tcx.adt_def(item.owner_id);
 
     enter_wf_checking_ctxt(tcx, item.owner_id.def_id, |wfcx| {

compiler/rustc_middle/src/queries.rs

@@ -593,18 +593,23 @@ rustc_queries! {
     }
 
     /// Checks whether a type is representable or infinitely sized
-    query representability(key: LocalDefId) -> rustc_middle::ty::Representability {
+    query check_representability(key: LocalDefId) -> rustc_middle::ty::Representability {
         desc { "checking if `{}` is representable", tcx.def_path_str(key) }
-        // infinitely sized types will cause a cycle
+        // Infinitely sized types will cause a cycle. The custom `FromCycleError` impl for
+        // `Representability` will print a custom error about the infinite size and then abort
+        // compilation. (In the past we recovered and continued, but in practice that leads to
+        // confusing subsequent error messages about cycles that then abort.)
         cycle_delay_bug
-        // we don't want recursive representability calls to be forced with
+        // We don't want recursive representability calls to be forced with
         // incremental compilation because, if a cycle occurs, we need the
-        // entire cycle to be in memory for diagnostics
+        // entire cycle to be in memory for diagnostics. This means we can't
+        // use `ensure_ok()` with this query.
         anon
     }
 
-    /// An implementation detail for the `representability` query
-    query representability_adt_ty(key: Ty<'tcx>) -> rustc_middle::ty::Representability {
+    /// An implementation detail for the `check_representability` query. See that query for more
+    /// details, particularly on the modifiers.
+    query check_representability_adt_ty(key: Ty<'tcx>) -> rustc_middle::ty::Representability {
         desc { "checking if `{}` is representable", key }
         cycle_delay_bug
         anon

compiler/rustc_middle/src/ty/adt.rs

@@ -741,8 +741,7 @@ impl<'tcx> AdtDef<'tcx> {
     }
 }
 
+/// This type exists just so a `FromCycleError` impl can be made for the `check_representability`
+/// query.
 #[derive(Clone, Copy, Debug, HashStable)]
-pub enum Representability {
-    Representable,
-    Infinite(ErrorGuaranteed),
-}
+pub struct Representability;

compiler/rustc_middle/src/ty/inhabitedness/mod.rs

@@ -61,10 +61,9 @@ pub(crate) fn provide(providers: &mut Providers) {
 /// requires calling [`InhabitedPredicate::instantiate`]
 fn inhabited_predicate_adt(tcx: TyCtxt<'_>, def_id: DefId) -> InhabitedPredicate<'_> {
     if let Some(def_id) = def_id.as_local() {
-        if matches!(tcx.representability(def_id), ty::Representability::Infinite(_)) {
-            return InhabitedPredicate::True;
-        }
+        let _ = tcx.check_representability(def_id);
     }
+
     let adt = tcx.adt_def(def_id);
     InhabitedPredicate::any(
         tcx,

compiler/rustc_query_impl/src/from_cycle_error.rs

@@ -95,7 +95,7 @@ impl<'tcx> FromCycleError<'tcx> for Representability {
         let mut item_and_field_ids = Vec::new();
         let mut representable_ids = FxHashSet::default();
         for info in &cycle_error.cycle {
-            if info.frame.dep_kind == DepKind::representability
+            if info.frame.dep_kind == DepKind::check_representability
                 && let Some(field_id) = info.frame.def_id
                 && let Some(field_id) = field_id.as_local()
                 && let Some(DefKind::Field) = info.frame.info.def_kind
@@ -109,16 +109,18 @@ impl<'tcx> FromCycleError<'tcx> for Representability {
             }
         }
         for info in &cycle_error.cycle {
-            if info.frame.dep_kind == DepKind::representability_adt_ty
+            if info.frame.dep_kind == DepKind::check_representability_adt_ty
                 && let Some(def_id) = info.frame.def_id_for_ty_in_cycle
                 && let Some(def_id) = def_id.as_local()
                 && !item_and_field_ids.iter().any(|&(id, _)| id == def_id)
             {
                 representable_ids.insert(def_id);
             }
         }
+        // We used to continue here, but the cycle error printed next is actually less useful than
+        // the error produced by `recursive_type_error`.
         let guar = recursive_type_error(tcx, item_and_field_ids, &representable_ids);
-        Representability::Infinite(guar)
+        guar.raise_fatal();
     }
 }
 

compiler/rustc_ty_utils/src/representability.rs

@@ -6,81 +6,83 @@ use rustc_middle::ty::{self, Representability, Ty, TyCtxt};
 use rustc_span::def_id::LocalDefId;
 
 pub(crate) fn provide(providers: &mut Providers) {
-    *providers =
-        Providers { representability, representability_adt_ty, params_in_repr, ..*providers };
-}
-
-macro_rules! rtry {
-    ($e:expr) => {
-        match $e {
-            e @ Representability::Infinite(_) => return e,
-            Representability::Representable => {}
-        }
+    *providers = Providers {
+        check_representability,
+        check_representability_adt_ty,
+        params_in_repr,
+        ..*providers
     };
 }
 
-fn representability(tcx: TyCtxt<'_>, def_id: LocalDefId) -> Representability {
+fn check_representability(tcx: TyCtxt<'_>, def_id: LocalDefId) -> Representability {
     match tcx.def_kind(def_id) {
         DefKind::Struct | DefKind::Union | DefKind::Enum => {
             for variant in tcx.adt_def(def_id).variants() {
                 for field in variant.fields.iter() {
-                    rtry!(tcx.representability(field.did.expect_local()));
+                    let _ = tcx.check_representability(field.did.expect_local());
                 }
             }
-            Representability::Representable
         }
-        DefKind::Field => representability_ty(tcx, tcx.type_of(def_id).instantiate_identity()),
+        DefKind::Field => {
+            check_representability_ty(tcx, tcx.type_of(def_id).instantiate_identity());
+        }
         def_kind => bug!("unexpected {def_kind:?}"),
     }
+    Representability
 }
 
-fn representability_ty<'tcx>(tcx: TyCtxt<'tcx>, ty: Ty<'tcx>) -> Representability {
+fn check_representability_ty<'tcx>(tcx: TyCtxt<'tcx>, ty: Ty<'tcx>) {
     match *ty.kind() {
-        ty::Adt(..) => tcx.representability_adt_ty(ty),
+        // This one must be a query rather than a vanilla `check_representability_adt_ty` call. See
+        // the comment on `check_representability_adt_ty` below for why.
+        ty::Adt(..) => {
+            let _ = tcx.check_representability_adt_ty(ty);
+        }
         // FIXME(#11924) allow zero-length arrays?
-        ty::Array(ty, _) => representability_ty(tcx, ty),
+        ty::Array(ty, _) => {
+            check_representability_ty(tcx, ty);
+        }
         ty::Tuple(tys) => {
             for ty in tys {
-                rtry!(representability_ty(tcx, ty));
+                check_representability_ty(tcx, ty);
             }
-            Representability::Representable
         }
-        _ => Representability::Representable,
+        _ => {}
     }
 }
 
-/*
-The reason for this being a separate query is very subtle:
-Consider this infinitely sized struct: `struct Foo(Box<Foo>, Bar<Foo>)`:
-When calling representability(Foo), a query cycle will occur:
-  representability(Foo)
-    -> representability_adt_ty(Bar<Foo>)
-    -> representability(Foo)
-For the diagnostic output (in `Value::from_cycle_error`), we want to detect that
-the `Foo` in the *second* field of the struct is culpable. This requires
-traversing the HIR of the struct and calling `params_in_repr(Bar)`. But we can't
-call params_in_repr for a given type unless it is known to be representable.
-params_in_repr will cycle/panic on infinitely sized types. Looking at the query
-cycle above, we know that `Bar` is representable because
-representability_adt_ty(Bar<..>) is in the cycle and representability(Bar) is
-*not* in the cycle.
-*/
-fn representability_adt_ty<'tcx>(tcx: TyCtxt<'tcx>, ty: Ty<'tcx>) -> Representability {
+// The reason for this being a separate query is very subtle. Consider this
+// infinitely sized struct: `struct Foo(Box<Foo>, Bar<Foo>)`. When calling
+// check_representability(Foo), a query cycle will occur:
+//
+//   check_representability(Foo)
+//     -> check_representability_adt_ty(Bar<Foo>)
+//     -> check_representability(Foo)
+//
+// For the diagnostic output (in `Value::from_cycle_error`), we want to detect
+// that the `Foo` in the *second* field of the struct is culpable. This
+// requires traversing the HIR of the struct and calling `params_in_repr(Bar)`.
+// But we can't call params_in_repr for a given type unless it is known to be
+// representable. params_in_repr will cycle/panic on infinitely sized types.
+// Looking at the query cycle above, we know that `Bar` is representable
+// because `check_representability_adt_ty(Bar<..>)` is in the cycle and
+// `check_representability(Bar)` is *not* in the cycle.
+fn check_representability_adt_ty<'tcx>(tcx: TyCtxt<'tcx>, ty: Ty<'tcx>) -> Representability {
     let ty::Adt(adt, args) = ty.kind() else { bug!("expected adt") };
     if let Some(def_id) = adt.did().as_local() {
-        rtry!(tcx.representability(def_id));
+        let _ = tcx.check_representability(def_id);
     }
     // At this point, we know that the item of the ADT type is representable;
     // but the type parameters may cause a cycle with an upstream type
     let params_in_repr = tcx.params_in_repr(adt.did());
     for (i, arg) in args.iter().enumerate() {
         if let ty::GenericArgKind::Type(ty) = arg.kind() {
             if params_in_repr.contains(i as u32) {
-                rtry!(representability_ty(tcx, ty));
+                check_representability_ty(tcx, ty);
             }
         }
     }
-    Representability::Representable
+    Representability
 }
 
 fn params_in_repr(tcx: TyCtxt<'_>, def_id: LocalDefId) -> DenseBitSet<u32> {

compiler/rustc_ty_utils/src/ty.rs

@@ -116,11 +116,10 @@ fn adt_sizedness_constraint<'tcx>(
     tcx: TyCtxt<'tcx>,
     (def_id, sizedness): (DefId, SizedTraitKind),
 ) -> Option<ty::EarlyBinder<'tcx, Ty<'tcx>>> {
-    if let Some(def_id) = def_id.as_local()
-        && let ty::Representability::Infinite(_) = tcx.representability(def_id)
-    {
-        return None;
+    if let Some(def_id) = def_id.as_local() {
+        let _ = tcx.check_representability(def_id);
     }
+
     let def = tcx.adt_def(def_id);
 
     if !def.is_struct() {

tests/ui/enum-discriminant/issue-72554.rs

@@ -3,7 +3,6 @@ use std::collections::BTreeSet;
 #[derive(Hash)]
 pub enum ElemDerived {
     //~^ ERROR recursive type `ElemDerived` has infinite size
-    //~| ERROR cycle detected
     A(ElemDerived)
 }
 

tests/ui/enum-discriminant/issue-72554.stderr

@@ -3,7 +3,7 @@ error[E0072]: recursive type `ElemDerived` has infinite size
    |
 LL | pub enum ElemDerived {
    | ^^^^^^^^^^^^^^^^^^^^
-...
+LL |
 LL |     A(ElemDerived)
    |       ----------- recursive without indirection
    |
@@ -12,21 +12,6 @@ help: insert some indirection (e.g., a `Box`, `Rc`, or `&`) to break the cycle
 LL |     A(Box<ElemDerived>)
    |       ++++           +
 
-error[E0391]: cycle detected when computing drop-check constraints for `ElemDerived`
-  --> $DIR/issue-72554.rs:4:1
-   |
-LL | pub enum ElemDerived {
-   | ^^^^^^^^^^^^^^^^^^^^
-   |
-   = note: ...which immediately requires computing drop-check constraints for `ElemDerived` again
-note: cycle used when computing drop-check constraints for `Elem`
-  --> $DIR/issue-72554.rs:11:1
-   |
-LL | pub enum Elem {
-   | ^^^^^^^^^^^^^
-   = 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
-
-error: aborting due to 2 previous errors
+error: aborting due to 1 previous error
 
-Some errors have detailed explanations: E0072, E0391.
-For more information about an error, try `rustc --explain E0072`.
+For more information about this error, try `rustc --explain E0072`.

tests/ui/infinite/infinite-struct.rs

@@ -1,7 +1,5 @@
 struct Take(Take);
 //~^ ERROR has infinite size
-//~| ERROR cycle
-//~| ERROR reached the recursion limit finding the struct tail for `Take`
 
 // check that we don't hang trying to find the tail of a recursive struct (#79437)
 fn foo() -> Take {