Add a TaggedQueryKey to identify a query instance

[Zoxc]

This adds back a TaggedQueryKey enum which represents a query kind and the associated key. This is used to replace QueryStackDeferred and QueryStackFrameExtra and the associated lifting operation for cycle errors

This approach has a couple of benefits:

• We only run description queries when printing the query stack trace in the panic handler
• The unsafe code for QueryStackDeferred is removed
• Cycle handles have access to query keys, which may be handy

Some further work could be replacing QueryStackFrame with TaggedQueryKey as the extra information can be derived from it.

[rustbot]

r? @mati865

rustbot has assigned @mati865.
They will have a look at your PR within the next two weeks and either review your PR or reassign to another reviewer.

Use r? to explicitly pick a reviewer

Why was this reviewer chosen?

The reviewer was selected based on:

• Owners of files modified in this PR: compiler, query-system
• compiler, query-system expanded to 69 candidates
• Random selection from 16 candidates

[Zoxc]

cc @nnethercote @Zalathar

[Zalathar]

If we can get rid of a lot of the QueryStackFrame complexity by just storing the query key in an enum, that seems great.

The name QueryKeyId seems strange to me, though. It’s not the ID of a query key; it’s the key itself, “tagged” with the query that it belongs to.

[Zoxc]

The name QueryKeyId seems strange to me, though. It’s not the ID of a query key; it’s the key itself, “tagged” with the query that it belongs to.

Yeah, QueryId was taken, maybe QueryInstance is better?

[nnethercote]

This looks great! I hate the deferred/extra split, this seems like a really clean way to eliminate it.

@Zoxc: can you expand the first paragraph of the PR description to explain (a) the current situation, and (b) how this PR changes that situation? I've skimmed the changes and they look good, but I will need to look again more closely on Monday to get my head fully around how the new QueryKeyId is used and how it lets so much of the existing complexity be removed. Extra description will help with that.

cc @zetanumbers

[Zalathar]

Yeah, QueryId was taken, maybe QueryInstance is better?

I was thinking of something like TaggedQueryKey, because the value being stored is a query key, tagged with its corresponding query.

I think it's easier to say “this is a key that also indicates what query it came from” than to try to come up with an intuitive name for query+key.

Or we could go for something like QueryAndKey or QueryWithKey.

[nnethercote]

TaggedQueryKey sounds good to me -- that's exactly what it is!

[nnethercote]

Just a little more work required to get this into shape.

@rustbot author

[rustbot]

Reminder, once the PR becomes ready for a review, use @rustbot ready.

[rustbot]

This PR was rebased onto a different main commit. Here's a range-diff highlighting what actually changed.

Rebasing is a normal part of keeping PRs up to date, so no action is needed—this note is just to help reviewers.