-
Notifications
You must be signed in to change notification settings - Fork 12.7k
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
Merge query property modules into one #111703
Conversation
This comment has been minimized.
This comment has been minimized.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Some nits and r=me
@@ -24,7 +24,7 @@ use rustc_hir::def::DefKind; | |||
use rustc_hir::def_id::LocalDefId; | |||
use rustc_interface::interface::Compiler; | |||
use rustc_interface::{Config, Queries}; | |||
use rustc_middle::query::query_values::mir_borrowck; | |||
use rustc_middle::query::queries::mir_borrowck::ProvidedValue; |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This should import the actual type from rustc_middle, not go through query type aliases.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I think the idea is to avoid relying on the return type, but I'm not sure why.
value: query_provided::$name<'tcx>, | ||
) -> Erase<query_values::$name<'tcx>> { | ||
value: ProvidedValue<'tcx>, | ||
) -> Erase<Value<'tcx>> { |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This probably deserves an ErasedValue
alias too.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
That saves just 1 character though.
@@ -503,11 +503,11 @@ macro_rules! define_queries { | |||
pub(super) fn $name<'tcx>( |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Should we attempt to use the same convention in this file?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Probably.
@bors r+ |
Merge query property modules into one This merges all the query modules that defines types into a single module per query with a normal naming convention for type aliases. r? `@cjgillot`
Rollup of 7 pull requests Successful merges: - rust-lang#110986 (Delay a bug when overwriting fed value.) - rust-lang#111054 (Do not recover when parsing stmt in cfg-eval.) - rust-lang#111685 (Fix typo in bootstrap command description) - rust-lang#111686 (Retire is_foreign_item query.) - rust-lang#111695 (Exclude inherent projections from some alias type `match`es) - rust-lang#111703 (Merge query property modules into one) - rust-lang#111707 (Remove unused `impl<T> WorkerLocal<Vec<T>>`.) r? `@ghost` `@rustbot` modify labels: rollup
Merge some query impl modules into one This merges some modules in `rustc_query_impl` into one per query, analogous to rust-lang#111703. r? `@cjgillot`
This merges all the query modules that defines types into a single module per query with a normal naming convention for type aliases.
r? @cjgillot