Skip to content

Commit

Permalink
rustdoc: Cleanup parent module tracking for doc links
Browse files Browse the repository at this point in the history
Keep ids of the documented items themselves, not their parent modules.
Parent modules can be retreived from those ids when necessary.
  • Loading branch information
petrochenkov committed Mar 19, 2023
1 parent ab9bb3e commit f7a9702
Show file tree
Hide file tree
Showing 10 changed files with 100 additions and 165 deletions.
18 changes: 10 additions & 8 deletions compiler/rustc_resolve/src/rustdoc.rs
Original file line number Diff line number Diff line change
Expand Up @@ -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,
Expand Down Expand Up @@ -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());
Expand All @@ -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
Expand Down
75 changes: 23 additions & 52 deletions src/librustdoc/clean/inline.rs
Original file line number Diff line number Diff line change
Expand Up @@ -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()?;
Expand All @@ -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) => {
Expand All @@ -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.
Expand Down Expand Up @@ -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);
Expand Down Expand Up @@ -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.
Expand All @@ -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)
},
Expand All @@ -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()) {
Expand Down Expand Up @@ -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!(
Expand Down Expand Up @@ -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)
}
}
Expand Down
20 changes: 5 additions & 15 deletions src/librustdoc/clean/mod.rs
Original file line number Diff line number Diff line change
Expand Up @@ -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;
}
Expand Down Expand Up @@ -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,
Expand Down
2 changes: 1 addition & 1 deletion src/librustdoc/clean/types/tests.rs
Original file line number Diff line number Diff line change
Expand Up @@ -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,
Expand Down
4 changes: 2 additions & 2 deletions src/librustdoc/clean/utils.rs
Original file line number Diff line number Diff line change
Expand Up @@ -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);
}
}
}
Expand Down
Loading

0 comments on commit f7a9702

Please sign in to comment.