Merge pull request #22329 from ChayimFriedman2/intern-ref

Encode the name instead of index in `EnumVariantId`

Author: Veykril

crates/base-db/src/lib.rs

@@ -49,6 +49,7 @@ macro_rules! impl_intern_key {
         #[salsa_macros::interned(no_lifetime, revisions = usize::MAX)]
         #[derive(PartialOrd, Ord)]
         pub struct $id {
+            #[returns(ref)]
             pub loc: $loc,
         }
 

crates/hir-def/src/db.rs

@@ -7,8 +7,7 @@ use hir_expand::{
 use triomphe::Arc;
 
 use crate::{
-    AssocItemId, AttrDefId, Macro2Loc, MacroExpander, MacroId, MacroRulesLoc, MacroRulesLocFlags,
-    TraitId,
+    AssocItemId, AttrDefId, MacroExpander, MacroId, MacroRulesLocFlags, TraitId,
     attrs::AttrFlags,
     item_tree::{ItemTree, file_item_tree},
     nameres::crate_def_map,
@@ -81,7 +80,7 @@ fn macro_def(db: &dyn DefDatabase, id: MacroId) -> MacroDefId {
 
     match id {
         MacroId::Macro2Id(it) => {
-            let loc: Macro2Loc = it.lookup(db);
+            let loc = it.lookup(db);
 
             MacroDefId {
                 krate: loc.container.krate(db),
@@ -92,7 +91,7 @@ fn macro_def(db: &dyn DefDatabase, id: MacroId) -> MacroDefId {
             }
         }
         MacroId::MacroRulesId(it) => {
-            let loc: MacroRulesLoc = it.lookup(db);
+            let loc = it.lookup(db);
 
             MacroDefId {
                 krate: loc.container.krate(db),

crates/hir-def/src/find_path.rs

@@ -142,9 +142,7 @@ fn find_path_inner(ctx: &FindPathCtx<'_>, item: ItemInNs, max_len: usize) -> Opt
         // - if the item is an enum variant, refer to it via the enum
         let loc = variant.lookup(ctx.db);
         if let Some(mut path) = find_path_inner(ctx, ItemInNs::Types(loc.parent.into()), max_len) {
-            path.push_segment(
-                loc.parent.enum_variants(ctx.db).variants[loc.index as usize].1.clone(),
-            );
+            path.push_segment(loc.name.clone());
             return Some(path);
         }
         // If this doesn't work, it seems we have no way of referring to the

crates/hir-def/src/lang_item.rs

@@ -74,7 +74,7 @@ pub fn crate_lang_items(db: &dyn DefDatabase, krate: Crate) -> Option<Box<LangIt
                 }
                 ModuleDefId::AdtId(AdtId::EnumId(e)) => {
                     lang_items.collect_lang_item(db, e);
-                    e.enum_variants(db).variants.iter().for_each(|&(id, _, _)| {
+                    e.enum_variants(db).variants.values().for_each(|&(id, _)| {
                         lang_items.collect_lang_item(db, id);
                     });
                 }

crates/hir-def/src/lib.rs

@@ -298,7 +298,7 @@ impl EnumId {
     pub fn enum_variants_with_diagnostics(
         self,
         db: &dyn DefDatabase,
-    ) -> &(EnumVariants, Option<ThinVec<InactiveEnumVariantCode>>) {
+    ) -> &(EnumVariants, ThinVec<InactiveEnumVariantCode>) {
         EnumVariants::of(db, self)
     }
 }
@@ -367,20 +367,35 @@ impl ExternBlockId {
     }
 }
 
-#[derive(Debug, Clone, Copy, PartialEq, Eq, Hash)]
+#[derive(Debug, Clone, PartialEq, Eq, Hash)]
 pub struct EnumVariantLoc {
     pub id: AstId<ast::Variant>,
     pub parent: EnumId,
-    pub index: u32,
+    pub name: Name,
 }
 impl_intern!(EnumVariantId, EnumVariantLoc);
 impl_loc!(EnumVariantLoc, id: Variant, parent: EnumId);
 
+impl EnumVariantLoc {
+    pub fn index(&self, db: &dyn DefDatabase) -> usize {
+        self.parent
+            .enum_variants(db)
+            .variants
+            .get_full(&self.name)
+            .expect("parent enum should include this variant")
+            .0
+    }
+}
+
 impl EnumVariantId {
     pub fn fields(self, db: &dyn DefDatabase) -> &VariantFields {
         VariantFields::of(db, self.into())
     }
 
+    pub fn index(self, db: &dyn DefDatabase) -> usize {
+        self.loc(db).index(db)
+    }
+
     pub fn fields_with_source_map(
         self,
         db: &dyn DefDatabase,

crates/hir-def/src/nameres.rs

@@ -427,7 +427,7 @@ pub(crate) fn crate_local_def_map(db: &dyn DefDatabase, crate_id: Crate) -> DefM
 
 #[salsa_macros::tracked(returns(ref))]
 pub fn block_def_map(db: &dyn DefDatabase, block_id: BlockId) -> DefMap {
-    let BlockLoc { ast_id, module } = block_id.lookup(db);
+    let BlockLoc { ast_id, module } = *block_id.lookup(db);
 
     let visibility = Visibility::Module(module, VisibilityExplicitness::Implicit);
     let module_data =

crates/hir-def/src/nameres/assoc.rs

@@ -50,7 +50,7 @@ impl TraitItems {
         db: &dyn DefDatabase,
         tr: TraitId,
     ) -> (TraitItems, DefDiagnostics) {
-        let ItemLoc { container: module_id, id: ast_id } = tr.lookup(db);
+        let ItemLoc { container: module_id, id: ast_id } = *tr.lookup(db);
         let ast_id_map = db.ast_id_map(ast_id.file_id);
         let source = ast_id.with_value(ast_id_map.get(ast_id.value)).to_node(db);
         if source.eq_token().is_some() {
@@ -115,7 +115,7 @@ impl ImplItems {
     #[salsa::tracked(returns(ref))]
     pub fn of(db: &dyn DefDatabase, id: ImplId) -> (ImplItems, DefDiagnostics) {
         let _p = tracing::info_span!("impl_items_with_diagnostics_query").entered();
-        let ItemLoc { container: module_id, id: ast_id } = id.lookup(db);
+        let ItemLoc { container: module_id, id: ast_id } = *id.lookup(db);
 
         let collector =
             AssocItemCollector::new(db, module_id, ItemContainerId::ImplId(id), ast_id.file_id);

crates/hir-def/src/nameres/collector.rs

@@ -1027,7 +1027,7 @@ impl<'db> DefCollector<'db> {
                             .enum_variants(self.db)
                             .variants
                             .iter()
-                            .map(|&(variant, ref name, _)| {
+                            .map(|(name, &(variant, _))| {
                                 let res = PerNs::both(variant.into(), variant.into(), vis, None);
                                 (Some(name.clone()), res)
                             })

crates/hir-def/src/nameres/path_resolution.rs

@@ -519,12 +519,8 @@ impl DefMap {
                     // enum variant
                     cov_mark::hit!(can_import_enum_variant);
 
-                    let res = e
-                        .enum_variants(db)
-                        .variants
-                        .iter()
-                        .find(|(_, name, _)| name == segment)
-                        .map(|&(variant, _, shape)| match shape {
+                    let res = e.enum_variants(db).variants.get(segment).map(|&(variant, shape)| {
+                        match shape {
                             FieldsShape::Record => {
                                 PerNs::types(variant.into(), Visibility::Public, None)
                             }
@@ -534,7 +530,8 @@ impl DefMap {
                                 Visibility::Public,
                                 None,
                             ),
-                        });
+                        }
+                    });
                     // FIXME: Need to filter visibility here and below? Not sure.
                     return match res {
                         Some(res) => {

crates/hir-def/src/signatures.rs

@@ -1,6 +1,6 @@
 //! Item signature IR definitions
 
-use std::{cell::LazyCell, ops::Not as _};
+use std::cell::LazyCell;
 
 use bitflags::bitflags;
 use cfg::{CfgExpr, CfgOptions};
@@ -19,8 +19,9 @@ use thin_vec::ThinVec;
 use triomphe::Arc;
 
 use crate::{
-    ConstId, EnumId, EnumVariantId, EnumVariantLoc, ExternBlockId, FunctionId, HasModule, ImplId,
-    ItemContainerId, ModuleId, StaticId, StructId, TraitId, TypeAliasId, UnionId, VariantId,
+    ConstId, EnumId, EnumVariantId, EnumVariantLoc, ExternBlockId, FunctionId, FxIndexMap,
+    HasModule, ImplId, ItemContainerId, ModuleId, StaticId, StructId, TraitId, TypeAliasId,
+    UnionId, VariantId,
     attrs::AttrFlags,
     db::DefDatabase,
     expr_store::{
@@ -1055,7 +1056,7 @@ pub struct InactiveEnumVariantCode {
 
 #[derive(Debug, Clone, PartialEq, Eq)]
 pub struct EnumVariants {
-    pub variants: Box<[(EnumVariantId, Name, FieldsShape)]>,
+    pub variants: FxIndexMap<Name, (EnumVariantId, FieldsShape)>,
 }
 
 #[salsa::tracked]
@@ -1064,30 +1065,31 @@ impl EnumVariants {
     pub(crate) fn of(
         db: &dyn DefDatabase,
         e: EnumId,
-    ) -> (EnumVariants, Option<ThinVec<InactiveEnumVariantCode>>) {
+    ) -> (EnumVariants, ThinVec<InactiveEnumVariantCode>) {
         let loc = e.lookup(db);
         let source = loc.source(db);
         let ast_id_map = db.ast_id_map(source.file_id);
 
         let mut diagnostics = ThinVec::new();
         let cfg_options = loc.container.krate(db).cfg_options(db);
-        let mut index = 0;
         let Some(variants) = source.value.variant_list() else {
-            return (EnumVariants { variants: Box::default() }, None);
+            return (EnumVariants { variants: FxIndexMap::default() }, ThinVec::new());
         };
-        let variants = variants
+        let mut variants = variants
             .variants()
             .filter_map(|variant| {
                 let ast_id = ast_id_map.ast_id(&variant);
                 match AttrFlags::is_cfg_enabled_for(&variant, cfg_options) {
                     Ok(()) => {
-                        let enum_variant =
-                            EnumVariantLoc { id: source.with_value(ast_id), parent: e, index }
-                                .intern(db);
-                        index += 1;
                         let name = as_name_opt(variant.name());
+                        let enum_variant = EnumVariantLoc {
+                            id: source.with_value(ast_id),
+                            parent: e,
+                            name: name.clone(),
+                        }
+                        .intern(db);
                         let shape = adt_shape(variant.kind());
-                        Some((enum_variant, name, shape))
+                        Some((name, (enum_variant, shape)))
                     }
                     Err(cfg) => {
                         diagnostics.push(InactiveEnumVariantCode {
@@ -1099,34 +1101,38 @@ impl EnumVariants {
                     }
                 }
             })
-            .collect();
+            .collect::<FxIndexMap<_, _>>();
+        variants.shrink_to_fit();
+        diagnostics.shrink_to_fit();
 
-        (EnumVariants { variants }, diagnostics.is_empty().not().then_some(diagnostics))
+        (EnumVariants { variants }, diagnostics)
     }
 }
 
 impl EnumVariants {
     pub fn variant(&self, name: &Name) -> Option<EnumVariantId> {
-        self.variants.iter().find_map(|(v, n, _)| if n == name { Some(*v) } else { None })
+        self.variants.get(name).map(|&(id, _)| id)
     }
 
     pub fn variant_name_by_id(&self, variant_id: EnumVariantId) -> Option<Name> {
         self.variants
             .iter()
-            .find_map(|(id, name, _)| if *id == variant_id { Some(name.clone()) } else { None })
+            .find_map(|(name, (id, _))| if *id == variant_id { Some(name.clone()) } else { None })
     }
 
     // [Adopted from rustc](https://github.com/rust-lang/rust/blob/bd53aa3bf7a24a70d763182303bd75e5fc51a9af/compiler/rustc_middle/src/ty/adt.rs#L446-L448)
     pub fn is_payload_free(&self, db: &dyn DefDatabase) -> bool {
-        self.variants.iter().all(|&(v, _, _)| {
+        self.variants.values().all(|&(v, shape)| {
             // The condition check order is slightly modified from rustc
             // to improve performance by early returning with relatively fast checks
-            let variant = v.fields(db);
-            if !variant.fields().is_empty() {
-                return false;
-            }
+
             // The outer if condition is whether this variant has const ctor or not
-            if !matches!(variant.shape, FieldsShape::Unit) {
+            if !matches!(shape, FieldsShape::Unit) {
+                let variant = v.fields(db);
+                if !variant.fields().is_empty() {
+                    return false;
+                }
+
                 let body = Body::of(db, v.into());
                 // A variant with explicit discriminant
                 if !matches!(body[body.root_expr()], crate::hir::Expr::Missing) {