Skip to content

Commit

Permalink
Auto merge of rust-lang#113574 - GuillaumeGomez:rustdoc-json-strip-hi…
Browse files Browse the repository at this point in the history
…dden-impl, r=aDotInTheVoid,notriddle

Strip impl if not re-exported and is doc(hidden)

Part of rust-lang#112852.

r? `@aDotInTheVoid`
  • Loading branch information
bors committed Jul 18, 2023
2 parents 745efcc + c132cdb commit ec362f0
Show file tree
Hide file tree
Showing 16 changed files with 107 additions and 24 deletions.
6 changes: 3 additions & 3 deletions src/librustdoc/clean/inline.rs
Original file line number Diff line number Diff line change
Expand Up @@ -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()
}
Expand All @@ -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;
}
}
Expand All @@ -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() {
Expand Down
8 changes: 4 additions & 4 deletions src/librustdoc/clean/mod.rs
Original file line number Diff line number Diff line change
Expand Up @@ -53,7 +53,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
Expand All @@ -63,7 +63,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.
Expand All @@ -84,7 +84,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));
}
}
Expand Down Expand Up @@ -2326,7 +2326,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));
}
}
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 @@ -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));
Expand Down
2 changes: 1 addition & 1 deletion src/librustdoc/core.rs
Original file line number Diff line number Diff line change
Expand Up @@ -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,
Expand Down
7 changes: 5 additions & 2 deletions src/librustdoc/formats/cache.rs
Original file line number Diff line number Diff line change
Expand Up @@ -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].
///
Expand Down Expand Up @@ -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
Expand Down
2 changes: 1 addition & 1 deletion src/librustdoc/html/render/context.rs
Original file line number Diff line number Diff line change
Expand Up @@ -798,7 +798,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;
}
Expand Down
4 changes: 2 additions & 2 deletions src/librustdoc/passes/check_doc_test_visibility.rs
Original file line number Diff line number Diff line change
Expand Up @@ -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;
Expand Down
1 change: 1 addition & 0 deletions src/librustdoc/passes/strip_hidden.rs
Original file line number Diff line number Diff line change
Expand Up @@ -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)
}
Expand Down
7 changes: 6 additions & 1 deletion src/librustdoc/passes/strip_priv_imports.rs
Original file line number Diff line number Diff line change
Expand Up @@ -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)
}
9 changes: 7 additions & 2 deletions src/librustdoc/passes/strip_private.rs
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand All @@ -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)
}
19 changes: 16 additions & 3 deletions src/librustdoc/passes/stripper.rs
Original file line number Diff line number Diff line change
Expand Up @@ -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> {
Expand Down Expand Up @@ -151,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, '_> {
Expand All @@ -162,7 +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()
&& (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
}
Expand Down Expand Up @@ -231,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> {
Expand All @@ -247,8 +256,12 @@ impl<'tcx> ImportStripper<'tcx> {
impl<'tcx> DocFolder for ImportStripper<'tcx> {
fn fold_item(&mut self, i: Item) -> Option<Item> {
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) =>
{
Expand Down
10 changes: 7 additions & 3 deletions src/librustdoc/visit_ast.rs
Original file line number Diff line number Diff line change
Expand Up @@ -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;
Expand All @@ -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 &&
Expand Down Expand Up @@ -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()
Expand Down
4 changes: 3 additions & 1 deletion src/librustdoc/visit_lib.rs
Original file line number Diff line number Diff line change
Expand Up @@ -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)
}
Expand All @@ -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<'_, '_> {
Expand All @@ -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);
Expand Down
15 changes: 15 additions & 0 deletions tests/rustdoc-json/impls/issue-112852-dangling-trait-impl-id-2.rs
Original file line number Diff line number Diff line change
@@ -0,0 +1,15 @@
#![feature(no_core)]
#![no_core]

// @count "$.index[*][?(@.inner.impl)]" 1
// @!has "$.index[*][?(@.name == 'HiddenPubStruct')]"
// @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 {}
14 changes: 14 additions & 0 deletions tests/rustdoc-json/impls/issue-112852-dangling-trait-impl-id-3.rs
Original file line number Diff line number Diff line change
@@ -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 {}
21 changes: 21 additions & 0 deletions tests/rustdoc-json/impls/issue-112852-dangling-trait-impl-id.rs
Original file line number Diff line number Diff line change
@@ -0,0 +1,21 @@
#![feature(no_core)]
#![no_core]

// @count "$.index[*][?(@.inner.impl)]" 1
// @!has "$.index[*][?(@.name == 'HiddenPubStruct')]"
// @has "$.index[*][?(@.name == 'NotHiddenPubStruct')]"
// @has "$.index[*][?(@.name=='PubTrait')]"
pub trait PubTrait {}

#[doc(hidden)]
pub mod hidden {
pub struct HiddenPubStruct;

impl crate::PubTrait for HiddenPubStruct {}
}

pub mod not_hidden {
pub struct NotHiddenPubStruct;

impl crate::PubTrait for NotHiddenPubStruct {}
}

0 comments on commit ec362f0

Please sign in to comment.