Auto merge of rust-lang#102895 - Nilstrieb:query-cleanups, r=cjgillot

Get rid of `rustc_query_description!`

**I am not entirely sure whether this is an improvement and would like to get your feedback on it.**

Helps with rust-lang#96524.

Queries can provide an arbitrary expression for their description and their caching behavior. Before, these expressions where stored in a `rustc_query_description` macro emitted by the `rustc_queries` macro, and then used in `rustc_query_impl` to fill out the methods for the `QueryDescription` trait.

Instead, we now emit two new modules from `rustc_queries` containing the functions with the expressions. `rustc_query_impl` calls these functions now instead of invoking the macro.

Since we are now defining some of the functions in `rustc_middle::query`, we now need all the imports for the key types mthere as well.

r? `@cjgillot`

Author: bors

compiler/rustc_macros/src/query.rs

@@ -237,27 +237,32 @@ fn doc_comment_from_desc(list: &Punctuated<Expr, token::Comma>) -> Result<Attrib
 }
 
 /// Add the impl of QueryDescription for the query to `impls` if one is requested
-fn add_query_description_impl(query: &Query, impls: &mut proc_macro2::TokenStream) {
-    let name = &query.name;
-    let key = &query.key;
-    let modifiers = &query.modifiers;
+fn add_query_desc_cached_impl(
+    query: &Query,
+    descs: &mut proc_macro2::TokenStream,
+    cached: &mut proc_macro2::TokenStream,
+) {
+    let Query { name, key, modifiers, .. } = &query;
 
     // Find out if we should cache the query on disk
     let cache = if let Some((args, expr)) = modifiers.cache.as_ref() {
         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.
+        // we're taking `key` by reference, but some rustc types usually prefer being passed by value
         quote! {
-            #[allow(unused_variables, unused_braces)]
+            #[allow(unused_variables, unused_braces, rustc::pass_by_value)]
             #[inline]
-            fn cache_on_disk(#tcx: TyCtxt<'tcx>, #key: &Self::Key) -> bool {
+            pub fn #name<'tcx>(#tcx: TyCtxt<'tcx>, #key: &crate::ty::query::query_keys::#name<'tcx>) -> bool {
                 #expr
             }
         }
     } else {
         quote! {
+            // we're taking `key` by reference, but some rustc types usually prefer being passed by value
+            #[allow(rustc::pass_by_value)]
             #[inline]
-            fn cache_on_disk(_: TyCtxt<'tcx>, _: &Self::Key) -> bool {
+            pub fn #name<'tcx>(_: TyCtxt<'tcx>, _: &crate::ty::query::query_keys::#name<'tcx>) -> bool {
                 false
             }
         }
@@ -268,19 +273,20 @@ fn add_query_description_impl(query: &Query, impls: &mut proc_macro2::TokenStrea
 
     let desc = quote! {
         #[allow(unused_variables)]
-        fn describe(tcx: QueryCtxt<'tcx>, key: Self::Key) -> String {
-            let (#tcx, #key) = (*tcx, key);
+        pub fn #name<'tcx>(tcx: TyCtxt<'tcx>, key: crate::ty::query::query_keys::#name<'tcx>) -> String {
+            let (#tcx, #key) = (tcx, key);
             ::rustc_middle::ty::print::with_no_trimmed_paths!(
                 format!(#desc)
             )
         }
     };
 
-    impls.extend(quote! {
-        (#name) => {
-            #desc
-            #cache
-        };
+    descs.extend(quote! {
+        #desc
+    });
+
+    cached.extend(quote! {
+        #cache
     });
 }
 
@@ -289,7 +295,7 @@ pub fn rustc_queries(input: TokenStream) -> TokenStream {
 
     let mut query_stream = quote! {};
     let mut query_description_stream = quote! {};
-    let mut cached_queries = quote! {};
+    let mut query_cached_stream = quote! {};
 
     for query in queries.0 {
         let Query { name, arg, modifiers, .. } = &query;
@@ -299,12 +305,6 @@ pub fn rustc_queries(input: TokenStream) -> TokenStream {
             _ => quote! { #result_full },
         };
 
-        if modifiers.cache.is_some() {
-            cached_queries.extend(quote! {
-                #name,
-            });
-        }
-
         let mut attributes = Vec::new();
 
         macro_rules! passthrough {
@@ -350,7 +350,7 @@ pub fn rustc_queries(input: TokenStream) -> TokenStream {
             [#attribute_stream] fn #name(#arg) #result,
         });
 
-        add_query_description_impl(&query, &mut query_description_stream);
+        add_query_desc_cached_impl(&query, &mut query_description_stream, &mut query_cached_stream);
     }
 
     TokenStream::from(quote! {
@@ -364,9 +364,13 @@ pub fn rustc_queries(input: TokenStream) -> TokenStream {
             }
         }
 
-        #[macro_export]
-        macro_rules! rustc_query_description {
+        pub mod descs {
+            use super::*;
             #query_description_stream
         }
+        pub mod cached {
+            use super::*;
+            #query_cached_stream
+        }
     })
 }

compiler/rustc_middle/src/query/mod.rs

@@ -4,6 +4,9 @@
 //! ["Queries: demand-driven compilation"](https://rustc-dev-guide.rust-lang.org/query.html).
 //! This chapter includes instructions for adding new queries.
 
+use crate::ty::{self, print::describe_as_module, TyCtxt};
+use rustc_span::def_id::LOCAL_CRATE;
+
 // Each of these queries corresponds to a function pointer field in the
 // `Providers` struct for requesting a value of that type, and a method
 // on `tcx: TyCtxt` (and `tcx.at(span)`) for doing that request in a way
@@ -1214,7 +1217,7 @@ rustc_queries! {
         desc { |tcx| "finding all vtable entries for trait {}", tcx.def_path_str(key.def_id()) }
     }
 
-    query vtable_trait_upcasting_coercion_new_vptr_slot(key: (ty::Ty<'tcx>, ty::Ty<'tcx>)) -> Option<usize> {
+    query vtable_trait_upcasting_coercion_new_vptr_slot(key: (Ty<'tcx>, Ty<'tcx>)) -> Option<usize> {
         desc { |tcx| "finding the slot within vtable for trait object {} vtable ptr during trait upcasting coercion from {} vtable",
             key.1, key.0 }
     }

compiler/rustc_middle/src/ty/print/mod.rs

@@ -3,7 +3,7 @@ use crate::ty::{self, DefIdTree, Ty, TyCtxt};
 
 use rustc_data_structures::fx::FxHashSet;
 use rustc_data_structures::sso::SsoHashSet;
-use rustc_hir::def_id::{CrateNum, DefId};
+use rustc_hir::def_id::{CrateNum, DefId, LocalDefId};
 use rustc_hir::definitions::{DefPathData, DisambiguatedDefPathData};
 
 // `pretty` is a separate module only for organization.
@@ -325,3 +325,12 @@ impl<'tcx, P: Printer<'tcx>> Print<'tcx, P> for ty::Const<'tcx> {
         cx.print_const(*self)
     }
 }
+
+// This is only used by query descriptions
+pub fn describe_as_module(def_id: LocalDefId, tcx: TyCtxt<'_>) -> String {
+    if def_id.is_top_level_module() {
+        "top-level module".to_string()
+    } else {
+        format!("module `{}`", tcx.def_path_str(def_id.to_def_id()))
+    }
+}

compiler/rustc_query_impl/src/lib.rs

@@ -22,8 +22,7 @@ use rustc_middle::arena::Arena;
 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};
-use rustc_span::def_id::{LocalDefId, LOCAL_CRATE};
+use rustc_middle::ty::TyCtxt;
 use rustc_span::Span;
 
 #[macro_use]
@@ -45,14 +44,6 @@ pub use on_disk_cache::OnDiskCache;
 mod profiling_support;
 pub use self::profiling_support::alloc_self_profile_query_strings;
 
-fn describe_as_module(def_id: LocalDefId, tcx: TyCtxt<'_>) -> String {
-    if def_id.is_top_level_module() {
-        "top-level module".to_string()
-    } else {
-        format!("module `{}`", tcx.def_path_str(def_id.to_def_id()))
-    }
-}
-
 rustc_query_append! { define_queries! }
 
 impl<'tcx> Queries<'tcx> {

compiler/rustc_query_impl/src/plumbing.rs

@@ -298,7 +298,7 @@ pub(crate) fn create_query_frame<
     K: Copy + Key + for<'a> HashStable<StableHashingContext<'a>>,
 >(
     tcx: QueryCtxt<'tcx>,
-    do_describe: fn(QueryCtxt<'tcx>, K) -> String,
+    do_describe: fn(TyCtxt<'tcx>, K) -> String,
     key: K,
     kind: DepKind,
     name: &'static str,
@@ -307,7 +307,7 @@ pub(crate) fn create_query_frame<
     // Showing visible path instead of any path is not that important in production.
     let description = ty::print::with_no_visible_paths!(
         // Force filename-line mode to avoid invoking `type_of` query.
-        ty::print::with_forced_impl_filename_line!(do_describe(tcx, key))
+        ty::print::with_forced_impl_filename_line!(do_describe(tcx.tcx, key))
     );
     let description =
         if tcx.sess.verbose() { format!("{} [{}]", description, name) } else { description };
@@ -466,7 +466,10 @@ macro_rules! define_queries {
         }
 
         impl<'tcx> QueryDescription<QueryCtxt<'tcx>> for queries::$name<'tcx> {
-            rustc_query_description! { $name }
+            #[inline]
+            fn cache_on_disk(tcx: TyCtxt<'tcx>, key: &Self::Key) -> bool {
+                ::rustc_middle::query::cached::$name(tcx, key)
+            }
 
             type Cache = query_storage::$name<'tcx>;
 
@@ -576,7 +579,7 @@ macro_rules! define_queries {
             use rustc_middle::ty::TyCtxt;
             use $crate::plumbing::{QueryStruct, QueryCtxt};
             use $crate::profiling_support::QueryKeyStringCache;
-            use rustc_query_system::query::{QueryDescription, QueryMap};
+            use rustc_query_system::query::QueryMap;
 
             pub(super) const fn dummy_query_struct<'tcx>() -> QueryStruct<'tcx> {
                 fn noop_try_collect_active_jobs(_: QueryCtxt<'_>, _: &mut QueryMap) -> Option<()> {
@@ -603,7 +606,7 @@ macro_rules! define_queries {
                     let make_query = |tcx, key| {
                         let kind = rustc_middle::dep_graph::DepKind::$name;
                         let name = stringify!($name);
-                        $crate::plumbing::create_query_frame(tcx, super::queries::$name::describe, key, kind, name)
+                        $crate::plumbing::create_query_frame(tcx, rustc_middle::query::descs::$name, key, kind, name)
                     };
                     tcx.queries.$name.try_collect_active_jobs(
                         tcx,

compiler/rustc_query_system/src/query/config.rs

@@ -49,8 +49,6 @@ impl<CTX: QueryContext, K, V> QueryVTable<CTX, K, V> {
 pub trait QueryDescription<CTX: QueryContext>: QueryConfig {
     type Cache: QueryCache<Key = Self::Key, Stored = Self::Stored, Value = Self::Value>;
 
-    fn describe(tcx: CTX, key: Self::Key) -> String;
-
     // Don't use this method to access query results, instead use the methods on TyCtxt
     fn query_state<'a>(tcx: CTX) -> &'a QueryState<Self::Key>
     where