Generally optimize diagnostics performance

Veykril · Veykril · commit 531a270d9182 · 2024-04-15T22:15:41.000+02:00
diff --git a/crates/base-db/src/lib.rs b/crates/base-db/src/lib.rs
@@ -45,7 +45,7 @@ pub trait Upcast<T: ?Sized> {
 
 pub const DEFAULT_FILE_TEXT_LRU_CAP: usize = 16;
 pub const DEFAULT_PARSE_LRU_CAP: usize = 128;
-pub const DEFAULT_BORROWCK_LRU_CAP: usize = 1024;
+pub const DEFAULT_BORROWCK_LRU_CAP: usize = 2024;
 
 pub trait FileLoader {
     /// Text of the file.
diff --git a/crates/hir-def/src/data.rs b/crates/hir-def/src/data.rs
@@ -510,6 +510,7 @@ pub struct ConstData {
     pub type_ref: Interned<TypeRef>,
     pub visibility: RawVisibility,
     pub rustc_allow_incoherent_impl: bool,
+    pub has_body: bool,
 }
 
 impl ConstData {
@@ -533,6 +534,7 @@ impl ConstData {
             type_ref: konst.type_ref.clone(),
             visibility,
             rustc_allow_incoherent_impl,
+            has_body: konst.has_body,
         })
     }
 }
diff --git a/crates/hir-def/src/data/adt.rs b/crates/hir-def/src/data/adt.rs
@@ -26,8 +26,7 @@ use crate::{
     tt::{Delimiter, DelimiterKind, Leaf, Subtree, TokenTree},
     type_ref::TypeRef,
     visibility::RawVisibility,
-    AdtId, EnumId, EnumVariantId, LocalFieldId, LocalModuleId, Lookup, StructId, UnionId,
-    VariantId,
+    EnumId, EnumVariantId, LocalFieldId, LocalModuleId, Lookup, StructId, UnionId, VariantId,
 };
 
 /// Note that we use `StructData` for unions as well!
@@ -380,6 +379,7 @@ impl VariantData {
         }
     }
 
+    #[allow(clippy::self_named_constructors)]
     pub(crate) fn variant_data(db: &dyn DefDatabase, id: VariantId) -> Arc<VariantData> {
         match id {
             VariantId::StructId(it) => db.struct_data(it).variant_data.clone(),
diff --git a/crates/hir-def/src/db.rs b/crates/hir-def/src/db.rs
@@ -22,13 +22,13 @@ use crate::{
     lang_item::{self, LangItem, LangItemTarget, LangItems},
     nameres::{diagnostics::DefDiagnostics, DefMap},
     visibility::{self, Visibility},
-    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,
+    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)]
diff --git a/crates/hir-def/src/item_tree.rs b/crates/hir-def/src/item_tree.rs
@@ -716,6 +716,7 @@ pub struct Const {
     pub visibility: RawVisibilityId,
     pub type_ref: Interned<TypeRef>,
     pub ast_id: FileAstId<ast::Const>,
+    pub has_body: bool,
 }
 
 #[derive(Debug, Clone, Eq, PartialEq)]
diff --git a/crates/hir-def/src/item_tree/lower.rs b/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))
     }
 
diff --git a/crates/hir-def/src/item_tree/pretty.rs b/crates/hir-def/src/item_tree/pretty.rs
@@ -357,7 +357,7 @@ impl Printer<'_> {
                 wln!(self, "}}");
             }
             ModItem::Const(it) => {
-                let Const { name, visibility, type_ref, ast_id } = &self.tree[it];
+                let Const { 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 ");
diff --git a/crates/hir-ty/src/diagnostics/decl_check.rs b/crates/hir-ty/src/diagnostics/decl_check.rs
@@ -43,7 +43,7 @@ mod allow {
 }
 
 pub fn incorrect_case(db: &dyn HirDatabase, 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();
     let mut validator = DeclValidator::new(db);
     validator.validate_item(owner);
     validator.sink
diff --git a/crates/hir-ty/src/diagnostics/expr.rs b/crates/hir-ty/src/diagnostics/expr.rs
@@ -191,45 +191,45 @@ impl ExprValidator {
         let pattern_arena = Arena::new();
         let mut m_arms = Vec::with_capacity(arms.len());
         let mut 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 {
-            if let Some(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;
-                    }
+            let Some(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;
         }
@@ -534,8 +534,16 @@ fn types_of_subpatterns_do_match(pat: PatId, body: &Body, infer: &InferenceResul
     fn walk(pat: PatId, body: &Body, infer: &InferenceResult, has_type_mismatches: &mut bool) {
         match infer.type_mismatch_for_pat(pat) {
             Some(_) => *has_type_mismatches = true,
+            None if *has_type_mismatches => (),
             None => {
-                body[pat].walk_child_pats(|subpat| walk(subpat, body, infer, has_type_mismatches))
+                let pat = &body[pat];
+                if let Pat::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))
             }
         }
     }
diff --git a/crates/hir-ty/src/diagnostics/unsafe_check.rs b/crates/hir-ty/src/diagnostics/unsafe_check.rs
@@ -4,7 +4,7 @@
 use hir_def::{
     body::Body,
     hir::{Expr, ExprId, UnaryOp},
-    resolver::{resolver_for_expr, ResolveValueResult, ValueNs},
+    resolver::{resolver_for_expr, ResolveValueResult, Resolver, ValueNs},
     DefWithBodyId,
 };
 
@@ -13,9 +13,9 @@ use crate::{
 };
 
 pub fn missing_unsafe(db: &dyn HirDatabase, def: DefWithBodyId) -> Vec<ExprId> {
-    let infer = db.infer(def);
-    let mut res = Vec::new();
+    let _p = tracing::span!(tracing::Level::INFO, "missing_unsafe",);
 
+    let mut res = Vec::new();
     let is_unsafe = match def {
         DefWithBodyId::FunctionId(it) => db.function_data(it).has_unsafe_kw(),
         DefWithBodyId::StaticId(_)
@@ -28,6 +28,7 @@ pub fn missing_unsafe(db: &dyn HirDatabase, def: DefWithBodyId) -> Vec<ExprId> {
     }
 
     let body = db.body(def);
+    let infer = db.infer(def);
     unsafe_expressions(db, &infer, def, &body, body.body_expr, &mut |expr| {
         if !expr.inside_unsafe_block {
             res.push(expr.expr);
@@ -51,14 +52,24 @@ pub fn unsafe_expressions(
     current: ExprId,
     unsafe_expr_cb: &mut dyn FnMut(UnsafeExpr),
 ) {
-    walk_unsafe(db, infer, def, body, current, false, unsafe_expr_cb)
+    walk_unsafe(
+        db,
+        infer,
+        body,
+        &mut resolver_for_expr(db.upcast(), def, current),
+        def,
+        current,
+        false,
+        unsafe_expr_cb,
+    )
 }
 
 fn walk_unsafe(
     db: &dyn HirDatabase,
     infer: &InferenceResult,
-    def: DefWithBodyId,
     body: &Body,
+    resolver: &mut Resolver,
+    def: DefWithBodyId,
     current: ExprId,
     inside_unsafe_block: bool,
     unsafe_expr_cb: &mut dyn FnMut(UnsafeExpr),
@@ -73,13 +84,14 @@ fn walk_unsafe(
             }
         }
         Expr::Path(path) => {
-            let resolver = resolver_for_expr(db.upcast(), def, current);
+            let g = resolver.update_to_inner_scope(db.upcast(), def, current);
             let value_or_partial = resolver.resolve_path_in_value_ns(db.upcast(), path);
             if let Some(ResolveValueResult::ValueNs(ValueNs::StaticId(id), _)) = value_or_partial {
                 if db.static_data(id).mutable {
                     unsafe_expr_cb(UnsafeExpr { expr: current, inside_unsafe_block });
                 }
             }
+            resolver.reset_to_guard(g);
         }
         Expr::MethodCall { .. } => {
             if infer
@@ -97,13 +109,13 @@ fn walk_unsafe(
         }
         Expr::Unsafe { .. } => {
             return expr.walk_child_exprs(|child| {
-                walk_unsafe(db, infer, def, body, child, true, unsafe_expr_cb);
+                walk_unsafe(db, infer, body, resolver, def, child, true, unsafe_expr_cb);
             });
         }
         _ => {}
     }
 
     expr.walk_child_exprs(|child| {
-        walk_unsafe(db, infer, def, body, child, inside_unsafe_block, unsafe_expr_cb);
+        walk_unsafe(db, infer, body, resolver, def, child, inside_unsafe_block, unsafe_expr_cb);
     });
 }
diff --git a/crates/hir-ty/src/infer.rs b/crates/hir-ty/src/infer.rs
@@ -432,6 +432,7 @@ pub struct InferenceResult {
     /// Whether there are any type-mismatching errors in the result.
     pub(crate) has_errors: bool,
     /// Interned common types to return references to.
+    // FIXME: Move this into `InferenceContext`
     standard_types: InternedStandardTypes,
     /// Stores the types which were implicitly dereferenced in pattern binding modes.
     pub pat_adjustments: FxHashMap<PatId, Vec<Ty>>,
diff --git a/crates/hir/src/lib.rs b/crates/hir/src/lib.rs
@@ -548,8 +548,7 @@ impl Module {
         acc: &mut Vec<AnyDiagnostic>,
         style_lints: bool,
     ) {
-        let name = self.name(db);
-        let _p = tracing::span!(tracing::Level::INFO, "Module::diagnostics", ?name);
+        let _p = tracing::span!(tracing::Level::INFO, "Module::diagnostics", name = ?self.name(db));
         let def_map = self.id.def_map(db.upcast());
         for diag in def_map.diagnostics() {
             if diag.in_module != self.id.local_id {
@@ -684,7 +683,7 @@ impl Module {
                 let items = &db.trait_data(trait_.into()).items;
                 let required_items = items.iter().filter(|&(_, assoc)| match *assoc {
                     AssocItemId::FunctionId(it) => !db.function_data(it).has_body(),
-                    AssocItemId::ConstId(id) => Const::from(id).value(db).is_none(),
+                    AssocItemId::ConstId(id) => !db.const_data(id).has_body,
                     AssocItemId::TypeAliasId(it) => db.type_alias_data(it).type_ref.is_none(),
                 });
                 impl_assoc_items_scratch.extend(db.impl_data(impl_def.id).items.iter().filter_map(
@@ -1628,7 +1627,6 @@ impl DefWithBody {
         acc: &mut Vec<AnyDiagnostic>,
         style_lints: bool,
     ) {
-        db.unwind_if_cancelled();
         let krate = self.module(db).id.krate();
 
         let (body, source_map) = db.body_with_source_map(self.into());
@@ -1762,7 +1760,9 @@ impl DefWithBody {
                         need_mut = &mir::MutabilityReason::Not;
                     }
                     let local = Local { parent: self.into(), binding_id };
-                    match (need_mut, local.is_mut(db)) {
+                    let is_mut = body[binding_id].mode == BindingAnnotation::Mutable;
+
+                    match (need_mut, is_mut) {
                         (mir::MutabilityReason::Unused, _) => {
                             let should_ignore = matches!(body[binding_id].name.as_str(), Some(it) if it.starts_with('_'));
                             if !should_ignore {
diff --git a/crates/ide-diagnostics/src/handlers/missing_match_arms.rs b/crates/ide-diagnostics/src/handlers/missing_match_arms.rs
@@ -317,7 +317,8 @@ fn main() {
     #[test]
     fn mismatched_types_issue_15883() {
         // Check we don't panic.
-        check_diagnostics_no_bails(
+        cov_mark::check!(validate_match_bailed_out);
+        check_diagnostics(
             r#"
 //- minicore: option
 fn main() {
diff --git a/crates/ide-diagnostics/src/lib.rs b/crates/ide-diagnostics/src/lib.rs
diff --git a/crates/rust-analyzer/src/integrated_benchmarks.rs b/crates/rust-analyzer/src/integrated_benchmarks.rs

Original file line number	Diff line number	Diff line change
`@@ -510,6 +510,7 @@ pub struct ConstData {`
`510`	`510`	`pub type_ref: Interned<TypeRef>,`
`511`	`511`	`pub visibility: RawVisibility,`
`512`	`512`	`pub rustc_allow_incoherent_impl: bool,`
	`513`	`+ pub has_body: bool,`
`513`	`514`	`}`
`514`	`515`
`515`	`516`	`impl ConstData {`
`@@ -533,6 +534,7 @@ impl ConstData {`
`533`	`534`	`type_ref: konst.type_ref.clone(),`
`534`	`535`	`visibility,`
`535`	`536`	`rustc_allow_incoherent_impl,`
	`537`	`+ has_body: konst.has_body,`
`536`	`538`	`})`
`537`	`539`	`}`
`538`	`540`	`}`
Original file line number	Diff line number	Diff line change
`@@ -716,6 +716,7 @@ pub struct Const {`
`716`	`716`	`pub visibility: RawVisibilityId,`
`717`	`717`	`pub type_ref: Interned<TypeRef>,`
`718`	`718`	`pub ast_id: FileAstId<ast::Const>,`
	`719`	`+ pub has_body: bool,`
`719`	`720`	`}`
`720`	`721`
`721`	`722`	`#[derive(Debug, Clone, Eq, PartialEq)]`
Original file line number	Diff line number	Diff line change
`@@ -446,7 +446,7 @@ impl<'a> Ctx<'a> {`
`446`	`446`	`let type_ref = self.lower_type_ref_opt(konst.ty());`
`447`	`447`	`let visibility = self.lower_visibility(konst);`
`448`	`448`	`let ast_id = self.source_ast_id_map.ast_id(konst);`
`449`		`- let res = Const { name, visibility, type_ref, ast_id };`
	`449`	`+ let res = Const { name, visibility, type_ref, ast_id, has_body: konst.body().is_some() };`
`450`	`450`	`id(self.data().consts.alloc(res))`
`451`	`451`	`}`
`452`	`452`
Original file line number	Diff line number	Diff line change
`@@ -357,7 +357,7 @@ impl Printer<'_> {`
`357`	`357`	`wln!(self, "}}");`
`358`	`358`	`}`
`359`	`359`	`ModItem::Const(it) => {`
`360`		`- let Const { name, visibility, type_ref, ast_id } = &self.tree[it];`
	`360`	`+ let Const { name, visibility, type_ref, ast_id, has_body: _ } = &self.tree[it];`
`361`	`361`	`self.print_ast_id(ast_id.erase());`
`362`	`362`	`self.print_visibility(*visibility);`
`363`	`363`	`w!(self, "const ");`
Original file line number	Diff line number	Diff line change
`@@ -43,7 +43,7 @@ mod allow {`
`43`	`43`	`}`
`44`	`44`
`45`	`45`	`pub fn incorrect_case(db: &dyn HirDatabase, owner: ModuleDefId) -> Vec<IncorrectCase> {`
`46`		`- let _p = tracing::span!(tracing::Level::INFO, "validate_module_item").entered();`
	`46`	`+ let _p = tracing::span!(tracing::Level::INFO, "incorrect_case").entered();`
`47`	`47`	`let mut validator = DeclValidator::new(db);`
`48`	`48`	`validator.validate_item(owner);`
`49`	`49`	`validator.sink`