From 331465a867fd450939d23d6ebd0580fc0ef18059 Mon Sep 17 00:00:00 2001 From: Noah Lev Date: Fri, 19 Nov 2021 17:51:45 -0500 Subject: [PATCH 1/4] rustdoc: Pass DocContext to `Cache::populate` This will allow removing `Crate.externs`. --- src/librustdoc/core.rs | 6 ++---- src/librustdoc/formats/cache.rs | 33 ++++++++++++++++----------------- 2 files changed, 18 insertions(+), 21 deletions(-) diff --git a/src/librustdoc/core.rs b/src/librustdoc/core.rs index a331f4cf3e6e7..c58310947d282 100644 --- a/src/librustdoc/core.rs +++ b/src/librustdoc/core.rs @@ -508,14 +508,12 @@ crate fn run_global_ctxt( rustc_errors::FatalError.raise(); } - let render_options = ctxt.render_options; - let mut cache = ctxt.cache; - krate = tcx.sess.time("create_format_cache", || cache.populate(krate, tcx, &render_options)); + krate = tcx.sess.time("create_format_cache", || Cache::populate(&mut ctxt, krate)); // The main crate doc comments are always collapsed. krate.collapsed = true; - (krate, render_options, cache) + (krate, ctxt.render_options, ctxt.cache) } /// Due to , diff --git a/src/librustdoc/formats/cache.rs b/src/librustdoc/formats/cache.rs index 05d8b643f502b..b6808732ad043 100644 --- a/src/librustdoc/formats/cache.rs +++ b/src/librustdoc/formats/cache.rs @@ -8,6 +8,7 @@ use rustc_span::symbol::sym; use crate::clean::{self, ItemId, PrimitiveType}; use crate::config::RenderOptions; +use crate::core::DocContext; use crate::fold::DocFolder; use crate::formats::item_type::ItemType; use crate::formats::Impl; @@ -136,15 +137,13 @@ impl Cache { /// Populates the `Cache` with more data. The returned `Crate` will be missing some data that was /// in `krate` due to the data being moved into the `Cache`. - crate fn populate( - &mut self, - mut krate: clean::Crate, - tcx: TyCtxt<'_>, - render_options: &RenderOptions, - ) -> clean::Crate { + crate fn populate(cx: &mut DocContext<'_>, mut krate: clean::Crate) -> clean::Crate { + let tcx = cx.tcx; + let render_options = &cx.render_options; + // Crawl the crate to build various caches used for the output - debug!(?self.crate_version); - self.traits = krate.external_traits.take(); + debug!(?cx.cache.crate_version); + cx.cache.traits = krate.external_traits.take(); let RenderOptions { extern_html_root_takes_precedence, output: dst, .. } = render_options; // Cache where all our extern crates are located @@ -154,28 +153,28 @@ impl Cache { let extern_url = render_options.extern_html_root_urls.get(&*name.as_str()).map(|u| &**u); let location = e.location(extern_url, *extern_html_root_takes_precedence, dst, tcx); - self.extern_locations.insert(e.crate_num, location); - self.external_paths.insert(e.def_id(), (vec![name.to_string()], ItemType::Module)); + cx.cache.extern_locations.insert(e.crate_num, location); + cx.cache.external_paths.insert(e.def_id(), (vec![name.to_string()], ItemType::Module)); } // FIXME: avoid this clone (requires implementing Default manually) - self.primitive_locations = PrimitiveType::primitive_locations(tcx).clone(); - for (prim, &def_id) in &self.primitive_locations { + cx.cache.primitive_locations = PrimitiveType::primitive_locations(tcx).clone(); + for (prim, &def_id) in &cx.cache.primitive_locations { let crate_name = tcx.crate_name(def_id.krate); // Recall that we only allow primitive modules to be at the root-level of the crate. // If that restriction is ever lifted, this will have to include the relative paths instead. - self.external_paths.insert( + cx.cache.external_paths.insert( def_id, (vec![crate_name.to_string(), prim.as_sym().to_string()], ItemType::Primitive), ); } - krate = CacheBuilder { tcx, cache: self }.fold_crate(krate); + krate = CacheBuilder { tcx, cache: &mut cx.cache }.fold_crate(krate); - for (trait_did, dids, impl_) in self.orphan_trait_impls.drain(..) { - if self.traits.contains_key(&trait_did) { + for (trait_did, dids, impl_) in cx.cache.orphan_trait_impls.drain(..) { + if cx.cache.traits.contains_key(&trait_did) { for did in dids { - self.impls.entry(did).or_default().push(impl_.clone()); + cx.cache.impls.entry(did).or_default().push(impl_.clone()); } } } From f41beab5cd939745924a61d5af691a05f156bd12 Mon Sep 17 00:00:00 2001 From: Noah Lev Date: Fri, 19 Nov 2021 21:16:48 -0500 Subject: [PATCH 2/4] rustdoc: Remove `Crate.externs` and compute on-demand instead --- src/librustdoc/clean/types.rs | 3 +-- src/librustdoc/clean/utils.rs | 17 +---------------- src/librustdoc/formats/cache.rs | 19 ++++++++++++++----- 3 files changed, 16 insertions(+), 23 deletions(-) diff --git a/src/librustdoc/clean/types.rs b/src/librustdoc/clean/types.rs index fb08ced205d86..2db751a6e409b 100644 --- a/src/librustdoc/clean/types.rs +++ b/src/librustdoc/clean/types.rs @@ -117,7 +117,6 @@ impl From for ItemId { #[derive(Clone, Debug)] crate struct Crate { crate module: Item, - crate externs: Vec, crate primitives: ThinVec<(DefId, PrimitiveType)>, /// Only here so that they can be filtered through the rustdoc passes. crate external_traits: Rc>>, @@ -126,7 +125,7 @@ crate struct Crate { // `Crate` is frequently moved by-value. Make sure it doesn't unintentionally get bigger. #[cfg(all(target_arch = "x86_64", target_pointer_width = "64"))] -rustc_data_structures::static_assert_size!(Crate, 104); +rustc_data_structures::static_assert_size!(Crate, 80); impl Crate { crate fn name(&self, tcx: TyCtxt<'_>) -> Symbol { diff --git a/src/librustdoc/clean/utils.rs b/src/librustdoc/clean/utils.rs index 2fa7efcc6509b..69f908b58517f 100644 --- a/src/librustdoc/clean/utils.rs +++ b/src/librustdoc/clean/utils.rs @@ -24,17 +24,8 @@ use std::mem; mod tests; crate fn krate(cx: &mut DocContext<'_>) -> Crate { - use crate::visit_lib::LibEmbargoVisitor; - let module = crate::visit_ast::RustdocVisitor::new(cx).visit(); - let mut externs = Vec::new(); - for &cnum in cx.tcx.crates(()) { - externs.push(ExternalCrate { crate_num: cnum }); - // Analyze doc-reachability for extern items - LibEmbargoVisitor::new(cx).visit_lib(cnum); - } - // Clean the crate, translating the entire librustc_ast AST to one that is // understood by rustdoc. let mut module = module.clean(cx); @@ -76,13 +67,7 @@ crate fn krate(cx: &mut DocContext<'_>) -> Crate { })); } - Crate { - module, - externs, - primitives, - external_traits: cx.external_traits.clone(), - collapsed: false, - } + Crate { module, primitives, external_traits: cx.external_traits.clone(), collapsed: false } } fn external_generic_args( diff --git a/src/librustdoc/formats/cache.rs b/src/librustdoc/formats/cache.rs index b6808732ad043..00ad75964d3ab 100644 --- a/src/librustdoc/formats/cache.rs +++ b/src/librustdoc/formats/cache.rs @@ -6,7 +6,7 @@ use rustc_middle::middle::privacy::AccessLevels; use rustc_middle::ty::TyCtxt; use rustc_span::symbol::sym; -use crate::clean::{self, ItemId, PrimitiveType}; +use crate::clean::{self, ExternalCrate, ItemId, PrimitiveType}; use crate::config::RenderOptions; use crate::core::DocContext; use crate::fold::DocFolder; @@ -15,6 +15,7 @@ use crate::formats::Impl; use crate::html::markdown::short_markdown_summary; use crate::html::render::cache::{get_index_search_type, ExternalLocation}; use crate::html::render::IndexItem; +use crate::visit_lib::LibEmbargoVisitor; /// This cache is used to store information about the [`clean::Crate`] being /// rendered in order to provide more useful documentation. This contains @@ -139,19 +140,27 @@ impl Cache { /// in `krate` due to the data being moved into the `Cache`. crate fn populate(cx: &mut DocContext<'_>, mut krate: clean::Crate) -> clean::Crate { let tcx = cx.tcx; - let render_options = &cx.render_options; // Crawl the crate to build various caches used for the output debug!(?cx.cache.crate_version); cx.cache.traits = krate.external_traits.take(); - let RenderOptions { extern_html_root_takes_precedence, output: dst, .. } = render_options; + + let mut externs = Vec::new(); + for &cnum in cx.tcx.crates(()) { + externs.push(ExternalCrate { crate_num: cnum }); + // Analyze doc-reachability for extern items + LibEmbargoVisitor::new(cx).visit_lib(cnum); + } + + let RenderOptions { extern_html_root_takes_precedence, output: dst, .. } = + &cx.render_options; // Cache where all our extern crates are located // FIXME: this part is specific to HTML so it'd be nice to remove it from the common code - for &e in &krate.externs { + for e in externs { let name = e.name(tcx); let extern_url = - render_options.extern_html_root_urls.get(&*name.as_str()).map(|u| &**u); + cx.render_options.extern_html_root_urls.get(&*name.as_str()).map(|u| &**u); let location = e.location(extern_url, *extern_html_root_takes_precedence, dst, tcx); cx.cache.extern_locations.insert(e.crate_num, location); cx.cache.external_paths.insert(e.def_id(), (vec![name.to_string()], ItemType::Module)); From 6c017f071d5894a75c3cb37aa48791e77f50a8f2 Mon Sep 17 00:00:00 2001 From: Noah Lev Date: Fri, 19 Nov 2021 21:24:12 -0500 Subject: [PATCH 3/4] rustdoc: Merge visits of extern crates into one loop --- src/librustdoc/formats/cache.rs | 24 ++++++++++-------------- 1 file changed, 10 insertions(+), 14 deletions(-) diff --git a/src/librustdoc/formats/cache.rs b/src/librustdoc/formats/cache.rs index 00ad75964d3ab..a5c9370c05ce5 100644 --- a/src/librustdoc/formats/cache.rs +++ b/src/librustdoc/formats/cache.rs @@ -7,7 +7,6 @@ use rustc_middle::ty::TyCtxt; use rustc_span::symbol::sym; use crate::clean::{self, ExternalCrate, ItemId, PrimitiveType}; -use crate::config::RenderOptions; use crate::core::DocContext; use crate::fold::DocFolder; use crate::formats::item_type::ItemType; @@ -145,23 +144,20 @@ impl Cache { debug!(?cx.cache.crate_version); cx.cache.traits = krate.external_traits.take(); - let mut externs = Vec::new(); - for &cnum in cx.tcx.crates(()) { - externs.push(ExternalCrate { crate_num: cnum }); - // Analyze doc-reachability for extern items - LibEmbargoVisitor::new(cx).visit_lib(cnum); - } - - let RenderOptions { extern_html_root_takes_precedence, output: dst, .. } = - &cx.render_options; - // Cache where all our extern crates are located // FIXME: this part is specific to HTML so it'd be nice to remove it from the common code - for e in externs { + for &crate_num in cx.tcx.crates(()) { + let e = ExternalCrate { crate_num }; + // Analyze doc-reachability for extern items + LibEmbargoVisitor::new(cx).visit_lib(e.crate_num); + let name = e.name(tcx); + let render_options = &cx.render_options; let extern_url = - cx.render_options.extern_html_root_urls.get(&*name.as_str()).map(|u| &**u); - let location = e.location(extern_url, *extern_html_root_takes_precedence, dst, tcx); + render_options.extern_html_root_urls.get(&*name.as_str()).map(|u| &**u); + let extern_url_takes_precedence = render_options.extern_html_root_takes_precedence; + let dst = &render_options.output; + let location = e.location(extern_url, extern_url_takes_precedence, dst, tcx); cx.cache.extern_locations.insert(e.crate_num, location); cx.cache.external_paths.insert(e.def_id(), (vec![name.to_string()], ItemType::Module)); } From bbc3825d26c0cdef066ca6aa665f85241befc3d0 Mon Sep 17 00:00:00 2001 From: Noah Lev Date: Fri, 19 Nov 2021 22:00:37 -0500 Subject: [PATCH 4/4] rustdoc: Move doc-reachability visiting back to cleaning It populates `cx.cache.access_levels`, which seems to be needed during cleaning since a bunch of tests are failing. --- src/librustdoc/clean/utils.rs | 6 ++++++ src/librustdoc/formats/cache.rs | 3 --- 2 files changed, 6 insertions(+), 3 deletions(-) diff --git a/src/librustdoc/clean/utils.rs b/src/librustdoc/clean/utils.rs index 69f908b58517f..9860b21e0a971 100644 --- a/src/librustdoc/clean/utils.rs +++ b/src/librustdoc/clean/utils.rs @@ -7,6 +7,7 @@ use crate::clean::{ }; use crate::core::DocContext; use crate::formats::item_type::ItemType; +use crate::visit_lib::LibEmbargoVisitor; use rustc_ast as ast; use rustc_ast::tokenstream::TokenTree; @@ -26,6 +27,11 @@ mod tests; crate fn krate(cx: &mut DocContext<'_>) -> Crate { let module = crate::visit_ast::RustdocVisitor::new(cx).visit(); + for &cnum in cx.tcx.crates(()) { + // Analyze doc-reachability for extern items + LibEmbargoVisitor::new(cx).visit_lib(cnum); + } + // Clean the crate, translating the entire librustc_ast AST to one that is // understood by rustdoc. let mut module = module.clean(cx); diff --git a/src/librustdoc/formats/cache.rs b/src/librustdoc/formats/cache.rs index a5c9370c05ce5..db2b836de86e4 100644 --- a/src/librustdoc/formats/cache.rs +++ b/src/librustdoc/formats/cache.rs @@ -14,7 +14,6 @@ use crate::formats::Impl; use crate::html::markdown::short_markdown_summary; use crate::html::render::cache::{get_index_search_type, ExternalLocation}; use crate::html::render::IndexItem; -use crate::visit_lib::LibEmbargoVisitor; /// This cache is used to store information about the [`clean::Crate`] being /// rendered in order to provide more useful documentation. This contains @@ -148,8 +147,6 @@ impl Cache { // FIXME: this part is specific to HTML so it'd be nice to remove it from the common code for &crate_num in cx.tcx.crates(()) { let e = ExternalCrate { crate_num }; - // Analyze doc-reachability for extern items - LibEmbargoVisitor::new(cx).visit_lib(e.crate_num); let name = e.name(tcx); let render_options = &cx.render_options;