Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

rustdoc: Cleanup parent module tracking for doc links #109312

Merged
merged 2 commits into from
Mar 22, 2023
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
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
26 changes: 8 additions & 18 deletions src/librustdoc/clean/mod.rs
Original file line number Diff line number Diff line change
Expand Up @@ -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]
})
}
Expand Down Expand Up @@ -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;
}
Expand Down Expand Up @@ -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,
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