Skip to content

Commit

Permalink
Auto merge of #80987 - GuillaumeGomez:remove-cache-key, r=jyn514
Browse files Browse the repository at this point in the history
Remove CACHE_KEY global

We realized in #80914 that the cache handling (through a global) needed to be updated to make it much easier to handle.

r? `@jyn514`
  • Loading branch information
bors committed Jan 27, 2021
2 parents 742c972 + d78e1ed commit 613ef74
Show file tree
Hide file tree
Showing 14 changed files with 791 additions and 622 deletions.
4 changes: 2 additions & 2 deletions src/librustdoc/clean/mod.rs
Expand Up @@ -2329,14 +2329,14 @@ impl Clean<Item> for (&hir::MacroDef<'_>, Option<Symbol>) {
if matchers.len() <= 1 {
format!(
"{}macro {}{} {{\n ...\n}}",
vis.print_with_space(cx.tcx, def_id),
vis.print_with_space(cx.tcx, def_id, &cx.cache),
name,
matchers.iter().map(|span| span.to_src(cx)).collect::<String>(),
)
} else {
format!(
"{}macro {} {{\n{}}}",
vis.print_with_space(cx.tcx, def_id),
vis.print_with_space(cx.tcx, def_id, &cx.cache),
name,
matchers
.iter()
Expand Down
88 changes: 63 additions & 25 deletions src/librustdoc/clean/types.rs
Expand Up @@ -37,7 +37,7 @@ use crate::clean::inline;
use crate::clean::types::Type::{QPath, ResolvedPath};
use crate::clean::Clean;
use crate::core::DocContext;
use crate::formats::cache::cache;
use crate::formats::cache::Cache;
use crate::formats::item_type::ItemType;
use crate::html::render::cache::ExternalLocation;

Expand Down Expand Up @@ -169,8 +169,8 @@ impl Item {
self.attrs.collapsed_doc_value()
}

crate fn links(&self) -> Vec<RenderedLink> {
self.attrs.links(&self.def_id.krate)
crate fn links(&self, cache: &Cache) -> Vec<RenderedLink> {
self.attrs.links(&self.def_id.krate, cache)
}

crate fn is_crate(&self) -> bool {
Expand Down Expand Up @@ -826,7 +826,7 @@ impl Attributes {
/// Gets links as a vector
///
/// Cache must be populated before call
crate fn links(&self, krate: &CrateNum) -> Vec<RenderedLink> {
crate fn links(&self, krate: &CrateNum, cache: &Cache) -> Vec<RenderedLink> {
use crate::html::format::href;
use crate::html::render::CURRENT_DEPTH;

Expand All @@ -835,7 +835,7 @@ impl Attributes {
.filter_map(|ItemLink { link: s, link_text, did, fragment }| {
match *did {
Some(did) => {
if let Some((mut href, ..)) = href(did) {
if let Some((mut href, ..)) = href(did, cache) {
if let Some(ref fragment) = *fragment {
href.push('#');
href.push_str(fragment);
Expand All @@ -851,7 +851,6 @@ impl Attributes {
}
None => {
if let Some(ref fragment) = *fragment {
let cache = cache();
let url = match cache.extern_locations.get(krate) {
Some(&(_, _, ExternalLocation::Local)) => {
let depth = CURRENT_DEPTH.with(|l| l.get());
Expand Down Expand Up @@ -1177,6 +1176,13 @@ impl GetDefId for FnRetTy {
DefaultReturn => None,
}
}

fn def_id_full(&self, cache: &Cache) -> Option<DefId> {
match *self {
Return(ref ty) => ty.def_id_full(cache),
DefaultReturn => None,
}
}
}

#[derive(Clone, Debug)]
Expand Down Expand Up @@ -1299,13 +1305,31 @@ crate enum TypeKind {
}

crate trait GetDefId {
/// Use this method to get the [`DefId`] of a [`clean`] AST node.
/// This will return [`None`] when called on a primitive [`clean::Type`].
/// Use [`Self::def_id_full`] if you want to include primitives.
///
/// [`clean`]: crate::clean
/// [`clean::Type`]: crate::clean::Type
// FIXME: get rid of this function and always use `def_id_full`
fn def_id(&self) -> Option<DefId>;

/// Use this method to get the [DefId] of a [clean] AST node, including [PrimitiveType]s.
///
/// See [`Self::def_id`] for more.
///
/// [clean]: crate::clean
fn def_id_full(&self, cache: &Cache) -> Option<DefId>;
}

impl<T: GetDefId> GetDefId for Option<T> {
fn def_id(&self) -> Option<DefId> {
self.as_ref().and_then(|d| d.def_id())
}

fn def_id_full(&self, cache: &Cache) -> Option<DefId> {
self.as_ref().and_then(|d| d.def_id_full(cache))
}
}

impl Type {
Expand Down Expand Up @@ -1393,30 +1417,40 @@ impl Type {
}
}

impl GetDefId for Type {
fn def_id(&self) -> Option<DefId> {
match *self {
ResolvedPath { did, .. } => Some(did),
Primitive(p) => cache().primitive_locations.get(&p).cloned(),
BorrowedRef { type_: box Generic(..), .. } => {
Primitive(PrimitiveType::Reference).def_id()
}
BorrowedRef { ref type_, .. } => type_.def_id(),
impl Type {
fn inner_def_id(&self, cache: Option<&Cache>) -> Option<DefId> {
let t: PrimitiveType = match *self {
ResolvedPath { did, .. } => return Some(did),
Primitive(p) => return cache.and_then(|c| c.primitive_locations.get(&p).cloned()),
BorrowedRef { type_: box Generic(..), .. } => PrimitiveType::Reference,
BorrowedRef { ref type_, .. } => return type_.inner_def_id(cache),
Tuple(ref tys) => {
if tys.is_empty() {
Primitive(PrimitiveType::Unit).def_id()
PrimitiveType::Unit
} else {
Primitive(PrimitiveType::Tuple).def_id()
PrimitiveType::Tuple
}
}
BareFunction(..) => Primitive(PrimitiveType::Fn).def_id(),
Never => Primitive(PrimitiveType::Never).def_id(),
Slice(..) => Primitive(PrimitiveType::Slice).def_id(),
Array(..) => Primitive(PrimitiveType::Array).def_id(),
RawPointer(..) => Primitive(PrimitiveType::RawPointer).def_id(),
QPath { ref self_type, .. } => self_type.def_id(),
_ => None,
}
BareFunction(..) => PrimitiveType::Fn,
Never => PrimitiveType::Never,
Slice(..) => PrimitiveType::Slice,
Array(..) => PrimitiveType::Array,
RawPointer(..) => PrimitiveType::RawPointer,
QPath { ref self_type, .. } => return self_type.inner_def_id(cache),
// FIXME: remove this wildcard
_ => return None,
};
cache.and_then(|c| Primitive(t).def_id_full(c))
}
}

impl GetDefId for Type {
fn def_id(&self) -> Option<DefId> {
self.inner_def_id(None)
}

fn def_id_full(&self, cache: &Cache) -> Option<DefId> {
self.inner_def_id(Some(cache))
}
}

Expand Down Expand Up @@ -1817,6 +1851,10 @@ impl GetDefId for Typedef {
fn def_id(&self) -> Option<DefId> {
self.type_.def_id()
}

fn def_id_full(&self, cache: &Cache) -> Option<DefId> {
self.type_.def_id_full(cache)
}
}

#[derive(Clone, Debug)]
Expand Down
4 changes: 2 additions & 2 deletions src/librustdoc/clean/utils.rs
Expand Up @@ -177,7 +177,7 @@ crate fn get_real_types(
return res;
}
if arg.is_full_generic() {
let arg_s = Symbol::intern(&arg.print().to_string());
let arg_s = Symbol::intern(&arg.print(&cx.cache).to_string());
if let Some(where_pred) = generics.where_predicates.iter().find(|g| match g {
WherePredicate::BoundPredicate { ty, .. } => ty.def_id() == arg.def_id(),
_ => false,
Expand Down Expand Up @@ -473,7 +473,7 @@ crate fn resolve_type(cx: &DocContext<'_>, path: Path, id: hir::HirId) -> Type {
return Generic(kw::SelfUpper);
}
Res::Def(DefKind::TyParam, _) if path.segments.len() == 1 => {
return Generic(Symbol::intern(&format!("{:#}", path.print())));
return Generic(Symbol::intern(&format!("{:#}", path.print(&cx.cache))));
}
Res::SelfTy(..) | Res::Def(DefKind::TyParam | DefKind::AssocTy, _) => true,
_ => false,
Expand Down
2 changes: 1 addition & 1 deletion src/librustdoc/config.rs
Expand Up @@ -261,7 +261,7 @@ crate struct RenderOptions {
}

/// Temporary storage for data obtained during `RustdocVisitor::clean()`.
/// Later on moved into `CACHE_KEY`.
/// Later on moved into `cache`.
#[derive(Default, Clone)]
crate struct RenderInfo {
crate inlined: FxHashSet<DefId>,
Expand Down
8 changes: 6 additions & 2 deletions src/librustdoc/core.rs
Expand Up @@ -32,6 +32,7 @@ use crate::clean;
use crate::clean::{AttributesExt, MAX_DEF_ID};
use crate::config::{Options as RustdocOptions, RenderOptions};
use crate::config::{OutputFormat, RenderInfo};
use crate::formats::cache::Cache;
use crate::passes::{self, Condition::*, ConditionalPass};

crate use rustc_session::config::{DebuggingOptions, Input, Options};
Expand All @@ -45,9 +46,9 @@ crate struct DocContext<'tcx> {
///
/// Most of this logic is copied from rustc_lint::late.
crate param_env: Cell<ParamEnv<'tcx>>,
/// Later on moved into `CACHE_KEY`
/// Later on moved into `cache`
crate renderinfo: RefCell<RenderInfo>,
/// Later on moved through `clean::Crate` into `CACHE_KEY`
/// Later on moved through `clean::Crate` into `cache`
crate external_traits: Rc<RefCell<FxHashMap<DefId, clean::Trait>>>,
/// Used while populating `external_traits` to ensure we don't process the same trait twice at
/// the same time.
Expand Down Expand Up @@ -75,6 +76,8 @@ crate struct DocContext<'tcx> {
/// See `collect_intra_doc_links::traits_implemented_by` for more details.
/// `map<module, set<trait>>`
crate module_trait_cache: RefCell<FxHashMap<DefId, FxHashSet<DefId>>>,
/// Fake empty cache used when cache is required as parameter.
crate cache: Cache,
}

impl<'tcx> DocContext<'tcx> {
Expand Down Expand Up @@ -524,6 +527,7 @@ crate fn run_global_ctxt(
.collect(),
render_options,
module_trait_cache: RefCell::new(FxHashMap::default()),
cache: Cache::default(),
};
debug!("crate: {:?}", tcx.hir().krate());

Expand Down
11 changes: 2 additions & 9 deletions src/librustdoc/formats/cache.rs
@@ -1,8 +1,6 @@
use std::cell::RefCell;
use std::collections::BTreeMap;
use std::mem;
use std::path::{Path, PathBuf};
use std::sync::Arc;

use rustc_data_structures::fx::{FxHashMap, FxHashSet};
use rustc_hir::def_id::{CrateNum, DefId, CRATE_DEF_INDEX};
Expand All @@ -19,8 +17,6 @@ use crate::html::markdown::short_markdown_summary;
use crate::html::render::cache::{extern_location, get_index_search_type, ExternalLocation};
use crate::html::render::IndexItem;

thread_local!(crate static CACHE_KEY: RefCell<Arc<Cache>> = Default::default());

/// This cache is used to store information about the [`clean::Crate`] being
/// rendered in order to provide more useful documentation. This contains
/// information like all implementors of a trait, all traits a type implements,
Expand Down Expand Up @@ -197,6 +193,7 @@ impl Cache {
}

cache.stack.push(krate.name.to_string());

krate = cache.fold_crate(krate);

for (trait_did, dids, impl_) in cache.orphan_trait_impls.drain(..) {
Expand Down Expand Up @@ -319,7 +316,7 @@ impl DocFolder for Cache {
.map_or_else(String::new, |x| short_markdown_summary(&x.as_str())),
parent,
parent_idx: None,
search_type: get_index_search_type(&item),
search_type: get_index_search_type(&item, None),
});

for alias in item.attrs.get_doc_aliases() {
Expand Down Expand Up @@ -477,7 +474,3 @@ impl DocFolder for Cache {
ret
}
}

crate fn cache() -> Arc<Cache> {
CACHE_KEY.with(|c| c.borrow().clone())
}
5 changes: 5 additions & 0 deletions src/librustdoc/formats/mod.rs
Expand Up @@ -8,6 +8,7 @@ use rustc_span::def_id::DefId;

use crate::clean;
use crate::clean::types::GetDefId;
use crate::formats::cache::Cache;

/// Specifies whether rendering directly implemented trait items or ones from a certain Deref
/// impl.
Expand Down Expand Up @@ -41,4 +42,8 @@ impl Impl {
crate fn trait_did(&self) -> Option<DefId> {
self.inner_impl().trait_.def_id()
}

crate fn trait_did_full(&self, cache: &Cache) -> Option<DefId> {
self.inner_impl().trait_.def_id_full(cache)
}
}
33 changes: 11 additions & 22 deletions src/librustdoc/formats/renderer.rs
@@ -1,12 +1,10 @@
use std::sync::Arc;

use rustc_middle::ty::TyCtxt;
use rustc_span::edition::Edition;

use crate::clean;
use crate::config::{RenderInfo, RenderOptions};
use crate::error::Error;
use crate::formats::cache::{Cache, CACHE_KEY};
use crate::formats::cache::Cache;

/// Allows for different backends to rustdoc to be used with the `run_format()` function. Each
/// backend renderer has hooks for initialization, documenting an item, entering and exiting a
Expand All @@ -22,20 +20,15 @@ crate trait FormatRenderer<'tcx>: Clone {
options: RenderOptions,
render_info: RenderInfo,
edition: Edition,
cache: &mut Cache,
cache: Cache,
tcx: TyCtxt<'tcx>,
) -> Result<(Self, clean::Crate), Error>;

/// Renders a single non-module item. This means no recursive sub-item rendering is required.
fn item(&mut self, item: clean::Item, cache: &Cache) -> Result<(), Error>;
fn item(&mut self, item: clean::Item) -> Result<(), Error>;

/// Renders a module (should not handle recursing into children).
fn mod_item_in(
&mut self,
item: &clean::Item,
item_name: &str,
cache: &Cache,
) -> Result<(), Error>;
fn mod_item_in(&mut self, item: &clean::Item, item_name: &str) -> Result<(), Error>;

/// Runs after recursively rendering all sub-items of a module.
fn mod_item_out(&mut self, item_name: &str) -> Result<(), Error>;
Expand All @@ -46,9 +39,10 @@ crate trait FormatRenderer<'tcx>: Clone {
fn after_krate(
&mut self,
krate: &clean::Crate,
cache: &Cache,
diag: &rustc_errors::Handler,
) -> Result<(), Error>;

fn cache(&self) -> &Cache;
}

/// Main method for rendering a crate.
Expand All @@ -60,7 +54,7 @@ crate fn run_format<'tcx, T: FormatRenderer<'tcx>>(
edition: Edition,
tcx: TyCtxt<'tcx>,
) -> Result<(), Error> {
let (krate, mut cache) = tcx.sess.time("create_format_cache", || {
let (krate, cache) = tcx.sess.time("create_format_cache", || {
Cache::from_krate(
render_info.clone(),
options.document_private,
Expand All @@ -73,12 +67,7 @@ crate fn run_format<'tcx, T: FormatRenderer<'tcx>>(

let (mut format_renderer, mut krate) = prof
.extra_verbose_generic_activity("create_renderer", T::descr())
.run(|| T::init(krate, options, render_info, edition, &mut cache, tcx))?;

let cache = Arc::new(cache);
// Freeze the cache now that the index has been built. Put an Arc into TLS for future
// parallelization opportunities
CACHE_KEY.with(|v| *v.borrow_mut() = cache.clone());
.run(|| T::init(krate, options, render_info, edition, cache, tcx))?;

let mut item = match krate.module.take() {
Some(i) => i,
Expand All @@ -101,7 +90,7 @@ crate fn run_format<'tcx, T: FormatRenderer<'tcx>>(
}
let _timer = prof.generic_activity_with_arg("render_mod_item", name.as_str());

cx.mod_item_in(&item, &name, &cache)?;
cx.mod_item_in(&item, &name)?;
let module = match *item.kind {
clean::StrippedItem(box clean::ModuleItem(m)) | clean::ModuleItem(m) => m,
_ => unreachable!(),
Expand All @@ -114,9 +103,9 @@ crate fn run_format<'tcx, T: FormatRenderer<'tcx>>(
cx.mod_item_out(&name)?;
} else if item.name.is_some() {
prof.generic_activity_with_arg("render_item", &*item.name.unwrap_or(unknown).as_str())
.run(|| cx.item(item, &cache))?;
.run(|| cx.item(item))?;
}
}
prof.extra_verbose_generic_activity("renderer_after_krate", T::descr())
.run(|| format_renderer.after_krate(&krate, &cache, diag))
.run(|| format_renderer.after_krate(&krate, diag))
}

0 comments on commit 613ef74

Please sign in to comment.