From 21eae58269b303898ef407369d09d4245c66036f Mon Sep 17 00:00:00 2001 From: Travis Cross Date: Fri, 9 Aug 2024 21:12:46 +0000 Subject: [PATCH] Revert "Optimize `find_path` for sysroot library search some more" This reverts commit 733cb1e645708faf3742991bf39a7ba4a3938387. Reverting this commit fixes the behavior of [`rust-analyzer.imports.preferNoStd`][]. This fixes #17840. [`rust-analyzer.imports.preferNoStd`]: https://rust-analyzer.github.io/manual.html#rust-analyzer.imports.preferNoStd --- crates/hir-def/src/find_path.rs | 208 +++++++++++++++----------------- 1 file changed, 97 insertions(+), 111 deletions(-) diff --git a/crates/hir-def/src/find_path.rs b/crates/hir-def/src/find_path.rs index 91594aecd044..87aa7abdbd59 100644 --- a/crates/hir-def/src/find_path.rs +++ b/crates/hir-def/src/find_path.rs @@ -1,6 +1,6 @@ //! An algorithm to find a path to refer to a certain item. -use std::{cell::Cell, cmp::Ordering, iter}; +use std::{cell::Cell, cmp::Ordering, iter, ops::BitOr}; use base_db::{CrateId, CrateOrigin, LangCrateOrigin}; use hir_expand::{ @@ -52,11 +52,11 @@ pub fn find_path( ignore_local_imports, from, from_def_map: &from.def_map(db), + is_std_item: db.crate_graph()[item_module.krate()].origin.is_lang(), fuel: Cell::new(FIND_PATH_FUEL), }, item, MAX_PATH_LEN, - db.crate_graph()[item_module.krate()].origin.is_lang(), ) } @@ -67,6 +67,13 @@ enum Stability { } use Stability::*; +fn zip_stability(a: Stability, b: Stability) -> Stability { + match (a, b) { + (Stable, Stable) => Stable, + _ => Unstable, + } +} + const MAX_PATH_LEN: usize = 15; const FIND_PATH_FUEL: usize = 10000; @@ -100,22 +107,17 @@ struct FindPathCtx<'db> { ignore_local_imports: bool, from: ModuleId, from_def_map: &'db DefMap, + is_std_item: bool, fuel: Cell, } /// Attempts to find a path to refer to the given `item` visible from the `from` ModuleId -fn find_path_inner( - ctx: &FindPathCtx<'_>, - item: ItemInNs, - max_len: usize, - is_std_item: bool, -) -> Option { +fn find_path_inner(ctx: &FindPathCtx<'_>, item: ItemInNs, max_len: usize) -> Option { // - if the item is a module, jump straight to module search - if !is_std_item { - if let ItemInNs::Types(ModuleDefId::ModuleId(module_id)) = item { - return find_path_for_module(ctx, &mut FxHashSet::default(), module_id, true, max_len) - .map(|choice| choice.path); - } + if let ItemInNs::Types(ModuleDefId::ModuleId(module_id)) = item { + let mut visited_modules = FxHashSet::default(); + return find_path_for_module(ctx, &mut visited_modules, module_id, true, max_len) + .map(|choice| choice.path); } let may_be_in_scope = match ctx.prefix { @@ -138,12 +140,9 @@ fn find_path_inner( if let Some(ModuleDefId::EnumVariantId(variant)) = item.as_module_def_id() { // - if the item is an enum variant, refer to it via the enum - if let Some(mut path) = find_path_inner( - ctx, - ItemInNs::Types(variant.lookup(ctx.db).parent.into()), - max_len, - is_std_item, - ) { + if let Some(mut path) = + find_path_inner(ctx, ItemInNs::Types(variant.lookup(ctx.db).parent.into()), max_len) + { path.push_segment(ctx.db.enum_variant_data(variant).name.clone()); return Some(path); } @@ -152,18 +151,10 @@ fn find_path_inner( // variant somewhere } - if is_std_item { - // The item we are searching for comes from the sysroot libraries, so skip prefer looking in - // the sysroot libraries directly. - // We do need to fallback as the item in question could be re-exported by another crate - // while not being a transitive dependency of the current crate. - if let Some(choice) = find_in_sysroot(ctx, &mut FxHashSet::default(), item, max_len) { - return Some(choice.path); - } - } + let mut visited_modules = FxHashSet::default(); let mut best_choice = None; - calculate_best_path(ctx, &mut FxHashSet::default(), item, max_len, &mut best_choice); + calculate_best_path(ctx, &mut visited_modules, item, max_len, &mut best_choice); best_choice.map(|choice| choice.path) } @@ -187,6 +178,7 @@ fn find_path_for_module( path_text_len: 5, stability: Stable, prefer_due_to_prelude: false, + sysroot_score: 0, }); } // - otherwise if the item is the crate root of a dependency crate, return the name from the extern prelude @@ -249,6 +241,7 @@ fn find_path_for_module( path_text_len: path_kind_len(kind), stability: Stable, prefer_due_to_prelude: false, + sysroot_score: 0, }); } } @@ -367,97 +360,86 @@ fn calculate_best_path( // dependency in this case. calculate_best_path_local(ctx, visited_modules, item, max_len, best_choice) } else { + let db = ctx.db; // Item was defined in some upstream crate. This means that it must be exported from one, // too (unless we can't name it at all). It could *also* be (re)exported by the same crate // that wants to import it here, but we always prefer to use the external path here. - ctx.db.crate_graph()[ctx.from.krate].dependencies.iter().for_each(|dep| { - find_in_dep(ctx, visited_modules, item, max_len, best_choice, dep.crate_id) - }); - } -} + let mut process_dep = |dep: CrateId, score| { + let import_map = db.import_map(dep); + let Some(import_info_for) = import_map.import_info_for(item) else { + return false; + }; + let mut processed_something = false; + for info in import_info_for { + if info.is_doc_hidden { + // the item or import is `#[doc(hidden)]`, so skip it as it is in an external crate + continue; + } -fn find_in_sysroot( - ctx: &FindPathCtx<'_>, - visited_modules: &mut FxHashSet<(ItemInNs, ModuleId)>, - item: ItemInNs, - max_len: usize, -) -> Option { - let crate_graph = ctx.db.crate_graph(); - let dependencies = &crate_graph[ctx.from.krate].dependencies; - let mut best_choice = None; - let mut search = |lang, best_choice: &mut _| { - if let Some(dep) = dependencies.iter().filter(|it| it.is_sysroot()).find(|dep| { - match crate_graph[dep.crate_id].origin { - CrateOrigin::Lang(l) => l == lang, - _ => false, + // Determine best path for containing module and append last segment from `info`. + // FIXME: we should guide this to look up the path locally, or from the same crate again? + let choice = find_path_for_module( + ctx, + visited_modules, + info.container, + true, + best_choice.as_ref().map_or(max_len, |it| it.path.len()) - 1, + ); + let Some(mut choice) = choice else { + continue; + }; + choice.sysroot_score = score; + cov_mark::hit!(partially_imported); + choice.stability = zip_stability( + choice.stability, + if info.is_unstable { Unstable } else { Stable }, + ); + + Choice::try_select(best_choice, choice, ctx.cfg.prefer_prelude, info.name.clone()); + processed_something = true; } - }) { - find_in_dep(ctx, visited_modules, item, max_len, best_choice, dep.crate_id); - } - }; - if ctx.cfg.prefer_no_std { - search(LangCrateOrigin::Core, &mut best_choice); - if matches!(best_choice, Some(Choice { stability: Stable, .. })) { - return best_choice; - } - search(LangCrateOrigin::Std, &mut best_choice); - if matches!(best_choice, Some(Choice { stability: Stable, .. })) { - return best_choice; - } - } else { - search(LangCrateOrigin::Std, &mut best_choice); - if matches!(best_choice, Some(Choice { stability: Stable, .. })) { - return best_choice; - } - search(LangCrateOrigin::Core, &mut best_choice); - if matches!(best_choice, Some(Choice { stability: Stable, .. })) { - return best_choice; - } - } - let mut best_choice = None; - dependencies.iter().filter(|it| it.is_sysroot()).for_each(|dep| { - find_in_dep(ctx, visited_modules, item, max_len, &mut best_choice, dep.crate_id); - }); - best_choice -} - -fn find_in_dep( - ctx: &FindPathCtx<'_>, - visited_modules: &mut FxHashSet<(ItemInNs, ModuleId)>, - item: ItemInNs, - max_len: usize, - best_choice: &mut Option, - dep: CrateId, -) { - let import_map = ctx.db.import_map(dep); - let Some(import_info_for) = import_map.import_info_for(item) else { - return; - }; - for info in import_info_for { - if info.is_doc_hidden { - // the item or import is `#[doc(hidden)]`, so skip it as it is in an external crate - continue; - } - - // Determine best path for containing module and append last segment from `info`. - // FIXME: we should guide this to look up the path locally, or from the same crate again? - let choice = find_path_for_module( - ctx, - visited_modules, - info.container, - true, - best_choice.as_ref().map_or(max_len, |it| it.path.len()) - 1, - ); - let Some(mut choice) = choice else { - continue; + processed_something }; - cov_mark::hit!(partially_imported); - if info.is_unstable { - choice.stability = Unstable; + + let crate_graph = db.crate_graph(); + let dependencies = &crate_graph[ctx.from.krate].dependencies; + if ctx.is_std_item { + // The item we are searching for comes from the sysroot libraries, so skip prefer looking in + // the sysroot libraries directly. + // We do need to fallback as the item in question could be re-exported by another crate + // while not being a transitive dependency of the current crate. + let processed = dependencies + .iter() + .filter(|it| it.is_sysroot()) + .map(|dep| { + ( + match crate_graph[dep.crate_id].origin { + CrateOrigin::Lang(LangCrateOrigin::Std) if ctx.cfg.prefer_no_std => 5, + CrateOrigin::Lang(LangCrateOrigin::Std) => 1, + CrateOrigin::Lang(LangCrateOrigin::Alloc) => 2, + CrateOrigin::Lang(LangCrateOrigin::ProcMacro) => 3, + CrateOrigin::Lang(LangCrateOrigin::Test) => 3, + CrateOrigin::Lang(LangCrateOrigin::Core) => 4, + CrateOrigin::Lang(LangCrateOrigin::Other) => 1, + _ => 0, + }, + dep.crate_id, + ) + }) + .map(|(score, crate_id)| process_dep(crate_id, score)) + .reduce(BitOr::bitor) + .unwrap_or(false); + if processed { + // Found a path in a sysroot crate + return; + } } - Choice::try_select(best_choice, choice, ctx.cfg.prefer_prelude, info.name.clone()); + dependencies + .iter() + .filter(|it| !ctx.is_std_item || !it.is_sysroot()) + .for_each(|dep| _ = process_dep(dep.crate_id, 0)); } } @@ -499,6 +481,8 @@ struct Choice { stability: Stability, /// Whether this path contains a prelude segment and preference for it has been signaled prefer_due_to_prelude: bool, + /// non-zero if this is a path in a sysroot crate, we want to choose the highest ranked std crate + sysroot_score: u8, } impl Choice { @@ -507,6 +491,7 @@ impl Choice { path_text_len: path_kind_len(kind) + name.as_str().len(), stability, prefer_due_to_prelude: prefer_prelude && name == sym::prelude, + sysroot_score: 0, path: ModPath::from_segments(kind, iter::once(name)), } } @@ -532,6 +517,7 @@ impl Choice { .stability .cmp(¤t.stability) .then_with(|| other.prefer_due_to_prelude.cmp(¤t.prefer_due_to_prelude)) + .then_with(|| current.sysroot_score.cmp(&other.sysroot_score)) .then_with(|| (current.path.len()).cmp(&(other.path.len() + 1))) { Ordering::Less => return,