Auto merge of #17078 - Veykril:diags-perf, r=Veykril

internal: Improve diagnostics performance

Author: bors

crates/base-db/src/lib.rs

@@ -45,7 +45,7 @@ pub trait Upcast<T: ?Sized> {
 
 pubconstDEFAULT_FILE_TEXT_LRU_CAP:usize = 16;
 pubconstDEFAULT_PARSE_LRU_CAP:usize = 128;
-pubconstDEFAULT_BORROWCK_LRU_CAP:usize = 1024;
+pubconstDEFAULT_BORROWCK_LRU_CAP:usize = 2024;
 
 pubtraitFileLoader{
 /// Text of the file.

crates/hir-def/src/data.rs

@@ -510,6 +510,7 @@ pub struct ConstData {
 pubtype_ref:Interned<TypeRef>,
 pubvisibility:RawVisibility,
 pubrustc_allow_incoherent_impl:bool,
+pubhas_body:bool,
 }
 
 implConstData{
@@ -533,6 +534,7 @@ impl ConstData {
 type_ref: konst.type_ref.clone(),
  visibility,
  rustc_allow_incoherent_impl,
+has_body: konst.has_body,
 })
 }
 }

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

@@ -26,7 +26,7 @@ use crate::{
  tt::{Delimiter,DelimiterKind,Leaf,Subtree,TokenTree},
  type_ref::TypeRef,
  visibility::RawVisibility,
-EnumId,EnumVariantId,LocalFieldId,LocalModuleId,Lookup,StructId,UnionId,
+EnumId,EnumVariantId,LocalFieldId,LocalModuleId,Lookup,StructId,UnionId,VariantId,
 };
 
 /// Note that we use `StructData` for unions as well!
@@ -378,6 +378,15 @@ impl VariantData {
 VariantData::Unit => StructKind::Unit,
 }
 }
+
+#[allow(clippy::self_named_constructors)]
+pub(crate)fnvariant_data(db:&dynDefDatabase,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,
 },
@@ -127,6 +127,9 @@ pub trait DefDatabase: InternDatabase + ExpandDatabase + Upcast<dyn ExpandDataba
 id:EnumVariantId,
 ) -> (Arc<EnumVariantData>,DefDiagnostics);
 
+#[salsa::transparent]
+#[salsa::invoke(VariantData::variant_data)]
+fnvariant_data(&self,id:VariantId) -> Arc<VariantData>;
 #[salsa::transparent]
 #[salsa::invoke(ImplData::impl_data_query)]
 fnimpl_data(&self,e:ImplId) -> Arc<ImplData>;

crates/hir-def/src/item_tree.rs

@@ -716,6 +716,7 @@ pub struct Const {
 pubvisibility:RawVisibilityId,
 pubtype_ref:Interned<TypeRef>,
 pubast_id:FileAstId<ast::Const>,
+pubhas_body:bool,
 }
 
 #[derive(Debug,Clone,Eq,PartialEq)]

crates/hir-def/src/item_tree/lower.rs

@@ -446,7 +446,7 @@ impl<'a> Ctx<'a> {
 let type_ref = self.lower_type_ref_opt(konst.ty());
 let visibility = self.lower_visibility(konst);
 let ast_id = self.source_ast_id_map.ast_id(konst);
-let res = Const{ name, visibility, type_ref, ast_id };
+let res = Const{ name, visibility, type_ref, ast_id,has_body: konst.body().is_some()};
 id(self.data().consts.alloc(res))
 }
 

crates/hir-def/src/item_tree/pretty.rs

@@ -357,7 +357,7 @@ impl Printer<'_> {
 wln!(self,"}}");
 }
 ModItem::Const(it) => {
-letConst{ name, visibility, type_ref, ast_id } = &self.tree[it];
+letConst{ name, visibility, type_ref, ast_id,has_body: _} = &self.tree[it];
 self.print_ast_id(ast_id.erase());
 self.print_visibility(*visibility);
 w!(self,"const ");

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

@@ -43,7 +43,7 @@ mod allow {
 }
 
 pubfnincorrect_case(db:&dynHirDatabase,owner:ModuleDefId) -> Vec<IncorrectCase>{
-let _p = tracing::span!(tracing::Level::INFO,"validate_module_item").entered();
+let _p = tracing::span!(tracing::Level::INFO,"incorrect_case").entered();
 letmut validator = DeclValidator::new(db);
  validator.validate_item(owner);
  validator.sink

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;
@@ -190,45 +191,45 @@ impl ExprValidator {
 let pattern_arena = Arena::new();
 letmut m_arms = Vec::with_capacity(arms.len());
 letmut has_lowering_errors = false;
+// Note: Skipping the entire diagnostic rather than just not including a faulty match arm is
+// preferred to avoid the chance of false positives.
 for arm in arms {
-ifletSome(pat_ty) = self.infer.type_of_pat.get(arm.pat){
-// We only include patterns whose type matches the type
-// of the scrutinee expression. If we had an InvalidMatchArmPattern
-// diagnostic or similar we could raise that in an else
-// block here.
-//
-// When comparing the types, we also have to consider that rustc
-// will automatically de-reference the scrutinee expression type if
-// necessary.
-//
-// FIXME we should use the type checker for this.
-if(pat_ty == scrut_ty
- || scrut_ty
-.as_reference()
-.map(|(match_expr_ty, ..)| match_expr_ty == pat_ty)
-.unwrap_or(false))
- && types_of_subpatterns_do_match(arm.pat,&self.body,&self.infer)
-{
-// If we had a NotUsefulMatchArm diagnostic, we could
-// check the usefulness of each pattern as we added it
-// to the matrix here.
-let pat = self.lower_pattern(&cx, arm.pat, db,&mut has_lowering_errors);
-let m_arm = pat_analysis::MatchArm{
-pat: pattern_arena.alloc(pat),
-has_guard: arm.guard.is_some(),
-arm_data:(),
-};
- m_arms.push(m_arm);
-if !has_lowering_errors {
-continue;
-}
+letSome(pat_ty) = self.infer.type_of_pat.get(arm.pat)else{
+return;
+};
+
+// We only include patterns whose type matches the type
+// of the scrutinee expression. If we had an InvalidMatchArmPattern
+// diagnostic or similar we could raise that in an else
+// block here.
+//
+// When comparing the types, we also have to consider that rustc
+// will automatically de-reference the scrutinee expression type if
+// necessary.
+//
+// FIXME we should use the type checker for this.
+if(pat_ty == scrut_ty
+ || scrut_ty
+.as_reference()
+.map(|(match_expr_ty, ..)| match_expr_ty == pat_ty)
+.unwrap_or(false))
+ && types_of_subpatterns_do_match(arm.pat,&self.body,&self.infer)
+{
+// If we had a NotUsefulMatchArm diagnostic, we could
+// check the usefulness of each pattern as we added it
+// to the matrix here.
+let pat = self.lower_pattern(&cx, arm.pat, db,&mut has_lowering_errors);
+let m_arm = pat_analysis::MatchArm{
+pat: pattern_arena.alloc(pat),
+has_guard: arm.guard.is_some(),
+arm_data:(),
+};
+ m_arms.push(m_arm);
+if !has_lowering_errors {
+continue;
 }
 }
-
-// If we can't resolve the type of a pattern, or the pattern type doesn't
-// fit the match expression, we skip this diagnostic. Skipping the entire
-// diagnostic rather than just not including this match arm is preferred
-// to avoid the chance of false positives.
+// If the pattern type doesn't fit the match expression, we skip this diagnostic.
  cov_mark::hit!(validate_match_bailed_out);
 return;
 }
@@ -266,15 +267,17 @@ impl ExprValidator {
 
 letmut 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) => {
@@ -531,8 +534,16 @@ fn types_of_subpatterns_do_match(pat: PatId, body: &Body, infer: &InferenceResul
 fnwalk(pat:PatId,body:&Body,infer:&InferenceResult,has_type_mismatches:&mutbool){
 match infer.type_mismatch_for_pat(pat){
 Some(_) => *has_type_mismatches = true,
+Noneif*has_type_mismatches => (),
 None => {
- body[pat].walk_child_pats(|subpat| walk(subpat, body, infer, has_type_mismatches))
+let pat = &body[pat];
+ifletPat::ConstBlock(expr) | Pat::Lit(expr) = *pat {
+*has_type_mismatches |= infer.type_mismatch_for_expr(expr).is_some();
+if*has_type_mismatches {
+return;
+}
+}
+ pat.walk_child_pats(|subpat| walk(subpat, body, infer, has_type_mismatches))
 }
 }
 }

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> {
 }
 
 fnis_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]`.
-fnis_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]`.
+fnis_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()
 }
 
 fnvariant_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{
-letmut variants = FxHashMap::default();
+letmut 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> {
 }
 
 fnbug(&self,fmt: fmt::Arguments<'_>){
-debug!("{}", fmt)
+never!("{}", fmt)
 }
 
 fncomplexity_exceeded(&self) -> Result<(),Self::Error>{