Auto merge of rust-lang#80987 - GuillaumeGomez:remove-cache-key, r=jyn514

Remove CACHE_KEY global

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

r? `@jyn514`

Author: bors

src/librustdoc/clean/mod.rs

@@ -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()

src/librustdoc/clean/types.rs

@@ -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;
 
@@ -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 {
@@ -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;
 
@@ -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);
@@ -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());
@@ -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)]
@@ -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 {
@@ -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))
     }
 }
 
@@ -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)]

src/librustdoc/clean/utils.rs

@@ -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,
@@ -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,

src/librustdoc/config.rs

@@ -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>,

src/librustdoc/core.rs

@@ -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};
@@ -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.
@@ -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> {
@@ -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());
 

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};
@@ -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,
@@ -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(..) {
@@ -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() {
@@ -477,7 +474,3 @@ impl DocFolder for Cache {
         ret
     }
 }
-
-crate fn cache() -> Arc<Cache> {
-    CACHE_KEY.with(|c| c.borrow().clone())
-}

src/librustdoc/formats/mod.rs

@@ -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.
@@ -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)
+    }
 }

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
@@ -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>;
@@ -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.
@@ -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,
@@ -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,
@@ -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!(),
@@ -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))
 }