From ea844187b27fbcff521bcbcbe6615d51d0196fa2 Mon Sep 17 00:00:00 2001 From: Guillaume Gomez Date: Fri, 27 Jan 2023 20:32:50 +0100 Subject: [PATCH] Special-case handling of impl blocks --- src/librustdoc/passes/strip_hidden.rs | 105 +++++++++++++++----------- src/librustdoc/visit_ast.rs | 18 ++++- 2 files changed, 75 insertions(+), 48 deletions(-) diff --git a/src/librustdoc/passes/strip_hidden.rs b/src/librustdoc/passes/strip_hidden.rs index cfd2171395ce..8c733ddefc0a 100644 --- a/src/librustdoc/passes/strip_hidden.rs +++ b/src/librustdoc/passes/strip_hidden.rs @@ -59,57 +59,74 @@ impl<'a, 'tcx> Stripper<'a, 'tcx> { self.is_in_hidden_item = prev; ret } + + /// In case `i` is a non-hidden impl block, then we special-case it by changing the value + /// of `is_in_hidden_item` to `true` because the impl children inherit its visibility. + fn recurse_in_impl(&mut self, i: Item) -> Item { + let prev = mem::replace(&mut self.is_in_hidden_item, false); + let ret = self.fold_item_recur(i); + self.is_in_hidden_item = prev; + ret + } } impl<'a, 'tcx> DocFolder for Stripper<'a, 'tcx> { fn fold_item(&mut self, i: Item) -> Option { let has_doc_hidden = i.attrs.lists(sym::doc).has_word(sym::hidden); - let mut is_hidden = self.is_in_hidden_item || has_doc_hidden; - if !is_hidden && i.inline_stmt_id.is_none() { - // We don't need to check if it's coming from a reexport since the reexport itself was - // already checked. - is_hidden = i - .item_id - .as_def_id() - .and_then(|def_id| def_id.as_local()) - .map(|def_id| inherits_doc_hidden(self.tcx, def_id)) - .unwrap_or(false); + let is_impl = matches!(*i.kind, clean::ImplItem(..)); + let mut is_hidden = has_doc_hidden; + if !is_impl { + is_hidden = self.is_in_hidden_item || has_doc_hidden; + if !is_hidden && i.inline_stmt_id.is_none() { + // We don't need to check if it's coming from a reexport since the reexport itself was + // already checked. + is_hidden = i + .item_id + .as_def_id() + .and_then(|def_id| def_id.as_local()) + .map(|def_id| inherits_doc_hidden(self.tcx, def_id)) + .unwrap_or(false); + } } - if is_hidden { - debug!("strip_hidden: stripping {:?} {:?}", i.type_(), i.name); - // Use a dedicated hidden item for fields, variants, and modules. - // We need to keep private fields and variants, so that the docs - // can show a placeholder "// some variants omitted". We need to keep - // private modules, because they can contain impl blocks, and impl - // block privacy is inherited from the type and trait, not from the - // module it's defined in. Both of these are marked "stripped," and - // not included in the final docs, but since they still have an effect - // on the final doc, cannot be completely removed from the Clean IR. - return match *i.kind { - clean::StructFieldItem(..) | clean::ModuleItem(..) | clean::VariantItem(..) => { - // We need to recurse into stripped modules to - // strip things like impl methods but when doing so - // we must not add any items to the `retained` set. - let old = mem::replace(&mut self.update_retained, false); - let ret = strip_item(self.set_is_in_hidden_item_and_fold(true, i)); - self.update_retained = old; - Some(ret) + if !is_hidden { + if self.update_retained { + self.retained.insert(i.item_id); + } + return Some(if is_impl { + self.recurse_in_impl(i) + } else { + self.set_is_in_hidden_item_and_fold(false, i) + }); + } + debug!("strip_hidden: stripping {:?} {:?}", i.type_(), i.name); + // Use a dedicated hidden item for fields, variants, and modules. + // We need to keep private fields and variants, so that the docs + // can show a placeholder "// some variants omitted". We need to keep + // private modules, because they can contain impl blocks, and impl + // block privacy is inherited from the type and trait, not from the + // module it's defined in. Both of these are marked "stripped," and + // not included in the final docs, but since they still have an effect + // on the final doc, cannot be completely removed from the Clean IR. + match *i.kind { + clean::StructFieldItem(..) | clean::ModuleItem(..) | clean::VariantItem(..) => { + // We need to recurse into stripped modules to + // strip things like impl methods but when doing so + // we must not add any items to the `retained` set. + let old = mem::replace(&mut self.update_retained, false); + let ret = strip_item(self.set_is_in_hidden_item_and_fold(true, i)); + self.update_retained = old; + Some(ret) + } + _ => { + let ret = self.set_is_in_hidden_item_and_fold(true, i); + if has_doc_hidden { + // If the item itself has `#[doc(hidden)]`, then we simply remove it. + None + } else { + // However if it's a "descendant" of a `#[doc(hidden)]` item, then we strip it. + Some(strip_item(ret)) } - _ => { - let ret = self.set_is_in_hidden_item_and_fold(true, i); - if has_doc_hidden { - // If the item itself has `#[doc(hidden)]`, then we simply remove it. - None - } else { - // However if it's a "descendant" of a `#[doc(hidden)]` item, then we strip it. - Some(strip_item(ret)) - } - } - }; + } } - if self.update_retained { - self.retained.insert(i.item_id); - } - Some(self.set_is_in_hidden_item_and_fold(is_hidden, i)) } } diff --git a/src/librustdoc/visit_ast.rs b/src/librustdoc/visit_ast.rs index 2d2afb83f9dd..088cb3f33949 100644 --- a/src/librustdoc/visit_ast.rs +++ b/src/librustdoc/visit_ast.rs @@ -55,11 +55,21 @@ fn def_id_to_path(tcx: TyCtxt<'_>, did: DefId) -> Vec { std::iter::once(crate_name).chain(relative).collect() } -pub(crate) fn inherits_doc_hidden(tcx: TyCtxt<'_>, mut node: LocalDefId) -> bool { - while let Some(id) = tcx.opt_local_parent(node) { - node = id; - if tcx.is_doc_hidden(node.to_def_id()) { +pub(crate) fn inherits_doc_hidden(tcx: TyCtxt<'_>, mut def_id: LocalDefId) -> bool { + let hir = tcx.hir(); + while let Some(id) = tcx.opt_local_parent(def_id) { + def_id = id; + if tcx.is_doc_hidden(def_id.to_def_id()) { return true; + } else if let Some(node) = hir.find_by_def_id(def_id) && + matches!( + node, + hir::Node::Item(hir::Item { kind: hir::ItemKind::Impl(_), .. }), + ) + { + // `impl` blocks stand a bit on their own: unless they have `#[doc(hidden)]` directly + // on them, they don't inherit it from the parent context. + return false; } } false