From a6e4ac705c845d037159ca6a7b1a37665a1aeaaa Mon Sep 17 00:00:00 2001 From: Lukas Wirth Date: Sun, 21 Jul 2024 10:53:33 +0200 Subject: [PATCH 1/6] Optimize `find_path` choice selection --- crates/hir-def/src/find_path.rs | 267 +++++++++++++++++--------------- 1 file changed, 145 insertions(+), 122 deletions(-) diff --git a/crates/hir-def/src/find_path.rs b/crates/hir-def/src/find_path.rs index fc08003052..0947a62a6a 100644 --- a/crates/hir-def/src/find_path.rs +++ b/crates/hir-def/src/find_path.rs @@ -2,12 +2,12 @@ use std::{cell::Cell, cmp::Ordering, iter, ops::BitOr}; -use base_db::CrateId; +use base_db::{CrateId, CrateOrigin, LangCrateOrigin}; use hir_expand::{ name::{AsName, Name}, Lookup, }; -use intern::{sym, Symbol}; +use intern::sym; use rustc_hash::FxHashSet; use crate::{ @@ -60,7 +60,7 @@ pub fn find_path( ) } -#[derive(Copy, Clone, Debug, PartialEq, Eq)] +#[derive(Copy, Clone, Debug, PartialEq, Eq, PartialOrd, Ord)] enum Stability { Unstable, Stable, @@ -117,7 +117,7 @@ fn find_path_inner(ctx: &FindPathCtx<'_>, item: ItemInNs, max_len: usize) -> Opt 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, max_len) - .map(|(item, _)| item); + .map(|choice| choice.path); } let may_be_in_scope = match ctx.prefix { @@ -135,7 +135,7 @@ fn find_path_inner(ctx: &FindPathCtx<'_>, item: ItemInNs, max_len: usize) -> Opt // - if the item is in the prelude, return the name from there if let Some(value) = find_in_prelude(ctx.db, ctx.from_def_map, item, ctx.from) { - return Some(value); + return Some(value.path); } if let Some(ModuleDefId::EnumVariantId(variant)) = item.as_module_def_id() { @@ -153,7 +153,7 @@ fn find_path_inner(ctx: &FindPathCtx<'_>, item: ItemInNs, max_len: usize) -> Opt let mut visited_modules = FxHashSet::default(); - calculate_best_path(ctx, &mut visited_modules, item, max_len).map(|(item, _)| item) + calculate_best_path(ctx, &mut visited_modules, item, max_len).map(|choice| choice.path) } #[tracing::instrument(skip_all)] @@ -162,11 +162,17 @@ fn find_path_for_module( visited_modules: &mut FxHashSet, module_id: ModuleId, max_len: usize, -) -> Option<(ModPath, Stability)> { +) -> Option { if let Some(crate_root) = module_id.as_crate_root() { if crate_root == ctx.from.derive_crate_root() { // - if the item is the crate root, return `crate` - return Some((ModPath::from_segments(PathKind::Crate, None), Stable)); + return Some(Choice { + path: ModPath::from_segments(PathKind::Crate, None), + path_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 @@ -193,7 +199,7 @@ fn find_path_for_module( } else { PathKind::Plain }; - return Some((ModPath::from_segments(kind, iter::once(name.clone())), Stable)); + return Some(Choice::new(ctx.cfg.prefer_prelude, kind, name.clone(), Stable)); } } @@ -211,25 +217,33 @@ fn find_path_for_module( ); if let Some(scope_name) = scope_name { // - if the item is already in scope, return the name under which it is - return Some(( - ModPath::from_segments(ctx.prefix.path_kind(), iter::once(scope_name)), + return Some(Choice::new( + ctx.cfg.prefer_prelude, + ctx.prefix.path_kind(), + scope_name, Stable, )); } } // - if the module can be referenced as self, super or crate, do that - if let Some(mod_path) = is_kw_kind_relative_to_from(ctx.from_def_map, module_id, ctx.from) { - if ctx.prefix != PrefixKind::ByCrate || mod_path.kind == PathKind::Crate { - return Some((mod_path, Stable)); + if let Some(kind) = is_kw_kind_relative_to_from(ctx.from_def_map, module_id, ctx.from) { + if ctx.prefix != PrefixKind::ByCrate || kind == PathKind::Crate { + return Some(Choice { + path: ModPath::from_segments(kind, None), + path_len: path_kind_len(kind), + stability: Stable, + prefer_due_to_prelude: false, + sysroot_score: 0, + }); } } // - if the module is in the prelude, return it by that path - if let Some(mod_path) = + if let Some(choice) = find_in_prelude(ctx.db, ctx.from_def_map, ItemInNs::Types(module_id.into()), ctx.from) { - return Some((mod_path, Stable)); + return Some(choice); } calculate_best_path(ctx, visited_modules, ItemInNs::Types(module_id.into()), max_len) } @@ -256,7 +270,7 @@ fn find_in_prelude( local_def_map: &DefMap, item: ItemInNs, from: ModuleId, -) -> Option { +) -> Option { let (prelude_module, _) = local_def_map.prelude()?; let prelude_def_map = prelude_module.def_map(db); let prelude_scope = &prelude_def_map[prelude_module.local_id].scope; @@ -278,7 +292,7 @@ fn find_in_prelude( }); if found_and_same_def.unwrap_or(true) { - Some(ModPath::from_segments(PathKind::Plain, iter::once(name.clone()))) + Some(Choice::new(false, PathKind::Plain, name.clone(), Stable)) } else { None } @@ -288,7 +302,7 @@ fn is_kw_kind_relative_to_from( def_map: &DefMap, item: ModuleId, from: ModuleId, -) -> Option { +) -> Option { if item.krate != from.krate || item.is_within_block() || from.is_within_block() { return None; } @@ -296,14 +310,11 @@ fn is_kw_kind_relative_to_from( let from = from.local_id; if item == from { // - if the item is the module we're in, use `self` - Some(ModPath::from_segments(PathKind::SELF, None)) + Some(PathKind::SELF) } else if let Some(parent_id) = def_map[from].parent { if item == parent_id { // - if the item is the parent module, use `super` (this is not used recursively, since `super::super` is ugly) - Some(ModPath::from_segments( - if parent_id == DefMap::ROOT { PathKind::Crate } else { PathKind::Super(1) }, - None, - )) + Some(if parent_id == DefMap::ROOT { PathKind::Crate } else { PathKind::Super(1) }) } else { None } @@ -318,7 +329,7 @@ fn calculate_best_path( visited_modules: &mut FxHashSet, item: ItemInNs, max_len: usize, -) -> Option<(ModPath, Stability)> { +) -> Option { if max_len <= 1 { // recursive base case, we can't find a path prefix of length 0, one segment is occupied by // the item's name itself. @@ -336,25 +347,7 @@ fn calculate_best_path( } ctx.fuel.set(fuel - 1); - let mut best_path = None; - let mut best_path_len = max_len; - let mut process = |mut path: (ModPath, Stability), name, best_path_len: &mut _| { - path.0.push_segment(name); - let new_path = match best_path.take() { - Some(best_path) => select_best_path(best_path, path, ctx.cfg), - None => path, - }; - if new_path.1 == Stable { - *best_path_len = new_path.0.len(); - } - match &mut best_path { - Some((old_path, old_stability)) => { - *old_path = new_path.0; - *old_stability = zip_stability(*old_stability, new_path.1); - } - None => best_path = Some(new_path), - } - }; + let mut best_choice = None::; let db = ctx.db; if item.krate(db) == Some(ctx.from.krate) { // Item was defined in the same crate that wants to import it. It cannot be found in any @@ -366,10 +359,16 @@ fn calculate_best_path( } // we are looking for paths of length up to best_path_len, any longer will make it be // less optimal. The -1 is due to us pushing name onto it afterwards. - if let Some(path) = - find_path_for_module(ctx, visited_modules, module_id, best_path_len - 1) - { - process(path, name.clone(), &mut best_path_len); + if let Some(path) = find_path_for_module( + ctx, + visited_modules, + module_id, + best_choice.as_ref().map_or(max_len, |it| it.path_len) - 1, + ) { + best_choice = Some(match best_choice.take() { + Some(best_choice) => best_choice.select(path, name.clone()), + None => path.push(ctx.cfg.prefer_prelude, name.clone()), + }); } }) } else { @@ -377,7 +376,7 @@ fn calculate_best_path( // 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. - let mut process_dep = |dep: CrateId| { + 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; @@ -391,24 +390,33 @@ fn calculate_best_path( // 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 path = - find_path_for_module(ctx, visited_modules, info.container, best_path_len - 1); - let Some((path, path_stability)) = path else { + let choice = find_path_for_module( + ctx, + visited_modules, + info.container, + 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); - let path = ( - path, - zip_stability(path_stability, if info.is_unstable { Unstable } else { Stable }), + choice.stability = zip_stability( + choice.stability, + if info.is_unstable { Unstable } else { Stable }, ); - process(path, info.name.clone(), &mut best_path_len); + best_choice = Some(match best_choice.take() { + Some(best_choice) => best_choice.select(choice, info.name.clone()), + None => choice.push(ctx.cfg.prefer_prelude, info.name.clone()), + }); processed_something = true; } processed_something }; - let dependencies = &db.crate_graph()[ctx.from.krate].dependencies; + 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. @@ -417,89 +425,104 @@ fn calculate_best_path( let processed = dependencies .iter() .filter(|it| it.is_sysroot()) - .map(|dep| process_dep(dep.crate_id)) + .map(|dep| { + ( + match crate_graph[dep.crate_id].origin { + CrateOrigin::Lang(LangCrateOrigin::Std) if ctx.cfg.prefer_no_std => 4, + CrateOrigin::Lang(LangCrateOrigin::Std) => 1, + CrateOrigin::Lang(LangCrateOrigin::Alloc) => 3, + CrateOrigin::Lang(LangCrateOrigin::ProcMacro) => 3, + CrateOrigin::Lang(LangCrateOrigin::Test) => 3, + CrateOrigin::Lang(LangCrateOrigin::Core) => 2, + 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, so return it. - return best_path; + return best_choice; } } dependencies .iter() .filter(|it| !ctx.is_std_item || !it.is_sysroot()) - .for_each(|dep| _ = process_dep(dep.crate_id)); + .for_each(|dep| _ = process_dep(dep.crate_id, 0)); } - best_path + best_choice } -/// Select the best (most relevant) path between two paths. -/// This accounts for stability, path length whether, std should be chosen over alloc/core paths as -/// well as ignoring prelude like paths or not. -fn select_best_path( - old_path @ (_, old_stability): (ModPath, Stability), - new_path @ (_, new_stability): (ModPath, Stability), - cfg: ImportPathConfig, -) -> (ModPath, Stability) { - match (old_stability, new_stability) { - (Stable, Unstable) => return old_path, - (Unstable, Stable) => return new_path, - _ => {} - } - let std_crates: [Symbol; 3] = [sym::std.clone(), sym::core.clone(), sym::alloc.clone()]; +struct Choice { + path: ModPath, + /// The length in characters of the path + path_len: usize, + /// The stability of the path + 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, +} - let choose = |new: (ModPath, _), old: (ModPath, _)| { - let (new_path, _) = &new; - let (old_path, _) = &old; - let new_has_prelude = new_path.segments().iter().any(|seg| *seg == sym::prelude.clone()); - let old_has_prelude = old_path.segments().iter().any(|seg| *seg == sym::prelude.clone()); - match (new_has_prelude, old_has_prelude, cfg.prefer_prelude) { - (true, false, true) | (false, true, false) => new, - (true, false, false) | (false, true, true) => old, - // no prelude difference in the paths, so pick the shorter one - (true, true, _) | (false, false, _) => { - let new_path_is_shorter = new_path - .len() - .cmp(&old_path.len()) - .then_with(|| new_path.textual_len().cmp(&old_path.textual_len())) - .is_lt(); - if new_path_is_shorter { - new - } else { - old +impl Choice { + fn new(prefer_prelude: bool, kind: PathKind, name: Name, stability: Stability) -> Self { + Self { + path_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)), + } + } + + fn push(mut self, prefer_prelude: bool, name: Name) -> Self { + self.path_len += name.as_str().len(); + self.prefer_due_to_prelude |= prefer_prelude && name == sym::prelude; + self.path.push_segment(name); + self + } + + fn select(self, mut other: Choice, name: Name) -> Self { + match other + .stability + .cmp(&self.stability) + .then_with(|| other.prefer_due_to_prelude.cmp(&self.prefer_due_to_prelude)) + .then_with(|| self.sysroot_score.cmp(&other.sysroot_score)) + .then_with(|| (self.path.len()).cmp(&(other.path.len() + 1))) + { + Ordering::Less => self, + Ordering::Equal => { + other.path_len += name.as_str().len(); + + match self.path_len.cmp(&other.path_len) { + Ordering::Less | Ordering::Equal => self, + Ordering::Greater => { + other.path.push_segment(name); + other + } } } - } - }; - - match (old_path.0.segments().first(), new_path.0.segments().first()) { - (Some(old), Some(new)) - if std_crates.contains(old.symbol()) && std_crates.contains(new.symbol()) => - { - let rank = match cfg.prefer_no_std { - false => |name: &Name| match name { - name if *name == sym::core.clone() => 0, - name if *name == sym::alloc.clone() => 1, - name if *name == sym::std.clone() => 2, - _ => unreachable!(), - }, - true => |name: &Name| match name { - name if *name == sym::core.clone() => 2, - name if *name == sym::alloc.clone() => 1, - name if *name == sym::std.clone() => 0, - _ => unreachable!(), - }, - }; - let nrank = rank(new); - let orank = rank(old); - match nrank.cmp(&orank) { - Ordering::Less => old_path, - Ordering::Equal => choose(new_path, old_path), - Ordering::Greater => new_path, + Ordering::Greater => { + other.path.push_segment(name); + other } } - _ => choose(new_path, old_path), + } +} + +fn path_kind_len(kind: PathKind) -> usize { + match kind { + PathKind::Plain => 0, + PathKind::Super(0) => 4, + PathKind::Super(s) => s as usize * 5, + PathKind::Crate => 5, + PathKind::Abs => 2, + PathKind::DollarCrate(_) => 0, } } From 2564b69e1ddf7334fef65c645d879cf1ed40ee9f Mon Sep 17 00:00:00 2001 From: Lukas Wirth Date: Sun, 21 Jul 2024 11:10:15 +0200 Subject: [PATCH 2/6] Specialize `find_path` local search --- crates/hir-def/src/find_path.rs | 73 ++++++++++++++++++++------------- 1 file changed, 45 insertions(+), 28 deletions(-) diff --git a/crates/hir-def/src/find_path.rs b/crates/hir-def/src/find_path.rs index 0947a62a6a..5164fa08f3 100644 --- a/crates/hir-def/src/find_path.rs +++ b/crates/hir-def/src/find_path.rs @@ -116,7 +116,7 @@ fn find_path_inner(ctx: &FindPathCtx<'_>, item: ItemInNs, max_len: usize) -> Opt // - if the item is a module, jump straight to module search 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, max_len) + return find_path_for_module(ctx, &mut visited_modules, module_id, true, max_len) .map(|choice| choice.path); } @@ -161,10 +161,11 @@ fn find_path_for_module( ctx: &FindPathCtx<'_>, visited_modules: &mut FxHashSet, module_id: ModuleId, + maybe_extern: bool, max_len: usize, ) -> Option { if let Some(crate_root) = module_id.as_crate_root() { - if crate_root == ctx.from.derive_crate_root() { + if !maybe_extern || crate_root == ctx.from.derive_crate_root() { // - if the item is the crate root, return `crate` return Some(Choice { path: ModPath::from_segments(PathKind::Crate, None), @@ -240,12 +241,15 @@ fn find_path_for_module( } // - if the module is in the prelude, return it by that path - if let Some(choice) = - find_in_prelude(ctx.db, ctx.from_def_map, ItemInNs::Types(module_id.into()), ctx.from) - { + let item = ItemInNs::Types(module_id.into()); + if let Some(choice) = find_in_prelude(ctx.db, ctx.from_def_map, item, ctx.from) { return Some(choice); } - calculate_best_path(ctx, visited_modules, ItemInNs::Types(module_id.into()), max_len) + if maybe_extern { + calculate_best_path(ctx, visited_modules, item, max_len) + } else { + calculate_best_path_local(ctx, visited_modules, item, max_len) + } } fn find_in_scope( @@ -347,31 +351,13 @@ fn calculate_best_path( } ctx.fuel.set(fuel - 1); - let mut best_choice = None::; - let db = ctx.db; - if item.krate(db) == Some(ctx.from.krate) { + if item.krate(ctx.db) == Some(ctx.from.krate) { // Item was defined in the same crate that wants to import it. It cannot be found in any // dependency in this case. - // FIXME: cache the `find_local_import_locations` output? - find_local_import_locations(db, item, ctx.from, ctx.from_def_map, |name, module_id| { - if !visited_modules.insert(module_id) { - return; - } - // we are looking for paths of length up to best_path_len, any longer will make it be - // less optimal. The -1 is due to us pushing name onto it afterwards. - if let Some(path) = find_path_for_module( - ctx, - visited_modules, - module_id, - best_choice.as_ref().map_or(max_len, |it| it.path_len) - 1, - ) { - best_choice = Some(match best_choice.take() { - Some(best_choice) => best_choice.select(path, name.clone()), - None => path.push(ctx.cfg.prefer_prelude, name.clone()), - }); - } - }) + calculate_best_path_local(ctx, visited_modules, item, max_len) } else { + let mut best_choice = None::; + 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. @@ -394,6 +380,7 @@ fn calculate_best_path( 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 { @@ -453,7 +440,37 @@ fn calculate_best_path( .iter() .filter(|it| !ctx.is_std_item || !it.is_sysroot()) .for_each(|dep| _ = process_dep(dep.crate_id, 0)); + best_choice } +} + +fn calculate_best_path_local( + ctx: &FindPathCtx<'_>, + visited_modules: &mut FxHashSet, + item: ItemInNs, + max_len: usize, +) -> Option { + let mut best_choice = None::; + // FIXME: cache the `find_local_import_locations` output? + find_local_import_locations(ctx.db, item, ctx.from, ctx.from_def_map, |name, module_id| { + if !visited_modules.insert(module_id) { + return; + } + // we are looking for paths of length up to best_path_len, any longer will make it be + // less optimal. The -1 is due to us pushing name onto it afterwards. + if let Some(path) = find_path_for_module( + ctx, + visited_modules, + module_id, + false, + best_choice.as_ref().map_or(max_len, |it| it.path_len) - 1, + ) { + best_choice = Some(match best_choice.take() { + Some(best_choice) => best_choice.select(path, name.clone()), + None => path.push(ctx.cfg.prefer_prelude, name.clone()), + }); + } + }); best_choice } From 9fd6d26e97dad0ca950997890de6bf58a71ff5ab Mon Sep 17 00:00:00 2001 From: Lukas Wirth Date: Sun, 21 Jul 2024 11:16:23 +0200 Subject: [PATCH 3/6] Fix using wrong length for max_len arg --- crates/hir-def/src/find_path.rs | 9 +++++++-- 1 file changed, 7 insertions(+), 2 deletions(-) diff --git a/crates/hir-def/src/find_path.rs b/crates/hir-def/src/find_path.rs index 5164fa08f3..2c4096ba90 100644 --- a/crates/hir-def/src/find_path.rs +++ b/crates/hir-def/src/find_path.rs @@ -381,7 +381,7 @@ fn calculate_best_path( visited_modules, info.container, true, - best_choice.as_ref().map_or(max_len, |it| it.path_len) - 1, + best_choice.as_ref().map_or(max_len, |it| it.path.len()) - 1, ); let Some(mut choice) = choice else { continue; @@ -450,6 +450,11 @@ fn calculate_best_path_local( item: ItemInNs, max_len: usize, ) -> Option { + if max_len <= 1 { + // recursive base case, we can't find a path prefix of length 0, one segment is occupied by + // the item's name itself. + return None; + } let mut best_choice = None::; // FIXME: cache the `find_local_import_locations` output? find_local_import_locations(ctx.db, item, ctx.from, ctx.from_def_map, |name, module_id| { @@ -463,7 +468,7 @@ fn calculate_best_path_local( visited_modules, module_id, false, - best_choice.as_ref().map_or(max_len, |it| it.path_len) - 1, + best_choice.as_ref().map_or(max_len, |it| it.path.len()) - 1, ) { best_choice = Some(match best_choice.take() { Some(best_choice) => best_choice.select(path, name.clone()), From 2a32976e90039f54716394739a068e6e8e1b7529 Mon Sep 17 00:00:00 2001 From: Lukas Wirth Date: Sun, 21 Jul 2024 11:32:48 +0200 Subject: [PATCH 4/6] Use out parameter instead of return value for `find_path` choice --- crates/hir-def/src/find_path.rs | 109 ++++++++++++++++---------------- 1 file changed, 53 insertions(+), 56 deletions(-) diff --git a/crates/hir-def/src/find_path.rs b/crates/hir-def/src/find_path.rs index 2c4096ba90..56e5475e07 100644 --- a/crates/hir-def/src/find_path.rs +++ b/crates/hir-def/src/find_path.rs @@ -153,7 +153,9 @@ fn find_path_inner(ctx: &FindPathCtx<'_>, item: ItemInNs, max_len: usize) -> Opt let mut visited_modules = FxHashSet::default(); - calculate_best_path(ctx, &mut visited_modules, item, max_len).map(|choice| choice.path) + let mut best_choice = None; + calculate_best_path(ctx, &mut visited_modules, item, max_len, &mut best_choice); + best_choice.map(|choice| choice.path) } #[tracing::instrument(skip_all)] @@ -164,12 +166,16 @@ fn find_path_for_module( maybe_extern: bool, max_len: usize, ) -> Option { + if max_len == 0 { + // recursive base case, we can't find a path of length 0 + return None; + } if let Some(crate_root) = module_id.as_crate_root() { if !maybe_extern || crate_root == ctx.from.derive_crate_root() { // - if the item is the crate root, return `crate` return Some(Choice { path: ModPath::from_segments(PathKind::Crate, None), - path_len: 5, + path_text_len: 5, stability: Stable, prefer_due_to_prelude: false, sysroot_score: 0, @@ -232,7 +238,7 @@ fn find_path_for_module( if ctx.prefix != PrefixKind::ByCrate || kind == PathKind::Crate { return Some(Choice { path: ModPath::from_segments(kind, None), - path_len: path_kind_len(kind), + path_text_len: path_kind_len(kind), stability: Stable, prefer_due_to_prelude: false, sysroot_score: 0, @@ -245,11 +251,13 @@ fn find_path_for_module( if let Some(choice) = find_in_prelude(ctx.db, ctx.from_def_map, item, ctx.from) { return Some(choice); } + let mut best_choice = None; if maybe_extern { - calculate_best_path(ctx, visited_modules, item, max_len) + calculate_best_path(ctx, visited_modules, item, max_len, &mut best_choice); } else { - calculate_best_path_local(ctx, visited_modules, item, max_len) + calculate_best_path_local(ctx, visited_modules, item, max_len, &mut best_choice); } + best_choice } fn find_in_scope( @@ -333,12 +341,8 @@ fn calculate_best_path( visited_modules: &mut FxHashSet, item: ItemInNs, max_len: usize, -) -> Option { - if max_len <= 1 { - // recursive base case, we can't find a path prefix of length 0, one segment is occupied by - // the item's name itself. - return None; - } + best_choice: &mut Option, +) { let fuel = ctx.fuel.get(); if fuel == 0 { // we ran out of fuel, so we stop searching here @@ -347,16 +351,15 @@ fn calculate_best_path( item.krate(ctx.db), ctx.from.krate() ); - return None; + return; } ctx.fuel.set(fuel - 1); if item.krate(ctx.db) == Some(ctx.from.krate) { // Item was defined in the same crate that wants to import it. It cannot be found in any // dependency in this case. - calculate_best_path_local(ctx, visited_modules, item, max_len) + calculate_best_path_local(ctx, visited_modules, item, max_len, best_choice) } else { - let mut best_choice = None::; 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 @@ -393,10 +396,7 @@ fn calculate_best_path( if info.is_unstable { Unstable } else { Stable }, ); - best_choice = Some(match best_choice.take() { - Some(best_choice) => best_choice.select(choice, info.name.clone()), - None => choice.push(ctx.cfg.prefer_prelude, info.name.clone()), - }); + Choice::try_select(best_choice, choice, ctx.cfg.prefer_prelude, info.name.clone()); processed_something = true; } processed_something @@ -415,12 +415,12 @@ fn calculate_best_path( .map(|dep| { ( match crate_graph[dep.crate_id].origin { - CrateOrigin::Lang(LangCrateOrigin::Std) if ctx.cfg.prefer_no_std => 4, + CrateOrigin::Lang(LangCrateOrigin::Std) if ctx.cfg.prefer_no_std => 5, CrateOrigin::Lang(LangCrateOrigin::Std) => 1, - CrateOrigin::Lang(LangCrateOrigin::Alloc) => 3, + CrateOrigin::Lang(LangCrateOrigin::Alloc) => 2, CrateOrigin::Lang(LangCrateOrigin::ProcMacro) => 3, CrateOrigin::Lang(LangCrateOrigin::Test) => 3, - CrateOrigin::Lang(LangCrateOrigin::Core) => 2, + CrateOrigin::Lang(LangCrateOrigin::Core) => 4, CrateOrigin::Lang(LangCrateOrigin::Other) => 1, _ => 0, }, @@ -431,8 +431,8 @@ fn calculate_best_path( .reduce(BitOr::bitor) .unwrap_or(false); if processed { - // Found a path in a sysroot crate, so return it. - return best_choice; + // Found a path in a sysroot crate + return; } } @@ -440,7 +440,6 @@ fn calculate_best_path( .iter() .filter(|it| !ctx.is_std_item || !it.is_sysroot()) .for_each(|dep| _ = process_dep(dep.crate_id, 0)); - best_choice } } @@ -449,13 +448,8 @@ fn calculate_best_path_local( visited_modules: &mut FxHashSet, item: ItemInNs, max_len: usize, -) -> Option { - if max_len <= 1 { - // recursive base case, we can't find a path prefix of length 0, one segment is occupied by - // the item's name itself. - return None; - } - let mut best_choice = None::; + best_choice: &mut Option, +) { // FIXME: cache the `find_local_import_locations` output? find_local_import_locations(ctx.db, item, ctx.from, ctx.from_def_map, |name, module_id| { if !visited_modules.insert(module_id) { @@ -463,26 +457,22 @@ fn calculate_best_path_local( } // we are looking for paths of length up to best_path_len, any longer will make it be // less optimal. The -1 is due to us pushing name onto it afterwards. - if let Some(path) = find_path_for_module( + if let Some(choice) = find_path_for_module( ctx, visited_modules, module_id, false, best_choice.as_ref().map_or(max_len, |it| it.path.len()) - 1, ) { - best_choice = Some(match best_choice.take() { - Some(best_choice) => best_choice.select(path, name.clone()), - None => path.push(ctx.cfg.prefer_prelude, name.clone()), - }); + Choice::try_select(best_choice, choice, ctx.cfg.prefer_prelude, name.clone()); } }); - best_choice } struct Choice { path: ModPath, /// The length in characters of the path - path_len: usize, + path_text_len: usize, /// The stability of the path stability: Stability, /// Whether this path contains a prelude segment and preference for it has been signaled @@ -494,7 +484,7 @@ struct Choice { impl Choice { fn new(prefer_prelude: bool, kind: PathKind, name: Name, stability: Stability) -> Self { Self { - path_len: path_kind_len(kind) + name.as_str().len(), + path_text_len: path_kind_len(kind) + name.as_str().len(), stability, prefer_due_to_prelude: prefer_prelude && name == sym::prelude, sysroot_score: 0, @@ -503,37 +493,44 @@ impl Choice { } fn push(mut self, prefer_prelude: bool, name: Name) -> Self { - self.path_len += name.as_str().len(); + self.path_text_len += name.as_str().len(); self.prefer_due_to_prelude |= prefer_prelude && name == sym::prelude; self.path.push_segment(name); self } - fn select(self, mut other: Choice, name: Name) -> Self { + fn try_select( + current: &mut Option, + mut other: Choice, + prefer_prelude: bool, + name: Name, + ) { + let Some(current) = current else { + *current = Some(other.push(prefer_prelude, name)); + return; + }; match other .stability - .cmp(&self.stability) - .then_with(|| other.prefer_due_to_prelude.cmp(&self.prefer_due_to_prelude)) - .then_with(|| self.sysroot_score.cmp(&other.sysroot_score)) - .then_with(|| (self.path.len()).cmp(&(other.path.len() + 1))) + .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 => self, + Ordering::Less => return, Ordering::Equal => { - other.path_len += name.as_str().len(); - - match self.path_len.cmp(&other.path_len) { - Ordering::Less | Ordering::Equal => self, - Ordering::Greater => { - other.path.push_segment(name); - other - } + other.path_text_len += name.as_str().len(); + if let Ordering::Less | Ordering::Equal = + current.path_text_len.cmp(&other.path_text_len) + { + return; } } Ordering::Greater => { - other.path.push_segment(name); - other + other.path_text_len += name.as_str().len(); } } + other.path.push_segment(name); + *current = other; } } From 4b2a1232803d5147b218d3e71cf3ff9bc34b3e08 Mon Sep 17 00:00:00 2001 From: Lukas Wirth Date: Sun, 21 Jul 2024 12:17:50 +0200 Subject: [PATCH 5/6] Fix visited module tracking not clearing itself on backtracking --- crates/hir-def/src/find_path.rs | 129 +++++++++++++++++++++----------- crates/hir-def/src/test_db.rs | 1 - 2 files changed, 85 insertions(+), 45 deletions(-) diff --git a/crates/hir-def/src/find_path.rs b/crates/hir-def/src/find_path.rs index 56e5475e07..1fbb407322 100644 --- a/crates/hir-def/src/find_path.rs +++ b/crates/hir-def/src/find_path.rs @@ -13,7 +13,7 @@ use rustc_hash::FxHashSet; use crate::{ db::DefDatabase, item_scope::ItemInNs, - nameres::DefMap, + nameres::{DefMap, ModuleData}, path::{ModPath, PathKind}, visibility::{Visibility, VisibilityExplicitness}, ImportPathConfig, ModuleDefId, ModuleId, @@ -161,7 +161,7 @@ fn find_path_inner(ctx: &FindPathCtx<'_>, item: ItemInNs, max_len: usize) -> Opt #[tracing::instrument(skip_all)] fn find_path_for_module( ctx: &FindPathCtx<'_>, - visited_modules: &mut FxHashSet, + visited_modules: &mut FxHashSet<(ItemInNs, ModuleId)>, module_id: ModuleId, maybe_extern: bool, max_len: usize, @@ -338,7 +338,7 @@ fn is_kw_kind_relative_to_from( #[tracing::instrument(skip_all)] fn calculate_best_path( ctx: &FindPathCtx<'_>, - visited_modules: &mut FxHashSet, + visited_modules: &mut FxHashSet<(ItemInNs, ModuleId)>, item: ItemInNs, max_len: usize, best_choice: &mut Option, @@ -445,28 +445,32 @@ fn calculate_best_path( fn calculate_best_path_local( ctx: &FindPathCtx<'_>, - visited_modules: &mut FxHashSet, + visited_modules: &mut FxHashSet<(ItemInNs, ModuleId)>, item: ItemInNs, max_len: usize, best_choice: &mut Option, ) { // FIXME: cache the `find_local_import_locations` output? - find_local_import_locations(ctx.db, item, ctx.from, ctx.from_def_map, |name, module_id| { - if !visited_modules.insert(module_id) { - return; - } - // we are looking for paths of length up to best_path_len, any longer will make it be - // less optimal. The -1 is due to us pushing name onto it afterwards. - if let Some(choice) = find_path_for_module( - ctx, - visited_modules, - module_id, - false, - best_choice.as_ref().map_or(max_len, |it| it.path.len()) - 1, - ) { - Choice::try_select(best_choice, choice, ctx.cfg.prefer_prelude, name.clone()); - } - }); + find_local_import_locations( + ctx.db, + item, + ctx.from, + ctx.from_def_map, + visited_modules, + |visited_modules, name, module_id| { + // we are looking for paths of length up to best_path_len, any longer will make it be + // less optimal. The -1 is due to us pushing name onto it afterwards. + if let Some(choice) = find_path_for_module( + ctx, + visited_modules, + module_id, + false, + best_choice.as_ref().map_or(max_len, |it| it.path.len()) - 1, + ) { + Choice::try_select(best_choice, choice, ctx.cfg.prefer_prelude, name.clone()); + } + }, + ); } struct Choice { @@ -551,7 +555,8 @@ fn find_local_import_locations( item: ItemInNs, from: ModuleId, def_map: &DefMap, - mut cb: impl FnMut(&Name, ModuleId), + visited_modules: &mut FxHashSet<(ItemInNs, ModuleId)>, + mut cb: impl FnMut(&mut FxHashSet<(ItemInNs, ModuleId)>, &Name, ModuleId), ) { let _p = tracing::info_span!("find_local_import_locations").entered(); @@ -564,32 +569,29 @@ fn find_local_import_locations( let mut worklist = def_map[from.local_id] .children .values() - .map(|child| def_map.module_id(*child)) - // FIXME: do we need to traverse out of block expressions here? + .map(|&child| def_map.module_id(child)) .chain(iter::successors(from.containing_module(db), |m| m.containing_module(db))) + .zip(iter::repeat(false)) .collect::>(); - let mut seen: FxHashSet<_> = FxHashSet::default(); let def_map = def_map.crate_root().def_map(db); + let mut block_def_map; + let mut cursor = 0; - while let Some(module) = worklist.pop() { - if !seen.insert(module) { - continue; // already processed this module + while let Some(&mut (module, ref mut processed)) = worklist.get_mut(cursor) { + cursor += 1; + if !visited_modules.insert((item, module)) { + // already processed this module + continue; } - let ext_def_map; - let data = if module.krate == from.krate { - if module.block.is_some() { - // Re-query the block's DefMap - ext_def_map = module.def_map(db); - &ext_def_map[module.local_id] - } else { - // Reuse the root DefMap - &def_map[module.local_id] - } + *processed = true; + let data = if module.block.is_some() { + // Re-query the block's DefMap + block_def_map = module.def_map(db); + &block_def_map[module.local_id] } else { - // The crate might reexport a module defined in another crate. - ext_def_map = module.def_map(db); - &ext_def_map[module.local_id] + // Reuse the root DefMap + &def_map[module.local_id] }; if let Some((name, vis, declared)) = data.scope.name_of(item) { @@ -612,18 +614,30 @@ fn find_local_import_locations( // the item and we're a submodule of it, so can we. // Also this keeps the cached data smaller. if declared || is_pub_or_explicit { - cb(name, module); + cb(visited_modules, name, module); } } } // Descend into all modules visible from `from`. for (module, vis) in data.scope.modules_in_scope() { + if module.krate != from.krate { + // We don't need to look at modules from other crates as our item has to be in the + // current crate + continue; + } + if visited_modules.contains(&(item, module)) { + continue; + } + if vis.is_visible_from(db, from) { - worklist.push(module); + worklist.push((module, false)); } } } + worklist.into_iter().filter(|&(_, processed)| processed).for_each(|(module, _)| { + visited_modules.remove(&(item, module)); + }); } #[cfg(test)] @@ -1591,8 +1605,6 @@ fn main() { #[test] fn from_inside_module() { - // This worked correctly, but the test suite logic was broken. - cov_mark::check!(submodule_in_testdb); check_found_path( r#" mod baz { @@ -1617,6 +1629,35 @@ mod bar { ) } + #[test] + fn from_inside_module2() { + check_found_path( + r#" +mod qux { + pub mod baz { + pub struct Foo {} + } + + mod bar { + fn bar() { + $0; + } + } +} + + "#, + "crate::qux::baz::Foo", + expect![[r#" + Plain (imports ✔): super::baz::Foo + Plain (imports ✖): super::baz::Foo + ByCrate(imports ✔): crate::qux::baz::Foo + ByCrate(imports ✖): crate::qux::baz::Foo + BySelf (imports ✔): super::baz::Foo + BySelf (imports ✖): super::baz::Foo + "#]], + ) + } + #[test] fn from_inside_module_with_inner_items() { check_found_path( diff --git a/crates/hir-def/src/test_db.rs b/crates/hir-def/src/test_db.rs index 3f002af9d2..f44472eae5 100644 --- a/crates/hir-def/src/test_db.rs +++ b/crates/hir-def/src/test_db.rs @@ -148,7 +148,6 @@ impl TestDB { }; if size != Some(new_size) { - cov_mark::hit!(submodule_in_testdb); size = Some(new_size); res = module; } From 733cb1e645708faf3742991bf39a7ba4a3938387 Mon Sep 17 00:00:00 2001 From: Lukas Wirth Date: Sun, 21 Jul 2024 12:49:10 +0200 Subject: [PATCH 6/6] Optimize `find_path` for sysroot library search some more --- crates/hir-def/src/find_path.rs | 210 +++++++++++++++++--------------- 1 file changed, 112 insertions(+), 98 deletions(-) diff --git a/crates/hir-def/src/find_path.rs b/crates/hir-def/src/find_path.rs index 1fbb407322..91594aecd0 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, ops::BitOr}; +use std::{cell::Cell, cmp::Ordering, iter}; use base_db::{CrateId, CrateOrigin, LangCrateOrigin}; use hir_expand::{ @@ -13,7 +13,7 @@ use rustc_hash::FxHashSet; use crate::{ db::DefDatabase, item_scope::ItemInNs, - nameres::{DefMap, ModuleData}, + nameres::DefMap, path::{ModPath, PathKind}, visibility::{Visibility, VisibilityExplicitness}, ImportPathConfig, ModuleDefId, ModuleId, @@ -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,13 +67,6 @@ 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; @@ -107,17 +100,22 @@ 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) -> Option { +fn find_path_inner( + ctx: &FindPathCtx<'_>, + item: ItemInNs, + max_len: usize, + is_std_item: bool, +) -> Option { // - if the item is a module, jump straight to module search - 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); + 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); + } } let may_be_in_scope = match ctx.prefix { @@ -140,9 +138,12 @@ fn find_path_inner(ctx: &FindPathCtx<'_>, item: ItemInNs, max_len: usize) -> Opt 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) - { + if let Some(mut path) = find_path_inner( + ctx, + ItemInNs::Types(variant.lookup(ctx.db).parent.into()), + max_len, + is_std_item, + ) { path.push_segment(ctx.db.enum_variant_data(variant).name.clone()); return Some(path); } @@ -151,10 +152,18 @@ fn find_path_inner(ctx: &FindPathCtx<'_>, item: ItemInNs, max_len: usize) -> Opt // variant somewhere } - let mut visited_modules = FxHashSet::default(); + 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 best_choice = None; - calculate_best_path(ctx, &mut visited_modules, item, max_len, &mut best_choice); + calculate_best_path(ctx, &mut FxHashSet::default(), item, max_len, &mut best_choice); best_choice.map(|choice| choice.path) } @@ -178,7 +187,6 @@ 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 @@ -241,7 +249,6 @@ fn find_path_for_module( path_text_len: path_kind_len(kind), stability: Stable, prefer_due_to_prelude: false, - sysroot_score: 0, }); } } @@ -360,86 +367,97 @@ 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. - 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; - } + 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) + }); + } +} - // 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; +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, } - processed_something - }; + }) { + 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 +} - 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; - } +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; } - dependencies - .iter() - .filter(|it| !ctx.is_std_item || !it.is_sysroot()) - .for_each(|dep| _ = process_dep(dep.crate_id, 0)); + // 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; + }; + cov_mark::hit!(partially_imported); + if info.is_unstable { + choice.stability = Unstable; + } + + Choice::try_select(best_choice, choice, ctx.cfg.prefer_prelude, info.name.clone()); } } @@ -481,8 +499,6 @@ 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 { @@ -491,7 +507,6 @@ 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)), } } @@ -517,7 +532,6 @@ 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,