Auto merge of rust-lang#88121 - camelid:better-recursive-alias-error, r=estebank

Improve errors for recursive type aliases

Fixes rust-lang#17539.

Author: bors

compiler/rustc_middle/src/query/mod.rs

@@ -120,7 +120,18 @@ rustc_queries! {
 
     /// Records the type of every item.
     query type_of(key: DefId) -> Ty<'tcx> {
-        desc { |tcx| "computing type of `{}`", tcx.def_path_str(key) }
+        desc { |tcx|
+            "{action} `{path}`",
+            action = {
+                use rustc_hir::def::DefKind;
+                match tcx.def_kind(key) {
+                    DefKind::TyAlias => "expanding type alias",
+                    DefKind::TraitAlias => "expanding trait alias",
+                    _ => "computing type of",
+                }
+            },
+            path = tcx.def_path_str(key),
+        }
         cache_on_disk_if { key.is_local() }
     }
 

compiler/rustc_query_impl/src/keys.rs

@@ -20,6 +20,12 @@ pub trait Key {
     /// In the event that a cycle occurs, if no explicit span has been
     /// given for a query with key `self`, what span should we use?
     fn default_span(&self, tcx: TyCtxt<'_>) -> Span;
+
+    /// If the key is a [`DefId`] or `DefId`--equivalent, return that `DefId`.
+    /// Otherwise, return `None`.
+    fn key_as_def_id(&self) -> Option<DefId> {
+        None
+    }
 }
 
 impl Key for () {
@@ -95,6 +101,9 @@ impl Key for LocalDefId {
     fn default_span(&self, tcx: TyCtxt<'_>) -> Span {
         self.to_def_id().default_span(tcx)
     }
+    fn key_as_def_id(&self) -> Option<DefId> {
+        Some(self.to_def_id())
+    }
 }
 
 impl Key for DefId {
@@ -105,6 +114,10 @@ impl Key for DefId {
     fn default_span(&self, tcx: TyCtxt<'_>) -> Span {
         tcx.def_span(*self)
     }
+    #[inline(always)]
+    fn key_as_def_id(&self) -> Option<DefId> {
+        Some(*self)
+    }
 }
 
 impl Key for ty::WithOptConstParam<LocalDefId> {
@@ -165,6 +178,10 @@ impl Key for (DefId, Option<Ident>) {
     fn default_span(&self, tcx: TyCtxt<'_>) -> Span {
         tcx.def_span(self.0)
     }
+    #[inline(always)]
+    fn key_as_def_id(&self) -> Option<DefId> {
+        Some(self.0)
+    }
 }
 
 impl Key for (DefId, LocalDefId, Ident) {

compiler/rustc_query_impl/src/lib.rs

@@ -51,6 +51,8 @@ pub use on_disk_cache::OnDiskCache;
 mod profiling_support;
 pub use self::profiling_support::alloc_self_profile_query_strings;
 
+mod util;
+
 rustc_query_append! { [define_queries!][<'tcx>] }
 
 impl<'tcx> Queries<'tcx> {

compiler/rustc_query_impl/src/plumbing.rs

@@ -337,6 +337,13 @@ macro_rules! define_queries {
                 } else {
                     Some(key.default_span(*tcx))
                 };
+                let def_id = key.key_as_def_id();
+                let def_kind = def_id
+                    .and_then(|def_id| def_id.as_local())
+                    // Use `tcx.hir().opt_def_kind()` to reduce the chance of
+                    // accidentally triggering an infinite query loop.
+                    .and_then(|def_id| tcx.hir().opt_def_kind(def_id))
+                    .map(|def_kind| $crate::util::def_kind_to_simple_def_kind(def_kind));
                 let hash = || {
                     let mut hcx = tcx.create_stable_hashing_context();
                     let mut hasher = StableHasher::new();
@@ -345,7 +352,7 @@ macro_rules! define_queries {
                     hasher.finish::<u64>()
                 };
 
-                QueryStackFrame::new(name, description, span, hash)
+                QueryStackFrame::new(name, description, span, def_kind, hash)
             })*
         }
 

compiler/rustc_query_impl/src/util.rs

@@ -0,0 +1,18 @@
+use rustc_hir::def::DefKind;
+use rustc_query_system::query::SimpleDefKind;
+
+/// Convert a [`DefKind`] to a [`SimpleDefKind`].
+///
+/// *See [`SimpleDefKind`]'s docs for more information.*
+pub(crate) fn def_kind_to_simple_def_kind(def_kind: DefKind) -> SimpleDefKind {
+    match def_kind {
+        DefKind::Struct => SimpleDefKind::Struct,
+        DefKind::Enum => SimpleDefKind::Enum,
+        DefKind::Union => SimpleDefKind::Union,
+        DefKind::Trait => SimpleDefKind::Trait,
+        DefKind::TyAlias => SimpleDefKind::TyAlias,
+        DefKind::TraitAlias => SimpleDefKind::TraitAlias,
+
+        _ => SimpleDefKind::Other,
+    }
+}

compiler/rustc_query_system/src/query/job.rs

@@ -1,6 +1,6 @@
 use crate::dep_graph::DepContext;
 use crate::query::plumbing::CycleError;
-use crate::query::{QueryContext, QueryStackFrame};
+use crate::query::{QueryContext, QueryStackFrame, SimpleDefKind};
 
 use rustc_data_structures::fx::FxHashMap;
 use rustc_errors::{struct_span_err, Diagnostic, DiagnosticBuilder, Handler, Level};
@@ -591,10 +591,33 @@ pub(crate) fn report_cycle<'a>(
         err.span_note(span, &format!("...which requires {}...", query.description));
     }
 
-    err.note(&format!(
-        "...which again requires {}, completing the cycle",
-        stack[0].query.description
-    ));
+    if stack.len() == 1 {
+        err.note(&format!("...which immediately requires {} again", stack[0].query.description));
+    } else {
+        err.note(&format!(
+            "...which again requires {}, completing the cycle",
+            stack[0].query.description
+        ));
+    }
+
+    if stack.iter().all(|entry| {
+        entry.query.def_kind.map_or(false, |def_kind| {
+            matches!(def_kind, SimpleDefKind::TyAlias | SimpleDefKind::TraitAlias)
+        })
+    }) {
+        if stack.iter().all(|entry| {
+            entry
+                .query
+                .def_kind
+                .map_or(false, |def_kind| matches!(def_kind, SimpleDefKind::TyAlias))
+        }) {
+            err.note("type aliases cannot be recursive");
+            err.help("consider using a struct, enum, or union instead to break the cycle");
+            err.help("see <https://doc.rust-lang.org/reference/types.html#recursive-types> for more information");
+        } else {
+            err.note("trait aliases cannot be recursive");
+        }
+    }
 
     if let Some((span, query)) = usage {
         err.span_note(fix_span(span, &query), &format!("cycle used when {}", query.description));

compiler/rustc_query_system/src/query/mod.rs

@@ -29,24 +29,53 @@ pub struct QueryStackFrame {
     pub name: &'static str,
     pub description: String,
     span: Option<Span>,
+    /// The `DefKind` this query frame is associated with, if applicable.
+    ///
+    /// We can't use `rustc_hir::def::DefKind` because `rustc_hir` is not
+    /// available in `rustc_query_system`. Instead, we have a simplified
+    /// custom version of it, called [`SimpleDefKind`].
+    def_kind: Option<SimpleDefKind>,
     /// This hash is used to deterministically pick
     /// a query to remove cycles in the parallel compiler.
     #[cfg(parallel_compiler)]
     hash: u64,
 }
 
+/// A simplified version of `rustc_hir::def::DefKind`.
+///
+/// It was added to help improve cycle errors caused by recursive type aliases.
+/// As of August 2021, `rustc_query_system` cannot depend on `rustc_hir`
+/// because it would create a dependency cycle. So, instead, a simplified
+/// version of `DefKind` was added to `rustc_query_system`.
+///
+/// `DefKind`s are converted to `SimpleDefKind`s in `rustc_query_impl`.
+#[derive(Debug, Copy, Clone)]
+pub enum SimpleDefKind {
+    Struct,
+    Enum,
+    Union,
+    Trait,
+    TyAlias,
+    TraitAlias,
+
+    // FIXME: add more from `rustc_hir::def::DefKind` and then remove `Other`
+    Other,
+}
+
 impl QueryStackFrame {
     #[inline]
     pub fn new(
         name: &'static str,
         description: String,
         span: Option<Span>,
+        def_kind: Option<SimpleDefKind>,
         _hash: impl FnOnce() -> u64,
     ) -> Self {
         Self {
             name,
             description,
             span,
+            def_kind,
             #[cfg(parallel_compiler)]
             hash: _hash(),
         }

src/test/ui/associated-type-bounds/ambiguous-associated-type2.stderr

@@ -4,7 +4,7 @@ error[E0391]: cycle detected when computing the super traits of `Baz` with assoc
 LL | trait Baz: Foo + Bar<Self::Item> {}
    | ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^
    |
-   = note: ...which again requires computing the super traits of `Baz` with associated type name `Item`, completing the cycle
+   = note: ...which immediately requires computing the super traits of `Baz` with associated type name `Item` again
 note: cycle used when computing the super traits of `Baz`
   --> $DIR/ambiguous-associated-type2.rs:7:1
    |

src/test/ui/coherence/coherence-inherited-assoc-ty-cycle-err.stderr

@@ -14,7 +14,7 @@ error[E0391]: cycle detected when building specialization graph of trait `Trait`
 LL | trait Trait<T> { type Assoc; }
    | ^^^^^^^^^^^^^^
    |
-   = note: ...which again requires building specialization graph of trait `Trait`, completing the cycle
+   = note: ...which immediately requires building specialization graph of trait `Trait` again
 note: cycle used when coherence checking all impls of trait `Trait`
   --> $DIR/coherence-inherited-assoc-ty-cycle-err.rs:9:1
    |

src/test/ui/cycle-trait/cycle-trait-default-type-trait.stderr

@@ -4,7 +4,7 @@ error[E0391]: cycle detected when computing type of `Foo::X`
 LL | trait Foo<X = Box<dyn Foo>> {
    |                       ^^^
    |
-   = note: ...which again requires computing type of `Foo::X`, completing the cycle
+   = note: ...which immediately requires computing type of `Foo::X` again
 note: cycle used when collecting item types in top-level module
   --> $DIR/cycle-trait-default-type-trait.rs:4:1
    |
@@ -17,7 +17,7 @@ error[E0391]: cycle detected when computing type of `Foo::X`
 LL | trait Foo<X = Box<dyn Foo>> {
    |                       ^^^
    |
-   = note: ...which again requires computing type of `Foo::X`, completing the cycle
+   = note: ...which immediately requires computing type of `Foo::X` again
 note: cycle used when collecting item types in top-level module
   --> $DIR/cycle-trait-default-type-trait.rs:4:1
    |