Rollup merge of rust-lang#153760 - Zalathar:query-impl, r=nnethercote

Move and expand the big `rustc_query_impl` macro into a physical `query_impl.rs`

While looking through rust-lang#153588, I came up with a related but different change that I think resolves a lot of tension in the current module arrangement.

The core idea is that if we both define and expand the big macro in the same physical module `rustc_query_impl::query_impl`, then we no longer need to worry about where `mod query_impl` should be declared, or where its imports should go, because those questions now have simple and obvious answers.

The second commit follows up with some more changes inspired by rust-lang#153588. Those particular follow-ups are not essential to the main idea of this PR.

r? nnethercote

Author: Zalathar

compiler/rustc_query_impl/src/execution.rs

@@ -18,9 +18,9 @@ use rustc_span::{DUMMY_SP, Span};
 use tracing::warn;
 
 use crate::dep_graph::{DepNode, DepNodeIndex};
-use crate::for_each_query_vtable;
 use crate::job::{QueryJobInfo, QueryJobMap, find_cycle_in_stack, report_cycle};
 use crate::plumbing::{current_query_job, next_job_id, start_query};
+use crate::query_impl::for_each_query_vtable;
 
 #[inline]
 fn equivalent_key<K: Eq, V>(k: K) -> impl Fn(&(K, V)) -> bool {

compiler/rustc_query_impl/src/lib.rs

@@ -10,26 +10,24 @@
 
 use rustc_data_structures::sync::AtomicU64;
 use rustc_middle::dep_graph;
-use rustc_middle::queries::{self, ExternProviders, Providers, TaggedQueryKey};
+use rustc_middle::queries::{ExternProviders, Providers};
+use rustc_middle::query::QueryCache;
 use rustc_middle::query::on_disk_cache::OnDiskCache;
 use rustc_middle::query::plumbing::{QuerySystem, QueryVTable};
-use rustc_middle::query::{AsLocalQueryKey, QueryCache, QueryMode};
 use rustc_middle::ty::TyCtxt;
-use rustc_span::Span;
 
 pub use crate::dep_kind_vtables::make_dep_kind_vtables;
 pub use crate::execution::{CollectActiveJobsKind, collect_active_jobs_from_all_queries};
 pub use crate::job::{QueryJobMap, break_query_cycles, print_query_stack};
 
-#[macro_use]
-mod plumbing;
-
 mod dep_kind_vtables;
 mod error;
 mod execution;
 mod from_cycle_error;
 mod job;
+mod plumbing;
 mod profiling_support;
+mod query_impl;
 
 /// Trait that knows how to look up the [`QueryVTable`] for a particular query.
 ///
@@ -51,7 +49,7 @@ pub fn query_system<'tcx>(
     on_disk_cache: Option<OnDiskCache>,
     incremental: bool,
 ) -> QuerySystem<'tcx> {
-    let mut query_vtables = make_query_vtables(incremental);
+    let mut query_vtables = query_impl::make_query_vtables(incremental);
     from_cycle_error::specialize_query_vtables(&mut query_vtables);
     QuerySystem {
         arenas: Default::default(),
@@ -63,8 +61,6 @@ pub fn query_system<'tcx>(
     }
 }
 
-rustc_middle::rustc_with_all_queries! { define_queries! }
-
 pub fn provide(providers: &mut rustc_middle::util::Providers) {
     providers.hooks.alloc_self_profile_query_strings =
         profiling_support::alloc_self_profile_query_strings;

compiler/rustc_query_impl/src/plumbing.rs

@@ -1,7 +1,3 @@
-//! The implementation of the query system itself. This defines the macros that
-//! generate the actual methods on tcx which find and execute the provider,
-//! manage the caches, and so forth.
-
 use std::num::NonZero;
 
 use rustc_data_structures::sync::{DynSend, DynSync};
@@ -29,10 +25,8 @@ use rustc_span::def_id::LOCAL_CRATE;
 use crate::error::{QueryOverflow, QueryOverflowNote};
 use crate::execution::{all_inactive, force_query};
 use crate::job::find_dep_kind_root;
-use crate::{
-    CollectActiveJobsKind, GetQueryVTable, collect_active_jobs_from_all_queries,
-    for_each_query_vtable,
-};
+use crate::query_impl::for_each_query_vtable;
+use crate::{CollectActiveJobsKind, GetQueryVTable, collect_active_jobs_from_all_queries};
 
 fn depth_limit_error<'tcx>(tcx: TyCtxt<'tcx>, job: QueryJobId) {
     let job_map = collect_active_jobs_from_all_queries(tcx, CollectActiveJobsKind::Full);
@@ -283,256 +277,3 @@ pub(crate) fn force_from_dep_node_inner<'tcx, Q: GetQueryVTable<'tcx>>(
         false
     }
 }
-
-macro_rules! define_queries {
-    (
-        // Note: `$K` and `$V` are unused but present so this can be called by
-        // `rustc_with_all_queries`.
-        queries {
-            $(
-                $(#[$attr:meta])*
-                fn $name:ident($K:ty) -> $V:ty
-                {
-                    // Search for (QMODLIST) to find all occurrences of this query modifier list.
-                    anon: $anon:literal,
-                    arena_cache: $arena_cache:literal,
-                    cache_on_disk: $cache_on_disk:literal,
-                    depth_limit: $depth_limit:literal,
-                    eval_always: $eval_always:literal,
-                    feedable: $feedable:literal,
-                    no_hash: $no_hash:literal,
-                    returns_error_guaranteed: $returns_error_guaranteed:literal,
-                    separate_provide_extern: $separate_provide_extern:literal,
-                }
-            )*
-        }
-        // Non-queries are unused here.
-        non_queries { $($_:tt)* }
-    ) => {
-        pub(crate) mod query_impl { $(pub(crate) mod $name {
-            use super::super::*;
-            use ::rustc_middle::query::erase::{self, Erased};
-
-            // It seems to be important that every query has its own monomorphic
-            // copy of `execute_query_incr` and `execute_query_non_incr`.
-            // Trying to inline these wrapper functions into their generic
-            // "inner" helpers tends to break `tests/run-make/short-ice`.
-
-            pub(crate) mod execute_query_incr {
-                use super::*;
-
-                // Adding `__rust_end_short_backtrace` marker to backtraces so that we emit the frames
-                // when `RUST_BACKTRACE=1`, add a new mod with `$name` here is to allow duplicate naming
-                #[inline(never)]
-                pub(crate) fn __rust_end_short_backtrace<'tcx>(
-                    tcx: TyCtxt<'tcx>,
-                    span: Span,
-                    key: queries::$name::Key<'tcx>,
-                    mode: QueryMode,
-                ) -> Option<Erased<queries::$name::Value<'tcx>>> {
-                    #[cfg(debug_assertions)]
-                    let _guard = tracing::span!(tracing::Level::TRACE, stringify!($name), ?key).entered();
-                    execution::execute_query_incr_inner(
-                        &tcx.query_system.query_vtables.$name,
-                        tcx,
-                        span,
-                        key,
-                        mode
-                    )
-                }
-            }
-
-            pub(crate) mod execute_query_non_incr {
-                use super::*;
-
-                #[inline(never)]
-                pub(crate) fn __rust_end_short_backtrace<'tcx>(
-                    tcx: TyCtxt<'tcx>,
-                    span: Span,
-                    key: queries::$name::Key<'tcx>,
-                    __mode: QueryMode,
-                ) -> Option<Erased<queries::$name::Value<'tcx>>> {
-                    Some(execution::execute_query_non_incr_inner(
-                        &tcx.query_system.query_vtables.$name,
-                        tcx,
-                        span,
-                        key,
-                    ))
-                }
-            }
-
-            /// Defines an `invoke_provider` function that calls the query's provider,
-            /// to be used as a function pointer in the query's vtable.
-            ///
-            /// To mark a short-backtrace boundary, the function's actual name
-            /// (after demangling) must be `__rust_begin_short_backtrace`.
-            mod invoke_provider_fn {
-                use super::*;
-                use ::rustc_middle::queries::$name::{Key, Value, provided_to_erased};
-
-                #[inline(never)]
-                pub(crate) fn __rust_begin_short_backtrace<'tcx>(
-                    tcx: TyCtxt<'tcx>,
-                    key: Key<'tcx>,
-                ) -> Erased<Value<'tcx>> {
-                    #[cfg(debug_assertions)]
-                    let _guard = tracing::span!(tracing::Level::TRACE, stringify!($name), ?key).entered();
-
-                    // Call the actual provider function for this query.
-
-                    #[cfg($separate_provide_extern)]
-                    let provided_value = if let Some(local_key) = key.as_local_key() {
-                        (tcx.query_system.local_providers.$name)(tcx, local_key)
-                    } else {
-                        (tcx.query_system.extern_providers.$name)(tcx, key)
-                    };
-
-                    #[cfg(not($separate_provide_extern))]
-                    let provided_value = (tcx.query_system.local_providers.$name)(tcx, key);
-
-                    rustc_middle::ty::print::with_reduced_queries!({
-                        tracing::trace!(?provided_value);
-                    });
-
-                    // Erase the returned value, because `QueryVTable` uses erased values.
-                    // For queries with `arena_cache`, this also arena-allocates the value.
-                    provided_to_erased(tcx, provided_value)
-                }
-            }
-
-            pub(crate) fn make_query_vtable<'tcx>(incremental: bool)
-                -> QueryVTable<'tcx, queries::$name::Cache<'tcx>>
-            {
-                QueryVTable {
-                    name: stringify!($name),
-                    anon: $anon,
-                    eval_always: $eval_always,
-                    depth_limit: $depth_limit,
-                    feedable: $feedable,
-                    dep_kind: dep_graph::DepKind::$name,
-                    state: Default::default(),
-                    cache: Default::default(),
-
-                    invoke_provider_fn: self::invoke_provider_fn::__rust_begin_short_backtrace,
-
-                    #[cfg($cache_on_disk)]
-                    will_cache_on_disk_for_key_fn:
-                        rustc_middle::queries::_cache_on_disk_if_fns::$name,
-                    #[cfg(not($cache_on_disk))]
-                    will_cache_on_disk_for_key_fn: |_, _| false,
-
-                    #[cfg($cache_on_disk)]
-                    try_load_from_disk_fn: |tcx, key, prev_index, index| {
-                        // Check the `cache_on_disk_if` condition for this key.
-                        if !rustc_middle::queries::_cache_on_disk_if_fns::$name(tcx, key) {
-                            return None;
-                        }
-
-                        let value: queries::$name::ProvidedValue<'tcx> =
-                            $crate::plumbing::try_load_from_disk(tcx, prev_index, index)?;
-
-                        // Arena-alloc the value if appropriate, and erase it.
-                        Some(queries::$name::provided_to_erased(tcx, value))
-                    },
-                    #[cfg(not($cache_on_disk))]
-                    try_load_from_disk_fn: |_tcx, _key, _prev_index, _index| None,
-
-                    #[cfg($cache_on_disk)]
-                    is_loadable_from_disk_fn: |tcx, key, index| -> bool {
-                        rustc_middle::queries::_cache_on_disk_if_fns::$name(tcx, key) &&
-                            $crate::plumbing::loadable_from_disk(tcx, index)
-                    },
-                    #[cfg(not($cache_on_disk))]
-                    is_loadable_from_disk_fn: |_tcx, _key, _index| false,
-
-                    // The default just emits `err` and then aborts.
-                    // `from_cycle_error::specialize_query_vtables` overwrites this default for
-                    // certain queries.
-                    value_from_cycle_error: |_tcx, _key, _cycle, err| {
-                        $crate::from_cycle_error::default(err)
-                    },
-
-                    #[cfg($no_hash)]
-                    hash_value_fn: None,
-                    #[cfg(not($no_hash))]
-                    hash_value_fn: Some(|hcx, erased_value: &erase::Erased<queries::$name::Value<'tcx>>| {
-                        let value = erase::restore_val(*erased_value);
-                        rustc_middle::dep_graph::hash_result(hcx, &value)
-                    }),
-
-                    format_value: |value| format!("{:?}", erase::restore_val::<queries::$name::Value<'tcx>>(*value)),
-                    create_tagged_key: TaggedQueryKey::$name,
-                    execute_query_fn: if incremental {
-                        query_impl::$name::execute_query_incr::__rust_end_short_backtrace
-                    } else {
-                        query_impl::$name::execute_query_non_incr::__rust_end_short_backtrace
-                    },
-                }
-            }
-
-            /// Marker type that implements [`GetQueryVTable`] for this query.
-            pub(crate) enum VTableGetter {}
-
-            impl<'tcx> GetQueryVTable<'tcx> for VTableGetter {
-                type Cache = rustc_middle::queries::$name::Cache<'tcx>;
-
-                #[inline(always)]
-                fn query_vtable(tcx: TyCtxt<'tcx>) -> &'tcx QueryVTable<'tcx, Self::Cache> {
-                    &tcx.query_system.query_vtables.$name
-                }
-            }
-        })*}
-
-        pub fn make_query_vtables<'tcx>(incremental: bool) -> queries::QueryVTables<'tcx> {
-            queries::QueryVTables {
-                $(
-                    $name: query_impl::$name::make_query_vtable(incremental),
-                )*
-            }
-        }
-
-        /// Given a filter condition (e.g. `ALL` or `CACHE_ON_DISK`), a `tcx`,
-        /// and a closure expression that accepts `&QueryVTable`, this macro
-        /// calls that closure with each query vtable that satisfies the filter
-        /// condition.
-        ///
-        /// This needs to be a macro, because the vtables can have different
-        /// key/value/cache types for different queries.
-        ///
-        /// This macro's argument syntax is specifically intended to look like
-        /// plain Rust code, so that `for_each_query_vtable!(..)` calls will be
-        /// formatted by rustfmt.
-        ///
-        /// To avoid too much nested-macro complication, filter conditions are
-        /// implemented by hand as needed.
-        macro_rules! for_each_query_vtable {
-            // Call with all queries.
-            (ALL, $tcx:expr, $closure:expr) => {{
-                let tcx: rustc_middle::ty::TyCtxt<'_> = $tcx;
-                $(
-                    let query: &rustc_middle::query::plumbing::QueryVTable<'_, _> =
-                        &tcx.query_system.query_vtables.$name;
-                    $closure(query);
-                )*
-            }};
-
-            // Only call with queries that can potentially cache to disk.
-            //
-            // This allows the use of trait bounds that only need to be satisfied
-            // by the subset of queries that actually cache to disk.
-            (CACHE_ON_DISK, $tcx:expr, $closure:expr) => {{
-                let tcx: rustc_middle::ty::TyCtxt<'_> = $tcx;
-                $(
-                    #[cfg($cache_on_disk)]
-                    {
-                        let query: &rustc_middle::query::plumbing::QueryVTable<'_, _> =
-                            &tcx.query_system.query_vtables.$name;
-                        $closure(query);
-                    }
-                )*
-            }}
-        }
-
-        pub(crate) use for_each_query_vtable;
-    }
-}

compiler/rustc_query_impl/src/profiling_support.rs

@@ -10,7 +10,7 @@ use rustc_middle::query::QueryCache;
 use rustc_middle::query::plumbing::QueryVTable;
 use rustc_middle::ty::TyCtxt;
 
-use crate::for_each_query_vtable;
+use crate::query_impl::for_each_query_vtable;
 
 pub(crate) struct QueryKeyStringCache {
     def_id_cache: FxHashMap<DefId, StringId>,