From 5feeca01563155c2836342e028f81dab51289dc0 Mon Sep 17 00:00:00 2001 From: Guillaume Gomez <guillaume.gomez@huawei.com> Date: Thu, 19 Jan 2023 20:25:46 +0100 Subject: [PATCH 1/6] Revert "Improve code" This reverts commit a954d6334d000225ae38f65f5f9e9c182e6764ae. --- src/librustdoc/visit_ast.rs | 183 ++++++++++++++++++------------------ 1 file changed, 93 insertions(+), 90 deletions(-) diff --git a/src/librustdoc/visit_ast.rs b/src/librustdoc/visit_ast.rs index 7db47035967..bcbdafd4b00 100644 --- a/src/librustdoc/visit_ast.rs +++ b/src/librustdoc/visit_ast.rs @@ -8,6 +8,7 @@ use rustc_hir::def_id::DefId; use rustc_hir::intravisit::{walk_item, Visitor}; use rustc_hir::Node; use rustc_hir::CRATE_HIR_ID; +use rustc_middle::hir::map::Map; use rustc_middle::hir::nested_filter; use rustc_middle::ty::TyCtxt; use rustc_span::def_id::{CRATE_DEF_ID, LOCAL_CRATE}; @@ -67,6 +68,7 @@ pub(crate) struct RustdocVisitor<'a, 'tcx> { inside_public_path: bool, exact_paths: FxHashMap<DefId, Vec<Symbol>>, modules: Vec<Module<'tcx>>, + map: Map<'tcx>, } impl<'a, 'tcx> RustdocVisitor<'a, 'tcx> { @@ -79,6 +81,7 @@ impl<'a, 'tcx> RustdocVisitor<'a, 'tcx> { hir::CRATE_HIR_ID, cx.tcx.hir().root_module().spans.inner_span, ); + let map = cx.tcx.hir(); RustdocVisitor { cx, @@ -87,6 +90,7 @@ impl<'a, 'tcx> RustdocVisitor<'a, 'tcx> { inside_public_path: true, exact_paths: FxHashMap::default(), modules: vec![om], + map, } } @@ -95,95 +99,6 @@ impl<'a, 'tcx> RustdocVisitor<'a, 'tcx> { self.exact_paths.entry(did).or_insert_with(|| def_id_to_path(tcx, did)); } - pub(crate) fn visit(mut self) -> Module<'tcx> { - let root_module = self.cx.tcx.hir().root_module(); - self.visit_mod_contents(CRATE_HIR_ID, root_module); - - let mut top_level_module = self.modules.pop().unwrap(); - - // `#[macro_export] macro_rules!` items are reexported at the top level of the - // crate, regardless of where they're defined. We want to document the - // top level rexport of the macro, not its original definition, since - // the rexport defines the path that a user will actually see. Accordingly, - // we add the rexport as an item here, and then skip over the original - // definition in `visit_item()` below. - // - // We also skip `#[macro_export] macro_rules!` that have already been inserted, - // it can happen if within the same module a `#[macro_export] macro_rules!` - // is declared but also a reexport of itself producing two exports of the same - // macro in the same module. - let mut inserted = FxHashSet::default(); - for export in self.cx.tcx.module_reexports(CRATE_DEF_ID).unwrap_or(&[]) { - if let Res::Def(DefKind::Macro(_), def_id) = export.res && - let Some(local_def_id) = def_id.as_local() && - self.cx.tcx.has_attr(def_id, sym::macro_export) && - inserted.insert(def_id) - { - let item = self.cx.tcx.hir().expect_item(local_def_id); - top_level_module.items.push((item, None, None)); - } - } - - self.cx.cache.hidden_cfg = self - .cx - .tcx - .hir() - .attrs(CRATE_HIR_ID) - .iter() - .filter(|attr| attr.has_name(sym::doc)) - .flat_map(|attr| attr.meta_item_list().into_iter().flatten()) - .filter(|attr| attr.has_name(sym::cfg_hide)) - .flat_map(|attr| { - attr.meta_item_list() - .unwrap_or(&[]) - .iter() - .filter_map(|attr| { - Cfg::parse(attr.meta_item()?) - .map_err(|e| self.cx.sess().diagnostic().span_err(e.span, e.msg)) - .ok() - }) - .collect::<Vec<_>>() - }) - .chain( - [Cfg::Cfg(sym::test, None), Cfg::Cfg(sym::doc, None), Cfg::Cfg(sym::doctest, None)] - .into_iter(), - ) - .collect(); - - self.cx.cache.exact_paths = self.exact_paths; - top_level_module - } - - /// This method will go through the given module items in two passes: - /// 1. The items which are not glob imports/reexports. - /// 2. The glob imports/reexports. - fn visit_mod_contents(&mut self, id: hir::HirId, m: &'tcx hir::Mod<'tcx>) { - debug!("Going through module {:?}", m); - let def_id = self.cx.tcx.hir().local_def_id(id).to_def_id(); - // Keep track of if there were any private modules in the path. - let orig_inside_public_path = self.inside_public_path; - self.inside_public_path &= self.cx.tcx.visibility(def_id).is_public(); - - // Reimplementation of `walk_mod` because we need to do it in two passes (explanations in - // the second loop): - for &i in m.item_ids { - let item = self.cx.tcx.hir().item(i); - if !matches!(item.kind, hir::ItemKind::Use(_, hir::UseKind::Glob)) { - self.visit_item(item); - } - } - for &i in m.item_ids { - let item = self.cx.tcx.hir().item(i); - // To match the way import precedence works, visit glob imports last. - // Later passes in rustdoc will de-duplicate by name and kind, so if glob- - // imported items appear last, then they'll be the ones that get discarded. - if matches!(item.kind, hir::ItemKind::Use(_, hir::UseKind::Glob)) { - self.visit_item(item); - } - } - self.inside_public_path = orig_inside_public_path; - } - /// Tries to resolve the target of a `pub use` statement and inlines the /// target if it is defined locally and would not be documented otherwise, /// or when it is specifically requested with `please_inline`. @@ -408,6 +323,65 @@ impl<'a, 'tcx> RustdocVisitor<'a, 'tcx> { } } + pub(crate) fn visit(mut self) -> Module<'tcx> { + let root_module = self.cx.tcx.hir().root_module(); + self.visit_mod_contents(CRATE_HIR_ID, root_module); + + let mut top_level_module = self.modules.pop().unwrap(); + + // `#[macro_export] macro_rules!` items are reexported at the top level of the + // crate, regardless of where they're defined. We want to document the + // top level rexport of the macro, not its original definition, since + // the rexport defines the path that a user will actually see. Accordingly, + // we add the rexport as an item here, and then skip over the original + // definition in `visit_item()` below. + // + // We also skip `#[macro_export] macro_rules!` that have already been inserted, + // it can happen if within the same module a `#[macro_export] macro_rules!` + // is declared but also a reexport of itself producing two exports of the same + // macro in the same module. + let mut inserted = FxHashSet::default(); + for export in self.cx.tcx.module_reexports(CRATE_DEF_ID).unwrap_or(&[]) { + if let Res::Def(DefKind::Macro(_), def_id) = export.res && + let Some(local_def_id) = def_id.as_local() && + self.cx.tcx.has_attr(def_id, sym::macro_export) && + inserted.insert(def_id) + { + let item = self.cx.tcx.hir().expect_item(local_def_id); + top_level_module.items.push((item, None, None)); + } + } + + self.cx.cache.hidden_cfg = self + .cx + .tcx + .hir() + .attrs(CRATE_HIR_ID) + .iter() + .filter(|attr| attr.has_name(sym::doc)) + .flat_map(|attr| attr.meta_item_list().into_iter().flatten()) + .filter(|attr| attr.has_name(sym::cfg_hide)) + .flat_map(|attr| { + attr.meta_item_list() + .unwrap_or(&[]) + .iter() + .filter_map(|attr| { + Cfg::parse(attr.meta_item()?) + .map_err(|e| self.cx.sess().diagnostic().span_err(e.span, e.msg)) + .ok() + }) + .collect::<Vec<_>>() + }) + .chain( + [Cfg::Cfg(sym::test, None), Cfg::Cfg(sym::doc, None), Cfg::Cfg(sym::doctest, None)] + .into_iter(), + ) + .collect(); + + self.cx.cache.exact_paths = self.exact_paths; + top_level_module + } + /// This method will create a new module and push it onto the "modules stack" then call /// `visit_mod_contents`. Once done, it'll remove it from the "modules stack" and instead /// add into the list of modules of the current module. @@ -419,6 +393,35 @@ impl<'a, 'tcx> RustdocVisitor<'a, 'tcx> { let last = self.modules.pop().unwrap(); self.modules.last_mut().unwrap().mods.push(last); } + + /// This method will go through the given module items in two passes: + /// 1. The items which are not glob imports/reexports. + /// 2. The glob imports/reexports. + fn visit_mod_contents(&mut self, id: hir::HirId, m: &'tcx hir::Mod<'tcx>) { + debug!("Going through module {:?}", m); + let def_id = self.cx.tcx.hir().local_def_id(id).to_def_id(); + // Keep track of if there were any private modules in the path. + let orig_inside_public_path = self.inside_public_path; + self.inside_public_path &= self.cx.tcx.visibility(def_id).is_public(); + + // Reimplementation of `walk_mod`: + for &i in m.item_ids { + let item = self.cx.tcx.hir().item(i); + if !matches!(item.kind, hir::ItemKind::Use(_, hir::UseKind::Glob)) { + self.visit_item(item); + } + } + for &i in m.item_ids { + let item = self.cx.tcx.hir().item(i); + // To match the way import precedence works, visit glob imports last. + // Later passes in rustdoc will de-duplicate by name and kind, so if glob- + // imported items appear last, then they'll be the ones that get discarded. + if matches!(item.kind, hir::ItemKind::Use(_, hir::UseKind::Glob)) { + self.visit_item(item); + } + } + self.inside_public_path = orig_inside_public_path; + } } // We need to implement this visitor so it'll go everywhere and retrieve items we're interested in @@ -427,7 +430,7 @@ impl<'a, 'tcx> Visitor<'tcx> for RustdocVisitor<'a, 'tcx> { type NestedFilter = nested_filter::All; fn nested_visit_map(&mut self) -> Self::Map { - self.cx.tcx.hir() + self.map } fn visit_item(&mut self, i: &'tcx hir::Item<'tcx>) { From 286a632e725826bb13f6710f188f4ed1af0d50cf Mon Sep 17 00:00:00 2001 From: Guillaume Gomez <guillaume.gomez@huawei.com> Date: Thu, 19 Jan 2023 20:25:53 +0100 Subject: [PATCH 2/6] Revert "Speed up execution a bit by removing some walks" This reverts commit a9d582f51f547583380f2f2894ae0c799b609a86. --- src/librustdoc/visit_ast.rs | 23 ++--------------------- 1 file changed, 2 insertions(+), 21 deletions(-) diff --git a/src/librustdoc/visit_ast.rs b/src/librustdoc/visit_ast.rs index bcbdafd4b00..c5aaae7a908 100644 --- a/src/librustdoc/visit_ast.rs +++ b/src/librustdoc/visit_ast.rs @@ -250,6 +250,7 @@ impl<'a, 'tcx> RustdocVisitor<'a, 'tcx> { res, ident, is_glob, + om, please_inline, ) { continue; @@ -445,26 +446,6 @@ impl<'a, 'tcx> Visitor<'tcx> for RustdocVisitor<'a, 'tcx> { } fn visit_mod(&mut self, _: &hir::Mod<'tcx>, _: Span, _: hir::HirId) { - // Handled in `visit_item_inner` - } - - fn visit_use(&mut self, _: &hir::UsePath<'tcx>, _: hir::HirId) { - // Handled in `visit_item_inner` - } - - fn visit_path(&mut self, _: &hir::Path<'tcx>, _: hir::HirId) { - // Handled in `visit_item_inner` - } - - fn visit_label(&mut self, _: &rustc_ast::Label) { - // Unneeded. - } - - fn visit_infer(&mut self, _: &hir::InferArg) { - // Unneeded. - } - - fn visit_lifetime(&mut self, _: &hir::Lifetime) { - // Unneeded. + // handled in `visit_item_inner` } } From 8b80bc1bf4719270a9b4bf8f4cc682bb18a574c7 Mon Sep 17 00:00:00 2001 From: Guillaume Gomez <guillaume.gomez@huawei.com> Date: Thu, 19 Jan 2023 20:26:01 +0100 Subject: [PATCH 3/6] Revert "Improve code readability" This reverts commit eb93d1bedeab64c6f5d661df6a309a5b8a9273ca. --- src/librustdoc/html/render/write_shared.rs | 2 +- src/librustdoc/visit_ast.rs | 36 +++++++++------------- 2 files changed, 15 insertions(+), 23 deletions(-) diff --git a/src/librustdoc/html/render/write_shared.rs b/src/librustdoc/html/render/write_shared.rs index ca3e9916487..bc8badad38e 100644 --- a/src/librustdoc/html/render/write_shared.rs +++ b/src/librustdoc/html/render/write_shared.rs @@ -138,7 +138,7 @@ pub(super) fn write_shared( Ok((ret, krates)) } - /// Read a file and return all lines that match the <code>"{crate}":{data},\ </code> format, + /// Read a file and return all lines that match the <code>"{crate}":{data},\</code> format, /// and return a tuple `(Vec<DataString>, Vec<CrateNameString>)`. /// /// This forms the payload of files that look like this: diff --git a/src/librustdoc/visit_ast.rs b/src/librustdoc/visit_ast.rs index c5aaae7a908..a8fc85a3ee1 100644 --- a/src/librustdoc/visit_ast.rs +++ b/src/librustdoc/visit_ast.rs @@ -187,16 +187,6 @@ impl<'a, 'tcx> RustdocVisitor<'a, 'tcx> { ret } - #[inline] - fn add_to_current_mod( - &mut self, - item: &'tcx hir::Item<'_>, - renamed: Option<Symbol>, - parent_id: Option<hir::HirId>, - ) { - self.modules.last_mut().unwrap().items.push((item, renamed, parent_id)) - } - fn visit_item_inner( &mut self, item: &'tcx hir::Item<'_>, @@ -257,7 +247,7 @@ impl<'a, 'tcx> RustdocVisitor<'a, 'tcx> { } } - self.add_to_current_mod(item, renamed, parent_id); + self.modules.last_mut().unwrap().items.push((item, renamed, parent_id)); } } hir::ItemKind::Macro(ref macro_def, _) => { @@ -277,7 +267,7 @@ impl<'a, 'tcx> RustdocVisitor<'a, 'tcx> { let nonexported = !self.cx.tcx.has_attr(def_id, sym::macro_export); if is_macro_2_0 || nonexported || self.inlining { - self.add_to_current_mod(item, renamed, None); + self.modules.last_mut().unwrap().items.push((item, renamed, None)); } } hir::ItemKind::Mod(ref m) => { @@ -293,20 +283,20 @@ impl<'a, 'tcx> RustdocVisitor<'a, 'tcx> { | hir::ItemKind::Static(..) | hir::ItemKind::Trait(..) | hir::ItemKind::TraitAlias(..) => { - self.add_to_current_mod(item, renamed, parent_id); + self.modules.last_mut().unwrap().items.push((item, renamed, parent_id)) } hir::ItemKind::Const(..) => { // Underscore constants do not correspond to a nameable item and // so are never useful in documentation. if name != kw::Underscore { - self.add_to_current_mod(item, renamed, parent_id); + self.modules.last_mut().unwrap().items.push((item, renamed, parent_id)); } } hir::ItemKind::Impl(impl_) => { // Don't duplicate impls when inlining or if it's implementing a trait, we'll pick // them up regardless of where they're located. if !self.inlining && impl_.of_trait.is_none() { - self.add_to_current_mod(item, None, None); + self.modules.last_mut().unwrap().items.push((item, None, None)); } } } @@ -343,13 +333,15 @@ impl<'a, 'tcx> RustdocVisitor<'a, 'tcx> { // macro in the same module. let mut inserted = FxHashSet::default(); for export in self.cx.tcx.module_reexports(CRATE_DEF_ID).unwrap_or(&[]) { - if let Res::Def(DefKind::Macro(_), def_id) = export.res && - let Some(local_def_id) = def_id.as_local() && - self.cx.tcx.has_attr(def_id, sym::macro_export) && - inserted.insert(def_id) - { - let item = self.cx.tcx.hir().expect_item(local_def_id); - top_level_module.items.push((item, None, None)); + if let Res::Def(DefKind::Macro(_), def_id) = export.res { + if let Some(local_def_id) = def_id.as_local() { + if self.cx.tcx.has_attr(def_id, sym::macro_export) { + if inserted.insert(def_id) { + let item = self.cx.tcx.hir().expect_item(local_def_id); + top_level_module.items.push((item, None, None)); + } + } + } } } From 69de8fbbebaede4b8cb89fb75d1c7a14bd4a2642 Mon Sep 17 00:00:00 2001 From: Guillaume Gomez <guillaume.gomez@huawei.com> Date: Thu, 19 Jan 2023 20:27:00 +0100 Subject: [PATCH 4/6] Revert "Update newly failing UI tests" This reverts commit 9c46173895430c63066731440e00faf0ab2195dd. --- .../infinite-recursive-type-impl-trait-return.rs | 4 +++- ...inite-recursive-type-impl-trait-return.stderr | 16 ---------------- .../infinite-recursive-type-impl-trait.rs | 5 ++++- .../infinite-recursive-type-impl-trait.stderr | 16 ---------------- 4 files changed, 7 insertions(+), 34 deletions(-) delete mode 100644 tests/rustdoc-ui/infinite-recursive-type-impl-trait-return.stderr delete mode 100644 tests/rustdoc-ui/infinite-recursive-type-impl-trait.stderr diff --git a/tests/rustdoc-ui/infinite-recursive-type-impl-trait-return.rs b/tests/rustdoc-ui/infinite-recursive-type-impl-trait-return.rs index 939da186fbc..4b1e04234c8 100644 --- a/tests/rustdoc-ui/infinite-recursive-type-impl-trait-return.rs +++ b/tests/rustdoc-ui/infinite-recursive-type-impl-trait-return.rs @@ -1,10 +1,12 @@ +// check-pass // normalize-stderr-test: "`.*`" -> "`DEF_ID`" // normalize-stdout-test: "`.*`" -> "`DEF_ID`" // edition:2018 pub async fn f() -> impl std::fmt::Debug { + // rustdoc doesn't care that this is infinitely sized #[derive(Debug)] - enum E { //~ ERROR + enum E { This(E), Unit, } diff --git a/tests/rustdoc-ui/infinite-recursive-type-impl-trait-return.stderr b/tests/rustdoc-ui/infinite-recursive-type-impl-trait-return.stderr deleted file mode 100644 index aff7402bc91..00000000000 --- a/tests/rustdoc-ui/infinite-recursive-type-impl-trait-return.stderr +++ /dev/null @@ -1,16 +0,0 @@ -error[E0072]: recursive type `DEF_ID` has infinite size - --> $DIR/infinite-recursive-type-impl-trait-return.rs:7:5 - | -LL | enum E { - | ^^^^^^ -LL | This(E), - | - recursive without indirection - | -help: insert some indirection (e.g., a `DEF_ID`) to break the cycle - | -LL | This(Box<E>), - | ++++ + - -error: aborting due to previous error - -For more information about this error, try `DEF_ID`. diff --git a/tests/rustdoc-ui/infinite-recursive-type-impl-trait.rs b/tests/rustdoc-ui/infinite-recursive-type-impl-trait.rs index ac517257498..ac79582fb3f 100644 --- a/tests/rustdoc-ui/infinite-recursive-type-impl-trait.rs +++ b/tests/rustdoc-ui/infinite-recursive-type-impl-trait.rs @@ -1,5 +1,8 @@ +// check-pass + fn f() -> impl Sized { - enum E { //~ ERROR + // rustdoc doesn't care that this is infinitely sized + 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 a61577bd14a..00000000000 --- 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<E>), - | ++++ + - -error: aborting due to previous error - -For more information about this error, try `rustc --explain E0072`. From 13239a9b8e75f871e004a8167f67c7ed399dde01 Mon Sep 17 00:00:00 2001 From: Guillaume Gomez <guillaume.gomez@huawei.com> Date: Thu, 19 Jan 2023 20:27:37 +0100 Subject: [PATCH 5/6] Revert "Add regression test for impl blocks in const expr" This reverts commit 9cce0bc583ee2cff88935ce0e08d8ec1eb1239a8. --- tests/rustdoc/impl-in-const-block.rs | 43 ---------------------------- 1 file changed, 43 deletions(-) delete mode 100644 tests/rustdoc/impl-in-const-block.rs diff --git a/tests/rustdoc/impl-in-const-block.rs b/tests/rustdoc/impl-in-const-block.rs deleted file mode 100644 index b44e7135246..00000000000 --- a/tests/rustdoc/impl-in-const-block.rs +++ /dev/null @@ -1,43 +0,0 @@ -// Regression test for #83026. -// The goal of this test is to ensure that impl blocks inside -// const expressions are documented as well. - -#![crate_name = "foo"] - -// @has 'foo/struct.A.html' -// @has - '//*[@id="method.new"]/*[@class="code-header"]' 'pub fn new() -> A' -// @has - '//*[@id="method.bar"]/*[@class="code-header"]' 'pub fn bar(&self)' -// @has - '//*[@id="method.woo"]/*[@class="code-header"]' 'pub fn woo(&self)' -// @has - '//*[@id="method.yoo"]/*[@class="code-header"]' 'pub fn yoo()' -// @has - '//*[@id="method.yuu"]/*[@class="code-header"]' 'pub fn yuu()' -pub struct A; - -const _: () = { - impl A { - const FOO: () = { - impl A { - pub fn woo(&self) {} - } - }; - - pub fn new() -> A { - A - } - } -}; -pub const X: () = { - impl A { - pub fn bar(&self) {} - } -}; - -fn foo() { - impl A { - pub fn yoo() {} - } - const _: () = { - impl A { - pub fn yuu() {} - } - }; -} From a22cd9cdfad00d28b7116a8ddae91f7fada7473f Mon Sep 17 00:00:00 2001 From: Guillaume Gomez <guillaume.gomez@huawei.com> Date: Thu, 19 Jan 2023 20:28:17 +0100 Subject: [PATCH 6/6] Revert "Fix missing const expression items visit" This reverts commit cdfc5051b1286938f56160243c28538f79dce6b1. --- src/librustdoc/visit_ast.rs | 505 +++++++++++++++++------------------- 1 file changed, 231 insertions(+), 274 deletions(-) diff --git a/src/librustdoc/visit_ast.rs b/src/librustdoc/visit_ast.rs index a8fc85a3ee1..22068ebe041 100644 --- a/src/librustdoc/visit_ast.rs +++ b/src/librustdoc/visit_ast.rs @@ -5,11 +5,8 @@ use rustc_data_structures::fx::{FxHashMap, FxHashSet}; use rustc_hir as hir; use rustc_hir::def::{DefKind, Res}; use rustc_hir::def_id::DefId; -use rustc_hir::intravisit::{walk_item, Visitor}; use rustc_hir::Node; use rustc_hir::CRATE_HIR_ID; -use rustc_middle::hir::map::Map; -use rustc_middle::hir::nested_filter; use rustc_middle::ty::TyCtxt; use rustc_span::def_id::{CRATE_DEF_ID, LOCAL_CRATE}; use rustc_span::symbol::{kw, sym, Symbol}; @@ -60,6 +57,9 @@ pub(crate) fn inherits_doc_hidden(tcx: TyCtxt<'_>, mut node: hir::HirId) -> bool false } +// Also, is there some reason that this doesn't use the 'visit' +// framework from syntax?. + pub(crate) struct RustdocVisitor<'a, 'tcx> { cx: &'a mut core::DocContext<'tcx>, view_item_stack: FxHashSet<hir::HirId>, @@ -67,8 +67,6 @@ pub(crate) struct RustdocVisitor<'a, 'tcx> { /// Are the current module and all of its parents public? inside_public_path: bool, exact_paths: FxHashMap<DefId, Vec<Symbol>>, - modules: Vec<Module<'tcx>>, - map: Map<'tcx>, } impl<'a, 'tcx> RustdocVisitor<'a, 'tcx> { @@ -76,21 +74,12 @@ impl<'a, 'tcx> RustdocVisitor<'a, 'tcx> { // If the root is re-exported, terminate all recursion. let mut stack = FxHashSet::default(); stack.insert(hir::CRATE_HIR_ID); - let om = Module::new( - cx.tcx.crate_name(LOCAL_CRATE), - hir::CRATE_HIR_ID, - cx.tcx.hir().root_module().spans.inner_span, - ); - let map = cx.tcx.hir(); - RustdocVisitor { cx, view_item_stack: stack, inlining: false, inside_public_path: true, exact_paths: FxHashMap::default(), - modules: vec![om], - map, } } @@ -99,226 +88,13 @@ impl<'a, 'tcx> RustdocVisitor<'a, 'tcx> { self.exact_paths.entry(did).or_insert_with(|| def_id_to_path(tcx, did)); } - /// Tries to resolve the target of a `pub use` statement and inlines the - /// target if it is defined locally and would not be documented otherwise, - /// or when it is specifically requested with `please_inline`. - /// (the latter is the case when the import is marked `doc(inline)`) - /// - /// Cross-crate inlining occurs later on during crate cleaning - /// and follows different rules. - /// - /// Returns `true` if the target has been inlined. - fn maybe_inline_local( - &mut self, - id: hir::HirId, - res: Res, - renamed: Option<Symbol>, - glob: bool, - please_inline: bool, - ) -> bool { - debug!("maybe_inline_local res: {:?}", res); - - if self.cx.output_format.is_json() { - return false; - } - - let tcx = self.cx.tcx; - let Some(res_did) = res.opt_def_id() else { - return false; - }; - - let use_attrs = tcx.hir().attrs(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); - - // For cross-crate impl inlining we need to know whether items are - // reachable in documentation -- a previously unreachable item can be - // made reachable by cross-crate inlining which we're checking here. - // (this is done here because we need to know this upfront). - if !res_did.is_local() && !is_no_inline { - crate::visit_lib::lib_embargo_visit_item(self.cx, res_did); - return false; - } - - let res_hir_id = match res_did.as_local() { - Some(n) => tcx.hir().local_def_id_to_hir_id(n), - None => return false, - }; - - let is_private = - !self.cx.cache.effective_visibilities.is_directly_public(self.cx.tcx, res_did); - let is_hidden = inherits_doc_hidden(self.cx.tcx, res_hir_id); - - // Only inline if requested or if the item would otherwise be stripped. - if (!please_inline && !is_private && !is_hidden) || is_no_inline { - return false; - } - - if !self.view_item_stack.insert(res_hir_id) { - return false; - } - - let ret = match tcx.hir().get(res_hir_id) { - Node::Item(&hir::Item { kind: hir::ItemKind::Mod(ref m), .. }) if glob => { - let prev = mem::replace(&mut self.inlining, true); - for &i in m.item_ids { - let i = self.cx.tcx.hir().item(i); - self.visit_item_inner(i, None, Some(id)); - } - self.inlining = prev; - true - } - Node::Item(it) if !glob => { - let prev = mem::replace(&mut self.inlining, true); - self.visit_item_inner(it, renamed, Some(id)); - self.inlining = prev; - true - } - Node::ForeignItem(it) if !glob => { - let prev = mem::replace(&mut self.inlining, true); - self.visit_foreign_item_inner(it, renamed); - self.inlining = prev; - true - } - _ => false, - }; - self.view_item_stack.remove(&res_hir_id); - ret - } - - fn visit_item_inner( - &mut self, - item: &'tcx hir::Item<'_>, - renamed: Option<Symbol>, - parent_id: Option<hir::HirId>, - ) -> bool { - debug!("visiting item {:?}", item); - let name = renamed.unwrap_or(item.ident.name); - - let def_id = item.owner_id.to_def_id(); - let is_pub = self.cx.tcx.visibility(def_id).is_public(); - - if is_pub { - self.store_path(item.owner_id.to_def_id()); - } - - match item.kind { - hir::ItemKind::ForeignMod { items, .. } => { - for item in items { - let item = self.cx.tcx.hir().foreign_item(item.id); - self.visit_foreign_item_inner(item, None); - } - } - // If we're inlining, skip private items or item reexported as "_". - _ if self.inlining && (!is_pub || renamed == Some(kw::Underscore)) => {} - hir::ItemKind::GlobalAsm(..) => {} - hir::ItemKind::Use(_, hir::UseKind::ListStem) => {} - hir::ItemKind::Use(path, kind) => { - for &res in &path.res { - // Struct and variant constructors and proc macro stubs always show up alongside - // their definitions, we've already processed them so just discard these. - if let Res::Def(DefKind::Ctor(..), _) | Res::SelfCtor(..) = res { - continue; - } - - let attrs = self.cx.tcx.hir().attrs(item.hir_id()); - - // If there was a private module in the current path then don't bother inlining - // anything as it will probably be stripped anyway. - if is_pub && self.inside_public_path { - let please_inline = attrs.iter().any(|item| match item.meta_item_list() { - Some(ref list) if item.has_name(sym::doc) => { - list.iter().any(|i| i.has_name(sym::inline)) - } - _ => false, - }); - let is_glob = kind == hir::UseKind::Glob; - let ident = if is_glob { None } else { Some(name) }; - if self.maybe_inline_local( - item.hir_id(), - res, - ident, - is_glob, - om, - please_inline, - ) { - continue; - } - } - - self.modules.last_mut().unwrap().items.push((item, renamed, parent_id)); - } - } - hir::ItemKind::Macro(ref macro_def, _) => { - // `#[macro_export] macro_rules!` items are handled separately in `visit()`, - // above, since they need to be documented at the module top level. Accordingly, - // we only want to handle macros if one of three conditions holds: - // - // 1. This macro was defined by `macro`, and thus isn't covered by the case - // above. - // 2. This macro isn't marked with `#[macro_export]`, and thus isn't covered - // by the case above. - // 3. We're inlining, since a reexport where inlining has been requested - // should be inlined even if it is also documented at the top level. - - let def_id = item.owner_id.to_def_id(); - let is_macro_2_0 = !macro_def.macro_rules; - let nonexported = !self.cx.tcx.has_attr(def_id, sym::macro_export); - - if is_macro_2_0 || nonexported || self.inlining { - self.modules.last_mut().unwrap().items.push((item, renamed, None)); - } - } - hir::ItemKind::Mod(ref m) => { - self.enter_mod(item.hir_id(), m, name); - } - hir::ItemKind::Fn(..) - | hir::ItemKind::ExternCrate(..) - | hir::ItemKind::Enum(..) - | hir::ItemKind::Struct(..) - | hir::ItemKind::Union(..) - | hir::ItemKind::TyAlias(..) - | hir::ItemKind::OpaqueTy(..) - | hir::ItemKind::Static(..) - | hir::ItemKind::Trait(..) - | hir::ItemKind::TraitAlias(..) => { - self.modules.last_mut().unwrap().items.push((item, renamed, parent_id)) - } - hir::ItemKind::Const(..) => { - // Underscore constants do not correspond to a nameable item and - // so are never useful in documentation. - if name != kw::Underscore { - self.modules.last_mut().unwrap().items.push((item, renamed, parent_id)); - } - } - hir::ItemKind::Impl(impl_) => { - // Don't duplicate impls when inlining or if it's implementing a trait, we'll pick - // them up regardless of where they're located. - if !self.inlining && impl_.of_trait.is_none() { - self.modules.last_mut().unwrap().items.push((item, None, None)); - } - } - } - true - } - - fn visit_foreign_item_inner( - &mut self, - item: &'tcx hir::ForeignItem<'_>, - renamed: Option<Symbol>, - ) { - // If inlining we only want to include public functions. - if !self.inlining || self.cx.tcx.visibility(item.owner_id).is_public() { - self.modules.last_mut().unwrap().foreigns.push((item, renamed)); - } - } - pub(crate) fn visit(mut self) -> Module<'tcx> { - let root_module = self.cx.tcx.hir().root_module(); - self.visit_mod_contents(CRATE_HIR_ID, root_module); - - let mut top_level_module = self.modules.pop().unwrap(); + let mut top_level_module = self.visit_mod_contents( + hir::CRATE_HIR_ID, + self.cx.tcx.hir().root_module(), + self.cx.tcx.crate_name(LOCAL_CRATE), + None, + ); // `#[macro_export] macro_rules!` items are reexported at the top level of the // crate, regardless of where they're defined. We want to document the @@ -375,34 +151,24 @@ impl<'a, 'tcx> RustdocVisitor<'a, 'tcx> { top_level_module } - /// This method will create a new module and push it onto the "modules stack" then call - /// `visit_mod_contents`. Once done, it'll remove it from the "modules stack" and instead - /// add into the list of modules of the current module. - fn enter_mod(&mut self, id: hir::HirId, m: &'tcx hir::Mod<'tcx>, name: Symbol) { - self.modules.push(Module::new(name, id, m.spans.inner_span)); - - self.visit_mod_contents(id, m); - - let last = self.modules.pop().unwrap(); - self.modules.last_mut().unwrap().mods.push(last); - } - - /// This method will go through the given module items in two passes: - /// 1. The items which are not glob imports/reexports. - /// 2. The glob imports/reexports. - fn visit_mod_contents(&mut self, id: hir::HirId, m: &'tcx hir::Mod<'tcx>) { - debug!("Going through module {:?}", m); + fn visit_mod_contents( + &mut self, + id: hir::HirId, + m: &'tcx hir::Mod<'tcx>, + name: Symbol, + parent_id: Option<hir::HirId>, + ) -> Module<'tcx> { + let mut om = Module::new(name, id, m.spans.inner_span); let def_id = self.cx.tcx.hir().local_def_id(id).to_def_id(); // Keep track of if there were any private modules in the path. let orig_inside_public_path = self.inside_public_path; self.inside_public_path &= self.cx.tcx.visibility(def_id).is_public(); - - // Reimplementation of `walk_mod`: for &i in m.item_ids { let item = self.cx.tcx.hir().item(i); - if !matches!(item.kind, hir::ItemKind::Use(_, hir::UseKind::Glob)) { - self.visit_item(item); + if matches!(item.kind, hir::ItemKind::Use(_, hir::UseKind::Glob)) { + continue; } + self.visit_item(item, None, &mut om, parent_id); } for &i in m.item_ids { let item = self.cx.tcx.hir().item(i); @@ -410,34 +176,225 @@ impl<'a, 'tcx> RustdocVisitor<'a, 'tcx> { // Later passes in rustdoc will de-duplicate by name and kind, so if glob- // imported items appear last, then they'll be the ones that get discarded. if matches!(item.kind, hir::ItemKind::Use(_, hir::UseKind::Glob)) { - self.visit_item(item); + self.visit_item(item, None, &mut om, parent_id); } } self.inside_public_path = orig_inside_public_path; - } -} - -// We need to implement this visitor so it'll go everywhere and retrieve items we're interested in -// such as impl blocks in const blocks. -impl<'a, 'tcx> Visitor<'tcx> for RustdocVisitor<'a, 'tcx> { - type NestedFilter = nested_filter::All; - - fn nested_visit_map(&mut self) -> Self::Map { - self.map + om } - fn visit_item(&mut self, i: &'tcx hir::Item<'tcx>) { - let parent_id = if self.modules.len() > 1 { - Some(self.modules[self.modules.len() - 2].id) - } else { - None + /// Tries to resolve the target of a `pub use` statement and inlines the + /// target if it is defined locally and would not be documented otherwise, + /// or when it is specifically requested with `please_inline`. + /// (the latter is the case when the import is marked `doc(inline)`) + /// + /// Cross-crate inlining occurs later on during crate cleaning + /// and follows different rules. + /// + /// Returns `true` if the target has been inlined. + fn maybe_inline_local( + &mut self, + id: hir::HirId, + res: Res, + renamed: Option<Symbol>, + glob: bool, + om: &mut Module<'tcx>, + please_inline: bool, + ) -> bool { + debug!("maybe_inline_local res: {:?}", res); + + if self.cx.output_format.is_json() { + return false; + } + + let tcx = self.cx.tcx; + let Some(res_did) = res.opt_def_id() else { + return false; }; - if self.visit_item_inner(i, None, parent_id) { - walk_item(self, i); + + let use_attrs = tcx.hir().attrs(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); + + // For cross-crate impl inlining we need to know whether items are + // reachable in documentation -- a previously unreachable item can be + // made reachable by cross-crate inlining which we're checking here. + // (this is done here because we need to know this upfront). + if !res_did.is_local() && !is_no_inline { + crate::visit_lib::lib_embargo_visit_item(self.cx, res_did); + return false; + } + + let res_hir_id = match res_did.as_local() { + Some(n) => tcx.hir().local_def_id_to_hir_id(n), + None => return false, + }; + + let is_private = + !self.cx.cache.effective_visibilities.is_directly_public(self.cx.tcx, res_did); + let is_hidden = inherits_doc_hidden(self.cx.tcx, res_hir_id); + + // Only inline if requested or if the item would otherwise be stripped. + if (!please_inline && !is_private && !is_hidden) || is_no_inline { + return false; + } + + if !self.view_item_stack.insert(res_hir_id) { + return false; + } + + let ret = match tcx.hir().get(res_hir_id) { + Node::Item(&hir::Item { kind: hir::ItemKind::Mod(ref m), .. }) if glob => { + let prev = mem::replace(&mut self.inlining, true); + for &i in m.item_ids { + let i = self.cx.tcx.hir().item(i); + self.visit_item(i, None, om, Some(id)); + } + self.inlining = prev; + true + } + Node::Item(it) if !glob => { + let prev = mem::replace(&mut self.inlining, true); + self.visit_item(it, renamed, om, Some(id)); + self.inlining = prev; + true + } + Node::ForeignItem(it) if !glob => { + let prev = mem::replace(&mut self.inlining, true); + self.visit_foreign_item(it, renamed, om); + self.inlining = prev; + true + } + _ => false, + }; + self.view_item_stack.remove(&res_hir_id); + ret + } + + fn visit_item( + &mut self, + item: &'tcx hir::Item<'_>, + renamed: Option<Symbol>, + om: &mut Module<'tcx>, + parent_id: Option<hir::HirId>, + ) { + debug!("visiting item {:?}", item); + let name = renamed.unwrap_or(item.ident.name); + + let def_id = item.owner_id.to_def_id(); + let is_pub = self.cx.tcx.visibility(def_id).is_public(); + + if is_pub { + self.store_path(item.owner_id.to_def_id()); + } + + match item.kind { + hir::ItemKind::ForeignMod { items, .. } => { + for item in items { + let item = self.cx.tcx.hir().foreign_item(item.id); + self.visit_foreign_item(item, None, om); + } + } + // If we're inlining, skip private items or item reexported as "_". + _ if self.inlining && (!is_pub || renamed == Some(kw::Underscore)) => {} + hir::ItemKind::GlobalAsm(..) => {} + hir::ItemKind::Use(_, hir::UseKind::ListStem) => {} + hir::ItemKind::Use(path, kind) => { + for &res in &path.res { + // Struct and variant constructors and proc macro stubs always show up alongside + // their definitions, we've already processed them so just discard these. + if let Res::Def(DefKind::Ctor(..), _) | Res::SelfCtor(..) = res { + continue; + } + + let attrs = self.cx.tcx.hir().attrs(item.hir_id()); + + // If there was a private module in the current path then don't bother inlining + // anything as it will probably be stripped anyway. + if is_pub && self.inside_public_path { + let please_inline = attrs.iter().any(|item| match item.meta_item_list() { + Some(ref list) if item.has_name(sym::doc) => { + list.iter().any(|i| i.has_name(sym::inline)) + } + _ => false, + }); + let is_glob = kind == hir::UseKind::Glob; + let ident = if is_glob { None } else { Some(name) }; + if self.maybe_inline_local( + item.hir_id(), + res, + ident, + is_glob, + om, + please_inline, + ) { + continue; + } + } + + om.items.push((item, renamed, parent_id)) + } + } + hir::ItemKind::Macro(ref macro_def, _) => { + // `#[macro_export] macro_rules!` items are handled separately in `visit()`, + // above, since they need to be documented at the module top level. Accordingly, + // we only want to handle macros if one of three conditions holds: + // + // 1. This macro was defined by `macro`, and thus isn't covered by the case + // above. + // 2. This macro isn't marked with `#[macro_export]`, and thus isn't covered + // by the case above. + // 3. We're inlining, since a reexport where inlining has been requested + // should be inlined even if it is also documented at the top level. + + let def_id = item.owner_id.to_def_id(); + let is_macro_2_0 = !macro_def.macro_rules; + let nonexported = !self.cx.tcx.has_attr(def_id, sym::macro_export); + + if is_macro_2_0 || nonexported || self.inlining { + om.items.push((item, renamed, None)); + } + } + hir::ItemKind::Mod(ref m) => { + om.mods.push(self.visit_mod_contents(item.hir_id(), m, name, parent_id)); + } + hir::ItemKind::Fn(..) + | hir::ItemKind::ExternCrate(..) + | hir::ItemKind::Enum(..) + | hir::ItemKind::Struct(..) + | hir::ItemKind::Union(..) + | hir::ItemKind::TyAlias(..) + | hir::ItemKind::OpaqueTy(..) + | hir::ItemKind::Static(..) + | hir::ItemKind::Trait(..) + | hir::ItemKind::TraitAlias(..) => om.items.push((item, renamed, parent_id)), + hir::ItemKind::Const(..) => { + // Underscore constants do not correspond to a nameable item and + // so are never useful in documentation. + if name != kw::Underscore { + om.items.push((item, renamed, parent_id)); + } + } + hir::ItemKind::Impl(impl_) => { + // Don't duplicate impls when inlining or if it's implementing a trait, we'll pick + // them up regardless of where they're located. + if !self.inlining && impl_.of_trait.is_none() { + om.items.push((item, None, None)); + } + } } } - fn visit_mod(&mut self, _: &hir::Mod<'tcx>, _: Span, _: hir::HirId) { - // handled in `visit_item_inner` + fn visit_foreign_item( + &mut self, + item: &'tcx hir::ForeignItem<'_>, + renamed: Option<Symbol>, + om: &mut Module<'tcx>, + ) { + // If inlining we only want to include public functions. + if !self.inlining || self.cx.tcx.visibility(item.owner_id).is_public() { + om.foreigns.push((item, renamed)); + } } }