Rollup merge of #107585 - compiler-errors:fndef-sig-cycle, r=oli-obk

Don't cause a cycle when formatting query description that references a FnDef

When a function returns `-> _`, we use typeck to compute what the resulting type of the body _should_ be. If we call another query inside of typeck and hit a cycle error, we attempt to report the cycle error which requires us to compute all of the query descriptions for the stack.

However, if one of the queries in that cycle has a query description that references this function as a FnDef type, we'll cause a *second* cycle error from within the cycle error reporting code, since rendering a FnDef requires us to compute its signature. This causes an unwrap to ICE, since during the *second* cycle reporting code, we try to look for a job that isn't in the active jobs list.

We can avoid this by using `with_no_queries!` when computing these query descriptions.

Fixes #107089

The only drawback is that the rendering of opaque types in cycles regresses a bit :| I'm open to alternate suggestions about how we may handle this...

7 files changed, 51 insertions, 20 deletions

diff --git a/compiler/rustc_middle/src/ty/print/pretty.rs b/compiler/rustc_middle/src/ty/print/pretty.rs
index e8e00d5feb8..bbb4fd999bc 100644
--- a/compiler/rustc_middle/src/ty/print/pretty.rs
+++ b/compiler/rustc_middle/src/ty/print/pretty.rs
@@ -675,8 +675,12 @@ pub trait PrettyPrinter<'tcx>:
                 p!(")")
             }
             ty::FnDef(def_id, substs) => {
-                let sig = self.tcx().fn_sig(def_id).subst(self.tcx(), substs);
-                p!(print(sig), " {{", print_value_path(def_id, substs), "}}");
+                if NO_QUERIES.with(|q| q.get()) {
+                    p!(print_def_path(def_id, substs));
+                } else {
+                    let sig = self.tcx().fn_sig(def_id).subst(self.tcx(), substs);
+                    p!(print(sig), " {{", print_value_path(def_id, substs), "}}");
+                }
             }
             ty::FnPtr(ref bare_fn) => p!(print(bare_fn)),
             ty::Infer(infer_ty) => {
@@ -734,13 +738,13 @@ pub trait PrettyPrinter<'tcx>:
             }
             ty::Placeholder(placeholder) => p!(write("Placeholder({:?})", placeholder)),
             ty::Alias(ty::Opaque, ty::AliasTy { def_id, substs, .. }) => {
-                // FIXME(eddyb) print this with `print_def_path`.
                 // We use verbose printing in 'NO_QUERIES' mode, to
                 // avoid needing to call `predicates_of`. This should
                 // only affect certain debug messages (e.g. messages printed
                 // from `rustc_middle::ty` during the computation of `tcx.predicates_of`),
                 // and should have no effect on any compiler output.
-                if self.should_print_verbose() || NO_QUERIES.with(|q| q.get()) {
+                if self.should_print_verbose() {
+                    // FIXME(eddyb) print this with `print_def_path`.
                     p!(write("Opaque({:?}, {:?})", def_id, substs));
                     return Ok(self);
                 }
@@ -748,6 +752,8 @@ pub trait PrettyPrinter<'tcx>:
                 let parent = self.tcx().parent(def_id);
                 match self.tcx().def_kind(parent) {
                     DefKind::TyAlias | DefKind::AssocTy => {
+                        // NOTE: I know we should check for NO_QUERIES here, but it's alright.
+                        // `type_of` on a type alias or assoc type should never cause a cycle.
                         if let ty::Alias(ty::Opaque, ty::AliasTy { def_id: d, .. }) =
                             *self.tcx().type_of(parent).kind()
                         {
@@ -762,7 +768,14 @@ pub trait PrettyPrinter<'tcx>:
                         p!(print_def_path(def_id, substs));
                         return Ok(self);
                     }
-                    _ => return self.pretty_print_opaque_impl_type(def_id, substs),
+                    _ => {
+                        if NO_QUERIES.with(|q| q.get()) {
+                            p!(print_def_path(def_id, &[]));
+                            return Ok(self);
+                        } else {
+                            return self.pretty_print_opaque_impl_type(def_id, substs);
+                        }
+                    }
                 }
             }
             ty::Str => p!("str"),
diff --git a/compiler/rustc_query_impl/src/plumbing.rs b/compiler/rustc_query_impl/src/plumbing.rs
index 6125ad4eff1..4dea03c1ef6 100644
--- a/compiler/rustc_query_impl/src/plumbing.rs
+++ b/compiler/rustc_query_impl/src/plumbing.rs
@@ -314,11 +314,14 @@ pub(crate) fn create_query_frame<
     kind: DepKind,
     name: &'static str,
 ) -> QueryStackFrame<DepKind> {
-    // Disable visible paths printing for performance reasons.
-    // Showing visible path instead of any path is not that important in production.
-    let description = ty::print::with_no_visible_paths!(
-        // Force filename-line mode to avoid invoking `type_of` query.
-        ty::print::with_forced_impl_filename_line!(do_describe(tcx.tcx, key))
+    // Avoid calling queries while formatting the description
+    let description = ty::print::with_no_queries!(
+        // Disable visible paths printing for performance reasons.
+        // Showing visible path instead of any path is not that important in production.
+        ty::print::with_no_visible_paths!(
+            // Force filename-line mode to avoid invoking `type_of` query.
+            ty::print::with_forced_impl_filename_line!(do_describe(tcx.tcx, key))
+        )
     );
     let description =
         if tcx.sess.verbose() { format!("{description} [{name:?}]") } else { description };
diff --git a/tests/ui/async-await/no-const-async.stderr b/tests/ui/async-await/no-const-async.stderr
index c5bd520aaea..71c228958f6 100644
--- a/tests/ui/async-await/no-const-async.stderr
+++ b/tests/ui/async-await/no-const-async.stderr
@@ -28,8 +28,8 @@ note: ...which requires const checking `x`...
    |
 LL | pub const async fn x() {}
    | ^^^^^^^^^^^^^^^^^^^^^^
-   = note: ...which requires computing whether `impl core::future::future::Future<Output = ()>` is freeze...
-   = note: ...which requires evaluating trait selection obligation `impl core::future::future::Future<Output = ()>: core::marker::Freeze`...
+   = note: ...which requires computing whether `x::{opaque#0}` is freeze...
+   = note: ...which requires evaluating trait selection obligation `x::{opaque#0}: core::marker::Freeze`...
    = note: ...which again requires computing type of `x::{opaque#0}`, completing the cycle
 note: cycle used when checking item types in top-level module
   --> $DIR/no-const-async.rs:4:1
diff --git a/tests/ui/impl-trait/auto-trait-leak.stderr b/tests/ui/impl-trait/auto-trait-leak.stderr
index feedfc40aaf..fd0358421eb 100644
--- a/tests/ui/impl-trait/auto-trait-leak.stderr
+++ b/tests/ui/impl-trait/auto-trait-leak.stderr
@@ -39,7 +39,7 @@ note: ...which requires type-checking `cycle1`...
    |
 LL |     send(cycle2().clone());
    |     ^^^^
-   = note: ...which requires evaluating trait selection obligation `impl core::clone::Clone: core::marker::Send`...
+   = note: ...which requires evaluating trait selection obligation `cycle2::{opaque#0}: core::marker::Send`...
 note: ...which requires computing type of `cycle2::{opaque#0}`...
   --> $DIR/auto-trait-leak.rs:19:16
    |
@@ -80,7 +80,7 @@ note: ...which requires type-checking `cycle2`...
    |
 LL |     send(cycle1().clone());
    |     ^^^^
-   = note: ...which requires evaluating trait selection obligation `impl core::clone::Clone: core::marker::Send`...
+   = note: ...which requires evaluating trait selection obligation `cycle1::{opaque#0}: core::marker::Send`...
    = note: ...which again requires computing type of `cycle1::{opaque#0}`, completing the cycle
 note: cycle used when checking item types in top-level module
   --> $DIR/auto-trait-leak.rs:1:1
diff --git a/tests/ui/parser/fn-header-semantic-fail.stderr b/tests/ui/parser/fn-header-semantic-fail.stderr
index 038fdfb2d51..2d8bd19a731 100644
--- a/tests/ui/parser/fn-header-semantic-fail.stderr
+++ b/tests/ui/parser/fn-header-semantic-fail.stderr
@@ -209,8 +209,8 @@ note: ...which requires const checking `main::ff5`...
    |
 LL |     const async unsafe extern "C" fn ff5() {}
    |     ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^
-   = note: ...which requires computing whether `impl core::future::future::Future<Output = ()>` is freeze...
-   = note: ...which requires evaluating trait selection obligation `impl core::future::future::Future<Output = ()>: core::marker::Freeze`...
+   = note: ...which requires computing whether `main::ff5::{opaque#0}` is freeze...
+   = note: ...which requires evaluating trait selection obligation `main::ff5::{opaque#0}: core::marker::Freeze`...
    = note: ...which again requires computing type of `main::ff5::{opaque#0}`, completing the cycle
 note: cycle used when checking item types in top-level module
   --> $DIR/fn-header-semantic-fail.rs:5:1
@@ -245,8 +245,8 @@ note: ...which requires const checking `main::<impl at $DIR/fn-header-semantic-f
    |
 LL |         const async unsafe extern "C" fn ft5() {}
    |         ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^
-   = note: ...which requires computing whether `impl core::future::future::Future<Output = ()>` is freeze...
-   = note: ...which requires evaluating trait selection obligation `impl core::future::future::Future<Output = ()>: core::marker::Freeze`...
+   = note: ...which requires computing whether `main::<impl at $DIR/fn-header-semantic-fail.rs:28:5: 28:17>::ft5::{opaque#0}` is freeze...
+   = note: ...which requires evaluating trait selection obligation `main::<impl at $DIR/fn-header-semantic-fail.rs:28:5: 28:17>::ft5::{opaque#0}: core::marker::Freeze`...
    = note: ...which again requires computing type of `main::<impl at $DIR/fn-header-semantic-fail.rs:28:5: 28:17>::ft5::{opaque#0}`, completing the cycle
 note: cycle used when checking item types in top-level module
   --> $DIR/fn-header-semantic-fail.rs:5:1
@@ -281,8 +281,8 @@ note: ...which requires const checking `main::<impl at $DIR/fn-header-semantic-f
    |
 LL |         const async unsafe extern "C" fn fi5() {}
    |         ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^
-   = note: ...which requires computing whether `impl core::future::future::Future<Output = ()>` is freeze...
-   = note: ...which requires evaluating trait selection obligation `impl core::future::future::Future<Output = ()>: core::marker::Freeze`...
+   = note: ...which requires computing whether `main::<impl at $DIR/fn-header-semantic-fail.rs:40:5: 40:11>::fi5::{opaque#0}` is freeze...
+   = note: ...which requires evaluating trait selection obligation `main::<impl at $DIR/fn-header-semantic-fail.rs:40:5: 40:11>::fi5::{opaque#0}: core::marker::Freeze`...
    = note: ...which again requires computing type of `main::<impl at $DIR/fn-header-semantic-fail.rs:40:5: 40:11>::fi5::{opaque#0}`, completing the cycle
 note: cycle used when checking item types in top-level module
   --> $DIR/fn-header-semantic-fail.rs:5:1
diff --git a/tests/ui/query-system/no-query-in-printing-during-query-descr.rs b/tests/ui/query-system/no-query-in-printing-during-query-descr.rs
new file mode 100644
index 00000000000..45b7043e2f6
--- /dev/null
+++ b/tests/ui/query-system/no-query-in-printing-during-query-descr.rs
@@ -0,0 +1,6 @@
+fn a() -> _ {
+    //~^ ERROR the placeholder `_` is not allowed within types on item signatures for return types
+    &a
+}
+
+fn main() {}
diff --git a/tests/ui/query-system/no-query-in-printing-during-query-descr.stderr b/tests/ui/query-system/no-query-in-printing-during-query-descr.stderr
new file mode 100644
index 00000000000..35e608b6b23
--- /dev/null
+++ b/tests/ui/query-system/no-query-in-printing-during-query-descr.stderr
@@ -0,0 +1,9 @@
+error[E0121]: the placeholder `_` is not allowed within types on item signatures for return types
+  --> $DIR/no-query-in-printing-during-query-descr.rs:1:11
+   |
+LL | fn a() -> _ {
+   |           ^ not allowed in type signatures
+
+error: aborting due to previous error
+
+For more information about this error, try `rustc --explain E0121`.