Skip to content

Commit

Permalink
Auto merge of rust-lang#102895 - Nilstrieb:query-cleanups, r=cjgillot
Browse files Browse the repository at this point in the history
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`
  • Loading branch information
bors committed Oct 15, 2022
2 parents c93ef33 + 24ce4cf commit b8c35ca
Show file tree
Hide file tree
Showing 6 changed files with 51 additions and 43 deletions.
52 changes: 28 additions & 24 deletions compiler/rustc_macros/src/query.rs
Original file line number Diff line number Diff line change
Expand Up @@ -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
}
}
Expand All @@ -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
});
}

Expand All @@ -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;
Expand All @@ -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 {
Expand Down Expand Up @@ -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! {
Expand All @@ -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
}
})
}
5 changes: 4 additions & 1 deletion compiler/rustc_middle/src/query/mod.rs
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand Down Expand Up @@ -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 }
}
Expand Down
11 changes: 10 additions & 1 deletion compiler/rustc_middle/src/ty/print/mod.rs
Original file line number Diff line number Diff line change
Expand Up @@ -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.
Expand Down Expand Up @@ -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()))
}
}
11 changes: 1 addition & 10 deletions compiler/rustc_query_impl/src/lib.rs
Original file line number Diff line number Diff line change
Expand Up @@ -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]
Expand All @@ -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> {
Expand Down
13 changes: 8 additions & 5 deletions compiler/rustc_query_impl/src/plumbing.rs
Original file line number Diff line number Diff line change
Expand Up @@ -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,
Expand All @@ -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 };
Expand Down Expand Up @@ -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>;

Expand Down Expand Up @@ -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<()> {
Expand All @@ -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,
Expand Down
2 changes: 0 additions & 2 deletions compiler/rustc_query_system/src/query/config.rs
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand Down

0 comments on commit b8c35ca

Please sign in to comment.