From 0bf5cae489e828a6678cab5144e638ae909d7b93 Mon Sep 17 00:00:00 2001 From: Camille GILLOT Date: Fri, 6 Mar 2020 21:36:09 +0100 Subject: [PATCH 01/29] Remove __query_compute module. --- src/librustc/ty/query/plumbing.rs | 28 ++++++++-------------------- 1 file changed, 8 insertions(+), 20 deletions(-) diff --git a/src/librustc/ty/query/plumbing.rs b/src/librustc/ty/query/plumbing.rs index acf67f52dceaa..bfae0075c8d07 100644 --- a/src/librustc/ty/query/plumbing.rs +++ b/src/librustc/ty/query/plumbing.rs @@ -951,16 +951,6 @@ macro_rules! define_queries_inner { })* } - // This module and the functions in it exist only to provide a - // predictable symbol name prefix for query providers. This is helpful - // for analyzing queries in profilers. - pub(super) mod __query_compute { - $(#[inline(never)] - pub fn $name R, R>(f: F) -> R { - f() - })* - } - $(impl<$tcx> QueryConfig<$tcx> for queries::$name<$tcx> { type Key = $K; type Value = $V; @@ -997,16 +987,14 @@ macro_rules! define_queries_inner { #[inline] fn compute(tcx: TyCtxt<'tcx>, key: Self::Key) -> Self::Value { - __query_compute::$name(move || { - let provider = tcx.queries.providers.get(key.query_crate()) - // HACK(eddyb) it's possible crates may be loaded after - // the query engine is created, and because crate loading - // is not yet integrated with the query engine, such crates - // would be missing appropriate entries in `providers`. - .unwrap_or(&tcx.queries.fallback_extern_providers) - .$name; - provider(tcx, key) - }) + let provider = tcx.queries.providers.get(key.query_crate()) + // HACK(eddyb) it's possible crates may be loaded after + // the query engine is created, and because crate loading + // is not yet integrated with the query engine, such crates + // would be missing appropriate entries in `providers`. + .unwrap_or(&tcx.queries.fallback_extern_providers) + .$name; + provider(tcx, key) } fn hash_result( From fc82376bc437d4494832b171d924e2f116174578 Mon Sep 17 00:00:00 2001 From: Camille GILLOT Date: Sat, 7 Mar 2020 00:43:17 +0100 Subject: [PATCH 02/29] Make QueryAccessor::dep_kind an associated const. --- src/librustc/ty/query/config.rs | 3 +-- src/librustc/ty/query/plumbing.rs | 14 +++++--------- 2 files changed, 6 insertions(+), 11 deletions(-) diff --git a/src/librustc/ty/query/config.rs b/src/librustc/ty/query/config.rs index 178c2362def6e..5a05acc90e43f 100644 --- a/src/librustc/ty/query/config.rs +++ b/src/librustc/ty/query/config.rs @@ -28,6 +28,7 @@ pub trait QueryConfig<'tcx> { pub(crate) trait QueryAccessors<'tcx>: QueryConfig<'tcx> { const ANON: bool; const EVAL_ALWAYS: bool; + const DEP_KIND: DepKind; type Cache: QueryCache; @@ -38,8 +39,6 @@ pub(crate) trait QueryAccessors<'tcx>: QueryConfig<'tcx> { fn to_dep_node(tcx: TyCtxt<'tcx>, key: &Self::Key) -> DepNode; - fn dep_kind() -> DepKind; - // Don't use this method to compute query results, instead use the methods on TyCtxt fn compute(tcx: TyCtxt<'tcx>, key: Self::Key) -> Self::Value; diff --git a/src/librustc/ty/query/plumbing.rs b/src/librustc/ty/query/plumbing.rs index bfae0075c8d07..603c4fd9b72c3 100644 --- a/src/librustc/ty/query/plumbing.rs +++ b/src/librustc/ty/query/plumbing.rs @@ -154,7 +154,7 @@ impl<'tcx, Q: QueryDescription<'tcx>> JobOwner<'tcx, Q> { }; // Create the id of the job we're waiting for - let id = QueryJobId::new(job.id, lookup.shard, Q::dep_kind()); + let id = QueryJobId::new(job.id, lookup.shard, Q::DEP_KIND); (job.latch(id), _query_blocked_prof_timer) } @@ -169,7 +169,7 @@ impl<'tcx, Q: QueryDescription<'tcx>> JobOwner<'tcx, Q> { lock.jobs = id; let id = QueryShardJobId(NonZeroU32::new(id).unwrap()); - let global_id = QueryJobId::new(id, lookup.shard, Q::dep_kind()); + let global_id = QueryJobId::new(id, lookup.shard, Q::DEP_KIND); let job = tls::with_related_context(tcx, |icx| QueryJob::new(id, span, icx.query)); @@ -498,7 +498,7 @@ impl<'tcx> TyCtxt<'tcx> { let ((result, dep_node_index), diagnostics) = with_diagnostics(|diagnostics| { self.start_query(job.id, diagnostics, |tcx| { - tcx.dep_graph.with_anon_task(Q::dep_kind(), || Q::compute(tcx, key)) + tcx.dep_graph.with_anon_task(Q::DEP_KIND, || Q::compute(tcx, key)) }) }); @@ -873,7 +873,7 @@ macro_rules! define_queries_inner { job: job.id, shard: u16::try_from(shard_id).unwrap(), kind: - as QueryAccessors<'tcx>>::dep_kind(), + as QueryAccessors<'tcx>>::DEP_KIND, }; let info = QueryInfo { span: job.span, @@ -961,6 +961,7 @@ macro_rules! define_queries_inner { impl<$tcx> QueryAccessors<$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; type Cache = query_storage!([$($modifiers)*][$K, $V]); @@ -980,11 +981,6 @@ macro_rules! define_queries_inner { DepConstructor::$node(tcx, *key) } - #[inline(always)] - fn dep_kind() -> dep_graph::DepKind { - dep_graph::DepKind::$node - } - #[inline] fn compute(tcx: TyCtxt<'tcx>, key: Self::Key) -> Self::Value { let provider = tcx.queries.providers.get(key.query_crate()) From cf238fd057651371731fde47a2ebf251bf37cfb5 Mon Sep 17 00:00:00 2001 From: Camille GILLOT Date: Sat, 7 Mar 2020 01:01:01 +0100 Subject: [PATCH 03/29] Inline QueryAccessor::query. --- src/librustc/ty/query/config.rs | 4 +--- src/librustc/ty/query/plumbing.rs | 7 +------ 2 files changed, 2 insertions(+), 9 deletions(-) diff --git a/src/librustc/ty/query/config.rs b/src/librustc/ty/query/config.rs index 5a05acc90e43f..29b9e0c3b40ef 100644 --- a/src/librustc/ty/query/config.rs +++ b/src/librustc/ty/query/config.rs @@ -2,7 +2,7 @@ use crate::dep_graph::SerializedDepNodeIndex; use crate::dep_graph::{DepKind, DepNode}; use crate::ty::query::caches::QueryCache; use crate::ty::query::plumbing::CycleError; -use crate::ty::query::{Query, QueryState}; +use crate::ty::query::QueryState; use crate::ty::TyCtxt; use rustc_data_structures::profiling::ProfileCategory; use rustc_hir::def_id::DefId; @@ -32,8 +32,6 @@ pub(crate) trait QueryAccessors<'tcx>: QueryConfig<'tcx> { type Cache: QueryCache; - fn query(key: Self::Key) -> Query<'tcx>; - // Don't use this method to access query results, instead use the methods on TyCtxt fn query_state<'a>(tcx: TyCtxt<'tcx>) -> &'a QueryState<'tcx, Self>; diff --git a/src/librustc/ty/query/plumbing.rs b/src/librustc/ty/query/plumbing.rs index 603c4fd9b72c3..32138b3e1d5fe 100644 --- a/src/librustc/ty/query/plumbing.rs +++ b/src/librustc/ty/query/plumbing.rs @@ -877,7 +877,7 @@ macro_rules! define_queries_inner { }; let info = QueryInfo { span: job.span, - query: queries::$name::query(k.clone()) + query: Query::$name(k.clone()) }; Some((id, QueryJobInfo { info, job: job.clone() })) } else { @@ -965,11 +965,6 @@ macro_rules! define_queries_inner { type Cache = query_storage!([$($modifiers)*][$K, $V]); - #[inline(always)] - fn query(key: Self::Key) -> Query<'tcx> { - Query::$name(key) - } - #[inline(always)] fn query_state<'a>(tcx: TyCtxt<$tcx>) -> &'a QueryState<$tcx, Self> { &tcx.queries.$name From 1249032aabe3a0a80c0a852ef803702d7fb70d21 Mon Sep 17 00:00:00 2001 From: Camille GILLOT Date: Sat, 7 Mar 2020 17:10:31 +0100 Subject: [PATCH 04/29] Move impl of Queries with its definition. --- src/librustc/ty/query/plumbing.rs | 98 +++++++++++++++---------------- 1 file changed, 49 insertions(+), 49 deletions(-) diff --git a/src/librustc/ty/query/plumbing.rs b/src/librustc/ty/query/plumbing.rs index 32138b3e1d5fe..5532d7e08cc92 100644 --- a/src/librustc/ty/query/plumbing.rs +++ b/src/librustc/ty/query/plumbing.rs @@ -842,55 +842,6 @@ macro_rules! define_queries_inner { input: ($(([$($modifiers)*] [$($attr)*] [$name]))*) } - impl<$tcx> Queries<$tcx> { - pub fn new( - providers: IndexVec>, - fallback_extern_providers: Providers<$tcx>, - on_disk_cache: OnDiskCache<'tcx>, - ) -> Self { - Queries { - providers, - fallback_extern_providers: Box::new(fallback_extern_providers), - on_disk_cache, - $($name: Default::default()),* - } - } - - pub fn try_collect_active_jobs( - &self - ) -> Option>> { - let mut jobs = FxHashMap::default(); - - $( - // We use try_lock_shards here since we are called from the - // deadlock handler, and this shouldn't be locked. - let shards = self.$name.shards.try_lock_shards()?; - let shards = shards.iter().enumerate(); - jobs.extend(shards.flat_map(|(shard_id, shard)| { - shard.active.iter().filter_map(move |(k, v)| { - if let QueryResult::Started(ref job) = *v { - let id = QueryJobId { - job: job.id, - shard: u16::try_from(shard_id).unwrap(), - kind: - as QueryAccessors<'tcx>>::DEP_KIND, - }; - let info = QueryInfo { - span: job.span, - query: Query::$name(k.clone()) - }; - Some((id, QueryJobInfo { info, job: job.clone() })) - } else { - None - } - }) - })); - )* - - Some(jobs) - } - } - #[allow(nonstandard_style)] #[derive(Clone, Debug)] pub enum Query<$tcx> { @@ -1120,6 +1071,55 @@ macro_rules! define_queries_struct { $($(#[$attr])* $name: QueryState<$tcx, queries::$name<$tcx>>,)* } + + impl<$tcx> Queries<$tcx> { + pub fn new( + providers: IndexVec>, + fallback_extern_providers: Providers<$tcx>, + on_disk_cache: OnDiskCache<'tcx>, + ) -> Self { + Queries { + providers, + fallback_extern_providers: Box::new(fallback_extern_providers), + on_disk_cache, + $($name: Default::default()),* + } + } + + pub fn try_collect_active_jobs( + &self + ) -> Option>> { + let mut jobs = FxHashMap::default(); + + $( + // We use try_lock_shards here since we are called from the + // deadlock handler, and this shouldn't be locked. + let shards = self.$name.shards.try_lock_shards()?; + let shards = shards.iter().enumerate(); + jobs.extend(shards.flat_map(|(shard_id, shard)| { + shard.active.iter().filter_map(move |(k, v)| { + if let QueryResult::Started(ref job) = *v { + let id = QueryJobId { + job: job.id, + shard: u16::try_from(shard_id).unwrap(), + kind: + as QueryAccessors<'tcx>>::DEP_KIND, + }; + let info = QueryInfo { + span: job.span, + query: Query::$name(k.clone()) + }; + Some((id, QueryJobInfo { info, job: job.clone() })) + } else { + None + } + }) + })); + )* + + Some(jobs) + } + } }; } From b08943358ec8adc2d8e542659c7dcd2514311918 Mon Sep 17 00:00:00 2001 From: Camille GILLOT Date: Fri, 6 Mar 2020 18:36:11 +0100 Subject: [PATCH 05/29] Unpack type arguments for QueryStateShard. --- src/librustc/ty/query/plumbing.rs | 35 ++++++++++++++++++------------- 1 file changed, 21 insertions(+), 14 deletions(-) diff --git a/src/librustc/ty/query/plumbing.rs b/src/librustc/ty/query/plumbing.rs index 5532d7e08cc92..b688365b2bb9a 100644 --- a/src/librustc/ty/query/plumbing.rs +++ b/src/librustc/ty/query/plumbing.rs @@ -4,7 +4,7 @@ use crate::dep_graph::{DepNode, DepNodeIndex, SerializedDepNodeIndex}; use crate::ty::query::caches::QueryCache; -use crate::ty::query::config::{QueryAccessors, QueryDescription}; +use crate::ty::query::config::{QueryAccessors, QueryConfig, QueryDescription}; use crate::ty::query::job::{QueryInfo, QueryJob, QueryJobId, QueryShardJobId}; use crate::ty::query::Query; use crate::ty::tls; @@ -27,25 +27,32 @@ use std::ptr; #[cfg(debug_assertions)] use std::sync::atomic::{AtomicUsize, Ordering}; -pub(crate) struct QueryStateShard<'tcx, D: QueryAccessors<'tcx> + ?Sized> { - pub(super) cache: <>::Cache as QueryCache>::Sharded, - pub(super) active: FxHashMap>, +pub(crate) type QueryStateShard<'tcx, Q> = QueryStateShardImpl< + 'tcx, + >::Key, + <>::Cache as QueryCache< + >::Key, + >::Value, + >>::Sharded, +>; + +pub(crate) struct QueryStateShardImpl<'tcx, K, C> { + pub(super) cache: C, + pub(super) active: FxHashMap>, /// Used to generate unique ids for active jobs. pub(super) jobs: u32, } -impl<'tcx, Q: QueryAccessors<'tcx>> QueryStateShard<'tcx, Q> { - fn get_cache( - &mut self, - ) -> &mut <>::Cache as QueryCache>::Sharded { +impl<'tcx, K, C> QueryStateShardImpl<'tcx, K, C> { + fn get_cache(&mut self) -> &mut C { &mut self.cache } } -impl<'tcx, Q: QueryAccessors<'tcx>> Default for QueryStateShard<'tcx, Q> { - fn default() -> QueryStateShard<'tcx, Q> { - QueryStateShard { cache: Default::default(), active: Default::default(), jobs: 0 } +impl<'tcx, K, C: Default> Default for QueryStateShardImpl<'tcx, K, C> { + fn default() -> QueryStateShardImpl<'tcx, K, C> { + QueryStateShardImpl { cache: Default::default(), active: Default::default(), jobs: 0 } } } @@ -122,7 +129,7 @@ pub(super) struct JobOwner<'tcx, Q: QueryDescription<'tcx>> { id: QueryJobId, } -impl<'tcx, Q: QueryDescription<'tcx>> JobOwner<'tcx, Q> { +impl<'tcx, Q: QueryDescription<'tcx> + 'tcx> JobOwner<'tcx, Q> { /// Either gets a `JobOwner` corresponding the query, allowing us to /// start executing the query, or returns with the result of the query. /// This function assumes that `try_get_cached` is already called and returned `lookup`. @@ -470,7 +477,7 @@ impl<'tcx> TyCtxt<'tcx> { } #[inline(always)] - pub(super) fn try_execute_query>( + pub(super) fn try_execute_query + 'tcx>( self, span: Span, key: Q::Key, @@ -634,7 +641,7 @@ impl<'tcx> TyCtxt<'tcx> { } #[inline(always)] - fn force_query_with_job>( + fn force_query_with_job + 'tcx>( self, key: Q::Key, job: JobOwner<'tcx, Q>, From 486a082c58c60959328e0b394f9e58850b3c6341 Mon Sep 17 00:00:00 2001 From: Camille GILLOT Date: Fri, 6 Mar 2020 19:54:19 +0100 Subject: [PATCH 06/29] Unpack type arguments for QueryLookup. --- src/librustc/ty/query/plumbing.rs | 7 ++++--- 1 file changed, 4 insertions(+), 3 deletions(-) diff --git a/src/librustc/ty/query/plumbing.rs b/src/librustc/ty/query/plumbing.rs index b688365b2bb9a..67f0fdfcd5488 100644 --- a/src/librustc/ty/query/plumbing.rs +++ b/src/librustc/ty/query/plumbing.rs @@ -74,7 +74,7 @@ impl<'tcx, Q: QueryAccessors<'tcx>> QueryState<'tcx, Q> { let shard = self.shards.get_shard_index_by_hash(key_hash); let lock = self.shards.get_shard_by_index(shard).lock(); - QueryLookup { key_hash, shard, lock } + QueryLookupImpl { key_hash, shard, lock } } } @@ -115,10 +115,11 @@ impl<'tcx, M: QueryAccessors<'tcx>> Default for QueryState<'tcx, M> { } /// Values used when checking a query cache which can be reused on a cache-miss to execute the query. -pub(crate) struct QueryLookup<'tcx, Q: QueryAccessors<'tcx>> { +pub(crate) type QueryLookup<'tcx, Q> = QueryLookupImpl<'tcx, QueryStateShard<'tcx, Q>>; +pub(crate) struct QueryLookupImpl<'tcx, QSS> { pub(super) key_hash: u64, pub(super) shard: usize, - pub(super) lock: LockGuard<'tcx, QueryStateShard<'tcx, Q>>, + pub(super) lock: LockGuard<'tcx, QSS>, } /// A type representing the responsibility to execute the job in the `job` field. From a0f57e24e35a365d9d55f37611edfc6666b5d3c9 Mon Sep 17 00:00:00 2001 From: Camille GILLOT Date: Fri, 6 Mar 2020 20:05:05 +0100 Subject: [PATCH 07/29] Unpack type arguments for QueryState. --- src/librustc/ty/query/plumbing.rs | 38 +++++++++++++++++++------------ 1 file changed, 23 insertions(+), 15 deletions(-) diff --git a/src/librustc/ty/query/plumbing.rs b/src/librustc/ty/query/plumbing.rs index 67f0fdfcd5488..37fc339ee6395 100644 --- a/src/librustc/ty/query/plumbing.rs +++ b/src/librustc/ty/query/plumbing.rs @@ -56,15 +56,25 @@ impl<'tcx, K, C: Default> Default for QueryStateShardImpl<'tcx, K, C> { } } -pub(crate) struct QueryState<'tcx, D: QueryAccessors<'tcx> + ?Sized> { - pub(super) cache: D::Cache, - pub(super) shards: Sharded>, +pub(crate) type QueryState<'tcx, Q> = QueryStateImpl< + 'tcx, + >::Key, + >::Value, + >::Cache, +>; + +pub(crate) struct QueryStateImpl<'tcx, K, V, C: QueryCache> { + pub(super) cache: C, + pub(super) shards: Sharded>, #[cfg(debug_assertions)] pub(super) cache_hits: AtomicUsize, } -impl<'tcx, Q: QueryAccessors<'tcx>> QueryState<'tcx, Q> { - pub(super) fn get_lookup(&'tcx self, key: &K) -> QueryLookup<'tcx, Q> { +impl<'tcx, K, V, C: QueryCache> QueryStateImpl<'tcx, K, V, C> { + pub(super) fn get_lookup( + &'tcx self, + key: &K2, + ) -> QueryLookupImpl<'tcx, QueryStateShardImpl<'tcx, K, C::Sharded>> { // We compute the key's hash once and then use it for both the // shard lookup and the hashmap lookup. This relies on the fact // that both of them use `FxHasher`. @@ -88,12 +98,10 @@ pub(super) enum QueryResult<'tcx> { Poisoned, } -impl<'tcx, M: QueryAccessors<'tcx>> QueryState<'tcx, M> { +impl<'tcx, K, V, C: QueryCache> QueryStateImpl<'tcx, K, V, C> { pub fn iter_results( &self, - f: impl for<'a> FnOnce( - Box + 'a>, - ) -> R, + f: impl for<'a> FnOnce(Box + 'a>) -> R, ) -> R { self.cache.iter(&self.shards, |shard| &mut shard.cache, f) } @@ -103,10 +111,10 @@ impl<'tcx, M: QueryAccessors<'tcx>> QueryState<'tcx, M> { } } -impl<'tcx, M: QueryAccessors<'tcx>> Default for QueryState<'tcx, M> { - fn default() -> QueryState<'tcx, M> { - QueryState { - cache: M::Cache::default(), +impl<'tcx, K, V, C: QueryCache> Default for QueryStateImpl<'tcx, K, V, C> { + fn default() -> QueryStateImpl<'tcx, K, V, C> { + QueryStateImpl { + cache: C::default(), shards: Default::default(), #[cfg(debug_assertions)] cache_hits: AtomicUsize::new(0), @@ -441,7 +449,7 @@ impl<'tcx> TyCtxt<'tcx> { { let state = Q::query_state(self); - state.cache.lookup( + state.cache.lookup::<_, _, _, _, Q>( state, QueryStateShard::::get_cache, key, @@ -1035,7 +1043,7 @@ macro_rules! define_queries_inner { let mut string_cache = QueryKeyStringCache::new(); $({ - alloc_self_profile_query_strings_for_query_cache( + alloc_self_profile_query_strings_for_query_cache::>( self, stringify!($name), &self.queries.$name, From fa02dca428314676716c78b5aa1953eea1627bf0 Mon Sep 17 00:00:00 2001 From: Camille GILLOT Date: Fri, 6 Mar 2020 20:10:53 +0100 Subject: [PATCH 08/29] Remove Q parameter from QueryCache::lookup. --- src/librustc/ty/query/caches.rs | 25 +++++++++++++------------ src/librustc/ty/query/plumbing.rs | 2 +- 2 files changed, 14 insertions(+), 13 deletions(-) diff --git a/src/librustc/ty/query/caches.rs b/src/librustc/ty/query/caches.rs index efc2804bd4d59..ae01dcb364bfc 100644 --- a/src/librustc/ty/query/caches.rs +++ b/src/librustc/ty/query/caches.rs @@ -1,6 +1,5 @@ use crate::dep_graph::DepNodeIndex; -use crate::ty::query::config::QueryAccessors; -use crate::ty::query::plumbing::{QueryLookup, QueryState, QueryStateShard}; +use crate::ty::query::plumbing::{QueryLookupImpl, QueryStateImpl, QueryStateShardImpl}; use crate::ty::TyCtxt; use rustc_data_structures::fx::FxHashMap; @@ -19,9 +18,9 @@ pub(crate) trait QueryCache: Default { /// It returns the shard index and a lock guard to the shard, /// which will be used if the query is not in the cache and we need /// to compute it. - fn lookup<'tcx, R, GetCache, OnHit, OnMiss, Q>( + fn lookup<'tcx, R, GetCache, OnHit, OnMiss>( &self, - state: &'tcx QueryState<'tcx, Q>, + state: &'tcx QueryStateImpl<'tcx, K, V, Self>, get_cache: GetCache, key: K, // `on_hit` can be called while holding a lock to the query state shard. @@ -29,10 +28,11 @@ pub(crate) trait QueryCache: Default { on_miss: OnMiss, ) -> R where - Q: QueryAccessors<'tcx>, - GetCache: for<'a> Fn(&'a mut QueryStateShard<'tcx, Q>) -> &'a mut Self::Sharded, + GetCache: for<'a> Fn( + &'a mut QueryStateShardImpl<'tcx, K, Self::Sharded>, + ) -> &'a mut Self::Sharded, OnHit: FnOnce(&V, DepNodeIndex) -> R, - OnMiss: FnOnce(K, QueryLookup<'tcx, Q>) -> R; + OnMiss: FnOnce(K, QueryLookupImpl<'tcx, QueryStateShardImpl<'tcx, K, Self::Sharded>>) -> R; fn complete( &self, @@ -64,19 +64,20 @@ impl QueryCache for DefaultCache { type Sharded = FxHashMap; #[inline(always)] - fn lookup<'tcx, R, GetCache, OnHit, OnMiss, Q>( + fn lookup<'tcx, R, GetCache, OnHit, OnMiss>( &self, - state: &'tcx QueryState<'tcx, Q>, + state: &'tcx QueryStateImpl<'tcx, K, V, Self>, get_cache: GetCache, key: K, on_hit: OnHit, on_miss: OnMiss, ) -> R where - Q: QueryAccessors<'tcx>, - GetCache: for<'a> Fn(&'a mut QueryStateShard<'tcx, Q>) -> &'a mut Self::Sharded, + GetCache: for<'a> Fn( + &'a mut QueryStateShardImpl<'tcx, K, Self::Sharded>, + ) -> &'a mut Self::Sharded, OnHit: FnOnce(&V, DepNodeIndex) -> R, - OnMiss: FnOnce(K, QueryLookup<'tcx, Q>) -> R, + OnMiss: FnOnce(K, QueryLookupImpl<'tcx, QueryStateShardImpl<'tcx, K, Self::Sharded>>) -> R, { let mut lookup = state.get_lookup(&key); let lock = &mut *lookup.lock; diff --git a/src/librustc/ty/query/plumbing.rs b/src/librustc/ty/query/plumbing.rs index 37fc339ee6395..e4d0e96d07c75 100644 --- a/src/librustc/ty/query/plumbing.rs +++ b/src/librustc/ty/query/plumbing.rs @@ -449,7 +449,7 @@ impl<'tcx> TyCtxt<'tcx> { { let state = Q::query_state(self); - state.cache.lookup::<_, _, _, _, Q>( + state.cache.lookup( state, QueryStateShard::::get_cache, key, From a18aa81bd8dd3012e43f195c52b8113a74479056 Mon Sep 17 00:00:00 2001 From: Camille GILLOT Date: Fri, 6 Mar 2020 20:17:03 +0100 Subject: [PATCH 09/29] Remove Q parameter from alloc_self_profile_query_strings_for_query_cache. --- src/librustc/ty/query/plumbing.rs | 2 +- src/librustc/ty/query/profiling_support.rs | 11 ++++++----- 2 files changed, 7 insertions(+), 6 deletions(-) diff --git a/src/librustc/ty/query/plumbing.rs b/src/librustc/ty/query/plumbing.rs index e4d0e96d07c75..98d8de89d8e9e 100644 --- a/src/librustc/ty/query/plumbing.rs +++ b/src/librustc/ty/query/plumbing.rs @@ -1043,7 +1043,7 @@ macro_rules! define_queries_inner { let mut string_cache = QueryKeyStringCache::new(); $({ - alloc_self_profile_query_strings_for_query_cache::>( + alloc_self_profile_query_strings_for_query_cache( self, stringify!($name), &self.queries.$name, diff --git a/src/librustc/ty/query/profiling_support.rs b/src/librustc/ty/query/profiling_support.rs index 99ada34d59ebe..290e37f2cf825 100644 --- a/src/librustc/ty/query/profiling_support.rs +++ b/src/librustc/ty/query/profiling_support.rs @@ -1,7 +1,7 @@ use crate::hir::map::definitions::DefPathData; use crate::ty::context::TyCtxt; -use crate::ty::query::config::QueryAccessors; -use crate::ty::query::plumbing::QueryState; +use crate::ty::query::caches::QueryCache; +use crate::ty::query::plumbing::QueryStateImpl; use measureme::{StringComponent, StringId}; use rustc_data_structures::fx::FxHashMap; use rustc_data_structures::profiling::SelfProfiler; @@ -157,13 +157,14 @@ where /// Allocate the self-profiling query strings for a single query cache. This /// method is called from `alloc_self_profile_query_strings` which knows all /// the queries via macro magic. -pub(super) fn alloc_self_profile_query_strings_for_query_cache<'tcx, Q>( +pub(super) fn alloc_self_profile_query_strings_for_query_cache<'tcx, K, V, C>( tcx: TyCtxt<'tcx>, query_name: &'static str, - query_state: &QueryState<'tcx, Q>, + query_state: &QueryStateImpl<'tcx, K, V, C>, string_cache: &mut QueryKeyStringCache, ) where - Q: QueryAccessors<'tcx>, + K: Debug + Clone, + C: QueryCache, { tcx.prof.with_profiler(|profiler| { let event_id_builder = profiler.event_id_builder(); From d125bbb12b9b1839068706834d350acf5a91244c Mon Sep 17 00:00:00 2001 From: Camille GILLOT Date: Fri, 6 Mar 2020 20:33:13 +0100 Subject: [PATCH 10/29] Remove Q parameter from query stats. --- src/librustc/ty/query/stats.rs | 23 ++++++++++++++--------- 1 file changed, 14 insertions(+), 9 deletions(-) diff --git a/src/librustc/ty/query/stats.rs b/src/librustc/ty/query/stats.rs index d257320d4eaf6..c4c81cd5a13cf 100644 --- a/src/librustc/ty/query/stats.rs +++ b/src/librustc/ty/query/stats.rs @@ -1,5 +1,6 @@ -use crate::ty::query::config::QueryAccessors; -use crate::ty::query::plumbing::QueryState; +use crate::ty::query::caches::QueryCache; +use crate::ty::query::config::{QueryAccessors, QueryConfig}; +use crate::ty::query::plumbing::QueryStateImpl; use crate::ty::query::queries; use crate::ty::TyCtxt; use rustc_hir::def_id::{DefId, LOCAL_CRATE}; @@ -37,9 +38,9 @@ struct QueryStats { local_def_id_keys: Option, } -fn stats<'tcx, Q: QueryAccessors<'tcx>>( +fn stats<'tcx, K, V, C: QueryCache>( name: &'static str, - map: &QueryState<'tcx, Q>, + map: &QueryStateImpl<'tcx, K, V, C>, ) -> QueryStats { let mut stats = QueryStats { name, @@ -47,10 +48,10 @@ fn stats<'tcx, Q: QueryAccessors<'tcx>>( cache_hits: map.cache_hits.load(Ordering::Relaxed), #[cfg(not(debug_assertions))] cache_hits: 0, - key_size: mem::size_of::(), - key_type: type_name::(), - value_size: mem::size_of::(), - value_type: type_name::(), + key_size: mem::size_of::(), + key_type: type_name::(), + value_size: mem::size_of::(), + value_type: type_name::(), entry_count: map.iter_results(|results| results.count()), local_def_id_keys: None, }; @@ -125,7 +126,11 @@ macro_rules! print_stats { let mut queries = Vec::new(); $($( - queries.push(stats::>( + queries.push(stats::< + as QueryConfig<'_>>::Key, + as QueryConfig<'_>>::Value, + as QueryAccessors<'_>>::Cache, + >( stringify!($name), &tcx.queries.$name, )); From fa0794db239d588b5822233d32fa7aa82bd17069 Mon Sep 17 00:00:00 2001 From: Camille GILLOT Date: Sat, 7 Mar 2020 10:38:44 +0100 Subject: [PATCH 11/29] Remove Q parameter from JobOwner. --- src/librustc/ty/query/plumbing.rs | 61 ++++++++++++++++++++++--------- 1 file changed, 43 insertions(+), 18 deletions(-) diff --git a/src/librustc/ty/query/plumbing.rs b/src/librustc/ty/query/plumbing.rs index 98d8de89d8e9e..9228e569f5586 100644 --- a/src/librustc/ty/query/plumbing.rs +++ b/src/librustc/ty/query/plumbing.rs @@ -20,6 +20,7 @@ use rustc_errors::{struct_span_err, Diagnostic, DiagnosticBuilder, FatalError, H use rustc_span::source_map::DUMMY_SP; use rustc_span::Span; use std::collections::hash_map::Entry; +use std::fmt::Debug; use std::hash::{Hash, Hasher}; use std::mem; use std::num::NonZeroU32; @@ -132,13 +133,28 @@ pub(crate) struct QueryLookupImpl<'tcx, QSS> { /// A type representing the responsibility to execute the job in the `job` field. /// This will poison the relevant query if dropped. -pub(super) struct JobOwner<'tcx, Q: QueryDescription<'tcx>> { - tcx: TyCtxt<'tcx>, - key: Q::Key, +pub(super) type JobOwner<'tcx, Q> = JobOwnerImpl< + 'tcx, + >::Key, + >::Value, + >::Cache, +>; + +pub(super) struct JobOwnerImpl<'tcx, K, V, C: QueryCache> +where + K: Eq + Hash + Clone + Debug, + V: Clone, +{ + state: &'tcx QueryStateImpl<'tcx, K, V, C>, + key: K, id: QueryJobId, } -impl<'tcx, Q: QueryDescription<'tcx> + 'tcx> JobOwner<'tcx, Q> { +impl<'tcx, K, V, C: QueryCache> JobOwnerImpl<'tcx, K, V, C> +where + K: Eq + Hash + Clone + Debug, + V: Clone, +{ /// Either gets a `JobOwner` corresponding the query, allowing us to /// start executing the query, or returns with the result of the query. /// This function assumes that `try_get_cached` is already called and returned `lookup`. @@ -148,12 +164,17 @@ impl<'tcx, Q: QueryDescription<'tcx> + 'tcx> JobOwner<'tcx, Q> { /// This function is inlined because that results in a noticeable speed-up /// for some compile-time benchmarks. #[inline(always)] - pub(super) fn try_start( + pub(super) fn try_start( tcx: TyCtxt<'tcx>, span: Span, - key: &Q::Key, + key: &K, mut lookup: QueryLookup<'tcx, Q>, - ) -> TryGetJob<'tcx, Q> { + ) -> TryGetJob<'tcx, Q> + where + K: Eq + Hash + Clone + Debug, + V: Clone, + Q: QueryDescription<'tcx, Key = K, Value = V, Cache = C> + 'tcx, + { let lock = &mut *lookup.lock; let (latch, mut _query_blocked_prof_timer) = match lock.active.entry((*key).clone()) { @@ -191,7 +212,8 @@ impl<'tcx, Q: QueryDescription<'tcx> + 'tcx> JobOwner<'tcx, Q> { entry.insert(QueryResult::Started(job)); - let owner = JobOwner { tcx, id: global_id, key: (*key).clone() }; + let owner = + JobOwnerImpl { state: Q::query_state(tcx), id: global_id, key: (*key).clone() }; return TryGetJob::NotYetStarted(owner); } }; @@ -231,16 +253,15 @@ impl<'tcx, Q: QueryDescription<'tcx> + 'tcx> JobOwner<'tcx, Q> { /// Completes the query by updating the query cache with the `result`, /// signals the waiter and forgets the JobOwner, so it won't poison the query #[inline(always)] - pub(super) fn complete(self, result: &Q::Value, dep_node_index: DepNodeIndex) { + pub(super) fn complete(self, tcx: TyCtxt<'tcx>, result: &V, dep_node_index: DepNodeIndex) { // We can move out of `self` here because we `mem::forget` it below let key = unsafe { ptr::read(&self.key) }; - let tcx = self.tcx; + let state = self.state; // Forget ourself so our destructor won't poison the query mem::forget(self); let job = { - let state = Q::query_state(tcx); let result = result.clone(); let mut lock = state.shards.get_shard_by_value(&key).lock(); let job = match lock.active.remove(&key).unwrap() { @@ -265,12 +286,16 @@ where (result, diagnostics.into_inner()) } -impl<'tcx, Q: QueryDescription<'tcx>> Drop for JobOwner<'tcx, Q> { +impl<'tcx, K, V, C: QueryCache> Drop for JobOwnerImpl<'tcx, K, V, C> +where + K: Eq + Hash + Clone + Debug, + V: Clone, +{ #[inline(never)] #[cold] fn drop(&mut self) { // Poison the query so jobs waiting on it panic. - let state = Q::query_state(self.tcx); + let state = self.state; let shard = state.shards.get_shard_by_value(&self.key); let job = { let mut shard = shard.lock(); @@ -492,7 +517,7 @@ impl<'tcx> TyCtxt<'tcx> { key: Q::Key, lookup: QueryLookup<'tcx, Q>, ) -> Q::Value { - let job = match JobOwner::try_start(self, span, &key, lookup) { + let job = match JobOwnerImpl::try_start::(self, span, &key, lookup) { TryGetJob::NotYetStarted(job) => job, TryGetJob::Cycle(result) => return result, #[cfg(parallel_compiler)] @@ -528,7 +553,7 @@ impl<'tcx> TyCtxt<'tcx> { .store_diagnostics_for_anon_node(dep_node_index, diagnostics); } - job.complete(&result, dep_node_index); + job.complete(self, &result, dep_node_index); return result; } @@ -554,7 +579,7 @@ impl<'tcx> TyCtxt<'tcx> { }) }); if let Some((result, dep_node_index)) = loaded { - job.complete(&result, dep_node_index); + job.complete(self, &result, dep_node_index); return result; } } @@ -696,7 +721,7 @@ impl<'tcx> TyCtxt<'tcx> { } } - job.complete(&result, dep_node_index); + job.complete(self, &result, dep_node_index); (result, dep_node_index) } @@ -751,7 +776,7 @@ impl<'tcx> TyCtxt<'tcx> { // Cache hit, do nothing }, |key, lookup| { - let job = match JobOwner::try_start(self, span, &key, lookup) { + let job = match JobOwnerImpl::try_start::(self, span, &key, lookup) { TryGetJob::NotYetStarted(job) => job, TryGetJob::Cycle(_) => return, #[cfg(parallel_compiler)] From 5dc7c2ed1aab0c6b0bfb15b4fb5a4d079d3aaa36 Mon Sep 17 00:00:00 2001 From: Camille GILLOT Date: Sat, 7 Mar 2020 17:19:47 +0100 Subject: [PATCH 12/29] Remove Q parameter from try_get_cached. --- src/librustc/ty/query/plumbing.rs | 24 +++++++++++++----------- 1 file changed, 13 insertions(+), 11 deletions(-) diff --git a/src/librustc/ty/query/plumbing.rs b/src/librustc/ty/query/plumbing.rs index 9228e569f5586..1ff7e44bfbb9c 100644 --- a/src/librustc/ty/query/plumbing.rs +++ b/src/librustc/ty/query/plumbing.rs @@ -236,7 +236,8 @@ where return TryGetJob::Cycle(Q::handle_cycle_error(tcx, cycle)); } - let cached = tcx.try_get_cached::( + let cached = tcx.try_get_cached( + Q::query_state(tcx), (*key).clone(), |value, index| (value.clone(), index), |_, _| panic!("value must be in cache after waiting"), @@ -460,23 +461,22 @@ impl<'tcx> TyCtxt<'tcx> { /// which will be used if the query is not in the cache and we need /// to compute it. #[inline(always)] - fn try_get_cached( + fn try_get_cached( self, - key: Q::Key, + state: &'tcx QueryStateImpl<'tcx, K, V, C>, + key: K, // `on_hit` can be called while holding a lock to the query cache on_hit: OnHit, on_miss: OnMiss, ) -> R where - Q: QueryDescription<'tcx> + 'tcx, - OnHit: FnOnce(&Q::Value, DepNodeIndex) -> R, - OnMiss: FnOnce(Q::Key, QueryLookup<'tcx, Q>) -> R, + C: QueryCache, + OnHit: FnOnce(&V, DepNodeIndex) -> R, + OnMiss: FnOnce(K, QueryLookupImpl<'tcx, QueryStateShardImpl<'tcx, K, C::Sharded>>) -> R, { - let state = Q::query_state(self); - state.cache.lookup( state, - QueryStateShard::::get_cache, + QueryStateShardImpl::::get_cache, key, |value, index| { if unlikely!(self.prof.enabled()) { @@ -500,7 +500,8 @@ impl<'tcx> TyCtxt<'tcx> { ) -> Q::Value { debug!("ty::query::get_query<{}>(key={:?}, span={:?})", Q::NAME, key, span); - self.try_get_cached::( + self.try_get_cached( + Q::query_state(self), key, |value, index| { self.dep_graph.read_index(index); @@ -770,7 +771,8 @@ impl<'tcx> TyCtxt<'tcx> { // We may be concurrently trying both execute and force a query. // Ensure that only one of them runs the query. - self.try_get_cached::( + self.try_get_cached( + Q::query_state(self), key, |_, _| { // Cache hit, do nothing From 7d84f4fb169d55b0423e4e379828700ec0214400 Mon Sep 17 00:00:00 2001 From: Camille GILLOT Date: Sat, 7 Mar 2020 11:03:49 +0100 Subject: [PATCH 13/29] Offload try_collect_active_jobs. --- src/librustc/ty/query/mod.rs | 1 - src/librustc/ty/query/plumbing.rs | 62 ++++++++++++++++++------------- 2 files changed, 37 insertions(+), 26 deletions(-) diff --git a/src/librustc/ty/query/mod.rs b/src/librustc/ty/query/mod.rs index 3d17883fec3bd..54e80b4dc0b15 100644 --- a/src/librustc/ty/query/mod.rs +++ b/src/librustc/ty/query/mod.rs @@ -57,7 +57,6 @@ use rustc_span::symbol::Symbol; use rustc_span::{Span, DUMMY_SP}; use std::borrow::Cow; use std::collections::BTreeMap; -use std::convert::TryFrom; use std::ops::Deref; use std::sync::Arc; diff --git a/src/librustc/ty/query/plumbing.rs b/src/librustc/ty/query/plumbing.rs index 1ff7e44bfbb9c..406ca18b59105 100644 --- a/src/librustc/ty/query/plumbing.rs +++ b/src/librustc/ty/query/plumbing.rs @@ -2,10 +2,10 @@ //! generate the actual methods on tcx which find and execute the provider, //! manage the caches, and so forth. -use crate::dep_graph::{DepNode, DepNodeIndex, SerializedDepNodeIndex}; +use crate::dep_graph::{DepKind, DepNode, DepNodeIndex, SerializedDepNodeIndex}; use crate::ty::query::caches::QueryCache; use crate::ty::query::config::{QueryAccessors, QueryConfig, QueryDescription}; -use crate::ty::query::job::{QueryInfo, QueryJob, QueryJobId, QueryShardJobId}; +use crate::ty::query::job::{QueryInfo, QueryJob, QueryJobId, QueryJobInfo, QueryShardJobId}; use crate::ty::query::Query; use crate::ty::tls; use crate::ty::{self, TyCtxt}; @@ -20,6 +20,7 @@ use rustc_errors::{struct_span_err, Diagnostic, DiagnosticBuilder, FatalError, H use rustc_span::source_map::DUMMY_SP; use rustc_span::Span; use std::collections::hash_map::Entry; +use std::convert::TryFrom; use std::fmt::Debug; use std::hash::{Hash, Hasher}; use std::mem; @@ -110,6 +111,35 @@ impl<'tcx, K, V, C: QueryCache> QueryStateImpl<'tcx, K, V, C> { let shards = self.shards.lock_shards(); shards.iter().all(|shard| shard.active.is_empty()) } + + pub(super) fn try_collect_active_jobs( + &self, + kind: DepKind, + make_query: fn(K) -> Query<'tcx>, + jobs: &mut FxHashMap>, + ) -> Option<()> + where + K: Clone, + { + // We use try_lock_shards here since we are called from the + // deadlock handler, and this shouldn't be locked. + let shards = self.shards.try_lock_shards()?; + let shards = shards.iter().enumerate(); + jobs.extend(shards.flat_map(|(shard_id, shard)| { + shard.active.iter().filter_map(move |(k, v)| { + if let QueryResult::Started(ref job) = *v { + let id = + QueryJobId { job: job.id, shard: u16::try_from(shard_id).unwrap(), kind }; + let info = QueryInfo { span: job.span, query: make_query(k.clone()) }; + Some((id, QueryJobInfo { info, job: job.clone() })) + } else { + None + } + }) + })); + + Some(()) + } } impl<'tcx, K, V, C: QueryCache> Default for QueryStateImpl<'tcx, K, V, C> { @@ -1135,29 +1165,11 @@ macro_rules! define_queries_struct { let mut jobs = FxHashMap::default(); $( - // We use try_lock_shards here since we are called from the - // deadlock handler, and this shouldn't be locked. - let shards = self.$name.shards.try_lock_shards()?; - let shards = shards.iter().enumerate(); - jobs.extend(shards.flat_map(|(shard_id, shard)| { - shard.active.iter().filter_map(move |(k, v)| { - if let QueryResult::Started(ref job) = *v { - let id = QueryJobId { - job: job.id, - shard: u16::try_from(shard_id).unwrap(), - kind: - as QueryAccessors<'tcx>>::DEP_KIND, - }; - let info = QueryInfo { - span: job.span, - query: Query::$name(k.clone()) - }; - Some((id, QueryJobInfo { info, job: job.clone() })) - } else { - None - } - }) - })); + self.$name.try_collect_active_jobs( + as QueryAccessors<'tcx>>::DEP_KIND, + Query::$name, + &mut jobs, + )?; )* Some(jobs) From 7309b3cd8be928a0ec9e3219d9562ddeb54ff994 Mon Sep 17 00:00:00 2001 From: Camille GILLOT Date: Sat, 7 Mar 2020 17:56:59 +0100 Subject: [PATCH 14/29] Simplify type aliases. --- src/librustc/ty/query/caches.rs | 16 ++++++------ src/librustc/ty/query/plumbing.rs | 41 +++++++++++-------------------- 2 files changed, 21 insertions(+), 36 deletions(-) diff --git a/src/librustc/ty/query/caches.rs b/src/librustc/ty/query/caches.rs index ae01dcb364bfc..17639545d12a1 100644 --- a/src/librustc/ty/query/caches.rs +++ b/src/librustc/ty/query/caches.rs @@ -1,5 +1,5 @@ use crate::dep_graph::DepNodeIndex; -use crate::ty::query::plumbing::{QueryLookupImpl, QueryStateImpl, QueryStateShardImpl}; +use crate::ty::query::plumbing::{QueryLookup, QueryStateImpl, QueryStateShard}; use crate::ty::TyCtxt; use rustc_data_structures::fx::FxHashMap; @@ -28,11 +28,10 @@ pub(crate) trait QueryCache: Default { on_miss: OnMiss, ) -> R where - GetCache: for<'a> Fn( - &'a mut QueryStateShardImpl<'tcx, K, Self::Sharded>, - ) -> &'a mut Self::Sharded, + GetCache: + for<'a> Fn(&'a mut QueryStateShard<'tcx, K, Self::Sharded>) -> &'a mut Self::Sharded, OnHit: FnOnce(&V, DepNodeIndex) -> R, - OnMiss: FnOnce(K, QueryLookupImpl<'tcx, QueryStateShardImpl<'tcx, K, Self::Sharded>>) -> R; + OnMiss: FnOnce(K, QueryLookup<'tcx, K, Self::Sharded>) -> R; fn complete( &self, @@ -73,11 +72,10 @@ impl QueryCache for DefaultCache { on_miss: OnMiss, ) -> R where - GetCache: for<'a> Fn( - &'a mut QueryStateShardImpl<'tcx, K, Self::Sharded>, - ) -> &'a mut Self::Sharded, + GetCache: + for<'a> Fn(&'a mut QueryStateShard<'tcx, K, Self::Sharded>) -> &'a mut Self::Sharded, OnHit: FnOnce(&V, DepNodeIndex) -> R, - OnMiss: FnOnce(K, QueryLookupImpl<'tcx, QueryStateShardImpl<'tcx, K, Self::Sharded>>) -> R, + OnMiss: FnOnce(K, QueryLookup<'tcx, K, Self::Sharded>) -> R, { let mut lookup = state.get_lookup(&key); let lock = &mut *lookup.lock; diff --git a/src/librustc/ty/query/plumbing.rs b/src/librustc/ty/query/plumbing.rs index 406ca18b59105..467b1a7e4a177 100644 --- a/src/librustc/ty/query/plumbing.rs +++ b/src/librustc/ty/query/plumbing.rs @@ -29,16 +29,7 @@ use std::ptr; #[cfg(debug_assertions)] use std::sync::atomic::{AtomicUsize, Ordering}; -pub(crate) type QueryStateShard<'tcx, Q> = QueryStateShardImpl< - 'tcx, - >::Key, - <>::Cache as QueryCache< - >::Key, - >::Value, - >>::Sharded, ->; - -pub(crate) struct QueryStateShardImpl<'tcx, K, C> { +pub(crate) struct QueryStateShard<'tcx, K, C> { pub(super) cache: C, pub(super) active: FxHashMap>, @@ -46,15 +37,15 @@ pub(crate) struct QueryStateShardImpl<'tcx, K, C> { pub(super) jobs: u32, } -impl<'tcx, K, C> QueryStateShardImpl<'tcx, K, C> { +impl<'tcx, K, C> QueryStateShard<'tcx, K, C> { fn get_cache(&mut self) -> &mut C { &mut self.cache } } -impl<'tcx, K, C: Default> Default for QueryStateShardImpl<'tcx, K, C> { - fn default() -> QueryStateShardImpl<'tcx, K, C> { - QueryStateShardImpl { cache: Default::default(), active: Default::default(), jobs: 0 } +impl<'tcx, K, C: Default> Default for QueryStateShard<'tcx, K, C> { + fn default() -> QueryStateShard<'tcx, K, C> { + QueryStateShard { cache: Default::default(), active: Default::default(), jobs: 0 } } } @@ -67,16 +58,13 @@ pub(crate) type QueryState<'tcx, Q> = QueryStateImpl< pub(crate) struct QueryStateImpl<'tcx, K, V, C: QueryCache> { pub(super) cache: C, - pub(super) shards: Sharded>, + pub(super) shards: Sharded>, #[cfg(debug_assertions)] pub(super) cache_hits: AtomicUsize, } impl<'tcx, K, V, C: QueryCache> QueryStateImpl<'tcx, K, V, C> { - pub(super) fn get_lookup( - &'tcx self, - key: &K2, - ) -> QueryLookupImpl<'tcx, QueryStateShardImpl<'tcx, K, C::Sharded>> { + pub(super) fn get_lookup(&'tcx self, key: &K2) -> QueryLookup<'tcx, K, C::Sharded> { // We compute the key's hash once and then use it for both the // shard lookup and the hashmap lookup. This relies on the fact // that both of them use `FxHasher`. @@ -86,7 +74,7 @@ impl<'tcx, K, V, C: QueryCache> QueryStateImpl<'tcx, K, V, C> { let shard = self.shards.get_shard_index_by_hash(key_hash); let lock = self.shards.get_shard_by_index(shard).lock(); - QueryLookupImpl { key_hash, shard, lock } + QueryLookup { key_hash, shard, lock } } } @@ -154,11 +142,10 @@ impl<'tcx, K, V, C: QueryCache> Default for QueryStateImpl<'tcx, K, V, C> } /// Values used when checking a query cache which can be reused on a cache-miss to execute the query. -pub(crate) type QueryLookup<'tcx, Q> = QueryLookupImpl<'tcx, QueryStateShard<'tcx, Q>>; -pub(crate) struct QueryLookupImpl<'tcx, QSS> { +pub(crate) struct QueryLookup<'tcx, K, C> { pub(super) key_hash: u64, pub(super) shard: usize, - pub(super) lock: LockGuard<'tcx, QSS>, + pub(super) lock: LockGuard<'tcx, QueryStateShard<'tcx, K, C>>, } /// A type representing the responsibility to execute the job in the `job` field. @@ -198,7 +185,7 @@ where tcx: TyCtxt<'tcx>, span: Span, key: &K, - mut lookup: QueryLookup<'tcx, Q>, + mut lookup: QueryLookup<'tcx, K, C::Sharded>, ) -> TryGetJob<'tcx, Q> where K: Eq + Hash + Clone + Debug, @@ -502,11 +489,11 @@ impl<'tcx> TyCtxt<'tcx> { where C: QueryCache, OnHit: FnOnce(&V, DepNodeIndex) -> R, - OnMiss: FnOnce(K, QueryLookupImpl<'tcx, QueryStateShardImpl<'tcx, K, C::Sharded>>) -> R, + OnMiss: FnOnce(K, QueryLookup<'tcx, K, C::Sharded>) -> R, { state.cache.lookup( state, - QueryStateShardImpl::::get_cache, + QueryStateShard::::get_cache, key, |value, index| { if unlikely!(self.prof.enabled()) { @@ -546,7 +533,7 @@ impl<'tcx> TyCtxt<'tcx> { self, span: Span, key: Q::Key, - lookup: QueryLookup<'tcx, Q>, + lookup: QueryLookup<'tcx, Q::Key, >::Sharded>, ) -> Q::Value { let job = match JobOwnerImpl::try_start::(self, span, &key, lookup) { TryGetJob::NotYetStarted(job) => job, From 3abd4753b7c9fa9a3855d4b41b557f81b3e06aab Mon Sep 17 00:00:00 2001 From: Camille GILLOT Date: Sat, 7 Mar 2020 18:36:24 +0100 Subject: [PATCH 15/29] Make QueryCache parameters associated types. --- src/librustc/ty/query/caches.rs | 45 +++++--- src/librustc/ty/query/config.rs | 7 +- src/librustc/ty/query/plumbing.rs | 118 +++++++++++---------- src/librustc/ty/query/profiling_support.rs | 8 +- src/librustc/ty/query/stats.rs | 17 ++- 5 files changed, 101 insertions(+), 94 deletions(-) diff --git a/src/librustc/ty/query/caches.rs b/src/librustc/ty/query/caches.rs index 17639545d12a1..71523ea39ca3e 100644 --- a/src/librustc/ty/query/caches.rs +++ b/src/librustc/ty/query/caches.rs @@ -6,12 +6,15 @@ use rustc_data_structures::fx::FxHashMap; use rustc_data_structures::sharded::Sharded; use std::default::Default; use std::hash::Hash; +use std::marker::PhantomData; pub(crate) trait CacheSelector { - type Cache: QueryCache; + type Cache: QueryCache; } -pub(crate) trait QueryCache: Default { +pub(crate) trait QueryCache: Default { + type Key; + type Value; type Sharded: Default; /// Checks if the query is already computed and in the cache. @@ -20,25 +23,26 @@ pub(crate) trait QueryCache: Default { /// to compute it. fn lookup<'tcx, R, GetCache, OnHit, OnMiss>( &self, - state: &'tcx QueryStateImpl<'tcx, K, V, Self>, + state: &'tcx QueryStateImpl<'tcx, Self>, get_cache: GetCache, - key: K, + key: Self::Key, // `on_hit` can be called while holding a lock to the query state shard. on_hit: OnHit, on_miss: OnMiss, ) -> R where - GetCache: - for<'a> Fn(&'a mut QueryStateShard<'tcx, K, Self::Sharded>) -> &'a mut Self::Sharded, - OnHit: FnOnce(&V, DepNodeIndex) -> R, - OnMiss: FnOnce(K, QueryLookup<'tcx, K, Self::Sharded>) -> R; + GetCache: for<'a> Fn( + &'a mut QueryStateShard<'tcx, Self::Key, Self::Sharded>, + ) -> &'a mut Self::Sharded, + OnHit: FnOnce(&Self::Value, DepNodeIndex) -> R, + OnMiss: FnOnce(Self::Key, QueryLookup<'tcx, Self::Key, Self::Sharded>) -> R; fn complete( &self, tcx: TyCtxt<'tcx>, lock_sharded_storage: &mut Self::Sharded, - key: K, - value: V, + key: Self::Key, + value: Self::Value, index: DepNodeIndex, ); @@ -46,26 +50,35 @@ pub(crate) trait QueryCache: Default { &self, shards: &Sharded, get_shard: impl Fn(&mut L) -> &mut Self::Sharded, - f: impl for<'a> FnOnce(Box + 'a>) -> R, + f: impl for<'a> FnOnce( + Box + 'a>, + ) -> R, ) -> R; } pub struct DefaultCacheSelector; impl CacheSelector for DefaultCacheSelector { - type Cache = DefaultCache; + type Cache = DefaultCache; } -#[derive(Default)] -pub struct DefaultCache; +pub struct DefaultCache(PhantomData<(K, V)>); + +impl Default for DefaultCache { + fn default() -> Self { + DefaultCache(PhantomData) + } +} -impl QueryCache for DefaultCache { +impl QueryCache for DefaultCache { + type Key = K; + type Value = V; type Sharded = FxHashMap; #[inline(always)] fn lookup<'tcx, R, GetCache, OnHit, OnMiss>( &self, - state: &'tcx QueryStateImpl<'tcx, K, V, Self>, + state: &'tcx QueryStateImpl<'tcx, Self>, get_cache: GetCache, key: K, on_hit: OnHit, diff --git a/src/librustc/ty/query/config.rs b/src/librustc/ty/query/config.rs index 29b9e0c3b40ef..5b0653bd599e1 100644 --- a/src/librustc/ty/query/config.rs +++ b/src/librustc/ty/query/config.rs @@ -30,7 +30,7 @@ pub(crate) trait QueryAccessors<'tcx>: QueryConfig<'tcx> { const EVAL_ALWAYS: bool; const DEP_KIND: DepKind; - type Cache: QueryCache; + type Cache: QueryCache; // Don't use this method to access query results, instead use the methods on TyCtxt fn query_state<'a>(tcx: TyCtxt<'tcx>) -> &'a QueryState<'tcx, Self>; @@ -59,10 +59,7 @@ pub(crate) trait QueryDescription<'tcx>: QueryAccessors<'tcx> { } } -impl<'tcx, M: QueryAccessors<'tcx, Key = DefId>> QueryDescription<'tcx> for M -where - >::Cache: QueryCache>::Value>, -{ +impl<'tcx, M: QueryAccessors<'tcx, Key = DefId>> QueryDescription<'tcx> for M { default fn describe(tcx: TyCtxt<'_>, def_id: DefId) -> Cow<'static, str> { if !tcx.sess.verbose() { format!("processing `{}`", tcx.def_path_str(def_id)).into() diff --git a/src/librustc/ty/query/plumbing.rs b/src/librustc/ty/query/plumbing.rs index 467b1a7e4a177..c2740733977e8 100644 --- a/src/librustc/ty/query/plumbing.rs +++ b/src/librustc/ty/query/plumbing.rs @@ -4,7 +4,7 @@ use crate::dep_graph::{DepKind, DepNode, DepNodeIndex, SerializedDepNodeIndex}; use crate::ty::query::caches::QueryCache; -use crate::ty::query::config::{QueryAccessors, QueryConfig, QueryDescription}; +use crate::ty::query::config::{QueryAccessors, QueryDescription}; use crate::ty::query::job::{QueryInfo, QueryJob, QueryJobId, QueryJobInfo, QueryShardJobId}; use crate::ty::query::Query; use crate::ty::tls; @@ -49,22 +49,20 @@ impl<'tcx, K, C: Default> Default for QueryStateShard<'tcx, K, C> { } } -pub(crate) type QueryState<'tcx, Q> = QueryStateImpl< - 'tcx, - >::Key, - >::Value, - >::Cache, ->; +pub(crate) type QueryState<'tcx, Q> = QueryStateImpl<'tcx, >::Cache>; -pub(crate) struct QueryStateImpl<'tcx, K, V, C: QueryCache> { +pub(crate) struct QueryStateImpl<'tcx, C: QueryCache> { pub(super) cache: C, - pub(super) shards: Sharded>, + pub(super) shards: Sharded>, #[cfg(debug_assertions)] pub(super) cache_hits: AtomicUsize, } -impl<'tcx, K, V, C: QueryCache> QueryStateImpl<'tcx, K, V, C> { - pub(super) fn get_lookup(&'tcx self, key: &K2) -> QueryLookup<'tcx, K, C::Sharded> { +impl<'tcx, C: QueryCache> QueryStateImpl<'tcx, C> { + pub(super) fn get_lookup( + &'tcx self, + key: &K2, + ) -> QueryLookup<'tcx, C::Key, C::Sharded> { // We compute the key's hash once and then use it for both the // shard lookup and the hashmap lookup. This relies on the fact // that both of them use `FxHasher`. @@ -88,10 +86,12 @@ pub(super) enum QueryResult<'tcx> { Poisoned, } -impl<'tcx, K, V, C: QueryCache> QueryStateImpl<'tcx, K, V, C> { +impl<'tcx, C: QueryCache> QueryStateImpl<'tcx, C> { pub fn iter_results( &self, - f: impl for<'a> FnOnce(Box + 'a>) -> R, + f: impl for<'a> FnOnce( + Box + 'a>, + ) -> R, ) -> R { self.cache.iter(&self.shards, |shard| &mut shard.cache, f) } @@ -103,11 +103,11 @@ impl<'tcx, K, V, C: QueryCache> QueryStateImpl<'tcx, K, V, C> { pub(super) fn try_collect_active_jobs( &self, kind: DepKind, - make_query: fn(K) -> Query<'tcx>, + make_query: fn(C::Key) -> Query<'tcx>, jobs: &mut FxHashMap>, ) -> Option<()> where - K: Clone, + C::Key: Clone, { // We use try_lock_shards here since we are called from the // deadlock handler, and this shouldn't be locked. @@ -130,8 +130,8 @@ impl<'tcx, K, V, C: QueryCache> QueryStateImpl<'tcx, K, V, C> { } } -impl<'tcx, K, V, C: QueryCache> Default for QueryStateImpl<'tcx, K, V, C> { - fn default() -> QueryStateImpl<'tcx, K, V, C> { +impl<'tcx, C: QueryCache> Default for QueryStateImpl<'tcx, C> { + fn default() -> QueryStateImpl<'tcx, C> { QueryStateImpl { cache: C::default(), shards: Default::default(), @@ -150,27 +150,22 @@ pub(crate) struct QueryLookup<'tcx, K, C> { /// A type representing the responsibility to execute the job in the `job` field. /// This will poison the relevant query if dropped. -pub(super) type JobOwner<'tcx, Q> = JobOwnerImpl< - 'tcx, - >::Key, - >::Value, - >::Cache, ->; - -pub(super) struct JobOwnerImpl<'tcx, K, V, C: QueryCache> +pub(super) struct JobOwner<'tcx, C> where - K: Eq + Hash + Clone + Debug, - V: Clone, + C: QueryCache, + C::Key: Eq + Hash + Clone + Debug, + C::Value: Clone, { - state: &'tcx QueryStateImpl<'tcx, K, V, C>, - key: K, + state: &'tcx QueryStateImpl<'tcx, C>, + key: C::Key, id: QueryJobId, } -impl<'tcx, K, V, C: QueryCache> JobOwnerImpl<'tcx, K, V, C> +impl<'tcx, C: QueryCache> JobOwner<'tcx, C> where - K: Eq + Hash + Clone + Debug, - V: Clone, + C: QueryCache, + C::Key: Eq + Hash + Clone + Debug, + C::Value: Clone, { /// Either gets a `JobOwner` corresponding the query, allowing us to /// start executing the query, or returns with the result of the query. @@ -184,13 +179,11 @@ where pub(super) fn try_start( tcx: TyCtxt<'tcx>, span: Span, - key: &K, - mut lookup: QueryLookup<'tcx, K, C::Sharded>, - ) -> TryGetJob<'tcx, Q> + key: &C::Key, + mut lookup: QueryLookup<'tcx, C::Key, C::Sharded>, + ) -> TryGetJob<'tcx, C> where - K: Eq + Hash + Clone + Debug, - V: Clone, - Q: QueryDescription<'tcx, Key = K, Value = V, Cache = C> + 'tcx, + Q: QueryDescription<'tcx, Key = C::Key, Value = C::Value, Cache = C>, { let lock = &mut *lookup.lock; @@ -230,7 +223,7 @@ where entry.insert(QueryResult::Started(job)); let owner = - JobOwnerImpl { state: Q::query_state(tcx), id: global_id, key: (*key).clone() }; + JobOwner { state: Q::query_state(tcx), id: global_id, key: (*key).clone() }; return TryGetJob::NotYetStarted(owner); } }; @@ -271,7 +264,12 @@ where /// Completes the query by updating the query cache with the `result`, /// signals the waiter and forgets the JobOwner, so it won't poison the query #[inline(always)] - pub(super) fn complete(self, tcx: TyCtxt<'tcx>, result: &V, dep_node_index: DepNodeIndex) { + pub(super) fn complete( + self, + tcx: TyCtxt<'tcx>, + result: &C::Value, + dep_node_index: DepNodeIndex, + ) { // We can move out of `self` here because we `mem::forget` it below let key = unsafe { ptr::read(&self.key) }; let state = self.state; @@ -304,10 +302,10 @@ where (result, diagnostics.into_inner()) } -impl<'tcx, K, V, C: QueryCache> Drop for JobOwnerImpl<'tcx, K, V, C> +impl<'tcx, C: QueryCache> Drop for JobOwner<'tcx, C> where - K: Eq + Hash + Clone + Debug, - V: Clone, + C::Key: Eq + Hash + Clone + Debug, + C::Value: Clone, { #[inline(never)] #[cold] @@ -338,18 +336,22 @@ pub struct CycleError<'tcx> { } /// The result of `try_start`. -pub(super) enum TryGetJob<'tcx, D: QueryDescription<'tcx>> { +pub(super) enum TryGetJob<'tcx, C: QueryCache> +where + C::Key: Eq + Hash + Clone + Debug, + C::Value: Clone, +{ /// The query is not yet started. Contains a guard to the cache eventually used to start it. - NotYetStarted(JobOwner<'tcx, D>), + NotYetStarted(JobOwner<'tcx, C>), /// The query was already completed. /// Returns the result of the query and its dep-node index /// if it succeeded or a cycle error if it failed. #[cfg(parallel_compiler)] - JobCompleted((D::Value, DepNodeIndex)), + JobCompleted((C::Value, DepNodeIndex)), /// Trying to execute the query resulted in a cycle. - Cycle(D::Value), + Cycle(C::Value), } impl<'tcx> TyCtxt<'tcx> { @@ -478,22 +480,22 @@ impl<'tcx> TyCtxt<'tcx> { /// which will be used if the query is not in the cache and we need /// to compute it. #[inline(always)] - fn try_get_cached( + fn try_get_cached( self, - state: &'tcx QueryStateImpl<'tcx, K, V, C>, - key: K, + state: &'tcx QueryStateImpl<'tcx, C>, + key: C::Key, // `on_hit` can be called while holding a lock to the query cache on_hit: OnHit, on_miss: OnMiss, ) -> R where - C: QueryCache, - OnHit: FnOnce(&V, DepNodeIndex) -> R, - OnMiss: FnOnce(K, QueryLookup<'tcx, K, C::Sharded>) -> R, + C: QueryCache, + OnHit: FnOnce(&C::Value, DepNodeIndex) -> R, + OnMiss: FnOnce(C::Key, QueryLookup<'tcx, C::Key, C::Sharded>) -> R, { state.cache.lookup( state, - QueryStateShard::::get_cache, + QueryStateShard::::get_cache, key, |value, index| { if unlikely!(self.prof.enabled()) { @@ -533,9 +535,9 @@ impl<'tcx> TyCtxt<'tcx> { self, span: Span, key: Q::Key, - lookup: QueryLookup<'tcx, Q::Key, >::Sharded>, + lookup: QueryLookup<'tcx, Q::Key, ::Sharded>, ) -> Q::Value { - let job = match JobOwnerImpl::try_start::(self, span, &key, lookup) { + let job = match JobOwner::try_start::(self, span, &key, lookup) { TryGetJob::NotYetStarted(job) => job, TryGetJob::Cycle(result) => return result, #[cfg(parallel_compiler)] @@ -696,7 +698,7 @@ impl<'tcx> TyCtxt<'tcx> { fn force_query_with_job + 'tcx>( self, key: Q::Key, - job: JobOwner<'tcx, Q>, + job: JobOwner<'tcx, Q::Cache>, dep_node: DepNode, ) -> (Q::Value, DepNodeIndex) { // If the following assertion triggers, it can have two reasons: @@ -795,7 +797,7 @@ impl<'tcx> TyCtxt<'tcx> { // Cache hit, do nothing }, |key, lookup| { - let job = match JobOwnerImpl::try_start::(self, span, &key, lookup) { + let job = match JobOwner::try_start::(self, span, &key, lookup) { TryGetJob::NotYetStarted(job) => job, TryGetJob::Cycle(_) => return, #[cfg(parallel_compiler)] diff --git a/src/librustc/ty/query/profiling_support.rs b/src/librustc/ty/query/profiling_support.rs index 290e37f2cf825..256bd86a3de38 100644 --- a/src/librustc/ty/query/profiling_support.rs +++ b/src/librustc/ty/query/profiling_support.rs @@ -157,14 +157,14 @@ where /// Allocate the self-profiling query strings for a single query cache. This /// method is called from `alloc_self_profile_query_strings` which knows all /// the queries via macro magic. -pub(super) fn alloc_self_profile_query_strings_for_query_cache<'tcx, K, V, C>( +pub(super) fn alloc_self_profile_query_strings_for_query_cache<'tcx, C>( tcx: TyCtxt<'tcx>, query_name: &'static str, - query_state: &QueryStateImpl<'tcx, K, V, C>, + query_state: &QueryStateImpl<'tcx, C>, string_cache: &mut QueryKeyStringCache, ) where - K: Debug + Clone, - C: QueryCache, + C: QueryCache, + C::Key: Debug + Clone, { tcx.prof.with_profiler(|profiler| { let event_id_builder = profiler.event_id_builder(); diff --git a/src/librustc/ty/query/stats.rs b/src/librustc/ty/query/stats.rs index c4c81cd5a13cf..20894a2a5d1e1 100644 --- a/src/librustc/ty/query/stats.rs +++ b/src/librustc/ty/query/stats.rs @@ -1,5 +1,5 @@ use crate::ty::query::caches::QueryCache; -use crate::ty::query::config::{QueryAccessors, QueryConfig}; +use crate::ty::query::config::QueryAccessors; use crate::ty::query::plumbing::QueryStateImpl; use crate::ty::query::queries; use crate::ty::TyCtxt; @@ -38,20 +38,17 @@ struct QueryStats { local_def_id_keys: Option, } -fn stats<'tcx, K, V, C: QueryCache>( - name: &'static str, - map: &QueryStateImpl<'tcx, K, V, C>, -) -> QueryStats { +fn stats<'tcx, C: QueryCache>(name: &'static str, map: &QueryStateImpl<'tcx, C>) -> QueryStats { let mut stats = QueryStats { name, #[cfg(debug_assertions)] cache_hits: map.cache_hits.load(Ordering::Relaxed), #[cfg(not(debug_assertions))] cache_hits: 0, - key_size: mem::size_of::(), - key_type: type_name::(), - value_size: mem::size_of::(), - value_type: type_name::(), + key_size: mem::size_of::(), + key_type: type_name::(), + value_size: mem::size_of::(), + value_type: type_name::(), entry_count: map.iter_results(|results| results.count()), local_def_id_keys: None, }; @@ -127,8 +124,6 @@ macro_rules! print_stats { $($( queries.push(stats::< - as QueryConfig<'_>>::Key, - as QueryConfig<'_>>::Value, as QueryAccessors<'_>>::Cache, >( stringify!($name), From 5557407fbb322dc1265ba3c05cd5474a20c2e1d3 Mon Sep 17 00:00:00 2001 From: Camille GILLOT Date: Sat, 7 Mar 2020 18:45:44 +0100 Subject: [PATCH 16/29] Remove QueryState type alias. --- src/librustc/ty/query/caches.rs | 6 ++--- src/librustc/ty/query/config.rs | 2 +- src/librustc/ty/query/plumbing.rs | 27 +++++++++++----------- src/librustc/ty/query/profiling_support.rs | 4 ++-- src/librustc/ty/query/stats.rs | 4 ++-- 5 files changed, 22 insertions(+), 21 deletions(-) diff --git a/src/librustc/ty/query/caches.rs b/src/librustc/ty/query/caches.rs index 71523ea39ca3e..a11b3bcba3ed3 100644 --- a/src/librustc/ty/query/caches.rs +++ b/src/librustc/ty/query/caches.rs @@ -1,5 +1,5 @@ use crate::dep_graph::DepNodeIndex; -use crate::ty::query::plumbing::{QueryLookup, QueryStateImpl, QueryStateShard}; +use crate::ty::query::plumbing::{QueryLookup, QueryState, QueryStateShard}; use crate::ty::TyCtxt; use rustc_data_structures::fx::FxHashMap; @@ -23,7 +23,7 @@ pub(crate) trait QueryCache: Default { /// to compute it. fn lookup<'tcx, R, GetCache, OnHit, OnMiss>( &self, - state: &'tcx QueryStateImpl<'tcx, Self>, + state: &'tcx QueryState<'tcx, Self>, get_cache: GetCache, key: Self::Key, // `on_hit` can be called while holding a lock to the query state shard. @@ -78,7 +78,7 @@ impl QueryCache for DefaultCache { #[inline(always)] fn lookup<'tcx, R, GetCache, OnHit, OnMiss>( &self, - state: &'tcx QueryStateImpl<'tcx, Self>, + state: &'tcx QueryState<'tcx, Self>, get_cache: GetCache, key: K, on_hit: OnHit, diff --git a/src/librustc/ty/query/config.rs b/src/librustc/ty/query/config.rs index 5b0653bd599e1..72a0fdf156726 100644 --- a/src/librustc/ty/query/config.rs +++ b/src/librustc/ty/query/config.rs @@ -33,7 +33,7 @@ pub(crate) trait QueryAccessors<'tcx>: QueryConfig<'tcx> { type Cache: QueryCache; // Don't use this method to access query results, instead use the methods on TyCtxt - fn query_state<'a>(tcx: TyCtxt<'tcx>) -> &'a QueryState<'tcx, Self>; + fn query_state<'a>(tcx: TyCtxt<'tcx>) -> &'a QueryState<'tcx, Self::Cache>; fn to_dep_node(tcx: TyCtxt<'tcx>, key: &Self::Key) -> DepNode; diff --git a/src/librustc/ty/query/plumbing.rs b/src/librustc/ty/query/plumbing.rs index c2740733977e8..442b3edcafe03 100644 --- a/src/librustc/ty/query/plumbing.rs +++ b/src/librustc/ty/query/plumbing.rs @@ -4,7 +4,7 @@ use crate::dep_graph::{DepKind, DepNode, DepNodeIndex, SerializedDepNodeIndex}; use crate::ty::query::caches::QueryCache; -use crate::ty::query::config::{QueryAccessors, QueryDescription}; +use crate::ty::query::config::QueryDescription; use crate::ty::query::job::{QueryInfo, QueryJob, QueryJobId, QueryJobInfo, QueryShardJobId}; use crate::ty::query::Query; use crate::ty::tls; @@ -49,16 +49,14 @@ impl<'tcx, K, C: Default> Default for QueryStateShard<'tcx, K, C> { } } -pub(crate) type QueryState<'tcx, Q> = QueryStateImpl<'tcx, >::Cache>; - -pub(crate) struct QueryStateImpl<'tcx, C: QueryCache> { +pub(crate) struct QueryState<'tcx, C: QueryCache> { pub(super) cache: C, pub(super) shards: Sharded>, #[cfg(debug_assertions)] pub(super) cache_hits: AtomicUsize, } -impl<'tcx, C: QueryCache> QueryStateImpl<'tcx, C> { +impl<'tcx, C: QueryCache> QueryState<'tcx, C> { pub(super) fn get_lookup( &'tcx self, key: &K2, @@ -86,7 +84,7 @@ pub(super) enum QueryResult<'tcx> { Poisoned, } -impl<'tcx, C: QueryCache> QueryStateImpl<'tcx, C> { +impl<'tcx, C: QueryCache> QueryState<'tcx, C> { pub fn iter_results( &self, f: impl for<'a> FnOnce( @@ -130,9 +128,9 @@ impl<'tcx, C: QueryCache> QueryStateImpl<'tcx, C> { } } -impl<'tcx, C: QueryCache> Default for QueryStateImpl<'tcx, C> { - fn default() -> QueryStateImpl<'tcx, C> { - QueryStateImpl { +impl<'tcx, C: QueryCache> Default for QueryState<'tcx, C> { + fn default() -> QueryState<'tcx, C> { + QueryState { cache: C::default(), shards: Default::default(), #[cfg(debug_assertions)] @@ -156,7 +154,7 @@ where C::Key: Eq + Hash + Clone + Debug, C::Value: Clone, { - state: &'tcx QueryStateImpl<'tcx, C>, + state: &'tcx QueryState<'tcx, C>, key: C::Key, id: QueryJobId, } @@ -482,7 +480,7 @@ impl<'tcx> TyCtxt<'tcx> { #[inline(always)] fn try_get_cached( self, - state: &'tcx QueryStateImpl<'tcx, C>, + state: &'tcx QueryState<'tcx, C>, key: C::Key, // `on_hit` can be called while holding a lock to the query cache on_hit: OnHit, @@ -979,7 +977,7 @@ macro_rules! define_queries_inner { type Cache = query_storage!([$($modifiers)*][$K, $V]); #[inline(always)] - fn query_state<'a>(tcx: TyCtxt<$tcx>) -> &'a QueryState<$tcx, Self> { + fn query_state<'a>(tcx: TyCtxt<$tcx>) -> &'a QueryState<$tcx, Self::Cache> { &tcx.queries.$name } @@ -1131,7 +1129,10 @@ macro_rules! define_queries_struct { providers: IndexVec>, fallback_extern_providers: Box>, - $($(#[$attr])* $name: QueryState<$tcx, queries::$name<$tcx>>,)* + $($(#[$attr])* $name: QueryState< + $tcx, + as QueryAccessors<'tcx>>::Cache, + >,)* } impl<$tcx> Queries<$tcx> { diff --git a/src/librustc/ty/query/profiling_support.rs b/src/librustc/ty/query/profiling_support.rs index 256bd86a3de38..58ace917786cf 100644 --- a/src/librustc/ty/query/profiling_support.rs +++ b/src/librustc/ty/query/profiling_support.rs @@ -1,7 +1,7 @@ use crate::hir::map::definitions::DefPathData; use crate::ty::context::TyCtxt; use crate::ty::query::caches::QueryCache; -use crate::ty::query::plumbing::QueryStateImpl; +use crate::ty::query::plumbing::QueryState; use measureme::{StringComponent, StringId}; use rustc_data_structures::fx::FxHashMap; use rustc_data_structures::profiling::SelfProfiler; @@ -160,7 +160,7 @@ where pub(super) fn alloc_self_profile_query_strings_for_query_cache<'tcx, C>( tcx: TyCtxt<'tcx>, query_name: &'static str, - query_state: &QueryStateImpl<'tcx, C>, + query_state: &QueryState<'tcx, C>, string_cache: &mut QueryKeyStringCache, ) where C: QueryCache, diff --git a/src/librustc/ty/query/stats.rs b/src/librustc/ty/query/stats.rs index 20894a2a5d1e1..527bb46c90888 100644 --- a/src/librustc/ty/query/stats.rs +++ b/src/librustc/ty/query/stats.rs @@ -1,6 +1,6 @@ use crate::ty::query::caches::QueryCache; use crate::ty::query::config::QueryAccessors; -use crate::ty::query::plumbing::QueryStateImpl; +use crate::ty::query::plumbing::QueryState; use crate::ty::query::queries; use crate::ty::TyCtxt; use rustc_hir::def_id::{DefId, LOCAL_CRATE}; @@ -38,7 +38,7 @@ struct QueryStats { local_def_id_keys: Option, } -fn stats<'tcx, C: QueryCache>(name: &'static str, map: &QueryStateImpl<'tcx, C>) -> QueryStats { +fn stats<'tcx, C: QueryCache>(name: &'static str, map: &QueryState<'tcx, C>) -> QueryStats { let mut stats = QueryStats { name, #[cfg(debug_assertions)] From 8aa13289b5fbfcbdf5afb19d15a41a15d8f8aa22 Mon Sep 17 00:00:00 2001 From: Camille GILLOT Date: Sun, 15 Mar 2020 09:44:23 +0100 Subject: [PATCH 17/29] Make stuff private. --- src/librustc/ty/query/mod.rs | 2 +- src/librustc/ty/query/plumbing.rs | 41 ++++++++++++++----------------- 2 files changed, 19 insertions(+), 24 deletions(-) diff --git a/src/librustc/ty/query/mod.rs b/src/librustc/ty/query/mod.rs index 54e80b4dc0b15..1279ae2059baa 100644 --- a/src/librustc/ty/query/mod.rs +++ b/src/librustc/ty/query/mod.rs @@ -62,7 +62,7 @@ use std::sync::Arc; #[macro_use] mod plumbing; -pub use self::plumbing::CycleError; +pub(crate) use self::plumbing::CycleError; use self::plumbing::*; mod stats; diff --git a/src/librustc/ty/query/plumbing.rs b/src/librustc/ty/query/plumbing.rs index 442b3edcafe03..0bfcae5fa2e66 100644 --- a/src/librustc/ty/query/plumbing.rs +++ b/src/librustc/ty/query/plumbing.rs @@ -30,11 +30,11 @@ use std::ptr; use std::sync::atomic::{AtomicUsize, Ordering}; pub(crate) struct QueryStateShard<'tcx, K, C> { - pub(super) cache: C, - pub(super) active: FxHashMap>, + cache: C, + active: FxHashMap>, /// Used to generate unique ids for active jobs. - pub(super) jobs: u32, + jobs: u32, } impl<'tcx, K, C> QueryStateShard<'tcx, K, C> { @@ -50,8 +50,8 @@ impl<'tcx, K, C: Default> Default for QueryStateShard<'tcx, K, C> { } pub(crate) struct QueryState<'tcx, C: QueryCache> { - pub(super) cache: C, - pub(super) shards: Sharded>, + cache: C, + shards: Sharded>, #[cfg(debug_assertions)] pub(super) cache_hits: AtomicUsize, } @@ -75,7 +75,7 @@ impl<'tcx, C: QueryCache> QueryState<'tcx, C> { } /// Indicates the state of a query for a given key in a query map. -pub(super) enum QueryResult<'tcx> { +enum QueryResult<'tcx> { /// An already executing query. The query job can be used to await for its completion. Started(QueryJob<'tcx>), @@ -85,7 +85,7 @@ pub(super) enum QueryResult<'tcx> { } impl<'tcx, C: QueryCache> QueryState<'tcx, C> { - pub fn iter_results( + pub(super) fn iter_results( &self, f: impl for<'a> FnOnce( Box + 'a>, @@ -93,7 +93,7 @@ impl<'tcx, C: QueryCache> QueryState<'tcx, C> { ) -> R { self.cache.iter(&self.shards, |shard| &mut shard.cache, f) } - pub fn all_inactive(&self) -> bool { + pub(super) fn all_inactive(&self) -> bool { let shards = self.shards.lock_shards(); shards.iter().all(|shard| shard.active.is_empty()) } @@ -142,13 +142,13 @@ impl<'tcx, C: QueryCache> Default for QueryState<'tcx, C> { /// Values used when checking a query cache which can be reused on a cache-miss to execute the query. pub(crate) struct QueryLookup<'tcx, K, C> { pub(super) key_hash: u64, - pub(super) shard: usize, + shard: usize, pub(super) lock: LockGuard<'tcx, QueryStateShard<'tcx, K, C>>, } /// A type representing the responsibility to execute the job in the `job` field. /// This will poison the relevant query if dropped. -pub(super) struct JobOwner<'tcx, C> +struct JobOwner<'tcx, C> where C: QueryCache, C::Key: Eq + Hash + Clone + Debug, @@ -174,7 +174,7 @@ where /// This function is inlined because that results in a noticeable speed-up /// for some compile-time benchmarks. #[inline(always)] - pub(super) fn try_start( + fn try_start( tcx: TyCtxt<'tcx>, span: Span, key: &C::Key, @@ -262,12 +262,7 @@ where /// Completes the query by updating the query cache with the `result`, /// signals the waiter and forgets the JobOwner, so it won't poison the query #[inline(always)] - pub(super) fn complete( - self, - tcx: TyCtxt<'tcx>, - result: &C::Value, - dep_node_index: DepNodeIndex, - ) { + fn complete(self, tcx: TyCtxt<'tcx>, result: &C::Value, dep_node_index: DepNodeIndex) { // We can move out of `self` here because we `mem::forget` it below let key = unsafe { ptr::read(&self.key) }; let state = self.state; @@ -327,14 +322,14 @@ where } #[derive(Clone)] -pub struct CycleError<'tcx> { +pub(crate) struct CycleError<'tcx> { /// The query and related span that uses the cycle. pub(super) usage: Option<(Span, Query<'tcx>)>, pub(super) cycle: Vec>, } /// The result of `try_start`. -pub(super) enum TryGetJob<'tcx, C: QueryCache> +enum TryGetJob<'tcx, C: QueryCache> where C::Key: Eq + Hash + Clone + Debug, C::Value: Clone, @@ -357,7 +352,7 @@ impl<'tcx> TyCtxt<'tcx> { /// new query job while it executes. It returns the diagnostics /// captured during execution and the actual result. #[inline(always)] - pub(super) fn start_query( + fn start_query( self, token: QueryJobId, diagnostics: Option<&Lock>>, @@ -529,7 +524,7 @@ impl<'tcx> TyCtxt<'tcx> { } #[inline(always)] - pub(super) fn try_execute_query + 'tcx>( + fn try_execute_query + 'tcx>( self, span: Span, key: Q::Key, @@ -1136,7 +1131,7 @@ macro_rules! define_queries_struct { } impl<$tcx> Queries<$tcx> { - pub fn new( + pub(crate) fn new( providers: IndexVec>, fallback_extern_providers: Providers<$tcx>, on_disk_cache: OnDiskCache<'tcx>, @@ -1149,7 +1144,7 @@ macro_rules! define_queries_struct { } } - pub fn try_collect_active_jobs( + pub(crate) fn try_collect_active_jobs( &self ) -> Option>> { let mut jobs = FxHashMap::default(); From f8178c7a2300e1963807ff1dc95051c29aee640e Mon Sep 17 00:00:00 2001 From: Eduard-Mihai Burtescu Date: Wed, 18 Mar 2020 22:31:31 +0200 Subject: [PATCH 18/29] rustc: use LocalDefId instead of DefId in TypeckTables. --- src/librustc/ty/context.rs | 127 +++++++----------- .../infer/error_reporting/mod.rs | 10 +- src/librustc_infer/infer/mod.rs | 4 +- .../traits/error_reporting/suggestions.rs | 6 +- src/librustc_typeck/check/compare_method.rs | 6 +- src/librustc_typeck/check/method/suggest.rs | 6 +- src/librustc_typeck/check/mod.rs | 24 ++-- src/librustc_typeck/check/wfcheck.rs | 4 +- src/librustc_typeck/check/writeback.rs | 32 ++--- 9 files changed, 94 insertions(+), 125 deletions(-) diff --git a/src/librustc/ty/context.rs b/src/librustc/ty/context.rs index 742d57fb58a51..a90a9b933700e 100644 --- a/src/librustc/ty/context.rs +++ b/src/librustc/ty/context.rs @@ -188,37 +188,37 @@ pub struct CommonConsts<'tcx> { } pub struct LocalTableInContext<'a, V> { - local_id_root: Option, + hir_owner: Option, data: &'a ItemLocalMap, } /// Validate that the given HirId (respectively its `local_id` part) can be /// safely used as a key in the tables of a TypeckTable. For that to be /// the case, the HirId must have the same `owner` as all the other IDs in -/// this table (signified by `local_id_root`). Otherwise the HirId +/// this table (signified by `hir_owner`). Otherwise the HirId /// would be in a different frame of reference and using its `local_id` /// would result in lookup errors, or worse, in silently wrong data being /// stored/returned. fn validate_hir_id_for_typeck_tables( - local_id_root: Option, + hir_owner: Option, hir_id: hir::HirId, mut_access: bool, ) { - if let Some(local_id_root) = local_id_root { - if hir_id.owner.to_def_id() != local_id_root { + if let Some(hir_owner) = hir_owner { + if hir_id.owner != hir_owner { ty::tls::with(|tcx| { bug!( "node {} with HirId::owner {:?} cannot be placed in \ - TypeckTables with local_id_root {:?}", + TypeckTables with hir_owner {:?}", tcx.hir().node_to_string(hir_id), hir_id.owner, - local_id_root + hir_owner ) }); } } else { // We use "Null Object" TypeckTables in some of the analysis passes. - // These are just expected to be empty and their `local_id_root` is + // These are just expected to be empty and their `hir_owner` is // `None`. Therefore we cannot verify whether a given `HirId` would // be a valid key for the given table. Instead we make sure that // nobody tries to write to such a Null Object table. @@ -230,12 +230,12 @@ fn validate_hir_id_for_typeck_tables( impl<'a, V> LocalTableInContext<'a, V> { pub fn contains_key(&self, id: hir::HirId) -> bool { - validate_hir_id_for_typeck_tables(self.local_id_root, id, false); + validate_hir_id_for_typeck_tables(self.hir_owner, id, false); self.data.contains_key(&id.local_id) } pub fn get(&self, id: hir::HirId) -> Option<&V> { - validate_hir_id_for_typeck_tables(self.local_id_root, id, false); + validate_hir_id_for_typeck_tables(self.hir_owner, id, false); self.data.get(&id.local_id) } @@ -253,28 +253,28 @@ impl<'a, V> ::std::ops::Index for LocalTableInContext<'a, V> { } pub struct LocalTableInContextMut<'a, V> { - local_id_root: Option, + hir_owner: Option, data: &'a mut ItemLocalMap, } impl<'a, V> LocalTableInContextMut<'a, V> { pub fn get_mut(&mut self, id: hir::HirId) -> Option<&mut V> { - validate_hir_id_for_typeck_tables(self.local_id_root, id, true); + validate_hir_id_for_typeck_tables(self.hir_owner, id, true); self.data.get_mut(&id.local_id) } pub fn entry(&mut self, id: hir::HirId) -> Entry<'_, hir::ItemLocalId, V> { - validate_hir_id_for_typeck_tables(self.local_id_root, id, true); + validate_hir_id_for_typeck_tables(self.hir_owner, id, true); self.data.entry(id.local_id) } pub fn insert(&mut self, id: hir::HirId, val: V) -> Option { - validate_hir_id_for_typeck_tables(self.local_id_root, id, true); + validate_hir_id_for_typeck_tables(self.hir_owner, id, true); self.data.insert(id.local_id, val) } pub fn remove(&mut self, id: hir::HirId) -> Option { - validate_hir_id_for_typeck_tables(self.local_id_root, id, true); + validate_hir_id_for_typeck_tables(self.hir_owner, id, true); self.data.remove(&id.local_id) } } @@ -322,8 +322,8 @@ pub struct GeneratorInteriorTypeCause<'tcx> { #[derive(RustcEncodable, RustcDecodable, Debug)] pub struct TypeckTables<'tcx> { - /// The HirId::owner all ItemLocalIds in this table are relative to. - pub local_id_root: Option, + /// The `HirId::owner` all `ItemLocalId`s in this table are relative to. + pub hir_owner: Option, /// Resolved definitions for `::X` associated paths and /// method calls, including those of overloaded operators. @@ -431,9 +431,9 @@ pub struct TypeckTables<'tcx> { } impl<'tcx> TypeckTables<'tcx> { - pub fn empty(local_id_root: Option) -> TypeckTables<'tcx> { + pub fn empty(hir_owner: Option) -> TypeckTables<'tcx> { TypeckTables { - local_id_root, + hir_owner, type_dependent_defs: Default::default(), field_indices: Default::default(), user_provided_types: Default::default(), @@ -469,11 +469,11 @@ impl<'tcx> TypeckTables<'tcx> { pub fn type_dependent_defs( &self, ) -> LocalTableInContext<'_, Result<(DefKind, DefId), ErrorReported>> { - LocalTableInContext { local_id_root: self.local_id_root, data: &self.type_dependent_defs } + LocalTableInContext { hir_owner: self.hir_owner, data: &self.type_dependent_defs } } pub fn type_dependent_def(&self, id: HirId) -> Option<(DefKind, DefId)> { - validate_hir_id_for_typeck_tables(self.local_id_root, id, false); + validate_hir_id_for_typeck_tables(self.hir_owner, id, false); self.type_dependent_defs.get(&id.local_id).cloned().and_then(|r| r.ok()) } @@ -484,39 +484,33 @@ impl<'tcx> TypeckTables<'tcx> { pub fn type_dependent_defs_mut( &mut self, ) -> LocalTableInContextMut<'_, Result<(DefKind, DefId), ErrorReported>> { - LocalTableInContextMut { - local_id_root: self.local_id_root, - data: &mut self.type_dependent_defs, - } + LocalTableInContextMut { hir_owner: self.hir_owner, data: &mut self.type_dependent_defs } } pub fn field_indices(&self) -> LocalTableInContext<'_, usize> { - LocalTableInContext { local_id_root: self.local_id_root, data: &self.field_indices } + LocalTableInContext { hir_owner: self.hir_owner, data: &self.field_indices } } pub fn field_indices_mut(&mut self) -> LocalTableInContextMut<'_, usize> { - LocalTableInContextMut { local_id_root: self.local_id_root, data: &mut self.field_indices } + LocalTableInContextMut { hir_owner: self.hir_owner, data: &mut self.field_indices } } pub fn user_provided_types(&self) -> LocalTableInContext<'_, CanonicalUserType<'tcx>> { - LocalTableInContext { local_id_root: self.local_id_root, data: &self.user_provided_types } + LocalTableInContext { hir_owner: self.hir_owner, data: &self.user_provided_types } } pub fn user_provided_types_mut( &mut self, ) -> LocalTableInContextMut<'_, CanonicalUserType<'tcx>> { - LocalTableInContextMut { - local_id_root: self.local_id_root, - data: &mut self.user_provided_types, - } + LocalTableInContextMut { hir_owner: self.hir_owner, data: &mut self.user_provided_types } } pub fn node_types(&self) -> LocalTableInContext<'_, Ty<'tcx>> { - LocalTableInContext { local_id_root: self.local_id_root, data: &self.node_types } + LocalTableInContext { hir_owner: self.hir_owner, data: &self.node_types } } pub fn node_types_mut(&mut self) -> LocalTableInContextMut<'_, Ty<'tcx>> { - LocalTableInContextMut { local_id_root: self.local_id_root, data: &mut self.node_types } + LocalTableInContextMut { hir_owner: self.hir_owner, data: &mut self.node_types } } pub fn node_type(&self, id: hir::HirId) -> Ty<'tcx> { @@ -526,21 +520,21 @@ impl<'tcx> TypeckTables<'tcx> { } pub fn node_type_opt(&self, id: hir::HirId) -> Option> { - validate_hir_id_for_typeck_tables(self.local_id_root, id, false); + validate_hir_id_for_typeck_tables(self.hir_owner, id, false); self.node_types.get(&id.local_id).cloned() } pub fn node_substs_mut(&mut self) -> LocalTableInContextMut<'_, SubstsRef<'tcx>> { - LocalTableInContextMut { local_id_root: self.local_id_root, data: &mut self.node_substs } + LocalTableInContextMut { hir_owner: self.hir_owner, data: &mut self.node_substs } } pub fn node_substs(&self, id: hir::HirId) -> SubstsRef<'tcx> { - validate_hir_id_for_typeck_tables(self.local_id_root, id, false); + validate_hir_id_for_typeck_tables(self.hir_owner, id, false); self.node_substs.get(&id.local_id).cloned().unwrap_or_else(|| InternalSubsts::empty()) } pub fn node_substs_opt(&self, id: hir::HirId) -> Option> { - validate_hir_id_for_typeck_tables(self.local_id_root, id, false); + validate_hir_id_for_typeck_tables(self.hir_owner, id, false); self.node_substs.get(&id.local_id).cloned() } @@ -573,17 +567,17 @@ impl<'tcx> TypeckTables<'tcx> { } pub fn adjustments(&self) -> LocalTableInContext<'_, Vec>> { - LocalTableInContext { local_id_root: self.local_id_root, data: &self.adjustments } + LocalTableInContext { hir_owner: self.hir_owner, data: &self.adjustments } } pub fn adjustments_mut( &mut self, ) -> LocalTableInContextMut<'_, Vec>> { - LocalTableInContextMut { local_id_root: self.local_id_root, data: &mut self.adjustments } + LocalTableInContextMut { hir_owner: self.hir_owner, data: &mut self.adjustments } } pub fn expr_adjustments(&self, expr: &hir::Expr<'_>) -> &[ty::adjustment::Adjustment<'tcx>] { - validate_hir_id_for_typeck_tables(self.local_id_root, expr.hir_id, false); + validate_hir_id_for_typeck_tables(self.hir_owner, expr.hir_id, false); self.adjustments.get(&expr.hir_id.local_id).map_or(&[], |a| &a[..]) } @@ -618,25 +612,19 @@ impl<'tcx> TypeckTables<'tcx> { } pub fn pat_binding_modes(&self) -> LocalTableInContext<'_, BindingMode> { - LocalTableInContext { local_id_root: self.local_id_root, data: &self.pat_binding_modes } + LocalTableInContext { hir_owner: self.hir_owner, data: &self.pat_binding_modes } } pub fn pat_binding_modes_mut(&mut self) -> LocalTableInContextMut<'_, BindingMode> { - LocalTableInContextMut { - local_id_root: self.local_id_root, - data: &mut self.pat_binding_modes, - } + LocalTableInContextMut { hir_owner: self.hir_owner, data: &mut self.pat_binding_modes } } pub fn pat_adjustments(&self) -> LocalTableInContext<'_, Vec>> { - LocalTableInContext { local_id_root: self.local_id_root, data: &self.pat_adjustments } + LocalTableInContext { hir_owner: self.hir_owner, data: &self.pat_adjustments } } pub fn pat_adjustments_mut(&mut self) -> LocalTableInContextMut<'_, Vec>> { - LocalTableInContextMut { - local_id_root: self.local_id_root, - data: &mut self.pat_adjustments, - } + LocalTableInContextMut { hir_owner: self.hir_owner, data: &mut self.pat_adjustments } } pub fn upvar_capture(&self, upvar_id: ty::UpvarId) -> ty::UpvarCapture<'tcx> { @@ -644,40 +632,31 @@ impl<'tcx> TypeckTables<'tcx> { } pub fn closure_kind_origins(&self) -> LocalTableInContext<'_, (Span, ast::Name)> { - LocalTableInContext { local_id_root: self.local_id_root, data: &self.closure_kind_origins } + LocalTableInContext { hir_owner: self.hir_owner, data: &self.closure_kind_origins } } pub fn closure_kind_origins_mut(&mut self) -> LocalTableInContextMut<'_, (Span, ast::Name)> { - LocalTableInContextMut { - local_id_root: self.local_id_root, - data: &mut self.closure_kind_origins, - } + LocalTableInContextMut { hir_owner: self.hir_owner, data: &mut self.closure_kind_origins } } pub fn liberated_fn_sigs(&self) -> LocalTableInContext<'_, ty::FnSig<'tcx>> { - LocalTableInContext { local_id_root: self.local_id_root, data: &self.liberated_fn_sigs } + LocalTableInContext { hir_owner: self.hir_owner, data: &self.liberated_fn_sigs } } pub fn liberated_fn_sigs_mut(&mut self) -> LocalTableInContextMut<'_, ty::FnSig<'tcx>> { - LocalTableInContextMut { - local_id_root: self.local_id_root, - data: &mut self.liberated_fn_sigs, - } + LocalTableInContextMut { hir_owner: self.hir_owner, data: &mut self.liberated_fn_sigs } } pub fn fru_field_types(&self) -> LocalTableInContext<'_, Vec>> { - LocalTableInContext { local_id_root: self.local_id_root, data: &self.fru_field_types } + LocalTableInContext { hir_owner: self.hir_owner, data: &self.fru_field_types } } pub fn fru_field_types_mut(&mut self) -> LocalTableInContextMut<'_, Vec>> { - LocalTableInContextMut { - local_id_root: self.local_id_root, - data: &mut self.fru_field_types, - } + LocalTableInContextMut { hir_owner: self.hir_owner, data: &mut self.fru_field_types } } pub fn is_coercion_cast(&self, hir_id: hir::HirId) -> bool { - validate_hir_id_for_typeck_tables(self.local_id_root, hir_id, true); + validate_hir_id_for_typeck_tables(self.hir_owner, hir_id, true); self.coercion_casts.contains(&hir_id.local_id) } @@ -693,7 +672,7 @@ impl<'tcx> TypeckTables<'tcx> { impl<'a, 'tcx> HashStable> for TypeckTables<'tcx> { fn hash_stable(&self, hcx: &mut StableHashingContext<'a>, hasher: &mut StableHasher) { let ty::TypeckTables { - local_id_root, + hir_owner, ref type_dependent_defs, ref field_indices, ref user_provided_types, @@ -730,18 +709,12 @@ impl<'a, 'tcx> HashStable> for TypeckTables<'tcx> { hash_stable_hashmap(hcx, hasher, upvar_capture_map, |up_var_id, hcx| { let ty::UpvarId { var_path, closure_expr_id } = *up_var_id; - let local_id_root = local_id_root.expect("trying to hash invalid TypeckTables"); + assert_eq!(Some(var_path.hir_id.owner), hir_owner); - let var_owner_def_id = DefId { - krate: local_id_root.krate, - index: var_path.hir_id.owner.local_def_index, - }; - let closure_def_id = - DefId { krate: local_id_root.krate, index: closure_expr_id.local_def_index }; ( - hcx.def_path_hash(var_owner_def_id), + hcx.local_def_path_hash(var_path.hir_id.owner), var_path.hir_id.local_id, - hcx.def_path_hash(closure_def_id), + hcx.local_def_path_hash(closure_expr_id), ) }); diff --git a/src/librustc_infer/infer/error_reporting/mod.rs b/src/librustc_infer/infer/error_reporting/mod.rs index ebbfcb28db2f5..78f97c40cbd12 100644 --- a/src/librustc_infer/infer/error_reporting/mod.rs +++ b/src/librustc_infer/infer/error_reporting/mod.rs @@ -1784,11 +1784,11 @@ impl<'a, 'tcx> InferCtxt<'a, 'tcx> { // suggest adding an explicit lifetime bound to it. let type_param_span = match (self.in_progress_tables, bound_kind) { (Some(ref table), GenericKind::Param(ref param)) => { - let table = table.borrow(); - table.local_id_root.and_then(|did| { - let generics = self.tcx.generics_of(did); - // Account for the case where `did` corresponds to `Self`, which doesn't have - // the expected type argument. + let table_owner = table.borrow().hir_owner; + table_owner.and_then(|table_owner| { + let generics = self.tcx.generics_of(table_owner.to_def_id()); + // Account for the case where `param` corresponds to `Self`, + // which doesn't have the expected type argument. if !(generics.has_self && param.index == 0) { let type_param = generics.type_param(param, self.tcx); let hir = &self.tcx.hir(); diff --git a/src/librustc_infer/infer/mod.rs b/src/librustc_infer/infer/mod.rs index 391fce946bf43..70967940e7bf8 100644 --- a/src/librustc_infer/infer/mod.rs +++ b/src/librustc_infer/infer/mod.rs @@ -29,7 +29,7 @@ use rustc_data_structures::sync::Lrc; use rustc_data_structures::unify as ut; use rustc_errors::DiagnosticBuilder; use rustc_hir as hir; -use rustc_hir::def_id::DefId; +use rustc_hir::def_id::{DefId, LocalDefId}; use rustc_session::config::BorrowckMode; use rustc_span::symbol::Symbol; use rustc_span::Span; @@ -559,7 +559,7 @@ impl TyCtxtInferExt<'tcx> for TyCtxt<'tcx> { impl<'tcx> InferCtxtBuilder<'tcx> { /// Used only by `rustc_typeck` during body type-checking/inference, /// will initialize `in_progress_tables` with fresh `TypeckTables`. - pub fn with_fresh_in_progress_tables(mut self, table_owner: DefId) -> Self { + pub fn with_fresh_in_progress_tables(mut self, table_owner: LocalDefId) -> Self { self.fresh_tables = Some(RefCell::new(ty::TypeckTables::empty(Some(table_owner)))); self } diff --git a/src/librustc_trait_selection/traits/error_reporting/suggestions.rs b/src/librustc_trait_selection/traits/error_reporting/suggestions.rs index 0523a2019861c..fec9ecbd64a2e 100644 --- a/src/librustc_trait_selection/traits/error_reporting/suggestions.rs +++ b/src/librustc_trait_selection/traits/error_reporting/suggestions.rs @@ -1105,15 +1105,15 @@ impl<'a, 'tcx> InferCtxtExt<'tcx> for InferCtxt<'a, 'tcx> { let generator_did_root = self.tcx.closure_base_def_id(generator_did); debug!( "maybe_note_obligation_cause_for_async_await: generator_did={:?} \ - generator_did_root={:?} in_progress_tables.local_id_root={:?} span={:?}", + generator_did_root={:?} in_progress_tables.hir_owner={:?} span={:?}", generator_did, generator_did_root, - in_progress_tables.as_ref().map(|t| t.local_id_root), + in_progress_tables.as_ref().map(|t| t.hir_owner), span ); let query_tables; let tables: &TypeckTables<'tcx> = match &in_progress_tables { - Some(t) if t.local_id_root == Some(generator_did_root) => t, + Some(t) if t.hir_owner.map(|owner| owner.to_def_id()) == Some(generator_did_root) => t, _ => { query_tables = self.tcx.typeck_tables_of(generator_did); &query_tables diff --git a/src/librustc_typeck/check/compare_method.rs b/src/librustc_typeck/check/compare_method.rs index a2832d92d4aeb..6178158e4e504 100644 --- a/src/librustc_typeck/check/compare_method.rs +++ b/src/librustc_typeck/check/compare_method.rs @@ -213,7 +213,7 @@ fn compare_predicate_entailment<'tcx>( ); tcx.infer_ctxt().enter(|infcx| { - let inh = Inherited::new(infcx, impl_m.def_id); + let inh = Inherited::new(infcx, impl_m.def_id.expect_local()); let infcx = &inh.infcx; debug!("compare_impl_method: caller_bounds={:?}", param_env.caller_bounds); @@ -950,7 +950,7 @@ crate fn compare_const_impl<'tcx>( tcx.infer_ctxt().enter(|infcx| { let param_env = tcx.param_env(impl_c.def_id); - let inh = Inherited::new(infcx, impl_c.def_id); + let inh = Inherited::new(infcx, impl_c.def_id.expect_local()); let infcx = &inh.infcx; // The below is for the most part highly similar to the procedure @@ -1130,7 +1130,7 @@ fn compare_type_predicate_entailment( normalize_cause.clone(), ); tcx.infer_ctxt().enter(|infcx| { - let inh = Inherited::new(infcx, impl_ty.def_id); + let inh = Inherited::new(infcx, impl_ty.def_id.expect_local()); let infcx = &inh.infcx; debug!("compare_type_predicate_entailment: caller_bounds={:?}", param_env.caller_bounds); diff --git a/src/librustc_typeck/check/method/suggest.rs b/src/librustc_typeck/check/method/suggest.rs index 2f0eb5e06709a..061433bcf6515 100644 --- a/src/librustc_typeck/check/method/suggest.rs +++ b/src/librustc_typeck/check/method/suggest.rs @@ -1018,9 +1018,9 @@ impl<'a, 'tcx> FnCtxt<'a, 'tcx> { // Obtain the span for `param` and use it for a structured suggestion. let mut suggested = false; if let (Some(ref param), Some(ref table)) = (param_type, self.in_progress_tables) { - let table = table.borrow(); - if let Some(did) = table.local_id_root { - let generics = self.tcx.generics_of(did); + let table_owner = table.borrow().hir_owner; + if let Some(table_owner) = table_owner { + let generics = self.tcx.generics_of(table_owner.to_def_id()); let type_param = generics.type_param(param, self.tcx); let hir = &self.tcx.hir(); if let Some(id) = hir.as_local_hir_id(type_param.def_id) { diff --git a/src/librustc_typeck/check/mod.rs b/src/librustc_typeck/check/mod.rs index 368f64e4d41aa..ab99918e00582 100644 --- a/src/librustc_typeck/check/mod.rs +++ b/src/librustc_typeck/check/mod.rs @@ -112,7 +112,7 @@ use rustc_data_structures::fx::{FxHashMap, FxHashSet}; use rustc_errors::{pluralize, struct_span_err, Applicability, DiagnosticBuilder, DiagnosticId}; use rustc_hir as hir; use rustc_hir::def::{CtorOf, DefKind, Res}; -use rustc_hir::def_id::{CrateNum, DefId, DefIdMap, DefIdSet, LOCAL_CRATE}; +use rustc_hir::def_id::{CrateNum, DefId, DefIdMap, DefIdSet, LocalDefId, LOCAL_CRATE}; use rustc_hir::intravisit::{self, NestedVisitorMap, Visitor}; use rustc_hir::itemlikevisit::ItemLikeVisitor; use rustc_hir::{ExprKind, GenericArg, HirIdMap, Item, ItemKind, Node, PatKind, QPath}; @@ -633,19 +633,15 @@ impl<'a, 'tcx> Deref for FnCtxt<'a, 'tcx> { /// `F: for<'b, 'tcx> where 'tcx FnOnce(Inherited<'b, 'tcx>)`. pub struct InheritedBuilder<'tcx> { infcx: infer::InferCtxtBuilder<'tcx>, - def_id: DefId, + def_id: LocalDefId, } impl Inherited<'_, 'tcx> { - pub fn build(tcx: TyCtxt<'tcx>, def_id: DefId) -> InheritedBuilder<'tcx> { - let hir_id_root = if let Some(def_id) = def_id.as_local() { - tcx.hir().local_def_id_to_hir_id(def_id).owner.to_def_id() - } else { - def_id - }; + pub fn build(tcx: TyCtxt<'tcx>, def_id: LocalDefId) -> InheritedBuilder<'tcx> { + let hir_owner = tcx.hir().local_def_id_to_hir_id(def_id).owner; InheritedBuilder { - infcx: tcx.infer_ctxt().with_fresh_in_progress_tables(hir_id_root), + infcx: tcx.infer_ctxt().with_fresh_in_progress_tables(hir_owner), def_id, } } @@ -662,10 +658,10 @@ impl<'tcx> InheritedBuilder<'tcx> { } impl Inherited<'a, 'tcx> { - fn new(infcx: InferCtxt<'a, 'tcx>, def_id: DefId) -> Self { + fn new(infcx: InferCtxt<'a, 'tcx>, def_id: LocalDefId) -> Self { let tcx = infcx.tcx; - let item_id = tcx.hir().as_local_hir_id(def_id); - let body_id = item_id.and_then(|id| tcx.hir().maybe_body_owned_by(id)); + let item_id = tcx.hir().local_def_id_to_hir_id(def_id); + let body_id = tcx.hir().maybe_body_owned_by(item_id); let implicit_region_bound = body_id.map(|body_id| { let body = tcx.hir().body(body_id); tcx.mk_region(ty::ReScope(region::Scope { @@ -1002,7 +998,7 @@ fn typeck_tables_of_with_fallback<'tcx>( }); let body = tcx.hir().body(body_id); - let tables = Inherited::build(tcx, def_id).enter(|inh| { + let tables = Inherited::build(tcx, def_id.expect_local()).enter(|inh| { let param_env = tcx.param_env(def_id); let fcx = if let (Some(header), Some(decl)) = (fn_header, fn_decl) { let fn_sig = if crate::collect::get_infer_ret_ty(&decl.output).is_some() { @@ -1127,7 +1123,7 @@ fn typeck_tables_of_with_fallback<'tcx>( // Consistency check our TypeckTables instance can hold all ItemLocalIds // it will need to hold. - assert_eq!(tables.local_id_root, Some(id.owner.to_def_id())); + assert_eq!(tables.hir_owner, Some(id.owner)); tables } diff --git a/src/librustc_typeck/check/wfcheck.rs b/src/librustc_typeck/check/wfcheck.rs index 3255d7b435c8f..72c58af7912fb 100644 --- a/src/librustc_typeck/check/wfcheck.rs +++ b/src/librustc_typeck/check/wfcheck.rs @@ -316,12 +316,12 @@ fn for_item<'tcx>(tcx: TyCtxt<'tcx>, item: &hir::Item<'_>) -> CheckWfFcxBuilder< } fn for_id(tcx: TyCtxt<'_>, id: hir::HirId, span: Span) -> CheckWfFcxBuilder<'_> { - let def_id = tcx.hir().local_def_id(id); + let def_id = tcx.hir().local_def_id(id).expect_local(); CheckWfFcxBuilder { inherited: Inherited::build(tcx, def_id), id, span, - param_env: tcx.param_env(def_id), + param_env: tcx.param_env(def_id.to_def_id()), } } diff --git a/src/librustc_typeck/check/writeback.rs b/src/librustc_typeck/check/writeback.rs index fd92284effb32..65f81ef033dd7 100644 --- a/src/librustc_typeck/check/writeback.rs +++ b/src/librustc_typeck/check/writeback.rs @@ -111,7 +111,7 @@ impl<'cx, 'tcx> WritebackCx<'cx, 'tcx> { WritebackCx { fcx, - tables: ty::TypeckTables::empty(Some(owner.to_def_id())), + tables: ty::TypeckTables::empty(Some(owner)), body, rustc_dump_user_substs, } @@ -338,11 +338,11 @@ impl<'cx, 'tcx> WritebackCx<'cx, 'tcx> { fn visit_closures(&mut self) { let fcx_tables = self.fcx.tables.borrow(); - debug_assert_eq!(fcx_tables.local_id_root, self.tables.local_id_root); - let common_local_id_root = fcx_tables.local_id_root.unwrap(); + assert_eq!(fcx_tables.hir_owner, self.tables.hir_owner); + let common_hir_owner = fcx_tables.hir_owner.unwrap(); for (&id, &origin) in fcx_tables.closure_kind_origins().iter() { - let hir_id = hir::HirId { owner: common_local_id_root.expect_local(), local_id: id }; + let hir_id = hir::HirId { owner: common_hir_owner, local_id: id }; self.tables.closure_kind_origins_mut().insert(hir_id, origin); } } @@ -350,7 +350,7 @@ impl<'cx, 'tcx> WritebackCx<'cx, 'tcx> { fn visit_coercion_casts(&mut self) { let fcx_tables = self.fcx.tables.borrow(); let fcx_coercion_casts = fcx_tables.coercion_casts(); - debug_assert_eq!(fcx_tables.local_id_root, self.tables.local_id_root); + assert_eq!(fcx_tables.hir_owner, self.tables.hir_owner); for local_id in fcx_coercion_casts { self.tables.set_coercion_cast(*local_id); @@ -359,12 +359,12 @@ impl<'cx, 'tcx> WritebackCx<'cx, 'tcx> { fn visit_user_provided_tys(&mut self) { let fcx_tables = self.fcx.tables.borrow(); - debug_assert_eq!(fcx_tables.local_id_root, self.tables.local_id_root); - let common_local_id_root = fcx_tables.local_id_root.unwrap(); + assert_eq!(fcx_tables.hir_owner, self.tables.hir_owner); + let common_hir_owner = fcx_tables.hir_owner.unwrap(); let mut errors_buffer = Vec::new(); for (&local_id, c_ty) in fcx_tables.user_provided_types().iter() { - let hir_id = hir::HirId { owner: common_local_id_root.expect_local(), local_id }; + let hir_id = hir::HirId { owner: common_hir_owner, local_id }; if cfg!(debug_assertions) && c_ty.has_local_value() { span_bug!(hir_id.to_span(self.fcx.tcx), "writeback: `{:?}` is a local value", c_ty); @@ -397,7 +397,7 @@ impl<'cx, 'tcx> WritebackCx<'cx, 'tcx> { fn visit_user_provided_sigs(&mut self) { let fcx_tables = self.fcx.tables.borrow(); - debug_assert_eq!(fcx_tables.local_id_root, self.tables.local_id_root); + assert_eq!(fcx_tables.hir_owner, self.tables.hir_owner); for (&def_id, c_sig) in fcx_tables.user_provided_sigs.iter() { if cfg!(debug_assertions) && c_sig.has_local_value() { @@ -414,7 +414,7 @@ impl<'cx, 'tcx> WritebackCx<'cx, 'tcx> { fn visit_generator_interior_types(&mut self) { let fcx_tables = self.fcx.tables.borrow(); - debug_assert_eq!(fcx_tables.local_id_root, self.tables.local_id_root); + assert_eq!(fcx_tables.hir_owner, self.tables.hir_owner); self.tables.generator_interior_types = fcx_tables.generator_interior_types.clone(); } @@ -553,11 +553,11 @@ impl<'cx, 'tcx> WritebackCx<'cx, 'tcx> { fn visit_liberated_fn_sigs(&mut self) { let fcx_tables = self.fcx.tables.borrow(); - debug_assert_eq!(fcx_tables.local_id_root, self.tables.local_id_root); - let common_local_id_root = fcx_tables.local_id_root.unwrap(); + assert_eq!(fcx_tables.hir_owner, self.tables.hir_owner); + let common_hir_owner = fcx_tables.hir_owner.unwrap(); for (&local_id, fn_sig) in fcx_tables.liberated_fn_sigs().iter() { - let hir_id = hir::HirId { owner: common_local_id_root.expect_local(), local_id }; + let hir_id = hir::HirId { owner: common_hir_owner, local_id }; let fn_sig = self.resolve(fn_sig, &hir_id); self.tables.liberated_fn_sigs_mut().insert(hir_id, fn_sig.clone()); } @@ -565,11 +565,11 @@ impl<'cx, 'tcx> WritebackCx<'cx, 'tcx> { fn visit_fru_field_types(&mut self) { let fcx_tables = self.fcx.tables.borrow(); - debug_assert_eq!(fcx_tables.local_id_root, self.tables.local_id_root); - let common_local_id_root = fcx_tables.local_id_root.unwrap(); + assert_eq!(fcx_tables.hir_owner, self.tables.hir_owner); + let common_hir_owner = fcx_tables.hir_owner.unwrap(); for (&local_id, ftys) in fcx_tables.fru_field_types().iter() { - let hir_id = hir::HirId { owner: common_local_id_root.expect_local(), local_id }; + let hir_id = hir::HirId { owner: common_hir_owner, local_id }; let ftys = self.resolve(ftys, &hir_id); self.tables.fru_field_types_mut().insert(hir_id, ftys); } From 82920f36c47fb649858a31caf840e29088dcd8ab Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?John=20K=C3=A5re=20Alsaker?= Date: Wed, 26 Feb 2020 23:43:49 +0100 Subject: [PATCH 19/29] Don't unwind when hitting the macro expansion recursion limit --- src/librustc_expand/base.rs | 2 ++ src/librustc_expand/expand.rs | 16 ++++++++++++---- src/librustc_interface/passes.rs | 13 +++++++++++-- 3 files changed, 25 insertions(+), 6 deletions(-) diff --git a/src/librustc_expand/base.rs b/src/librustc_expand/base.rs index 74c304c96b9a4..0fc477bbd0b4c 100644 --- a/src/librustc_expand/base.rs +++ b/src/librustc_expand/base.rs @@ -922,6 +922,7 @@ pub struct ExpansionData { pub struct ExtCtxt<'a> { pub parse_sess: &'a ParseSess, pub ecfg: expand::ExpansionConfig<'a>, + pub reduced_recursion_limit: Option, pub root_path: PathBuf, pub resolver: &'a mut dyn Resolver, pub current_expansion: ExpansionData, @@ -940,6 +941,7 @@ impl<'a> ExtCtxt<'a> { ExtCtxt { parse_sess, ecfg, + reduced_recursion_limit: None, resolver, extern_mod_loaded, root_path: PathBuf::new(), diff --git a/src/librustc_expand/expand.rs b/src/librustc_expand/expand.rs index b6cc192cc33d6..4f568e5456c72 100644 --- a/src/librustc_expand/expand.rs +++ b/src/librustc_expand/expand.rs @@ -17,7 +17,7 @@ use rustc_ast::util::map_in_place::MapInPlace; use rustc_ast::visit::{self, AssocCtxt, Visitor}; use rustc_ast_pretty::pprust; use rustc_attr::{self as attr, is_builtin_attr, HasAttrs}; -use rustc_errors::{Applicability, FatalError, PResult}; +use rustc_errors::{Applicability, PResult}; use rustc_feature::Features; use rustc_parse::parser::Parser; use rustc_parse::validate_attr; @@ -645,7 +645,6 @@ impl<'a, 'b> MacroExpander<'a, 'b> { )) .emit(); self.cx.trace_macros_diag(); - FatalError.raise(); } /// A macro's expansion does not fit in this fragment kind. @@ -665,8 +664,17 @@ impl<'a, 'b> MacroExpander<'a, 'b> { invoc: Invocation, ext: &SyntaxExtensionKind, ) -> ExpandResult { - if self.cx.current_expansion.depth > self.cx.ecfg.recursion_limit { - self.error_recursion_limit_reached(); + let recursion_limit = + self.cx.reduced_recursion_limit.unwrap_or(self.cx.ecfg.recursion_limit); + if self.cx.current_expansion.depth > recursion_limit { + if self.cx.reduced_recursion_limit.is_none() { + self.error_recursion_limit_reached(); + } + + // Reduce the recursion limit by half each time it triggers. + self.cx.reduced_recursion_limit = Some(recursion_limit / 2); + + return ExpandResult::Ready(invoc.fragment_kind.dummy(invoc.span())); } let (fragment_kind, span) = (invoc.fragment_kind, invoc.span()); diff --git a/src/librustc_interface/passes.rs b/src/librustc_interface/passes.rs index ee323b204b7a0..22089f9de312d 100644 --- a/src/librustc_interface/passes.rs +++ b/src/librustc_interface/passes.rs @@ -311,6 +311,8 @@ fn configure_and_expand_inner<'a>( ecx.parse_sess.missing_fragment_specifiers.borrow().iter().cloned().collect(); missing_fragment_specifiers.sort(); + let recursion_limit_hit = ecx.reduced_recursion_limit.is_some(); + for span in missing_fragment_specifiers { let lint = lint::builtin::MISSING_FRAGMENT_SPECIFIER; let msg = "missing fragment specifier"; @@ -319,8 +321,15 @@ fn configure_and_expand_inner<'a>( if cfg!(windows) { env::set_var("PATH", &old_path); } - krate - }); + + if recursion_limit_hit { + // If we hit a recursion limit, exit early to avoid later passes getting overwhelmed + // with a large AST + Err(ErrorReported) + } else { + Ok(krate) + } + })?; sess.time("maybe_building_test_harness", || { rustc_builtin_macros::test_harness::inject( From d641ad044eb535741cd2160e7cbf96d91c2c54c1 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?John=20K=C3=A5re=20Alsaker?= Date: Sat, 29 Feb 2020 04:23:58 +0100 Subject: [PATCH 20/29] Update test --- src/test/ui/macros/trace_faulty_macros.rs | 2 +- src/test/ui/macros/trace_faulty_macros.stderr | 13 ++++++++++++- 2 files changed, 13 insertions(+), 2 deletions(-) diff --git a/src/test/ui/macros/trace_faulty_macros.rs b/src/test/ui/macros/trace_faulty_macros.rs index 627d58abf4ca2..a55f05414b206 100644 --- a/src/test/ui/macros/trace_faulty_macros.rs +++ b/src/test/ui/macros/trace_faulty_macros.rs @@ -13,7 +13,7 @@ macro_rules! pat_macro { pat_macro!(A{a:a, b:0, c:_, ..}); }; ($a:pat) => { - $a + $a //~ ERROR expected expression }; } diff --git a/src/test/ui/macros/trace_faulty_macros.stderr b/src/test/ui/macros/trace_faulty_macros.stderr index a18e22e07f8bc..109b493b43717 100644 --- a/src/test/ui/macros/trace_faulty_macros.stderr +++ b/src/test/ui/macros/trace_faulty_macros.stderr @@ -49,5 +49,16 @@ LL | my_recursive_macro!(); = note: expanding `my_recursive_macro! { }` = note: to `my_recursive_macro ! () ;` -error: aborting due to 2 previous errors +error: expected expression, found `A { a: a, b: 0, c: _, .. }` + --> $DIR/trace_faulty_macros.rs:16:9 + | +LL | $a + | ^^ expected expression +... +LL | let a = pat_macro!(); + | ------------ in this macro invocation + | + = note: this error originates in a macro (in Nightly builds, run with -Z macro-backtrace for more info) + +error: aborting due to 3 previous errors From 6cb584608cf3047750c2335325cccaa7521e8e04 Mon Sep 17 00:00:00 2001 From: Bastian Kauschke Date: Thu, 19 Mar 2020 20:20:09 +0100 Subject: [PATCH 21/29] sort generic param order in generics_of --- src/librustc_ast_passes/ast_validation.rs | 2 + src/librustc_typeck/collect.rs | 94 +++++++++++-------- src/test/ui/const-generics/argument_order.rs | 9 ++ .../ui/const-generics/argument_order.stderr | 16 ++++ 4 files changed, 84 insertions(+), 37 deletions(-) create mode 100644 src/test/ui/const-generics/argument_order.rs create mode 100644 src/test/ui/const-generics/argument_order.stderr diff --git a/src/librustc_ast_passes/ast_validation.rs b/src/librustc_ast_passes/ast_validation.rs index a2d83a525cc78..41209392b0811 100644 --- a/src/librustc_ast_passes/ast_validation.rs +++ b/src/librustc_ast_passes/ast_validation.rs @@ -645,6 +645,8 @@ impl<'a> AstValidator<'a> { } } +/// Checks that generic parameters are in the correct order, +/// which is lifetimes, then types and then consts. (`<'a, T, const N: usize>`) fn validate_generic_param_order<'a>( sess: &Session, handler: &rustc_errors::Handler, diff --git a/src/librustc_typeck/collect.rs b/src/librustc_typeck/collect.rs index cd63dacdcda0e..42d6e945ee2c2 100644 --- a/src/librustc_typeck/collect.rs +++ b/src/librustc_typeck/collect.rs @@ -1291,47 +1291,67 @@ fn generics_of(tcx: TyCtxt<'_>, def_id: DefId) -> &ty::Generics { // Now create the real type and const parameters. let type_start = own_start - has_self as u32 + params.len() as u32; let mut i = 0; - params.extend(ast_generics.params.iter().filter_map(|param| { - let kind = match param.kind { - GenericParamKind::Type { ref default, synthetic, .. } => { - if !allow_defaults && default.is_some() { - if !tcx.features().default_type_parameter_fallback { - tcx.struct_span_lint_hir( - lint::builtin::INVALID_TYPE_PARAM_DEFAULT, - param.hir_id, - param.span, - |lint| { - lint.build( - "defaults for type parameters are only allowed in \ - `struct`, `enum`, `type`, or `trait` definitions.", - ) - .emit(); - }, - ); - } - } - ty::GenericParamDefKind::Type { - has_default: default.is_some(), - object_lifetime_default: object_lifetime_defaults - .as_ref() - .map_or(rl::Set1::Empty, |o| o[i]), - synthetic, + // FIXME(const_generics): a few places in the compiler expect generic params + // to be in the order lifetimes, then type params, then const params. + // + // To prevent internal errors in case a const params are supplied in front of + // type parameters we first add all type params, then all const params. + params.extend(ast_generics.params.iter().filter_map(|param| { + if let GenericParamKind::Type { ref default, synthetic, .. } = param.kind { + if !allow_defaults && default.is_some() { + if !tcx.features().default_type_parameter_fallback { + tcx.struct_span_lint_hir( + lint::builtin::INVALID_TYPE_PARAM_DEFAULT, + param.hir_id, + param.span, + |lint| { + lint.build( + "defaults for type parameters are only allowed in \ + `struct`, `enum`, `type`, or `trait` definitions.", + ) + .emit(); + }, + ); } } - GenericParamKind::Const { .. } => ty::GenericParamDefKind::Const, - _ => return None, - }; - let param_def = ty::GenericParamDef { - index: type_start + i as u32, - name: param.name.ident().name, - def_id: tcx.hir().local_def_id(param.hir_id), - pure_wrt_drop: param.pure_wrt_drop, - kind, - }; - i += 1; - Some(param_def) + let kind = ty::GenericParamDefKind::Type { + has_default: default.is_some(), + object_lifetime_default: object_lifetime_defaults + .as_ref() + .map_or(rl::Set1::Empty, |o| o[i]), + synthetic, + }; + + let param_def = ty::GenericParamDef { + index: type_start + i as u32, + name: param.name.ident().name, + def_id: tcx.hir().local_def_id(param.hir_id), + pure_wrt_drop: param.pure_wrt_drop, + kind, + }; + i += 1; + Some(param_def) + } else { + None + } + })); + + params.extend(ast_generics.params.iter().filter_map(|param| { + if let GenericParamKind::Const { .. } = param.kind { + let param_def = ty::GenericParamDef { + index: type_start + i as u32, + name: param.name.ident().name, + def_id: tcx.hir().local_def_id(param.hir_id), + pure_wrt_drop: param.pure_wrt_drop, + kind: ty::GenericParamDefKind::Const, + }; + i += 1; + Some(param_def) + } else { + None + } })); // provide junk type parameter defs - the only place that diff --git a/src/test/ui/const-generics/argument_order.rs b/src/test/ui/const-generics/argument_order.rs new file mode 100644 index 0000000000000..3446600d0495f --- /dev/null +++ b/src/test/ui/const-generics/argument_order.rs @@ -0,0 +1,9 @@ +#![feature(const_generics)] +//~^ WARN the feature `const_generics` is incomplete and may cause the compiler to crash + +struct Bad { //~ ERROR type parameters must be declared prior + arr: [u8; { N }], + another: T, +} + +fn main() { } diff --git a/src/test/ui/const-generics/argument_order.stderr b/src/test/ui/const-generics/argument_order.stderr new file mode 100644 index 0000000000000..1e3b364eb6089 --- /dev/null +++ b/src/test/ui/const-generics/argument_order.stderr @@ -0,0 +1,16 @@ +error: type parameters must be declared prior to const parameters + --> $DIR/argument_order.rs:4:28 + | +LL | struct Bad { + | -----------------^- help: reorder the parameters: lifetimes, then types, then consts: `` + +warning: the feature `const_generics` is incomplete and may cause the compiler to crash + --> $DIR/argument_order.rs:1:12 + | +LL | #![feature(const_generics)] + | ^^^^^^^^^^^^^^ + | + = note: `#[warn(incomplete_features)]` on by default + +error: aborting due to previous error + From 17c94c6746e94a4349a05bbf6326bebe35893acc Mon Sep 17 00:00:00 2001 From: Bastian Kauschke Date: Fri, 20 Mar 2020 00:28:49 +0100 Subject: [PATCH 22/29] fix FIXME comment Co-Authored-By: varkor --- src/librustc_typeck/collect.rs | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/librustc_typeck/collect.rs b/src/librustc_typeck/collect.rs index 42d6e945ee2c2..dadc5b1c12956 100644 --- a/src/librustc_typeck/collect.rs +++ b/src/librustc_typeck/collect.rs @@ -1295,7 +1295,7 @@ fn generics_of(tcx: TyCtxt<'_>, def_id: DefId) -> &ty::Generics { // FIXME(const_generics): a few places in the compiler expect generic params // to be in the order lifetimes, then type params, then const params. // - // To prevent internal errors in case a const params are supplied in front of + // To prevent internal errors in case const parameters are supplied before // type parameters we first add all type params, then all const params. params.extend(ast_generics.params.iter().filter_map(|param| { if let GenericParamKind::Type { ref default, synthetic, .. } = param.kind { From 5edaa7eefd76d4996dcf85dfc1c1a3f737087257 Mon Sep 17 00:00:00 2001 From: Alex Crichton Date: Thu, 12 Mar 2020 11:39:30 -0700 Subject: [PATCH 23/29] Fix abort-on-eprintln during process shutdown This commit fixes an issue where if `eprintln!` is used in a TLS destructor it can accidentally cause the process to abort. TLS destructors are executed after `main` returns on the main thread, and at this point we've also deinitialized global `Lazy` values like those which store the `Stderr` and `Stdout` internals. This means that despite handling TLS not being accessible in `eprintln!`, we will fail due to not being able to call `stderr()`. This means that we'll double-panic quickly because panicking also attempt to write to stderr. The fix here is to reimplement the global stderr handle to avoid the need for destruction. This avoids the need for `Lazy` as well as the hidden panic inside of the `stderr` function. Overall this should improve the robustness of printing errors and/or panics in weird situations, since the `stderr` accessor should be infallible in more situations. --- src/libstd/io/stdio.rs | 49 +++++++----- src/libstd/sys/cloudabi/mutex.rs | 8 +- src/libstd/sys/hermit/mutex.rs | 6 +- src/libstd/sys/sgx/mutex.rs | 2 +- src/libstd/sys/unix/mutex.rs | 4 +- src/libstd/sys/vxworks/mutex.rs | 4 +- src/libstd/sys/wasm/mutex.rs | 4 +- src/libstd/sys/wasm/mutex_atomics.rs | 4 +- src/libstd/sys/windows/mutex.rs | 6 +- src/libstd/sys_common/remutex.rs | 114 ++++++++++++--------------- src/test/ui/eprint-on-tls-drop.rs | 48 +++++++++++ 11 files changed, 149 insertions(+), 100 deletions(-) create mode 100644 src/test/ui/eprint-on-tls-drop.rs diff --git a/src/libstd/io/stdio.rs b/src/libstd/io/stdio.rs index d410faca30d9e..0ac928817184d 100644 --- a/src/libstd/io/stdio.rs +++ b/src/libstd/io/stdio.rs @@ -6,7 +6,7 @@ use crate::cell::RefCell; use crate::fmt; use crate::io::lazy::Lazy; use crate::io::{self, BufReader, Initializer, IoSlice, IoSliceMut, LineWriter}; -use crate::sync::{Arc, Mutex, MutexGuard}; +use crate::sync::{Arc, Mutex, MutexGuard, Once}; use crate::sys::stdio; use crate::sys_common::remutex::{ReentrantMutex, ReentrantMutexGuard}; use crate::thread::LocalKey; @@ -493,7 +493,11 @@ pub fn stdout() -> Stdout { Ok(stdout) => Maybe::Real(stdout), _ => Maybe::Fake, }; - Arc::new(ReentrantMutex::new(RefCell::new(LineWriter::new(stdout)))) + unsafe { + let ret = Arc::new(ReentrantMutex::new(RefCell::new(LineWriter::new(stdout)))); + ret.init(); + return ret; + } } } @@ -520,7 +524,7 @@ impl Stdout { /// ``` #[stable(feature = "rust1", since = "1.0.0")] pub fn lock(&self) -> StdoutLock<'_> { - StdoutLock { inner: self.inner.lock().unwrap_or_else(|e| e.into_inner()) } + StdoutLock { inner: self.inner.lock() } } } @@ -581,7 +585,7 @@ impl fmt::Debug for StdoutLock<'_> { /// an error. #[stable(feature = "rust1", since = "1.0.0")] pub struct Stderr { - inner: Arc>>>, + inner: &'static ReentrantMutex>>, } /// A locked reference to the `Stderr` handle. @@ -639,19 +643,28 @@ pub struct StderrLock<'a> { /// ``` #[stable(feature = "rust1", since = "1.0.0")] pub fn stderr() -> Stderr { - static INSTANCE: Lazy>>> = Lazy::new(); - return Stderr { - inner: unsafe { INSTANCE.get(stderr_init).expect("cannot access stderr during shutdown") }, - }; - - fn stderr_init() -> Arc>>> { - // This must not reentrantly access `INSTANCE` - let stderr = match stderr_raw() { - Ok(stderr) => Maybe::Real(stderr), - _ => Maybe::Fake, - }; - Arc::new(ReentrantMutex::new(RefCell::new(stderr))) - } + // Note that unlike `stdout()` we don't use `Lazy` here which registers a + // destructor. Stderr is not buffered nor does the `stderr_raw` type consume + // any owned resources, so there's no need to run any destructors at some + // point in the future. + // + // This has the added benefit of allowing `stderr` to be usable during + // process shutdown as well! + static INSTANCE: ReentrantMutex>> = + unsafe { ReentrantMutex::new(RefCell::new(Maybe::Fake)) }; + + // When accessing stderr we need one-time initialization of the reentrant + // mutex, followed by one-time detection of whether we actually have a + // stderr handle or not. Afterwards we can just always use the now-filled-in + // `INSTANCE` value. + static INIT: Once = Once::new(); + INIT.call_once(|| unsafe { + INSTANCE.init(); + if let Ok(stderr) = stderr_raw() { + *INSTANCE.lock().borrow_mut() = Maybe::Real(stderr); + } + }); + return Stderr { inner: &INSTANCE }; } impl Stderr { @@ -677,7 +690,7 @@ impl Stderr { /// ``` #[stable(feature = "rust1", since = "1.0.0")] pub fn lock(&self) -> StderrLock<'_> { - StderrLock { inner: self.inner.lock().unwrap_or_else(|e| e.into_inner()) } + StderrLock { inner: self.inner.lock() } } } diff --git a/src/libstd/sys/cloudabi/mutex.rs b/src/libstd/sys/cloudabi/mutex.rs index 4aa25e2505271..580ab0e8ad863 100644 --- a/src/libstd/sys/cloudabi/mutex.rs +++ b/src/libstd/sys/cloudabi/mutex.rs @@ -53,16 +53,16 @@ pub struct ReentrantMutex { } impl ReentrantMutex { - pub unsafe fn uninitialized() -> ReentrantMutex { + pub const unsafe fn uninitialized() -> ReentrantMutex { ReentrantMutex { lock: UnsafeCell::new(MaybeUninit::uninit()), recursion: UnsafeCell::new(MaybeUninit::uninit()), } } - pub unsafe fn init(&mut self) { - self.lock = UnsafeCell::new(MaybeUninit::new(AtomicU32::new(abi::LOCK_UNLOCKED.0))); - self.recursion = UnsafeCell::new(MaybeUninit::new(0)); + pub unsafe fn init(&self) { + *self.lock.get() = MaybeUninit::new(AtomicU32::new(abi::LOCK_UNLOCKED.0)); + *self.recursion.get() = MaybeUninit::new(0); } pub unsafe fn try_lock(&self) -> bool { diff --git a/src/libstd/sys/hermit/mutex.rs b/src/libstd/sys/hermit/mutex.rs index b5c75f738d228..3d4813209cbc4 100644 --- a/src/libstd/sys/hermit/mutex.rs +++ b/src/libstd/sys/hermit/mutex.rs @@ -46,13 +46,13 @@ pub struct ReentrantMutex { } impl ReentrantMutex { - pub unsafe fn uninitialized() -> ReentrantMutex { + pub const unsafe fn uninitialized() -> ReentrantMutex { ReentrantMutex { inner: ptr::null() } } #[inline] - pub unsafe fn init(&mut self) { - let _ = abi::recmutex_init(&mut self.inner as *mut *const c_void); + pub unsafe fn init(&self) { + let _ = abi::recmutex_init(&self.inner as *const *const c_void as *mut _); } #[inline] diff --git a/src/libstd/sys/sgx/mutex.rs b/src/libstd/sys/sgx/mutex.rs index eebbea1b285ba..4911c2f538769 100644 --- a/src/libstd/sys/sgx/mutex.rs +++ b/src/libstd/sys/sgx/mutex.rs @@ -75,7 +75,7 @@ impl ReentrantMutex { } #[inline] - pub unsafe fn init(&mut self) {} + pub unsafe fn init(&self) {} #[inline] pub unsafe fn lock(&self) { diff --git a/src/libstd/sys/unix/mutex.rs b/src/libstd/sys/unix/mutex.rs index b38375a2e03c5..103d87e3d2f91 100644 --- a/src/libstd/sys/unix/mutex.rs +++ b/src/libstd/sys/unix/mutex.rs @@ -92,11 +92,11 @@ unsafe impl Send for ReentrantMutex {} unsafe impl Sync for ReentrantMutex {} impl ReentrantMutex { - pub unsafe fn uninitialized() -> ReentrantMutex { + pub const unsafe fn uninitialized() -> ReentrantMutex { ReentrantMutex { inner: UnsafeCell::new(libc::PTHREAD_MUTEX_INITIALIZER) } } - pub unsafe fn init(&mut self) { + pub unsafe fn init(&self) { let mut attr = MaybeUninit::::uninit(); let result = libc::pthread_mutexattr_init(attr.as_mut_ptr()); debug_assert_eq!(result, 0); diff --git a/src/libstd/sys/vxworks/mutex.rs b/src/libstd/sys/vxworks/mutex.rs index b38375a2e03c5..103d87e3d2f91 100644 --- a/src/libstd/sys/vxworks/mutex.rs +++ b/src/libstd/sys/vxworks/mutex.rs @@ -92,11 +92,11 @@ unsafe impl Send for ReentrantMutex {} unsafe impl Sync for ReentrantMutex {} impl ReentrantMutex { - pub unsafe fn uninitialized() -> ReentrantMutex { + pub const unsafe fn uninitialized() -> ReentrantMutex { ReentrantMutex { inner: UnsafeCell::new(libc::PTHREAD_MUTEX_INITIALIZER) } } - pub unsafe fn init(&mut self) { + pub unsafe fn init(&self) { let mut attr = MaybeUninit::::uninit(); let result = libc::pthread_mutexattr_init(attr.as_mut_ptr()); debug_assert_eq!(result, 0); diff --git a/src/libstd/sys/wasm/mutex.rs b/src/libstd/sys/wasm/mutex.rs index 07238d087308f..7aaf1b3a343b6 100644 --- a/src/libstd/sys/wasm/mutex.rs +++ b/src/libstd/sys/wasm/mutex.rs @@ -47,11 +47,11 @@ impl Mutex { pub struct ReentrantMutex {} impl ReentrantMutex { - pub unsafe fn uninitialized() -> ReentrantMutex { + pub const unsafe fn uninitialized() -> ReentrantMutex { ReentrantMutex {} } - pub unsafe fn init(&mut self) {} + pub unsafe fn init(&self) {} pub unsafe fn lock(&self) {} diff --git a/src/libstd/sys/wasm/mutex_atomics.rs b/src/libstd/sys/wasm/mutex_atomics.rs index 90c628a19c22e..268a53bb5641c 100644 --- a/src/libstd/sys/wasm/mutex_atomics.rs +++ b/src/libstd/sys/wasm/mutex_atomics.rs @@ -80,11 +80,11 @@ unsafe impl Sync for ReentrantMutex {} // released when this recursion counter reaches 0. impl ReentrantMutex { - pub unsafe fn uninitialized() -> ReentrantMutex { + pub const unsafe fn uninitialized() -> ReentrantMutex { ReentrantMutex { owner: AtomicU32::new(0), recursions: UnsafeCell::new(0) } } - pub unsafe fn init(&mut self) { + pub unsafe fn init(&self) { // nothing to do... } diff --git a/src/libstd/sys/windows/mutex.rs b/src/libstd/sys/windows/mutex.rs index 281eb294c65d8..63dfc640908e9 100644 --- a/src/libstd/sys/windows/mutex.rs +++ b/src/libstd/sys/windows/mutex.rs @@ -109,7 +109,7 @@ impl Mutex { 0 => {} n => return n as *mut _, } - let mut re = box ReentrantMutex::uninitialized(); + let re = box ReentrantMutex::uninitialized(); re.init(); let re = Box::into_raw(re); match self.lock.compare_and_swap(0, re as usize, Ordering::SeqCst) { @@ -157,11 +157,11 @@ unsafe impl Send for ReentrantMutex {} unsafe impl Sync for ReentrantMutex {} impl ReentrantMutex { - pub fn uninitialized() -> ReentrantMutex { + pub const fn uninitialized() -> ReentrantMutex { ReentrantMutex { inner: UnsafeCell::new(MaybeUninit::uninit()) } } - pub unsafe fn init(&mut self) { + pub unsafe fn init(&self) { c::InitializeCriticalSection((&mut *self.inner.get()).as_mut_ptr()); } diff --git a/src/libstd/sys_common/remutex.rs b/src/libstd/sys_common/remutex.rs index a1ad44a3666ed..4f19bbc467f33 100644 --- a/src/libstd/sys_common/remutex.rs +++ b/src/libstd/sys_common/remutex.rs @@ -3,7 +3,6 @@ use crate::marker; use crate::ops::Deref; use crate::panic::{RefUnwindSafe, UnwindSafe}; use crate::sys::mutex as sys; -use crate::sys_common::poison::{self, LockResult, TryLockError, TryLockResult}; /// A re-entrant mutual exclusion /// @@ -11,8 +10,7 @@ use crate::sys_common::poison::{self, LockResult, TryLockError, TryLockResult}; /// available. The thread which has already locked the mutex can lock it /// multiple times without blocking, preventing a common source of deadlocks. pub struct ReentrantMutex { - inner: Box, - poison: poison::Flag, + inner: sys::ReentrantMutex, data: T, } @@ -39,23 +37,30 @@ pub struct ReentrantMutexGuard<'a, T: 'a> { // funny underscores due to how Deref currently works (it disregards field // privacy). __lock: &'a ReentrantMutex, - __poison: poison::Guard, } impl !marker::Send for ReentrantMutexGuard<'_, T> {} impl ReentrantMutex { /// Creates a new reentrant mutex in an unlocked state. - pub fn new(t: T) -> ReentrantMutex { - unsafe { - let mut mutex = ReentrantMutex { - inner: box sys::ReentrantMutex::uninitialized(), - poison: poison::Flag::new(), - data: t, - }; - mutex.inner.init(); - mutex - } + /// + /// # Unsafety + /// + /// This function is unsafe because it is required that `init` is called + /// once this mutex is in its final resting place, and only then are the + /// lock/unlock methods safe. + pub const unsafe fn new(t: T) -> ReentrantMutex { + ReentrantMutex { inner: sys::ReentrantMutex::uninitialized(), data: t } + } + + /// Initializes this mutex so it's ready for use. + /// + /// # Unsafety + /// + /// Unsafe to call more than once, and must be called after this will no + /// longer move in memory. + pub unsafe fn init(&self) { + self.inner.init(); } /// Acquires a mutex, blocking the current thread until it is able to do so. @@ -70,7 +75,7 @@ impl ReentrantMutex { /// If another user of this mutex panicked while holding the mutex, then /// this call will return failure if the mutex would otherwise be /// acquired. - pub fn lock(&self) -> LockResult> { + pub fn lock(&self) -> ReentrantMutexGuard<'_, T> { unsafe { self.inner.lock() } ReentrantMutexGuard::new(&self) } @@ -87,12 +92,8 @@ impl ReentrantMutex { /// If another user of this mutex panicked while holding the mutex, then /// this call will return failure if the mutex would otherwise be /// acquired. - pub fn try_lock(&self) -> TryLockResult> { - if unsafe { self.inner.try_lock() } { - Ok(ReentrantMutexGuard::new(&self)?) - } else { - Err(TryLockError::WouldBlock) - } + pub fn try_lock(&self) -> Option> { + if unsafe { self.inner.try_lock() } { Some(ReentrantMutexGuard::new(&self)) } else { None } } } @@ -108,11 +109,8 @@ impl Drop for ReentrantMutex { impl fmt::Debug for ReentrantMutex { fn fmt(&self, f: &mut fmt::Formatter<'_>) -> fmt::Result { match self.try_lock() { - Ok(guard) => f.debug_struct("ReentrantMutex").field("data", &*guard).finish(), - Err(TryLockError::Poisoned(err)) => { - f.debug_struct("ReentrantMutex").field("data", &**err.get_ref()).finish() - } - Err(TryLockError::WouldBlock) => { + Some(guard) => f.debug_struct("ReentrantMutex").field("data", &*guard).finish(), + None => { struct LockedPlaceholder; impl fmt::Debug for LockedPlaceholder { fn fmt(&self, f: &mut fmt::Formatter<'_>) -> fmt::Result { @@ -127,11 +125,8 @@ impl fmt::Debug for ReentrantMutex { } impl<'mutex, T> ReentrantMutexGuard<'mutex, T> { - fn new(lock: &'mutex ReentrantMutex) -> LockResult> { - poison::map_result(lock.poison.borrow(), |guard| ReentrantMutexGuard { - __lock: lock, - __poison: guard, - }) + fn new(lock: &'mutex ReentrantMutex) -> ReentrantMutexGuard<'mutex, T> { + ReentrantMutexGuard { __lock: lock } } } @@ -147,7 +142,6 @@ impl Drop for ReentrantMutexGuard<'_, T> { #[inline] fn drop(&mut self) { unsafe { - self.__lock.poison.done(&self.__poison); self.__lock.inner.unlock(); } } @@ -162,13 +156,17 @@ mod tests { #[test] fn smoke() { - let m = ReentrantMutex::new(()); + let m = unsafe { + let m = ReentrantMutex::new(()); + m.init(); + m + }; { - let a = m.lock().unwrap(); + let a = m.lock(); { - let b = m.lock().unwrap(); + let b = m.lock(); { - let c = m.lock().unwrap(); + let c = m.lock(); assert_eq!(*c, ()); } assert_eq!(*b, ()); @@ -179,15 +177,19 @@ mod tests { #[test] fn is_mutex() { - let m = Arc::new(ReentrantMutex::new(RefCell::new(0))); + let m = unsafe { + let m = Arc::new(ReentrantMutex::new(RefCell::new(0))); + m.init(); + m + }; let m2 = m.clone(); - let lock = m.lock().unwrap(); + let lock = m.lock(); let child = thread::spawn(move || { - let lock = m2.lock().unwrap(); + let lock = m2.lock(); assert_eq!(*lock.borrow(), 4950); }); for i in 0..100 { - let lock = m.lock().unwrap(); + let lock = m.lock(); *lock.borrow_mut() += i; } drop(lock); @@ -196,17 +198,21 @@ mod tests { #[test] fn trylock_works() { - let m = Arc::new(ReentrantMutex::new(())); + let m = unsafe { + let m = Arc::new(ReentrantMutex::new(())); + m.init(); + m + }; let m2 = m.clone(); - let _lock = m.try_lock().unwrap(); - let _lock2 = m.try_lock().unwrap(); + let _lock = m.try_lock(); + let _lock2 = m.try_lock(); thread::spawn(move || { let lock = m2.try_lock(); - assert!(lock.is_err()); + assert!(lock.is_none()); }) .join() .unwrap(); - let _lock3 = m.try_lock().unwrap(); + let _lock3 = m.try_lock(); } pub struct Answer<'a>(pub ReentrantMutexGuard<'a, RefCell>); @@ -215,22 +221,4 @@ mod tests { *self.0.borrow_mut() = 42; } } - - #[test] - fn poison_works() { - let m = Arc::new(ReentrantMutex::new(RefCell::new(0))); - let mc = m.clone(); - let result = thread::spawn(move || { - let lock = mc.lock().unwrap(); - *lock.borrow_mut() = 1; - let lock2 = mc.lock().unwrap(); - *lock.borrow_mut() = 2; - let _answer = Answer(lock2); - panic!("What the answer to my lifetimes dilemma is?"); - }) - .join(); - assert!(result.is_err()); - let r = m.lock().err().unwrap().into_inner(); - assert_eq!(*r.borrow(), 42); - } } diff --git a/src/test/ui/eprint-on-tls-drop.rs b/src/test/ui/eprint-on-tls-drop.rs new file mode 100644 index 0000000000000..9c4800c1a3fa1 --- /dev/null +++ b/src/test/ui/eprint-on-tls-drop.rs @@ -0,0 +1,48 @@ +// run-pass +// ignore-emscripten no processes + +use std::cell::RefCell; +use std::env; +use std::process::Command; + +fn main() { + let name = "YOU_ARE_THE_TEST"; + if env::var(name).is_ok() { + std::thread::spawn(|| { + TLS.with(|f| f.borrow().ensure()); + }) + .join() + .unwrap(); + } else { + let me = env::current_exe().unwrap(); + let output = Command::new(&me).env(name, "1").output().unwrap(); + println!("{:?}", output); + assert!(output.status.success()); + let stderr = String::from_utf8(output.stderr).unwrap(); + assert!(stderr.contains("hello new\n")); + assert!(stderr.contains("hello drop\n")); + } +} + +struct Stuff { + _x: usize, +} + +impl Stuff { + fn new() -> Self { + eprintln!("hello new"); + Self { _x: 0 } + } + + fn ensure(&self) {} +} + +impl Drop for Stuff { + fn drop(&mut self) { + eprintln!("hello drop"); + } +} + +thread_local! { + static TLS: RefCell = RefCell::new(Stuff::new()); +} From 0296d4968eb28dad447a9b0e0f00925236be1ee7 Mon Sep 17 00:00:00 2001 From: Ralf Jung Date: Tue, 10 Mar 2020 21:53:22 +0100 Subject: [PATCH 24/29] fix layout_test visitor name --- src/librustc_passes/layout_test.rs | 8 ++++---- 1 file changed, 4 insertions(+), 4 deletions(-) diff --git a/src/librustc_passes/layout_test.rs b/src/librustc_passes/layout_test.rs index 9f6598a0ec1fe..b0edbb46e2929 100644 --- a/src/librustc_passes/layout_test.rs +++ b/src/librustc_passes/layout_test.rs @@ -17,15 +17,15 @@ use rustc_span::symbol::sym; pub fn test_layout(tcx: TyCtxt<'_>) { if tcx.features().rustc_attrs { // if the `rustc_attrs` feature is not enabled, don't bother testing layout - tcx.hir().krate().visit_all_item_likes(&mut VarianceTest { tcx }); + tcx.hir().krate().visit_all_item_likes(&mut LayoutTest { tcx }); } } -struct VarianceTest<'tcx> { +struct LayoutTest<'tcx> { tcx: TyCtxt<'tcx>, } -impl ItemLikeVisitor<'tcx> for VarianceTest<'tcx> { +impl ItemLikeVisitor<'tcx> for LayoutTest<'tcx> { fn visit_item(&mut self, item: &'tcx hir::Item<'tcx>) { let item_def_id = self.tcx.hir().local_def_id(item.hir_id); @@ -42,7 +42,7 @@ impl ItemLikeVisitor<'tcx> for VarianceTest<'tcx> { fn visit_impl_item(&mut self, _: &'tcx hir::ImplItem<'tcx>) {} } -impl VarianceTest<'tcx> { +impl LayoutTest<'tcx> { fn dump_layout_of(&self, item_def_id: DefId, item: &hir::Item<'tcx>, attr: &Attribute) { let tcx = self.tcx; let param_env = self.tcx.param_env(item_def_id); From 55c2cf2a3214cc3be11d9e27da5aa419653cac0c Mon Sep 17 00:00:00 2001 From: Ralf Jung Date: Tue, 10 Mar 2020 22:25:53 +0100 Subject: [PATCH 25/29] add debug option to #[rustc_layout] --- src/librustc_passes/layout_test.rs | 7 +++++++ src/librustc_span/symbol.rs | 1 + 2 files changed, 8 insertions(+) diff --git a/src/librustc_passes/layout_test.rs b/src/librustc_passes/layout_test.rs index b0edbb46e2929..66297eb972736 100644 --- a/src/librustc_passes/layout_test.rs +++ b/src/librustc_passes/layout_test.rs @@ -81,6 +81,13 @@ impl LayoutTest<'tcx> { ); } + sym::debug => { + self.tcx.sess.span_err( + item.span, + &format!("layout debugging: {:#?}", *ty_layout), + ); + } + name => { self.tcx.sess.span_err( meta_item.span(), diff --git a/src/librustc_span/symbol.rs b/src/librustc_span/symbol.rs index 5685505f6948d..771de54707e9e 100644 --- a/src/librustc_span/symbol.rs +++ b/src/librustc_span/symbol.rs @@ -253,6 +253,7 @@ symbols! { debug_trait, declare_lint_pass, decl_macro, + debug, Debug, Decodable, Default, From d9f60bcf67ea175ab3298608d8a94563e1ac0f6d Mon Sep 17 00:00:00 2001 From: Ralf Jung Date: Tue, 10 Mar 2020 22:31:27 +0100 Subject: [PATCH 26/29] add a test for rustc_layout(debug) --- src/test/ui/layout/debug.rs | 7 ++ src/test/ui/layout/debug.stderr | 120 ++++++++++++++++++++++++++++++++ 2 files changed, 127 insertions(+) create mode 100644 src/test/ui/layout/debug.rs create mode 100644 src/test/ui/layout/debug.stderr diff --git a/src/test/ui/layout/debug.rs b/src/test/ui/layout/debug.rs new file mode 100644 index 0000000000000..64a02ee5a22c8 --- /dev/null +++ b/src/test/ui/layout/debug.rs @@ -0,0 +1,7 @@ +#![feature(never_type, rustc_attrs)] +#![crate_type = "lib"] + +enum E { Foo, Bar(!, i32, i32) } + +#[rustc_layout(debug)] +type Test = E; //~ ERROR: layout debugging diff --git a/src/test/ui/layout/debug.stderr b/src/test/ui/layout/debug.stderr new file mode 100644 index 0000000000000..df8b70f307074 --- /dev/null +++ b/src/test/ui/layout/debug.stderr @@ -0,0 +1,120 @@ +error: layout debugging: LayoutDetails { + fields: Arbitrary { + offsets: [ + Size { + raw: 0, + }, + ], + memory_index: [ + 0, + ], + }, + variants: Multiple { + discr: Scalar { + value: Int( + I32, + false, + ), + valid_range: 0..=0, + }, + discr_kind: Tag, + discr_index: 0, + variants: [ + LayoutDetails { + fields: Arbitrary { + offsets: [], + memory_index: [], + }, + variants: Single { + index: 0, + }, + abi: Aggregate { + sized: true, + }, + largest_niche: None, + align: AbiAndPrefAlign { + abi: Align { + pow2: 0, + }, + pref: Align { + pow2: 3, + }, + }, + size: Size { + raw: 4, + }, + }, + LayoutDetails { + fields: Arbitrary { + offsets: [ + Size { + raw: 4, + }, + Size { + raw: 4, + }, + Size { + raw: 8, + }, + ], + memory_index: [ + 0, + 1, + 2, + ], + }, + variants: Single { + index: 1, + }, + abi: Uninhabited, + largest_niche: None, + align: AbiAndPrefAlign { + abi: Align { + pow2: 2, + }, + pref: Align { + pow2: 3, + }, + }, + size: Size { + raw: 12, + }, + }, + ], + }, + abi: Aggregate { + sized: true, + }, + largest_niche: Some( + Niche { + offset: Size { + raw: 0, + }, + scalar: Scalar { + value: Int( + I32, + false, + ), + valid_range: 0..=0, + }, + }, + ), + align: AbiAndPrefAlign { + abi: Align { + pow2: 2, + }, + pref: Align { + pow2: 3, + }, + }, + size: Size { + raw: 12, + }, +} + --> $DIR/debug.rs:7:1 + | +LL | type Test = E; + | ^^^^^^^^^^^^^^ + +error: aborting due to previous error + From c62e36bf4c6f7a128f1af4dadd2abd5ecce397f0 Mon Sep 17 00:00:00 2001 From: Ralf Jung Date: Fri, 20 Mar 2020 17:48:03 +0100 Subject: [PATCH 27/29] make rustc_layout also work for type definitions --- src/librustc_passes/layout_test.rs | 14 +- src/test/ui/layout/debug.rs | 11 +- src/test/ui/layout/debug.stderr | 223 ++++++++++++++++++++++++++++- 3 files changed, 238 insertions(+), 10 deletions(-) diff --git a/src/librustc_passes/layout_test.rs b/src/librustc_passes/layout_test.rs index 66297eb972736..32561c6bd87e7 100644 --- a/src/librustc_passes/layout_test.rs +++ b/src/librustc_passes/layout_test.rs @@ -29,12 +29,18 @@ impl ItemLikeVisitor<'tcx> for LayoutTest<'tcx> { fn visit_item(&mut self, item: &'tcx hir::Item<'tcx>) { let item_def_id = self.tcx.hir().local_def_id(item.hir_id); - if let ItemKind::TyAlias(..) = item.kind { - for attr in self.tcx.get_attrs(item_def_id).iter() { - if attr.check_name(sym::rustc_layout) { - self.dump_layout_of(item_def_id, item, attr); + match item.kind { + ItemKind::TyAlias(..) | + ItemKind::Enum(..) | + ItemKind::Struct(..) | + ItemKind::Union(..) => { + for attr in self.tcx.get_attrs(item_def_id).iter() { + if attr.check_name(sym::rustc_layout) { + self.dump_layout_of(item_def_id, item, attr); + } } } + _ => {} } } diff --git a/src/test/ui/layout/debug.rs b/src/test/ui/layout/debug.rs index 64a02ee5a22c8..047002c9c99e2 100644 --- a/src/test/ui/layout/debug.rs +++ b/src/test/ui/layout/debug.rs @@ -1,7 +1,14 @@ #![feature(never_type, rustc_attrs)] #![crate_type = "lib"] -enum E { Foo, Bar(!, i32, i32) } +#[rustc_layout(debug)] +enum E { Foo, Bar(!, i32, i32) } //~ ERROR: layout debugging + +#[rustc_layout(debug)] +struct S { f1: i32, f2: (), f3: i32 } //~ ERROR: layout debugging + +#[rustc_layout(debug)] +union U { f1: (i32, i32), f3: i32 } //~ ERROR: layout debugging #[rustc_layout(debug)] -type Test = E; //~ ERROR: layout debugging +type Test = Result; //~ ERROR: layout debugging diff --git a/src/test/ui/layout/debug.stderr b/src/test/ui/layout/debug.stderr index df8b70f307074..6e704528c4130 100644 --- a/src/test/ui/layout/debug.stderr +++ b/src/test/ui/layout/debug.stderr @@ -111,10 +111,225 @@ error: layout debugging: LayoutDetails { raw: 12, }, } - --> $DIR/debug.rs:7:1 + --> $DIR/debug.rs:5:1 | -LL | type Test = E; - | ^^^^^^^^^^^^^^ +LL | enum E { Foo, Bar(!, i32, i32) } + | ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ -error: aborting due to previous error +error: layout debugging: LayoutDetails { + fields: Arbitrary { + offsets: [ + Size { + raw: 0, + }, + Size { + raw: 0, + }, + Size { + raw: 4, + }, + ], + memory_index: [ + 1, + 0, + 2, + ], + }, + variants: Single { + index: 0, + }, + abi: ScalarPair( + Scalar { + value: Int( + I32, + true, + ), + valid_range: 0..=4294967295, + }, + Scalar { + value: Int( + I32, + true, + ), + valid_range: 0..=4294967295, + }, + ), + largest_niche: None, + align: AbiAndPrefAlign { + abi: Align { + pow2: 2, + }, + pref: Align { + pow2: 3, + }, + }, + size: Size { + raw: 8, + }, +} + --> $DIR/debug.rs:8:1 + | +LL | struct S { f1: i32, f2: (), f3: i32 } + | ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ + +error: layout debugging: LayoutDetails { + fields: Union( + 2, + ), + variants: Single { + index: 0, + }, + abi: Aggregate { + sized: true, + }, + largest_niche: None, + align: AbiAndPrefAlign { + abi: Align { + pow2: 2, + }, + pref: Align { + pow2: 3, + }, + }, + size: Size { + raw: 8, + }, +} + --> $DIR/debug.rs:11:1 + | +LL | union U { f1: (i32, i32), f3: i32 } + | ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ + +error: layout debugging: LayoutDetails { + fields: Arbitrary { + offsets: [ + Size { + raw: 0, + }, + ], + memory_index: [ + 0, + ], + }, + variants: Multiple { + discr: Scalar { + value: Int( + I32, + false, + ), + valid_range: 0..=1, + }, + discr_kind: Tag, + discr_index: 0, + variants: [ + LayoutDetails { + fields: Arbitrary { + offsets: [ + Size { + raw: 4, + }, + ], + memory_index: [ + 0, + ], + }, + variants: Single { + index: 0, + }, + abi: Aggregate { + sized: true, + }, + largest_niche: None, + align: AbiAndPrefAlign { + abi: Align { + pow2: 2, + }, + pref: Align { + pow2: 3, + }, + }, + size: Size { + raw: 8, + }, + }, + LayoutDetails { + fields: Arbitrary { + offsets: [ + Size { + raw: 4, + }, + ], + memory_index: [ + 0, + ], + }, + variants: Single { + index: 1, + }, + abi: Aggregate { + sized: true, + }, + largest_niche: None, + align: AbiAndPrefAlign { + abi: Align { + pow2: 2, + }, + pref: Align { + pow2: 3, + }, + }, + size: Size { + raw: 8, + }, + }, + ], + }, + abi: ScalarPair( + Scalar { + value: Int( + I32, + false, + ), + valid_range: 0..=1, + }, + Scalar { + value: Int( + I32, + true, + ), + valid_range: 0..=4294967295, + }, + ), + largest_niche: Some( + Niche { + offset: Size { + raw: 0, + }, + scalar: Scalar { + value: Int( + I32, + false, + ), + valid_range: 0..=1, + }, + }, + ), + align: AbiAndPrefAlign { + abi: Align { + pow2: 2, + }, + pref: Align { + pow2: 3, + }, + }, + size: Size { + raw: 8, + }, +} + --> $DIR/debug.rs:14:1 + | +LL | type Test = Result; + | ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ + +error: aborting due to 4 previous errors From 7b49678f5f6e473e69c4a737acd1acc1b8d9a0f1 Mon Sep 17 00:00:00 2001 From: Ralf Jung Date: Fri, 20 Mar 2020 23:22:36 +0100 Subject: [PATCH 28/29] fmt --- src/librustc_passes/layout_test.rs | 8 ++++---- 1 file changed, 4 insertions(+), 4 deletions(-) diff --git a/src/librustc_passes/layout_test.rs b/src/librustc_passes/layout_test.rs index 32561c6bd87e7..9d8b1422bdf24 100644 --- a/src/librustc_passes/layout_test.rs +++ b/src/librustc_passes/layout_test.rs @@ -30,10 +30,10 @@ impl ItemLikeVisitor<'tcx> for LayoutTest<'tcx> { let item_def_id = self.tcx.hir().local_def_id(item.hir_id); match item.kind { - ItemKind::TyAlias(..) | - ItemKind::Enum(..) | - ItemKind::Struct(..) | - ItemKind::Union(..) => { + ItemKind::TyAlias(..) + | ItemKind::Enum(..) + | ItemKind::Struct(..) + | ItemKind::Union(..) => { for attr in self.tcx.get_attrs(item_def_id).iter() { if attr.check_name(sym::rustc_layout) { self.dump_layout_of(item_def_id, item, attr); From e548df7e362ec31f00a5661f207320587d91b529 Mon Sep 17 00:00:00 2001 From: Ralf Jung Date: Sat, 21 Mar 2020 10:07:44 +0100 Subject: [PATCH 29/29] normalize away preferred alignment --- src/test/ui/layout/debug.rs | 1 + src/test/ui/layout/debug.stderr | 40 ++++++++++----------------------- 2 files changed, 13 insertions(+), 28 deletions(-) diff --git a/src/test/ui/layout/debug.rs b/src/test/ui/layout/debug.rs index 047002c9c99e2..70ae200e3e5aa 100644 --- a/src/test/ui/layout/debug.rs +++ b/src/test/ui/layout/debug.rs @@ -1,3 +1,4 @@ +// normalize-stderr-test "pref: Align \{\n *pow2: [1-3],\n *\}" -> "pref: $$PREF_ALIGN" #![feature(never_type, rustc_attrs)] #![crate_type = "lib"] diff --git a/src/test/ui/layout/debug.stderr b/src/test/ui/layout/debug.stderr index 6e704528c4130..0ce538285f8c4 100644 --- a/src/test/ui/layout/debug.stderr +++ b/src/test/ui/layout/debug.stderr @@ -36,9 +36,7 @@ error: layout debugging: LayoutDetails { abi: Align { pow2: 0, }, - pref: Align { - pow2: 3, - }, + pref: $PREF_ALIGN, }, size: Size { raw: 4, @@ -72,9 +70,7 @@ error: layout debugging: LayoutDetails { abi: Align { pow2: 2, }, - pref: Align { - pow2: 3, - }, + pref: $PREF_ALIGN, }, size: Size { raw: 12, @@ -103,15 +99,13 @@ error: layout debugging: LayoutDetails { abi: Align { pow2: 2, }, - pref: Align { - pow2: 3, - }, + pref: $PREF_ALIGN, }, size: Size { raw: 12, }, } - --> $DIR/debug.rs:5:1 + --> $DIR/debug.rs:6:1 | LL | enum E { Foo, Bar(!, i32, i32) } | ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ @@ -159,15 +153,13 @@ error: layout debugging: LayoutDetails { abi: Align { pow2: 2, }, - pref: Align { - pow2: 3, - }, + pref: $PREF_ALIGN, }, size: Size { raw: 8, }, } - --> $DIR/debug.rs:8:1 + --> $DIR/debug.rs:9:1 | LL | struct S { f1: i32, f2: (), f3: i32 } | ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ @@ -187,15 +179,13 @@ error: layout debugging: LayoutDetails { abi: Align { pow2: 2, }, - pref: Align { - pow2: 3, - }, + pref: $PREF_ALIGN, }, size: Size { raw: 8, }, } - --> $DIR/debug.rs:11:1 + --> $DIR/debug.rs:12:1 | LL | union U { f1: (i32, i32), f3: i32 } | ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ @@ -244,9 +234,7 @@ error: layout debugging: LayoutDetails { abi: Align { pow2: 2, }, - pref: Align { - pow2: 3, - }, + pref: $PREF_ALIGN, }, size: Size { raw: 8, @@ -274,9 +262,7 @@ error: layout debugging: LayoutDetails { abi: Align { pow2: 2, }, - pref: Align { - pow2: 3, - }, + pref: $PREF_ALIGN, }, size: Size { raw: 8, @@ -318,15 +304,13 @@ error: layout debugging: LayoutDetails { abi: Align { pow2: 2, }, - pref: Align { - pow2: 3, - }, + pref: $PREF_ALIGN, }, size: Size { raw: 8, }, } - --> $DIR/debug.rs:14:1 + --> $DIR/debug.rs:15:1 | LL | type Test = Result; | ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^