From a5e0ca962ac2dbd5973cf2a171cd6f695d28c3f1 Mon Sep 17 00:00:00 2001 From: Guillaume Gomez Date: Tue, 11 Jul 2023 16:31:57 +0200 Subject: [PATCH 1/5] Strip impl if not re-exported and is doc(hidden) --- src/librustdoc/passes/stripper.rs | 8 +++++++- 1 file changed, 7 insertions(+), 1 deletion(-) diff --git a/src/librustdoc/passes/stripper.rs b/src/librustdoc/passes/stripper.rs index 90c361d9d2812..19b543c3f6b70 100644 --- a/src/librustdoc/passes/stripper.rs +++ b/src/librustdoc/passes/stripper.rs @@ -6,6 +6,7 @@ use std::mem; use crate::clean::{self, Item, ItemId, ItemIdSet}; use crate::fold::{strip_item, DocFolder}; use crate::formats::cache::Cache; +use crate::visit_ast::inherits_doc_hidden; use crate::visit_lib::RustdocEffectiveVisibilities; pub(crate) struct Stripper<'a, 'tcx> { @@ -162,7 +163,12 @@ impl<'a> ImplStripper<'a, '_> { // If the "for" item is exported and the impl block isn't `#[doc(hidden)]`, then we // need to keep it. self.cache.effective_visibilities.is_exported(self.tcx, for_def_id) - && !item.is_doc_hidden() + && ((!item.is_doc_hidden() + && for_def_id + .as_local() + .map(|def_id| !inherits_doc_hidden(self.tcx, def_id, None)) + .unwrap_or(true)) + || self.cache.inlined_items.contains(&for_def_id)) } else { false } From 449cc6549f9f7a6e1d73e1d7a45b856c0b5f7320 Mon Sep 17 00:00:00 2001 From: Guillaume Gomez Date: Tue, 11 Jul 2023 16:32:11 +0200 Subject: [PATCH 2/5] Add regression test for #112852 --- .../impls/issue-112852-dangling-trait-impl-id.rs | 14 ++++++++++++++ 1 file changed, 14 insertions(+) create mode 100644 tests/rustdoc-json/impls/issue-112852-dangling-trait-impl-id.rs diff --git a/tests/rustdoc-json/impls/issue-112852-dangling-trait-impl-id.rs b/tests/rustdoc-json/impls/issue-112852-dangling-trait-impl-id.rs new file mode 100644 index 0000000000000..035d147d34607 --- /dev/null +++ b/tests/rustdoc-json/impls/issue-112852-dangling-trait-impl-id.rs @@ -0,0 +1,14 @@ +#![feature(no_core)] +#![no_core] + +// @!has "$.index[*][?(@.name == 'HiddenPubStruct')]" +// @!has "$.index[*][?(@.inner.impl)]" +// @has "$.index[*][?(@.name=='PubTrait')]" +pub trait PubTrait {} + +#[doc(hidden)] +pub mod hidden { + pub struct HiddenPubStruct; + + impl crate::PubTrait for HiddenPubStruct {} +} From ab80b36452030192638fe14b4e126f8c52dcc521 Mon Sep 17 00:00:00 2001 From: Guillaume Gomez Date: Fri, 14 Jul 2023 16:44:41 +0200 Subject: [PATCH 3/5] Correctly handle `--document-hidden-items` --- src/librustdoc/clean/inline.rs | 6 ++--- src/librustdoc/clean/mod.rs | 8 +++---- src/librustdoc/clean/types/tests.rs | 2 +- src/librustdoc/core.rs | 2 +- src/librustdoc/formats/cache.rs | 7 ++++-- src/librustdoc/html/render/context.rs | 2 +- .../passes/check_doc_test_visibility.rs | 4 ++-- src/librustdoc/passes/strip_hidden.rs | 1 + src/librustdoc/passes/strip_priv_imports.rs | 7 +++++- src/librustdoc/passes/strip_private.rs | 9 ++++++-- src/librustdoc/passes/stripper.rs | 23 ++++++++++++------- src/librustdoc/visit_ast.rs | 10 +++++--- src/librustdoc/visit_lib.rs | 4 +++- 13 files changed, 56 insertions(+), 29 deletions(-) diff --git a/src/librustdoc/clean/inline.rs b/src/librustdoc/clean/inline.rs index 870cfa93058aa..e1d4a3b910a45 100644 --- a/src/librustdoc/clean/inline.rs +++ b/src/librustdoc/clean/inline.rs @@ -473,7 +473,7 @@ pub(crate) fn build_impl( associated_trait.def_id, ) .unwrap(); // corresponding associated item has to exist - !tcx.is_doc_hidden(trait_item.def_id) + document_hidden || !tcx.is_doc_hidden(trait_item.def_id) } else { item.visibility(tcx).is_public() } @@ -496,7 +496,7 @@ pub(crate) fn build_impl( let mut stack: Vec<&Type> = vec![&for_]; if let Some(did) = trait_.as_ref().map(|t| t.def_id()) { - if tcx.is_doc_hidden(did) { + if !document_hidden && tcx.is_doc_hidden(did) { return; } } @@ -505,7 +505,7 @@ pub(crate) fn build_impl( } while let Some(ty) = stack.pop() { - if let Some(did) = ty.def_id(&cx.cache) && tcx.is_doc_hidden(did) { + if let Some(did) = ty.def_id(&cx.cache) && !document_hidden && tcx.is_doc_hidden(did) { return; } if let Some(generics) = ty.generics() { diff --git a/src/librustdoc/clean/mod.rs b/src/librustdoc/clean/mod.rs index d26de77a78d38..2157909122eb8 100644 --- a/src/librustdoc/clean/mod.rs +++ b/src/librustdoc/clean/mod.rs @@ -54,7 +54,7 @@ pub(crate) fn clean_doc_module<'tcx>(doc: &DocModule<'tcx>, cx: &mut DocContext< let mut inserted = FxHashSet::default(); items.extend(doc.foreigns.iter().map(|(item, renamed)| { let item = clean_maybe_renamed_foreign_item(cx, item, *renamed); - if let Some(name) = item.name && !item.is_doc_hidden() { + if let Some(name) = item.name && (cx.render_options.document_hidden || !item.is_doc_hidden()) { inserted.insert((item.type_(), name)); } item @@ -64,7 +64,7 @@ pub(crate) fn clean_doc_module<'tcx>(doc: &DocModule<'tcx>, cx: &mut DocContext< return None; } let item = clean_doc_module(x, cx); - if item.is_doc_hidden() { + if !cx.render_options.document_hidden && item.is_doc_hidden() { // Hidden modules are stripped at a later stage. // If a hidden module has the same name as a visible one, we want // to keep both of them around. @@ -85,7 +85,7 @@ pub(crate) fn clean_doc_module<'tcx>(doc: &DocModule<'tcx>, cx: &mut DocContext< } let v = clean_maybe_renamed_item(cx, item, *renamed, *import_id); for item in &v { - if let Some(name) = item.name && !item.is_doc_hidden() { + if let Some(name) = item.name && (cx.render_options.document_hidden || !item.is_doc_hidden()) { inserted.insert((item.type_(), name)); } } @@ -2323,7 +2323,7 @@ fn get_all_import_attributes<'hir>( attrs = import_attrs.iter().map(|attr| (Cow::Borrowed(attr), Some(def_id))).collect(); first = false; // We don't add attributes of an intermediate re-export if it has `#[doc(hidden)]`. - } else if !cx.tcx.is_doc_hidden(def_id) { + } else if cx.render_options.document_hidden || !cx.tcx.is_doc_hidden(def_id) { add_without_unwanted_attributes(&mut attrs, import_attrs, is_inline, Some(def_id)); } } diff --git a/src/librustdoc/clean/types/tests.rs b/src/librustdoc/clean/types/tests.rs index 394954208a48e..4907a55270b8e 100644 --- a/src/librustdoc/clean/types/tests.rs +++ b/src/librustdoc/clean/types/tests.rs @@ -73,7 +73,7 @@ fn should_not_trim() { fn is_same_generic() { use crate::clean::types::{PrimitiveType, Type}; use crate::formats::cache::Cache; - let cache = Cache::new(false); + let cache = Cache::new(false, false); let generic = Type::Generic(rustc_span::symbol::sym::Any); let unit = Type::Primitive(PrimitiveType::Unit); assert!(!generic.is_doc_subtype_of(&unit, &cache)); diff --git a/src/librustdoc/core.rs b/src/librustdoc/core.rs index 9687b8b18878e..4d920089e7dce 100644 --- a/src/librustdoc/core.rs +++ b/src/librustdoc/core.rs @@ -345,7 +345,7 @@ pub(crate) fn run_global_ctxt( impl_trait_bounds: Default::default(), generated_synthetics: Default::default(), auto_traits, - cache: Cache::new(render_options.document_private), + cache: Cache::new(render_options.document_private, render_options.document_hidden), inlined: FxHashSet::default(), output_format, render_options, diff --git a/src/librustdoc/formats/cache.rs b/src/librustdoc/formats/cache.rs index dac762e9ff9f6..a15d43793b7e0 100644 --- a/src/librustdoc/formats/cache.rs +++ b/src/librustdoc/formats/cache.rs @@ -86,6 +86,9 @@ pub(crate) struct Cache { /// Whether to document private items. /// This is stored in `Cache` so it doesn't need to be passed through all rustdoc functions. pub(crate) document_private: bool, + /// Whether to document hidden items. + /// This is stored in `Cache` so it doesn't need to be passed through all rustdoc functions. + pub(crate) document_hidden: bool, /// Crates marked with [`#[doc(masked)]`][doc_masked]. /// @@ -137,8 +140,8 @@ struct CacheBuilder<'a, 'tcx> { } impl Cache { - pub(crate) fn new(document_private: bool) -> Self { - Cache { document_private, ..Cache::default() } + pub(crate) fn new(document_private: bool, document_hidden: bool) -> Self { + Cache { document_private, document_hidden, ..Cache::default() } } /// Populates the `Cache` with more data. The returned `Crate` will be missing some data that was diff --git a/src/librustdoc/html/render/context.rs b/src/librustdoc/html/render/context.rs index d0a1e223eb85a..130fe8a9bedbe 100644 --- a/src/librustdoc/html/render/context.rs +++ b/src/librustdoc/html/render/context.rs @@ -803,7 +803,7 @@ impl<'tcx> FormatRenderer<'tcx> for Context<'tcx> { if let Some(def_id) = item.def_id() && self.cache().inlined_items.contains(&def_id) { self.is_inside_inlined_module = true; } - } else if item.is_doc_hidden() { + } else if !self.cache().document_hidden && item.is_doc_hidden() { // We're not inside an inlined module anymore since this one cannot be re-exported. self.is_inside_inlined_module = false; } diff --git a/src/librustdoc/passes/check_doc_test_visibility.rs b/src/librustdoc/passes/check_doc_test_visibility.rs index 1aa12e3ced2e5..011ca9a496142 100644 --- a/src/librustdoc/passes/check_doc_test_visibility.rs +++ b/src/librustdoc/passes/check_doc_test_visibility.rs @@ -92,8 +92,8 @@ pub(crate) fn should_have_doc_example(cx: &DocContext<'_>, item: &clean::Item) - return false; } - if cx.tcx.is_doc_hidden(def_id.to_def_id()) - || inherits_doc_hidden(cx.tcx, def_id, None) + if (!cx.render_options.document_hidden + && (cx.tcx.is_doc_hidden(def_id.to_def_id()) || inherits_doc_hidden(cx.tcx, def_id, None))) || cx.tcx.def_span(def_id.to_def_id()).in_derive_expansion() { return false; diff --git a/src/librustdoc/passes/strip_hidden.rs b/src/librustdoc/passes/strip_hidden.rs index e2e38d3e79f73..7b990cd434882 100644 --- a/src/librustdoc/passes/strip_hidden.rs +++ b/src/librustdoc/passes/strip_hidden.rs @@ -42,6 +42,7 @@ pub(crate) fn strip_hidden(krate: clean::Crate, cx: &mut DocContext<'_>) -> clea cache: &cx.cache, is_json_output, document_private: cx.render_options.document_private, + document_hidden: cx.render_options.document_hidden, }; stripper.fold_crate(krate) } diff --git a/src/librustdoc/passes/strip_priv_imports.rs b/src/librustdoc/passes/strip_priv_imports.rs index 4c992e94833d6..468712ba3d0ba 100644 --- a/src/librustdoc/passes/strip_priv_imports.rs +++ b/src/librustdoc/passes/strip_priv_imports.rs @@ -13,5 +13,10 @@ pub(crate) const STRIP_PRIV_IMPORTS: Pass = Pass { pub(crate) fn strip_priv_imports(krate: clean::Crate, cx: &mut DocContext<'_>) -> clean::Crate { let is_json_output = cx.output_format.is_json() && !cx.show_coverage; - ImportStripper { tcx: cx.tcx, is_json_output }.fold_crate(krate) + ImportStripper { + tcx: cx.tcx, + is_json_output, + document_hidden: cx.render_options.document_hidden, + } + .fold_crate(krate) } diff --git a/src/librustdoc/passes/strip_private.rs b/src/librustdoc/passes/strip_private.rs index bb6dccb7c9499..3b6f484fde650 100644 --- a/src/librustdoc/passes/strip_private.rs +++ b/src/librustdoc/passes/strip_private.rs @@ -28,8 +28,12 @@ pub(crate) fn strip_private(mut krate: clean::Crate, cx: &mut DocContext<'_>) -> is_json_output, tcx: cx.tcx, }; - krate = - ImportStripper { tcx: cx.tcx, is_json_output }.fold_crate(stripper.fold_crate(krate)); + krate = ImportStripper { + tcx: cx.tcx, + is_json_output, + document_hidden: cx.render_options.document_hidden, + } + .fold_crate(stripper.fold_crate(krate)); } // strip all impls referencing private items @@ -39,6 +43,7 @@ pub(crate) fn strip_private(mut krate: clean::Crate, cx: &mut DocContext<'_>) -> cache: &cx.cache, is_json_output, document_private: cx.render_options.document_private, + document_hidden: cx.render_options.document_hidden, }; stripper.fold_crate(krate) } diff --git a/src/librustdoc/passes/stripper.rs b/src/librustdoc/passes/stripper.rs index 19b543c3f6b70..a6d31534f1d1b 100644 --- a/src/librustdoc/passes/stripper.rs +++ b/src/librustdoc/passes/stripper.rs @@ -152,6 +152,7 @@ pub(crate) struct ImplStripper<'a, 'tcx> { pub(crate) cache: &'a Cache, pub(crate) is_json_output: bool, pub(crate) document_private: bool, + pub(crate) document_hidden: bool, } impl<'a> ImplStripper<'a, '_> { @@ -163,12 +164,13 @@ impl<'a> ImplStripper<'a, '_> { // If the "for" item is exported and the impl block isn't `#[doc(hidden)]`, then we // need to keep it. self.cache.effective_visibilities.is_exported(self.tcx, for_def_id) - && ((!item.is_doc_hidden() - && for_def_id - .as_local() - .map(|def_id| !inherits_doc_hidden(self.tcx, def_id, None)) - .unwrap_or(true)) - || self.cache.inlined_items.contains(&for_def_id)) + && (self.document_hidden + || ((!item.is_doc_hidden() + && for_def_id + .as_local() + .map(|def_id| !inherits_doc_hidden(self.tcx, def_id, None)) + .unwrap_or(true)) + || self.cache.inlined_items.contains(&for_def_id))) } else { false } @@ -237,6 +239,7 @@ impl<'a> DocFolder for ImplStripper<'a, '_> { pub(crate) struct ImportStripper<'tcx> { pub(crate) tcx: TyCtxt<'tcx>, pub(crate) is_json_output: bool, + pub(crate) document_hidden: bool, } impl<'tcx> ImportStripper<'tcx> { @@ -253,8 +256,12 @@ impl<'tcx> ImportStripper<'tcx> { impl<'tcx> DocFolder for ImportStripper<'tcx> { fn fold_item(&mut self, i: Item) -> Option { match *i.kind { - clean::ImportItem(imp) if self.import_should_be_hidden(&i, &imp) => None, - clean::ImportItem(_) if i.is_doc_hidden() => None, + clean::ImportItem(imp) + if !self.document_hidden && self.import_should_be_hidden(&i, &imp) => + { + None + } + // clean::ImportItem(_) if !self.document_hidden && i.is_doc_hidden() => None, clean::ExternCrateItem { .. } | clean::ImportItem(..) if i.visibility(self.tcx) != Some(Visibility::Public) => { diff --git a/src/librustdoc/visit_ast.rs b/src/librustdoc/visit_ast.rs index fcf591a932896..265123ddf6c8b 100644 --- a/src/librustdoc/visit_ast.rs +++ b/src/librustdoc/visit_ast.rs @@ -262,10 +262,11 @@ impl<'a, 'tcx> RustdocVisitor<'a, 'tcx> { return false; }; + let document_hidden = self.cx.render_options.document_hidden; let use_attrs = tcx.hir().attrs(tcx.hir().local_def_id_to_hir_id(def_id)); // Don't inline `doc(hidden)` imports so they can be stripped at a later stage. let is_no_inline = use_attrs.lists(sym::doc).has_word(sym::no_inline) - || use_attrs.lists(sym::doc).has_word(sym::hidden); + || (document_hidden && use_attrs.lists(sym::doc).has_word(sym::hidden)); if is_no_inline { return false; @@ -285,11 +286,11 @@ impl<'a, 'tcx> RustdocVisitor<'a, 'tcx> { }; let is_private = !self.cx.cache.effective_visibilities.is_directly_public(tcx, ori_res_did); - let is_hidden = tcx.is_doc_hidden(ori_res_did); + let is_hidden = !document_hidden && tcx.is_doc_hidden(ori_res_did); let item = tcx.hir().get_by_def_id(res_did); if !please_inline { - let inherits_hidden = inherits_doc_hidden(tcx, res_did, None); + let inherits_hidden = !document_hidden && inherits_doc_hidden(tcx, res_did, None); // Only inline if requested or if the item would otherwise be stripped. if (!is_private && !inherits_hidden) || ( is_hidden && @@ -359,6 +360,9 @@ impl<'a, 'tcx> RustdocVisitor<'a, 'tcx> { import_def_id: LocalDefId, target_def_id: LocalDefId, ) -> bool { + if self.cx.render_options.document_hidden { + return true; + } let tcx = self.cx.tcx; let item_def_id = reexport_chain(tcx, import_def_id, target_def_id) .iter() diff --git a/src/librustdoc/visit_lib.rs b/src/librustdoc/visit_lib.rs index fd4f9254107ca..82c97466111e5 100644 --- a/src/librustdoc/visit_lib.rs +++ b/src/librustdoc/visit_lib.rs @@ -33,6 +33,7 @@ pub(crate) fn lib_embargo_visit_item(cx: &mut DocContext<'_>, def_id: DefId) { tcx: cx.tcx, extern_public: &mut cx.cache.effective_visibilities.extern_public, visited_mods: Default::default(), + document_hidden: cx.render_options.document_hidden, } .visit_item(def_id) } @@ -45,6 +46,7 @@ struct LibEmbargoVisitor<'a, 'tcx> { extern_public: &'a mut DefIdSet, // Keeps track of already visited modules, in case a module re-exports its parent visited_mods: DefIdSet, + document_hidden: bool, } impl LibEmbargoVisitor<'_, '_> { @@ -63,7 +65,7 @@ impl LibEmbargoVisitor<'_, '_> { } fn visit_item(&mut self, def_id: DefId) { - if !self.tcx.is_doc_hidden(def_id) { + if self.document_hidden || !self.tcx.is_doc_hidden(def_id) { self.extern_public.insert(def_id); if self.tcx.def_kind(def_id) == DefKind::Mod { self.visit_mod(def_id); From 2861a56178f2e3637177c96f51ecb0eaa64492e6 Mon Sep 17 00:00:00 2001 From: Guillaume Gomez Date: Fri, 14 Jul 2023 16:45:41 +0200 Subject: [PATCH 4/5] Add more tests for not-reexported impl --- .../impls/issue-112852-dangling-trait-impl-id-2.rs | 12 ++++++++++++ .../impls/issue-112852-dangling-trait-impl-id-3.rs | 14 ++++++++++++++ 2 files changed, 26 insertions(+) create mode 100644 tests/rustdoc-json/impls/issue-112852-dangling-trait-impl-id-2.rs create mode 100644 tests/rustdoc-json/impls/issue-112852-dangling-trait-impl-id-3.rs diff --git a/tests/rustdoc-json/impls/issue-112852-dangling-trait-impl-id-2.rs b/tests/rustdoc-json/impls/issue-112852-dangling-trait-impl-id-2.rs new file mode 100644 index 0000000000000..f8c21f937b2e9 --- /dev/null +++ b/tests/rustdoc-json/impls/issue-112852-dangling-trait-impl-id-2.rs @@ -0,0 +1,12 @@ +#![feature(no_core)] +#![no_core] + +// @!has "$.index[*][?(@.name == 'HiddenPubStruct')]" +// @!has "$.index[*][?(@.inner.impl)]" +// @has "$.index[*][?(@.name=='PubTrait')]" +pub trait PubTrait {} + +#[doc(hidden)] +pub struct HiddenPubStruct; + +impl PubTrait for HiddenPubStruct {} diff --git a/tests/rustdoc-json/impls/issue-112852-dangling-trait-impl-id-3.rs b/tests/rustdoc-json/impls/issue-112852-dangling-trait-impl-id-3.rs new file mode 100644 index 0000000000000..fcd27ca4b7c57 --- /dev/null +++ b/tests/rustdoc-json/impls/issue-112852-dangling-trait-impl-id-3.rs @@ -0,0 +1,14 @@ +// compile-flags: --document-hidden-items + +#![feature(no_core)] +#![no_core] + +// @has "$.index[*][?(@.name == 'HiddenPubStruct')]" +// @has "$.index[*][?(@.inner.impl)]" +// @has "$.index[*][?(@.name=='PubTrait')]" +pub trait PubTrait {} + +#[doc(hidden)] +pub struct HiddenPubStruct; + +impl PubTrait for HiddenPubStruct {} From c132cdbe5f3186089517e4fb288320be85388cbc Mon Sep 17 00:00:00 2001 From: Guillaume Gomez Date: Mon, 17 Jul 2023 23:36:33 +0200 Subject: [PATCH 5/5] Improve issue-112852 tests --- .../impls/issue-112852-dangling-trait-impl-id-2.rs | 5 ++++- .../impls/issue-112852-dangling-trait-impl-id.rs | 9 ++++++++- 2 files changed, 12 insertions(+), 2 deletions(-) diff --git a/tests/rustdoc-json/impls/issue-112852-dangling-trait-impl-id-2.rs b/tests/rustdoc-json/impls/issue-112852-dangling-trait-impl-id-2.rs index f8c21f937b2e9..d2ac316d47d4d 100644 --- a/tests/rustdoc-json/impls/issue-112852-dangling-trait-impl-id-2.rs +++ b/tests/rustdoc-json/impls/issue-112852-dangling-trait-impl-id-2.rs @@ -1,12 +1,15 @@ #![feature(no_core)] #![no_core] +// @count "$.index[*][?(@.inner.impl)]" 1 // @!has "$.index[*][?(@.name == 'HiddenPubStruct')]" -// @!has "$.index[*][?(@.inner.impl)]" +// @has "$.index[*][?(@.name == 'NotHiddenPubStruct')]" // @has "$.index[*][?(@.name=='PubTrait')]" pub trait PubTrait {} #[doc(hidden)] pub struct HiddenPubStruct; +pub struct NotHiddenPubStruct; impl PubTrait for HiddenPubStruct {} +impl PubTrait for NotHiddenPubStruct {} diff --git a/tests/rustdoc-json/impls/issue-112852-dangling-trait-impl-id.rs b/tests/rustdoc-json/impls/issue-112852-dangling-trait-impl-id.rs index 035d147d34607..141c54a57dd11 100644 --- a/tests/rustdoc-json/impls/issue-112852-dangling-trait-impl-id.rs +++ b/tests/rustdoc-json/impls/issue-112852-dangling-trait-impl-id.rs @@ -1,8 +1,9 @@ #![feature(no_core)] #![no_core] +// @count "$.index[*][?(@.inner.impl)]" 1 // @!has "$.index[*][?(@.name == 'HiddenPubStruct')]" -// @!has "$.index[*][?(@.inner.impl)]" +// @has "$.index[*][?(@.name == 'NotHiddenPubStruct')]" // @has "$.index[*][?(@.name=='PubTrait')]" pub trait PubTrait {} @@ -12,3 +13,9 @@ pub mod hidden { impl crate::PubTrait for HiddenPubStruct {} } + +pub mod not_hidden { + pub struct NotHiddenPubStruct; + + impl crate::PubTrait for NotHiddenPubStruct {} +}