From f91d02b153a357b9937d6a5193db931076795deb Mon Sep 17 00:00:00 2001 From: Guillaume Gomez Date: Mon, 17 Apr 2023 11:13:29 +0200 Subject: [PATCH 1/3] Remove unused RustdocVisitor::visit_item_inner return type --- src/librustdoc/visit_ast.rs | 8 +++----- 1 file changed, 3 insertions(+), 5 deletions(-) diff --git a/src/librustdoc/visit_ast.rs b/src/librustdoc/visit_ast.rs index 393d51fe09069..5ac68da150a9a 100644 --- a/src/librustdoc/visit_ast.rs +++ b/src/librustdoc/visit_ast.rs @@ -331,7 +331,7 @@ impl<'a, 'tcx> RustdocVisitor<'a, 'tcx> { item: &'tcx hir::Item<'_>, renamed: Option, import_id: Option, - ) -> bool { + ) { debug!("visiting item {:?}", item); let name = renamed.unwrap_or(item.ident.name); let tcx = self.cx.tcx; @@ -448,7 +448,6 @@ impl<'a, 'tcx> RustdocVisitor<'a, 'tcx> { } } } - true } fn visit_foreign_item_inner( @@ -485,9 +484,8 @@ impl<'a, 'tcx> Visitor<'tcx> for RustdocVisitor<'a, 'tcx> { } fn visit_item(&mut self, i: &'tcx hir::Item<'tcx>) { - if self.visit_item_inner(i, None, None) { - walk_item(self, i); - } + self.visit_item_inner(i, None, None); + walk_item(self, i); } fn visit_mod(&mut self, _: &hir::Mod<'tcx>, _: Span, _: hir::HirId) { From c3c9f8f5f8cb96e0536bc8d4e76e66c5e7caba56 Mon Sep 17 00:00:00 2001 From: Guillaume Gomez Date: Mon, 17 Apr 2023 14:34:50 +0200 Subject: [PATCH 2/3] Fix invalid handling of nested items with `--document-private-items` --- src/librustdoc/visit_ast.rs | 40 ++++++++++++++++++++++++++++++++----- 1 file changed, 35 insertions(+), 5 deletions(-) diff --git a/src/librustdoc/visit_ast.rs b/src/librustdoc/visit_ast.rs index 5ac68da150a9a..071a85f4da6ea 100644 --- a/src/librustdoc/visit_ast.rs +++ b/src/librustdoc/visit_ast.rs @@ -10,6 +10,7 @@ use rustc_hir::{Node, CRATE_HIR_ID}; use rustc_middle::hir::nested_filter; use rustc_middle::ty::TyCtxt; use rustc_span::def_id::{CRATE_DEF_ID, LOCAL_CRATE}; +use rustc_span::hygiene::MacroKind; use rustc_span::symbol::{kw, sym, Symbol}; use rustc_span::Span; @@ -87,6 +88,7 @@ pub(crate) struct RustdocVisitor<'a, 'tcx> { inside_public_path: bool, exact_paths: DefIdMap>, modules: Vec>, + is_importable_from_parent: bool, } impl<'a, 'tcx> RustdocVisitor<'a, 'tcx> { @@ -107,6 +109,7 @@ impl<'a, 'tcx> RustdocVisitor<'a, 'tcx> { inside_public_path: true, exact_paths: Default::default(), modules: vec![om], + is_importable_from_parent: true, } } @@ -319,11 +322,23 @@ impl<'a, 'tcx> RustdocVisitor<'a, 'tcx> { renamed: Option, parent_id: Option, ) { - self.modules - .last_mut() - .unwrap() - .items - .insert((item.owner_id.def_id, renamed), (item, renamed, parent_id)); + if self.is_importable_from_parent + // If we're inside an item, only impl blocks and `macro_rules!` with the `macro_export` + // attribute can still be visible. + || match item.kind { + hir::ItemKind::Impl(..) => true, + hir::ItemKind::Macro(_, MacroKind::Bang) => { + self.cx.tcx.has_attr(item.owner_id.def_id, sym::macro_export) + } + _ => false, + } + { + self.modules + .last_mut() + .unwrap() + .items + .insert((item.owner_id.def_id, renamed), (item, renamed, parent_id)); + } } fn visit_item_inner( @@ -485,7 +500,22 @@ impl<'a, 'tcx> Visitor<'tcx> for RustdocVisitor<'a, 'tcx> { fn visit_item(&mut self, i: &'tcx hir::Item<'tcx>) { self.visit_item_inner(i, None, None); + let new_value = if self.is_importable_from_parent { + matches!( + i.kind, + hir::ItemKind::Mod(..) + | hir::ItemKind::ForeignMod { .. } + | hir::ItemKind::Impl(..) + | hir::ItemKind::Trait(..) + ) + } else { + // Whatever the context, if it's an impl block, the items inside it can be used so they + // should be visible. + matches!(i.kind, hir::ItemKind::Impl(..)) + }; + let prev = mem::replace(&mut self.is_importable_from_parent, new_value); walk_item(self, i); + self.is_importable_from_parent = prev; } fn visit_mod(&mut self, _: &hir::Mod<'tcx>, _: Span, _: hir::HirId) { From c456e15855b538aecff7b088dda6144dcde9d6ca Mon Sep 17 00:00:00 2001 From: Guillaume Gomez Date: Mon, 17 Apr 2023 14:35:33 +0200 Subject: [PATCH 3/3] Add regression tests for #110422 --- .../infinite-recursive-type-impl-trait.rs | 5 +- .../infinite-recursive-type-impl-trait.stderr | 16 ----- tests/rustdoc/issue-110422-inner-private.rs | 64 +++++++++++++++++++ 3 files changed, 68 insertions(+), 17 deletions(-) delete mode 100644 tests/rustdoc-ui/infinite-recursive-type-impl-trait.stderr create mode 100644 tests/rustdoc/issue-110422-inner-private.rs diff --git a/tests/rustdoc-ui/infinite-recursive-type-impl-trait.rs b/tests/rustdoc-ui/infinite-recursive-type-impl-trait.rs index ac51725749867..096130d776828 100644 --- a/tests/rustdoc-ui/infinite-recursive-type-impl-trait.rs +++ b/tests/rustdoc-ui/infinite-recursive-type-impl-trait.rs @@ -1,6 +1,9 @@ +// check-pass + fn f() -> impl Sized { - enum E { //~ ERROR + enum E { V(E), } + unimplemented!() } diff --git a/tests/rustdoc-ui/infinite-recursive-type-impl-trait.stderr b/tests/rustdoc-ui/infinite-recursive-type-impl-trait.stderr deleted file mode 100644 index a61577bd14afc..0000000000000 --- a/tests/rustdoc-ui/infinite-recursive-type-impl-trait.stderr +++ /dev/null @@ -1,16 +0,0 @@ -error[E0072]: recursive type `f::E` has infinite size - --> $DIR/infinite-recursive-type-impl-trait.rs:2:5 - | -LL | enum E { - | ^^^^^^ -LL | V(E), - | - recursive without indirection - | -help: insert some indirection (e.g., a `Box`, `Rc`, or `&`) to break the cycle - | -LL | V(Box), - | ++++ + - -error: aborting due to previous error - -For more information about this error, try `rustc --explain E0072`. diff --git a/tests/rustdoc/issue-110422-inner-private.rs b/tests/rustdoc/issue-110422-inner-private.rs new file mode 100644 index 0000000000000..ee8ed5cc6e17f --- /dev/null +++ b/tests/rustdoc/issue-110422-inner-private.rs @@ -0,0 +1,64 @@ +// Regression test for . +// This test ensures that inner items (except for implementations and macros) +// don't appear in documentation. + +// compile-flags: --document-private-items + +#![crate_name = "foo"] + +// @has 'foo/index.html' +// Checking there is no "trait" entry. +// @count - '//*[@id="main-content"]/*[@class="small-section-header"]' 4 +// @has - '//*[@id="main-content"]/*[@class="small-section-header"]' 'Structs' +// @has - '//*[@id="main-content"]/*[@class="small-section-header"]' 'Constants' +// @has - '//*[@id="main-content"]/*[@class="small-section-header"]' 'Functions' +// @has - '//*[@id="main-content"]/*[@class="small-section-header"]' 'Macros' + +// @has - '//a[@href="fn.foo.html"]' 'foo' +fn foo() { + fn bar() {} + + // @has - '//a[@class="macro"]' 'visible_macro' + // @!has - '//a[@class="macro"]' 'non_visible_macro' + // @has 'foo/macro.visible_macro.html' + // @!has 'foo/macro.non_visible_macro.html' + #[macro_export] + macro_rules! visible_macro { + () => {} + } + + macro_rules! non_visible_macro { + () => {} + } +} + +// @has 'foo/index.html' +// @has - '//a[@href="struct.Bar.html"]' 'Bar' +struct Bar; + +const BAR: i32 = { + // @!has - '//a[@href="fn.yo.html"]' 'yo' + // @!has 'foo/fn.yo.html' + fn yo() {} + + // @!has 'foo/index.html' '//a[@href="trait.Foo.html"]' 'Foo' + // @!has 'foo/trait.Foo.html' + trait Foo { + fn babar() {} + } + impl Foo for Bar {} + + // @has 'foo/struct.Bar.html' + // @has - '//*[@id="method.foo"]/*[@class="code-header"]' 'pub(crate) fn foo()' + // @count - '//*[@id="main-content"]/*[@class="small-section-header"]' 3 + // We now check that the `Foo` trait is not documented nor visible on `Bar` page. + // @has - '//*[@id="main-content"]/*[@class="small-section-header"]' 'Implementations' + // @has - '//*[@id="main-content"]/*[@class="small-section-header"]' 'Auto Trait Implementations' + // @has - '//*[@id="main-content"]/*[@class="small-section-header"]' 'Blanket Implementations' + // @!has - '//*[@href="trait.Foo.html#method.babar"]/*[@class="code-header"]' 'fn babar()' + impl Bar { + fn foo() {} + } + + 1 +};