Skip to content

Commit 1b420a5

Browse files
Prefer visible paths which are not doc-hidden
Co-authored-by: Alik Aslanyan <[email protected]>
1 parent 3ff249a commit 1b420a5

File tree

9 files changed

+106
-77
lines changed

9 files changed

+106
-77
lines changed

compiler/rustc_data_structures/src/stable_hasher.rs

+3-2
Original file line numberDiff line numberDiff line change
@@ -394,9 +394,10 @@ where
394394
}
395395
}
396396

397-
impl<A, CTX> HashStable<CTX> for SmallVec<[A; 1]>
397+
impl<T, CTX, const N: usize> HashStable<CTX> for SmallVec<[T; N]>
398398
where
399-
A: HashStable<CTX>,
399+
[T; N]: smallvec::Array,
400+
<[T; N] as smallvec::Array>::Item: HashStable<CTX>,
400401
{
401402
#[inline]
402403
fn hash_stable(&self, ctx: &mut CTX, hasher: &mut StableHasher) {

compiler/rustc_metadata/src/rmeta/decoder/cstore_impl.rs

+82-55
Original file line numberDiff line numberDiff line change
@@ -1,10 +1,11 @@
11
use crate::creader::{CStore, LoadedMacro};
22
use crate::foreign_modules;
33
use crate::native_libs;
4+
use crate::rmeta::ty::util::is_doc_hidden;
45

56
use rustc_ast as ast;
67
use rustc_attr::Deprecation;
7-
use rustc_hir::def::{CtorKind, DefKind, Res};
8+
use rustc_hir::def::{CtorKind, DefKind};
89
use rustc_hir::def_id::{CrateNum, DefId, DefIdMap, LOCAL_CRATE};
910
use rustc_hir::definitions::{DefKey, DefPath, DefPathHash};
1011
use rustc_middle::arena::ArenaAllocatable;
@@ -367,15 +368,15 @@ pub(in crate::rmeta) fn provide(providers: &mut Providers) {
367368
// external item that is visible from at least one local module) to a
368369
// sufficiently visible parent (considering modules that re-export the
369370
// external item to be parents).
370-
visible_parent_map: |tcx, ()| {
371-
use std::collections::hash_map::Entry;
372-
use std::collections::vec_deque::VecDeque;
371+
// Returns a map from a sufficiently visible external item (i.e., an
372+
// external item that is visible from at least one local module) to a
373+
// sufficiently visible parent (considering modules that re-export the
374+
// external item to be parents).
375+
visible_parents_map: |tcx, ()| {
376+
use rustc_data_structures::fx::FxHashSet;
377+
use std::collections::VecDeque;
373378

374-
let mut visible_parent_map: DefIdMap<DefId> = Default::default();
375-
// This is a secondary visible_parent_map, storing the DefId of parents that re-export
376-
// the child as `_`. Since we prefer parents that don't do this, merge this map at the
377-
// end, only if we're missing any keys from the former.
378-
let mut fallback_map: DefIdMap<DefId> = Default::default();
379+
let mut visible_parents_map: DefIdMap<SmallVec<[_; 4]>> = DefIdMap::default();
379380

380381
// Issue 46112: We want the map to prefer the shortest
381382
// paths when reporting the path to an item. Therefore we
@@ -387,62 +388,88 @@ pub(in crate::rmeta) fn provide(providers: &mut Providers) {
387388
// only get paths that are locally minimal with respect to
388389
// whatever crate we happened to encounter first in this
389390
// traversal, but not globally minimal across all crates.
390-
let bfs_queue = &mut VecDeque::new();
391-
392-
for &cnum in tcx.crates(()) {
393-
// Ignore crates without a corresponding local `extern crate` item.
394-
if tcx.missing_extern_crate_item(cnum) {
395-
continue;
391+
let mut bfs_queue = VecDeque::default();
392+
393+
bfs_queue.extend(
394+
tcx.crates(())
395+
.into_iter()
396+
// Ignore crates without a corresponding local `extern crate` item.
397+
.filter(|cnum| !tcx.missing_extern_crate_item(**cnum))
398+
.map(|cnum| DefId { krate: *cnum, index: super::CRATE_DEF_INDEX }),
399+
);
400+
401+
// Iterate over graph using BFS.
402+
// Filter out any non-public items.
403+
while let Some(parent) = bfs_queue.pop_front() {
404+
if matches!(tcx.def_kind(parent), DefKind::Mod | DefKind::Enum | DefKind::Trait) {
405+
for (child_def_id, child_name) in tcx
406+
.module_children(parent)
407+
.iter()
408+
.filter(|child| child.vis.is_public())
409+
.filter_map(|child| Some((child.res.opt_def_id()?, child.ident.name)))
410+
{
411+
visible_parents_map
412+
.entry(child_def_id)
413+
.or_insert_with(|| {
414+
// If we encounter node the first time
415+
// add it to queue for next iterations
416+
bfs_queue.push_back(child_def_id);
417+
Default::default()
418+
})
419+
.push((parent, child_name));
420+
}
396421
}
397-
398-
bfs_queue.push_back(cnum.as_def_id());
399422
}
400423

401-
let mut add_child = |bfs_queue: &mut VecDeque<_>, child: &ModChild, parent: DefId| {
402-
if !child.vis.is_public() {
403-
return;
404-
}
424+
// Iterate over parents vector to remove duplicate elements
425+
// while preserving order
426+
let mut dedup_set = FxHashSet::default();
427+
for (_, parents) in &mut visible_parents_map {
428+
parents.retain(|parent| dedup_set.insert(*parent));
429+
430+
// Reuse hashset allocation.
431+
dedup_set.clear();
432+
}
405433

406-
if let Some(def_id) = child.res.opt_def_id() {
407-
if child.ident.name == kw::Underscore {
408-
fallback_map.insert(def_id, parent);
409-
return;
434+
visible_parents_map
435+
},
436+
best_visible_parent: |tcx, child| {
437+
// Use `min_by_key` because it returns
438+
// first match in case keys are equal
439+
tcx.visible_parents_map(())
440+
.get(&child)?
441+
.into_iter()
442+
.min_by_key(|(parent, child_name)| {
443+
// If this is just regular export in another module, assign it a neutral score.
444+
let mut score = 0;
445+
446+
// If child and parent are local, we prefer them
447+
if child.is_local() && parent.is_local() {
448+
score += 1;
410449
}
411450

412-
match visible_parent_map.entry(def_id) {
413-
Entry::Occupied(mut entry) => {
414-
// If `child` is defined in crate `cnum`, ensure
415-
// that it is mapped to a parent in `cnum`.
416-
if def_id.is_local() && entry.get().is_local() {
417-
entry.insert(parent);
418-
}
419-
}
420-
Entry::Vacant(entry) => {
421-
entry.insert(parent);
422-
if matches!(
423-
child.res,
424-
Res::Def(DefKind::Mod | DefKind::Enum | DefKind::Trait, _)
425-
) {
426-
bfs_queue.push_back(def_id);
427-
}
428-
}
451+
// Even if child and parent are local, if parent is `#[doc(hidden)]`
452+
// We reduce their score to avoid showing items not popping in documentation.
453+
if is_doc_hidden(tcx, *parent) {
454+
score -= 2;
429455
}
430-
}
431-
};
432456

433-
while let Some(def) = bfs_queue.pop_front() {
434-
for child in tcx.module_children(def).iter() {
435-
add_child(bfs_queue, child, def);
436-
}
437-
}
457+
// If parent identifier is _ we prefer it only as last resort if other items are not available
458+
if let Some(name) = tcx.opt_item_name(*parent)
459+
&& name == kw::Underscore
460+
{
461+
score -= 3;
462+
}
438463

439-
// Fill in any missing entries with the (less preferable) path ending in `::_`.
440-
// We still use this path in a diagnostic that suggests importing `::*`.
441-
for (child, parent) in fallback_map {
442-
visible_parent_map.entry(child).or_insert(parent);
443-
}
464+
// If the name of the child inside of the parent (i.e. its re-exported name),
465+
// then also don't prefer that...
466+
if *child_name == kw::Underscore {
467+
score -= 3;
468+
}
444469

445-
visible_parent_map
470+
-score
471+
})
472+
.map(|(parent, _)| *parent)
446473
},
447474

448475
dependency_formats: |tcx, ()| Lrc::new(crate::dependency_format::calculate(tcx)),

compiler/rustc_middle/src/middle/stability.rs

+1-1
Original file line numberDiff line numberDiff line change
@@ -369,7 +369,7 @@ impl<'tcx> TyCtxt<'tcx> {
369369
let depr_attr = &depr_entry.attr;
370370
if !skip || depr_attr.is_since_rustc_version {
371371
// Calculating message for lint involves calling `self.def_path_str`.
372-
// Which by default to calculate visible path will invoke expensive `visible_parent_map` query.
372+
// Which by default to calculate visible path will invoke expensive `visible_parents_map` query.
373373
// So we skip message calculation altogether, if lint is allowed.
374374
let is_in_effect = deprecation_in_effect(depr_attr);
375375
let lint = deprecation_lint(is_in_effect);

compiler/rustc_middle/src/query/mod.rs

+4-1
Original file line numberDiff line numberDiff line change
@@ -1668,10 +1668,13 @@ rustc_queries! {
16681668
desc { "calculating the missing lang items in a crate" }
16691669
separate_provide_extern
16701670
}
1671-
query visible_parent_map(_: ()) -> DefIdMap<DefId> {
1671+
query visible_parents_map(_: ()) -> DefIdMap<smallvec::SmallVec<[(DefId, Symbol); 4]>> {
16721672
storage(ArenaCacheSelector<'tcx>)
16731673
desc { "calculating the visible parent map" }
16741674
}
1675+
query best_visible_parent(def_id: DefId) -> Option<DefId> {
1676+
desc { |tcx| "computing the best visible parent for `{}`", tcx.def_path_str(def_id) }
1677+
}
16751678
query trimmed_def_paths(_: ()) -> FxHashMap<DefId, Symbol> {
16761679
storage(ArenaCacheSelector<'tcx>)
16771680
desc { "calculating trimmed def paths" }

compiler/rustc_middle/src/ty/print/pretty.rs

+3-5
Original file line numberDiff line numberDiff line change
@@ -390,8 +390,6 @@ pub trait PrettyPrinter<'tcx>:
390390
return Ok((self, false));
391391
}
392392

393-
let visible_parent_map = self.tcx().visible_parent_map(());
394-
395393
let mut cur_def_key = self.tcx().def_key(def_id);
396394
debug!("try_print_visible_def_path: cur_def_key={:?}", cur_def_key);
397395

@@ -407,7 +405,7 @@ pub trait PrettyPrinter<'tcx>:
407405
cur_def_key = self.tcx().def_key(parent);
408406
}
409407

410-
let Some(visible_parent) = visible_parent_map.get(&def_id).cloned() else {
408+
let Some(visible_parent) = self.tcx().best_visible_parent(def_id) else {
411409
return Ok((self, false));
412410
};
413411

@@ -433,7 +431,7 @@ pub trait PrettyPrinter<'tcx>:
433431
// `std::os::unix` reexports the contents of `std::sys::unix::ext`. `std::sys` is
434432
// private so the "true" path to `CommandExt` isn't accessible.
435433
//
436-
// In this case, the `visible_parent_map` will look something like this:
434+
// In this case, the `visible_parents_map` will look something like this:
437435
//
438436
// (child) -> (parent)
439437
// `std::sys::unix::ext::process::CommandExt` -> `std::sys::unix::ext::process`
@@ -453,7 +451,7 @@ pub trait PrettyPrinter<'tcx>:
453451
// do this, we compare the parent of `std::sys::unix::ext` (`std::sys::unix`) with
454452
// the visible parent (`std::os`). If these do not match, then we iterate over
455453
// the children of the visible parent (as was done when computing
456-
// `visible_parent_map`), looking for the specific child we currently have and then
454+
// `visible_parents_map`), looking for the specific child we currently have and then
457455
// have access to the re-exported name.
458456
DefPathData::TypeNs(ref mut name) if Some(visible_parent) != actual_parent => {
459457
// Item might be re-exported several times, but filter for the one

compiler/rustc_typeck/src/check/method/suggest.rs

+5-7
Original file line numberDiff line numberDiff line change
@@ -1642,17 +1642,15 @@ impl<'a, 'tcx> FnCtxt<'a, 'tcx> {
16421642
}
16431643

16441644
fn suggest_use_candidates(&self, err: &mut Diagnostic, msg: String, candidates: Vec<DefId>) {
1645-
let parent_map = self.tcx.visible_parent_map(());
1646-
16471645
// Separate out candidates that must be imported with a glob, because they are named `_`
16481646
// and cannot be referred with their identifier.
16491647
let (candidates, globs): (Vec<_>, Vec<_>) = candidates.into_iter().partition(|trait_did| {
1650-
if let Some(parent_did) = parent_map.get(trait_did) {
1648+
if let Some(parent_did) = self.tcx.best_visible_parent(trait_did) {
16511649
// If the item is re-exported as `_`, we should suggest a glob-import instead.
1652-
if *parent_did != self.tcx.parent(*trait_did)
1650+
if parent_did != self.tcx.parent(*trait_did)
16531651
&& self
16541652
.tcx
1655-
.module_children(*parent_did)
1653+
.module_children(parent_did)
16561654
.iter()
16571655
.filter(|child| child.res.opt_def_id() == Some(*trait_did))
16581656
.all(|child| child.ident.name == kw::Underscore)
@@ -1673,10 +1671,10 @@ impl<'a, 'tcx> FnCtxt<'a, 'tcx> {
16731671
});
16741672

16751673
let glob_path_strings = globs.iter().map(|trait_did| {
1676-
let parent_did = parent_map.get(trait_did).unwrap();
1674+
let parent_did = self.tcx.best_visible_parent(trait_did).unwrap();
16771675
format!(
16781676
"use {}::*; // trait {}\n",
1679-
with_crate_prefix!(self.tcx.def_path_str(*parent_did)),
1677+
with_crate_prefix!(self.tcx.def_path_str(parent_did)),
16801678
self.tcx.item_name(*trait_did),
16811679
)
16821680
});

src/test/ui/proc-macro/issue-37788.rs

+1-1
Original file line numberDiff line numberDiff line change
@@ -4,6 +4,6 @@
44
extern crate test_macros;
55

66
fn main() {
7-
// Test that constructing the `visible_parent_map` (in `cstore_impl.rs`) does not ICE.
7+
// Test that constructing the `visible_parents_map` (in `cstore_impl.rs`) does not ICE.
88
std::cell::Cell::new(0) //~ ERROR mismatched types
99
}

src/test/ui/proc-macro/issue-37788.stderr

+1-1
Original file line numberDiff line numberDiff line change
@@ -3,7 +3,7 @@ error[E0308]: mismatched types
33
|
44
LL | fn main() {
55
| - expected `()` because of default return type
6-
LL | // Test that constructing the `visible_parent_map` (in `cstore_impl.rs`) does not ICE.
6+
LL | // Test that constructing the `visible_parents_map` (in `cstore_impl.rs`) does not ICE.
77
LL | std::cell::Cell::new(0)
88
| ^^^^^^^^^^^^^^^^^^^^^^^- help: consider using a semicolon here: `;`
99
| |

src/test/ui/suggestions/dont-suggest-doc-hidden-variant-for-enum/dont-suggest-doc-hidden-variant-for-enum.stderr

+6-4
Original file line numberDiff line numberDiff line change
@@ -2,14 +2,16 @@ error[E0308]: mismatched types
22
--> $DIR/dont-suggest-doc-hidden-variant-for-enum.rs:6:26
33
|
44
LL | let x: Option<i32> = 1i32;
5-
| ----------- ^^^^
6-
| | |
7-
| | expected enum `Option`, found `i32`
8-
| | help: try using a variant of the expected enum: `Some(1i32)`
5+
| ----------- ^^^^ expected enum `Option`, found `i32`
6+
| |
97
| expected due to this
108
|
119
= note: expected enum `Option<i32>`
1210
found type `i32`
11+
help: try wrapping the expression in `Some`
12+
|
13+
LL | let x: Option<i32> = Some(1i32);
14+
| +++++ +
1315

1416
error: aborting due to previous error
1517

0 commit comments

Comments
 (0)