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.

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

Take impl Into<DefId> for query methods on TyCtxt #70961

Merged
merged 8 commits into from
Apr 13, 2020
Merged
28 changes: 28 additions & 0 deletions src/librustc_middle/ty/query/mod.rs
Original file line number Diff line number Diff line change
Expand Up @@ -189,3 +189,31 @@ pub fn force_from_dep_node<'tcx>(tcx: TyCtxt<'tcx>, dep_node: &DepNode) -> bool
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)
}

mod sealed {
use super::{DefId, LocalDefId};

/// An analogue of the `Into` trait that's intended only for query paramaters.
///
/// This exists to allow queries to accept either `DefId` or `LocalDefId` while requiring that the
/// user call `to_def_id` to convert between them everywhere else.
pub trait IntoQueryParam<P> {
fn into_query_param(self) -> P;
}

impl<P> IntoQueryParam<P> for P {
#[inline(always)]
fn into_query_param(self) -> P {
self
}
}

impl IntoQueryParam<DefId> for LocalDefId {
#[inline(always)]
fn into_query_param(self) -> DefId {
Copy link
Member

Choose a reason for hiding this comment

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

Try adding #[inline(always)] to this (and to_def_id if it doesn't have it).

Although, wait, are we using this at all now? Is the slowdown just from going through the noop impl?

Copy link
Contributor Author

@ecstatic-morse ecstatic-morse Apr 11, 2020

Choose a reason for hiding this comment

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

It's used exactly once as a test 6e5a210. I'm skeptical that this will help, since no into_query_param call appears in the optimized binary, and the DefId queries I looked at inline down to get_query. There's no predicting the optimizer though.

self.to_def_id()
}
}
}

use sealed::IntoQueryParam;
29 changes: 17 additions & 12 deletions src/librustc_middle/ty/query/plumbing.rs
Original file line number Diff line number Diff line change
Expand Up @@ -234,18 +234,23 @@ macro_rules! hash_result {

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

use std::mem;
use crate::{
Expand All @@ -263,7 +268,7 @@ macro_rules! define_queries_inner {
#[allow(nonstandard_style)]
#[derive(Clone, Debug)]
pub enum Query<$tcx> {
$($(#[$attr])* $name($K)),*
$($(#[$attr])* $name($($K)*)),*
}

impl<$tcx> Query<$tcx> {
Expand Down Expand Up @@ -321,7 +326,7 @@ macro_rules! define_queries_inner {
}

$(impl<$tcx> QueryConfig<TyCtxt<$tcx>> for queries::$name<$tcx> {
type Key = $K;
type Key = $($K)*;
type Value = $V;
const NAME: &'static str = stringify!($name);
const CATEGORY: ProfileCategory = $category;
Expand All @@ -332,7 +337,7 @@ macro_rules! define_queries_inner {
const EVAL_ALWAYS: bool = is_eval_always!([$($modifiers)*]);
const DEP_KIND: dep_graph::DepKind = dep_graph::DepKind::$node;

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

#[inline(always)]
fn query_state<'a>(tcx: TyCtxt<$tcx>) -> &'a QueryState<TyCtxt<$tcx>, Self::Cache> {
Expand Down Expand Up @@ -380,8 +385,8 @@ macro_rules! define_queries_inner {
impl TyCtxtEnsure<$tcx> {
$($(#[$attr])*
#[inline(always)]
pub fn $name(self, key: $K) {
ensure_query::<queries::$name<'_>, _>(self.tcx, key)
pub fn $name(self, key: query_helper_param_ty!($($K)*)) {
ensure_query::<queries::$name<'_>, _>(self.tcx, key.into_query_param())
Comment on lines -383 to +389
Copy link
Member

Choose a reason for hiding this comment

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

Actually, are these changes needed? "ensuring" should be rather rare AFAIK?

})*
}

Expand Down Expand Up @@ -421,7 +426,7 @@ macro_rules! define_queries_inner {

$($(#[$attr])*
#[inline(always)]
pub fn $name(self, key: $K) -> $V {
pub fn $name(self, key: query_helper_param_ty!($($K)*)) -> $V {
self.at(DUMMY_SP).$name(key)
})*

Expand Down Expand Up @@ -458,14 +463,14 @@ macro_rules! define_queries_inner {
impl TyCtxtAt<$tcx> {
$($(#[$attr])*
#[inline(always)]
pub fn $name(self, key: $K) -> $V {
get_query::<queries::$name<'_>, _>(self.tcx, self.span, key)
pub fn $name(self, key: query_helper_param_ty!($($K)*)) -> $V {
get_query::<queries::$name<'_>, _>(self.tcx, self.span, key.into_query_param())
})*
}

define_provider_struct! {
tcx: $tcx,
input: ($(([$($modifiers)*] [$name] [$K] [$V]))*)
input: ($(([$($modifiers)*] [$name] [$($K)*] [$V]))*)
}

impl<$tcx> Copy for Providers<$tcx> {}
Expand Down
2 changes: 1 addition & 1 deletion src/librustc_middle/ty/trait_def.rs
Original file line number Diff line number Diff line change
Expand Up @@ -193,7 +193,7 @@ pub(super) fn trait_impls_of_provider(tcx: TyCtxt<'_>, trait_id: DefId) -> &Trai
let mut impls = TraitImpls::default();

{
let mut add_impl = |impl_def_id| {
let mut add_impl = |impl_def_id: DefId| {
let impl_self_ty = tcx.type_of(impl_def_id);
if impl_def_id.is_local() && impl_self_ty.references_error() {
return;
Expand Down
2 changes: 1 addition & 1 deletion src/librustc_typeck/check/wfcheck.rs
Original file line number Diff line number Diff line change
Expand Up @@ -335,7 +335,7 @@ fn for_id(tcx: TyCtxt<'_>, id: hir::HirId, span: Span) -> CheckWfFcxBuilder<'_>
inherited: Inherited::build(tcx, def_id),
id,
span,
param_env: tcx.param_env(def_id.to_def_id()),
param_env: tcx.param_env(def_id),
}
}

Expand Down