From fa1b15f6279a95923b8a19046f45f27f243d49d2 Mon Sep 17 00:00:00 2001 From: Joshua Nelson Date: Sat, 3 Oct 2020 20:06:30 -0400 Subject: [PATCH] Preserve the parent module of `DocFragment`s - Add `parent_module` to `DocFragment` - Require the `parent_module` of the item being inlined - Preserve the hir_id for ExternCrates so rustdoc can find the parent module later - Take an optional `parent_module` for `build_impl` and `merge_attrs`. Preserve the difference between parent modules for each doc-comment. - Support arbitrarily many re-exports in from_ast. In retrospect this is probably not used and could be simplified to a single `Option<(Attrs, DefId)>`. - Don't require the parent_module for all `impl`s, just inlined items In particular, this will be `None` whenever the attribute is not on a re-export. - Only store the parent_module, not the HirId When re-exporting a re-export, the HirId is not available. Fortunately, `collect_intra_doc_links` doesn't actually need all the info from a HirId, just the parent module. --- src/librustdoc/clean/inline.rs | 61 ++++++++++++------- src/librustdoc/clean/mod.rs | 24 +++++--- src/librustdoc/clean/types.rs | 31 ++++++++-- src/librustdoc/clean/utils.rs | 4 +- src/librustdoc/doctest.rs | 2 +- src/librustdoc/doctree.rs | 2 + .../passes/collect_intra_doc_links.rs | 10 ++- src/librustdoc/passes/collect_trait_impls.rs | 6 +- src/librustdoc/visit_ast.rs | 1 + 9 files changed, 101 insertions(+), 40 deletions(-) diff --git a/src/librustdoc/clean/inline.rs b/src/librustdoc/clean/inline.rs index 7f64d20d8e7..a6c754ab67f 100644 --- a/src/librustdoc/clean/inline.rs +++ b/src/librustdoc/clean/inline.rs @@ -15,7 +15,7 @@ use rustc_span::hygiene::MacroKind; use rustc_span::symbol::{sym, Symbol}; use rustc_span::Span; -use crate::clean::{self, GetDefId, ToSource, TypeKind}; +use crate::clean::{self, Attributes, GetDefId, ToSource, TypeKind}; use crate::core::DocContext; use crate::doctree; @@ -35,8 +35,11 @@ type Attrs<'hir> = rustc_middle::ty::Attributes<'hir>; /// /// 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 fn try_inline( cx: &DocContext<'_>, + parent_module: DefId, res: Res, name: Symbol, attrs: Option>, @@ -48,12 +51,13 @@ pub fn try_inline( } let mut ret = Vec::new(); + debug!("attrs={:?}", attrs); let attrs_clone = attrs; let inner = match res { Res::Def(DefKind::Trait, did) => { record_extern_fqn(cx, did, clean::TypeKind::Trait); - ret.extend(build_impls(cx, did, attrs)); + ret.extend(build_impls(cx, Some(parent_module), did, attrs)); clean::TraitItem(build_external_trait(cx, did)) } Res::Def(DefKind::Fn, did) => { @@ -62,27 +66,27 @@ pub fn try_inline( } Res::Def(DefKind::Struct, did) => { record_extern_fqn(cx, did, clean::TypeKind::Struct); - ret.extend(build_impls(cx, did, attrs)); + ret.extend(build_impls(cx, Some(parent_module), did, attrs)); clean::StructItem(build_struct(cx, did)) } Res::Def(DefKind::Union, did) => { record_extern_fqn(cx, did, clean::TypeKind::Union); - ret.extend(build_impls(cx, did, attrs)); + ret.extend(build_impls(cx, Some(parent_module), did, attrs)); clean::UnionItem(build_union(cx, did)) } Res::Def(DefKind::TyAlias, did) => { record_extern_fqn(cx, did, clean::TypeKind::Typedef); - ret.extend(build_impls(cx, did, attrs)); + ret.extend(build_impls(cx, Some(parent_module), did, attrs)); clean::TypedefItem(build_type_alias(cx, did), false) } Res::Def(DefKind::Enum, did) => { record_extern_fqn(cx, did, clean::TypeKind::Enum); - ret.extend(build_impls(cx, did, attrs)); + ret.extend(build_impls(cx, Some(parent_module), did, attrs)); clean::EnumItem(build_enum(cx, did)) } Res::Def(DefKind::ForeignTy, did) => { record_extern_fqn(cx, did, clean::TypeKind::Foreign); - ret.extend(build_impls(cx, did, attrs)); + ret.extend(build_impls(cx, Some(parent_module), did, attrs)); clean::ForeignTypeItem } // Never inline enum variants but leave them shown as re-exports. @@ -117,7 +121,7 @@ pub fn try_inline( }; let target_attrs = load_attrs(cx, did); - let attrs = merge_attrs(cx, target_attrs, attrs_clone); + let attrs = merge_attrs(cx, Some(parent_module), target_attrs, attrs_clone); cx.renderinfo.borrow_mut().inlined.insert(did); ret.push(clean::Item { @@ -291,40 +295,52 @@ pub fn build_ty(cx: &DocContext<'_>, did: DefId) -> Option { } /// Builds all inherent implementations of an ADT (struct/union/enum) or Trait item/path/reexport. -pub fn build_impls(cx: &DocContext<'_>, did: DefId, attrs: Option>) -> Vec { +pub fn build_impls( + cx: &DocContext<'_>, + parent_module: Option, + did: DefId, + attrs: Option>, +) -> Vec { let tcx = cx.tcx; let mut impls = Vec::new(); // 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, did, attrs, &mut impls); + build_impl(cx, parent_module, did, attrs, &mut impls); } impls } +/// `parent_module` refers to the parent of the re-export, not the original item fn merge_attrs( cx: &DocContext<'_>, - attrs: Attrs<'_>, - other_attrs: Option>, + parent_module: Option, + old_attrs: Attrs<'_>, + new_attrs: Option>, ) -> clean::Attributes { // 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. - let merged_attrs = if let Some(inner) = other_attrs { - let mut both = inner.to_vec(); - both.extend_from_slice(attrs); - both + if let Some(inner) = new_attrs { + if let Some(new_id) = parent_module { + let diag = cx.sess().diagnostic(); + Attributes::from_ast(diag, old_attrs, Some((inner, new_id))) + } else { + let mut both = inner.to_vec(); + both.extend_from_slice(old_attrs); + both.clean(cx) + } } else { - attrs.to_vec() - }; - merged_attrs.clean(cx) + old_attrs.clean(cx) + } } /// Builds a specific implementation of a type. The `did` could be a type method or trait method. pub fn build_impl( cx: &DocContext<'_>, + parent_module: impl Into>, did: DefId, attrs: Option>, ret: &mut Vec, @@ -333,7 +349,8 @@ pub fn build_impl( return; } - let attrs = merge_attrs(cx, load_attrs(cx, did), attrs); + let attrs = merge_attrs(cx, parent_module.into(), load_attrs(cx, did), attrs); + debug!("merged_attrs={:?}", attrs); let tcx = cx.tcx; let associated_trait = tcx.impl_trait_ref(did); @@ -499,7 +516,9 @@ fn build_module(cx: &DocContext<'_>, did: DefId, visited: &mut FxHashSet) }, )), }); - } else if let Some(i) = try_inline(cx, item.res, item.ident.name, None, visited) { + } else if let Some(i) = + try_inline(cx, did, item.res, item.ident.name, None, visited) + { items.extend(i) } } diff --git a/src/librustdoc/clean/mod.rs b/src/librustdoc/clean/mod.rs index ac1e2b46704..ca9135cd11a 100644 --- a/src/librustdoc/clean/mod.rs +++ b/src/librustdoc/clean/mod.rs @@ -284,7 +284,7 @@ impl Clean for doctree::Module<'_> { impl Clean for [ast::Attribute] { fn clean(&self, cx: &DocContext<'_>) -> Attributes { - Attributes::from_ast(cx.sess().diagnostic(), self) + Attributes::from_ast(cx.sess().diagnostic(), self, None) } } @@ -2205,9 +2205,14 @@ impl Clean> for doctree::ExternCrate<'_> { let res = Res::Def(DefKind::Mod, DefId { krate: self.cnum, index: CRATE_DEF_INDEX }); - if let Some(items) = - inline::try_inline(cx, res, self.name, Some(self.attrs), &mut visited) - { + if let Some(items) = inline::try_inline( + cx, + cx.tcx.parent_module(self.hir_id).to_def_id(), + res, + self.name, + Some(self.attrs), + &mut visited, + ) { return items; } } @@ -2268,9 +2273,14 @@ impl Clean> for doctree::Import<'_> { } if !denied { let mut visited = FxHashSet::default(); - if let Some(items) = - inline::try_inline(cx, path.res, name, Some(self.attrs), &mut visited) - { + if let Some(items) = inline::try_inline( + cx, + cx.tcx.parent_module(self.id).to_def_id(), + path.res, + name, + Some(self.attrs), + &mut visited, + ) { return items; } } diff --git a/src/librustdoc/clean/types.rs b/src/librustdoc/clean/types.rs index bb6f449e355..cd20e241c8f 100644 --- a/src/librustdoc/clean/types.rs +++ b/src/librustdoc/clean/types.rs @@ -373,6 +373,11 @@ impl> NestedAttributesExt for I { pub struct DocFragment { pub line: usize, pub span: rustc_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, pub doc: String, pub kind: DocFragmentKind, } @@ -521,16 +526,25 @@ impl Attributes { false } - pub fn from_ast(diagnostic: &::rustc_errors::Handler, attrs: &[ast::Attribute]) -> Attributes { + pub fn from_ast( + diagnostic: &::rustc_errors::Handler, + attrs: &[ast::Attribute], + additional_attrs: Option<(&[ast::Attribute], DefId)>, + ) -> Attributes { let mut doc_strings = vec![]; let mut sp = None; let mut cfg = Cfg::True; let mut doc_line = 0; - let other_attrs = attrs - .iter() - .filter_map(|attr| { + // Additional documentation should be shown before the original documentation + let other_attrs = additional_attrs + .into_iter() + .map(|(attrs, id)| attrs.iter().map(move |attr| (attr, Some(id)))) + .flatten() + .chain(attrs.iter().map(|attr| (attr, None))) + .filter_map(|(attr, parent_module)| { if let Some(value) = attr.doc_str() { + trace!("got doc_str={:?}", value); let value = beautify_doc_string(value); let kind = if attr.is_doc_comment() { DocFragmentKind::SugaredDoc @@ -540,7 +554,13 @@ impl Attributes { let line = doc_line; doc_line += value.lines().count(); - doc_strings.push(DocFragment { line, span: attr.span, doc: value, kind }); + doc_strings.push(DocFragment { + line, + span: attr.span, + doc: value, + kind, + parent_module, + }); if sp.is_none() { sp = Some(attr.span); @@ -565,6 +585,7 @@ impl Attributes { span: attr.span, doc: contents, kind: DocFragmentKind::Include { filename }, + parent_module: parent_module, }); } } diff --git a/src/librustdoc/clean/utils.rs b/src/librustdoc/clean/utils.rs index 58b76d24a5b..913342e2715 100644 --- a/src/librustdoc/clean/utils.rs +++ b/src/librustdoc/clean/utils.rs @@ -361,7 +361,7 @@ pub fn build_deref_target_impls(cx: &DocContext<'_>, items: &[Item], ret: &mut V let primitive = match *target { ResolvedPath { did, .. } if did.is_local() => continue, ResolvedPath { did, .. } => { - ret.extend(inline::build_impls(cx, did, None)); + ret.extend(inline::build_impls(cx, None, did, None)); continue; } _ => match target.primitive_type() { @@ -371,7 +371,7 @@ pub fn build_deref_target_impls(cx: &DocContext<'_>, items: &[Item], ret: &mut V }; for &did in primitive.impls(tcx) { if !did.is_local() { - inline::build_impl(cx, did, None, ret); + inline::build_impl(cx, None, did, None, ret); } } } diff --git a/src/librustdoc/doctest.rs b/src/librustdoc/doctest.rs index 7a6c9eabb5f..eb33890fb5f 100644 --- a/src/librustdoc/doctest.rs +++ b/src/librustdoc/doctest.rs @@ -927,7 +927,7 @@ impl<'a, 'hir, 'tcx> HirCollector<'a, 'hir, 'tcx> { sp: Span, nested: F, ) { - let mut attrs = Attributes::from_ast(self.sess.diagnostic(), attrs); + let mut attrs = Attributes::from_ast(self.sess.diagnostic(), attrs, None); if let Some(ref cfg) = attrs.cfg { if !cfg.matches(&self.sess.parse_sess, Some(&self.sess.features_untracked())) { return; diff --git a/src/librustdoc/doctree.rs b/src/librustdoc/doctree.rs index cfa51dcf4f1..6bb9b58bead 100644 --- a/src/librustdoc/doctree.rs +++ b/src/librustdoc/doctree.rs @@ -8,6 +8,7 @@ use rustc_span::{self, Span, Symbol}; use rustc_hir as hir; use rustc_hir::def_id::CrateNum; +use rustc_hir::HirId; pub struct Module<'hir> { pub name: Option, @@ -236,6 +237,7 @@ pub struct Macro<'hir> { pub struct ExternCrate<'hir> { pub name: Symbol, + pub hir_id: HirId, pub cnum: CrateNum, pub path: Option, pub vis: &'hir hir::Visibility<'hir>, diff --git a/src/librustdoc/passes/collect_intra_doc_links.rs b/src/librustdoc/passes/collect_intra_doc_links.rs index f234dc5c03b..6d52733b384 100644 --- a/src/librustdoc/passes/collect_intra_doc_links.rs +++ b/src/librustdoc/passes/collect_intra_doc_links.rs @@ -767,8 +767,16 @@ impl<'a, 'tcx> DocFolder for LinkCollector<'a, 'tcx> { self.mod_ids.push(item.def_id); } + #[cfg(debug_assertions)] + for attr in &item.attrs.doc_strings { + if let Some(id) = attr.parent_module { + trace!("docs {:?} came from {:?}", attr.doc, id); + } else { + debug!("no parent found for {:?}", attr.doc); + } + } let dox = item.attrs.collapsed_doc_value().unwrap_or_else(String::new); - trace!("got documentation '{}'", dox); + //trace!("got documentation '{}'", dox); // find item's parent to resolve `Self` in item's docs below let parent_name = self.cx.as_local_hir_id(item.def_id).and_then(|item_hir| { diff --git a/src/librustdoc/passes/collect_trait_impls.rs b/src/librustdoc/passes/collect_trait_impls.rs index b2c4c30d8ff..5eb3f98b123 100644 --- a/src/librustdoc/passes/collect_trait_impls.rs +++ b/src/librustdoc/passes/collect_trait_impls.rs @@ -30,7 +30,7 @@ pub fn collect_trait_impls(krate: Crate, cx: &DocContext<'_>) -> Crate { for &cnum in cx.tcx.crates().iter() { for &(did, _) in cx.tcx.all_trait_implementations(cnum).iter() { cx.tcx.sess.time("build_extern_trait_impl", || { - inline::build_impl(cx, did, None, &mut new_items); + inline::build_impl(cx, None, did, None, &mut new_items); }); } } @@ -38,7 +38,7 @@ pub fn collect_trait_impls(krate: Crate, cx: &DocContext<'_>) -> Crate { // Also try to inline primitive impls from other crates. for &def_id in PrimitiveType::all_impls(cx.tcx).values().flatten() { if !def_id.is_local() { - inline::build_impl(cx, def_id, None, &mut new_items); + inline::build_impl(cx, None, def_id, None, &mut new_items); // FIXME(eddyb) is this `doc(hidden)` check needed? if !cx.tcx.get_attrs(def_id).lists(sym::doc).has_word(sym::hidden) { @@ -90,7 +90,7 @@ pub fn collect_trait_impls(krate: Crate, cx: &DocContext<'_>) -> Crate { for &impl_node in cx.tcx.hir().trait_impls(trait_did) { let impl_did = cx.tcx.hir().local_def_id(impl_node); cx.tcx.sess.time("build_local_trait_impl", || { - inline::build_impl(cx, impl_did.to_def_id(), None, &mut new_items); + inline::build_impl(cx, None, impl_did.to_def_id(), None, &mut new_items); }); } } diff --git a/src/librustdoc/visit_ast.rs b/src/librustdoc/visit_ast.rs index 33578dc0619..cbfd2199d9f 100644 --- a/src/librustdoc/visit_ast.rs +++ b/src/librustdoc/visit_ast.rs @@ -399,6 +399,7 @@ impl<'a, 'tcx> RustdocVisitor<'a, 'tcx> { om.extern_crates.push(ExternCrate { cnum: self.cx.tcx.extern_mod_stmt_cnum(def_id).unwrap_or(LOCAL_CRATE), name: ident.name, + hir_id: item.hir_id, path: orig_name.map(|x| x.to_string()), vis: &item.vis, attrs: &item.attrs,