Don't use queries when printing the query stack

[Zoxc]

This adds a parameter to try_collect_active_jobs indicating if it can use queries to describe query frames. This is set to false when when printing the query stack during an ICE to work around the infinite recursion seen in #112522.

[rustbot]

r? @eholk

(rustbot has picked a reviewer for you, use r? to override)

[compiler-errors]

This seems like a really heavy hammer to solve this issue.

Is it possible to just use with_no_queries/NO_QUERIES more judiciously within the pretty-printing code? I'd rather we print something for the query stack -- knowing the query frame descr is often very useful...

[bjorn3]

Can this be moved below the description formatting? It seems like that shouldn't use any queries.

[Zoxc]

That can and does use queries.

[Zoxc]

It would be nice to at least print query keys, but I think we need either a separate non-query printing path or a better way to statically prevent queries to run for the existing path.

[bors]

☔ The latest upstream changes (presumably #113303) made this pull request unmergeable. Please resolve the merge conflicts.

[eholk]

@compiler-errors - is the root of your concern that we're losing a lot of really helpful query information in the common case to avoid recursion in pathological cases?

I wonder if having a recursion_depth parameter we thread around would be a better compromise?

[compiler-errors]

is the root of your concern that we're losing a lot of really helpful query information in the common case to avoid recursion in pathological cases?

Pretty much, yeah. The query stack being able to print keys on ICEs, for example, is quite helpful in my experience debugging the compiler.

[eholk]

@Zoxc - Do you think a recursion limit would work?

@rustbot author

[Zoxc]

A recursion limit doesn't help for the parallel compiler which has more interesting failure scenarios. A more reliable way to print def paths is probably the way to go to restore debugging information.

[Zoxc]

I've introduced PrintContext and PrintArg types which get passed to the description function of queries. They hide tcx and the query key from direct access enabling the regular query descriptions to be used for the panic stack trace. If the query description use methods on PrintArg to access TyCtxt or the inner type, it will print <inaccessible> instead of doing the access for the panic case.

cc @cjgillot

[bors]

☔ The latest upstream changes (presumably #115326) made this pull request unmergeable. Please resolve the merge conflicts.

[cjgillot]

This PR is very complex, and makes the query description weird by changing the key type behind our back.
Alternative version: could we remove access to the tcx from TLS when we are in the plumbing code?

Today, we manipulate TLS twice when setting up a query:

• once in start_query to setup the recursion, token and side-effect trackers,
• once in with_task to setup dependency tracking.

Can the outer one remove access to the tcx, and have the inner one put it back?

[JohnCSimon]

@Zoxc

ping from triage - can you post your status on this PR? This PR has not received an update in a few months. Thank you!

FYI: when a PR is ready for review, send a message containing
@rustbot ready to switch to S-waiting-on-review so the PR is in the reviewer's backlog.

[Zoxc]

It doesn't seem like either approaches taken in this PR are very popular.

We may instead make the existing printing code respect with_no_queries!.