Optimize exhaustiveness checking perf a bit

Author: Veykril

crates/hir-def/src/data/adt.rs

@@ -26,7 +26,8 @@ use crate::{
     tt::{Delimiter, DelimiterKind, Leaf, Subtree, TokenTree},
     type_ref::TypeRef,
     visibility::RawVisibility,
-    EnumId, EnumVariantId, LocalFieldId, LocalModuleId, Lookup, StructId, UnionId,
+    AdtId, EnumId, EnumVariantId, LocalFieldId, LocalModuleId, Lookup, StructId, UnionId,
+    VariantId,
 };
 
 /// Note that we use `StructData` for unions as well!
@@ -378,6 +379,14 @@ impl VariantData {
             VariantData::Unit => StructKind::Unit,
         }
     }
+
+    pub(crate) fn variant_data(db: &dyn DefDatabase, id: VariantId) -> Arc<VariantData> {
+        match id {
+            VariantId::StructId(it) => db.struct_data(it).variant_data.clone(),
+            VariantId::EnumVariantId(it) => db.enum_variant_data(it).variant_data.clone(),
+            VariantId::UnionId(it) => db.union_data(it).variant_data.clone(),
+        }
+    }
 }
 
 #[derive(Debug, Copy, Clone, PartialEq, Eq)]

crates/hir-def/src/db.rs

@@ -12,7 +12,7 @@ use crate::{
     attr::{Attrs, AttrsWithOwner},
     body::{scope::ExprScopes, Body, BodySourceMap},
     data::{
-        adt::{EnumData, EnumVariantData, StructData},
+        adt::{EnumData, EnumVariantData, StructData, VariantData},
         ConstData, ExternCrateDeclData, FunctionData, ImplData, Macro2Data, MacroRulesData,
         ProcMacroData, StaticData, TraitAliasData, TraitData, TypeAliasData,
     },
@@ -22,13 +22,13 @@ use crate::{
     lang_item::{self, LangItem, LangItemTarget, LangItems},
     nameres::{diagnostics::DefDiagnostics, DefMap},
     visibility::{self, Visibility},
-    AttrDefId, BlockId, BlockLoc, ConstBlockId, ConstBlockLoc, ConstId, ConstLoc, DefWithBodyId,
-    EnumId, EnumLoc, EnumVariantId, EnumVariantLoc, ExternBlockId, ExternBlockLoc, ExternCrateId,
-    ExternCrateLoc, FunctionId, FunctionLoc, GenericDefId, ImplId, ImplLoc, InTypeConstId,
-    InTypeConstLoc, LocalFieldId, Macro2Id, Macro2Loc, MacroId, MacroRulesId, MacroRulesLoc,
-    MacroRulesLocFlags, ProcMacroId, ProcMacroLoc, StaticId, StaticLoc, StructId, StructLoc,
-    TraitAliasId, TraitAliasLoc, TraitId, TraitLoc, TypeAliasId, TypeAliasLoc, UnionId, UnionLoc,
-    UseId, UseLoc, VariantId,
+    AdtId, AttrDefId, BlockId, BlockLoc, ConstBlockId, ConstBlockLoc, ConstId, ConstLoc,
+    DefWithBodyId, EnumId, EnumLoc, EnumVariantId, EnumVariantLoc, ExternBlockId, ExternBlockLoc,
+    ExternCrateId, ExternCrateLoc, FunctionId, FunctionLoc, GenericDefId, ImplId, ImplLoc,
+    InTypeConstId, InTypeConstLoc, LocalFieldId, Macro2Id, Macro2Loc, MacroId, MacroRulesId,
+    MacroRulesLoc, MacroRulesLocFlags, ProcMacroId, ProcMacroLoc, StaticId, StaticLoc, StructId,
+    StructLoc, TraitAliasId, TraitAliasLoc, TraitId, TraitLoc, TypeAliasId, TypeAliasLoc, UnionId,
+    UnionLoc, UseId, UseLoc, VariantId,
 };
 
 #[salsa::query_group(InternDatabaseStorage)]
@@ -127,6 +127,9 @@ pub trait DefDatabase: InternDatabase + ExpandDatabase + Upcast<dyn ExpandDataba
         id: EnumVariantId,
     ) -> (Arc<EnumVariantData>, DefDiagnostics);
 
+    #[salsa::transparent]
+    #[salsa::invoke(VariantData::variant_data)]
+    fn variant_data(&self, id: VariantId) -> Arc<VariantData>;
     #[salsa::transparent]
     #[salsa::invoke(ImplData::impl_data_query)]
     fn impl_data(&self, e: ImplId) -> Arc<ImplData>;

crates/hir-ty/src/diagnostics/expr.rs

@@ -11,6 +11,7 @@ use hir_def::{ItemContainerId, Lookup};
 use hir_expand::name;
 use itertools::Itertools;
 use rustc_hash::FxHashSet;
+use rustc_pattern_analysis::constructor::Constructor;
 use syntax::{ast, AstNode};
 use tracing::debug;
 use triomphe::Arc;
@@ -266,15 +267,17 @@ impl ExprValidator {
 
             let mut have_errors = false;
             let deconstructed_pat = self.lower_pattern(&cx, pat, db, &mut have_errors);
+
+            // optimization, wildcard trivially hold
+            if have_errors || matches!(deconstructed_pat.ctor(), Constructor::Wildcard) {
+                continue;
+            }
+
             let match_arm = rustc_pattern_analysis::MatchArm {
                 pat: pattern_arena.alloc(deconstructed_pat),
                 has_guard: false,
                 arm_data: (),
             };
-            if have_errors {
-                continue;
-            }
-
             let report = match cx.compute_match_usefulness(&[match_arm], ty.clone()) {
                 Ok(v) => v,
                 Err(e) => {

crates/hir-ty/src/diagnostics/match_check/pat_analysis.rs

@@ -1,9 +1,9 @@
 //! Interface with `rustc_pattern_analysis`.
 
 use std::fmt;
-use tracing::debug;
 
 use hir_def::{DefWithBodyId, EnumId, EnumVariantId, HasModule, LocalFieldId, ModuleId, VariantId};
+use once_cell::unsync::Lazy;
 use rustc_hash::FxHashMap;
 use rustc_pattern_analysis::{
     constructor::{Constructor, ConstructorSet, VariantVisibility},
@@ -91,20 +91,13 @@ impl<'p> MatchCheckCtx<'p> {
     }
 
     fn is_uninhabited(&self, ty: &Ty) -> bool {
-        is_ty_uninhabited_from(ty, self.module, self.db)
+        is_ty_uninhabited_from(self.db, ty, self.module)
     }
 
-    /// Returns whether the given type is an enum from another crate declared `#[non_exhaustive]`.
-    fn is_foreign_non_exhaustive_enum(&self, ty: &Ty) -> bool {
-        match ty.as_adt() {
-            Some((adt @ hir_def::AdtId::EnumId(_), _)) => {
-                let has_non_exhaustive_attr =
-                    self.db.attrs(adt.into()).by_key("non_exhaustive").exists();
-                let is_local = adt.module(self.db.upcast()).krate() == self.module.krate();
-                has_non_exhaustive_attr && !is_local
-            }
-            _ => false,
-        }
+    /// Returns whether the given ADT is from another crate declared `#[non_exhaustive]`.
+    fn is_foreign_non_exhaustive(&self, adt: hir_def::AdtId) -> bool {
+        let is_local = adt.krate(self.db.upcast()) == self.module.krate();
+        !is_local && self.db.attrs(adt.into()).by_key("non_exhaustive").exists()
     }
 
     fn variant_id_for_adt(
@@ -376,24 +369,21 @@ impl<'p> PatCx for MatchCheckCtx<'p> {
                         single(subst_ty)
                     } else {
                         let variant = Self::variant_id_for_adt(self.db, ctor, adt).unwrap();
-                        let (adt, _) = ty.as_adt().unwrap();
 
-                        let adt_is_local =
-                            variant.module(self.db.upcast()).krate() == self.module.krate();
                         // Whether we must not match the fields of this variant exhaustively.
-                        let is_non_exhaustive =
-                            self.db.attrs(variant.into()).by_key("non_exhaustive").exists()
-                                && !adt_is_local;
-                        let visibilities = self.db.field_visibilities(variant);
+                        let is_non_exhaustive = Lazy::new(|| self.is_foreign_non_exhaustive(adt));
+                        let visibilities = Lazy::new(|| self.db.field_visibilities(variant));
 
                         self.list_variant_fields(ty, variant)
                             .map(move |(fid, ty)| {
-                                let is_visible = matches!(adt, hir_def::AdtId::EnumId(..))
-                                    || visibilities[fid]
-                                        .is_visible_from(self.db.upcast(), self.module);
+                                let is_visible = || {
+                                    matches!(adt, hir_def::AdtId::EnumId(..))
+                                        || visibilities[fid]
+                                            .is_visible_from(self.db.upcast(), self.module)
+                                };
                                 let is_uninhabited = self.is_uninhabited(&ty);
                                 let private_uninhabited =
-                                    is_uninhabited && (!is_visible || is_non_exhaustive);
+                                    is_uninhabited && (!is_visible() || *is_non_exhaustive);
                                 (ty, PrivateUninhabitedField(private_uninhabited))
                             })
                             .collect()
@@ -445,17 +435,20 @@ impl<'p> PatCx for MatchCheckCtx<'p> {
             TyKind::Scalar(Scalar::Char) => unhandled(),
             TyKind::Scalar(Scalar::Int(..) | Scalar::Uint(..)) => unhandled(),
             TyKind::Array(..) | TyKind::Slice(..) => unhandled(),
-            TyKind::Adt(AdtId(hir_def::AdtId::EnumId(enum_id)), subst) => {
-                let enum_data = cx.db.enum_data(*enum_id);
-                let is_declared_nonexhaustive = cx.is_foreign_non_exhaustive_enum(ty);
+            &TyKind::Adt(AdtId(adt @ hir_def::AdtId::EnumId(enum_id)), ref subst) => {
+                let enum_data = cx.db.enum_data(enum_id);
+                let is_declared_nonexhaustive = cx.is_foreign_non_exhaustive(adt);
 
                 if enum_data.variants.is_empty() && !is_declared_nonexhaustive {
                     ConstructorSet::NoConstructors
                 } else {
-                    let mut variants = FxHashMap::default();
+                    let mut variants = FxHashMap::with_capacity_and_hasher(
+                        enum_data.variants.len(),
+                        Default::default(),
+                    );
                     for (i, &(variant, _)) in enum_data.variants.iter().enumerate() {
                         let is_uninhabited =
-                            is_enum_variant_uninhabited_from(variant, subst, cx.module, cx.db);
+                            is_enum_variant_uninhabited_from(cx.db, variant, subst, cx.module);
                         let visibility = if is_uninhabited {
                             VariantVisibility::Empty
                         } else {
@@ -506,7 +499,7 @@ impl<'p> PatCx for MatchCheckCtx<'p> {
     }
 
     fn bug(&self, fmt: fmt::Arguments<'_>) {
-        debug!("{}", fmt)
+        never!("{}", fmt)
     }
 
     fn complexity_exceeded(&self) -> Result<(), Self::Error> {

crates/hir-ty/src/inhabitedness.rs

@@ -5,42 +5,36 @@ use chalk_ir::{
     visit::{TypeSuperVisitable, TypeVisitable, TypeVisitor},
     DebruijnIndex,
 };
-use hir_def::{
-    attr::Attrs, data::adt::VariantData, visibility::Visibility, AdtId, EnumVariantId, HasModule,
-    ModuleId, VariantId,
-};
+use hir_def::{visibility::Visibility, AdtId, EnumVariantId, HasModule, ModuleId, VariantId};
 use rustc_hash::FxHashSet;
 
 use crate::{
     consteval::try_const_usize, db::HirDatabase, Binders, Interner, Substitution, Ty, TyKind,
 };
 
+// FIXME: Turn this into a query, it can be quite slow
 /// Checks whether a type is visibly uninhabited from a particular module.
-pub(crate) fn is_ty_uninhabited_from(ty: &Ty, target_mod: ModuleId, db: &dyn HirDatabase) -> bool {
+pub(crate) fn is_ty_uninhabited_from(db: &dyn HirDatabase, ty: &Ty, target_mod: ModuleId) -> bool {
+    let _p = tracing::span!(tracing::Level::INFO, "is_ty_uninhabited_from", ?ty);
     let mut uninhabited_from =
         UninhabitedFrom { target_mod, db, max_depth: 500, recursive_ty: FxHashSet::default() };
     let inhabitedness = ty.visit_with(&mut uninhabited_from, DebruijnIndex::INNERMOST);
     inhabitedness == BREAK_VISIBLY_UNINHABITED
 }
 
+// FIXME: Turn this into a query, it can be quite slow
 /// Checks whether a variant is visibly uninhabited from a particular module.
 pub(crate) fn is_enum_variant_uninhabited_from(
+    db: &dyn HirDatabase,
     variant: EnumVariantId,
     subst: &Substitution,
     target_mod: ModuleId,
-    db: &dyn HirDatabase,
 ) -> bool {
-    let is_local = variant.module(db.upcast()).krate() == target_mod.krate();
+    let _p = tracing::span!(tracing::Level::INFO, "is_enum_variant_uninhabited_from",);
 
     let mut uninhabited_from =
         UninhabitedFrom { target_mod, db, max_depth: 500, recursive_ty: FxHashSet::default() };
-    let inhabitedness = uninhabited_from.visit_variant(
-        variant.into(),
-        &db.enum_variant_data(variant).variant_data,
-        subst,
-        &db.attrs(variant.into()),
-        is_local,
-    );
+    let inhabitedness = uninhabited_from.visit_variant(variant.into(), subst);
     inhabitedness == BREAK_VISIBLY_UNINHABITED
 }
 
@@ -98,34 +92,18 @@ impl TypeVisitor<Interner> for UninhabitedFrom<'_> {
 
 impl UninhabitedFrom<'_> {
     fn visit_adt(&mut self, adt: AdtId, subst: &Substitution) -> ControlFlow<VisiblyUninhabited> {
-        let attrs = self.db.attrs(adt.into());
-        let adt_non_exhaustive = attrs.by_key("non_exhaustive").exists();
-        let is_local = adt.module(self.db.upcast()).krate() == self.target_mod.krate();
-        if adt_non_exhaustive && !is_local {
-            return CONTINUE_OPAQUELY_INHABITED;
-        }
-
         // An ADT is uninhabited iff all its variants uninhabited.
         match adt {
             // rustc: For now, `union`s are never considered uninhabited.
             AdtId::UnionId(_) => CONTINUE_OPAQUELY_INHABITED,
-            AdtId::StructId(s) => {
-                let struct_data = self.db.struct_data(s);
-                self.visit_variant(s.into(), &struct_data.variant_data, subst, &attrs, is_local)
-            }
+            AdtId::StructId(s) => self.visit_variant(s.into(), subst),
             AdtId::EnumId(e) => {
                 let enum_data = self.db.enum_data(e);
 
                 for &(variant, _) in enum_data.variants.iter() {
-                    let variant_inhabitedness = self.visit_variant(
-                        variant.into(),
-                        &self.db.enum_variant_data(variant).variant_data,
-                        subst,
-                        &self.db.attrs(variant.into()),
-                        is_local,
-                    );
+                    let variant_inhabitedness = self.visit_variant(variant.into(), subst);
                     match variant_inhabitedness {
-                        Break(VisiblyUninhabited) => continue,
+                        Break(VisiblyUninhabited) => (),
                         Continue(()) => return CONTINUE_OPAQUELY_INHABITED,
                     }
                 }
@@ -137,34 +115,36 @@ impl UninhabitedFrom<'_> {
     fn visit_variant(
         &mut self,
         variant: VariantId,
-        variant_data: &VariantData,
         subst: &Substitution,
-        attrs: &Attrs,
-        is_local: bool,
     ) -> ControlFlow<VisiblyUninhabited> {
-        let non_exhaustive_field_list = attrs.by_key("non_exhaustive").exists();
-        if non_exhaustive_field_list && !is_local {
+        let is_local = variant.krate(self.db.upcast()) == self.target_mod.krate();
+        if !is_local && self.db.attrs(variant.into()).by_key("non_exhaustive").exists() {
+            return CONTINUE_OPAQUELY_INHABITED;
+        }
+
+        let variant_data = self.db.variant_data(variant);
+        let fields = variant_data.fields();
+        if fields.is_empty() {
             return CONTINUE_OPAQUELY_INHABITED;
         }
 
         let is_enum = matches!(variant, VariantId::EnumVariantId(..));
         let field_tys = self.db.field_types(variant);
-        let field_vis = self.db.field_visibilities(variant);
+        let field_vis = if is_enum { None } else { Some(self.db.field_visibilities(variant)) };
 
-        for (fid, _) in variant_data.fields().iter() {
-            self.visit_field(field_vis[fid], &field_tys[fid], subst, is_enum)?;
+        for (fid, _) in fields.iter() {
+            self.visit_field(field_vis.as_ref().map(|it| it[fid]), &field_tys[fid], subst)?;
         }
         CONTINUE_OPAQUELY_INHABITED
     }
 
     fn visit_field(
         &mut self,
-        vis: Visibility,
+        vis: Option<Visibility>,
         ty: &Binders<Ty>,
         subst: &Substitution,
-        is_enum: bool,
     ) -> ControlFlow<VisiblyUninhabited> {
-        if is_enum || vis.is_visible_from(self.db.upcast(), self.target_mod) {
+        if vis.map_or(true, |it| it.is_visible_from(self.db.upcast(), self.target_mod)) {
             let ty = ty.clone().substitute(Interner, subst);
             ty.visit_with(self, DebruijnIndex::INNERMOST)
         } else {

crates/hir-ty/src/mir/lower.rs

@@ -1706,7 +1706,7 @@ impl<'ctx> MirLowerCtx<'ctx> {
     }
 
     fn is_uninhabited(&self, expr_id: ExprId) -> bool {
-        is_ty_uninhabited_from(&self.infer[expr_id], self.owner.module(self.db.upcast()), self.db)
+        is_ty_uninhabited_from(self.db, &self.infer[expr_id], self.owner.module(self.db.upcast()))
     }
 
     /// This function push `StorageLive` statement for the binding, and applies changes to add `StorageDead` and

crates/rust-analyzer/src/integrated_benchmarks.rs

@@ -300,7 +300,7 @@ fn integrated_diagnostics_benchmark() {
         .diagnostics(&diagnostics_config, ide::AssistResolveStrategy::None, file_id)
         .unwrap();
 
-    let _g = crate::tracing::hprof::init("*>1");
+    let _g = crate::tracing::hprof::init("*");
 
     {
         let _it = stdx::timeit("change");