From 241fd2ff4d16c910d1cf7ceda96d4a1a40737af7 Mon Sep 17 00:00:00 2001 From: Lukas Wirth Date: Sat, 18 Jan 2025 09:06:46 +0100 Subject: [PATCH 1/5] Properly record meaningful imports as re-exports in symbol index --- src/tools/rust-analyzer/Cargo.lock | 1 + .../crates/hir-def/src/item_scope.rs | 19 +- .../rust-analyzer/crates/hir-def/src/lib.rs | 2 +- .../crates/hir-def/src/nameres/collector.rs | 4 +- .../crates/hir-def/src/visibility.rs | 4 +- src/tools/rust-analyzer/crates/hir/Cargo.toml | 1 + .../rust-analyzer/crates/hir/src/symbols.rs | 210 ++++++++++++------ .../ide-completion/src/tests/flyimport.rs | 6 +- .../ide-db/src/imports/import_assets.rs | 15 +- .../crates/ide-db/src/items_locator.rs | 1 + .../crates/ide-db/src/symbol_index.rs | 6 +- .../test_symbol_index_collection.txt | 87 +++----- 12 files changed, 213 insertions(+), 143 deletions(-) diff --git a/src/tools/rust-analyzer/Cargo.lock b/src/tools/rust-analyzer/Cargo.lock index d62c55712213b..f92668a6a9786 100644 --- a/src/tools/rust-analyzer/Cargo.lock +++ b/src/tools/rust-analyzer/Cargo.lock @@ -523,6 +523,7 @@ dependencies = [ "hir-def", "hir-expand", "hir-ty", + "indexmap", "intern", "itertools", "rustc-hash 2.0.0", diff --git a/src/tools/rust-analyzer/crates/hir-def/src/item_scope.rs b/src/tools/rust-analyzer/crates/hir-def/src/item_scope.rs index 2c3eb5c8e5e45..0fec7674109bc 100644 --- a/src/tools/rust-analyzer/crates/hir-def/src/item_scope.rs +++ b/src/tools/rust-analyzer/crates/hir-def/src/item_scope.rs @@ -162,6 +162,20 @@ impl ItemScope { .map(move |name| (name, self.get(name))) } + pub fn values(&self) -> impl Iterator)> + '_ { + self.values.iter().map(|(n, &i)| (n, i)) + } + + pub fn types( + &self, + ) -> impl Iterator)> + '_ { + self.types.iter().map(|(n, &i)| (n, i)) + } + + pub fn macros(&self) -> impl Iterator)> + '_ { + self.macros.iter().map(|(n, &i)| (n, i)) + } + pub fn imports(&self) -> impl Iterator + '_ { self.use_imports_types .keys() @@ -263,11 +277,6 @@ impl ItemScope { self.unnamed_consts.iter().copied() } - /// Iterate over all module scoped macros - pub(crate) fn macros(&self) -> impl Iterator + '_ { - self.entries().filter_map(|(name, def)| def.take_macros().map(|macro_| (name, macro_))) - } - /// Iterate over all legacy textual scoped macros visible at the end of the module pub fn legacy_macros(&self) -> impl Iterator + '_ { self.legacy_macros.iter().map(|(name, def)| (name, &**def)) diff --git a/src/tools/rust-analyzer/crates/hir-def/src/lib.rs b/src/tools/rust-analyzer/crates/hir-def/src/lib.rs index 8af27513ebc03..84c105a0a3467 100644 --- a/src/tools/rust-analyzer/crates/hir-def/src/lib.rs +++ b/src/tools/rust-analyzer/crates/hir-def/src/lib.rs @@ -502,7 +502,7 @@ impl ModuleId { } /// Whether this module represents the crate root module - fn is_crate_root(&self) -> bool { + pub fn is_crate_root(&self) -> bool { self.local_id == DefMap::ROOT && self.block.is_none() } } diff --git a/src/tools/rust-analyzer/crates/hir-def/src/nameres/collector.rs b/src/tools/rust-analyzer/crates/hir-def/src/nameres/collector.rs index 6808c3cd4d79f..1e4b42dff5fb7 100644 --- a/src/tools/rust-analyzer/crates/hir-def/src/nameres/collector.rs +++ b/src/tools/rust-analyzer/crates/hir-def/src/nameres/collector.rs @@ -717,8 +717,8 @@ impl DefCollector<'_> { } } None => { - for (name, def) in root_scope.macros() { - self.def_map.macro_use_prelude.insert(name.clone(), (def, extern_crate)); + for (name, it) in root_scope.macros() { + self.def_map.macro_use_prelude.insert(name.clone(), (it.def, extern_crate)); } } } diff --git a/src/tools/rust-analyzer/crates/hir-def/src/visibility.rs b/src/tools/rust-analyzer/crates/hir-def/src/visibility.rs index 4edb683592253..c4473e454a1bc 100644 --- a/src/tools/rust-analyzer/crates/hir-def/src/visibility.rs +++ b/src/tools/rust-analyzer/crates/hir-def/src/visibility.rs @@ -240,12 +240,12 @@ impl Visibility { if a_ancestors.any(|m| m == mod_b.local_id) { // B is above A - return Some(Visibility::Module(mod_a, expl_b)); + return Some(Visibility::Module(mod_a, expl_a)); } if b_ancestors.any(|m| m == mod_a.local_id) { // A is above B - return Some(Visibility::Module(mod_b, expl_a)); + return Some(Visibility::Module(mod_b, expl_b)); } None diff --git a/src/tools/rust-analyzer/crates/hir/Cargo.toml b/src/tools/rust-analyzer/crates/hir/Cargo.toml index 6aadc5c4f7e79..c68ff706e4814 100644 --- a/src/tools/rust-analyzer/crates/hir/Cargo.toml +++ b/src/tools/rust-analyzer/crates/hir/Cargo.toml @@ -20,6 +20,7 @@ itertools.workspace = true smallvec.workspace = true tracing.workspace = true triomphe.workspace = true +indexmap.workspace = true # local deps base-db.workspace = true diff --git a/src/tools/rust-analyzer/crates/hir/src/symbols.rs b/src/tools/rust-analyzer/crates/hir/src/symbols.rs index f8416f86bf9fa..9feaa0b7da508 100644 --- a/src/tools/rust-analyzer/crates/hir/src/symbols.rs +++ b/src/tools/rust-analyzer/crates/hir/src/symbols.rs @@ -1,22 +1,28 @@ //! File symbol extraction. +use either::Either; use hir_def::{ db::DefDatabase, - item_scope::ItemInNs, + item_scope::{ImportId, ImportOrExternCrate}, + per_ns::Item, src::{HasChildSource, HasSource}, - AdtId, AssocItemId, DefWithBodyId, HasModule, ImplId, Lookup, MacroId, ModuleDefId, ModuleId, - TraitId, + visibility::{Visibility, VisibilityExplicitness}, + AdtId, AssocItemId, DefWithBodyId, ExternCrateId, HasModule, ImplId, Lookup, MacroId, + ModuleDefId, ModuleId, TraitId, }; -use hir_expand::HirFileId; +use hir_expand::{name::Name, HirFileId}; use hir_ty::{ db::HirDatabase, display::{hir_display_with_types_map, HirDisplay}, }; +use rustc_hash::FxHashMap; use span::Edition; use syntax::{ast::HasName, AstNode, AstPtr, SmolStr, SyntaxNode, SyntaxNodePtr, ToSmolStr}; use crate::{Module, ModuleDef, Semantics}; +pub type FxIndexSet = indexmap::IndexSet>; + /// The actual data that is stored in the index. It should be as compact as /// possible. #[derive(Debug, Clone, PartialEq, Eq, Hash)] @@ -37,7 +43,7 @@ pub struct DeclarationLocation { /// This points to the whole syntax node of the declaration. pub ptr: SyntaxNodePtr, /// This points to the [`syntax::ast::Name`] identifier of the declaration. - pub name_ptr: AstPtr, + pub name_ptr: AstPtr>, } impl DeclarationLocation { @@ -55,7 +61,7 @@ struct SymbolCollectorWork { pub struct SymbolCollector<'a> { db: &'a dyn HirDatabase, - symbols: Vec, + symbols: FxIndexSet, work: Vec, current_container_name: Option, edition: Edition, @@ -87,7 +93,7 @@ impl<'a> SymbolCollector<'a> { } pub fn finish(self) -> Vec { - self.symbols + self.symbols.into_iter().collect() } pub fn collect_module(db: &dyn HirDatabase, module: Module) -> Vec { @@ -104,83 +110,161 @@ impl<'a> SymbolCollector<'a> { } fn collect_from_module(&mut self, module_id: ModuleId) { - let def_map = module_id.def_map(self.db.upcast()); - let scope = &def_map[module_id.local_id].scope; - - for module_def_id in scope.declarations() { - match module_def_id { - ModuleDefId::ModuleId(id) => self.push_module(id), + let push_decl = |this: &mut Self, def| { + match def { + ModuleDefId::ModuleId(id) => this.push_module(id), ModuleDefId::FunctionId(id) => { - self.push_decl(id, false); - self.collect_from_body(id); + this.push_decl(id, false); + this.collect_from_body(id); } - ModuleDefId::AdtId(AdtId::StructId(id)) => self.push_decl(id, false), - ModuleDefId::AdtId(AdtId::EnumId(id)) => self.push_decl(id, false), - ModuleDefId::AdtId(AdtId::UnionId(id)) => self.push_decl(id, false), + 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::ConstId(id) => { - self.push_decl(id, false); - self.collect_from_body(id); + this.push_decl(id, false); + this.collect_from_body(id); } ModuleDefId::StaticId(id) => { - self.push_decl(id, false); - self.collect_from_body(id); + this.push_decl(id, false); + this.collect_from_body(id); } ModuleDefId::TraitId(id) => { - self.push_decl(id, false); - self.collect_from_trait(id); + this.push_decl(id, false); + this.collect_from_trait(id); } ModuleDefId::TraitAliasId(id) => { - self.push_decl(id, false); + this.push_decl(id, false); } ModuleDefId::TypeAliasId(id) => { - self.push_decl(id, false); + this.push_decl(id, false); } ModuleDefId::MacroId(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) => this.push_decl(id, false), + MacroId::MacroRulesId(id) => this.push_decl(id, false), + MacroId::ProcMacroId(id) => this.push_decl(id, false), }, // Don't index these. ModuleDefId::BuiltinType(_) => {} ModuleDefId::EnumVariantId(_) => {} } - } + }; - for impl_id in scope.impls() { - self.collect_from_impl(impl_id); - } + // Nested trees are very common, so a cache here will hit a lot. + let import_child_source_cache = &mut FxHashMap::default(); + + let mut push_import = |this: &mut Self, i: ImportId, name: &Name, def: ModuleDefId| { + let source = import_child_source_cache + .entry(i.import) + .or_insert_with(|| i.import.child_source(this.db.upcast())); + let Some(use_tree_src) = source.value.get(i.idx) else { return }; + let Some(name_ptr) = use_tree_src + .rename() + .and_then(|rename| rename.name()) + .map(Either::Left) + .or_else(|| use_tree_src.path()?.segment()?.name_ref().map(Either::Right)) + .map(|it| AstPtr::new(&it)) + else { + return; + }; + let dec_loc = DeclarationLocation { + hir_file_id: source.file_id, + ptr: SyntaxNodePtr::new(use_tree_src.syntax()), + name_ptr, + }; + this.symbols.insert(FileSymbol { + name: name.as_str().into(), + def: def.into(), + container_name: this.current_container_name.clone(), + loc: dec_loc, + is_alias: false, + is_assoc: false, + }); + }; - // Record renamed imports. - // FIXME: In case it imports multiple items under different namespaces we just pick one arbitrarily - // for now. - for id in scope.imports() { - let source = id.import.child_source(self.db.upcast()); - let Some(use_tree_src) = source.value.get(id.idx) else { continue }; - let Some(rename) = use_tree_src.rename() else { continue }; - let Some(name) = rename.name() else { continue }; - - let res = scope.fully_resolve_import(self.db.upcast(), id); - res.iter_items().for_each(|(item, _)| { - let def = match item { - ItemInNs::Types(def) | ItemInNs::Values(def) => def, - ItemInNs::Macros(def) => ModuleDefId::from(def), - } - .into(); + let push_extern_crate = + |this: &mut Self, i: ExternCrateId, name: &Name, def: ModuleDefId| { + let loc = i.lookup(this.db.upcast()); + let source = loc.source(this.db.upcast()); + let Some(name_ptr) = source + .value + .rename() + .and_then(|rename| rename.name()) + .map(Either::Left) + .or_else(|| source.value.name_ref().map(Either::Right)) + .map(|it| AstPtr::new(&it)) + else { + return; + }; let dec_loc = DeclarationLocation { hir_file_id: source.file_id, - ptr: SyntaxNodePtr::new(use_tree_src.syntax()), - name_ptr: AstPtr::new(&name), + ptr: SyntaxNodePtr::new(source.value.syntax()), + name_ptr, }; - - self.symbols.push(FileSymbol { - name: name.text().into(), - def, - container_name: self.current_container_name.clone(), + this.symbols.insert(FileSymbol { + name: name.as_str().into(), + def: def.into(), + container_name: this.current_container_name.clone(), loc: dec_loc, is_alias: false, is_assoc: false, }); - }); + }; + + let is_explicit_import = |vis| { + match vis { + Visibility::Module(_, VisibilityExplicitness::Explicit) => true, + Visibility::Module(_, VisibilityExplicitness::Implicit) => { + // consider imports in the crate root explicit, as these are visibly + // crate-wide anyways + module_id.is_crate_root() + } + Visibility::Public => true, + } + }; + + let def_map = module_id.def_map(self.db.upcast()); + let scope = &def_map[module_id.local_id].scope; + + for impl_id in scope.impls() { + self.collect_from_impl(impl_id); + } + + for (name, Item { def, vis, import }) in scope.types() { + if let Some(i) = import { + if is_explicit_import(vis) { + match i { + ImportOrExternCrate::Import(i) => push_import(self, i, name, def), + ImportOrExternCrate::ExternCrate(i) => { + push_extern_crate(self, i, name, def) + } + } + } + continue; + } + // self is a declaration + push_decl(self, def) + } + + for (name, Item { def, vis, import }) in scope.macros() { + if let Some(i) = import { + if is_explicit_import(vis) { + push_import(self, i, name, def.into()); + } + continue; + } + // self is a declaration + push_decl(self, def.into()) + } + + for (name, Item { def, vis, import }) in scope.values() { + if let Some(i) = import { + if is_explicit_import(vis) { + push_import(self, i, name, def); + } + continue; + } + // self is a declaration + push_decl(self, def) } for const_id in scope.unnamed_consts() { @@ -287,12 +371,12 @@ impl<'a> SymbolCollector<'a> { let dec_loc = DeclarationLocation { hir_file_id: source.file_id, ptr: SyntaxNodePtr::new(source.value.syntax()), - name_ptr: AstPtr::new(&name_node), + name_ptr: AstPtr::new(&name_node).wrap_left(), }; if let Some(attrs) = def.attrs(self.db) { for alias in attrs.doc_aliases() { - self.symbols.push(FileSymbol { + self.symbols.insert(FileSymbol { name: alias.as_str().into(), def, loc: dec_loc.clone(), @@ -303,7 +387,7 @@ impl<'a> SymbolCollector<'a> { } } - self.symbols.push(FileSymbol { + self.symbols.insert(FileSymbol { name: name_node.text().into(), def, container_name: self.current_container_name.clone(), @@ -322,14 +406,14 @@ impl<'a> SymbolCollector<'a> { let dec_loc = DeclarationLocation { hir_file_id: declaration.file_id, ptr: SyntaxNodePtr::new(module.syntax()), - name_ptr: AstPtr::new(&name_node), + name_ptr: AstPtr::new(&name_node).wrap_left(), }; let def = ModuleDef::Module(module_id.into()); if let Some(attrs) = def.attrs(self.db) { for alias in attrs.doc_aliases() { - self.symbols.push(FileSymbol { + self.symbols.insert(FileSymbol { name: alias.as_str().into(), def, loc: dec_loc.clone(), @@ -340,7 +424,7 @@ impl<'a> SymbolCollector<'a> { } } - self.symbols.push(FileSymbol { + self.symbols.insert(FileSymbol { name: name_node.text().into(), def: ModuleDef::Module(module_id.into()), container_name: self.current_container_name.clone(), diff --git a/src/tools/rust-analyzer/crates/ide-completion/src/tests/flyimport.rs b/src/tools/rust-analyzer/crates/ide-completion/src/tests/flyimport.rs index 0adf1b825f7b6..d491e438feffc 100644 --- a/src/tools/rust-analyzer/crates/ide-completion/src/tests/flyimport.rs +++ b/src/tools/rust-analyzer/crates/ide-completion/src/tests/flyimport.rs @@ -1746,7 +1746,7 @@ fn intrinsics() { fn function() { transmute$0 } - "#, +"#, expect![[r#" fn transmute(…) (use core::mem::transmute) unsafe fn(Src) -> Dst "#]], @@ -1767,7 +1767,9 @@ fn function() { mem::transmute$0 } "#, - expect![""], + expect![[r#" + fn transmute(…) (use core::mem) unsafe fn(Src) -> Dst + "#]], ); } diff --git a/src/tools/rust-analyzer/crates/ide-db/src/imports/import_assets.rs b/src/tools/rust-analyzer/crates/ide-db/src/imports/import_assets.rs index 8f0be1d90354d..3ed101ff36a84 100644 --- a/src/tools/rust-analyzer/crates/ide-db/src/imports/import_assets.rs +++ b/src/tools/rust-analyzer/crates/ide-db/src/imports/import_assets.rs @@ -350,6 +350,9 @@ fn path_applicable_imports( .take(DEFAULT_QUERY_SEARCH_LIMIT.inner()) .collect() } + // we have some unresolved qualifier that we search an import for + // The key here is that whatever we import must form a resolved path for the remainder of + // what follows [first_qsegment, qualifier_rest @ ..] => items_locator::items_with_name( sema, current_crate, @@ -357,14 +360,16 @@ fn path_applicable_imports( AssocSearchMode::Exclude, ) .filter_map(|item| { - import_for_item( + // we found imports for `first_qsegment`, now we need to filter these imports by whether + // they result in resolving the rest of the path successfully + validate_resolvable( sema, scope, mod_path, + scope_filter, &path_candidate.name, item, qualifier_rest, - scope_filter, ) }) .take(DEFAULT_QUERY_SEARCH_LIMIT.inner()) @@ -372,14 +377,16 @@ fn path_applicable_imports( } } -fn import_for_item( +/// Validates and builds an import for `resolved_qualifier` if the `unresolved_qualifier` appended +/// to it resolves and there is a validate `candidate` after that. +fn validate_resolvable( sema: &Semantics<'_, RootDatabase>, scope: &SemanticsScope<'_>, mod_path: impl Fn(ItemInNs) -> Option, + scope_filter: impl Fn(ItemInNs) -> bool, candidate: &NameToImport, resolved_qualifier: ItemInNs, unresolved_qualifier: &[SmolStr], - scope_filter: impl Fn(ItemInNs) -> bool, ) -> Option { let _p = tracing::info_span!("ImportAssets::import_for_item").entered(); diff --git a/src/tools/rust-analyzer/crates/ide-db/src/items_locator.rs b/src/tools/rust-analyzer/crates/ide-db/src/items_locator.rs index 7f66ea0c103f4..d6dcf7147fb56 100644 --- a/src/tools/rust-analyzer/crates/ide-db/src/items_locator.rs +++ b/src/tools/rust-analyzer/crates/ide-db/src/items_locator.rs @@ -108,6 +108,7 @@ pub fn items_with_name_in_module<'a>( } }; let mut local_results = Vec::new(); + // FIXME: This using module_symbols is likely wrong? local_query.search(&[sema.db.module_symbols(module)], |local_candidate| { local_results.push(match local_candidate.def { hir::ModuleDef::Macro(macro_def) => ItemInNs::Macros(macro_def), diff --git a/src/tools/rust-analyzer/crates/ide-db/src/symbol_index.rs b/src/tools/rust-analyzer/crates/ide-db/src/symbol_index.rs index 94d354d28e51e..864011d09c02a 100644 --- a/src/tools/rust-analyzer/crates/ide-db/src/symbol_index.rs +++ b/src/tools/rust-analyzer/crates/ide-db/src/symbol_index.rs @@ -476,9 +476,9 @@ use Macro as ItemLikeMacro; use Macro as Trait; // overlay namespaces //- /b_mod.rs struct StructInModB; -use super::Macro as SuperItemLikeMacro; -use crate::b_mod::StructInModB as ThisStruct; -use crate::Trait as IsThisJustATrait; +pub(self) use super::Macro as SuperItemLikeMacro; +pub(self) use crate::b_mod::StructInModB as ThisStruct; +pub(self) use crate::Trait as IsThisJustATrait; "#, ); diff --git a/src/tools/rust-analyzer/crates/ide-db/src/test_data/test_symbol_index_collection.txt b/src/tools/rust-analyzer/crates/ide-db/src/test_data/test_symbol_index_collection.txt index 9d70942199c8e..535777dfcbea6 100644 --- a/src/tools/rust-analyzer/crates/ide-db/src/test_data/test_symbol_index_collection.txt +++ b/src/tools/rust-analyzer/crates/ide-db/src/test_data/test_symbol_index_collection.txt @@ -631,7 +631,7 @@ def: Function( Function { id: FunctionId( - 3, + 2, ), }, ), @@ -664,7 +664,7 @@ def: Function( Function { id: FunctionId( - 2, + 1, ), }, ), @@ -794,7 +794,7 @@ def: Function( Function { id: FunctionId( - 1, + 3, ), }, ), @@ -879,12 +879,10 @@ [ FileSymbol { name: "IsThisJustATrait", - def: Macro( - Macro { - id: Macro2Id( - Macro2Id( - 0, - ), + def: Trait( + Trait { + id: TraitId( + 0, ), }, ), @@ -897,12 +895,12 @@ ), ptr: SyntaxNodePtr { kind: USE_TREE, - range: 111..143, + range: 141..173, }, name_ptr: AstPtr( SyntaxNodePtr { kind: NAME, - range: 127..143, + range: 157..173, }, ), }, @@ -911,40 +909,7 @@ is_assoc: false, }, FileSymbol { - name: "StructInModB", - def: Adt( - Struct( - Struct { - id: StructId( - 4, - ), - }, - ), - ), - loc: DeclarationLocation { - hir_file_id: EditionedFileId( - FileId( - 1, - ), - Edition2021, - ), - ptr: SyntaxNodePtr { - kind: STRUCT, - range: 0..20, - }, - name_ptr: AstPtr( - SyntaxNodePtr { - kind: NAME, - range: 7..19, - }, - ), - }, - container_name: None, - is_alias: false, - is_assoc: false, - }, - FileSymbol { - name: "SuperItemLikeMacro", + name: "IsThisJustATrait", def: Macro( Macro { id: Macro2Id( @@ -963,12 +928,12 @@ ), ptr: SyntaxNodePtr { kind: USE_TREE, - range: 25..59, + range: 141..173, }, name_ptr: AstPtr( SyntaxNodePtr { kind: NAME, - range: 41..59, + range: 157..173, }, ), }, @@ -977,7 +942,7 @@ is_assoc: false, }, FileSymbol { - name: "ThisStruct", + name: "StructInModB", def: Adt( Struct( Struct { @@ -995,13 +960,13 @@ Edition2021, ), ptr: SyntaxNodePtr { - kind: USE_TREE, - range: 65..105, + kind: STRUCT, + range: 0..20, }, name_ptr: AstPtr( SyntaxNodePtr { kind: NAME, - range: 95..105, + range: 7..19, }, ), }, @@ -1010,15 +975,15 @@ is_assoc: false, }, FileSymbol { - name: "ThisStruct", - def: Adt( - Struct( - Struct { - id: StructId( - 4, + name: "SuperItemLikeMacro", + def: Macro( + Macro { + id: Macro2Id( + Macro2Id( + 0, ), - }, - ), + ), + }, ), loc: DeclarationLocation { hir_file_id: EditionedFileId( @@ -1029,12 +994,12 @@ ), ptr: SyntaxNodePtr { kind: USE_TREE, - range: 65..105, + range: 35..69, }, name_ptr: AstPtr( SyntaxNodePtr { kind: NAME, - range: 95..105, + range: 51..69, }, ), }, From 70e93ed8988d050f13b25f2e956984fa36ea98be Mon Sep 17 00:00:00 2001 From: Lukas Wirth Date: Sat, 18 Jan 2025 10:03:32 +0100 Subject: [PATCH 2/5] Vec -> Box<[_]> --- src/tools/rust-analyzer/crates/hir/src/symbols.rs | 4 ++-- .../rust-analyzer/crates/ide-db/src/symbol_index.rs | 11 ++++------- 2 files changed, 6 insertions(+), 9 deletions(-) diff --git a/src/tools/rust-analyzer/crates/hir/src/symbols.rs b/src/tools/rust-analyzer/crates/hir/src/symbols.rs index 9feaa0b7da508..9b165d1655d34 100644 --- a/src/tools/rust-analyzer/crates/hir/src/symbols.rs +++ b/src/tools/rust-analyzer/crates/hir/src/symbols.rs @@ -92,11 +92,11 @@ impl<'a> SymbolCollector<'a> { } } - pub fn finish(self) -> Vec { + pub fn finish(self) -> Box<[FileSymbol]> { self.symbols.into_iter().collect() } - pub fn collect_module(db: &dyn HirDatabase, module: Module) -> Vec { + pub fn collect_module(db: &dyn HirDatabase, module: Module) -> Box<[FileSymbol]> { let mut symbol_collector = SymbolCollector::new(db); symbol_collector.collect(module); symbol_collector.finish() diff --git a/src/tools/rust-analyzer/crates/ide-db/src/symbol_index.rs b/src/tools/rust-analyzer/crates/ide-db/src/symbol_index.rs index 864011d09c02a..3af536dff50a4 100644 --- a/src/tools/rust-analyzer/crates/ide-db/src/symbol_index.rs +++ b/src/tools/rust-analyzer/crates/ide-db/src/symbol_index.rs @@ -136,16 +136,13 @@ fn library_symbols(db: &dyn SymbolsDatabase, source_root_id: SourceRootId) -> Ar // the module or crate indices for those in salsa unless we need to. .for_each(|module| symbol_collector.collect(module)); - let mut symbols = symbol_collector.finish(); - symbols.shrink_to_fit(); - Arc::new(SymbolIndex::new(symbols)) + Arc::new(SymbolIndex::new(symbol_collector.finish())) } fn module_symbols(db: &dyn SymbolsDatabase, module: Module) -> Arc { let _p = tracing::info_span!("module_symbols").entered(); - let symbols = SymbolCollector::collect_module(db.upcast(), module); - Arc::new(SymbolIndex::new(symbols)) + Arc::new(SymbolIndex::new(SymbolCollector::collect_module(db.upcast(), module))) } pub fn crate_symbols(db: &dyn SymbolsDatabase, krate: Crate) -> Box<[Arc]> { @@ -228,7 +225,7 @@ pub fn world_symbols(db: &RootDatabase, query: Query) -> Vec { #[derive(Default)] pub struct SymbolIndex { - symbols: Vec, + symbols: Box<[FileSymbol]>, map: fst::Map>, } @@ -253,7 +250,7 @@ impl Hash for SymbolIndex { } impl SymbolIndex { - fn new(mut symbols: Vec) -> 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()); From dbf7ccc889f223e1d9e3e27242c875a4f6fcd4ab Mon Sep 17 00:00:00 2001 From: Lukas Wirth Date: Sat, 18 Jan 2025 10:23:00 +0100 Subject: [PATCH 3/5] Preserve impl assoc names in ImplData --- .../rust-analyzer/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 +++---- .../rust-analyzer/crates/hir-ty/src/tests.rs | 2 +- src/tools/rust-analyzer/crates/hir/src/lib.rs | 27 ++----- .../hir/src/semantics/child_by_source.rs | 2 +- .../rust-analyzer/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/src/tools/rust-analyzer/crates/hir-def/src/data.rs b/src/tools/rust-analyzer/crates/hir-def/src/data.rs index d85bc9a4320f9..12f5f6ad79abe 100644 --- a/src/tools/rust-analyzer/crates/hir-def/src/data.rs +++ b/src/tools/rust-analyzer/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/src/tools/rust-analyzer/crates/hir-def/src/lang_item.rs b/src/tools/rust-analyzer/crates/hir-def/src/lang_item.rs index 0629d87e5444f..afdc49a2dc59a 100644 --- a/src/tools/rust-analyzer/crates/hir-def/src/lang_item.rs +++ b/src/tools/rust-analyzer/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/src/tools/rust-analyzer/crates/hir-ty/src/chalk_db.rs b/src/tools/rust-analyzer/crates/hir-ty/src/chalk_db.rs index 9f01f1eb25900..c8ff6cba3dd18 100644 --- a/src/tools/rust-analyzer/crates/hir-ty/src/chalk_db.rs +++ b/src/tools/rust-analyzer/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/src/tools/rust-analyzer/crates/hir-ty/src/method_resolution.rs b/src/tools/rust-analyzer/crates/hir-ty/src/method_resolution.rs index 62b071b2f3264..182032f04812d 100644 --- a/src/tools/rust-analyzer/crates/hir-ty/src/method_resolution.rs +++ b/src/tools/rust-analyzer/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/src/tools/rust-analyzer/crates/hir-ty/src/tests.rs b/src/tools/rust-analyzer/crates/hir-ty/src/tests.rs index acdcbb4757d7e..00da9b251768e 100644 --- a/src/tools/rust-analyzer/crates/hir-ty/src/tests.rs +++ b/src/tools/rust-analyzer/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/src/tools/rust-analyzer/crates/hir/src/lib.rs b/src/tools/rust-analyzer/crates/hir/src/lib.rs index efa88d7c83eec..db3121d3cd35f 100644 --- a/src/tools/rust-analyzer/crates/hir/src/lib.rs +++ b/src/tools/rust-analyzer/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/src/tools/rust-analyzer/crates/hir/src/semantics/child_by_source.rs b/src/tools/rust-analyzer/crates/hir/src/semantics/child_by_source.rs index ec65ea9a9a8b6..d5dfb98571864 100644 --- a/src/tools/rust-analyzer/crates/hir/src/semantics/child_by_source.rs +++ b/src/tools/rust-analyzer/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/src/tools/rust-analyzer/crates/hir/src/symbols.rs b/src/tools/rust-analyzer/crates/hir/src/symbols.rs index 9b165d1655d34..a6b8ed70c363a 100644 --- a/src/tools/rust-analyzer/crates/hir/src/symbols.rs +++ b/src/tools/rust-analyzer/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/src/tools/rust-analyzer/crates/ide-db/src/symbol_index.rs b/src/tools/rust-analyzer/crates/ide-db/src/symbol_index.rs index 3af536dff50a4..79f89a89a1ab9 100644 --- a/src/tools/rust-analyzer/crates/ide-db/src/symbol_index.rs +++ b/src/tools/rust-analyzer/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/src/tools/rust-analyzer/crates/ide/src/navigation_target.rs b/src/tools/rust-analyzer/crates/ide/src/navigation_target.rs index 702f274ae3e63..d9f80cb53dd6b 100644 --- a/src/tools/rust-analyzer/crates/ide/src/navigation_target.rs +++ b/src/tools/rust-analyzer/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, From 7ddeabaa74ab421fef281671e5cc3a5ceb615b1d Mon Sep 17 00:00:00 2001 From: Lukas Wirth Date: Sat, 18 Jan 2025 12:02:34 +0100 Subject: [PATCH 4/5] Less allocs --- .../ide-db/src/imports/import_assets.rs | 8 ++++++- .../crates/ide-db/src/items_locator.rs | 21 ++++++++++--------- .../crates/ide-db/src/symbol_index.rs | 20 +++++++++++------- 3 files changed, 30 insertions(+), 19 deletions(-) diff --git a/src/tools/rust-analyzer/crates/ide-db/src/imports/import_assets.rs b/src/tools/rust-analyzer/crates/ide-db/src/imports/import_assets.rs index 3ed101ff36a84..573018f9ccc08 100644 --- a/src/tools/rust-analyzer/crates/ide-db/src/imports/import_assets.rs +++ b/src/tools/rust-analyzer/crates/ide-db/src/imports/import_assets.rs @@ -1,5 +1,7 @@ //! Look up accessible paths for items. +use std::ops::ControlFlow; + use hir::{ db::HirDatabase, AsAssocItem, AssocItem, AssocItemContainer, Crate, HasCrate, ImportPathConfig, ItemInNs, ModPath, Module, ModuleDef, PathResolution, PrefixKind, ScopeDef, Semantics, @@ -353,6 +355,7 @@ fn path_applicable_imports( // we have some unresolved qualifier that we search an import for // The key here is that whatever we import must form a resolved path for the remainder of // what follows + // FIXME: This doesn't handle visibility [first_qsegment, qualifier_rest @ ..] => items_locator::items_with_name( sema, current_crate, @@ -417,8 +420,11 @@ fn validate_resolvable( module, candidate.clone(), AssocSearchMode::Exclude, + |it| match scope_filter(it) { + true => ControlFlow::Break(it), + false => ControlFlow::Continue(()), + }, ) - .find(|&it| scope_filter(it)) .map(|item| LocatedImport::new(import_path_candidate, resolved_qualifier, item)) } // FIXME diff --git a/src/tools/rust-analyzer/crates/ide-db/src/items_locator.rs b/src/tools/rust-analyzer/crates/ide-db/src/items_locator.rs index d6dcf7147fb56..405d2d91d8e0d 100644 --- a/src/tools/rust-analyzer/crates/ide-db/src/items_locator.rs +++ b/src/tools/rust-analyzer/crates/ide-db/src/items_locator.rs @@ -2,6 +2,8 @@ //! by its name and a few criteria. //! The main reason for this module to exist is the fact that project's items and dependencies' items //! are located in different caches, with different APIs. +use std::ops::ControlFlow; + use either::Either; use hir::{import_map, Crate, ItemInNs, Module, Semantics}; use limit::Limit; @@ -17,6 +19,7 @@ pub static DEFAULT_QUERY_SEARCH_LIMIT: Limit = Limit::new(100); pub use import_map::AssocSearchMode; +// FIXME: Do callbacks instead to avoid allocations. /// Searches for importable items with the given name in the crate and its dependencies. pub fn items_with_name<'a>( sema: &'a Semantics<'_, RootDatabase>, @@ -70,12 +73,13 @@ pub fn items_with_name<'a>( } /// Searches for importable items with the given name in the crate and its dependencies. -pub fn items_with_name_in_module<'a>( - sema: &'a Semantics<'_, RootDatabase>, +pub fn items_with_name_in_module( + sema: &Semantics<'_, RootDatabase>, module: Module, name: NameToImport, assoc_item_search: AssocSearchMode, -) -> impl Iterator + 'a { + mut cb: impl FnMut(ItemInNs) -> ControlFlow, +) -> Option { let _p = tracing::info_span!("items_with_name_in", name = name.text(), assoc_item_search = ?assoc_item_search, ?module) .entered(); @@ -107,15 +111,12 @@ pub fn items_with_name_in_module<'a>( local_query } }; - let mut local_results = Vec::new(); - // FIXME: This using module_symbols is likely wrong? local_query.search(&[sema.db.module_symbols(module)], |local_candidate| { - local_results.push(match local_candidate.def { + cb(match local_candidate.def { hir::ModuleDef::Macro(macro_def) => ItemInNs::Macros(macro_def), def => ItemInNs::from(def), }) - }); - local_results.into_iter() + }) } fn find_items<'a>( @@ -140,10 +141,10 @@ fn find_items<'a>( // Query the local crate using the symbol index. let mut local_results = Vec::new(); local_query.search(&symbol_index::crate_symbols(db, krate), |local_candidate| { - local_results.push(match local_candidate.def { + ControlFlow::<()>::Continue(local_results.push(match local_candidate.def { hir::ModuleDef::Macro(macro_def) => ItemInNs::Macros(macro_def), def => ItemInNs::from(def), - }) + })) }); local_results.into_iter().chain(external_importables) } diff --git a/src/tools/rust-analyzer/crates/ide-db/src/symbol_index.rs b/src/tools/rust-analyzer/crates/ide-db/src/symbol_index.rs index 79f89a89a1ab9..738db95133115 100644 --- a/src/tools/rust-analyzer/crates/ide-db/src/symbol_index.rs +++ b/src/tools/rust-analyzer/crates/ide-db/src/symbol_index.rs @@ -25,6 +25,7 @@ use std::{ fmt, hash::{Hash, Hasher}, mem, + ops::ControlFlow, }; use base_db::{ @@ -219,7 +220,7 @@ pub fn world_symbols(db: &RootDatabase, query: Query) -> Vec { }; let mut res = vec![]; - query.search(&indices, |f| res.push(f.clone())); + query.search::<()>(&indices, |f| ControlFlow::Continue(res.push(f.clone()))); res } @@ -313,11 +314,11 @@ impl SymbolIndex { } impl Query { - pub(crate) fn search<'sym>( + pub(crate) fn search<'sym, T>( self, indices: &'sym [Arc], - cb: impl FnMut(&'sym FileSymbol), - ) { + cb: impl FnMut(&'sym FileSymbol) -> ControlFlow, + ) -> Option { let _p = tracing::info_span!("symbol_index::Query::search").entered(); let mut op = fst::map::OpBuilder::new(); match self.mode { @@ -348,12 +349,12 @@ impl Query { } } - fn search_maps<'sym>( + fn search_maps<'sym, T>( &self, indices: &'sym [Arc], mut stream: fst::map::Union<'_>, - mut cb: impl FnMut(&'sym FileSymbol), - ) { + mut cb: impl FnMut(&'sym FileSymbol) -> ControlFlow, + ) -> Option { let ignore_underscore_prefixed = !self.query.starts_with("__"); while let Some((_, indexed_values)) = stream.next() { for &IndexedValue { index, value } in indexed_values { @@ -379,11 +380,14 @@ impl Query { continue; } if self.mode.check(&self.query, self.case_sensitive, symbol_name) { - cb(symbol); + if let Some(b) = cb(symbol).break_value() { + return Some(b); + } } } } } + None } fn matches_assoc_mode(&self, is_trait_assoc_item: bool) -> bool { From 4193abc3fec86f5d29a84d5e41120923637badf3 Mon Sep 17 00:00:00 2001 From: Lukas Wirth Date: Mon, 20 Jan 2025 14:21:37 +0100 Subject: [PATCH 5/5] Fix import search not discarding rawness --- src/tools/rust-analyzer/Cargo.toml | 2 +- .../crates/hir-expand/src/name.rs | 40 +++++++++++++++---- .../rust-analyzer/crates/hir/src/semantics.rs | 11 ++--- .../src/completions/flyimport.rs | 9 +++-- .../crates/ide-completion/src/render.rs | 2 +- .../ide-db/src/imports/import_assets.rs | 20 +++++++--- .../crates/ide-db/src/items_locator.rs | 5 ++- .../crates/ide-db/src/symbol_index.rs | 5 ++- 8 files changed, 65 insertions(+), 29 deletions(-) diff --git a/src/tools/rust-analyzer/Cargo.toml b/src/tools/rust-analyzer/Cargo.toml index 3cecae0dc4ab0..1029844cd3ab4 100644 --- a/src/tools/rust-analyzer/Cargo.toml +++ b/src/tools/rust-analyzer/Cargo.toml @@ -4,7 +4,7 @@ exclude = ["crates/proc-macro-srv/proc-macro-test/imp"] resolver = "2" [workspace.package] -rust-version = "1.82" +rust-version = "1.83" edition = "2021" license = "MIT OR Apache-2.0" authors = ["rust-analyzer team"] diff --git a/src/tools/rust-analyzer/crates/hir-expand/src/name.rs b/src/tools/rust-analyzer/crates/hir-expand/src/name.rs index 54a61a096d704..cc53d2e34aacb 100644 --- a/src/tools/rust-analyzer/crates/hir-expand/src/name.rs +++ b/src/tools/rust-analyzer/crates/hir-expand/src/name.rs @@ -11,7 +11,7 @@ use syntax::utils::is_raw_identifier; /// and declarations. In theory, names should also carry hygiene info, but we are /// not there yet! /// -/// Note that the rawness (`r#`) of names does not depend on whether they are written raw. +/// Note that the rawness (`r#`) of names is not preserved. Names are always stored without a `r#` prefix. /// This is because we want to show (in completions etc.) names as raw depending on the needs /// of the current crate, for example if it is edition 2021 complete `gen` even if the defining /// crate is in edition 2024 and wrote `r#gen`, and the opposite holds as well. @@ -77,6 +77,7 @@ impl Name { /// Hopefully, this should allow us to integrate hygiene cleaner in the /// future, and to switch to interned representation of names. fn new_text(text: &str) -> Name { + debug_assert!(!text.starts_with("r#")); Name { symbol: Symbol::intern(text), ctx: () } } @@ -91,15 +92,34 @@ impl Name { pub fn new_root(text: &str) -> Name { // The edition doesn't matter for hygiene. - Self::new(text, SyntaxContextId::root(Edition::Edition2015)) + Self::new(text.trim_start_matches("r#"), SyntaxContextId::root(Edition::Edition2015)) } pub fn new_tuple_field(idx: usize) -> Name { - Name { symbol: Symbol::intern(&idx.to_string()), ctx: () } + let symbol = match idx { + 0 => sym::INTEGER_0.clone(), + 1 => sym::INTEGER_1.clone(), + 2 => sym::INTEGER_2.clone(), + 3 => sym::INTEGER_3.clone(), + 4 => sym::INTEGER_4.clone(), + 5 => sym::INTEGER_5.clone(), + 6 => sym::INTEGER_6.clone(), + 7 => sym::INTEGER_7.clone(), + 8 => sym::INTEGER_8.clone(), + 9 => sym::INTEGER_9.clone(), + 10 => sym::INTEGER_10.clone(), + 11 => sym::INTEGER_11.clone(), + 12 => sym::INTEGER_12.clone(), + 13 => sym::INTEGER_13.clone(), + 14 => sym::INTEGER_14.clone(), + 15 => sym::INTEGER_15.clone(), + _ => Symbol::intern(&idx.to_string()), + }; + Name { symbol, ctx: () } } pub fn new_lifetime(lt: &ast::Lifetime) -> Name { - Name { symbol: Symbol::intern(lt.text().as_str()), ctx: () } + Self::new_text(lt.text().as_str().trim_start_matches("r#")) } /// Resolve a name from the text of token. @@ -142,15 +162,18 @@ impl Name { } /// Returns the text this name represents if it isn't a tuple field. + /// + /// Do not use this for user-facing text, use `display` instead to handle editions properly. pub fn as_str(&self) -> &str { self.symbol.as_str() } + // FIXME: Remove this pub fn unescaped(&self) -> UnescapedName<'_> { UnescapedName(self) } - pub fn is_escaped(&self, edition: Edition) -> bool { + pub fn needs_escape(&self, edition: Edition) -> bool { is_raw_identifier(self.symbol.as_str(), edition) } @@ -173,16 +196,19 @@ impl Name { &self.symbol } - pub const fn new_symbol(symbol: Symbol, ctx: SyntaxContextId) -> Self { + pub fn new_symbol(symbol: Symbol, ctx: SyntaxContextId) -> Self { + debug_assert!(!symbol.as_str().starts_with("r#")); _ = ctx; Self { symbol, ctx: () } } // FIXME: This needs to go once we have hygiene - pub const fn new_symbol_root(sym: Symbol) -> Self { + pub fn new_symbol_root(sym: Symbol) -> Self { + debug_assert!(!sym.as_str().starts_with("r#")); Self { symbol: sym, ctx: () } } + // FIXME: Remove this #[inline] pub fn eq_ident(&self, ident: &str) -> bool { self.as_str() == ident.trim_start_matches("r#") diff --git a/src/tools/rust-analyzer/crates/hir/src/semantics.rs b/src/tools/rust-analyzer/crates/hir/src/semantics.rs index 92f6c4b5ab3aa..af98e5f2fd031 100644 --- a/src/tools/rust-analyzer/crates/hir/src/semantics.rs +++ b/src/tools/rust-analyzer/crates/hir/src/semantics.rs @@ -39,8 +39,8 @@ use stdx::TupleExt; use syntax::{ algo::skip_trivia_token, ast::{self, HasAttrs as _, HasGenericParams}, - AstNode, AstToken, Direction, SmolStr, SyntaxKind, SyntaxNode, SyntaxNodePtr, SyntaxToken, - TextRange, TextSize, + AstNode, AstToken, Direction, SyntaxKind, SyntaxNode, SyntaxNodePtr, SyntaxToken, TextRange, + TextSize, }; use triomphe::Arc; @@ -1587,14 +1587,11 @@ impl<'db> SemanticsImpl<'db> { pub fn resolve_mod_path_relative( &self, to: Module, - segments: impl IntoIterator, + segments: impl IntoIterator, ) -> Option> { let items = to.id.resolver(self.db.upcast()).resolve_module_path_in_items( self.db.upcast(), - &ModPath::from_segments( - hir_def::path::PathKind::Plain, - segments.into_iter().map(|it| Name::new_root(&it)), - ), + &ModPath::from_segments(hir_def::path::PathKind::Plain, segments), ); Some(items.iter_items().map(|(item, _)| item.into())) } diff --git a/src/tools/rust-analyzer/crates/ide-completion/src/completions/flyimport.rs b/src/tools/rust-analyzer/crates/ide-completion/src/completions/flyimport.rs index 3b2b2fd706e18..73313eeaa6b79 100644 --- a/src/tools/rust-analyzer/crates/ide-completion/src/completions/flyimport.rs +++ b/src/tools/rust-analyzer/crates/ide-completion/src/completions/flyimport.rs @@ -5,7 +5,7 @@ use ide_db::imports::{ insert_use::ImportScope, }; use itertools::Itertools; -use syntax::{ast, AstNode, SyntaxNode, ToSmolStr, T}; +use syntax::{ast, AstNode, SyntaxNode, ToSmolStr}; use crate::{ config::AutoImportExclusionType, @@ -403,10 +403,11 @@ fn import_on_the_fly_method( fn import_name(ctx: &CompletionContext<'_>) -> String { let token_kind = ctx.token.kind(); - if matches!(token_kind, T![.] | T![::]) { - String::new() - } else { + + if token_kind.is_any_identifier() { ctx.token.to_string() + } else { + String::new() } } diff --git a/src/tools/rust-analyzer/crates/ide-completion/src/render.rs b/src/tools/rust-analyzer/crates/ide-completion/src/render.rs index 7fee2240908de..61e8114d381aa 100644 --- a/src/tools/rust-analyzer/crates/ide-completion/src/render.rs +++ b/src/tools/rust-analyzer/crates/ide-completion/src/render.rs @@ -423,7 +423,7 @@ fn render_resolution_path( let name = local_name.display_no_db(ctx.completion.edition).to_smolstr(); let mut item = render_resolution_simple_(ctx, &local_name, import_to_add, resolution); - if local_name.is_escaped(completion.edition) { + if local_name.needs_escape(completion.edition) { item.insert_text(local_name.display_no_db(completion.edition).to_smolstr()); } // Add `<>` for generic types diff --git a/src/tools/rust-analyzer/crates/ide-db/src/imports/import_assets.rs b/src/tools/rust-analyzer/crates/ide-db/src/imports/import_assets.rs index 573018f9ccc08..ad86d855b553e 100644 --- a/src/tools/rust-analyzer/crates/ide-db/src/imports/import_assets.rs +++ b/src/tools/rust-analyzer/crates/ide-db/src/imports/import_assets.rs @@ -4,14 +4,14 @@ use std::ops::ControlFlow; use hir::{ db::HirDatabase, AsAssocItem, AssocItem, AssocItemContainer, Crate, HasCrate, ImportPathConfig, - ItemInNs, ModPath, Module, ModuleDef, PathResolution, PrefixKind, ScopeDef, Semantics, + ItemInNs, ModPath, Module, ModuleDef, Name, PathResolution, PrefixKind, ScopeDef, Semantics, SemanticsScope, Trait, TyFingerprint, Type, }; use itertools::Itertools; use rustc_hash::{FxHashMap, FxHashSet}; use syntax::{ ast::{self, make, HasName}, - AstNode, SmolStr, SyntaxNode, + AstNode, SyntaxNode, }; use crate::{ @@ -53,7 +53,7 @@ pub struct TraitImportCandidate { #[derive(Debug)] pub struct PathImportCandidate { /// Optional qualifier before name. - pub qualifier: Vec, + pub qualifier: Vec, /// The name the item (struct, trait, enum, etc.) should have. pub name: NameToImport, } @@ -72,10 +72,18 @@ pub enum NameToImport { impl NameToImport { pub fn exact_case_sensitive(s: String) -> NameToImport { + let s = match s.strip_prefix("r#") { + Some(s) => s.to_owned(), + None => s, + }; NameToImport::Exact(s, true) } pub fn fuzzy(s: String) -> NameToImport { + let s = match s.strip_prefix("r#") { + Some(s) => s.to_owned(), + None => s, + }; // unless all chars are lowercase, we do a case sensitive search let case_sensitive = s.chars().any(|c| c.is_uppercase()); NameToImport::Fuzzy(s, case_sensitive) @@ -359,7 +367,7 @@ fn path_applicable_imports( [first_qsegment, qualifier_rest @ ..] => items_locator::items_with_name( sema, current_crate, - NameToImport::Exact(first_qsegment.to_string(), true), + NameToImport::Exact(first_qsegment.as_str().to_owned(), true), AssocSearchMode::Exclude, ) .filter_map(|item| { @@ -389,7 +397,7 @@ fn validate_resolvable( scope_filter: impl Fn(ItemInNs) -> bool, candidate: &NameToImport, resolved_qualifier: ItemInNs, - unresolved_qualifier: &[SmolStr], + unresolved_qualifier: &[Name], ) -> Option { let _p = tracing::info_span!("ImportAssets::import_for_item").entered(); @@ -722,7 +730,7 @@ fn path_import_candidate( if qualifier.first_qualifier().is_none_or(|it| sema.resolve_path(&it).is_none()) { let qualifier = qualifier .segments() - .map(|seg| seg.name_ref().map(|name| SmolStr::new(name.text()))) + .map(|seg| seg.name_ref().map(|name| Name::new_root(&name.text()))) .collect::>>()?; ImportCandidate::Path(PathImportCandidate { qualifier, name }) } else { diff --git a/src/tools/rust-analyzer/crates/ide-db/src/items_locator.rs b/src/tools/rust-analyzer/crates/ide-db/src/items_locator.rs index 405d2d91d8e0d..a2062f36d3fd3 100644 --- a/src/tools/rust-analyzer/crates/ide-db/src/items_locator.rs +++ b/src/tools/rust-analyzer/crates/ide-db/src/items_locator.rs @@ -141,10 +141,11 @@ fn find_items<'a>( // Query the local crate using the symbol index. let mut local_results = Vec::new(); local_query.search(&symbol_index::crate_symbols(db, krate), |local_candidate| { - ControlFlow::<()>::Continue(local_results.push(match local_candidate.def { + local_results.push(match local_candidate.def { hir::ModuleDef::Macro(macro_def) => ItemInNs::Macros(macro_def), def => ItemInNs::from(def), - })) + }); + ControlFlow::<()>::Continue(()) }); local_results.into_iter().chain(external_importables) } diff --git a/src/tools/rust-analyzer/crates/ide-db/src/symbol_index.rs b/src/tools/rust-analyzer/crates/ide-db/src/symbol_index.rs index 738db95133115..c94644eeb89be 100644 --- a/src/tools/rust-analyzer/crates/ide-db/src/symbol_index.rs +++ b/src/tools/rust-analyzer/crates/ide-db/src/symbol_index.rs @@ -220,7 +220,10 @@ pub fn world_symbols(db: &RootDatabase, query: Query) -> Vec { }; let mut res = vec![]; - query.search::<()>(&indices, |f| ControlFlow::Continue(res.push(f.clone()))); + query.search::<()>(&indices, |f| { + res.push(f.clone()); + ControlFlow::Continue(()) + }); res }