From 0480ebef75e0295bd26ccaba9c47ca30be3c129b Mon Sep 17 00:00:00 2001 From: Lukas Wirth Date: Sat, 18 Jan 2025 10:23:00 +0100 Subject: [PATCH] Preserve impl assoc names in ImplData --- crates/hir-def/src/data.rs | 9 ++- crates/hir-def/src/lang_item.rs | 2 +- crates/hir-ty/src/chalk_db.rs | 2 +- crates/hir-ty/src/method_resolution.rs | 26 ++++--- crates/hir-ty/src/tests.rs | 2 +- crates/hir/src/lib.rs | 27 ++------ crates/hir/src/semantics/child_by_source.rs | 2 +- crates/hir/src/symbols.rs | 77 +++++++++++---------- crates/ide-db/src/symbol_index.rs | 13 ++-- crates/ide/src/navigation_target.rs | 7 +- 10 files changed, 78 insertions(+), 89 deletions(-) diff --git a/crates/hir-def/src/data.rs b/crates/hir-def/src/data.rs index d85bc9a432..12f5f6ad79 100644 --- a/crates/hir-def/src/data.rs +++ b/crates/hir-def/src/data.rs @@ -244,7 +244,7 @@ bitflags::bitflags! { #[derive(Debug, Clone, PartialEq, Eq)] pub struct TraitData { pub name: Name, - pub items: Vec<(Name, AssocItemId)>, + pub items: Box<[(Name, AssocItemId)]>, pub flags: TraitFlags, pub visibility: RawVisibility, // box it as the vec is usually empty anyways @@ -360,7 +360,7 @@ impl TraitAliasData { pub struct ImplData { pub target_trait: Option, pub self_ty: TypeRefId, - pub items: Box<[AssocItemId]>, + pub items: Box<[(Name, AssocItemId)]>, pub is_negative: bool, pub is_unsafe: bool, // box it as the vec is usually empty anyways @@ -393,7 +393,6 @@ impl ImplData { collector.collect(&item_tree, tree_id.tree_id(), &impl_def.items); let (items, macro_calls, diagnostics) = collector.finish(); - let items = items.into_iter().map(|(_, item)| item).collect(); ( Arc::new(ImplData { @@ -648,12 +647,12 @@ impl<'a> AssocItemCollector<'a> { fn finish( self, ) -> ( - Vec<(Name, AssocItemId)>, + Box<[(Name, AssocItemId)]>, Option, MacroCallId)>>>, Vec, ) { ( - self.items, + self.items.into_boxed_slice(), if self.macro_calls.is_empty() { None } else { Some(Box::new(self.macro_calls)) }, self.diagnostics, ) diff --git a/crates/hir-def/src/lang_item.rs b/crates/hir-def/src/lang_item.rs index 0629d87e54..afdc49a2dc 100644 --- a/crates/hir-def/src/lang_item.rs +++ b/crates/hir-def/src/lang_item.rs @@ -107,7 +107,7 @@ impl LangItems { for (_, module_data) in crate_def_map.modules() { for impl_def in module_data.scope.impls() { lang_items.collect_lang_item(db, impl_def, LangItemTarget::ImplDef); - for assoc in db.impl_data(impl_def).items.iter().copied() { + for &(_, assoc) in db.impl_data(impl_def).items.iter() { match assoc { AssocItemId::FunctionId(f) => { lang_items.collect_lang_item(db, f, LangItemTarget::Function) diff --git a/crates/hir-ty/src/chalk_db.rs b/crates/hir-ty/src/chalk_db.rs index 9f01f1eb25..c8ff6cba3d 100644 --- a/crates/hir-ty/src/chalk_db.rs +++ b/crates/hir-ty/src/chalk_db.rs @@ -856,7 +856,7 @@ fn impl_def_datum( let associated_ty_value_ids = impl_data .items .iter() - .filter_map(|item| match item { + .filter_map(|(_, item)| match item { AssocItemId::TypeAliasId(type_alias) => Some(*type_alias), _ => None, }) diff --git a/crates/hir-ty/src/method_resolution.rs b/crates/hir-ty/src/method_resolution.rs index 62b071b2f3..182032f048 100644 --- a/crates/hir-ty/src/method_resolution.rs +++ b/crates/hir-ty/src/method_resolution.rs @@ -746,16 +746,9 @@ fn lookup_impl_assoc_item_for_trait_ref( let table = InferenceTable::new(db, env); let (impl_data, impl_subst) = find_matching_impl(impls, table, trait_ref)?; - let item = impl_data.items.iter().find_map(|&it| match it { - AssocItemId::FunctionId(f) => { - (db.function_data(f).name == *name).then_some(AssocItemId::FunctionId(f)) - } - AssocItemId::ConstId(c) => db - .const_data(c) - .name - .as_ref() - .map(|n| n == name) - .and_then(|result| if result { Some(AssocItemId::ConstId(c)) } else { None }), + let item = impl_data.items.iter().find_map(|(n, it)| match *it { + AssocItemId::FunctionId(f) => (n == name).then_some(AssocItemId::FunctionId(f)), + AssocItemId::ConstId(c) => (n == name).then_some(AssocItemId::ConstId(c)), AssocItemId::TypeAliasId(_) => None, })?; Some((item, impl_subst)) @@ -850,7 +843,7 @@ fn is_inherent_impl_coherent( }; rustc_has_incoherent_inherent_impls && !impl_data.items.is_empty() - && impl_data.items.iter().copied().all(|assoc| match assoc { + && impl_data.items.iter().all(|&(_, assoc)| match assoc { AssocItemId::FunctionId(it) => db.function_data(it).rustc_allow_incoherent_impl, AssocItemId::ConstId(it) => db.const_data(it).rustc_allow_incoherent_impl, AssocItemId::TypeAliasId(it) => db.type_alias_data(it).rustc_allow_incoherent_impl, @@ -1399,7 +1392,7 @@ fn iterate_inherent_methods( callback: &mut dyn FnMut(ReceiverAdjustments, AssocItemId, bool) -> ControlFlow<()>, ) -> ControlFlow<()> { for &impl_id in impls.for_self_ty(self_ty) { - for &item in table.db.impl_data(impl_id).items.iter() { + for &(ref item_name, item) in table.db.impl_data(impl_id).items.iter() { let visible = match is_valid_impl_method_candidate( table, self_ty, @@ -1408,6 +1401,7 @@ fn iterate_inherent_methods( name, impl_id, item, + item_name, ) { IsValidCandidate::Yes => true, IsValidCandidate::NotVisible => false, @@ -1467,6 +1461,7 @@ fn is_valid_impl_method_candidate( name: Option<&Name>, impl_id: ImplId, item: AssocItemId, + item_name: &Name, ) -> IsValidCandidate { match item { AssocItemId::FunctionId(f) => is_valid_impl_fn_candidate( @@ -1477,11 +1472,12 @@ fn is_valid_impl_method_candidate( receiver_ty, self_ty, visible_from_module, + item_name, ), AssocItemId::ConstId(c) => { let db = table.db; check_that!(receiver_ty.is_none()); - check_that!(name.is_none_or(|n| db.const_data(c).name.as_ref() == Some(n))); + check_that!(name.is_none_or(|n| n == item_name)); if let Some(from_module) = visible_from_module { if !db.const_visibility(c).is_visible_from(db.upcast(), from_module) { @@ -1565,11 +1561,13 @@ fn is_valid_impl_fn_candidate( receiver_ty: Option<&Ty>, self_ty: &Ty, visible_from_module: Option, + item_name: &Name, ) -> IsValidCandidate { + check_that!(name.is_none_or(|n| n == item_name)); + let db = table.db; let data = db.function_data(fn_id); - check_that!(name.is_none_or(|n| n == &data.name)); if let Some(from_module) = visible_from_module { if !db.function_visibility(fn_id).is_visible_from(db.upcast(), from_module) { cov_mark::hit!(autoderef_candidate_not_visible); diff --git a/crates/hir-ty/src/tests.rs b/crates/hir-ty/src/tests.rs index acdcbb4757..00da9b2517 100644 --- a/crates/hir-ty/src/tests.rs +++ b/crates/hir-ty/src/tests.rs @@ -435,7 +435,7 @@ pub(crate) fn visit_module( visit_scope(db, crate_def_map, &crate_def_map[module_id].scope, cb); for impl_id in crate_def_map[module_id].scope.impls() { let impl_data = db.impl_data(impl_id); - for &item in impl_data.items.iter() { + for &(_, item) in impl_data.items.iter() { match item { AssocItemId::FunctionId(it) => { let body = db.body(it.into()); diff --git a/crates/hir/src/lib.rs b/crates/hir/src/lib.rs index efa88d7c83..db3121d3cd 100644 --- a/crates/hir/src/lib.rs +++ b/crates/hir/src/lib.rs @@ -775,29 +775,16 @@ impl Module { 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( - |&item| { - Some(( - item, - match item { - AssocItemId::FunctionId(it) => db.function_data(it).name.clone(), - AssocItemId::ConstId(it) => { - db.const_data(it).name.as_ref()?.clone() - } - AssocItemId::TypeAliasId(it) => db.type_alias_data(it).name.clone(), - }, - )) - }, - )); + impl_assoc_items_scratch.extend(db.impl_data(impl_def.id).items.iter().cloned()); let redundant = impl_assoc_items_scratch .iter() - .filter(|(id, name)| { + .filter(|(name, id)| { !items.iter().any(|(impl_name, impl_item)| { discriminant(impl_item) == discriminant(id) && impl_name == name }) }) - .map(|(item, name)| (name.clone(), AssocItem::from(*item))); + .map(|(name, item)| (name.clone(), AssocItem::from(*item))); for (name, assoc_item) in redundant { acc.push( TraitImplRedundantAssocItems { @@ -812,7 +799,7 @@ impl Module { let missing: Vec<_> = required_items .filter(|(name, id)| { - !impl_assoc_items_scratch.iter().any(|(impl_item, impl_name)| { + !impl_assoc_items_scratch.iter().any(|(impl_name, impl_item)| { discriminant(impl_item) == discriminant(id) && impl_name == name }) }) @@ -844,7 +831,7 @@ impl Module { source_map, ); - for &item in db.impl_data(impl_def.id).items.iter() { + for &(_, item) in db.impl_data(impl_def.id).items.iter() { AssocItem::from(item).diagnostics(db, acc, style_lints); } } @@ -4307,7 +4294,7 @@ impl Impl { } pub fn items(self, db: &dyn HirDatabase) -> Vec { - db.impl_data(self.id).items.iter().map(|&it| it.into()).collect() + db.impl_data(self.id).items.iter().map(|&(_, it)| it.into()).collect() } pub fn is_negative(self, db: &dyn HirDatabase) -> bool { @@ -5165,7 +5152,7 @@ impl Type { let impls = db.inherent_impls_in_crate(krate); for impl_def in impls.for_self_ty(&self.ty) { - for &item in db.impl_data(*impl_def).items.iter() { + for &(_, item) in db.impl_data(*impl_def).items.iter() { if callback(item) { return; } diff --git a/crates/hir/src/semantics/child_by_source.rs b/crates/hir/src/semantics/child_by_source.rs index ec65ea9a9a..d5dfb98571 100644 --- a/crates/hir/src/semantics/child_by_source.rs +++ b/crates/hir/src/semantics/child_by_source.rs @@ -56,7 +56,7 @@ impl ChildBySource for ImplId { res[keys::ATTR_MACRO_CALL].insert(ast_id.to_ptr(db.upcast()), call_id); }, ); - data.items.iter().for_each(|&item| { + data.items.iter().for_each(|&(_, item)| { add_assoc_item(db, res, file_id, item); }); } diff --git a/crates/hir/src/symbols.rs b/crates/hir/src/symbols.rs index 9b165d1655..a6b8ed70c3 100644 --- a/crates/hir/src/symbols.rs +++ b/crates/hir/src/symbols.rs @@ -15,6 +15,7 @@ use hir_ty::{ db::HirDatabase, display::{hir_display_with_types_map, HirDisplay}, }; +use intern::Symbol; use rustc_hash::FxHashMap; use span::Edition; use syntax::{ast::HasName, AstNode, AstPtr, SmolStr, SyntaxNode, SyntaxNodePtr, ToSmolStr}; @@ -27,7 +28,7 @@ pub type FxIndexSet = indexmap::IndexSet, @@ -110,38 +111,38 @@ impl<'a> SymbolCollector<'a> { } fn collect_from_module(&mut self, module_id: ModuleId) { - let push_decl = |this: &mut Self, def| { + let push_decl = |this: &mut Self, def, name| { match def { - ModuleDefId::ModuleId(id) => this.push_module(id), + ModuleDefId::ModuleId(id) => this.push_module(id, name), ModuleDefId::FunctionId(id) => { - this.push_decl(id, false); + this.push_decl(id, name, false); this.collect_from_body(id); } - ModuleDefId::AdtId(AdtId::StructId(id)) => this.push_decl(id, false), - ModuleDefId::AdtId(AdtId::EnumId(id)) => this.push_decl(id, false), - ModuleDefId::AdtId(AdtId::UnionId(id)) => this.push_decl(id, false), + ModuleDefId::AdtId(AdtId::StructId(id)) => this.push_decl(id, name, false), + ModuleDefId::AdtId(AdtId::EnumId(id)) => this.push_decl(id, name, false), + ModuleDefId::AdtId(AdtId::UnionId(id)) => this.push_decl(id, name, false), ModuleDefId::ConstId(id) => { - this.push_decl(id, false); + this.push_decl(id, name, false); this.collect_from_body(id); } ModuleDefId::StaticId(id) => { - this.push_decl(id, false); + this.push_decl(id, name, false); this.collect_from_body(id); } ModuleDefId::TraitId(id) => { - this.push_decl(id, false); + this.push_decl(id, name, false); this.collect_from_trait(id); } ModuleDefId::TraitAliasId(id) => { - this.push_decl(id, false); + this.push_decl(id, name, false); } ModuleDefId::TypeAliasId(id) => { - this.push_decl(id, false); + this.push_decl(id, name, false); } ModuleDefId::MacroId(id) => match id { - MacroId::Macro2Id(id) => this.push_decl(id, false), - MacroId::MacroRulesId(id) => this.push_decl(id, false), - MacroId::ProcMacroId(id) => this.push_decl(id, false), + MacroId::Macro2Id(id) => this.push_decl(id, name, false), + MacroId::MacroRulesId(id) => this.push_decl(id, name, false), + MacroId::ProcMacroId(id) => this.push_decl(id, name, false), }, // Don't index these. ModuleDefId::BuiltinType(_) => {} @@ -172,7 +173,7 @@ impl<'a> SymbolCollector<'a> { name_ptr, }; this.symbols.insert(FileSymbol { - name: name.as_str().into(), + name: name.symbol().clone(), def: def.into(), container_name: this.current_container_name.clone(), loc: dec_loc, @@ -201,7 +202,7 @@ impl<'a> SymbolCollector<'a> { name_ptr, }; this.symbols.insert(FileSymbol { - name: name.as_str().into(), + name: name.symbol().clone(), def: def.into(), container_name: this.current_container_name.clone(), loc: dec_loc, @@ -242,7 +243,7 @@ impl<'a> SymbolCollector<'a> { continue; } // self is a declaration - push_decl(self, def) + push_decl(self, def, name) } for (name, Item { def, vis, import }) in scope.macros() { @@ -253,7 +254,7 @@ impl<'a> SymbolCollector<'a> { continue; } // self is a declaration - push_decl(self, def.into()) + push_decl(self, def.into(), name) } for (name, Item { def, vis, import }) in scope.values() { @@ -264,20 +265,20 @@ impl<'a> SymbolCollector<'a> { continue; } // self is a declaration - push_decl(self, def) + push_decl(self, def, name) } for const_id in scope.unnamed_consts() { self.collect_from_body(const_id); } - for (_, id) in scope.legacy_macros() { + for (name, id) in scope.legacy_macros() { for &id in id { if id.module(self.db.upcast()) == module_id { match id { - MacroId::Macro2Id(id) => self.push_decl(id, false), - MacroId::MacroRulesId(id) => self.push_decl(id, false), - MacroId::ProcMacroId(id) => self.push_decl(id, false), + MacroId::Macro2Id(id) => self.push_decl(id, name, false), + MacroId::MacroRulesId(id) => self.push_decl(id, name, false), + MacroId::ProcMacroId(id) => self.push_decl(id, name, false), } } } @@ -307,8 +308,8 @@ impl<'a> SymbolCollector<'a> { .to_smolstr(), ); self.with_container_name(impl_name, |s| { - for &assoc_item_id in impl_data.items.iter() { - s.push_assoc_item(assoc_item_id) + for &(ref name, assoc_item_id) in &impl_data.items { + s.push_assoc_item(assoc_item_id, name) } }) } @@ -316,8 +317,8 @@ impl<'a> SymbolCollector<'a> { fn collect_from_trait(&mut self, trait_id: TraitId) { let trait_data = self.db.trait_data(trait_id); self.with_container_name(Some(trait_data.name.as_str().into()), |s| { - for &(_, assoc_item_id) in &trait_data.items { - s.push_assoc_item(assoc_item_id); + for &(ref name, assoc_item_id) in &trait_data.items { + s.push_assoc_item(assoc_item_id, name); } }); } @@ -350,15 +351,15 @@ impl<'a> SymbolCollector<'a> { } } - fn push_assoc_item(&mut self, assoc_item_id: AssocItemId) { + fn push_assoc_item(&mut self, assoc_item_id: AssocItemId, name: &Name) { match assoc_item_id { - AssocItemId::FunctionId(id) => self.push_decl(id, true), - AssocItemId::ConstId(id) => self.push_decl(id, true), - AssocItemId::TypeAliasId(id) => self.push_decl(id, true), + AssocItemId::FunctionId(id) => self.push_decl(id, name, true), + AssocItemId::ConstId(id) => self.push_decl(id, name, true), + AssocItemId::TypeAliasId(id) => self.push_decl(id, name, true), } } - fn push_decl<'db, L>(&mut self, id: L, is_assoc: bool) + fn push_decl<'db, L>(&mut self, id: L, name: &Name, is_assoc: bool) where L: Lookup = dyn DefDatabase + 'db> + Into, ::Data: HasSource, @@ -377,7 +378,7 @@ impl<'a> SymbolCollector<'a> { if let Some(attrs) = def.attrs(self.db) { for alias in attrs.doc_aliases() { self.symbols.insert(FileSymbol { - name: alias.as_str().into(), + name: alias.clone(), def, loc: dec_loc.clone(), container_name: self.current_container_name.clone(), @@ -388,7 +389,7 @@ impl<'a> SymbolCollector<'a> { } self.symbols.insert(FileSymbol { - name: name_node.text().into(), + name: name.symbol().clone(), def, container_name: self.current_container_name.clone(), loc: dec_loc, @@ -397,7 +398,7 @@ impl<'a> SymbolCollector<'a> { }); } - fn push_module(&mut self, module_id: ModuleId) { + fn push_module(&mut self, module_id: ModuleId, name: &Name) { let def_map = module_id.def_map(self.db.upcast()); let module_data = &def_map[module_id.local_id]; let Some(declaration) = module_data.origin.declaration() else { return }; @@ -414,7 +415,7 @@ impl<'a> SymbolCollector<'a> { if let Some(attrs) = def.attrs(self.db) { for alias in attrs.doc_aliases() { self.symbols.insert(FileSymbol { - name: alias.as_str().into(), + name: alias.clone(), def, loc: dec_loc.clone(), container_name: self.current_container_name.clone(), @@ -425,7 +426,7 @@ impl<'a> SymbolCollector<'a> { } self.symbols.insert(FileSymbol { - name: name_node.text().into(), + name: name.symbol().clone(), def: ModuleDef::Module(module_id.into()), container_name: self.current_container_name.clone(), loc: dec_loc, diff --git a/crates/ide-db/src/symbol_index.rs b/crates/ide-db/src/symbol_index.rs index 3af536dff5..79f89a89a1 100644 --- a/crates/ide-db/src/symbol_index.rs +++ b/crates/ide-db/src/symbol_index.rs @@ -252,8 +252,8 @@ impl Hash for SymbolIndex { impl SymbolIndex { fn new(mut symbols: Box<[FileSymbol]>) -> SymbolIndex { fn cmp(lhs: &FileSymbol, rhs: &FileSymbol) -> Ordering { - let lhs_chars = lhs.name.chars().map(|c| c.to_ascii_lowercase()); - let rhs_chars = rhs.name.chars().map(|c| c.to_ascii_lowercase()); + let lhs_chars = lhs.name.as_str().chars().map(|c| c.to_ascii_lowercase()); + let rhs_chars = rhs.name.as_str().chars().map(|c| c.to_ascii_lowercase()); lhs_chars.cmp(rhs_chars) } @@ -374,10 +374,11 @@ impl Query { continue; } // Hide symbols that start with `__` unless the query starts with `__` - if ignore_underscore_prefixed && symbol.name.starts_with("__") { + let symbol_name = symbol.name.as_str(); + if ignore_underscore_prefixed && symbol_name.starts_with("__") { continue; } - if self.mode.check(&self.query, self.case_sensitive, &symbol.name) { + if self.mode.check(&self.query, self.case_sensitive, symbol_name) { cb(symbol); } } @@ -484,7 +485,7 @@ pub(self) use crate::Trait as IsThisJustATrait; .into_iter() .map(|module_id| { let mut symbols = SymbolCollector::collect_module(&db, module_id); - symbols.sort_by_key(|it| it.name.clone()); + symbols.sort_by_key(|it| it.name.as_str().to_owned()); (module_id, symbols) }) .collect(); @@ -511,7 +512,7 @@ struct Duplicate; .into_iter() .map(|module_id| { let mut symbols = SymbolCollector::collect_module(&db, module_id); - symbols.sort_by_key(|it| it.name.clone()); + symbols.sort_by_key(|it| it.name.as_str().to_owned()); (module_id, symbols) }) .collect(); diff --git a/crates/ide/src/navigation_target.rs b/crates/ide/src/navigation_target.rs index 702f274ae3..d9f80cb53d 100644 --- a/crates/ide/src/navigation_target.rs +++ b/crates/ide/src/navigation_target.rs @@ -44,13 +44,16 @@ pub struct NavigationTarget { /// /// This range must be contained within [`Self::full_range`]. pub focus_range: Option, + // FIXME: Symbol pub name: SmolStr, pub kind: Option, + // FIXME: Symbol pub container_name: Option, pub description: Option, pub docs: Option, /// In addition to a `name` field, a `NavigationTarget` may also be aliased /// In such cases we want a `NavigationTarget` to be accessible by its alias + // FIXME: Symbol pub alias: Option, } @@ -191,10 +194,10 @@ impl TryToNav for FileSymbol { NavigationTarget { file_id, name: self.is_alias.then(|| self.def.name(db)).flatten().map_or_else( - || self.name.clone(), + || self.name.as_str().into(), |it| it.display_no_db(edition).to_smolstr(), ), - alias: self.is_alias.then(|| self.name.clone()), + alias: self.is_alias.then(|| self.name.as_str().into()), kind: Some(self.def.into()), full_range, focus_range,