Rollup merge of rust-lang#109312 - petrochenkov:docice5, r=GuillaumeGomez

rustdoc: Cleanup parent module tracking for doc links

Keep ids of the documented items themselves, not their parent modules. Parent modules can be retreived from those ids when necessary.

Fixes rust-lang#108501.
That issue could be fixed in a more local way, but this refactoring is something that I wanted to do since rust-lang#93805 anyway.

Author: Dylan-DPC

compiler/rustc_resolve/src/rustdoc.rs

@@ -26,11 +26,13 @@ pub enum DocFragmentKind {
 #[derive(Clone, PartialEq, Eq, Debug)]
 pub struct DocFragment {
     pub span: Span,
-    /// The module this doc-comment came from.
-    ///
-    /// This allows distinguishing between the original documentation and a pub re-export.
-    /// If it is `None`, the item was not re-exported.
-    pub parent_module: Option<DefId>,
+    /// The item this doc-comment came from.
+    /// Used to determine the scope in which doc links in this fragment are resolved.
+    /// Typically filled for reexport docs when they are merged into the docs of the
+    /// original reexported item.
+    /// If the id is not filled, which happens for the original reexported item, then
+    /// it has to be taken from somewhere else during doc link resolution.
+    pub item_id: Option<DefId>,
     pub doc: Symbol,
     pub kind: DocFragmentKind,
     pub indent: usize,
@@ -186,15 +188,15 @@ pub fn attrs_to_doc_fragments<'a>(
 ) -> (Vec<DocFragment>, ast::AttrVec) {
     let mut doc_fragments = Vec::new();
     let mut other_attrs = ast::AttrVec::new();
-    for (attr, parent_module) in attrs {
+    for (attr, item_id) in attrs {
         if let Some((doc_str, comment_kind)) = attr.doc_str_and_comment_kind() {
             let doc = beautify_doc_string(doc_str, comment_kind);
             let kind = if attr.is_doc_comment() {
                 DocFragmentKind::SugaredDoc
             } else {
                 DocFragmentKind::RawDoc
             };
-            let fragment = DocFragment { span: attr.span, doc, kind, parent_module, indent: 0 };
+            let fragment = DocFragment { span: attr.span, doc, kind, item_id, indent: 0 };
             doc_fragments.push(fragment);
         } else if !doc_only {
             other_attrs.push(attr.clone());
@@ -216,7 +218,7 @@ pub fn prepare_to_doc_link_resolution(
 ) -> FxHashMap<Option<DefId>, String> {
     let mut res = FxHashMap::default();
     for fragment in doc_fragments {
-        let out_str = res.entry(fragment.parent_module).or_default();
+        let out_str = res.entry(fragment.item_id).or_default();
         add_doc_fragment(out_str, fragment);
     }
     res

src/librustdoc/clean/inline.rs

@@ -36,15 +36,11 @@ use crate::formats::item_type::ItemType;
 ///
 /// The returned value is `None` if the definition could not be inlined,
 /// and `Some` of a vector of items if it was successfully expanded.
-///
-/// `parent_module` refers to the parent of the *re-export*, not the original item.
 pub(crate) fn try_inline(
     cx: &mut DocContext<'_>,
-    parent_module: DefId,
-    import_def_id: Option<DefId>,
     res: Res,
     name: Symbol,
-    attrs: Option<&[ast::Attribute]>,
+    attrs: Option<(&[ast::Attribute], Option<DefId>)>,
     visited: &mut DefIdSet,
 ) -> Option<Vec<clean::Item>> {
     let did = res.opt_def_id()?;
@@ -55,38 +51,17 @@ pub(crate) fn try_inline(
 
     debug!("attrs={:?}", attrs);
 
-    let attrs_without_docs = attrs.map(|attrs| {
-        attrs.into_iter().filter(|a| a.doc_str().is_none()).cloned().collect::<Vec<_>>()
+    let attrs_without_docs = attrs.map(|(attrs, def_id)| {
+        (attrs.into_iter().filter(|a| a.doc_str().is_none()).cloned().collect::<Vec<_>>(), def_id)
     });
-    // We need this ugly code because:
-    //
-    // ```
-    // attrs_without_docs.map(|a| a.as_slice())
-    // ```
-    //
-    // will fail because it returns a temporary slice and:
-    //
-    // ```
-    // attrs_without_docs.map(|s| {
-    //     vec = s.as_slice();
-    //     vec
-    // })
-    // ```
-    //
-    // will fail because we're moving an uninitialized variable into a closure.
-    let vec;
-    let attrs_without_docs = match attrs_without_docs {
-        Some(s) => {
-            vec = s;
-            Some(vec.as_slice())
-        }
-        None => None,
-    };
+    let attrs_without_docs =
+        attrs_without_docs.as_ref().map(|(attrs, def_id)| (&attrs[..], *def_id));
 
+    let import_def_id = attrs.and_then(|(_, def_id)| def_id);
     let kind = match res {
         Res::Def(DefKind::Trait, did) => {
             record_extern_fqn(cx, did, ItemType::Trait);
-            build_impls(cx, Some(parent_module), did, attrs_without_docs, &mut ret);
+            build_impls(cx, did, attrs_without_docs, &mut ret);
             clean::TraitItem(Box::new(build_external_trait(cx, did)))
         }
         Res::Def(DefKind::Fn, did) => {
@@ -95,27 +70,27 @@ pub(crate) fn try_inline(
         }
         Res::Def(DefKind::Struct, did) => {
             record_extern_fqn(cx, did, ItemType::Struct);
-            build_impls(cx, Some(parent_module), did, attrs_without_docs, &mut ret);
+            build_impls(cx, did, attrs_without_docs, &mut ret);
             clean::StructItem(build_struct(cx, did))
         }
         Res::Def(DefKind::Union, did) => {
             record_extern_fqn(cx, did, ItemType::Union);
-            build_impls(cx, Some(parent_module), did, attrs_without_docs, &mut ret);
+            build_impls(cx, did, attrs_without_docs, &mut ret);
             clean::UnionItem(build_union(cx, did))
         }
         Res::Def(DefKind::TyAlias, did) => {
             record_extern_fqn(cx, did, ItemType::Typedef);
-            build_impls(cx, Some(parent_module), did, attrs_without_docs, &mut ret);
+            build_impls(cx, did, attrs_without_docs, &mut ret);
             clean::TypedefItem(build_type_alias(cx, did))
         }
         Res::Def(DefKind::Enum, did) => {
             record_extern_fqn(cx, did, ItemType::Enum);
-            build_impls(cx, Some(parent_module), did, attrs_without_docs, &mut ret);
+            build_impls(cx, did, attrs_without_docs, &mut ret);
             clean::EnumItem(build_enum(cx, did))
         }
         Res::Def(DefKind::ForeignTy, did) => {
             record_extern_fqn(cx, did, ItemType::ForeignType);
-            build_impls(cx, Some(parent_module), did, attrs_without_docs, &mut ret);
+            build_impls(cx, did, attrs_without_docs, &mut ret);
             clean::ForeignTypeItem
         }
         // Never inline enum variants but leave them shown as re-exports.
@@ -149,7 +124,7 @@ pub(crate) fn try_inline(
         _ => return None,
     };
 
-    let (attrs, cfg) = merge_attrs(cx, Some(parent_module), load_attrs(cx, did), attrs);
+    let (attrs, cfg) = merge_attrs(cx, load_attrs(cx, did), attrs);
     cx.inlined.insert(did.into());
     let mut item =
         clean::Item::from_def_id_and_attrs_and_parts(did, Some(name), kind, Box::new(attrs), cfg);
@@ -316,17 +291,16 @@ fn build_type_alias(cx: &mut DocContext<'_>, did: DefId) -> Box<clean::Typedef>
 /// Builds all inherent implementations of an ADT (struct/union/enum) or Trait item/path/reexport.
 pub(crate) fn build_impls(
     cx: &mut DocContext<'_>,
-    parent_module: Option<DefId>,
     did: DefId,
-    attrs: Option<&[ast::Attribute]>,
+    attrs: Option<(&[ast::Attribute], Option<DefId>)>,
     ret: &mut Vec<clean::Item>,
 ) {
     let _prof_timer = cx.tcx.sess.prof.generic_activity("build_inherent_impls");
     let tcx = cx.tcx;
 
     // for each implementation of an item represented by `did`, build the clean::Item for that impl
     for &did in tcx.inherent_impls(did).iter() {
-        build_impl(cx, parent_module, did, attrs, ret);
+        build_impl(cx, did, attrs, ret);
     }
 
     // This pretty much exists expressly for `dyn Error` traits that exist in the `alloc` crate.
@@ -340,28 +314,26 @@ pub(crate) fn build_impls(
         let type_ =
             if tcx.is_trait(did) { TraitSimplifiedType(did) } else { AdtSimplifiedType(did) };
         for &did in tcx.incoherent_impls(type_) {
-            build_impl(cx, parent_module, did, attrs, ret);
+            build_impl(cx, did, attrs, ret);
         }
     }
 }
 
-/// `parent_module` refers to the parent of the re-export, not the original item
 pub(crate) fn merge_attrs(
     cx: &mut DocContext<'_>,
-    parent_module: Option<DefId>,
     old_attrs: &[ast::Attribute],
-    new_attrs: Option<&[ast::Attribute]>,
+    new_attrs: Option<(&[ast::Attribute], Option<DefId>)>,
 ) -> (clean::Attributes, Option<Arc<clean::cfg::Cfg>>) {
     // NOTE: If we have additional attributes (from a re-export),
     // always insert them first. This ensure that re-export
     // doc comments show up before the original doc comments
     // when we render them.
-    if let Some(inner) = new_attrs {
+    if let Some((inner, item_id)) = new_attrs {
         let mut both = inner.to_vec();
         both.extend_from_slice(old_attrs);
         (
-            if let Some(new_id) = parent_module {
-                Attributes::from_ast_with_additional(old_attrs, (inner, new_id))
+            if let Some(item_id) = item_id {
+                Attributes::from_ast_with_additional(old_attrs, (inner, item_id))
             } else {
                 Attributes::from_ast(&both)
             },
@@ -375,9 +347,8 @@ pub(crate) fn merge_attrs(
 /// Inline an `impl`, inherent or of a trait. The `did` must be for an `impl`.
 pub(crate) fn build_impl(
     cx: &mut DocContext<'_>,
-    parent_module: Option<DefId>,
     did: DefId,
-    attrs: Option<&[ast::Attribute]>,
+    attrs: Option<(&[ast::Attribute], Option<DefId>)>,
     ret: &mut Vec<clean::Item>,
 ) {
     if !cx.inlined.insert(did.into()) {
@@ -539,7 +510,7 @@ pub(crate) fn build_impl(
         record_extern_trait(cx, did);
     }
 
-    let (merged_attrs, cfg) = merge_attrs(cx, parent_module, load_attrs(cx, did), attrs);
+    let (merged_attrs, cfg) = merge_attrs(cx, load_attrs(cx, did), attrs);
     trace!("merged_attrs={:?}", merged_attrs);
 
     trace!(
@@ -635,7 +606,7 @@ fn build_module_items(
                     cfg: None,
                     inline_stmt_id: None,
                 });
-            } else if let Some(i) = try_inline(cx, did, None, res, item.ident.name, None, visited) {
+            } else if let Some(i) = try_inline(cx, res, item.ident.name, None, visited) {
                 items.extend(i)
             }
         }

src/librustdoc/clean/mod.rs

@@ -2388,12 +2388,12 @@ fn clean_maybe_renamed_item<'tcx>(
             target_attrs.extend_from_slice(inline::load_attrs(cx, def_id));
         }
 
-        let import_parent = import_id.map(|import_id| cx.tcx.local_parent(import_id).to_def_id());
-        let (attrs, cfg) =  merge_attrs(cx, import_parent, &target_attrs, Some(&import_attrs));
+        let import_id = import_id.map(|def_id| def_id.to_def_id());
+        let (attrs, cfg) =  merge_attrs(cx, &target_attrs, Some((&import_attrs, import_id)));
 
         let mut item =
             Item::from_def_id_and_attrs_and_parts(def_id, Some(name), kind, Box::new(attrs), cfg);
-        item.inline_stmt_id = import_id.map(|def_id| def_id.to_def_id());
+        item.inline_stmt_id = import_id;
         vec![item]
     })
 }
@@ -2478,18 +2478,12 @@ fn clean_extern_crate<'tcx>(
 
     let krate_owner_def_id = krate.owner_id.to_def_id();
     if please_inline {
-        let mut visited = DefIdSet::default();
-
-        let res = Res::Def(DefKind::Mod, crate_def_id);
-
         if let Some(items) = inline::try_inline(
             cx,
-            cx.tcx.parent_module(krate.hir_id()).to_def_id(),
-            Some(krate_owner_def_id),
-            res,
+            Res::Def(DefKind::Mod, crate_def_id),
             name,
-            Some(attrs),
-            &mut visited,
+            Some((attrs, Some(krate_owner_def_id))),
+            &mut Default::default(),
         ) {
             return items;
         }
@@ -2613,17 +2607,13 @@ fn clean_use_statement_inner<'tcx>(
             denied = true;
         }
         if !denied {
-            let mut visited = DefIdSet::default();
             let import_def_id = import.owner_id.to_def_id();
-
             if let Some(mut items) = inline::try_inline(
                 cx,
-                cx.tcx.parent_module(import.hir_id()).to_def_id(),
-                Some(import_def_id),
                 path.res,
                 name,
-                Some(attrs),
-                &mut visited,
+                Some((attrs, Some(import_def_id))),
+                &mut Default::default(),
             ) {
                 items.push(Item::from_def_id_and_parts(
                     import_def_id,

src/librustdoc/clean/types/tests.rs

@@ -10,7 +10,7 @@ use rustc_span::symbol::Symbol;
 fn create_doc_fragment(s: &str) -> Vec<DocFragment> {
     vec![DocFragment {
         span: DUMMY_SP,
-        parent_module: None,
+        item_id: None,
         doc: Symbol::intern(s),
         kind: DocFragmentKind::SugaredDoc,
         indent: 0,

src/librustdoc/clean/utils.rs

@@ -195,12 +195,12 @@ pub(crate) fn build_deref_target_impls(
         if let Some(prim) = target.primitive_type() {
             let _prof_timer = cx.tcx.sess.prof.generic_activity("build_primitive_inherent_impls");
             for did in prim.impls(tcx).filter(|did| !did.is_local()) {
-                inline::build_impl(cx, None, did, None, ret);
+                inline::build_impl(cx, did, None, ret);
             }
         } else if let Type::Path { path } = target {
             let did = path.def_id();
             if !did.is_local() {
-                inline::build_impls(cx, None, did, None, ret);
+                inline::build_impls(cx, did, None, ret);
             }
         }
     }