-
-
Notifications
You must be signed in to change notification settings - Fork 14.4k
Reduce generics use in the query system. #151777
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
base: main
Are you sure you want to change the base?
Conversation
Instead of passing a closure that computes a hash and immediately calling it, just compute the hash and pass the value.
It produces a `QueryStackFrameExtra`, so `stack_` should be in the name.
`QueryStackFrame<I>` is a generic type, and the `I` parameter leaks out to many other related types. However, it only has two instantiations in practice: - `QueryStackFrame<QueryStackFrameExtra>` - `QueryStackFrame<QueryStackDeferred<'tcx>>` And most of the places that currently use `QueryStackFrame<I>` are actually specific to one of the instantiations. This commit removes the unneeded genericity. The following types are only ever used with `QueryStackDeferred<'tcx>`: - QueryMap - QueryJobInfo - QueryJob - QueryWaiter - QueryLatchInfo - QueryLatch - QueryState - QueryResult and their `<I>` parameter is changed to `<'tcx>`. Also, the `QueryContext::QueryInfo` associated type is removed. `CycleError<I>` and `QueryInfo<I>` are still generic over type, because they are used with both instantiations. This required also adding a `<'tcx>` parameter to the traits `QueryConfig` and `QueryContext`, which is annoying but I can't see how to avoid it.
|
|
|
Yea this makes sense. Not much of an improvement, but lifetime generics are better than type generics for readability |
|
Be aware that this will textually conflict with some of the renames in #151666, though the changes should be compatible. Also, after that PR I intend to change the generic type of |
|
☔ The latest upstream changes (presumably #151778) made this pull request unmergeable. Please resolve the merge conflicts. This pull request was unapproved. |
|
The original implementation tried to avoid adding the |
In #151203 I tried and failed to simplify
QueryStackFrame. This is an alternative approach. There is no functional change, just tweaking of some names and types to make things clearer. Best reviewed one commit at a time.r? @oli-obk