Rollup merge of #154389 - Zoxc:nested-cycles, r=petrochenkov

Add more robust handling of nested query cycles

This adds more robust handling of query cycle that occur while we are already printing a query cycle error. Such nested query cycle are compiler bugs and this adds special handling so that both the nested query cycle and the outer query cycle are printed. Doubly nested query cycle errors are ignored to prevent infinite recursion.

Author: JonathanBrouwer

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)]
@@ -446,6 +448,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() {
@@ -463,6 +470,11 @@ macro_rules! define_callbacks {
                     )*
                 }
             }
+
+            /// 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)
+            }
         }
 
         /// Holds a `QueryVTable` for each query.

compiler/rustc_query_impl/src/error.rs

@@ -79,3 +79,28 @@ pub(crate) struct Cycle {
     )]
     pub note_span: (),
 }
+
+#[derive(Subdiagnostic)]
+#[note("...when {$stack_bottom}")]
+pub(crate) struct NestedCycleBottom {
+    pub stack_bottom: String,
+}
+
+#[derive(Diagnostic)]
+#[diag("internal compiler error: query cycle when printing cycle detected")]
+pub(crate) struct NestedCycle {
+    #[primary_span]
+    pub span: Span,
+    #[subdiagnostic]
+    pub stack_bottom: NestedCycleBottom,
+    #[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

@@ -210,7 +210,7 @@ pub(crate) fn layout_of<'tcx>(
                 ControlFlow::Continue(())
             }
         },
-        || create_cycle_error(tcx, &cycle),
+        || create_cycle_error(tcx, &cycle, false),
     );
 
     diag.emit().raise_fatal()

compiler/rustc_query_impl/src/job.rs

@@ -413,15 +413,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 {
@@ -430,14 +431,14 @@ 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 is_all_def_kind = |def_kind| {
@@ -454,23 +455,36 @@ pub(crate) fn create_cycle_error<'tcx>(
         })
     };
 
-    let alias = 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)
+    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
     };
 
-    let cycle_diag = crate::error::Cycle {
-        span,
-        cycle_stack,
-        stack_bottom,
-        alias,
-        cycle_usage,
-        stack_count,
-        note_span: (),
-    };
-
-    tcx.sess.dcx().create_err(cycle_diag)
+    if nested {
+        tcx.sess.dcx().create_err(crate::error::NestedCycle {
+            span,
+            cycle_stack,
+            stack_bottom: crate::error::NestedCycleBottom { 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;
@@ -56,6 +56,7 @@ pub fn query_system<'tcx>(
         local_providers,
         extern_providers,
         jobs: AtomicU64::new(1),
+        cycle_handler_nesting: Lock::new(0),
     }
 }
 

tests/ui/parallel-rustc/default-trait-shadow-cycle-issue-151358.rs

@@ -1,5 +1,6 @@
 // Test for #151358, assertion failed: !worker_thread.is_null()
-//~^ ERROR cycle detected when looking up span for `Default`
+//~^ ERROR internal compiler error: query cycle when printing cycle detected
+//~^^ ERROR cycle detected when getting the resolver for lowering
 //
 //@ compile-flags: -Z threads=2
 //@ compare-output-by-lines

tests/ui/parallel-rustc/default-trait-shadow-cycle-issue-151358.stderr

@@ -1,9 +1,21 @@
-error[E0391]: cycle detected when looking up span for `Default`
+error: internal compiler error: query cycle when printing cycle detected
    |
-   = note: ...which immediately requires looking up span for `Default` again
-   = note: cycle used when perform lints prior to AST lowering
+   = note: ...when getting HIR ID of `Default`
+   = 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/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() {