Cycle error from pretty printer with RUST_LOG

[davidtwco]

(I wasn't able to find any existing discussion, issues or PRs about this, I'd not be surprised if someone had found it already, so my bad if this isn't a new problem)

Yesterday, I was about to take a look into #59494 (the test case for that will trigger this), running that test case with RUST_LOG=rustc::traits makes a cycle error happen instead of the expected ICE. I can get the ICE with just RUST_LOG=rustc::traits::select (IIRC), but not when I include other modules.

The cycle error comes from the pretty printer, in particular this line. I managed to resolve the issue, here's the diff that fixed it - when the pretty printer calls predicates_of for existential types then it ends up calling code with debug logs that print the same type, causing a cycle.

The diff above only bypasses the cycle error when RUST_LOG=rustc::ty::subst,rustc::ty::context,rustc_typeck, I assume it won't when you add more modules. Here's the relevant Zulip topic.

[davidtwco]

This can be worked around using -Z verbose (relevant Discord chat).

[eddyb]

cc @nikomatsakis @arielb1 How was this dealt with previously? For debug printing that happens to result in cycles with certain values of RUST_LOG?

[nikomatsakis]

This is a thorny problem. We've never had a good solution.

In general, I think it's a problem that {:?} and {} share as much code as they do. I tend to think that {:?} should really be much more conservative and closer to the internal representation than it is, which might help us avoid some of these problems. (I basically only use the compiler with -Zverbose , for example, which tends to force it to include more information.) But this is really the sort of topic that's going to take some exploration.

(In fact, I don't particularly like that we implement {:?} at all, since I think we should be threading down contextual information -- and I think that information should be more than the tcx, which can use TLS, but should include e.g. the infcx or other such things. I liked the idea of the DebugWith trait we were using in Lark (along with a custom derive), which let us thread down different kinds of context and get different kinds of output as a result -- e.g., including the infcx would "resolve" inference variables -- but I never used it deeply enough to know if it was ultimately kind of annoying.)

[eddyb]

We used to require methods for printing and it was really cumbersome.
IMO the best solution would be implicit scoped contexts that type-checked and passed as regular arguments (but behind the scenes, so the user doesn't have to mention them at every callsite).
But that's a language feature, and a pretty big one for Rust.

cc @eternaleye (who introduced me to this idea)