From fb0c36a3fe8d6da31c221cf3b65e6e316e7d9c8e Mon Sep 17 00:00:00 2001 From: Joshua Nelson Date: Thu, 1 Sep 2022 21:17:35 -0500 Subject: [PATCH 1/8] Make the `storage` query modifier less general In practice, it was only ever used with `ArenaCacheSelector`. Change it to a single boolean `arena_cache` rather than allowing queries to specify an arbitrary type. --- compiler/rustc_macros/src/query.rs | 20 +++--- compiler/rustc_middle/src/query/mod.rs | 98 +++++++++++++------------- compiler/rustc_middle/src/ty/query.rs | 4 +- 3 files changed, 59 insertions(+), 63 deletions(-) diff --git a/compiler/rustc_macros/src/query.rs b/compiler/rustc_macros/src/query.rs index f93fe2d519508..a99cba817d96e 100644 --- a/compiler/rustc_macros/src/query.rs +++ b/compiler/rustc_macros/src/query.rs @@ -86,7 +86,7 @@ struct QueryModifiers { desc: (Option, Punctuated), /// Use this type for the in-memory cache. - storage: Option, + arena_cache: Option, /// Cache the query to disk if the `Block` returns true. cache: Option<(Option, Block)>, @@ -121,7 +121,7 @@ struct QueryModifiers { fn parse_query_modifiers(input: ParseStream<'_>) -> Result { let mut load_cached = None; - let mut storage = None; + let mut arena_cache = None; let mut cache = None; let mut desc = None; let mut fatal_cycle = None; @@ -183,11 +183,8 @@ fn parse_query_modifiers(input: ParseStream<'_>) -> Result { let id = args.parse()?; let block = input.parse()?; try_insert!(load_cached = (tcx, id, block)); - } else if modifier == "storage" { - let args; - parenthesized!(args in input); - let ty = args.parse()?; - try_insert!(storage = ty); + } else if modifier == "arena_cache" { + try_insert!(arena_cache = modifier); } else if modifier == "fatal_cycle" { try_insert!(fatal_cycle = modifier); } else if modifier == "cycle_delay_bug" { @@ -213,7 +210,7 @@ fn parse_query_modifiers(input: ParseStream<'_>) -> Result { }; Ok(QueryModifiers { load_cached, - storage, + arena_cache, cache, desc, fatal_cycle, @@ -351,10 +348,9 @@ pub fn rustc_queries(input: TokenStream) -> TokenStream { if let Some(fatal_cycle) = &modifiers.fatal_cycle { attributes.push(quote! { (#fatal_cycle) }); }; - // Pass on the storage modifier - if let Some(ref ty) = modifiers.storage { - let span = ty.span(); - attributes.push(quote_spanned! {span=> (storage #ty) }); + // Pass on the arena modifier + if let Some(ref arena_cache) = modifiers.arena_cache { + attributes.push(quote! {span=> (#arena_cache) }); }; // Pass on the cycle_delay_bug modifier if let Some(cycle_delay_bug) = &modifiers.cycle_delay_bug { diff --git a/compiler/rustc_middle/src/query/mod.rs b/compiler/rustc_middle/src/query/mod.rs index adf479c423604..8e62b05d5f69f 100644 --- a/compiler/rustc_middle/src/query/mod.rs +++ b/compiler/rustc_middle/src/query/mod.rs @@ -47,14 +47,14 @@ rustc_queries! { /// To avoid this fate, do not call `tcx.hir().krate()`; instead, /// prefer wrappers like `tcx.visit_all_items_in_krate()`. query hir_crate(key: ()) -> Crate<'tcx> { - storage(ArenaCacheSelector<'tcx>) + arena_cache eval_always desc { "get the crate HIR" } } /// All items in the crate. query hir_crate_items(_: ()) -> rustc_middle::hir::ModuleItems { - storage(ArenaCacheSelector<'tcx>) + arena_cache eval_always desc { "get HIR crate items" } } @@ -64,7 +64,7 @@ rustc_queries! { /// This can be conveniently accessed by `tcx.hir().visit_item_likes_in_module`. /// Avoid calling this query directly. query hir_module_items(key: LocalDefId) -> rustc_middle::hir::ModuleItems { - storage(ArenaCacheSelector<'tcx>) + arena_cache desc { |tcx| "HIR module items in `{}`", tcx.def_path_str(key.to_def_id()) } cache_on_disk_if { true } } @@ -196,7 +196,7 @@ rustc_queries! { /// associated generics. query generics_of(key: DefId) -> ty::Generics { desc { |tcx| "computing generics of `{}`", tcx.def_path_str(key) } - storage(ArenaCacheSelector<'tcx>) + arena_cache cache_on_disk_if { key.is_local() } separate_provide_extern } @@ -268,13 +268,13 @@ rustc_queries! { } query native_libraries(_: CrateNum) -> Vec { - storage(ArenaCacheSelector<'tcx>) + arena_cache desc { "looking up the native libraries of a linked crate" } separate_provide_extern } query lint_levels(_: ()) -> LintLevelMap { - storage(ArenaCacheSelector<'tcx>) + arena_cache eval_always desc { "computing the lint levels for items in this crate" } } @@ -307,7 +307,7 @@ rustc_queries! { /// Create a THIR tree for debugging. query thir_tree(key: ty::WithOptConstParam) -> String { no_hash - storage(ArenaCacheSelector<'tcx>) + arena_cache desc { |tcx| "constructing THIR tree for `{}`", tcx.def_path_str(key.did.to_def_id()) } } @@ -315,7 +315,7 @@ rustc_queries! { /// them. This includes all the body owners, but also things like struct /// constructors. query mir_keys(_: ()) -> rustc_data_structures::fx::FxIndexSet { - storage(ArenaCacheSelector<'tcx>) + arena_cache desc { "getting a list of all mir_keys" } } @@ -422,7 +422,7 @@ rustc_queries! { query symbols_for_closure_captures( key: (LocalDefId, LocalDefId) ) -> Vec { - storage(ArenaCacheSelector<'tcx>) + arena_cache desc { |tcx| "symbols for captures of closure `{}` in `{}`", tcx.def_path_str(key.1.to_def_id()), @@ -442,7 +442,7 @@ rustc_queries! { /// MIR pass (assuming the -Cinstrument-coverage option is enabled). query coverageinfo(key: ty::InstanceDef<'tcx>) -> mir::CoverageInfo { desc { |tcx| "retrieving coverage info from MIR for `{}`", tcx.def_path_str(key.def_id()) } - storage(ArenaCacheSelector<'tcx>) + arena_cache } /// Returns the `CodeRegions` for a function that has instrumented coverage, in case the @@ -452,7 +452,7 @@ rustc_queries! { |tcx| "retrieving the covered `CodeRegion`s, if instrumented, for `{}`", tcx.def_path_str(key) } - storage(ArenaCacheSelector<'tcx>) + arena_cache cache_on_disk_if { key.is_local() } } @@ -490,7 +490,7 @@ rustc_queries! { } query wasm_import_module_map(_: CrateNum) -> FxHashMap { - storage(ArenaCacheSelector<'tcx>) + arena_cache desc { "wasm import module map" } } @@ -566,7 +566,7 @@ rustc_queries! { query trait_def(key: DefId) -> ty::TraitDef { desc { |tcx| "computing trait definition for `{}`", tcx.def_path_str(key) } - storage(ArenaCacheSelector<'tcx>) + arena_cache cache_on_disk_if { key.is_local() } separate_provide_extern } @@ -644,7 +644,7 @@ rustc_queries! { /// Gets a map with the variance of every item; use `item_variance` instead. query crate_variances(_: ()) -> ty::CrateVariancesMap<'tcx> { - storage(ArenaCacheSelector<'tcx>) + arena_cache desc { "computing the variances for items in this crate" } } @@ -657,7 +657,7 @@ rustc_queries! { /// Maps from thee `DefId` of a type to its (inferred) outlives. query inferred_outlives_crate(_: ()) -> ty::CratePredicatesMap<'tcx> { - storage(ArenaCacheSelector<'tcx>) + arena_cache desc { "computing the inferred outlives predicates for items in this crate" } } @@ -671,14 +671,14 @@ rustc_queries! { /// Maps from a trait item to the trait item "descriptor". query associated_item(key: DefId) -> ty::AssocItem { desc { |tcx| "computing associated item data for `{}`", tcx.def_path_str(key) } - storage(ArenaCacheSelector<'tcx>) + arena_cache cache_on_disk_if { key.is_local() } separate_provide_extern } /// Collects the associated items defined on a trait or impl. query associated_items(key: DefId) -> ty::AssocItems<'tcx> { - storage(ArenaCacheSelector<'tcx>) + arena_cache desc { |tcx| "collecting associated items of {}", tcx.def_path_str(key) } } @@ -704,7 +704,7 @@ rustc_queries! { /// The map returned for `tcx.impl_item_implementor_ids(impl_id)` would be ///`{ trait_f: impl_f, trait_g: impl_g }` query impl_item_implementor_ids(impl_id: DefId) -> FxHashMap { - storage(ArenaCacheSelector<'tcx>) + arena_cache desc { |tcx| "comparing impl items against trait for {}", tcx.def_path_str(impl_id) } } @@ -837,7 +837,7 @@ rustc_queries! { FxHashSet, FxHashMap> ) { - storage(ArenaCacheSelector<'tcx>) + arena_cache desc { "find live symbols in crate" } } @@ -921,7 +921,7 @@ rustc_queries! { /// Gets a complete map from all types to their inherent impls. /// Not meant to be used directly outside of coherence. query crate_inherent_impls(k: ()) -> CrateInherentImpls { - storage(ArenaCacheSelector<'tcx>) + arena_cache desc { "all inherent impls defined in crate" } } @@ -1054,7 +1054,7 @@ rustc_queries! { } query reachable_set(_: ()) -> FxHashSet { - storage(ArenaCacheSelector<'tcx>) + arena_cache desc { "reachability" } } @@ -1066,7 +1066,7 @@ rustc_queries! { /// Generates a MIR body for the shim. query mir_shims(key: ty::InstanceDef<'tcx>) -> mir::Body<'tcx> { - storage(ArenaCacheSelector<'tcx>) + arena_cache desc { |tcx| "generating MIR shim for `{}`", tcx.def_path_str(key.def_id()) } } @@ -1140,7 +1140,7 @@ rustc_queries! { query codegen_fn_attrs(def_id: DefId) -> CodegenFnAttrs { desc { |tcx| "computing codegen attributes of `{}`", tcx.def_path_str(def_id) } - storage(ArenaCacheSelector<'tcx>) + arena_cache cache_on_disk_if { def_id.is_local() } separate_provide_extern } @@ -1157,7 +1157,7 @@ rustc_queries! { /// Gets the rendered value of the specified constant or associated constant. /// Used by rustdoc. query rendered_const(def_id: DefId) -> String { - storage(ArenaCacheSelector<'tcx>) + arena_cache desc { |tcx| "rendering constant initializer of `{}`", tcx.def_path_str(def_id) } cache_on_disk_if { def_id.is_local() } separate_provide_extern @@ -1219,12 +1219,12 @@ rustc_queries! { /// Given a trait `trait_id`, return all known `impl` blocks. query trait_impls_of(trait_id: DefId) -> ty::trait_def::TraitImpls { - storage(ArenaCacheSelector<'tcx>) + arena_cache desc { |tcx| "trait impls of `{}`", tcx.def_path_str(trait_id) } } query specialization_graph_of(trait_id: DefId) -> specialization_graph::Graph { - storage(ArenaCacheSelector<'tcx>) + arena_cache desc { |tcx| "building specialization graph of trait `{}`", tcx.def_path_str(trait_id) } cache_on_disk_if { true } } @@ -1351,7 +1351,7 @@ rustc_queries! { } query dependency_formats(_: ()) -> Lrc { - storage(ArenaCacheSelector<'tcx>) + arena_cache desc { "get the linkage format of all dependencies" } } @@ -1444,7 +1444,7 @@ rustc_queries! { // like the compiler-generated `main` function and so on. query reachable_non_generics(_: CrateNum) -> DefIdMap { - storage(ArenaCacheSelector<'tcx>) + arena_cache desc { "looking up the exported symbols of a crate" } separate_provide_extern } @@ -1467,7 +1467,7 @@ rustc_queries! { /// `upstream_monomorphizations_for`, `upstream_drop_glue_for`, or, even /// better, `Instance::upstream_monomorphization()`. query upstream_monomorphizations(_: ()) -> DefIdMap, CrateNum>> { - storage(ArenaCacheSelector<'tcx>) + arena_cache desc { "collecting available upstream monomorphizations" } } @@ -1481,7 +1481,7 @@ rustc_queries! { query upstream_monomorphizations_for(def_id: DefId) -> Option<&'tcx FxHashMap, CrateNum>> { - storage(ArenaCacheSelector<'tcx>) + arena_cache desc { |tcx| "collecting available upstream monomorphizations for `{}`", tcx.def_path_str(def_id), @@ -1509,7 +1509,7 @@ rustc_queries! { } query foreign_modules(_: CrateNum) -> FxHashMap { - storage(ArenaCacheSelector<'tcx>) + arena_cache desc { "looking up the foreign modules of a linked crate" } separate_provide_extern } @@ -1535,13 +1535,13 @@ rustc_queries! { separate_provide_extern } query extra_filename(_: CrateNum) -> String { - storage(ArenaCacheSelector<'tcx>) + arena_cache eval_always desc { "looking up the extra filename for a crate" } separate_provide_extern } query crate_extern_paths(_: CrateNum) -> Vec { - storage(ArenaCacheSelector<'tcx>) + arena_cache eval_always desc { "looking up the paths for extern crates" } separate_provide_extern @@ -1583,14 +1583,14 @@ rustc_queries! { /// the same lifetimes and is responsible for diagnostics. /// See `rustc_resolve::late::lifetimes for details. query resolve_lifetimes_trait_definition(_: LocalDefId) -> ResolveLifetimes { - storage(ArenaCacheSelector<'tcx>) + arena_cache desc { "resolving lifetimes for a trait definition" } } /// Does lifetime resolution on items. Importantly, we can't resolve /// lifetimes directly on things like trait methods, because of trait params. /// See `rustc_resolve::late::lifetimes for details. query resolve_lifetimes(_: LocalDefId) -> ResolveLifetimes { - storage(ArenaCacheSelector<'tcx>) + arena_cache desc { "resolving lifetimes" } } query named_region_map(_: LocalDefId) -> @@ -1650,7 +1650,7 @@ rustc_queries! { } query lib_features(_: ()) -> LibFeatures { - storage(ArenaCacheSelector<'tcx>) + arena_cache desc { "calculating the lib features map" } } query defined_lib_features(_: CrateNum) -> &'tcx [(Symbol, Option)] { @@ -1658,7 +1658,7 @@ rustc_queries! { separate_provide_extern } query stability_implications(_: CrateNum) -> FxHashMap { - storage(ArenaCacheSelector<'tcx>) + arena_cache desc { "calculating the implications between `#[unstable]` features defined in a crate" } separate_provide_extern } @@ -1669,14 +1669,14 @@ rustc_queries! { } /// Returns the lang items defined in another crate by loading it from metadata. query get_lang_items(_: ()) -> LanguageItems { - storage(ArenaCacheSelector<'tcx>) + arena_cache eval_always desc { "calculating the lang items map" } } /// Returns all diagnostic items defined in all crates. query all_diagnostic_items(_: ()) -> rustc_hir::diagnostic_items::DiagnosticItems { - storage(ArenaCacheSelector<'tcx>) + arena_cache eval_always desc { "calculating the diagnostic items map" } } @@ -1689,7 +1689,7 @@ rustc_queries! { /// Returns the diagnostic items defined in a crate. query diagnostic_items(_: CrateNum) -> rustc_hir::diagnostic_items::DiagnosticItems { - storage(ArenaCacheSelector<'tcx>) + arena_cache desc { "calculating the diagnostic items map in a crate" } separate_provide_extern } @@ -1699,11 +1699,11 @@ rustc_queries! { separate_provide_extern } query visible_parent_map(_: ()) -> DefIdMap { - storage(ArenaCacheSelector<'tcx>) + arena_cache desc { "calculating the visible parent map" } } query trimmed_def_paths(_: ()) -> FxHashMap { - storage(ArenaCacheSelector<'tcx>) + arena_cache desc { "calculating trimmed def paths" } } query missing_extern_crate_item(_: CrateNum) -> bool { @@ -1712,14 +1712,14 @@ rustc_queries! { separate_provide_extern } query used_crate_source(_: CrateNum) -> Lrc { - storage(ArenaCacheSelector<'tcx>) + arena_cache eval_always desc { "looking at the source for a crate" } separate_provide_extern } /// Returns the debugger visualizers defined for this crate. query debugger_visualizers(_: CrateNum) -> Vec { - storage(ArenaCacheSelector<'tcx>) + arena_cache desc { "looking up the debugger visualizers for this crate" } separate_provide_extern } @@ -1753,7 +1753,7 @@ rustc_queries! { } query stability_index(_: ()) -> stability::Index { - storage(ArenaCacheSelector<'tcx>) + arena_cache eval_always desc { "calculating the stability index for the local crate" } } @@ -1994,7 +1994,7 @@ rustc_queries! { } query supported_target_features(_: CrateNum) -> FxHashMap> { - storage(ArenaCacheSelector<'tcx>) + arena_cache eval_always desc { "looking up supported target features" } } @@ -2064,7 +2064,7 @@ rustc_queries! { /// all of the cases that the normal `ty::Ty`-based wfcheck does. This is fine, /// because the `ty::Ty`-based wfcheck is always run. query diagnostic_hir_wf_check(key: (ty::Predicate<'tcx>, traits::WellFormedLoc)) -> Option> { - storage(ArenaCacheSelector<'tcx>) + arena_cache eval_always no_hash desc { "performing HIR wf-checking for predicate {:?} at item {:?}", key.0, key.1 } @@ -2074,13 +2074,13 @@ rustc_queries! { /// The list of backend features computed from CLI flags (`-Ctarget-cpu`, `-Ctarget-feature`, /// `--target` and similar). query global_backend_features(_: ()) -> Vec { - storage(ArenaCacheSelector<'tcx>) + arena_cache eval_always desc { "computing the backend features for CLI flags" } } query generator_diagnostic_data(key: DefId) -> Option> { - storage(ArenaCacheSelector<'tcx>) + arena_cache desc { |tcx| "looking up generator diagnostic data of `{}`", tcx.def_path_str(key) } separate_provide_extern } diff --git a/compiler/rustc_middle/src/ty/query.rs b/compiler/rustc_middle/src/ty/query.rs index 5665bb866d46c..a300a8df23d28 100644 --- a/compiler/rustc_middle/src/ty/query.rs +++ b/compiler/rustc_middle/src/ty/query.rs @@ -121,8 +121,8 @@ macro_rules! query_storage { ([][$K:ty, $V:ty]) => { >::Cache }; - ([(storage $ty:ty) $($rest:tt)*][$K:ty, $V:ty]) => { - <$ty as CacheSelector<$K, $V>>::Cache + ([(arena_cache) $($rest:tt)*][$K:ty, $V:ty]) => { + as CacheSelector<$K, $V>>::Cache }; ([$other:tt $($modifiers:tt)*][$($args:tt)*]) => { query_storage!([$($modifiers)*][$($args)*]) From b7c9687d2e2504de0e0047b19fec00c54efc08a7 Mon Sep 17 00:00:00 2001 From: Joshua Nelson Date: Thu, 1 Sep 2022 21:21:29 -0500 Subject: [PATCH 2/8] Simplify the implementation of `rustc_queries` --- compiler/rustc_macros/src/query.rs | 53 ++++++++++-------------------- 1 file changed, 18 insertions(+), 35 deletions(-) diff --git a/compiler/rustc_macros/src/query.rs b/compiler/rustc_macros/src/query.rs index a99cba817d96e..77fb997dc48bd 100644 --- a/compiler/rustc_macros/src/query.rs +++ b/compiler/rustc_macros/src/query.rs @@ -344,43 +344,26 @@ pub fn rustc_queries(input: TokenStream) -> TokenStream { let mut attributes = Vec::new(); - // Pass on the fatal_cycle modifier - if let Some(fatal_cycle) = &modifiers.fatal_cycle { - attributes.push(quote! { (#fatal_cycle) }); - }; - // Pass on the arena modifier - if let Some(ref arena_cache) = modifiers.arena_cache { - attributes.push(quote! {span=> (#arena_cache) }); - }; - // Pass on the cycle_delay_bug modifier - if let Some(cycle_delay_bug) = &modifiers.cycle_delay_bug { - attributes.push(quote! { (#cycle_delay_bug) }); - }; - // Pass on the no_hash modifier - if let Some(no_hash) = &modifiers.no_hash { - attributes.push(quote! { (#no_hash) }); - }; - // Pass on the anon modifier - if let Some(anon) = &modifiers.anon { - attributes.push(quote! { (#anon) }); - }; - // Pass on the eval_always modifier - if let Some(eval_always) = &modifiers.eval_always { - attributes.push(quote! { (#eval_always) }); - }; - // Pass on the depth_limit modifier - if let Some(depth_limit) = &modifiers.depth_limit { - attributes.push(quote! { (#depth_limit) }); - }; - // Pass on the separate_provide_extern modifier - if let Some(separate_provide_extern) = &modifiers.separate_provide_extern { - attributes.push(quote! { (#separate_provide_extern) }); - } - // Pass on the remap_env_constness modifier - if let Some(remap_env_constness) = &modifiers.remap_env_constness { - attributes.push(quote! { (#remap_env_constness) }); + macro_rules! passthrough { + ( $( $modifier:ident ),+ $(,)? ) => { + $( if let Some($modifier) = &modifiers.$modifier { + attributes.push(quote! { (#$modifier) }); + }; )+ + } } + passthrough!( + fatal_cycle, + arena_cache, + cycle_delay_bug, + no_hash, + anon, + eval_always, + depth_limit, + separate_provide_extern, + remap_env_constness, + ); + // This uses the span of the query definition for the commas, // which can be important if we later encounter any ambiguity // errors with any of the numerous macro_rules! macros that From 8da754ef00488ffdb1259f104fd6084327b3bd45 Mon Sep 17 00:00:00 2001 From: Joshua Nelson Date: Thu, 1 Sep 2022 21:49:57 -0500 Subject: [PATCH 3/8] Don't use a custom disk loader for diagnostic_only_typeck This uses exactly the same types for query results as `typeck`, which doesn't have custom code. It's not clear to me why this code exists (it goes back even before queries used a proc macro), but it compiles fine without the custom loader. Remove it for simplicity. --- compiler/rustc_middle/src/query/mod.rs | 7 ------- 1 file changed, 7 deletions(-) diff --git a/compiler/rustc_middle/src/query/mod.rs b/compiler/rustc_middle/src/query/mod.rs index 8e62b05d5f69f..30e947258bad6 100644 --- a/compiler/rustc_middle/src/query/mod.rs +++ b/compiler/rustc_middle/src/query/mod.rs @@ -883,13 +883,6 @@ rustc_queries! { query diagnostic_only_typeck(key: LocalDefId) -> &'tcx ty::TypeckResults<'tcx> { desc { |tcx| "type-checking `{}`", tcx.def_path_str(key.to_def_id()) } cache_on_disk_if { true } - load_cached(tcx, id) { - let typeck_results: Option> = tcx - .on_disk_cache().as_ref() - .and_then(|c| c.try_load_query_result(*tcx, id)); - - typeck_results.map(|x| &*tcx.arena.alloc(x)) - } } query used_trait_imports(key: LocalDefId) -> &'tcx FxHashSet { From 112419c9f0b7f369df149e002429c85fc05f5e86 Mon Sep 17 00:00:00 2001 From: Joshua Nelson Date: Thu, 1 Sep 2022 21:54:06 -0500 Subject: [PATCH 4/8] Remove dead `load_cached` code in rustc_query --- compiler/rustc_macros/src/query.rs | 35 ++---------------------------- 1 file changed, 2 insertions(+), 33 deletions(-) diff --git a/compiler/rustc_macros/src/query.rs b/compiler/rustc_macros/src/query.rs index 77fb997dc48bd..50bd95834b1e5 100644 --- a/compiler/rustc_macros/src/query.rs +++ b/compiler/rustc_macros/src/query.rs @@ -91,9 +91,6 @@ struct QueryModifiers { /// Cache the query to disk if the `Block` returns true. cache: Option<(Option, Block)>, - /// Custom code to load the query from disk. - load_cached: Option<(Ident, Ident, Block)>, - /// A cycle error for this query aborting the compilation with a fatal error. fatal_cycle: Option, @@ -120,7 +117,6 @@ struct QueryModifiers { } fn parse_query_modifiers(input: ParseStream<'_>) -> Result { - let mut load_cached = None; let mut arena_cache = None; let mut cache = None; let mut desc = None; @@ -173,16 +169,6 @@ fn parse_query_modifiers(input: ParseStream<'_>) -> Result { }; let block = input.parse()?; try_insert!(cache = (args, block)); - } else if modifier == "load_cached" { - // Parse a load_cached modifier like: - // `load_cached(tcx, id) { tcx.on_disk_cache.try_load_query_result(tcx, id) }` - let args; - parenthesized!(args in input); - let tcx = args.parse()?; - args.parse::()?; - let id = args.parse()?; - let block = input.parse()?; - try_insert!(load_cached = (tcx, id, block)); } else if modifier == "arena_cache" { try_insert!(arena_cache = modifier); } else if modifier == "fatal_cycle" { @@ -209,7 +195,6 @@ fn parse_query_modifiers(input: ParseStream<'_>) -> Result { return Err(input.error("no description provided")); }; Ok(QueryModifiers { - load_cached, arena_cache, cache, desc, @@ -259,20 +244,6 @@ fn add_query_description_impl(query: &Query, impls: &mut proc_macro2::TokenStrea // Find out if we should cache the query on disk let cache = if let Some((args, expr)) = modifiers.cache.as_ref() { - let try_load_from_disk = if let Some((tcx, id, block)) = modifiers.load_cached.as_ref() { - // Use custom code to load the query from disk - quote! { - const TRY_LOAD_FROM_DISK: Option, SerializedDepNodeIndex) -> Option> - = Some(|#tcx, #id| { #block }); - } - } else { - // Use the default code to load the query from disk - quote! { - const TRY_LOAD_FROM_DISK: Option, SerializedDepNodeIndex) -> Option> - = Some(|tcx, id| tcx.on_disk_cache().as_ref()?.try_load_query_result(*tcx, id)); - } - }; - let tcx = args.as_ref().map(|t| quote! { #t }).unwrap_or_else(|| quote! { _ }); // expr is a `Block`, meaning that `{ #expr }` gets expanded // to `{ { stmts... } }`, which triggers the `unused_braces` lint. @@ -283,12 +254,10 @@ fn add_query_description_impl(query: &Query, impls: &mut proc_macro2::TokenStrea #expr } - #try_load_from_disk + const TRY_LOAD_FROM_DISK: Option, SerializedDepNodeIndex) -> Option> + = Some(|tcx, id| tcx.on_disk_cache().as_ref()?.try_load_query_result(*tcx, id)); } } else { - if modifiers.load_cached.is_some() { - panic!("load_cached modifier on query `{}` without a cache modifier", name); - } quote! { #[inline] fn cache_on_disk(_: TyCtxt<'tcx>, _: &Self::Key) -> bool { From b164dbc2715aa15be5c9f363a00d71e0847b2e77 Mon Sep 17 00:00:00 2001 From: Joshua Nelson Date: Thu, 1 Sep 2022 22:20:17 -0500 Subject: [PATCH 5/8] Don't create a new `try_load_from_disk` closure for each query Instead, define a single function, parameterized only by the return type. --- compiler/rustc_macros/src/query.rs | 2 +- compiler/rustc_query_impl/src/lib.rs | 3 ++- compiler/rustc_query_impl/src/plumbing.rs | 24 +++++++++++++++++++++++ 3 files changed, 27 insertions(+), 2 deletions(-) diff --git a/compiler/rustc_macros/src/query.rs b/compiler/rustc_macros/src/query.rs index 50bd95834b1e5..55d23ba1c6722 100644 --- a/compiler/rustc_macros/src/query.rs +++ b/compiler/rustc_macros/src/query.rs @@ -255,7 +255,7 @@ fn add_query_description_impl(query: &Query, impls: &mut proc_macro2::TokenStrea } const TRY_LOAD_FROM_DISK: Option, SerializedDepNodeIndex) -> Option> - = Some(|tcx, id| tcx.on_disk_cache().as_ref()?.try_load_query_result(*tcx, id)); + = Some(crate::plumbing::try_load_from_disk::); } } else { quote! { diff --git a/compiler/rustc_query_impl/src/lib.rs b/compiler/rustc_query_impl/src/lib.rs index 26d397f70e0ce..8148f8e017c0d 100644 --- a/compiler/rustc_query_impl/src/lib.rs +++ b/compiler/rustc_query_impl/src/lib.rs @@ -17,7 +17,7 @@ extern crate rustc_middle; use rustc_data_structures::sync::AtomicU64; use rustc_middle::arena::Arena; -use rustc_middle::dep_graph::{self, DepKindStruct, SerializedDepNodeIndex}; +use rustc_middle::dep_graph::{self, DepKindStruct}; use rustc_middle::ty::query::{query_keys, query_storage, query_stored, query_values}; use rustc_middle::ty::query::{ExternProviders, Providers, QueryEngine}; use rustc_middle::ty::{self, TyCtxt}; @@ -34,6 +34,7 @@ pub use rustc_query_system::query::{deadlock, QueryContext}; mod keys; use keys::Key; +use rustc_query_system::dep_graph::SerializedDepNodeIndex; pub use rustc_query_system::query::QueryConfig; pub(crate) use rustc_query_system::query::{QueryDescription, QueryVTable}; diff --git a/compiler/rustc_query_impl/src/plumbing.rs b/compiler/rustc_query_impl/src/plumbing.rs index 1e375deb20d1d..48539d580c7ca 100644 --- a/compiler/rustc_query_impl/src/plumbing.rs +++ b/compiler/rustc_query_impl/src/plumbing.rs @@ -3,6 +3,7 @@ //! manage the caches, and so forth. use crate::keys::Key; +use crate::on_disk_cache::CacheDecoder; use crate::{on_disk_cache, Queries}; use rustc_data_structures::stable_hasher::{HashStable, StableHasher}; use rustc_data_structures::sync::Lock; @@ -19,6 +20,7 @@ use rustc_query_system::query::{ QuerySideEffects, QueryStackFrame, }; use rustc_query_system::Value; +use rustc_serialize::Decodable; use std::any::Any; use std::num::NonZeroU64; use thin_vec::ThinVec; @@ -253,6 +255,18 @@ macro_rules! get_provider { }; } +macro_rules! should_ever_cache_on_disk { + ([]) => {{ + None + }}; + ([(cache) $($rest:tt)*]) => {{ + Some($crate::plumbing::try_load_from_disk::) + }}; + ([$other:tt $($modifiers:tt)*]) => { + should_ever_cache_on_disk!([$($modifiers)*]) + }; +} + pub(crate) fn create_query_frame< 'tcx, K: Copy + Key + for<'a> HashStable>, @@ -313,6 +327,16 @@ where } } +pub(crate) fn try_load_from_disk<'tcx, V>( + tcx: QueryCtxt<'tcx>, + id: SerializedDepNodeIndex, +) -> Option +where + V: for<'a> Decodable>, +{ + tcx.on_disk_cache().as_ref()?.try_load_query_result(*tcx, id) +} + fn force_from_dep_node<'tcx, Q>(tcx: TyCtxt<'tcx>, dep_node: DepNode) -> bool where Q: QueryDescription>, From 7208bdee33460b9915e6b389b236d231d2ca3ffc Mon Sep 17 00:00:00 2001 From: Joshua Nelson Date: Thu, 1 Sep 2022 22:26:03 -0500 Subject: [PATCH 6/8] Remove `cache_on_disk` from `QueryVTable` This is not only simpler, but removes a generic function and unwrap. I have hope it will see compile time and bootstrap time improvements. --- compiler/rustc_query_impl/src/plumbing.rs | 3 +-- compiler/rustc_query_system/src/query/config.rs | 10 ++-------- compiler/rustc_query_system/src/query/plumbing.rs | 6 +++--- 3 files changed, 6 insertions(+), 13 deletions(-) diff --git a/compiler/rustc_query_impl/src/plumbing.rs b/compiler/rustc_query_impl/src/plumbing.rs index 48539d580c7ca..4ff3917e113da 100644 --- a/compiler/rustc_query_impl/src/plumbing.rs +++ b/compiler/rustc_query_impl/src/plumbing.rs @@ -442,8 +442,7 @@ macro_rules! define_queries { hash_result: hash_result!([$($modifiers)*]), handle_cycle_error: handle_cycle_error!([$($modifiers)*]), compute, - cache_on_disk, - try_load_from_disk: Self::TRY_LOAD_FROM_DISK, + try_load_from_disk: if cache_on_disk { Self::TRY_LOAD_FROM_DISK } else { None }, } } diff --git a/compiler/rustc_query_system/src/query/config.rs b/compiler/rustc_query_system/src/query/config.rs index c63e110a62e39..340deb8891576 100644 --- a/compiler/rustc_query_system/src/query/config.rs +++ b/compiler/rustc_query_system/src/query/config.rs @@ -25,11 +25,12 @@ pub struct QueryVTable { pub dep_kind: CTX::DepKind, pub eval_always: bool, pub depth_limit: bool, - pub cache_on_disk: bool, pub compute: fn(CTX::DepContext, K) -> V, pub hash_result: Option, &V) -> Fingerprint>, pub handle_cycle_error: HandleCycleError, + // NOTE: this is not quite the same as `Q::TRY_LOAD_FROM_DISK`; it can also be `None` if + // `cache_on_disk` returned false for this key. pub try_load_from_disk: Option Option>, } @@ -44,13 +45,6 @@ impl QueryVTable { pub(crate) fn compute(&self, tcx: CTX::DepContext, key: K) -> V { (self.compute)(tcx, key) } - - pub(crate) fn try_load_from_disk(&self, tcx: CTX, index: SerializedDepNodeIndex) -> Option { - self.try_load_from_disk - .expect("QueryDescription::load_from_disk() called for an unsupported query.")( - tcx, index, - ) - } } pub trait QueryDescription: QueryConfig { diff --git a/compiler/rustc_query_system/src/query/plumbing.rs b/compiler/rustc_query_system/src/query/plumbing.rs index e39e39860cbb8..8179a674afaec 100644 --- a/compiler/rustc_query_system/src/query/plumbing.rs +++ b/compiler/rustc_query_system/src/query/plumbing.rs @@ -488,14 +488,14 @@ where // First we try to load the result from the on-disk cache. // Some things are never cached on disk. - if query.cache_on_disk { + if let Some(try_load_from_disk) = query.try_load_from_disk { let prof_timer = tcx.dep_context().profiler().incr_cache_loading(); // The call to `with_query_deserialization` enforces that no new `DepNodes` // are created during deserialization. See the docs of that method for more // details. - let result = dep_graph - .with_query_deserialization(|| query.try_load_from_disk(tcx, prev_dep_node_index)); + let result = + dep_graph.with_query_deserialization(|| try_load_from_disk(tcx, prev_dep_node_index)); prof_timer.finish_with_query_invocation_id(dep_node_index.into()); From 9273782d559a342beb2443018f2f8fc873f53b79 Mon Sep 17 00:00:00 2001 From: Joshua Nelson Date: Mon, 5 Sep 2022 06:43:11 -0500 Subject: [PATCH 7/8] Move `TRY_LOAD_FROM_DISK` out of `rustc_queries` to `rustc_query_impl` We want to refer to `crate::plumbing::try_load_from_disk` in the const, but hard-coding it in rustc_queries, where we don't yet know the crate this macro will be called in, seems kind of hacky. Do it in query_impl instead. --- compiler/rustc_macros/src/query.rs | 9 ++++----- compiler/rustc_query_impl/src/plumbing.rs | 3 +++ 2 files changed, 7 insertions(+), 5 deletions(-) diff --git a/compiler/rustc_macros/src/query.rs b/compiler/rustc_macros/src/query.rs index 55d23ba1c6722..742ae964f5c70 100644 --- a/compiler/rustc_macros/src/query.rs +++ b/compiler/rustc_macros/src/query.rs @@ -253,9 +253,6 @@ fn add_query_description_impl(query: &Query, impls: &mut proc_macro2::TokenStrea fn cache_on_disk(#tcx: TyCtxt<'tcx>, #key: &Self::Key) -> bool { #expr } - - const TRY_LOAD_FROM_DISK: Option, SerializedDepNodeIndex) -> Option> - = Some(crate::plumbing::try_load_from_disk::); } } else { quote! { @@ -263,8 +260,6 @@ fn add_query_description_impl(query: &Query, impls: &mut proc_macro2::TokenStrea fn cache_on_disk(_: TyCtxt<'tcx>, _: &Self::Key) -> bool { false } - - const TRY_LOAD_FROM_DISK: Option, SerializedDepNodeIndex) -> Option> = None; } }; @@ -333,6 +328,10 @@ pub fn rustc_queries(input: TokenStream) -> TokenStream { remap_env_constness, ); + if modifiers.cache.is_some() { + attributes.push(quote! { (cache) }); + } + // This uses the span of the query definition for the commas, // which can be important if we later encounter any ambiguity // errors with any of the numerous macro_rules! macros that diff --git a/compiler/rustc_query_impl/src/plumbing.rs b/compiler/rustc_query_impl/src/plumbing.rs index 4ff3917e113da..e38de2a0e407a 100644 --- a/compiler/rustc_query_impl/src/plumbing.rs +++ b/compiler/rustc_query_impl/src/plumbing.rs @@ -412,6 +412,9 @@ macro_rules! define_queries { impl<'tcx> QueryDescription> for queries::$name<'tcx> { rustc_query_description! { $name } + const TRY_LOAD_FROM_DISK: Option, SerializedDepNodeIndex) -> Option> + = should_ever_cache_on_disk!([$($modifiers)*]); + type Cache = query_storage::$name<'tcx>; #[inline(always)] From 0a9d7dbca23bb05fab13c1dc75a87cdb2bf69ff5 Mon Sep 17 00:00:00 2001 From: Joshua Nelson Date: Tue, 6 Sep 2022 19:09:32 -0500 Subject: [PATCH 8/8] Remove unnecessary `TRY_LOAD_FROM_DISK` constant --- compiler/rustc_query_impl/src/lib.rs | 1 - compiler/rustc_query_impl/src/plumbing.rs | 5 +---- compiler/rustc_query_system/src/query/config.rs | 5 +---- 3 files changed, 2 insertions(+), 9 deletions(-) diff --git a/compiler/rustc_query_impl/src/lib.rs b/compiler/rustc_query_impl/src/lib.rs index 8148f8e017c0d..c87d26b3950a1 100644 --- a/compiler/rustc_query_impl/src/lib.rs +++ b/compiler/rustc_query_impl/src/lib.rs @@ -34,7 +34,6 @@ pub use rustc_query_system::query::{deadlock, QueryContext}; mod keys; use keys::Key; -use rustc_query_system::dep_graph::SerializedDepNodeIndex; pub use rustc_query_system::query::QueryConfig; pub(crate) use rustc_query_system::query::{QueryDescription, QueryVTable}; diff --git a/compiler/rustc_query_impl/src/plumbing.rs b/compiler/rustc_query_impl/src/plumbing.rs index e38de2a0e407a..6e0649cc47168 100644 --- a/compiler/rustc_query_impl/src/plumbing.rs +++ b/compiler/rustc_query_impl/src/plumbing.rs @@ -412,9 +412,6 @@ macro_rules! define_queries { impl<'tcx> QueryDescription> for queries::$name<'tcx> { rustc_query_description! { $name } - const TRY_LOAD_FROM_DISK: Option, SerializedDepNodeIndex) -> Option> - = should_ever_cache_on_disk!([$($modifiers)*]); - type Cache = query_storage::$name<'tcx>; #[inline(always)] @@ -445,7 +442,7 @@ macro_rules! define_queries { hash_result: hash_result!([$($modifiers)*]), handle_cycle_error: handle_cycle_error!([$($modifiers)*]), compute, - try_load_from_disk: if cache_on_disk { Self::TRY_LOAD_FROM_DISK } else { None }, + try_load_from_disk: if cache_on_disk { should_ever_cache_on_disk!([$($modifiers)*]) } else { None }, } } diff --git a/compiler/rustc_query_system/src/query/config.rs b/compiler/rustc_query_system/src/query/config.rs index 340deb8891576..c4549cc9eb411 100644 --- a/compiler/rustc_query_system/src/query/config.rs +++ b/compiler/rustc_query_system/src/query/config.rs @@ -29,8 +29,7 @@ pub struct QueryVTable { pub compute: fn(CTX::DepContext, K) -> V, pub hash_result: Option, &V) -> Fingerprint>, pub handle_cycle_error: HandleCycleError, - // NOTE: this is not quite the same as `Q::TRY_LOAD_FROM_DISK`; it can also be `None` if - // `cache_on_disk` returned false for this key. + // NOTE: this is also `None` if `cache_on_disk()` returns false, not just if it's unsupported by the query pub try_load_from_disk: Option Option>, } @@ -48,8 +47,6 @@ impl QueryVTable { } pub trait QueryDescription: QueryConfig { - const TRY_LOAD_FROM_DISK: Option Option>; - type Cache: QueryCache; fn describe(tcx: CTX, key: Self::Key) -> String;