Prefer suggestion paths which are not doc-hidden

compiler/rustc_ast/src/attr/mod.rs

@@ -580,3 +580,12 @@ impl NestedMetaItem {
         MetaItem::from_tokens(tokens).map(NestedMetaItem::MetaItem)
     }
 }
+
+/// Return true if the attributes contain `#[doc(hidden)]`
+pub fn is_doc_hidden(attrs: &[ast::Attribute]) -> bool {
+    attrs
+        .iter()
+        .filter(|attr| attr.has_name(sym::doc))
+        .filter_map(ast::Attribute::meta_item_list)
+        .any(|l| list_contains_name(&l, sym::hidden))
+}

compiler/rustc_data_structures/src/stable_hasher.rs

@@ -346,9 +346,11 @@ where
     }
 }
 
-impl<A, CTX> HashStable<CTX> for SmallVec<[A; 1]>
+impl<T, CTX, const N: usize> HashStable<CTX> for SmallVec<[T; N]>
 where
-    A: HashStable<CTX>,
+    T: HashStable<CTX>,
+    [T; N]: smallvec::Array,
+    <[T; N] as smallvec::Array>::Item: HashStable<CTX>,
 {
     #[inline]
     fn hash_stable(&self, ctx: &mut CTX, hasher: &mut StableHasher) {

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

@@ -133,8 +133,8 @@ provide! { <'tcx> tcx, def_id, other, cdata,
     generator_kind => { cdata.generator_kind(def_id.index) }
     opt_def_kind => { Some(cdata.def_kind(def_id.index)) }
     def_span => { cdata.get_span(def_id.index, &tcx.sess) }
-    def_ident_span => {
-        cdata.try_item_ident(def_id.index, &tcx.sess).ok().map(|ident| ident.span)
+    def_ident => {
+        cdata.try_item_ident(def_id.index, &tcx.sess).ok()
     }
     lookup_stability => {
         cdata.get_stability(def_id.index).map(|s| tcx.intern_stability(s))
@@ -290,15 +290,11 @@ pub fn provide(providers: &mut Providers) {
         // external item that is visible from at least one local module) to a
         // sufficiently visible parent (considering modules that re-export the
         // external item to be parents).
-        visible_parent_map: |tcx, ()| {
-            use std::collections::hash_map::Entry;
-            use std::collections::vec_deque::VecDeque;
+        visible_parents_map: |tcx, ()| {
+            use rustc_data_structures::fx::FxHashSet;
+            use std::collections::VecDeque;
 
-            let mut visible_parent_map: DefIdMap<DefId> = Default::default();
-            // This is a secondary visible_parent_map, storing the DefId of parents that re-export
-            // the child as `_`. Since we prefer parents that don't do this, merge this map at the
-            // end, only if we're missing any keys from the former.
-            let mut fallback_map: DefIdMap<DefId> = Default::default();
+            let mut visible_parents_map: DefIdMap<SmallVec<[DefId; 4]>> = DefIdMap::default();
 
             // Issue 46112: We want the map to prefer the shortest
             // paths when reporting the path to an item. Therefore we
@@ -310,59 +306,81 @@ pub fn provide(providers: &mut Providers) {
             // only get paths that are locally minimal with respect to
             // whatever crate we happened to encounter first in this
             // traversal, but not globally minimal across all crates.
-            let bfs_queue = &mut VecDeque::new();
-
-            for &cnum in tcx.crates(()) {
-                // Ignore crates without a corresponding local `extern crate` item.
-                if tcx.missing_extern_crate_item(cnum) {
-                    continue;
+            let mut bfs_queue = VecDeque::default();
+
+            bfs_queue.extend(
+                tcx.crates(())
+                    .into_iter()
+                    // Ignore crates without a corresponding local `extern crate` item.
+                    .filter(|cnum| !tcx.missing_extern_crate_item(**cnum))
+                    .map(|cnum| DefId { krate: *cnum, index: CRATE_DEF_INDEX }),
+            );
+
+            // Iterate over graph using BFS.
+            // Filter out any non-public items.
+            while let Some(parent) = bfs_queue.pop_front() {
+                for child in tcx
+                    .item_children(parent)
+                    .iter()
+                    .filter(|child| child.vis.is_public())
+                    .filter_map(|child| child.res.opt_def_id())
+                {
+                    visible_parents_map
+                        .entry(child)
+                        .or_insert_with(|| {
+                            // If we encounter node the first time
+                            // add it to queue for next iterations
+                            bfs_queue.push_back(child);
+                            Default::default()
+                        })
+                        .push(parent);
                 }
+            }
 
-                bfs_queue.push_back(DefId { krate: cnum, index: CRATE_DEF_INDEX });
+            // Iterate over parents vector to remove duplicate elements
+            // while preserving order
+            let mut dedup_set = FxHashSet::default();
+            for (_, parents) in &mut visible_parents_map {
+                parents.retain(|parent| dedup_set.insert(*parent));
+
+                // Reuse hashset allocation.
+                dedup_set.clear();
             }
 
-            let mut add_child = |bfs_queue: &mut VecDeque<_>, export: &Export, parent: DefId| {
-                if !export.vis.is_public() {
-                    return;
-                }
+            visible_parents_map
+        },
+        best_visible_parent: |tcx, child| {
+            // Use `min_by_key` because it returns
+            // first match in case keys are equal
+            tcx.visible_parents_map(())
+                .get(&child)?
+                .into_iter()
+                .min_by_key(|parent| {
+                    // If this is just regular export in another module, assign it a neutral score.
+                    let mut score = 0;
+
+                    // If child and parent are local, we prefer them
+                    if child.is_local() && parent.is_local() {
+                        score += 1;
+                    }
 
-                if let Some(child) = export.res.opt_def_id() {
-                    if export.ident.name == kw::Underscore {
-                        fallback_map.insert(child, parent);
-                        return;
+                    // Even if child and parent are local, if parent is `#[doc(hidden)]`
+                    // We reduce their score to avoid showing items not popping in documentation.
+                    if ast::attr::is_doc_hidden(tcx.item_attrs(**parent)) {
+                        score -= 2;
                     }
 
-                    match visible_parent_map.entry(child) {
-                        Entry::Occupied(mut entry) => {
-                            // If `child` is defined in crate `cnum`, ensure
-                            // that it is mapped to a parent in `cnum`.
-                            if child.is_local() && entry.get().is_local() {
-                                entry.insert(parent);
-                            }
-                        }
-                        Entry::Vacant(entry) => {
-                            entry.insert(parent);
-                            bfs_queue.push_back(child);
+                    // If parent identifier is _ we prefer it only as last resort if other items are not available
+                    if let Some(ident) = tcx.def_ident(**parent) {
+                        if ident.name == kw::Underscore {
+                            score -= 3;
                         }
                     }
-                }
-            };
-
-            while let Some(def) = bfs_queue.pop_front() {
-                for child in tcx.item_children(def).iter() {
-                    add_child(bfs_queue, child, def);
-                }
-            }
 
-            // Fill in any missing entries with the (less preferable) path ending in `::_`.
-            // We still use this path in a diagnostic that suggests importing `::*`.
-            for (child, parent) in fallback_map {
-                visible_parent_map.entry(child).or_insert(parent);
-            }
-
-            visible_parent_map
+                    -score
+                })
+                .map(ToOwned::to_owned)
         },
-
         dependency_formats: |tcx, ()| Lrc::new(crate::dependency_format::calculate(tcx)),
         has_global_allocator: |tcx, cnum| {
             assert_eq!(cnum, LOCAL_CRATE);

compiler/rustc_middle/src/query/mod.rs

@@ -977,8 +977,8 @@ rustc_queries! {
     }
 
     /// Gets the span for the identifier of the definition.
-    query def_ident_span(def_id: DefId) -> Option<Span> {
-        desc { |tcx| "looking up span for `{}`'s identifier", tcx.def_path_str(def_id) }
+    query def_ident(def_id: DefId) -> Option<Ident> {
+        desc { |tcx| "looking up ident for `{}`'s identifier", tcx.def_path_str(def_id) }
         separate_provide_extern
     }
 
@@ -1552,10 +1552,14 @@ rustc_queries! {
         desc { "calculating the missing lang items in a crate" }
         separate_provide_extern
     }
-    query visible_parent_map(_: ()) -> DefIdMap<DefId> {
+
+    query visible_parents_map(_: ()) -> DefIdMap<smallvec::SmallVec<[DefId; 4]>> {
         storage(ArenaCacheSelector<'tcx>)
         desc { "calculating the visible parent map" }
     }
+    query best_visible_parent(child: DefId) -> Option<DefId> {
+        desc { "calculating best visible parent" }
+    }
     query trimmed_def_paths(_: ()) -> FxHashMap<DefId, Symbol> {
         storage(ArenaCacheSelector<'tcx>)
         desc { "calculating trimmed def paths" }

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

@@ -387,8 +387,6 @@ pub trait PrettyPrinter<'tcx>:
             return Ok((self, false));
         }
 
-        let visible_parent_map = self.tcx().visible_parent_map(());
-
         let mut cur_def_key = self.tcx().def_key(def_id);
         debug!("try_print_visible_def_path: cur_def_key={:?}", cur_def_key);
 
@@ -404,7 +402,7 @@ pub trait PrettyPrinter<'tcx>:
             cur_def_key = self.tcx().def_key(parent);
         }
 
-        let visible_parent = match visible_parent_map.get(&def_id).cloned() {
+        let visible_parent = match self.tcx().best_visible_parent(def_id) {
             Some(parent) => parent,
             None => return Ok((self, false)),
         };
@@ -431,7 +429,7 @@ pub trait PrettyPrinter<'tcx>:
             // `std::os::unix` rexports the contents of `std::sys::unix::ext`. `std::sys` is
             // private so the "true" path to `CommandExt` isn't accessible.
             //
-            // In this case, the `visible_parent_map` will look something like this:
+            // In this case, the `visible_parents_map` will look something like this:
             //
             // (child) -> (parent)
             // `std::sys::unix::ext::process::CommandExt` -> `std::sys::unix::ext::process`
@@ -451,7 +449,7 @@ pub trait PrettyPrinter<'tcx>:
             // do this, we compare the parent of `std::sys::unix::ext` (`std::sys::unix`) with
             // the visible parent (`std::os`). If these do not match, then we iterate over
             // the children of the visible parent (as was done when computing
-            // `visible_parent_map`), looking for the specific child we currently have and then
+            // `visible_parents_map`), looking for the specific child we currently have and then
             // have access to the re-exported name.
             DefPathData::TypeNs(ref mut name) if Some(visible_parent) != actual_parent => {
                 // Item might be re-exported several times, but filter for the one

compiler/rustc_middle/src/ty/query.rs

@@ -30,7 +30,7 @@ use crate::traits::specialization_graph;
 use crate::traits::{self, ImplSource};
 use crate::ty::subst::{GenericArg, SubstsRef};
 use crate::ty::util::AlwaysRequiresDrop;
-use crate::ty::{self, AdtSizedConstraint, CrateInherentImpls, ParamEnvAnd, Ty, TyCtxt};
+use crate::ty::{self, AdtSizedConstraint, CrateInherentImpls, Ident, ParamEnvAnd, Ty, TyCtxt};
 use rustc_ast::expand::allocator::AllocatorKind;
 use rustc_data_structures::fx::{FxHashMap, FxHashSet, FxIndexMap};
 use rustc_data_structures::steal::Steal;

compiler/rustc_ty_utils/src/ty.rs

@@ -3,7 +3,7 @@ use rustc_hir as hir;
 use rustc_hir::def_id::{DefId, LocalDefId};
 use rustc_middle::ty::subst::Subst;
 use rustc_middle::ty::{self, Binder, Predicate, PredicateKind, ToPredicate, Ty, TyCtxt};
-use rustc_span::{sym, Span};
+use rustc_span::{sym, symbol::Ident};
 use rustc_trait_selection::traits;
 
 fn sized_constraint_for_ty<'tcx>(
@@ -216,19 +216,16 @@ fn associated_items(tcx: TyCtxt<'_>, def_id: DefId) -> ty::AssocItems<'_> {
     ty::AssocItems::new(items)
 }
 
-fn def_ident_span(tcx: TyCtxt<'_>, def_id: DefId) -> Option<Span> {
-    tcx.hir()
-        .get_if_local(def_id)
-        .and_then(|node| match node {
-            // A `Ctor` doesn't have an identifier itself, but its parent
-            // struct/variant does. Compare with `hir::Map::opt_span`.
-            hir::Node::Ctor(ctor) => ctor
-                .ctor_hir_id()
-                .and_then(|ctor_id| tcx.hir().find(tcx.hir().get_parent_node(ctor_id)))
-                .and_then(|parent| parent.ident()),
-            _ => node.ident(),
-        })
-        .map(|ident| ident.span)
+fn def_ident(tcx: TyCtxt<'_>, def_id: DefId) -> Option<Ident> {
+    tcx.hir().get_if_local(def_id).and_then(|node| match node {
+        // A `Ctor` doesn't have an identifier itself, but its parent
+        // struct/variant does. Compare with `hir::Map::opt_span`.
+        hir::Node::Ctor(ctor) => ctor
+            .ctor_hir_id()
+            .and_then(|ctor_id| tcx.hir().find(tcx.hir().get_parent_node(ctor_id)))
+            .and_then(|parent| parent.ident()),
+        _ => node.ident(),
+    })
 }
 
 /// If the given `DefId` describes an item belonging to a trait,
@@ -624,7 +621,7 @@ pub fn provide(providers: &mut ty::query::Providers) {
         associated_item_def_ids,
         associated_items,
         adt_sized_constraint,
-        def_ident_span,
+        def_ident,
         param_env,
         param_env_reveal_all_normalized,
         trait_of_item,

compiler/rustc_typeck/src/check/fn_ctxt/checks.rs

@@ -197,8 +197,8 @@ impl<'a, 'tcx> FnCtxt<'a, 'tcx> {
             }
 
             if let Some(def_id) = def_id {
-                if let Some(def_span) = tcx.def_ident_span(def_id) {
-                    let mut spans: MultiSpan = def_span.into();
+                if let Some(def_ident) = tcx.def_ident(def_id) {
+                    let mut spans: MultiSpan = def_ident.span.into();
 
                     let params = tcx
                         .hir()

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

@@ -12,7 +12,7 @@ use rustc_hir::{ExprKind, Node, QPath};
 use rustc_infer::infer::type_variable::{TypeVariableOrigin, TypeVariableOriginKind};
 use rustc_middle::ty::fast_reject::{simplify_type, SimplifyParams, StripReferences};
 use rustc_middle::ty::print::with_crate_prefix;
-use rustc_middle::ty::{self, DefIdTree, ToPredicate, Ty, TyCtxt, TypeFoldable};
+use rustc_middle::ty::{self, ToPredicate, Ty, TyCtxt, TypeFoldable};
 use rustc_span::lev_distance;
 use rustc_span::symbol::{kw, sym, Ident};
 use rustc_span::{source_map, FileName, MultiSpan, Span, Symbol};
@@ -1310,17 +1310,15 @@ impl<'a, 'tcx> FnCtxt<'a, 'tcx> {
         mut msg: String,
         candidates: Vec<DefId>,
     ) {
-        let parent_map = self.tcx.visible_parent_map(());
-
         // Separate out candidates that must be imported with a glob, because they are named `_`
         // and cannot be referred with their identifier.
         let (candidates, globs): (Vec<_>, Vec<_>) = candidates.into_iter().partition(|trait_did| {
-            if let Some(parent_did) = parent_map.get(trait_did) {
+            if let Some(parent_did) = self.tcx.best_visible_parent(*trait_did) {
                 // If the item is re-exported as `_`, we should suggest a glob-import instead.
-                if Some(*parent_did) != self.tcx.parent(*trait_did)
+                if Some(parent_did) != self.tcx.best_visible_parent(*trait_did)
                     && self
                         .tcx
-                        .item_children(*parent_did)
+                        .item_children(parent_did)
                         .iter()
                         .filter(|child| child.res.opt_def_id() == Some(*trait_did))
                         .all(|child| child.ident.name == kw::Underscore)
@@ -1347,14 +1345,14 @@ impl<'a, 'tcx> FnCtxt<'a, 'tcx> {
             });
 
             let glob_path_strings = globs.iter().map(|trait_did| {
-                let parent_did = parent_map.get(trait_did).unwrap();
-
                 // Produce an additional newline to separate the new use statement
                 // from the directly following item.
                 let additional_newline = if found_use { "" } else { "\n" };
                 format!(
                     "use {}::*; // trait {}\n{}",
-                    with_crate_prefix(|| self.tcx.def_path_str(*parent_did)),
+                    with_crate_prefix(|| self
+                        .tcx
+                        .def_path_str(self.tcx.best_visible_parent(*trait_did).unwrap())),
                     self.tcx.item_name(*trait_did),
                     additional_newline
                 )
@@ -1385,19 +1383,19 @@ impl<'a, 'tcx> FnCtxt<'a, 'tcx> {
             for (i, trait_did) in
                 globs.iter().take(limit.saturating_sub(candidates.len())).enumerate()
             {
-                let parent_did = parent_map.get(trait_did).unwrap();
+                let parent_did = self.tcx.best_visible_parent(*trait_did).unwrap();
 
                 if candidates.len() + globs.len() > 1 {
                     msg.push_str(&format!(
                         "\ncandidate #{}: `use {}::*; // trait {}`",
                         candidates.len() + i + 1,
-                        with_crate_prefix(|| self.tcx.def_path_str(*parent_did)),
+                        with_crate_prefix(|| self.tcx.def_path_str(parent_did)),
                         self.tcx.item_name(*trait_did),
                     ));
                 } else {
                     msg.push_str(&format!(
                         "\n`use {}::*; // trait {}`",
-                        with_crate_prefix(|| self.tcx.def_path_str(*parent_did)),
+                        with_crate_prefix(|| self.tcx.def_path_str(parent_did)),
                         self.tcx.item_name(*trait_did),
                     ));
                 }

compiler/rustc_typeck/src/check/pat.rs

@@ -1025,7 +1025,8 @@ impl<'a, 'tcx> FnCtxt<'a, 'tcx> {
         };
         let last_subpat_span = *subpat_spans.last().unwrap();
         let res_span = self.tcx.def_span(res.def_id());
-        let def_ident_span = self.tcx.def_ident_span(res.def_id()).unwrap_or(res_span);
+        let def_ident_span =
+            self.tcx.def_ident(res.def_id()).map(|ident| ident.span).unwrap_or(res_span);
         let field_def_spans = if fields.is_empty() {
             vec![res_span]
         } else {