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...

Author: Dylan-DPC

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,20 +738,22 @@ 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);
                 }
 
                 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"),

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 };

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

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

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

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() {}

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`.