Skip to content

Commit

Permalink
Rollup merge of rust-lang#109312 - petrochenkov:docice5, r=GuillaumeG…
Browse files Browse the repository at this point in the history 
…omez

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.

Fixes rust-lang#108501.
That issue could be fixed in a more local way, but this refactoring is something that I wanted to do since rust-lang#93805 anyway.
  • Loading branch information
@Dylan-DPC
Dylan-DPC authored Mar 22, 2023
2 parents 46a3442 + 0f45d85 commit b3e6cbb
Show file tree
Hide file tree
Showing 10 changed files with 123 additions and 185 deletions.
18 changes: 10 additions & 8 deletions compiler/rustc_resolve/src/rustdoc.rs
View file
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
View file
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
View file
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
View file
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
View file
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 b3e6cbb

Please sign in to comment.