Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Jump to bottom

Simplify query proc-macros #77830

Merged
merged 5 commits into from
Oct 25, 2020
Merged

Simplify query proc-macros #77830

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
5 commits
Select commit Hold shift + click to select a range
0a4d948
Remove unused ProfileCategory.
cjgillot Jun 7, 2020
de76370
Remove unused category from macros.
cjgillot Oct 11, 2020
de7da7f
Unify query name and node name.
cjgillot Oct 11, 2020
e853cc0
Retire rustc_dep_node_force.
cjgillot Oct 11, 2020
57ba8ed
Retire rustc_dep_node_try_load_from_on_disk_cache.
cjgillot Oct 11, 2020
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
11 changes: 0 additions & 11 deletions compiler/rustc_data_structures/src/profiling.rs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -111,17 +111,6 @@ cfg_if! {

type Profiler = measureme::Profiler<SerializationSink>;

#[derive(Clone, Copy, Debug, PartialEq, Eq, Ord, PartialOrd)]
pub enum ProfileCategory {
Parsing,
Expansion,
TypeChecking,
BorrowChecking,
Codegen,
Linking,
Other,
}

bitflags::bitflags! {
struct EventFilter: u32 {
const GENERIC_ACTIVITIES = 1 << 0;
Expand Down
72 changes: 6 additions & 66 deletions compiler/rustc_macros/src/query.rs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -190,7 +190,11 @@ impl<T: Parse> Parse for List<T> {
}

/// A named group containing queries.
///
/// For now, the name is not used any more, but the capability remains interesting for future
/// developments of the query system.
struct Group {
#[allow(unused)]
name: Ident,
queries: List<Query>,
}
Expand Down Expand Up @@ -417,12 +421,9 @@ pub fn rustc_queries(input: TokenStream) -> TokenStream {
let mut query_stream = quote! {};
let mut query_description_stream = quote! {};
let mut dep_node_def_stream = quote! {};
let mut dep_node_force_stream = quote! {};
let mut try_load_from_on_disk_cache_stream = quote! {};
let mut cached_queries = quote! {};

for group in groups.0 {
let mut group_stream = quote! {};
for mut query in group.queries.0 {
let modifiers = process_modifiers(&mut query);
let name = &query.name;
Expand All @@ -437,22 +438,6 @@ pub fn rustc_queries(input: TokenStream) -> TokenStream {
cached_queries.extend(quote! {
#name,
});

try_load_from_on_disk_cache_stream.extend(quote! {
::rustc_middle::dep_graph::DepKind::#name => {
if <#arg as DepNodeParams<TyCtxt<'_>>>::can_reconstruct_query_key() {
debug_assert!($tcx.dep_graph
.node_color($dep_node)
.map(|c| c.is_green())
.unwrap_or(false));

let key = <#arg as DepNodeParams<TyCtxt<'_>>>::recover($tcx, $dep_node).unwrap();
if queries::#name::cache_on_disk($tcx, &key, None) {
let _ = $tcx.#name(key);
}
}
}
});
}

let mut attributes = Vec::new();
Expand Down Expand Up @@ -485,47 +470,20 @@ pub fn rustc_queries(input: TokenStream) -> TokenStream {
let attribute_stream = quote! {#(#attributes),*};
let doc_comments = query.doc_comments.iter();
// Add the query to the group
group_stream.extend(quote! {
query_stream.extend(quote! {
#(#doc_comments)*
[#attribute_stream] fn #name: #name(#arg) #result,
[#attribute_stream] fn #name(#arg) #result,
});

// Create a dep node for the query
dep_node_def_stream.extend(quote! {
[#attribute_stream] #name(#arg),
});

// Add a match arm to force the query given the dep node
dep_node_force_stream.extend(quote! {
::rustc_middle::dep_graph::DepKind::#name => {
if <#arg as DepNodeParams<TyCtxt<'_>>>::can_reconstruct_query_key() {
if let Some(key) = <#arg as DepNodeParams<TyCtxt<'_>>>::recover($tcx, $dep_node) {
force_query::<crate::ty::query::queries::#name<'_>, _>(
$tcx,
key,
DUMMY_SP,
*$dep_node
);
return true;
}
}
}
});

add_query_description_impl(&query, modifiers, &mut query_description_stream);
}
let name = &group.name;
query_stream.extend(quote! {
#name { #group_stream },
});
}

dep_node_force_stream.extend(quote! {
::rustc_middle::dep_graph::DepKind::Null => {
bug!("Cannot force dep node: {:?}", $dep_node)
}
});

TokenStream::from(quote! {
macro_rules! rustc_query_append {
([$($macro:tt)*][$($other:tt)*]) => {
Expand All @@ -546,30 +504,12 @@ pub fn rustc_queries(input: TokenStream) -> TokenStream {
);
}
}
macro_rules! rustc_dep_node_force {
([$dep_node:expr, $tcx:expr] $($other:tt)*) => {
match $dep_node.kind {
$($other)*

#dep_node_force_stream
}
}
}
macro_rules! rustc_cached_queries {
($($macro:tt)*) => {
$($macro)*(#cached_queries);
}
}

#query_description_stream

macro_rules! rustc_dep_node_try_load_from_on_disk_cache {
($dep_node:expr, $tcx:expr) => {
match $dep_node.kind {
#try_load_from_on_disk_cache_stream
_ => (),
}
}
}
})
}
70 changes: 57 additions & 13 deletions compiler/rustc_middle/src/ty/query/mod.rs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -34,7 +34,6 @@ use crate::ty::util::AlwaysRequiresDrop;
use crate::ty::{self, AdtSizedConstraint, CrateInherentImpls, ParamEnvAnd, Ty, TyCtxt};
use rustc_data_structures::fingerprint::Fingerprint;
use rustc_data_structures::fx::{FxHashMap, FxHashSet, FxIndexMap};
use rustc_data_structures::profiling::ProfileCategory::*;
use rustc_data_structures::stable_hasher::StableVec;
use rustc_data_structures::svh::Svh;
use rustc_data_structures::sync::Lrc;
Expand Down Expand Up @@ -169,26 +168,71 @@ pub fn force_from_dep_node<'tcx>(tcx: TyCtxt<'tcx>, dep_node: &DepNode) -> bool
return false;
}

rustc_dep_node_force!([dep_node, tcx]
// These are inputs that are expected to be pre-allocated and that
// should therefore always be red or green already.
DepKind::CrateMetadata |
macro_rules! force_from_dep_node {
($($(#[$attr:meta])* [$($modifiers:tt)*] $name:ident($K:ty),)*) => {
match dep_node.kind {
// These are inputs that are expected to be pre-allocated and that
// should therefore always be red or green already.
DepKind::CrateMetadata |

// These are anonymous nodes.
DepKind::TraitSelect |
// These are anonymous nodes.
DepKind::TraitSelect |

// We don't have enough information to reconstruct the query key of
// these.
DepKind::CompileCodegenUnit => {
bug!("force_from_dep_node: encountered {:?}", dep_node)
// We don't have enough information to reconstruct the query key of
// these.
DepKind::CompileCodegenUnit |

// Forcing this makes no sense.
DepKind::Null => {
bug!("force_from_dep_node: encountered {:?}", dep_node)
}

$(DepKind::$name => {
debug_assert!(<$K as DepNodeParams<TyCtxt<'_>>>::can_reconstruct_query_key());

if let Some(key) = <$K as DepNodeParams<TyCtxt<'_>>>::recover(tcx, dep_node) {
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Can the None case happen here? Or does recover only return None when can_reconstruct_query_key() would return false?
I don't understand the code here, but it seems like a fragile interaction of different parts. I'd add an else { bug!() } branch instead of the debug assert.
It's probably fine though, because the previous code did the same.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

recover is implemented in terms of extract_def_id, which uses def_path_hash_to_def_id.get(...), which can return None.

force_query::<queries::$name<'_>, _>(
tcx,
key,
DUMMY_SP,
*dep_node
);
return true;
}
})*
}
}
);
}

rustc_dep_node_append! { [force_from_dep_node!][] }

false
}

pub(crate) fn try_load_from_on_disk_cache<'tcx>(tcx: TyCtxt<'tcx>, dep_node: &DepNode) {
rustc_dep_node_try_load_from_on_disk_cache!(dep_node, tcx)
macro_rules! try_load_from_on_disk_cache {
($($name:ident,)*) => {
match dep_node.kind {
$(DepKind::$name => {
if <query_keys::$name<'tcx> as DepNodeParams<TyCtxt<'_>>>::can_reconstruct_query_key() {
debug_assert!(tcx.dep_graph
.node_color(dep_node)
.map(|c| c.is_green())
.unwrap_or(false));

let key = <query_keys::$name<'tcx> as DepNodeParams<TyCtxt<'_>>>::recover(tcx, dep_node).unwrap();
if queries::$name::cache_on_disk(tcx, &key, None) {
let _ = tcx.$name(key);
}
}
})*

_ => (),
}
}
}

rustc_cached_queries!(try_load_from_on_disk_cache!);
}

mod sealed {
Expand Down
20 changes: 4 additions & 16 deletions compiler/rustc_middle/src/ty/query/plumbing.rs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -242,33 +242,22 @@ macro_rules! hash_result {
};
}

macro_rules! define_queries {
(<$tcx:tt> $($category:tt {
$($(#[$attr:meta])* [$($modifiers:tt)*] fn $name:ident: $node:ident($($K:tt)*) -> $V:ty,)*
},)*) => {
define_queries_inner! { <$tcx>
$($( $(#[$attr])* category<$category> [$($modifiers)*] fn $name: $node($($K)*) -> $V,)*)*
}
}
}

macro_rules! query_helper_param_ty {
(DefId) => { impl IntoQueryParam<DefId> };
($K:ty) => { $K };
}

macro_rules! define_queries_inner {
macro_rules! define_queries {
(<$tcx:tt>
$($(#[$attr:meta])* category<$category:tt>
[$($modifiers:tt)*] fn $name:ident: $node:ident($($K:tt)*) -> $V:ty,)*) => {
$($(#[$attr:meta])*
[$($modifiers:tt)*] fn $name:ident($($K:tt)*) -> $V:ty,)*) => {

use std::mem;
use crate::{
rustc_data_structures::stable_hasher::HashStable,
rustc_data_structures::stable_hasher::StableHasher,
ich::StableHashingContext
};
use rustc_data_structures::profiling::ProfileCategory;

define_queries_struct! {
tcx: $tcx,
Expand Down Expand Up @@ -362,13 +351,12 @@ macro_rules! define_queries_inner {
as QueryStorage
>::Stored;
const NAME: &'static str = stringify!($name);
const CATEGORY: ProfileCategory = $category;
}

impl<$tcx> QueryAccessors<TyCtxt<$tcx>> for queries::$name<$tcx> {
const ANON: bool = is_anon!([$($modifiers)*]);
const EVAL_ALWAYS: bool = is_eval_always!([$($modifiers)*]);
const DEP_KIND: dep_graph::DepKind = dep_graph::DepKind::$node;
const DEP_KIND: dep_graph::DepKind = dep_graph::DepKind::$name;

type Cache = query_storage!([$($modifiers)*][$($K)*, $V]);

Expand Down
10 changes: 5 additions & 5 deletions compiler/rustc_middle/src/ty/query/stats.rs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -120,13 +120,13 @@ pub fn print_stats(tcx: TyCtxt<'_>) {
}

macro_rules! print_stats {
(<$tcx:tt> $($category:tt {
$($(#[$attr:meta])* [$($modifiers:tt)*] fn $name:ident: $node:ident($K:ty) -> $V:ty,)*
},)*) => {
(<$tcx:tt>
$($(#[$attr:meta])* [$($modifiers:tt)*] fn $name:ident($K:ty) -> $V:ty,)*
) => {
fn query_stats(tcx: TyCtxt<'_>) -> Vec<QueryStats> {
let mut queries = Vec::new();

$($(
$(
queries.push(stats::<
crate::dep_graph::DepKind,
<TyCtxt<'_> as QueryContext>::Query,
Expand All @@ -135,7 +135,7 @@ macro_rules! print_stats {
stringify!($name),
&tcx.queries.$name,
));
)*)*
)*

queries
}
Expand Down
2 changes: 0 additions & 2 deletions compiler/rustc_query_system/src/query/config.rs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -5,7 +5,6 @@ use crate::dep_graph::SerializedDepNodeIndex;
use crate::query::caches::QueryCache;
use crate::query::plumbing::CycleError;
use crate::query::{QueryContext, QueryState};
use rustc_data_structures::profiling::ProfileCategory;

use rustc_data_structures::fingerprint::Fingerprint;
use std::borrow::Cow;
Expand All @@ -14,7 +13,6 @@ use std::hash::Hash;

pub trait QueryConfig {
const NAME: &'static str;
const CATEGORY: ProfileCategory;

type Key: Eq + Hash + Clone + Debug;
type Value;
Expand Down