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

cc @nnethercote @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.

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?

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

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

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.

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