Auto merge of #80891 - cjgillot:noq, r=Mark-Simulacrum

Make the `Query` enum a simple struct.

A lot of code in `rustc_query_system` is generic over it, only to encode an exceptional error case: query cycles.
The delayed computations are now done at cycle detection.

Author: bors

Cargo.lock

@@ -4202,6 +4202,7 @@ dependencies = [
  "rustc_index",
  "rustc_macros",
  "rustc_serialize",
+ "rustc_session",
  "rustc_span",
  "smallvec 1.6.1",
  "tracing",

compiler/rustc_middle/src/dep_graph/mod.rs

@@ -2,6 +2,7 @@ use crate::ich::StableHashingContext;
 use crate::ty::{self, TyCtxt};
 use rustc_data_structures::profiling::SelfProfilerRef;
 use rustc_data_structures::sync::Lock;
+use rustc_session::Session;
 
 #[macro_use]
 mod dep_node;
@@ -101,20 +102,18 @@ impl<'tcx> DepContext for TyCtxt<'tcx> {
         TyCtxt::create_stable_hashing_context(*self)
     }
 
-    fn debug_dep_tasks(&self) -> bool {
-        self.sess.opts.debugging_opts.dep_tasks
-    }
-    fn debug_dep_node(&self) -> bool {
-        self.sess.opts.debugging_opts.incremental_info
-            || self.sess.opts.debugging_opts.query_dep_graph
-    }
-
     #[inline]
     fn dep_graph(&self) -> &DepGraph {
         &self.dep_graph
     }
 
+    #[inline(always)]
     fn profiler(&self) -> &SelfProfilerRef {
         &self.prof
     }
+
+    #[inline(always)]
+    fn sess(&self) -> &Session {
+        self.sess
+    }
 }

compiler/rustc_query_impl/src/lib.rs

@@ -17,19 +17,17 @@ extern crate rustc_middle;
 extern crate tracing;
 
 use rustc_data_structures::fingerprint::Fingerprint;
-use rustc_data_structures::fx::FxHashMap;
 use rustc_data_structures::stable_hasher::{HashStable, StableHasher};
-use rustc_errors::{Diagnostic, Handler, Level};
+use rustc_errors::{DiagnosticBuilder, Handler};
 use rustc_hir::def_id::CrateNum;
 use rustc_index::vec::IndexVec;
 use rustc_middle::dep_graph;
 use rustc_middle::ich::StableHashingContext;
 use rustc_middle::ty::query::{query_keys, query_storage, query_stored, query_values};
 use rustc_middle::ty::query::{Providers, QueryEngine};
-use rustc_middle::ty::TyCtxt;
+use rustc_middle::ty::{self, TyCtxt};
 use rustc_serialize::opaque;
 use rustc_span::{Span, DUMMY_SP};
-use std::mem;
 
 #[macro_use]
 mod plumbing;

compiler/rustc_query_impl/src/plumbing.rs

@@ -2,22 +2,19 @@
 //! generate the actual methods on tcx which find and execute the provider,
 //! manage the caches, and so forth.
 
-use super::{queries, Query};
+use super::queries;
 use rustc_middle::dep_graph::{DepKind, DepNode, DepNodeExt, DepNodeIndex, SerializedDepNodeIndex};
 use rustc_middle::ty::query::on_disk_cache;
 use rustc_middle::ty::tls::{self, ImplicitCtxt};
 use rustc_middle::ty::{self, TyCtxt};
 use rustc_query_system::dep_graph::HasDepContext;
-use rustc_query_system::query::{CycleError, QueryJobId, QueryJobInfo};
-use rustc_query_system::query::{QueryContext, QueryDescription};
+use rustc_query_system::query::{QueryContext, QueryDescription, QueryJobId, QueryMap};
 
-use rustc_data_structures::fx::FxHashMap;
 use rustc_data_structures::sync::Lock;
 use rustc_data_structures::thin_vec::ThinVec;
-use rustc_errors::{struct_span_err, Diagnostic, DiagnosticBuilder};
+use rustc_errors::Diagnostic;
 use rustc_serialize::opaque;
 use rustc_span::def_id::{DefId, LocalDefId};
-use rustc_span::Span;
 
 #[derive(Copy, Clone)]
 pub struct QueryCtxt<'tcx> {
@@ -45,15 +42,6 @@ impl HasDepContext for QueryCtxt<'tcx> {
 }
 
 impl QueryContext for QueryCtxt<'tcx> {
-    type Query = Query<'tcx>;
-
-    fn incremental_verify_ich(&self) -> bool {
-        self.sess.opts.debugging_opts.incremental_verify_ich
-    }
-    fn verbose(&self) -> bool {
-        self.sess.verbose()
-    }
-
     fn def_path_str(&self, def_id: DefId) -> String {
         self.tcx.def_path_str(def_id)
     }
@@ -62,11 +50,8 @@ impl QueryContext for QueryCtxt<'tcx> {
         tls::with_related_context(**self, |icx| icx.query)
     }
 
-    fn try_collect_active_jobs(
-        &self,
-    ) -> Option<FxHashMap<QueryJobId<Self::DepKind>, QueryJobInfo<Self::DepKind, Self::Query>>>
-    {
-        self.queries.try_collect_active_jobs()
+    fn try_collect_active_jobs(&self) -> Option<QueryMap<Self::DepKind>> {
+        self.queries.try_collect_active_jobs(**self)
     }
 
     fn try_load_from_on_disk_cache(&self, dep_node: &DepNode) {
@@ -132,14 +117,6 @@ impl QueryContext for QueryCtxt<'tcx> {
         (cb.force_from_dep_node)(*self, dep_node)
     }
 
-    fn has_errors_or_delayed_span_bugs(&self) -> bool {
-        self.sess.has_errors_or_delayed_span_bugs()
-    }
-
-    fn diagnostic(&self) -> &rustc_errors::Handler {
-        self.sess.diagnostic()
-    }
-
     // Interactions with on_disk_cache
     fn load_diagnostics(&self, prev_dep_node_index: SerializedDepNodeIndex) -> Vec<Diagnostic> {
         self.on_disk_cache
@@ -196,54 +173,6 @@ impl QueryContext for QueryCtxt<'tcx> {
 }
 
 impl<'tcx> QueryCtxt<'tcx> {
-    #[inline(never)]
-    #[cold]
-    pub(super) fn report_cycle(
-        self,
-        CycleError { usage, cycle: stack }: CycleError<Query<'tcx>>,
-    ) -> DiagnosticBuilder<'tcx> {
-        assert!(!stack.is_empty());
-
-        let fix_span = |span: Span, query: &Query<'tcx>| {
-            self.sess.source_map().guess_head_span(query.default_span(*self, span))
-        };
-
-        // Disable naming impls with types in this path, since that
-        // sometimes cycles itself, leading to extra cycle errors.
-        // (And cycle errors around impls tend to occur during the
-        // collect/coherence phases anyhow.)
-        ty::print::with_forced_impl_filename_line(|| {
-            let span = fix_span(stack[1 % stack.len()].span, &stack[0].query);
-            let mut err = struct_span_err!(
-                self.sess,
-                span,
-                E0391,
-                "cycle detected when {}",
-                stack[0].query.describe(self)
-            );
-
-            for i in 1..stack.len() {
-                let query = &stack[i].query;
-                let span = fix_span(stack[(i + 1) % stack.len()].span, query);
-                err.span_note(span, &format!("...which requires {}...", query.describe(self)));
-            }
-
-            err.note(&format!(
-                "...which again requires {}, completing the cycle",
-                stack[0].query.describe(self)
-            ));
-
-            if let Some((span, query)) = usage {
-                err.span_note(
-                    fix_span(span, &query),
-                    &format!("cycle used when {}", query.describe(self)),
-                );
-            }
-
-            err
-        })
-    }
-
     pub(super) fn encode_query_results(
         self,
         encoder: &mut on_disk_cache::CacheEncoder<'a, 'tcx, opaque::FileEncoder>,
@@ -323,16 +252,16 @@ pub struct QueryStruct {
 
 macro_rules! handle_cycle_error {
     ([][$tcx: expr, $error:expr]) => {{
-        $tcx.report_cycle($error).emit();
+        $error.emit();
         Value::from_cycle_error($tcx)
     }};
     ([fatal_cycle $($rest:tt)*][$tcx:expr, $error:expr]) => {{
-        $tcx.report_cycle($error).emit();
+        $error.emit();
         $tcx.sess.abort_if_errors();
         unreachable!()
     }};
     ([cycle_delay_bug $($rest:tt)*][$tcx:expr, $error:expr]) => {{
-        $tcx.report_cycle($error).delay_as_bug();
+        $error.delay_as_bug();
         Value::from_cycle_error($tcx)
     }};
     ([$other:ident $(($($other_args:tt)*))* $(, $($modifiers:tt)*)*][$($args:tt)*]) => {
@@ -386,55 +315,40 @@ macro_rules! define_queries {
             input: ($(([$($modifiers)*] [$($attr)*] [$name]))*)
         }
 
-        #[allow(nonstandard_style)]
-        #[derive(Clone, Debug)]
-        pub enum Query<$tcx> {
-            $($(#[$attr])* $name(query_keys::$name<$tcx>)),*
-        }
-
-        impl<$tcx> Query<$tcx> {
-            pub fn name(&self) -> &'static str {
-                match *self {
-                    $(Query::$name(_) => stringify!($name),)*
-                }
-            }
+        mod make_query {
+            use super::*;
 
-            pub(crate) fn describe(&self, tcx: QueryCtxt<$tcx>) -> String {
-                let (r, name) = match *self {
-                    $(Query::$name(key) => {
-                        (queries::$name::describe(tcx, key), stringify!($name))
-                    })*
+            // Create an eponymous constructor for each query.
+            $(#[allow(nonstandard_style)] $(#[$attr])*
+            pub fn $name<$tcx>(tcx: QueryCtxt<$tcx>, key: query_keys::$name<$tcx>) -> QueryStackFrame {
+                let kind = dep_graph::DepKind::$name;
+                let name = stringify!($name);
+                let description = ty::print::with_forced_impl_filename_line(
+                    // Force filename-line mode to avoid invoking `type_of` query.
+                    || queries::$name::describe(tcx, key)
+                );
+                let description = if tcx.sess.verbose() {
+                    format!("{} [{}]", description, name)
+                } else {
+                    description
                 };
-                if tcx.sess.verbose() {
-                    format!("{} [{}]", r, name)
+                let span = if kind == dep_graph::DepKind::def_span {
+                    // The `def_span` query is used to calculate `default_span`,
+                    // so exit to avoid infinite recursion.
+                    None
                 } else {
-                    r
-                }
-            }
-
-            // FIXME(eddyb) Get more valid `Span`s on queries.
-            pub fn default_span(&self, tcx: TyCtxt<$tcx>, span: Span) -> Span {
-                if !span.is_dummy() {
-                    return span;
-                }
-                // The `def_span` query is used to calculate `default_span`,
-                // so exit to avoid infinite recursion.
-                if let Query::def_span(..) = *self {
-                    return span
-                }
-                match *self {
-                    $(Query::$name(key) => key.default_span(tcx),)*
-                }
-            }
-        }
+                    Some(key.default_span(*tcx))
+                };
+                let hash = || {
+                    let mut hcx = tcx.create_stable_hashing_context();
+                    let mut hasher = StableHasher::new();
+                    std::mem::discriminant(&kind).hash_stable(&mut hcx, &mut hasher);
+                    key.hash_stable(&mut hcx, &mut hasher);
+                    hasher.finish::<u64>()
+                };
 
-        impl<'a, $tcx> HashStable<StableHashingContext<'a>> for Query<$tcx> {
-            fn hash_stable(&self, hcx: &mut StableHashingContext<'a>, hasher: &mut StableHasher) {
-                mem::discriminant(self).hash_stable(hcx, hasher);
-                match *self {
-                    $(Query::$name(key) => key.hash_stable(hcx, hasher),)*
-                }
-            }
+                QueryStackFrame::new(name, description, span, hash)
+            })*
         }
 
         #[allow(nonstandard_style)]
@@ -461,7 +375,9 @@ macro_rules! define_queries {
             type Cache = query_storage::$name<$tcx>;
 
             #[inline(always)]
-            fn query_state<'a>(tcx: QueryCtxt<$tcx>) -> &'a QueryState<crate::dep_graph::DepKind, Query<$tcx>, Self::Key> {
+            fn query_state<'a>(tcx: QueryCtxt<$tcx>) -> &'a QueryState<crate::dep_graph::DepKind, Self::Key>
+                where QueryCtxt<$tcx>: 'a
+            {
                 &tcx.queries.$name
             }
 
@@ -493,7 +409,7 @@ macro_rules! define_queries {
 
             fn handle_cycle_error(
                 tcx: QueryCtxt<'tcx>,
-                error: CycleError<Query<'tcx>>
+                mut error: DiagnosticBuilder<'_>,
             ) -> Self::Value {
                 handle_cycle_error!([$($modifiers)*][tcx, error])
             }
@@ -596,7 +512,6 @@ macro_rules! define_queries_struct {
 
             $($(#[$attr])*  $name: QueryState<
                 crate::dep_graph::DepKind,
-                Query<$tcx>,
                 query_keys::$name<$tcx>,
             >,)*
         }
@@ -614,14 +529,17 @@ macro_rules! define_queries_struct {
             }
 
             pub(crate) fn try_collect_active_jobs(
-                &self
-            ) -> Option<FxHashMap<QueryJobId<crate::dep_graph::DepKind>, QueryJobInfo<crate::dep_graph::DepKind, Query<$tcx>>>> {
-                let mut jobs = FxHashMap::default();
+                &$tcx self,
+                tcx: TyCtxt<$tcx>,
+            ) -> Option<QueryMap<crate::dep_graph::DepKind>> {
+                let tcx = QueryCtxt { tcx, queries: self };
+                let mut jobs = QueryMap::default();
 
                 $(
                     self.$name.try_collect_active_jobs(
-                        <queries::$name<'tcx> as QueryAccessors<QueryCtxt<'tcx>>>::DEP_KIND,
-                        Query::$name,
+                        tcx,
+                        dep_graph::DepKind::$name,
+                        make_query::$name,
                         &mut jobs,
                     )?;
                 )*
@@ -666,38 +584,8 @@ macro_rules! define_queries_struct {
                 handler: &Handler,
                 num_frames: Option<usize>,
             ) -> usize {
-                let query_map = self.try_collect_active_jobs();
-
-                let mut current_query = query;
-                let mut i = 0;
-
-                while let Some(query) = current_query {
-                    if Some(i) == num_frames {
-                        break;
-                    }
-                    let query_info = if let Some(info) = query_map.as_ref().and_then(|map| map.get(&query))
-                    {
-                        info
-                    } else {
-                        break;
-                    };
-                    let mut diag = Diagnostic::new(
-                        Level::FailureNote,
-                        &format!(
-                            "#{} [{}] {}",
-                            i,
-                            query_info.info.query.name(),
-                            query_info.info.query.describe(QueryCtxt { tcx, queries: self })
-                        ),
-                    );
-                    diag.span = tcx.sess.source_map().guess_head_span(query_info.info.span).into();
-                    handler.force_print_diagnostic(diag);
-
-                    current_query = query_info.job.parent;
-                    i += 1;
-                }
-
-                i
+                let qcx = QueryCtxt { tcx, queries: self };
+                rustc_query_system::query::print_query_stack(qcx, query, handler, num_frames)
             }
 
             $($(#[$attr])*

compiler/rustc_query_system/Cargo.toml

@@ -16,6 +16,7 @@ rustc_errors = { path = "../rustc_errors" }
 rustc_macros = { path = "../rustc_macros" }
 rustc_index = { path = "../rustc_index" }
 rustc_serialize = { path = "../rustc_serialize" }
+rustc_session = { path = "../rustc_session" }
 rustc_span = { path = "../rustc_span" }
 parking_lot = "0.11"
 smallvec = { version = "1.6.1", features = ["union", "may_dangle"] }