Skip to content

Commit

Permalink
Preserve the parent module of DocFragments
Browse files Browse the repository at this point in the history
- 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.
  • Loading branch information
jyn514 committed Oct 8, 2020
1 parent 4437b4b commit fa1b15f
Show file tree
Hide file tree
Showing 9 changed files with 101 additions and 40 deletions.
61 changes: 40 additions & 21 deletions src/librustdoc/clean/inline.rs
Expand Up @@ -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;

Expand All @@ -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<Attrs<'_>>,
Expand All @@ -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) => {
Expand All @@ -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.
Expand Down Expand Up @@ -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 {
Expand Down Expand Up @@ -291,40 +295,52 @@ pub fn build_ty(cx: &DocContext<'_>, did: DefId) -> Option<clean::Type> {
}

/// 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<Attrs<'_>>) -> Vec<clean::Item> {
pub fn build_impls(
cx: &DocContext<'_>,
parent_module: Option<DefId>,
did: DefId,
attrs: Option<Attrs<'_>>,
) -> Vec<clean::Item> {
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<Attrs<'_>>,
parent_module: Option<DefId>,
old_attrs: Attrs<'_>,
new_attrs: Option<Attrs<'_>>,
) -> 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<Option<DefId>>,
did: DefId,
attrs: Option<Attrs<'_>>,
ret: &mut Vec<clean::Item>,
Expand All @@ -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);
Expand Down Expand Up @@ -499,7 +516,9 @@ fn build_module(cx: &DocContext<'_>, did: DefId, visited: &mut FxHashSet<DefId>)
},
)),
});
} 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)
}
}
Expand Down
24 changes: 17 additions & 7 deletions src/librustdoc/clean/mod.rs
Expand Up @@ -284,7 +284,7 @@ impl Clean<Item> for doctree::Module<'_> {

impl Clean<Attributes> for [ast::Attribute] {
fn clean(&self, cx: &DocContext<'_>) -> Attributes {
Attributes::from_ast(cx.sess().diagnostic(), self)
Attributes::from_ast(cx.sess().diagnostic(), self, None)
}
}

Expand Down Expand Up @@ -2205,9 +2205,14 @@ impl Clean<Vec<Item>> 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;
}
}
Expand Down Expand Up @@ -2268,9 +2273,14 @@ impl Clean<Vec<Item>> 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;
}
}
Expand Down
31 changes: 26 additions & 5 deletions src/librustdoc/clean/types.rs
Expand Up @@ -373,6 +373,11 @@ impl<I: IntoIterator<Item = ast::NestedMetaItem>> 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<DefId>,
pub doc: String,
pub kind: DocFragmentKind,
}
Expand Down Expand Up @@ -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
Expand All @@ -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);
Expand All @@ -565,6 +585,7 @@ impl Attributes {
span: attr.span,
doc: contents,
kind: DocFragmentKind::Include { filename },
parent_module: parent_module,
});
}
}
Expand Down
4 changes: 2 additions & 2 deletions src/librustdoc/clean/utils.rs
Expand Up @@ -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() {
Expand All @@ -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);
}
}
}
Expand Down
2 changes: 1 addition & 1 deletion src/librustdoc/doctest.rs
Expand Up @@ -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;
Expand Down
2 changes: 2 additions & 0 deletions src/librustdoc/doctree.rs
Expand Up @@ -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<Symbol>,
Expand Down Expand Up @@ -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<String>,
pub vis: &'hir hir::Visibility<'hir>,
Expand Down
10 changes: 9 additions & 1 deletion src/librustdoc/passes/collect_intra_doc_links.rs
Expand Up @@ -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| {
Expand Down
6 changes: 3 additions & 3 deletions src/librustdoc/passes/collect_trait_impls.rs
Expand Up @@ -30,15 +30,15 @@ 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);
});
}
}

// 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) {
Expand Down Expand Up @@ -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);
});
}
}
Expand Down
1 change: 1 addition & 0 deletions src/librustdoc/visit_ast.rs
Expand Up @@ -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,
Expand Down

0 comments on commit fa1b15f

Please sign in to comment.