From f7a97027b2d116a1f799db0f67ab340ccc60b4df Mon Sep 17 00:00:00 2001 From: Vadim Petrochenkov Date: Sat, 18 Mar 2023 19:18:51 +0400 Subject: [PATCH 1/2] 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. --- compiler/rustc_resolve/src/rustdoc.rs | 18 +-- src/librustdoc/clean/inline.rs | 75 ++++------- src/librustdoc/clean/mod.rs | 20 +-- src/librustdoc/clean/types/tests.rs | 2 +- src/librustdoc/clean/utils.rs | 4 +- .../passes/collect_intra_doc_links.rs | 126 ++++++------------ src/librustdoc/passes/collect_trait_impls.rs | 6 +- src/librustdoc/passes/propagate_doc_cfg.rs | 3 +- .../intra-doc/auxiliary/inner-crate-doc.rs | 1 + .../intra-doc/import-inline-merge-module.rs | 10 ++ 10 files changed, 100 insertions(+), 165 deletions(-) create mode 100644 tests/rustdoc-ui/intra-doc/auxiliary/inner-crate-doc.rs create mode 100644 tests/rustdoc-ui/intra-doc/import-inline-merge-module.rs diff --git a/compiler/rustc_resolve/src/rustdoc.rs b/compiler/rustc_resolve/src/rustdoc.rs index b8853c1744c92..0e40f794f1860 100644 --- a/compiler/rustc_resolve/src/rustdoc.rs +++ b/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, + /// 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, pub doc: Symbol, pub kind: DocFragmentKind, pub indent: usize, @@ -186,7 +188,7 @@ pub fn attrs_to_doc_fragments<'a>( ) -> (Vec, 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() { @@ -194,7 +196,7 @@ pub fn attrs_to_doc_fragments<'a>( } 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, 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 diff --git a/src/librustdoc/clean/inline.rs b/src/librustdoc/clean/inline.rs index 148243683cbbf..768f8bb7bc899 100644 --- a/src/librustdoc/clean/inline.rs +++ b/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, res: Res, name: Symbol, - attrs: Option<&[ast::Attribute]>, + attrs: Option<(&[ast::Attribute], Option)>, visited: &mut DefIdSet, ) -> Option> { 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::>() + let attrs_without_docs = attrs.map(|(attrs, def_id)| { + (attrs.into_iter().filter(|a| a.doc_str().is_none()).cloned().collect::>(), 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,9 +291,8 @@ fn build_type_alias(cx: &mut DocContext<'_>, did: DefId) -> Box /// 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, did: DefId, - attrs: Option<&[ast::Attribute]>, + attrs: Option<(&[ast::Attribute], Option)>, ret: &mut Vec, ) { let _prof_timer = cx.tcx.sess.prof.generic_activity("build_inherent_impls"); @@ -326,7 +300,7 @@ pub(crate) fn build_impls( // 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, old_attrs: &[ast::Attribute], - new_attrs: Option<&[ast::Attribute]>, + new_attrs: Option<(&[ast::Attribute], Option)>, ) -> (clean::Attributes, Option>) { // 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, did: DefId, - attrs: Option<&[ast::Attribute]>, + attrs: Option<(&[ast::Attribute], Option)>, ret: &mut Vec, ) { 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) } } diff --git a/src/librustdoc/clean/mod.rs b/src/librustdoc/clean/mod.rs index 989e091a0d2d8..dfe800050c524 100644 --- a/src/librustdoc/clean/mod.rs +++ b/src/librustdoc/clean/mod.rs @@ -2476,18 +2476,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; } @@ -2611,17 +2605,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, diff --git a/src/librustdoc/clean/types/tests.rs b/src/librustdoc/clean/types/tests.rs index 20627c2cfc164..8f2331785f50d 100644 --- a/src/librustdoc/clean/types/tests.rs +++ b/src/librustdoc/clean/types/tests.rs @@ -10,7 +10,7 @@ use rustc_span::symbol::Symbol; fn create_doc_fragment(s: &str) -> Vec { vec![DocFragment { span: DUMMY_SP, - parent_module: None, + item_id: None, doc: Symbol::intern(s), kind: DocFragmentKind::SugaredDoc, indent: 0, diff --git a/src/librustdoc/clean/utils.rs b/src/librustdoc/clean/utils.rs index cafb00df51ed2..cca50df0db213 100644 --- a/src/librustdoc/clean/utils.rs +++ b/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); } } } diff --git a/src/librustdoc/passes/collect_intra_doc_links.rs b/src/librustdoc/passes/collect_intra_doc_links.rs index 358f6ad566c25..e335cec04d2ec 100644 --- a/src/librustdoc/passes/collect_intra_doc_links.rs +++ b/src/librustdoc/passes/collect_intra_doc_links.rs @@ -28,7 +28,7 @@ use std::mem; use std::ops::Range; use crate::clean::{self, utils::find_nearest_parent_module}; -use crate::clean::{Crate, Item, ItemId, ItemLink, PrimitiveType}; +use crate::clean::{Crate, Item, ItemLink, PrimitiveType}; use crate::core::DocContext; use crate::html::markdown::{markdown_links, MarkdownLink}; use crate::lint::{BROKEN_INTRA_DOC_LINKS, PRIVATE_INTRA_DOC_LINKS}; @@ -42,8 +42,7 @@ pub(crate) const COLLECT_INTRA_DOC_LINKS: Pass = Pass { }; fn collect_intra_doc_links(krate: Crate, cx: &mut DocContext<'_>) -> Crate { - let mut collector = - LinkCollector { cx, mod_ids: Vec::new(), visited_links: FxHashMap::default() }; + let mut collector = LinkCollector { cx, visited_links: FxHashMap::default() }; collector.visit_crate(&krate); krate } @@ -149,7 +148,7 @@ impl TryFrom for Res { #[derive(Debug)] struct UnresolvedPath<'a> { /// Item on which the link is resolved, used for resolving `Self`. - item_id: ItemId, + item_id: DefId, /// The scope the link was resolved in. module_id: DefId, /// If part of the link resolved, this has the `Res`. @@ -225,7 +224,7 @@ impl UrlFragment { #[derive(Clone, Debug, Hash, PartialEq, Eq)] struct ResolutionInfo { - item_id: ItemId, + item_id: DefId, module_id: DefId, dis: Option, path_str: Box, @@ -242,11 +241,6 @@ struct DiagnosticInfo<'a> { struct LinkCollector<'a, 'tcx> { cx: &'a mut DocContext<'tcx>, - /// A stack of modules used to decide what scope to resolve in. - /// - /// The last module will be used if the parent scope of the current item is - /// unknown. - mod_ids: Vec, /// Cache the resolved links so we can avoid resolving (and emitting errors for) the same link. /// The link will be `None` if it could not be resolved (i.e. the error was cached). visited_links: FxHashMap)>>, @@ -262,7 +256,7 @@ impl<'a, 'tcx> LinkCollector<'a, 'tcx> { fn variant_field<'path>( &self, path_str: &'path str, - item_id: ItemId, + item_id: DefId, module_id: DefId, ) -> Result<(Res, DefId), UnresolvedPath<'path>> { let tcx = self.cx.tcx; @@ -334,35 +328,33 @@ impl<'a, 'tcx> LinkCollector<'a, 'tcx> { }) } - fn resolve_self_ty(&self, path_str: &str, ns: Namespace, item_id: ItemId) -> Option { + fn resolve_self_ty(&self, path_str: &str, ns: Namespace, item_id: DefId) -> Option { if ns != TypeNS || path_str != "Self" { return None; } let tcx = self.cx.tcx; - item_id - .as_def_id() - .map(|def_id| match tcx.def_kind(def_id) { - def_kind @ (DefKind::AssocFn - | DefKind::AssocConst - | DefKind::AssocTy - | DefKind::Variant - | DefKind::Field) => { - let parent_def_id = tcx.parent(def_id); - if def_kind == DefKind::Field && tcx.def_kind(parent_def_id) == DefKind::Variant - { - tcx.parent(parent_def_id) - } else { - parent_def_id - } + let self_id = match tcx.def_kind(item_id) { + def_kind @ (DefKind::AssocFn + | DefKind::AssocConst + | DefKind::AssocTy + | DefKind::Variant + | DefKind::Field) => { + let parent_def_id = tcx.parent(item_id); + if def_kind == DefKind::Field && tcx.def_kind(parent_def_id) == DefKind::Variant { + tcx.parent(parent_def_id) + } else { + parent_def_id } - _ => def_id, - }) - .and_then(|self_id| match tcx.def_kind(self_id) { - DefKind::Impl { .. } => self.def_id_to_res(self_id), - DefKind::Use => None, - def_kind => Some(Res::Def(def_kind, self_id)), - }) + } + _ => item_id, + }; + + match tcx.def_kind(self_id) { + DefKind::Impl { .. } => self.def_id_to_res(self_id), + DefKind::Use => None, + def_kind => Some(Res::Def(def_kind, self_id)), + } } /// Convenience wrapper around `doc_link_resolutions`. @@ -374,7 +366,7 @@ impl<'a, 'tcx> LinkCollector<'a, 'tcx> { &self, path_str: &str, ns: Namespace, - item_id: ItemId, + item_id: DefId, module_id: DefId, ) -> Option { if let res @ Some(..) = self.resolve_self_ty(path_str, ns, item_id) { @@ -401,7 +393,7 @@ impl<'a, 'tcx> LinkCollector<'a, 'tcx> { &mut self, path_str: &'path str, ns: Namespace, - item_id: ItemId, + item_id: DefId, module_id: DefId, ) -> Result<(Res, Option), UnresolvedPath<'path>> { if let Some(res) = self.resolve_path(path_str, ns, item_id, module_id) { @@ -781,48 +773,31 @@ fn is_derive_trait_collision(ns: &PerNS DocVisitor for LinkCollector<'a, 'tcx> { fn visit_item(&mut self, item: &Item) { - let parent_node = - item.item_id.as_def_id().and_then(|did| find_nearest_parent_module(self.cx.tcx, did)); - if parent_node.is_some() { - trace!("got parent node for {:?} {:?}, id {:?}", item.type_(), item.name, item.item_id); - } - - let inner_docs = item.inner_docs(self.cx.tcx); - - if item.is_mod() && inner_docs { - self.mod_ids.push(item.item_id.expect_def_id()); - } - // We want to resolve in the lexical scope of the documentation. // In the presence of re-exports, this is not the same as the module of the item. // Rather than merging all documentation into one, resolve it one attribute at a time // so we know which module it came from. - for (parent_module, doc) in prepare_to_doc_link_resolution(&item.attrs.doc_strings) { + for (item_id, doc) in prepare_to_doc_link_resolution(&item.attrs.doc_strings) { if !may_have_doc_links(&doc) { continue; } debug!("combined_docs={}", doc); // NOTE: if there are links that start in one crate and end in another, this will not resolve them. // This is a degenerate case and it's not supported by rustdoc. - let parent_node = parent_module.or(parent_node); + let item_id = item_id.unwrap_or_else(|| item.item_id.expect_def_id()); + let module_id = match self.cx.tcx.def_kind(item_id) { + DefKind::Mod if item.inner_docs(self.cx.tcx) => item_id, + _ => find_nearest_parent_module(self.cx.tcx, item_id).unwrap(), + }; for md_link in preprocessed_markdown_links(&doc) { - let link = self.resolve_link(item, &doc, parent_node, &md_link); + let link = self.resolve_link(item, item_id, module_id, &doc, &md_link); if let Some(link) = link { self.cx.cache.intra_doc_links.entry(item.item_id).or_default().push(link); } } } - if item.is_mod() { - if !inner_docs { - self.mod_ids.push(item.item_id.expect_def_id()); - } - - self.visit_item_recur(item); - self.mod_ids.pop(); - } else { - self.visit_item_recur(item) - } + self.visit_item_recur(item) } } @@ -954,8 +929,9 @@ impl LinkCollector<'_, '_> { fn resolve_link( &mut self, item: &Item, + item_id: DefId, + module_id: DefId, dox: &str, - parent_node: Option, link: &PreprocessedMarkdownLink, ) -> Option { let PreprocessedMarkdownLink(pp_link, ori_link) = link; @@ -972,25 +948,9 @@ impl LinkCollector<'_, '_> { pp_link.as_ref().map_err(|err| err.report(self.cx, diag_info.clone())).ok()?; let disambiguator = *disambiguator; - // In order to correctly resolve intra-doc links we need to - // pick a base AST node to work from. If the documentation for - // this module came from an inner comment (//!) then we anchor - // our name resolution *inside* the module. If, on the other - // hand it was an outer comment (///) then we anchor the name - // resolution in the parent module on the basis that the names - // used are more likely to be intended to be parent names. For - // this, we set base_node to None for inner comments since - // we've already pushed this node onto the resolution stack but - // for outer comments we explicitly try and resolve against the - // parent_node first. - let inner_docs = item.inner_docs(self.cx.tcx); - let base_node = - if item.is_mod() && inner_docs { self.mod_ids.last().copied() } else { parent_node }; - let module_id = base_node.expect("doc link without parent module"); - let (mut res, fragment) = self.resolve_with_disambiguator_cached( ResolutionInfo { - item_id: item.item_id, + item_id, module_id, dis: disambiguator, path_str: path_str.clone(), @@ -1231,11 +1191,11 @@ impl LinkCollector<'_, '_> { let disambiguator = key.dis; let path_str = &key.path_str; let item_id = key.item_id; - let base_node = key.module_id; + let module_id = key.module_id; match disambiguator.map(Disambiguator::ns) { Some(expected_ns) => { - match self.resolve(path_str, expected_ns, item_id, base_node) { + match self.resolve(path_str, expected_ns, item_id, module_id) { Ok(res) => Some(res), Err(err) => { // We only looked in one namespace. Try to give a better error if possible. @@ -1245,7 +1205,7 @@ impl LinkCollector<'_, '_> { for other_ns in [TypeNS, ValueNS, MacroNS] { if other_ns != expected_ns { if let Ok(res) = - self.resolve(path_str, other_ns, item_id, base_node) + self.resolve(path_str, other_ns, item_id, module_id) { err = ResolutionFailure::WrongNamespace { res: full_res(self.cx.tcx, res), @@ -1262,7 +1222,7 @@ impl LinkCollector<'_, '_> { None => { // Try everything! let mut candidate = |ns| { - self.resolve(path_str, ns, item_id, base_node) + self.resolve(path_str, ns, item_id, module_id) .map_err(ResolutionFailure::NotResolved) }; diff --git a/src/librustdoc/passes/collect_trait_impls.rs b/src/librustdoc/passes/collect_trait_impls.rs index d32e8185d3f96..8d204ddb79e39 100644 --- a/src/librustdoc/passes/collect_trait_impls.rs +++ b/src/librustdoc/passes/collect_trait_impls.rs @@ -49,7 +49,7 @@ pub(crate) fn collect_trait_impls(mut krate: Crate, cx: &mut DocContext<'_>) -> let _prof_timer = cx.tcx.sess.prof.generic_activity("build_extern_trait_impls"); for &cnum in cx.tcx.crates(()) { for &impl_def_id in cx.tcx.trait_impls_in_crate(cnum) { - inline::build_impl(cx, None, impl_def_id, None, &mut new_items_external); + inline::build_impl(cx, impl_def_id, None, &mut new_items_external); } } } @@ -75,7 +75,7 @@ pub(crate) fn collect_trait_impls(mut krate: Crate, cx: &mut DocContext<'_>) -> ); parent = cx.tcx.opt_parent(did); } - inline::build_impl(cx, None, impl_def_id, Some(&attr_buf), &mut new_items_local); + inline::build_impl(cx, impl_def_id, Some((&attr_buf, None)), &mut new_items_local); attr_buf.clear(); } } @@ -84,7 +84,7 @@ pub(crate) fn collect_trait_impls(mut krate: Crate, cx: &mut DocContext<'_>) -> for def_id in PrimitiveType::all_impls(cx.tcx) { // Try to inline primitive impls from other crates. if !def_id.is_local() { - inline::build_impl(cx, None, def_id, None, &mut new_items_external); + inline::build_impl(cx, def_id, None, &mut new_items_external); } } for (prim, did) in PrimitiveType::primitive_locations(cx.tcx) { diff --git a/src/librustdoc/passes/propagate_doc_cfg.rs b/src/librustdoc/passes/propagate_doc_cfg.rs index f35643af63738..8a33e51b3beb1 100644 --- a/src/librustdoc/passes/propagate_doc_cfg.rs +++ b/src/librustdoc/passes/propagate_doc_cfg.rs @@ -57,7 +57,8 @@ impl<'a, 'tcx> CfgPropagator<'a, 'tcx> { next_def_id = parent_def_id; } - let (_, cfg) = merge_attrs(self.cx, None, item.attrs.other_attrs.as_slice(), Some(&attrs)); + let (_, cfg) = + merge_attrs(self.cx, item.attrs.other_attrs.as_slice(), Some((&attrs, None))); item.cfg = cfg; } } diff --git a/tests/rustdoc-ui/intra-doc/auxiliary/inner-crate-doc.rs b/tests/rustdoc-ui/intra-doc/auxiliary/inner-crate-doc.rs new file mode 100644 index 0000000000000..15bf51e6f8e2b --- /dev/null +++ b/tests/rustdoc-ui/intra-doc/auxiliary/inner-crate-doc.rs @@ -0,0 +1 @@ +//! Inner doc comment diff --git a/tests/rustdoc-ui/intra-doc/import-inline-merge-module.rs b/tests/rustdoc-ui/intra-doc/import-inline-merge-module.rs new file mode 100644 index 0000000000000..4d6a325664578 --- /dev/null +++ b/tests/rustdoc-ui/intra-doc/import-inline-merge-module.rs @@ -0,0 +1,10 @@ +// Test for issue #108501. +// Module parent scope doesn't hijack import's parent scope for the import's doc links. + +// check-pass +// aux-build: inner-crate-doc.rs +// compile-flags: --extern inner_crate_doc --edition 2018 + +/// Import doc comment [inner_crate_doc] +#[doc(inline)] +pub use inner_crate_doc; From ae4781081a56f7c5ebcce69c23f400f85567cc65 Mon Sep 17 00:00:00 2001 From: Vadim Petrochenkov Date: Sun, 19 Mar 2023 20:53:39 +0400 Subject: [PATCH 2/2] rustdoc: Factor out some doc link resolution code into a separate function --- .../passes/collect_intra_doc_links.rs | 51 ++++++++++--------- 1 file changed, 27 insertions(+), 24 deletions(-) diff --git a/src/librustdoc/passes/collect_intra_doc_links.rs b/src/librustdoc/passes/collect_intra_doc_links.rs index e335cec04d2ec..ec9de8a43ee3d 100644 --- a/src/librustdoc/passes/collect_intra_doc_links.rs +++ b/src/librustdoc/passes/collect_intra_doc_links.rs @@ -773,30 +773,7 @@ fn is_derive_trait_collision(ns: &PerNS DocVisitor for LinkCollector<'a, 'tcx> { fn visit_item(&mut self, item: &Item) { - // We want to resolve in the lexical scope of the documentation. - // In the presence of re-exports, this is not the same as the module of the item. - // Rather than merging all documentation into one, resolve it one attribute at a time - // so we know which module it came from. - for (item_id, doc) in prepare_to_doc_link_resolution(&item.attrs.doc_strings) { - if !may_have_doc_links(&doc) { - continue; - } - debug!("combined_docs={}", doc); - // NOTE: if there are links that start in one crate and end in another, this will not resolve them. - // This is a degenerate case and it's not supported by rustdoc. - let item_id = item_id.unwrap_or_else(|| item.item_id.expect_def_id()); - let module_id = match self.cx.tcx.def_kind(item_id) { - DefKind::Mod if item.inner_docs(self.cx.tcx) => item_id, - _ => find_nearest_parent_module(self.cx.tcx, item_id).unwrap(), - }; - for md_link in preprocessed_markdown_links(&doc) { - let link = self.resolve_link(item, item_id, module_id, &doc, &md_link); - if let Some(link) = link { - self.cx.cache.intra_doc_links.entry(item.item_id).or_default().push(link); - } - } - } - + self.resolve_links(item); self.visit_item_recur(item) } } @@ -923,6 +900,32 @@ fn preprocessed_markdown_links(s: &str) -> Vec { } impl LinkCollector<'_, '_> { + fn resolve_links(&mut self, item: &Item) { + // We want to resolve in the lexical scope of the documentation. + // In the presence of re-exports, this is not the same as the module of the item. + // Rather than merging all documentation into one, resolve it one attribute at a time + // so we know which module it came from. + for (item_id, doc) in prepare_to_doc_link_resolution(&item.attrs.doc_strings) { + if !may_have_doc_links(&doc) { + continue; + } + debug!("combined_docs={}", doc); + // NOTE: if there are links that start in one crate and end in another, this will not resolve them. + // This is a degenerate case and it's not supported by rustdoc. + let item_id = item_id.unwrap_or_else(|| item.item_id.expect_def_id()); + let module_id = match self.cx.tcx.def_kind(item_id) { + DefKind::Mod if item.inner_docs(self.cx.tcx) => item_id, + _ => find_nearest_parent_module(self.cx.tcx, item_id).unwrap(), + }; + for md_link in preprocessed_markdown_links(&doc) { + let link = self.resolve_link(item, item_id, module_id, &doc, &md_link); + if let Some(link) = link { + self.cx.cache.intra_doc_links.entry(item.item_id).or_default().push(link); + } + } + } + } + /// This is the entry point for resolving an intra-doc link. /// /// FIXME(jynelson): this is way too many arguments