Unrolled build for #154698

Rollup merge of #154698 - nnethercote:rm-desc-cache-modules-2, r=Zalathar

Improve workings of the `desc` query modifier and custom query cycle handlers

This PR does two things involving query modifiers.
- It changes how the `desc` query modifier is handled, eliminating the generated `_description_fns` module.
- It adds a new `handle_cycle_error` query modifier for queries that do custom cycle handling.

Details are in the individual commits.

r? @Zalathar

Author: rust-timer

compiler/rustc_macros/src/query.rs

@@ -128,6 +128,7 @@ impl<T: Parse> Parse for List<T> {
 }
 
 struct Desc {
+    // This ident is always `desc` but we need it for its span, for `crate::query::modifiers`.
     modifier: Ident,
     expr_list: Punctuated<Expr, Token![,]>,
 }
@@ -141,22 +142,26 @@ struct QueryModifiers {
     desc: Desc,
     eval_always: Option<Ident>,
     feedable: Option<Ident>,
+    handle_cycle_error: Option<Ident>,
     no_force: Option<Ident>,
     no_hash: Option<Ident>,
     separate_provide_extern: Option<Ident>,
     // tidy-alphabetical-end
 }
 
 fn parse_query_modifiers(input: ParseStream<'_>) -> Result<QueryModifiers> {
+    // tidy-alphabetical-start
     let mut arena_cache = None;
     let mut cache_on_disk = None;
+    let mut depth_limit = None;
     let mut desc = None;
+    let mut eval_always = None;
+    let mut feedable = None;
+    let mut handle_cycle_error = None;
     let mut no_force = None;
     let mut no_hash = None;
-    let mut eval_always = None;
-    let mut depth_limit = None;
     let mut separate_provide_extern = None;
-    let mut feedable = None;
+    // tidy-alphabetical-end
 
     while !input.is_empty() {
         let modifier: Ident = input.parse()?;
@@ -170,29 +175,31 @@ fn parse_query_modifiers(input: ParseStream<'_>) -> Result<QueryModifiers> {
             };
         }
 
-        if modifier == "desc" {
+        if modifier == "arena_cache" {
+            try_insert!(arena_cache = modifier);
+        } else if modifier == "cache_on_disk" {
+            try_insert!(cache_on_disk = modifier);
+        } else if modifier == "depth_limit" {
+            try_insert!(depth_limit = modifier);
+        } else if modifier == "desc" {
             // Parse a description modifier like:
             // `desc { "foo {}", tcx.item_path(key) }`
             let attr_content;
             braced!(attr_content in input);
             let expr_list = attr_content.parse_terminated(Expr::parse, Token![,])?;
             try_insert!(desc = Desc { modifier, expr_list });
-        } else if modifier == "cache_on_disk" {
-            try_insert!(cache_on_disk = modifier);
-        } else if modifier == "arena_cache" {
-            try_insert!(arena_cache = modifier);
+        } else if modifier == "eval_always" {
+            try_insert!(eval_always = modifier);
+        } else if modifier == "feedable" {
+            try_insert!(feedable = modifier);
+        } else if modifier == "handle_cycle_error" {
+            try_insert!(handle_cycle_error = modifier);
         } else if modifier == "no_force" {
             try_insert!(no_force = modifier);
         } else if modifier == "no_hash" {
             try_insert!(no_hash = modifier);
-        } else if modifier == "eval_always" {
-            try_insert!(eval_always = modifier);
-        } else if modifier == "depth_limit" {
-            try_insert!(depth_limit = modifier);
         } else if modifier == "separate_provide_extern" {
             try_insert!(separate_provide_extern = modifier);
-        } else if modifier == "feedable" {
-            try_insert!(feedable = modifier);
         } else {
             return Err(Error::new(modifier.span(), "unknown query modifier"));
         }
@@ -201,15 +208,18 @@ fn parse_query_modifiers(input: ParseStream<'_>) -> Result<QueryModifiers> {
         return Err(input.error("no description provided"));
     };
     Ok(QueryModifiers {
+        // tidy-alphabetical-start
         arena_cache,
         cache_on_disk,
+        depth_limit,
         desc,
+        eval_always,
+        feedable,
+        handle_cycle_error,
         no_force,
         no_hash,
-        eval_always,
-        depth_limit,
         separate_provide_extern,
-        feedable,
+        // tidy-alphabetical-end
     })
 }
 
@@ -238,24 +248,40 @@ fn make_modifiers_stream(query: &Query) -> proc_macro2::TokenStream {
         arena_cache,
         cache_on_disk,
         depth_limit,
-        desc: _,
+        desc,
         eval_always,
         feedable,
+        handle_cycle_error,
         no_force,
         no_hash,
         separate_provide_extern,
         // tidy-alphabetical-end
     } = &query.modifiers;
 
+    // tidy-alphabetical-start
     let arena_cache = arena_cache.is_some();
     let cache_on_disk = cache_on_disk.is_some();
     let depth_limit = depth_limit.is_some();
+    let desc = {
+        // Put a description closure in the `desc` modifier.
+        let key_pat = &query.key_pat;
+        let key_ty = &query.key_ty;
+        let desc_expr_list = &desc.expr_list;
+        quote! {
+            {
+                #[allow(unused_variables)]
+                |tcx: TyCtxt<'tcx>, #key_pat: #key_ty| format!(#desc_expr_list)
+            }
+        }
+    };
     let eval_always = eval_always.is_some();
     let feedable = feedable.is_some();
+    let handle_cycle_error = handle_cycle_error.is_some();
     let no_force = no_force.is_some();
     let no_hash = no_hash.is_some();
     let returns_error_guaranteed = returns_error_guaranteed(&query.return_ty);
     let separate_provide_extern = separate_provide_extern.is_some();
+    // tidy-alphabetical-end
 
     // Giving an input span to the modifier names in the modifier list seems
     // to give slightly more helpful errors when one of the callback macros
@@ -268,8 +294,10 @@ fn make_modifiers_stream(query: &Query) -> proc_macro2::TokenStream {
         arena_cache: #arena_cache,
         cache_on_disk: #cache_on_disk,
         depth_limit: #depth_limit,
+        desc: #desc,
         eval_always: #eval_always,
         feedable: #feedable,
+        handle_cycle_error: #handle_cycle_error,
         no_force: #no_force,
         no_hash: #no_hash,
         returns_error_guaranteed: #returns_error_guaranteed,
@@ -305,37 +333,6 @@ fn doc_comment_from_desc(list: &Punctuated<Expr, token::Comma>) -> Result<Attrib
     Ok(parse_quote! { #[doc = #doc_string] })
 }
 
-/// Contains token streams that are used to accumulate per-query helper
-/// functions, to be used by the final output of `rustc_queries!`.
-///
-/// Helper items typically have the same name as the query they relate to,
-/// and expect to be interpolated into a dedicated module.
-#[derive(Default)]
-struct HelperTokenStreams {
-    description_fns_stream: proc_macro2::TokenStream,
-}
-
-fn make_helpers_for_query(query: &Query, streams: &mut HelperTokenStreams) {
-    let Query { name, key_pat, key_ty, modifiers, .. } = &query;
-
-    // Replace span for `name` to make rust-analyzer ignore it.
-    let mut erased_name = name.clone();
-    erased_name.set_span(Span::call_site());
-
-    let Desc { expr_list, .. } = &modifiers.desc;
-
-    let desc = quote! {
-        #[allow(unused_variables)]
-        pub fn #erased_name<'tcx>(tcx: TyCtxt<'tcx>, #key_pat: #key_ty) -> String {
-            format!(#expr_list)
-        }
-    };
-
-    streams.description_fns_stream.extend(quote! {
-        #desc
-    });
-}
-
 /// Add hints for rust-analyzer
 fn add_to_analyzer_stream(query: &Query, analyzer_stream: &mut proc_macro2::TokenStream) {
     // Add links to relevant modifiers
@@ -366,8 +363,10 @@ fn add_to_analyzer_stream(query: &Query, analyzer_stream: &mut proc_macro2::Toke
         arena_cache,
         cache_on_disk,
         depth_limit,
+        // `desc` is handled above
         eval_always,
         feedable,
+        handle_cycle_error,
         no_force,
         no_hash,
         separate_provide_extern,
@@ -409,7 +408,6 @@ pub(super) fn rustc_queries(input: TokenStream) -> TokenStream {
 
     let mut query_stream = quote! {};
     let mut non_query_stream = quote! {};
-    let mut helpers = HelperTokenStreams::default();
     let mut analyzer_stream = quote! {};
     let mut errors = quote! {};
 
@@ -462,11 +460,8 @@ pub(super) fn rustc_queries(input: TokenStream) -> TokenStream {
         }
 
         add_to_analyzer_stream(&query, &mut analyzer_stream);
-        make_helpers_for_query(&query, &mut helpers);
     }
 
-    let HelperTokenStreams { description_fns_stream } = helpers;
-
     TokenStream::from(quote! {
         /// Higher-order macro that invokes the specified macro with (a) a list of all query
         /// signatures (including modifiers), and (b) a list of non-query names. This allows
@@ -490,17 +485,6 @@ pub(super) fn rustc_queries(input: TokenStream) -> TokenStream {
             #analyzer_stream
         }
 
-        /// Functions that format a human-readable description of each query
-        /// and its key, as specified by the `desc` query modifier.
-        ///
-        /// (The leading `_` avoids collisions with actual query names when
-        /// expanded in `rustc_middle::queries`, and makes this macro-generated
-        /// module easier to search for.)
-        pub mod _description_fns {
-            use super::*;
-            #description_fns_stream
-        }
-
         #errors
     })
 }

compiler/rustc_middle/src/queries.rs

@@ -583,6 +583,7 @@ rustc_queries! {
     // messages about cycles that then abort.)
     query check_representability(key: LocalDefId) {
         desc { "checking if `{}` is representable", tcx.def_path_str(key) }
+        handle_cycle_error
         // We don't want recursive representability calls to be forced with
         // incremental compilation because, if a cycle occurs, we need the
         // entire cycle to be in memory for diagnostics.
@@ -593,6 +594,7 @@ rustc_queries! {
     /// details, particularly on the modifiers.
     query check_representability_adt_ty(key: Ty<'tcx>) {
         desc { "checking if `{}` is representable", key }
+        handle_cycle_error
         no_force
     }
 
@@ -1032,6 +1034,7 @@ rustc_queries! {
     query variances_of(def_id: DefId) -> &'tcx [ty::Variance] {
         desc { "computing the variances of `{}`", tcx.def_path_str(def_id) }
         cache_on_disk
+        handle_cycle_error
         separate_provide_extern
     }
 
@@ -1164,6 +1167,7 @@ rustc_queries! {
     query fn_sig(key: DefId) -> ty::EarlyBinder<'tcx, ty::PolyFnSig<'tcx>> {
         desc { "computing function signature of `{}`", tcx.def_path_str(key) }
         cache_on_disk
+        handle_cycle_error
         separate_provide_extern
     }
 
@@ -1756,6 +1760,7 @@ rustc_queries! {
     ) -> Result<ty::layout::TyAndLayout<'tcx>, &'tcx ty::layout::LayoutError<'tcx>> {
         depth_limit
         desc { "computing layout of `{}`", key.value }
+        handle_cycle_error
     }
 
     /// Compute a `FnAbi` suitable for indirect calls, i.e. to `fn` pointers.

compiler/rustc_middle/src/query/modifiers.rs

@@ -58,6 +58,14 @@ pub(crate) struct eval_always;
 /// Generate a `feed` method to set the query's value from another query.
 pub(crate) struct feedable;
 
+/// # `handle_cycle_error` query modifier
+///
+/// The default behaviour for a query cycle is to emit a cycle error and halt
+/// compilation. Queries with this modifier will instead use a custom handler,
+/// which must be provided at `rustc_query_impl::handle_cycle_error::$name`,
+/// where `$name` is the query name.
+pub(crate) struct handle_cycle_error;
+
 /// # `no_force` query modifier
 ///
 /// Dep nodes of queries with this modifier will never be "forced" when trying

compiler/rustc_middle/src/query/plumbing.rs

@@ -301,8 +301,10 @@ macro_rules! define_callbacks {
                     arena_cache: $arena_cache:literal,
                     cache_on_disk: $cache_on_disk:literal,
                     depth_limit: $depth_limit:literal,
+                    desc: $desc:expr,
                     eval_always: $eval_always:literal,
                     feedable: $feedable:literal,
+                    handle_cycle_error: $handle_cycle_error:literal,
                     no_force: $no_force:literal,
                     no_hash: $no_hash:literal,
                     returns_error_guaranteed: $returns_error_guaranteed:literal,
@@ -435,8 +437,7 @@ macro_rules! define_callbacks {
             pub fn description(&self, tcx: TyCtxt<'tcx>) -> String {
                 let (name, description) = ty::print::with_no_queries!(match self {
                     $(
-                        TaggedQueryKey::$name(key) =>
-                            (stringify!($name), _description_fns::$name(tcx, *key)),
+                        TaggedQueryKey::$name(key) => (stringify!($name), ($desc)(tcx, *key)),
                     )*
                 });
                 if tcx.sess.verbose_internals() {

compiler/rustc_query_impl/src/dep_kind_vtables.rs

@@ -135,8 +135,10 @@ macro_rules! define_dep_kind_vtables {
                     arena_cache: $arena_cache:literal,
                     cache_on_disk: $cache_on_disk:literal,
                     depth_limit: $depth_limit:literal,
+                    desc: $desc:expr,
                     eval_always: $eval_always:literal,
                     feedable: $feedable:literal,
+                    handle_cycle_error: $handle_cycle_error:literal,
                     no_force: $no_force:literal,
                     no_hash: $no_hash:literal,
                     returns_error_guaranteed: $returns_error_guaranteed:literal,