Add more robust handling of nested query cycles

compiler/rustc_middle/src/queries.rs

@@ -60,7 +60,7 @@ use rustc_data_structures::sorted_map::SortedMap;
 use rustc_data_structures::steal::Steal;
 use rustc_data_structures::svh::Svh;
 use rustc_data_structures::unord::{UnordMap, UnordSet};
-use rustc_errors::ErrorGuaranteed;
+use rustc_errors::{ErrorGuaranteed, catch_fatal_errors};
 use rustc_hir as hir;
 use rustc_hir::attrs::{EiiDecl, EiiImpl, StrippedCfgItem};
 use rustc_hir::def::{DefKind, DocLinkResMap};

compiler/rustc_middle/src/query/plumbing.rs

@@ -166,6 +166,8 @@ pub struct QuerySystem<'tcx> {
     pub extern_providers: ExternProviders,
 
     pub jobs: AtomicU64,
+
+    pub cycle_handler_nesting: Lock<u8>,
 }
 
 #[derive(Copy, Clone)]
@@ -431,6 +433,11 @@ macro_rules! define_callbacks {
                 }
             }
 
+            /// Calls `self.description` or returns a fallback if there was a fatal error
+            pub fn catch_description(&self, tcx: TyCtxt<'tcx>) -> String {
+                catch_fatal_errors(|| self.description(tcx)).unwrap_or_else(|_| format!("<error describing {}>", self.query_name()))
+            }
+
             /// Returns the default span for this query if `span` is a dummy span.
             pub fn default_span(&self, tcx: TyCtxt<'tcx>, span: Span) -> Span {
                 if !span.is_dummy() {
@@ -449,28 +456,9 @@ macro_rules! define_callbacks {
                 }
             }
 
-            pub fn def_kind(&self, tcx: TyCtxt<'tcx>) -> Option<DefKind> {
-                // This is used to reduce code generation as it
-                // can be reused for queries with the same key type.
-                fn inner<'tcx>(key: &impl $crate::query::QueryKey, tcx: TyCtxt<'tcx>)
-                    -> Option<DefKind>
-                {
-                    key
-                        .key_as_def_id()
-                        .and_then(|def_id| def_id.as_local())
-                        .map(|def_id| tcx.def_kind(def_id))
-                }
-
-                if let TaggedQueryKey::def_kind(..) = self {
-                    // Try to avoid infinite recursion.
-                    return None
-                }
-
-                match self {
-                    $(
-                        TaggedQueryKey::$name(key) => inner(key, tcx),
-                    )*
-                }
+            /// Calls `self.default_span` or returns `DUMMY_SP` if there was a fatal error
+            pub fn catch_default_span(&self, tcx: TyCtxt<'tcx>, span: Span) -> Span {
+                catch_fatal_errors(|| self.default_span(tcx, span)).unwrap_or(DUMMY_SP)
             }
         }
 

compiler/rustc_query_impl/src/error.rs

@@ -79,3 +79,21 @@ pub(crate) struct Cycle {
     )]
     pub note_span: (),
 }
+
+#[derive(Diagnostic)]
+#[diag("cycle when printing cycle detected when {$stack_bottom}")]
+pub(crate) struct NestedCycle {
+    #[primary_span]
+    pub span: Span,
+    pub stack_bottom: String,
+    #[subdiagnostic]
+    pub cycle_stack: Vec<CycleStack>,
+    #[subdiagnostic]
+    pub stack_count: StackCount,
+    #[subdiagnostic]
+    pub cycle_usage: Option<CycleUsage>,
+    #[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"
+    )]
+    pub note_span: (),
+}

compiler/rustc_query_impl/src/execution.rs

@@ -4,7 +4,7 @@ use std::mem::ManuallyDrop;
 use rustc_data_structures::hash_table::{Entry, HashTable};
 use rustc_data_structures::stack::ensure_sufficient_stack;
 use rustc_data_structures::sync::{DynSend, DynSync};
-use rustc_data_structures::{outline, sharded, sync};
+use rustc_data_structures::{defer, outline, sharded, sync};
 use rustc_errors::FatalError;
 use rustc_middle::dep_graph::{DepGraphData, DepNodeKey, SerializedDepNodeIndex};
 use rustc_middle::query::{
@@ -17,6 +17,7 @@ use rustc_span::{DUMMY_SP, Span};
 use tracing::warn;
 
 use crate::dep_graph::{DepNode, DepNodeIndex};
+use crate::handle_cycle_error;
 use crate::job::{QueryJobInfo, QueryJobMap, create_cycle_error, find_cycle_in_stack};
 use crate::plumbing::{current_query_job, loadable_from_disk, next_job_id, start_query};
 use crate::query_impl::for_each_query_vtable;
@@ -114,8 +115,29 @@ fn handle_cycle<'tcx, C: QueryCache>(
     key: C::Key,
     cycle: Cycle<'tcx>,
 ) -> C::Value {
-    let error = create_cycle_error(tcx, &cycle);
-    (query.handle_cycle_error_fn)(tcx, key, cycle, error)
+    let nested;
+    {
+        let mut nesting = tcx.query_system.cycle_handler_nesting.lock();
+        nested = match *nesting {
+            0 => false,
+            1 => true,
+            _ => {
+                // Don't print further nested errors to avoid cases of infinite recursion
+                tcx.dcx().delayed_bug("doubly nested cycle error").raise_fatal()
+            }
+        };
+        *nesting += 1;
+    }
+    let _guard = defer(|| *tcx.query_system.cycle_handler_nesting.lock() -= 1);
+
+    let error = create_cycle_error(tcx, &cycle, nested);
+
+    if nested {
+        // Avoid custom handlers and only use the robust `create_cycle_error` for nested cycle errors
+        handle_cycle_error::default(error)
+    } else {
+        (query.handle_cycle_error_fn)(tcx, key, cycle, error)
+    }
 }
 
 /// Guard object representing the responsibility to execute a query job and

compiler/rustc_query_impl/src/handle_cycle_error.rs

@@ -200,7 +200,7 @@ fn layout_of<'tcx>(tcx: TyCtxt<'tcx>, cycle: Cycle<'tcx>) -> &'tcx ty::layout::L
                 ControlFlow::Continue(())
             }
         },
-        || create_cycle_error(tcx, &cycle),
+        || create_cycle_error(tcx, &cycle, false),
     );
 
     let guar = diag.emit();

compiler/rustc_query_impl/src/job.rs

@@ -454,15 +454,16 @@ pub fn print_query_stack<'tcx>(
 pub(crate) fn create_cycle_error<'tcx>(
     tcx: TyCtxt<'tcx>,
     Cycle { usage, frames }: &Cycle<'tcx>,
+    nested: bool,
 ) -> Diag<'tcx> {
     assert!(!frames.is_empty());
 
-    let span = frames[0].tagged_key.default_span(tcx, frames[1 % frames.len()].span);
+    let span = frames[0].tagged_key.catch_default_span(tcx, frames[1 % frames.len()].span);
 
     let mut cycle_stack = Vec::new();
 
     use crate::error::StackCount;
-    let stack_bottom = frames[0].tagged_key.description(tcx);
+    let stack_bottom = frames[0].tagged_key.catch_description(tcx);
     let stack_count = if frames.len() == 1 {
         StackCount::Single { stack_bottom: stack_bottom.clone() }
     } else {
@@ -471,37 +472,60 @@ pub(crate) fn create_cycle_error<'tcx>(
 
     for i in 1..frames.len() {
         let frame = &frames[i];
-        let span = frame.tagged_key.default_span(tcx, frames[(i + 1) % frames.len()].span);
+        let span = frame.tagged_key.catch_default_span(tcx, frames[(i + 1) % frames.len()].span);
         cycle_stack
-            .push(crate::error::CycleStack { span, desc: frame.tagged_key.description(tcx) });
+            .push(crate::error::CycleStack { span, desc: frame.tagged_key.catch_description(tcx) });
     }
 
     let cycle_usage = usage.as_ref().map(|usage| crate::error::CycleUsage {
-        span: usage.tagged_key.default_span(tcx, usage.span),
-        usage: usage.tagged_key.description(tcx),
+        span: usage.tagged_key.catch_default_span(tcx, usage.span),
+        usage: usage.tagged_key.catch_description(tcx),
     });
 
-    let alias = if frames
-        .iter()
-        .all(|frame| frame.tagged_key.def_kind(tcx) == Some(DefKind::TyAlias))
-    {
-        Some(crate::error::Alias::Ty)
-    } else if frames.iter().all(|frame| frame.tagged_key.def_kind(tcx) == Some(DefKind::TraitAlias))
-    {
-        Some(crate::error::Alias::Trait)
-    } else {
-        None
+    let is_all_def_kind = |def_kind| {
+        // Trivial type alias and trait alias cycles consists of `type_of` and
+        // `explicit_implied_predicates_of` queries, so we just check just these here.
+        frames.iter().all(|frame| match frame.tagged_key {
+            TaggedQueryKey::type_of(def_id)
+            | TaggedQueryKey::explicit_implied_predicates_of(def_id)
+                if tcx.def_kind(def_id) == def_kind =>
+            {
+                true
+            }
+            _ => false,
+        })
     };
 
-    let cycle_diag = crate::error::Cycle {
-        span,
-        cycle_stack,
-        stack_bottom,
-        alias,
-        cycle_usage,
-        stack_count,
-        note_span: (),
+    let alias = if !nested {
+        if is_all_def_kind(DefKind::TyAlias) {
+            Some(crate::error::Alias::Ty)
+        } else if is_all_def_kind(DefKind::TraitAlias) {
+            Some(crate::error::Alias::Trait)
+        } else {
+            None
+        }
+    } else {
+        None
     };
 
-    tcx.sess.dcx().create_err(cycle_diag)
+    if nested {
+        tcx.sess.dcx().create_err(crate::error::NestedCycle {
+            span,
+            cycle_stack,
+            stack_bottom,
+            cycle_usage,
+            stack_count,
+            note_span: (),
+        })
+    } else {
+        tcx.sess.dcx().create_err(crate::error::Cycle {
+            span,
+            cycle_stack,
+            stack_bottom,
+            alias,
+            cycle_usage,
+            stack_count,
+            note_span: (),
+        })
+    }
 }

compiler/rustc_query_impl/src/lib.rs

@@ -8,7 +8,7 @@
 #![feature(try_blocks)]
 // tidy-alphabetical-end
 
-use rustc_data_structures::sync::AtomicU64;
+use rustc_data_structures::sync::{AtomicU64, Lock};
 use rustc_middle::dep_graph;
 use rustc_middle::queries::{ExternProviders, Providers};
 use rustc_middle::query::on_disk_cache::OnDiskCache;
@@ -58,6 +58,7 @@ pub fn query_system<'tcx>(
         local_providers,
         extern_providers,
         jobs: AtomicU64::new(1),
+        cycle_handler_nesting: Lock::new(0),
     }
 }
 

tests/ui/query-system/query-cycle-printing-issue-151358.rs

@@ -1,4 +1,5 @@
-//~ ERROR: cycle detected when looking up span for `Default`
+//~ ERROR: cycle when printing cycle detected
+//~^ ERROR: cycle detected
 trait Default {}
 use std::num::NonZero;
 fn main() {

tests/ui/query-system/query-cycle-printing-issue-151358.stderr

@@ -1,9 +1,20 @@
-error[E0391]: cycle detected when looking up span for `Default`
+error: cycle when printing cycle detected when getting HIR ID of `Default`
    |
-   = note: ...which immediately requires looking up span for `Default` again
-   = note: cycle used when perform lints prior to AST lowering
+   = note: ...which requires getting the crate HIR...
+   = note: ...which requires perform lints prior to AST lowering...
+   = note: ...which requires looking up span for `Default`...
+   = note: ...which again requires getting HIR ID of `Default`, completing the cycle
+   = note: cycle used when getting the resolver for lowering
    = 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 1 previous error
+error[E0391]: cycle detected when getting the resolver for lowering
+   |
+   = note: ...which requires getting HIR ID of `Default`...
+   = note: ...which requires getting the crate HIR...
+   = note: ...which requires perform lints prior to AST lowering...
+   = note: ...which again requires getting the resolver for lowering, completing the cycle
+   = 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
 
 For more information about this error, try `rustc --explain E0391`.

tests/ui/resolve/query-cycle-issue-124901.rs

@@ -1,4 +1,5 @@
-//~ ERROR: cycle detected when looking up span for `Default`
+//~ ERROR: cycle when printing cycle detected
+//~^ ERROR: cycle detected
 trait Default {
     type Id;
 

tests/ui/resolve/query-cycle-issue-124901.stderr

@@ -1,9 +1,20 @@
-error[E0391]: cycle detected when looking up span for `Default`
+error: cycle when printing cycle detected when getting HIR ID of `Default`
    |
-   = note: ...which immediately requires looking up span for `Default` again
-   = note: cycle used when perform lints prior to AST lowering
+   = note: ...which requires getting the crate HIR...
+   = note: ...which requires perform lints prior to AST lowering...
+   = note: ...which requires looking up span for `Default`...
+   = note: ...which again requires getting HIR ID of `Default`, completing the cycle
+   = note: cycle used when getting the resolver for lowering
    = 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 1 previous error
+error[E0391]: cycle detected when getting the resolver for lowering
+   |
+   = note: ...which requires getting HIR ID of `Default`...
+   = note: ...which requires getting the crate HIR...
+   = note: ...which requires perform lints prior to AST lowering...
+   = note: ...which again requires getting the resolver for lowering, completing the cycle
+   = 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
 
 For more information about this error, try `rustc --explain E0391`.