Move enum variant collection from defmap to separate query

Veykril · Veykril · commit e09d309ecaa6 · 2025-04-05T10:21:35.000+02:00
diff --git a/crates/hir-def/src/db.rs b/crates/hir-def/src/db.rs
@@ -6,6 +6,7 @@ use intern::sym;
 use la_arena::ArenaMap;
 use span::{EditionedFileId, MacroCallId};
 use syntax::{AstPtr, ast};
+use thin_vec::ThinVec;
 use triomphe::Arc;
 
 use crate::{
@@ -30,8 +31,8 @@ use crate::{
     },
     signatures::{
         ConstSignature, EnumSignature, EnumVariants, FunctionSignature, ImplSignature,
-        StaticSignature, StructSignature, TraitAliasSignature, TraitSignature, TypeAliasSignature,
-        UnionSignature, VariantFields,
+        InactiveEnumVariantCode, StaticSignature, StructSignature, TraitAliasSignature,
+        TraitSignature, TypeAliasSignature, UnionSignature, VariantFields,
     },
     tt,
     visibility::{self, Visibility},
@@ -141,8 +142,16 @@ pub trait DefDatabase:
         id: VariantId,
     ) -> (Arc<VariantFields>, Arc<ExpressionStoreSourceMap>);
 
+    #[salsa::tracked]
+    fn enum_variants(&self, id: EnumId) -> Arc<EnumVariants> {
+        self.enum_variants_with_diagnostics(id).0
+    }
+
     #[salsa::invoke_actual(EnumVariants::enum_variants_query)]
-    fn enum_variants(&self, id: EnumId) -> Arc<EnumVariants>;
+    fn enum_variants_with_diagnostics(
+        &self,
+        id: EnumId,
+    ) -> (Arc<EnumVariants>, Arc<ThinVec<InactiveEnumVariantCode>>);
 
     #[salsa::transparent]
     #[salsa::invoke_actual(ImplItems::impl_items_query)]
diff --git a/crates/hir-def/src/lang_item.rs b/crates/hir-def/src/lang_item.rs
@@ -140,7 +140,7 @@ impl LangItems {
                     }
                     ModuleDefId::AdtId(AdtId::EnumId(e)) => {
                         lang_items.collect_lang_item(db, e, LangItemTarget::EnumId);
-                        crate_def_map.enum_definitions[&e].iter().for_each(|&id| {
+                        db.enum_variants(e).variants.iter().for_each(|&(id, _)| {
                             lang_items.collect_lang_item(db, id, LangItemTarget::EnumVariant);
                         });
                     }
diff --git a/crates/hir-def/src/nameres.rs b/crates/hir-def/src/nameres.rs
@@ -76,8 +76,8 @@ use triomphe::Arc;
 use tt::TextRange;
 
 use crate::{
-    AstId, BlockId, BlockLoc, CrateRootModuleId, EnumId, EnumVariantId, ExternCrateId, FunctionId,
-    FxIndexMap, LocalModuleId, Lookup, MacroExpander, MacroId, ModuleId, ProcMacroId, UseId,
+    AstId, BlockId, BlockLoc, CrateRootModuleId, ExternCrateId, FunctionId, FxIndexMap,
+    LocalModuleId, Lookup, MacroExpander, MacroId, ModuleId, ProcMacroId, UseId,
     db::DefDatabase,
     item_scope::{BuiltinShadowMode, ItemScope},
     item_tree::{ItemTreeId, Mod, TreeId},
@@ -158,7 +158,6 @@ pub struct DefMap {
     /// this contains all kinds of macro, not just `macro_rules!` macro.
     /// ExternCrateId being None implies it being imported from the general prelude import.
     macro_use_prelude: FxHashMap<Name, (MacroId, Option<ExternCrateId>)>,
-    pub(crate) enum_definitions: FxHashMap<EnumId, Box<[EnumVariantId]>>,
 
     // FIXME: AstId's are fairly unstable
     /// Tracks which custom derives are in scope for an item, to allow resolution of derive helper
@@ -458,7 +457,6 @@ impl DefMap {
             macro_use_prelude: FxHashMap::default(),
             derive_helpers_in_scope: FxHashMap::default(),
             diagnostics: Vec::new(),
-            enum_definitions: FxHashMap::default(),
             data: crate_data,
             macro_def_to_macro_id: FxHashMap::default(),
         }
@@ -474,7 +472,6 @@ impl DefMap {
             krate: _,
             prelude: _,
             data: _,
-            enum_definitions,
             macro_def_to_macro_id,
         } = self;
 
@@ -483,7 +480,6 @@ impl DefMap {
         diagnostics.shrink_to_fit();
         modules.shrink_to_fit();
         derive_helpers_in_scope.shrink_to_fit();
-        enum_definitions.shrink_to_fit();
         for (_, module) in modules.iter_mut() {
             module.children.shrink_to_fit();
             module.scope.shrink_to_fit();
diff --git a/crates/hir-def/src/nameres/collector.rs b/crates/hir-def/src/nameres/collector.rs
@@ -26,12 +26,11 @@ use syntax::ast;
 use triomphe::Arc;
 
 use crate::{
-    AdtId, AstId, AstIdWithPath, ConstLoc, CrateRootModuleId, EnumLoc, EnumVariantLoc,
-    ExternBlockLoc, ExternCrateId, ExternCrateLoc, FunctionId, FunctionLoc, ImplLoc, Intern,
-    ItemContainerId, LocalModuleId, Lookup, Macro2Id, Macro2Loc, MacroExpander, MacroId,
-    MacroRulesId, MacroRulesLoc, MacroRulesLocFlags, ModuleDefId, ModuleId, ProcMacroId,
-    ProcMacroLoc, StaticLoc, StructLoc, TraitAliasLoc, TraitLoc, TypeAliasLoc, UnionLoc,
-    UnresolvedMacro, UseId, UseLoc,
+    AdtId, AstId, AstIdWithPath, ConstLoc, CrateRootModuleId, EnumLoc, ExternBlockLoc,
+    ExternCrateId, ExternCrateLoc, FunctionId, FunctionLoc, ImplLoc, Intern, ItemContainerId,
+    LocalModuleId, Lookup, Macro2Id, Macro2Loc, MacroExpander, MacroId, MacroRulesId,
+    MacroRulesLoc, MacroRulesLocFlags, ModuleDefId, ModuleId, ProcMacroId, ProcMacroLoc, StaticLoc,
+    StructLoc, TraitAliasLoc, TraitLoc, TypeAliasLoc, UnionLoc, UnresolvedMacro, UseId, UseLoc,
     attr::Attrs,
     db::DefDatabase,
     item_scope::{GlobId, ImportId, ImportOrExternCrate, PerNsGlobImports},
@@ -971,27 +970,16 @@ impl DefCollector<'_> {
                     Some(ModuleDefId::AdtId(AdtId::EnumId(e))) => {
                         cov_mark::hit!(glob_enum);
                         // glob import from enum => just import all the variants
-
-                        // We need to check if the def map the enum is from is us, if it is we can't
-                        // call the def-map query since we are currently constructing it!
-                        let loc = e.lookup(self.db);
-                        let tree = loc.id.item_tree(self.db);
-                        let current_def_map = self.def_map.krate == loc.container.krate
-                            && self.def_map.block_id() == loc.container.block;
-                        let def_map;
-                        let resolutions = if current_def_map {
-                            &self.def_map.enum_definitions[&e]
-                        } else {
-                            def_map = loc.container.def_map(self.db);
-                            &def_map.enum_definitions[&e]
-                        }
-                        .iter()
-                        .map(|&variant| {
-                            let name = tree[variant.lookup(self.db).id.value].name.clone();
-                            let res = PerNs::both(variant.into(), variant.into(), vis, None);
-                            (Some(name), res)
-                        })
-                        .collect::<Vec<_>>();
+                        let resolutions = self
+                            .db
+                            .enum_variants(e)
+                            .variants
+                            .iter()
+                            .map(|&(variant, ref name)| {
+                                let res = PerNs::both(variant.into(), variant.into(), vis, None);
+                                (Some(name.clone()), res)
+                            })
+                            .collect::<Vec<_>>();
                         self.update(
                             module_id,
                             &resolutions,
@@ -1894,39 +1882,6 @@ impl ModCollector<'_, '_> {
 
                     let vis = resolve_vis(def_map, local_def_map, &self.item_tree[it.visibility]);
                     update_def(self.def_collector, enum_.into(), &it.name, vis, false);
-
-                    let mut index = 0;
-                    let variants = FileItemTreeId::range_iter(it.variants.clone())
-                        .filter_map(|variant| {
-                            let is_enabled = self
-                                .item_tree
-                                .attrs(db, krate, variant.into())
-                                .cfg()
-                                .and_then(|cfg| self.is_cfg_enabled(&cfg).not().then_some(cfg))
-                                .map_or(Ok(()), Err);
-                            match is_enabled {
-                                Err(cfg) => {
-                                    self.emit_unconfigured_diagnostic(
-                                        self.tree_id,
-                                        variant.into(),
-                                        &cfg,
-                                    );
-                                    None
-                                }
-                                Ok(()) => Some({
-                                    let loc = EnumVariantLoc {
-                                        id: ItemTreeId::new(self.tree_id, variant),
-                                        parent: enum_,
-                                        index,
-                                    }
-                                    .intern(db);
-                                    index += 1;
-                                    loc
-                                }),
-                            }
-                        })
-                        .collect();
-                    self.def_collector.def_map.enum_definitions.insert(enum_, variants);
                 }
                 ModItem::Const(id) => {
                     let it = &self.item_tree[id];
diff --git a/crates/hir-def/src/nameres/path_resolution.rs b/crates/hir-def/src/nameres/path_resolution.rs
@@ -509,33 +509,24 @@ impl DefMap {
                 ModuleDefId::AdtId(AdtId::EnumId(e)) => {
                     // enum variant
                     cov_mark::hit!(can_import_enum_variant);
-                    let def_map;
 
-                    let loc = e.lookup(db);
-                    let tree = loc.id.item_tree(db);
-                    let current_def_map =
-                        self.krate == loc.container.krate && self.block_id() == loc.container.block;
-                    let res = if current_def_map {
-                        &self.enum_definitions[&e]
-                    } else {
-                        def_map = loc.container.def_map(db);
-                        &def_map.enum_definitions[&e]
-                    }
-                    .iter()
-                    .find_map(|&variant| {
-                        let variant_data = &tree[variant.lookup(db).id.value];
-                        (variant_data.name == *segment).then(|| match variant_data.shape {
-                            FieldsShape::Record => {
-                                PerNs::types(variant.into(), Visibility::Public, None)
-                            }
-                            FieldsShape::Tuple | FieldsShape::Unit => PerNs::both(
-                                variant.into(),
-                                variant.into(),
-                                Visibility::Public,
-                                None,
-                            ),
-                        })
-                    });
+                    let res =
+                        db.enum_variants(e).variants.iter().find(|(_, name)| name == segment).map(
+                            |&(variant, _)| {
+                                let item_tree_id = variant.lookup(db).id;
+                                match item_tree_id.item_tree(db)[item_tree_id.value].shape {
+                                    FieldsShape::Record => {
+                                        PerNs::types(variant.into(), Visibility::Public, None)
+                                    }
+                                    FieldsShape::Tuple | FieldsShape::Unit => PerNs::both(
+                                        variant.into(),
+                                        variant.into(),
+                                        Visibility::Public,
+                                        None,
+                                    ),
+                                }
+                            },
+                        );
                     // FIXME: Need to filter visibility here and below? Not sure.
                     return match res {
                         Some(res) => {
diff --git a/crates/hir-def/src/signatures.rs b/crates/hir-def/src/signatures.rs
@@ -1,20 +1,22 @@
 //! Item signature IR definitions
 
 use bitflags::bitflags;
+use cfg::{CfgExpr, CfgOptions};
 use either::Either;
-use hir_expand::{InFile, Lookup, name::Name};
+use hir_expand::{InFile, Intern, Lookup, name::Name};
 use intern::{Symbol, sym};
 use la_arena::{Arena, Idx};
 use rustc_abi::{IntegerType, ReprOptions};
 use syntax::{
     AstNode, SyntaxNodePtr,
     ast::{self, HasGenericParams, IsString},
 };
+use thin_vec::ThinVec;
 use triomphe::Arc;
 
 use crate::{
-    ConstId, EnumId, EnumVariantId, FunctionId, HasModule, ImplId, ItemContainerId, ModuleId,
-    StaticId, StructId, TraitAliasId, TraitId, TypeAliasId, UnionId, VariantId,
+    ConstId, EnumId, EnumVariantId, EnumVariantLoc, FunctionId, HasModule, ImplId, ItemContainerId,
+    ModuleId, StaticId, StructId, TraitAliasId, TraitId, TypeAliasId, UnionId, VariantId,
     db::DefDatabase,
     expr_store::{
         ExpressionStore, ExpressionStoreSourceMap,
@@ -25,8 +27,8 @@ use crate::{
     },
     hir::{ExprId, PatId, generics::GenericParams},
     item_tree::{
-        AttrOwner, Field, FieldParent, FieldsShape, ItemTree, ModItem, RawVisibility,
-        RawVisibilityId,
+        AttrOwner, Field, FieldParent, FieldsShape, FileItemTreeId, ItemTree, ItemTreeId, ModItem,
+        RawVisibility, RawVisibilityId,
     },
     lang_item::LangItem,
     src::HasSource,
@@ -889,21 +891,53 @@ fn lower_fields<'a>(
     (arena, store, col.source_map)
 }
 
+#[derive(Debug, PartialEq, Eq)]
+pub struct InactiveEnumVariantCode {
+    pub cfg: CfgExpr,
+    pub opts: CfgOptions,
+    pub ast_id: span::FileAstId<ast::Variant>,
+}
+
 #[derive(Debug, Clone, PartialEq, Eq)]
 pub struct EnumVariants {
     pub variants: Box<[(EnumVariantId, Name)]>,
 }
+
 impl EnumVariants {
-    pub(crate) fn enum_variants_query(db: &dyn DefDatabase, e: EnumId) -> Arc<EnumVariants> {
+    pub(crate) fn enum_variants_query(
+        db: &dyn DefDatabase,
+        e: EnumId,
+    ) -> (Arc<EnumVariants>, Arc<ThinVec<InactiveEnumVariantCode>>) {
         let loc = e.lookup(db);
         let item_tree = loc.id.item_tree(db);
 
-        Arc::new(EnumVariants {
-            variants: loc.container.def_map(db).enum_definitions[&e]
-                .iter()
-                .map(|&id| (id, item_tree[id.lookup(db).id.value].name.clone()))
-                .collect(),
-        })
+        let mut diagnostics = ThinVec::new();
+        let cfg_options = loc.container.krate.cfg_options(db);
+        let mut index = 0;
+        let variants = FileItemTreeId::range_iter(item_tree[loc.id.value].variants.clone())
+            .filter_map(|variant| {
+                let attrs = item_tree.attrs(db, loc.container.krate, variant.into());
+                if attrs.is_cfg_enabled(cfg_options) {
+                    let enum_variant = EnumVariantLoc {
+                        id: ItemTreeId::new(loc.id.tree_id(), variant),
+                        parent: e,
+                        index,
+                    }
+                    .intern(db);
+                    index += 1;
+                    Some((enum_variant, item_tree[variant].name.clone()))
+                } else {
+                    diagnostics.push(InactiveEnumVariantCode {
+                        ast_id: item_tree[variant].ast_id,
+                        cfg: attrs.cfg().unwrap(),
+                        opts: cfg_options.clone(),
+                    });
+                    None
+                }
+            })
+            .collect();
+
+        (Arc::new(EnumVariants { variants }), Arc::new(diagnostics))
     }
 
     pub fn variant(&self, name: &Name) -> Option<EnumVariantId> {
diff --git a/crates/hir/src/lib.rs b/crates/hir/src/lib.rs
@@ -677,12 +677,28 @@ impl Module {
                         Adt::Enum(e) => {
                             let source_map = db.enum_signature_with_source_map(e.id).1;
                             expr_store_diagnostics(db, acc, &source_map);
-                            for v in e.variants(db) {
-                                let source_map = db.variant_fields_with_source_map(v.id.into()).1;
+                            let (variants, diagnostics) = db.enum_variants_with_diagnostics(e.id);
+                            let file = e.id.lookup(db.upcast()).id.file_id();
+                            let ast_id_map = db.ast_id_map(file);
+                            for diag in diagnostics.iter() {
+                                acc.push(
+                                    InactiveCode {
+                                        node: InFile::new(
+                                            file,
+                                            ast_id_map.get(diag.ast_id).syntax_node_ptr(),
+                                        ),
+                                        cfg: diag.cfg.clone(),
+                                        opts: diag.opts.clone(),
+                                    }
+                                    .into(),
+                                );
+                            }
+                            for &(v, _) in &variants.variants {
+                                let source_map = db.variant_fields_with_source_map(v.into()).1;
                                 push_ty_diagnostics(
                                     db,
                                     acc,
-                                    db.field_types_with_diagnostics(v.id.into()).1,
+                                    db.field_types_with_diagnostics(v.into()).1,
                                     &source_map,
                                 );
                                 expr_store_diagnostics(db, acc, &source_map);
diff --git a/crates/ide/src/lib.rs b/crates/ide/src/lib.rs
@@ -753,7 +753,7 @@ impl Analysis {
         frange: FileRange,
     ) -> Cancellable<Vec<Assist>> {
         let include_fixes = match &assist_config.allowed {
-            Some(it) => it.iter().any(|&it| it == AssistKind::QuickFix),
+            Some(it) => it.contains(&AssistKind::QuickFix),
             None => true,
         };
 

Original file line number	Diff line number	Diff line change
`@@ -140,7 +140,7 @@ impl LangItems {`
`140`	`140`	`}`
`141`	`141`	`ModuleDefId::AdtId(AdtId::EnumId(e)) => {`
`142`	`142`	`lang_items.collect_lang_item(db, e, LangItemTarget::EnumId);`
`143`		`- crate_def_map.enum_definitions[&e].iter().for_each(\|&id\| {`
	`143`	`+ db.enum_variants(e).variants.iter().for_each(\|&(id, _)\| {`
`144`	`144`	`lang_items.collect_lang_item(db, id, LangItemTarget::EnumVariant);`
`145`	`145`	`});`
`146`	`146`	`}`