Auto merge of #109500 - petrochenkov:modchainld, r=oli-obk

resolve: Preserve reexport chains in `ModChild`ren

This may be potentially useful for
- avoiding uses of `hir::ItemKind::Use` (which usually lead to correctness issues)
- preserving documentation comments on all reexports, including those from other crates
- preserving and checking stability/deprecation info on reexports
- all kinds of diagnostics

The second commit then migrates some hacky logic from rustdoc to `module_reexports` to make it simpler and more correct.
Ideally rustdoc should use `module_reexports` immediately at the top level, so `hir::ItemKind::Use`s are never used.
The second commit also fixes issues with https://github.com/rust-lang/rust/pull/109330 and therefore
Fixes https://github.com/rust-lang/rust/issues/109631
Fixes https://github.com/rust-lang/rust/issues/109614
Fixes https://github.com/rust-lang/rust/issues/109424

compiler/rustc_metadata/src/rmeta/decoder.rs

@@ -991,7 +991,7 @@ impl<'a, 'tcx> CrateMetadataRef<'a> {
             _ => false,
         };
 
-        ModChild { ident, res, vis, span, macro_rules }
+        ModChild { ident, res, vis, span, macro_rules, reexport_chain: Default::default() }
     }
 
     /// Iterates over all named children of the given module,

compiler/rustc_metadata/src/rmeta/encoder.rs

@@ -1327,8 +1327,8 @@ impl<'a, 'tcx> EncodeContext<'a, 'tcx> {
                 }
             }));
 
-            if let Some(reexports) = tcx.module_reexports(local_def_id) {
-                assert!(!reexports.is_empty());
+            let reexports = tcx.module_reexports(local_def_id);
+            if !reexports.is_empty() {
                 record_array!(self.tables.module_reexports[def_id] <- reexports);
             }
         }

compiler/rustc_middle/src/arena.rs

@@ -119,6 +119,7 @@ macro_rules! arena_types {
             [] external_constraints: rustc_middle::traits::solve::ExternalConstraintsData<'tcx>,
             [decode] doc_link_resolutions: rustc_hir::def::DocLinkResMap,
             [] closure_kind_origin: (rustc_span::Span, rustc_middle::hir::place::Place<'tcx>),
+            [] mod_child: rustc_middle::metadata::ModChild,
         ]);
     )
 }

compiler/rustc_middle/src/metadata.rs

@@ -5,13 +5,34 @@ use rustc_macros::HashStable;
 use rustc_span::def_id::DefId;
 use rustc_span::symbol::Ident;
 use rustc_span::Span;
+use smallvec::SmallVec;
+
+/// A simplified version of `ImportKind` from resolve.
+/// `DefId`s here correspond to `use` and `extern crate` items themselves, not their targets.
+#[derive(Clone, Copy, Debug, TyEncodable, TyDecodable, HashStable)]
+pub enum Reexport {
+    Single(DefId),
+    Glob(DefId),
+    ExternCrate(DefId),
+    MacroUse,
+    MacroExport,
+}
+
+impl Reexport {
+    pub fn id(self) -> Option<DefId> {
+        match self {
+            Reexport::Single(id) | Reexport::Glob(id) | Reexport::ExternCrate(id) => Some(id),
+            Reexport::MacroUse | Reexport::MacroExport => None,
+        }
+    }
+}
 
 /// This structure is supposed to keep enough data to re-create `NameBinding`s for other crates
 /// during name resolution. Right now the bindings are not recreated entirely precisely so we may
 /// need to add more data in the future to correctly support macros 2.0, for example.
 /// Module child can be either a proper item or a reexport (including private imports).
 /// In case of reexport all the fields describe the reexport item itself, not what it refers to.
-#[derive(Copy, Clone, Debug, TyEncodable, TyDecodable, HashStable)]
+#[derive(Debug, TyEncodable, TyDecodable, HashStable)]
 pub struct ModChild {
     /// Name of the item.
     pub ident: Ident,
@@ -24,4 +45,7 @@ pub struct ModChild {
     pub span: Span,
     /// A proper `macro_rules` item (not a reexport).
     pub macro_rules: bool,
+    /// Reexport chain linking this module child to its original reexported item.
+    /// Empty if the module child is a proper item.
+    pub reexport_chain: SmallVec<[Reexport; 2]>,
 }

compiler/rustc_middle/src/query/erase.rs

@@ -235,7 +235,6 @@ trivial! {
     rustc_hir::OwnerId,
     rustc_hir::Upvar,
     rustc_index::bit_set::FiniteBitSet<u32>,
-    rustc_middle::metadata::ModChild,
     rustc_middle::middle::dependency_format::Linkage,
     rustc_middle::middle::exported_symbols::SymbolExportInfo,
     rustc_middle::middle::resolve_bound_vars::ObjectLifetimeDefault,

compiler/rustc_middle/src/query/mod.rs

@@ -1510,7 +1510,7 @@ rustc_queries! {
         desc { "getting traits in scope at a block" }
     }
 
-    query module_reexports(def_id: LocalDefId) -> Option<&'tcx [ModChild]> {
+    query module_reexports(def_id: LocalDefId) -> &'tcx [ModChild] {
         desc { |tcx| "looking up reexports of module `{}`", tcx.def_path_str(def_id.to_def_id()) }
     }
 

compiler/rustc_middle/src/ty/context.rs

@@ -2502,7 +2502,7 @@ pub struct DeducedParamAttrs {
 
 pub fn provide(providers: &mut ty::query::Providers) {
     providers.module_reexports =
-        |tcx, id| tcx.resolutions(()).reexport_map.get(&id).map(|v| &v[..]);
+        |tcx, id| tcx.resolutions(()).reexport_map.get(&id).map_or(&[], |v| &v[..]);
     providers.maybe_unused_trait_imports =
         |tcx, ()| &tcx.resolutions(()).maybe_unused_trait_imports;
     providers.names_imported_by_glob_use = |tcx, id| {

compiler/rustc_privacy/src/lib.rs

@@ -515,16 +515,12 @@ impl<'tcx> EmbargoVisitor<'tcx> {
             let vis = self.tcx.local_visibility(item_id.owner_id.def_id);
             self.update_macro_reachable_def(item_id.owner_id.def_id, def_kind, vis, defining_mod);
         }
-        if let Some(exports) = self.tcx.module_reexports(module_def_id) {
-            for export in exports {
-                if export.vis.is_accessible_from(defining_mod, self.tcx) {
-                    if let Res::Def(def_kind, def_id) = export.res {
-                        if let Some(def_id) = def_id.as_local() {
-                            let vis = self.tcx.local_visibility(def_id);
-                            self.update_macro_reachable_def(def_id, def_kind, vis, defining_mod);
-                        }
-                    }
-                }
+        for export in self.tcx.module_reexports(module_def_id) {
+            if export.vis.is_accessible_from(defining_mod, self.tcx)
+                && let Res::Def(def_kind, def_id) = export.res
+                && let Some(def_id) = def_id.as_local() {
+                let vis = self.tcx.local_visibility(def_id);
+                self.update_macro_reachable_def(def_id, def_kind, vis, defining_mod);
             }
         }
     }

compiler/rustc_resolve/src/build_reduced_graph.rs

@@ -931,7 +931,7 @@ impl<'a, 'b, 'tcx> BuildReducedGraphVisitor<'a, 'b, 'tcx> {
     /// Builds the reduced graph for a single item in an external crate.
     fn build_reduced_graph_for_external_crate_res(&mut self, child: ModChild) {
         let parent = self.parent_scope.module;
-        let ModChild { ident, res, vis, span, macro_rules } = child;
+        let ModChild { ident, res, vis, span, macro_rules, .. } = child;
         let res = res.expect_non_local();
         let expansion = self.parent_scope.expansion;
         // Record primary definitions.

compiler/rustc_resolve/src/imports.rs

@@ -17,6 +17,7 @@ use rustc_data_structures::intern::Interned;
 use rustc_errors::{pluralize, struct_span_err, Applicability, MultiSpan};
 use rustc_hir::def::{self, DefKind, PartialRes};
 use rustc_middle::metadata::ModChild;
+use rustc_middle::metadata::Reexport;
 use rustc_middle::span_bug;
 use rustc_middle::ty;
 use rustc_session::lint::builtin::{
@@ -27,6 +28,7 @@ use rustc_span::edit_distance::find_best_match_for_name;
 use rustc_span::hygiene::LocalExpnId;
 use rustc_span::symbol::{kw, Ident, Symbol};
 use rustc_span::Span;
+use smallvec::SmallVec;
 
 use std::cell::Cell;
 use std::{mem, ptr};
@@ -190,6 +192,17 @@ impl<'a> Import<'a> {
             ImportKind::MacroUse | ImportKind::MacroExport => None,
         }
     }
+
+    fn simplify(&self, r: &Resolver<'_, '_>) -> Reexport {
+        let to_def_id = |id| r.local_def_id(id).to_def_id();
+        match self.kind {
+            ImportKind::Single { id, .. } => Reexport::Single(to_def_id(id)),
+            ImportKind::Glob { id, .. } => Reexport::Glob(to_def_id(id)),
+            ImportKind::ExternCrate { id, .. } => Reexport::ExternCrate(to_def_id(id)),
+            ImportKind::MacroUse => Reexport::MacroUse,
+            ImportKind::MacroExport => Reexport::MacroExport,
+        }
+    }
 }
 
 /// Records information about the resolution of a name in a namespace of a module.
@@ -1252,12 +1265,20 @@ impl<'a, 'tcx> Resolver<'a, 'tcx> {
 
             module.for_each_child(self, |this, ident, _, binding| {
                 if let Some(res) = this.is_reexport(binding) {
+                    let mut reexport_chain = SmallVec::new();
+                    let mut next_binding = binding;
+                    while let NameBindingKind::Import { binding, import, .. } = next_binding.kind {
+                        reexport_chain.push(import.simplify(this));
+                        next_binding = binding;
+                    }
+
                     reexports.push(ModChild {
                         ident,
                         res,
                         vis: binding.vis,
                         span: binding.span,
                         macro_rules: false,
+                        reexport_chain,
                     });
                 }
             });

library/std/src/collections/mod.rs

@@ -416,8 +416,10 @@ pub use alloc_crate::collections::{BTreeMap, BTreeSet, BinaryHeap};
 pub use alloc_crate::collections::{LinkedList, VecDeque};
 
 #[stable(feature = "rust1", since = "1.0.0")]
+#[doc(inline)]
 pub use self::hash_map::HashMap;
 #[stable(feature = "rust1", since = "1.0.0")]
+#[doc(inline)]
 pub use self::hash_set::HashSet;
 
 #[stable(feature = "try_reserve", since = "1.57.0")]

src/librustdoc/clean/inline.rs

@@ -153,7 +153,6 @@ pub(crate) fn try_inline_glob(
             let reexports = cx
                 .tcx
                 .module_reexports(current_mod)
-                .unwrap_or_default()
                 .iter()
                 .filter_map(|child| child.res.opt_def_id())
                 .collect();
@@ -558,7 +557,7 @@ fn build_module_items(
     // If we're re-exporting a re-export it may actually re-export something in
     // two namespaces, so the target may be listed twice. Make sure we only
     // visit each node at most once.
-    for &item in cx.tcx.module_children(did).iter() {
+    for item in cx.tcx.module_children(did).iter() {
         if item.vis.is_public() {
             let res = item.res.expect_non_local();
             if let Some(def_id) = res.opt_def_id()

src/librustdoc/clean/mod.rs

@@ -21,6 +21,7 @@ use rustc_hir::def_id::{DefId, DefIdMap, DefIdSet, LocalDefId, LOCAL_CRATE};
 use rustc_hir::PredicateOrigin;
 use rustc_hir_analysis::hir_ty_to_ty;
 use rustc_infer::infer::region_constraints::{Constraint, RegionConstraintData};
+use rustc_middle::metadata::Reexport;
 use rustc_middle::middle::resolve_bound_vars as rbv;
 use rustc_middle::ty::fold::TypeFolder;
 use rustc_middle::ty::InternalSubsts;
@@ -2056,141 +2057,44 @@ fn clean_bare_fn_ty<'tcx>(
     BareFunctionDecl { unsafety: bare_fn.unsafety, abi: bare_fn.abi, decl, generic_params }
 }
 
-/// Get DefId of of an item's user-visible parent.
-///
-/// "User-visible" should account for re-exporting and inlining, which is why this function isn't
-/// just `tcx.parent(def_id)`. If the provided `path` has more than one path element, the `DefId`
-/// of the second-to-last will be given.
-///
-/// ```text
-/// use crate::foo::Bar;
-///            ^^^ DefId of this item will be returned
-/// ```
-///
-/// If the provided path has only one item, `tcx.parent(def_id)` will be returned instead.
-fn get_path_parent_def_id(
-    tcx: TyCtxt<'_>,
-    def_id: DefId,
-    path: &hir::UsePath<'_>,
-) -> Option<DefId> {
-    if let [.., parent_segment, _] = &path.segments {
-        match parent_segment.res {
-            hir::def::Res::Def(_, parent_def_id) => Some(parent_def_id),
-            _ if parent_segment.ident.name == kw::Crate => {
-                // In case the "parent" is the crate, it'll give `Res::Err` so we need to
-                // circumvent it this way.
-                Some(tcx.parent(def_id))
-            }
-            _ => None,
-        }
-    } else {
-        // If the path doesn't have a parent, then the parent is the current module.
-        Some(tcx.parent(def_id))
-    }
-}
-
-/// This visitor is used to find an HIR Item based on its `use` path. This doesn't use the ordinary
-/// name resolver because it does not walk all the way through a chain of re-exports.
-pub(crate) struct OneLevelVisitor<'hir> {
-    map: rustc_middle::hir::map::Map<'hir>,
-    pub(crate) item: Option<&'hir hir::Item<'hir>>,
-    looking_for: Ident,
+pub(crate) fn reexport_chain<'tcx>(
+    tcx: TyCtxt<'tcx>,
+    import_def_id: LocalDefId,
     target_def_id: LocalDefId,
-}
-
-impl<'hir> OneLevelVisitor<'hir> {
-    pub(crate) fn new(map: rustc_middle::hir::map::Map<'hir>, target_def_id: LocalDefId) -> Self {
-        Self { map, item: None, looking_for: Ident::empty(), target_def_id }
-    }
-
-    pub(crate) fn find_target(
-        &mut self,
-        tcx: TyCtxt<'_>,
-        def_id: DefId,
-        path: &hir::UsePath<'_>,
-    ) -> Option<&'hir hir::Item<'hir>> {
-        let parent_def_id = get_path_parent_def_id(tcx, def_id, path)?;
-        let parent = self.map.get_if_local(parent_def_id)?;
-
-        // We get the `Ident` we will be looking for into `item`.
-        self.looking_for = path.segments[path.segments.len() - 1].ident;
-        // We reset the `item`.
-        self.item = None;
-
-        match parent {
-            hir::Node::Item(parent_item) => {
-                hir::intravisit::walk_item(self, parent_item);
-            }
-            hir::Node::Crate(m) => {
-                hir::intravisit::walk_mod(
-                    self,
-                    m,
-                    tcx.local_def_id_to_hir_id(parent_def_id.as_local().unwrap()),
-                );
-            }
-            _ => return None,
-        }
-        self.item
-    }
-}
-
-impl<'hir> hir::intravisit::Visitor<'hir> for OneLevelVisitor<'hir> {
-    type NestedFilter = rustc_middle::hir::nested_filter::All;
-
-    fn nested_visit_map(&mut self) -> Self::Map {
-        self.map
-    }
-
-    fn visit_item(&mut self, item: &'hir hir::Item<'hir>) {
-        if self.item.is_none()
-            && item.ident == self.looking_for
-            && (matches!(item.kind, hir::ItemKind::Use(_, _))
-                || item.owner_id.def_id == self.target_def_id)
+) -> &'tcx [Reexport] {
+    for child in tcx.module_reexports(tcx.local_parent(import_def_id)) {
+        if child.res.opt_def_id() == Some(target_def_id.to_def_id())
+            && child.reexport_chain[0].id() == Some(import_def_id.to_def_id())
         {
-            self.item = Some(item);
+            return &child.reexport_chain;
         }
     }
+    &[]
 }
 
-/// Because a `Use` item directly links to the imported item, we need to manually go through each
-/// import one by one. To do so, we go to the parent item and look for the `Ident` into it. Then,
-/// if we found the "end item" (the imported one), we stop there because we don't need its
-/// documentation. Otherwise, we repeat the same operation until we find the "end item".
+/// Collect attributes from the whole import chain.
 fn get_all_import_attributes<'hir>(
-    mut item: &hir::Item<'hir>,
     cx: &mut DocContext<'hir>,
+    import_def_id: LocalDefId,
     target_def_id: LocalDefId,
     is_inline: bool,
-    mut prev_import: LocalDefId,
 ) -> Vec<(Cow<'hir, ast::Attribute>, Option<DefId>)> {
-    let mut attributes: Vec<(Cow<'hir, ast::Attribute>, Option<DefId>)> = Vec::new();
+    let mut attrs = Vec::new();
     let mut first = true;
-    let hir_map = cx.tcx.hir();
-    let mut visitor = OneLevelVisitor::new(hir_map, target_def_id);
-    let mut visited = FxHashSet::default();
-
-    // If the item is an import and has at least a path with two parts, we go into it.
-    while let hir::ItemKind::Use(path, _) = item.kind && visited.insert(item.hir_id()) {
-        let import_parent = cx.tcx.opt_local_parent(prev_import).map(|def_id| def_id.to_def_id());
+    for def_id in reexport_chain(cx.tcx, import_def_id, target_def_id)
+        .iter()
+        .flat_map(|reexport| reexport.id())
+    {
+        let import_attrs = inline::load_attrs(cx, def_id);
         if first {
             // This is the "original" reexport so we get all its attributes without filtering them.
-            attributes = hir_map.attrs(item.hir_id())
-                .iter()
-                .map(|attr| (Cow::Borrowed(attr), import_parent))
-                .collect::<Vec<_>>();
+            attrs = import_attrs.iter().map(|attr| (Cow::Borrowed(attr), Some(def_id))).collect();
             first = false;
         } else {
-            add_without_unwanted_attributes(&mut attributes, hir_map.attrs(item.hir_id()), is_inline, import_parent);
+            add_without_unwanted_attributes(&mut attrs, import_attrs, is_inline, Some(def_id));
         }
-
-        if let Some(i) = visitor.find_target(cx.tcx, item.owner_id.def_id.to_def_id(), path) {
-            item = i;
-        } else {
-            break;
-        }
-        prev_import = item.owner_id.def_id;
     }
-    attributes
+    attrs
 }
 
 fn filter_tokens_from_list(
@@ -2375,39 +2279,24 @@ fn clean_maybe_renamed_item<'tcx>(
             _ => unreachable!("not yet converted"),
         };
 
-        let attrs = if let Some(import_id) = import_id &&
-            let Some(hir::Node::Item(use_node)) = cx.tcx.hir().find_by_def_id(import_id)
-        {
+        let target_attrs = inline::load_attrs(cx, def_id);
+        let attrs = if let Some(import_id) = import_id {
             let is_inline = inline::load_attrs(cx, import_id.to_def_id())
                 .lists(sym::doc)
                 .get_word_attr(sym::inline)
                 .is_some();
-            // Then we get all the various imports' attributes.
-            let mut attrs = get_all_import_attributes(
-                use_node,
-                cx,
-                item.owner_id.def_id,
-                is_inline,
-                import_id,
-            );
-
-            add_without_unwanted_attributes(
-                &mut attrs,
-                inline::load_attrs(cx, def_id),
-                is_inline,
-                None
-            );
+            let mut attrs =
+                get_all_import_attributes(cx, import_id, item.owner_id.def_id, is_inline);
+            add_without_unwanted_attributes(&mut attrs, target_attrs, is_inline, None);
             attrs
         } else {
             // We only keep the item's attributes.
-            inline::load_attrs(cx, def_id).iter().map(|attr| (Cow::Borrowed(attr), None)).collect::<Vec<_>>()
+            target_attrs.iter().map(|attr| (Cow::Borrowed(attr), None)).collect()
         };
 
         let cfg = attrs.cfg(cx.tcx, &cx.cache.hidden_cfg);
-        let attrs = Attributes::from_ast_iter(attrs.iter().map(|(attr, did)| match attr {
-            Cow::Borrowed(attr) => (*attr, *did),
-            Cow::Owned(attr) => (attr, *did)
-        }), false);
+        let attrs =
+            Attributes::from_ast_iter(attrs.iter().map(|(attr, did)| (&**attr, *did)), false);
 
         let mut item =
             Item::from_def_id_and_attrs_and_parts(def_id, Some(name), kind, Box::new(attrs), cfg);

src/librustdoc/visit_ast.rs

@@ -13,9 +13,9 @@ use rustc_span::def_id::{CRATE_DEF_ID, LOCAL_CRATE};
 use rustc_span::symbol::{kw, sym, Symbol};
 use rustc_span::Span;
 
-use std::mem;
+use std::{iter, mem};
 
-use crate::clean::{cfg::Cfg, AttributesExt, NestedAttributesExt, OneLevelVisitor};
+use crate::clean::{cfg::Cfg, reexport_chain, AttributesExt, NestedAttributesExt};
 use crate::core;
 
 /// This module is used to store stuff from Rust's AST in a more convenient
@@ -133,7 +133,7 @@ impl<'a, 'tcx> RustdocVisitor<'a, 'tcx> {
         // is declared but also a reexport of itself producing two exports of the same
         // macro in the same module.
         let mut inserted = FxHashSet::default();
-        for export in self.cx.tcx.module_reexports(CRATE_DEF_ID).unwrap_or(&[]) {
+        for export in self.cx.tcx.module_reexports(CRATE_DEF_ID) {
             if let Res::Def(DefKind::Macro(_), def_id) = export.res &&
                 let Some(local_def_id) = def_id.as_local() &&
                 self.cx.tcx.has_attr(def_id, sym::macro_export) &&
@@ -220,7 +220,6 @@ impl<'a, 'tcx> RustdocVisitor<'a, 'tcx> {
         renamed: Option<Symbol>,
         glob: bool,
         please_inline: bool,
-        path: &hir::UsePath<'_>,
     ) -> bool {
         debug!("maybe_inline_local res: {:?}", res);
 
@@ -266,9 +265,9 @@ impl<'a, 'tcx> RustdocVisitor<'a, 'tcx> {
         }
 
         if !please_inline &&
-            let mut visitor = OneLevelVisitor::new(self.cx.tcx.hir(), res_did) &&
-            let Some(item) = visitor.find_target(self.cx.tcx, def_id.to_def_id(), path) &&
-            let item_def_id = item.owner_id.def_id &&
+            let Some(item_def_id) = reexport_chain(self.cx.tcx, def_id, res_did).iter()
+                .flat_map(|reexport| reexport.id()).map(|id| id.expect_local())
+                .chain(iter::once(res_did)).nth(1) &&
             item_def_id != def_id &&
             self
                 .cx
@@ -383,7 +382,6 @@ impl<'a, 'tcx> RustdocVisitor<'a, 'tcx> {
                             ident,
                             is_glob,
                             please_inline,
-                            path,
                         ) {
                             continue;
                         }