Auto merge of rust-lang#79670 - Nadrieril:uninhabited-query, r=estebank

Turn type inhabitedness into a query to fix `exhaustive_patterns` perf

We measured in rust-lang#79394 that enabling the [`exhaustive_patterns` feature](rust-lang#51085) causes significant perf degradation. It was conjectured that the culprit is type inhabitedness checking, and [I hypothesized](rust-lang#79394 (comment)) that turning this computation into a query would solve most of the problem.

This PR turns `tcx.is_ty_uninhabited_from` into a query, and I measured a 25% perf gain on the benchmark that stress-tests `exhaustiveness_patterns`. This more than compensates for the 30% perf hit I measured [when creating it](rust-lang/rustc-perf#801). We'll have to measure enabling the feature again, but I suspect this fixes the perf regression entirely.
I'd like a perf run on this PR obviously.
I made small atomic commits to help reviewing. The first one is just me discovering the "revisions" feature of the testing framework.

I believe there's a push to move things out of `rustc_middle` because it's huge. I guess `inhabitedness/mod.rs` could be moved out, but it's quite small. `DefIdForest` might be movable somewhere too. I don't know what the policy is for that.

Ping `@camelid` since you were interested in following along
`@rustbot` modify labels: +A-exhaustiveness-checking

Author: bors

compiler/rustc_middle/src/query/mod.rs

@@ -1319,6 +1319,15 @@ rustc_queries! {
             eval_always
             desc { |tcx| "computing visibility of `{}`", tcx.def_path_str(def_id) }
         }
+
+        /// Computes the set of modules from which this type is visibly uninhabited.
+        /// To check whether a type is uninhabited at all (not just from a given module), you could
+        /// check whether the forest is empty.
+        query type_uninhabited_from(
+            key: ty::ParamEnvAnd<'tcx, Ty<'tcx>>
+        ) -> ty::inhabitedness::DefIdForest {
+            desc { "computing the inhabitedness of `{:?}`", key }
+        }
     }
 
     Other {

compiler/rustc_middle/src/ty/inhabitedness/def_id_forest.rs

@@ -3,6 +3,9 @@ use crate::ty::{DefId, DefIdTree};
 use rustc_hir::CRATE_HIR_ID;
 use smallvec::SmallVec;
 use std::mem;
+use std::sync::Arc;
+
+use DefIdForest::*;
 
 /// Represents a forest of `DefId`s closed under the ancestor relation. That is,
 /// if a `DefId` representing a module is contained in the forest then all
@@ -11,45 +14,77 @@ use std::mem;
 ///
 /// This is used to represent a set of modules in which a type is visibly
 /// uninhabited.
-#[derive(Clone)]
-pub struct DefIdForest {
-    /// The minimal set of `DefId`s required to represent the whole set.
-    /// If A and B are DefIds in the `DefIdForest`, and A is a descendant
-    /// of B, then only B will be in `root_ids`.
-    /// We use a `SmallVec` here because (for its use for caching inhabitedness)
-    /// it's rare that this will contain even two IDs.
-    root_ids: SmallVec<[DefId; 1]>,
+///
+/// We store the minimal set of `DefId`s required to represent the whole set. If A and B are
+/// `DefId`s in the `DefIdForest`, and A is a parent of B, then only A will be stored. When this is
+/// used with `type_uninhabited_from`, there will very rarely be more than one `DefId` stored.
+#[derive(Clone, HashStable)]
+pub enum DefIdForest {
+    Empty,
+    Single(DefId),
+    /// This variant is very rare.
+    /// Invariant: >1 elements
+    /// We use `Arc` because this is used in the output of a query.
+    Multiple(Arc<[DefId]>),
+}
+
+/// Tests whether a slice of roots contains a given DefId.
+#[inline]
+fn slice_contains(tcx: TyCtxt<'tcx>, slice: &[DefId], id: DefId) -> bool {
+    slice.iter().any(|root_id| tcx.is_descendant_of(id, *root_id))
 }
 
 impl<'tcx> DefIdForest {
     /// Creates an empty forest.
     pub fn empty() -> DefIdForest {
-        DefIdForest { root_ids: SmallVec::new() }
+        DefIdForest::Empty
     }
 
     /// Creates a forest consisting of a single tree representing the entire
     /// crate.
     #[inline]
     pub fn full(tcx: TyCtxt<'tcx>) -> DefIdForest {
-        let crate_id = tcx.hir().local_def_id(CRATE_HIR_ID);
-        DefIdForest::from_id(crate_id.to_def_id())
+        DefIdForest::from_id(tcx.hir().local_def_id(CRATE_HIR_ID).to_def_id())
     }
 
     /// Creates a forest containing a `DefId` and all its descendants.
     pub fn from_id(id: DefId) -> DefIdForest {
-        let mut root_ids = SmallVec::new();
-        root_ids.push(id);
-        DefIdForest { root_ids }
+        DefIdForest::Single(id)
+    }
+
+    fn as_slice(&self) -> &[DefId] {
+        match self {
+            Empty => &[],
+            Single(id) => std::slice::from_ref(id),
+            Multiple(root_ids) => root_ids,
+        }
+    }
+
+    // Only allocates in the rare `Multiple` case.
+    fn from_slice(root_ids: &[DefId]) -> DefIdForest {
+        match root_ids {
+            [] => Empty,
+            [id] => Single(*id),
+            _ => DefIdForest::Multiple(root_ids.into()),
+        }
     }
 
     /// Tests whether the forest is empty.
     pub fn is_empty(&self) -> bool {
-        self.root_ids.is_empty()
+        match self {
+            Empty => true,
+            Single(..) | Multiple(..) => false,
+        }
+    }
+
+    /// Iterate over the set of roots.
+    fn iter(&self) -> impl Iterator<Item = DefId> + '_ {
+        self.as_slice().iter().copied()
     }
 
     /// Tests whether the forest contains a given DefId.
     pub fn contains(&self, tcx: TyCtxt<'tcx>, id: DefId) -> bool {
-        self.root_ids.iter().any(|root_id| tcx.is_descendant_of(id, *root_id))
+        slice_contains(tcx, self.as_slice(), id)
     }
 
     /// Calculate the intersection of a collection of forests.
@@ -58,56 +93,55 @@ impl<'tcx> DefIdForest {
         I: IntoIterator<Item = DefIdForest>,
     {
         let mut iter = iter.into_iter();
-        let mut ret = if let Some(first) = iter.next() {
-            first
+        let mut ret: SmallVec<[_; 1]> = if let Some(first) = iter.next() {
+            SmallVec::from_slice(first.as_slice())
         } else {
             return DefIdForest::full(tcx);
         };
 
-        let mut next_ret = SmallVec::new();
-        let mut old_ret: SmallVec<[DefId; 1]> = SmallVec::new();
+        let mut next_ret: SmallVec<[_; 1]> = SmallVec::new();
         for next_forest in iter {
             // No need to continue if the intersection is already empty.
-            if ret.is_empty() {
-                break;
+            if ret.is_empty() || next_forest.is_empty() {
+                return DefIdForest::empty();
             }
 
-            for id in ret.root_ids.drain(..) {
-                if next_forest.contains(tcx, id) {
-                    next_ret.push(id);
-                } else {
-                    old_ret.push(id);
-                }
-            }
-            ret.root_ids.extend(old_ret.drain(..));
+            // We keep the elements in `ret` that are also in `next_forest`.
+            next_ret.extend(ret.iter().copied().filter(|&id| next_forest.contains(tcx, id)));
+            // We keep the elements in `next_forest` that are also in `ret`.
+            next_ret.extend(next_forest.iter().filter(|&id| slice_contains(tcx, &ret, id)));
 
-            next_ret.extend(next_forest.root_ids.into_iter().filter(|&id| ret.contains(tcx, id)));
-
-            mem::swap(&mut next_ret, &mut ret.root_ids);
-            next_ret.drain(..);
+            mem::swap(&mut next_ret, &mut ret);
+            next_ret.clear();
         }
-        ret
+        DefIdForest::from_slice(&ret)
     }
 
     /// Calculate the union of a collection of forests.
     pub fn union<I>(tcx: TyCtxt<'tcx>, iter: I) -> DefIdForest
     where
         I: IntoIterator<Item = DefIdForest>,
     {
-        let mut ret = DefIdForest::empty();
-        let mut next_ret = SmallVec::new();
+        let mut ret: SmallVec<[_; 1]> = SmallVec::new();
+        let mut next_ret: SmallVec<[_; 1]> = SmallVec::new();
         for next_forest in iter {
-            next_ret.extend(ret.root_ids.drain(..).filter(|&id| !next_forest.contains(tcx, id)));
+            // Union with the empty set is a no-op.
+            if next_forest.is_empty() {
+                continue;
+            }
 
-            for id in next_forest.root_ids {
-                if !next_ret.contains(&id) {
+            // We add everything in `ret` that is not in `next_forest`.
+            next_ret.extend(ret.iter().copied().filter(|&id| !next_forest.contains(tcx, id)));
+            // We add everything in `next_forest` that we haven't added yet.
+            for id in next_forest.iter() {
+                if !slice_contains(tcx, &next_ret, id) {
                     next_ret.push(id);
                 }
             }
 
-            mem::swap(&mut next_ret, &mut ret.root_ids);
-            next_ret.drain(..);
+            mem::swap(&mut next_ret, &mut ret);
+            next_ret.clear();
         }
-        ret
+        DefIdForest::from_slice(&ret)
     }
 }

compiler/rustc_middle/src/ty/inhabitedness/mod.rs

@@ -6,7 +6,6 @@ use crate::ty::TyKind::*;
 use crate::ty::{AdtDef, FieldDef, Ty, TyS, VariantDef};
 use crate::ty::{AdtKind, Visibility};
 use crate::ty::{DefId, SubstsRef};
-use rustc_data_structures::stack::ensure_sufficient_stack;
 
 mod def_id_forest;
 
@@ -187,34 +186,46 @@ impl<'tcx> FieldDef {
 
 impl<'tcx> TyS<'tcx> {
     /// Calculates the forest of `DefId`s from which this type is visibly uninhabited.
-    fn uninhabited_from(&self, tcx: TyCtxt<'tcx>, param_env: ty::ParamEnv<'tcx>) -> DefIdForest {
-        match *self.kind() {
-            Adt(def, substs) => {
-                ensure_sufficient_stack(|| def.uninhabited_from(tcx, substs, param_env))
-            }
+    fn uninhabited_from(
+        &'tcx self,
+        tcx: TyCtxt<'tcx>,
+        param_env: ty::ParamEnv<'tcx>,
+    ) -> DefIdForest {
+        tcx.type_uninhabited_from(param_env.and(self))
+    }
+}
 
-            Never => DefIdForest::full(tcx),
+// Query provider for `type_uninhabited_from`.
+pub(crate) fn type_uninhabited_from<'tcx>(
+    tcx: TyCtxt<'tcx>,
+    key: ty::ParamEnvAnd<'tcx, Ty<'tcx>>,
+) -> DefIdForest {
+    let ty = key.value;
+    let param_env = key.param_env;
+    match *ty.kind() {
+        Adt(def, substs) => def.uninhabited_from(tcx, substs, param_env),
 
-            Tuple(ref tys) => DefIdForest::union(
-                tcx,
-                tys.iter().map(|ty| ty.expect_ty().uninhabited_from(tcx, param_env)),
-            ),
+        Never => DefIdForest::full(tcx),
 
-            Array(ty, len) => match len.try_eval_usize(tcx, param_env) {
-                Some(0) | None => DefIdForest::empty(),
-                // If the array is definitely non-empty, it's uninhabited if
-                // the type of its elements is uninhabited.
-                Some(1..) => ty.uninhabited_from(tcx, param_env),
-            },
+        Tuple(ref tys) => DefIdForest::union(
+            tcx,
+            tys.iter().map(|ty| ty.expect_ty().uninhabited_from(tcx, param_env)),
+        ),
 
-            // References to uninitialised memory are valid for any type, including
-            // uninhabited types, in unsafe code, so we treat all references as
-            // inhabited.
-            // The precise semantics of inhabitedness with respect to references is currently
-            // undecided.
-            Ref(..) => DefIdForest::empty(),
+        Array(ty, len) => match len.try_eval_usize(tcx, param_env) {
+            Some(0) | None => DefIdForest::empty(),
+            // If the array is definitely non-empty, it's uninhabited if
+            // the type of its elements is uninhabited.
+            Some(1..) => ty.uninhabited_from(tcx, param_env),
+        },
 
-            _ => DefIdForest::empty(),
-        }
+        // References to uninitialised memory are valid for any type, including
+        // uninhabited types, in unsafe code, so we treat all references as
+        // inhabited.
+        // The precise semantics of inhabitedness with respect to references is currently
+        // undecided.
+        Ref(..) => DefIdForest::empty(),
+
+        _ => DefIdForest::empty(),
     }
 }

compiler/rustc_middle/src/ty/mod.rs

@@ -3155,6 +3155,7 @@ pub fn provide(providers: &mut ty::query::Providers) {
     *providers = ty::query::Providers {
         trait_impls_of: trait_def::trait_impls_of_provider,
         all_local_trait_impls: trait_def::all_local_trait_impls,
+        type_uninhabited_from: inhabitedness::type_uninhabited_from,
         ..*providers
     };
 }

src/test/ui/pattern/usefulness/auxiliary/empty.rs

@@ -1,2 +1,10 @@
 #![crate_type = "rlib"]
 pub enum EmptyForeignEnum {}
+
+pub struct VisiblyUninhabitedForeignStruct {
+    pub field: EmptyForeignEnum,
+}
+
+pub struct SecretlyUninhabitedForeignStruct {
+    _priv: EmptyForeignEnum,
+}