Rollup merge of #84601 - tdelabro:rustdoc-get-rid-of-cache-extern_locations, r=jyn514

JohnTitor · web-flow · commit e30d952d8b8e · 2021-05-01T18:32:31.000+09:00
rustdoc: Only store locations in Cache::extern_locations and calculate the other info on-demand help #84588
diff --git a/src/librustdoc/clean/types.rs b/src/librustdoc/clean/types.rs
@@ -4,6 +4,7 @@ use std::fmt;
 use std::hash::{Hash, Hasher};
 use std::iter::FromIterator;
 use std::lazy::SyncOnceCell as OnceCell;
+use std::path::PathBuf;
 use std::rc::Rc;
 use std::sync::Arc;
 use std::{slice, vec};
@@ -90,6 +91,58 @@ impl ExternalCrate {
         tcx.crate_name(self.crate_num)
     }
 
+    crate fn src_root(&self, tcx: TyCtxt<'_>) -> PathBuf {
+        match self.src(tcx) {
+            FileName::Real(ref p) => match p.local_path().parent() {
+                Some(p) => p.to_path_buf(),
+                None => PathBuf::new(),
+            },
+            _ => PathBuf::new(),
+        }
+    }
+
+    /// Attempts to find where an external crate is located, given that we're
+    /// rendering in to the specified source destination.
+    crate fn location(
+        &self,
+        extern_url: Option<&str>,
+        dst: &std::path::Path,
+        tcx: TyCtxt<'_>,
+    ) -> ExternalLocation {
+        use ExternalLocation::*;
+
+        fn to_remote(url: impl ToString) -> ExternalLocation {
+            let mut url = url.to_string();
+            if !url.ends_with('/') {
+                url.push('/');
+            }
+            Remote(url)
+        }
+
+        // See if there's documentation generated into the local directory
+        // WARNING: since rustdoc creates these directories as it generates documentation, this check is only accurate before rendering starts.
+        // Make sure to call `location()` by that time.
+        let local_location = dst.join(&*self.name(tcx).as_str());
+        if local_location.is_dir() {
+            return Local;
+        }
+
+        if let Some(url) = extern_url {
+            return to_remote(url);
+        }
+
+        // Failing that, see if there's an attribute specifying where to find this
+        // external crate
+        let did = DefId { krate: self.crate_num, index: CRATE_DEF_INDEX };
+        tcx.get_attrs(did)
+            .lists(sym::doc)
+            .filter(|a| a.has_name(sym::html_root_url))
+            .filter_map(|a| a.value_str())
+            .map(to_remote)
+            .next()
+            .unwrap_or(Unknown) // Well, at least we tried.
+    }
+
     crate fn keywords(&self, tcx: TyCtxt<'_>) -> ThinVec<(DefId, Symbol)> {
         let root = self.def_id();
 
@@ -381,7 +434,7 @@ impl Item {
                         let relative_to = &cx.current;
                         if let Some(ref fragment) = *fragment {
                             let url = match cx.cache().extern_locations.get(&self.def_id.krate) {
-                                Some(&(_, _, ExternalLocation::Local)) => {
+                                Some(ExternalLocation::Local) => {
                                     if relative_to[0] == "std" {
                                         let depth = relative_to.len() - 1;
                                         "../".repeat(depth)
@@ -390,10 +443,10 @@ impl Item {
                                         format!("{}std/", "../".repeat(depth))
                                     }
                                 }
-                                Some(&(_, _, ExternalLocation::Remote(ref s))) => {
+                                Some(ExternalLocation::Remote(ref s)) => {
                                     format!("{}/std/", s.trim_end_matches('/'))
                                 }
-                                Some(&(_, _, ExternalLocation::Unknown)) | None => format!(
+                                Some(ExternalLocation::Unknown) | None => format!(
                                     "https://doc.rust-lang.org/{}/std/",
                                     crate::doc_rust_lang_org_channel(),
                                 ),
diff --git a/src/librustdoc/formats/cache.rs b/src/librustdoc/formats/cache.rs
@@ -1,21 +1,19 @@
 use std::collections::BTreeMap;
 use std::mem;
-use std::path::{Path, PathBuf};
+use std::path::Path;
 
 use rustc_data_structures::fx::{FxHashMap, FxHashSet};
 use rustc_hir::def_id::{CrateNum, DefId, CRATE_DEF_INDEX};
 use rustc_middle::middle::privacy::AccessLevels;
 use rustc_middle::ty::TyCtxt;
-use rustc_span::source_map::FileName;
 use rustc_span::symbol::sym;
-use rustc_span::Symbol;
 
 use crate::clean::{self, GetDefId};
 use crate::fold::DocFolder;
 use crate::formats::item_type::ItemType;
 use crate::formats::Impl;
 use crate::html::markdown::short_markdown_summary;
-use crate::html::render::cache::{extern_location, get_index_search_type, ExternalLocation};
+use crate::html::render::cache::{get_index_search_type, ExternalLocation};
 use crate::html::render::IndexItem;
 
 /// This cache is used to store information about the [`clean::Crate`] being
@@ -72,7 +70,7 @@ crate struct Cache {
     crate implementors: FxHashMap<DefId, Vec<Impl>>,
 
     /// Cache of where external crate documentation can be found.
-    crate extern_locations: FxHashMap<CrateNum, (Symbol, PathBuf, ExternalLocation)>,
+    crate extern_locations: FxHashMap<CrateNum, ExternalLocation>,
 
     /// Cache of where documentation for primitives can be found.
     crate primitive_locations: FxHashMap<clean::PrimitiveType, DefId>,
@@ -155,21 +153,10 @@ impl Cache {
         // 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 &(n, ref e) in &krate.externs {
-            let src_root = match e.src(tcx) {
-                FileName::Real(ref p) => match p.local_path().parent() {
-                    Some(p) => p.to_path_buf(),
-                    None => PathBuf::new(),
-                },
-                _ => PathBuf::new(),
-            };
             let name = e.name(tcx);
             let extern_url = extern_html_root_urls.get(&*name.as_str()).map(|u| &**u);
             let did = DefId { krate: n, index: CRATE_DEF_INDEX };
-            self.extern_locations.insert(
-                n,
-                (name, src_root, extern_location(e, extern_url, tcx.get_attrs(did), &dst, tcx)),
-            );
-
+            self.extern_locations.insert(n, e.location(extern_url, &dst, tcx));
             self.external_paths.insert(did, (vec![name.to_string()], ItemType::Module));
         }
 
diff --git a/src/librustdoc/html/format.rs b/src/librustdoc/html/format.rs
@@ -16,7 +16,7 @@ use rustc_middle::ty::TyCtxt;
 use rustc_span::def_id::{DefId, CRATE_DEF_INDEX};
 use rustc_target::spec::abi::Abi;
 
-use crate::clean::{self, utils::find_nearest_parent_module, PrimitiveType};
+use crate::clean::{self, utils::find_nearest_parent_module, ExternalCrate, PrimitiveType};
 use crate::formats::item_type::ItemType;
 use crate::html::escape::Escape;
 use crate::html::render::cache::ExternalLocation;
@@ -465,14 +465,14 @@ crate fn href(did: DefId, cx: &Context<'_>) -> Option<(String, ItemType, Vec<Str
                 fqp,
                 shortty,
                 match cache.extern_locations[&did.krate] {
-                    (.., ExternalLocation::Remote(ref s)) => {
+                    ExternalLocation::Remote(ref s) => {
                         let s = s.trim_end_matches('/');
                         let mut s = vec![&s[..]];
                         s.extend(module_fqp[..].iter().map(String::as_str));
                         s
                     }
-                    (.., ExternalLocation::Local) => href_relative_parts(module_fqp, relative_to),
-                    (.., ExternalLocation::Unknown) => return None,
+                    ExternalLocation::Local => href_relative_parts(module_fqp, relative_to),
+                    ExternalLocation::Unknown => return None,
                 },
             )
         }
@@ -578,20 +578,22 @@ fn primitive_link(
             Some(&def_id) => {
                 let cname_str;
                 let loc = match m.extern_locations[&def_id.krate] {
-                    (ref cname, _, ExternalLocation::Remote(ref s)) => {
-                        cname_str = cname.as_str();
+                    ExternalLocation::Remote(ref s) => {
+                        cname_str =
+                            ExternalCrate { crate_num: def_id.krate }.name(cx.tcx()).as_str();
                         Some(vec![s.trim_end_matches('/'), &cname_str[..]])
                     }
-                    (ref cname, _, ExternalLocation::Local) => {
-                        cname_str = cname.as_str();
+                    ExternalLocation::Local => {
+                        cname_str =
+                            ExternalCrate { crate_num: def_id.krate }.name(cx.tcx()).as_str();
                         Some(if cx.current.first().map(|x| &x[..]) == Some(&cname_str[..]) {
                             iter::repeat("..").take(cx.current.len() - 1).collect()
                         } else {
                             let cname = iter::once(&cname_str[..]);
                             iter::repeat("..").take(cx.current.len()).chain(cname).collect()
                         })
                     }
-                    (.., ExternalLocation::Unknown) => None,
+                    ExternalLocation::Unknown => None,
                 };
                 if let Some(loc) = loc {
                     write!(
diff --git a/src/librustdoc/html/render/cache.rs b/src/librustdoc/html/render/cache.rs
@@ -1,15 +1,13 @@
 use std::collections::BTreeMap;
-use std::path::Path;
 
-use rustc_ast::ast;
 use rustc_data_structures::fx::{FxHashMap, FxHashSet};
 use rustc_middle::ty::TyCtxt;
-use rustc_span::symbol::{sym, Symbol};
+use rustc_span::symbol::Symbol;
 use serde::ser::{Serialize, SerializeStruct, Serializer};
 
 use crate::clean;
 use crate::clean::types::{
-    AttributesExt, FnDecl, FnRetTy, GenericBound, Generics, GetDefId, Type, WherePredicate,
+    FnDecl, FnRetTy, GenericBound, Generics, GetDefId, Type, WherePredicate,
 };
 use crate::formats::cache::Cache;
 use crate::formats::item_type::ItemType;
@@ -26,47 +24,6 @@ crate enum ExternalLocation {
     Unknown,
 }
 
-/// Attempts to find where an external crate is located, given that we're
-/// rendering in to the specified source destination.
-crate fn extern_location(
-    e: &clean::ExternalCrate,
-    extern_url: Option<&str>,
-    ast_attrs: &[ast::Attribute],
-    dst: &Path,
-    tcx: TyCtxt<'_>,
-) -> ExternalLocation {
-    use ExternalLocation::*;
-    // See if there's documentation generated into the local directory
-    let local_location = dst.join(&*e.name(tcx).as_str());
-    if local_location.is_dir() {
-        return Local;
-    }
-
-    if let Some(url) = extern_url {
-        let mut url = url.to_string();
-        if !url.ends_with('/') {
-            url.push('/');
-        }
-        return Remote(url);
-    }
-
-    // Failing that, see if there's an attribute specifying where to find this
-    // external crate
-    ast_attrs
-        .lists(sym::doc)
-        .filter(|a| a.has_name(sym::html_root_url))
-        .filter_map(|a| a.value_str())
-        .map(|url| {
-            let mut url = url.to_string();
-            if !url.ends_with('/') {
-                url.push('/')
-            }
-            Remote(url)
-        })
-        .next()
-        .unwrap_or(Unknown) // Well, at least we tried.
-}
-
 /// Builds the search index from the collected metadata
 crate fn build_index<'tcx>(krate: &clean::Crate, cache: &mut Cache, tcx: TyCtxt<'tcx>) -> String {
     let mut defid_to_pathid = FxHashMap::default();
diff --git a/src/librustdoc/html/render/context.rs b/src/librustdoc/html/render/context.rs
@@ -18,7 +18,7 @@ use super::print_item::{full_path, item_path, print_item};
 use super::write_shared::write_shared;
 use super::{print_sidebar, settings, AllTypes, NameDoc, StylePath, BASIC_KEYWORDS};
 
-use crate::clean;
+use crate::clean::{self, ExternalCrate};
 use crate::config::RenderOptions;
 use crate::docfs::{DocFS, PathError};
 use crate::error::Error;
@@ -304,12 +304,16 @@ impl<'tcx> Context<'tcx> {
             }
         } else {
             let (krate, src_root) = match *self.cache.extern_locations.get(&cnum)? {
-                (name, ref src, ExternalLocation::Local) => (name, src),
-                (name, ref src, ExternalLocation::Remote(ref s)) => {
+                ExternalLocation::Local => {
+                    let e = ExternalCrate { crate_num: cnum };
+                    (e.name(self.tcx()), e.src_root(self.tcx()))
+                }
+                ExternalLocation::Remote(ref s) => {
                     root = s.to_string();
-                    (name, src)
+                    let e = ExternalCrate { crate_num: cnum };
+                    (e.name(self.tcx()), e.src_root(self.tcx()))
                 }
-                (_, _, ExternalLocation::Unknown) => return None,
+                ExternalLocation::Unknown => return None,
             };
 
             sources::clean_path(&src_root, file, false, |component| {
diff --git a/src/librustdoc/json/mod.rs b/src/librustdoc/json/mod.rs
@@ -17,7 +17,7 @@ use rustc_session::Session;
 
 use rustdoc_json_types as types;
 
-use crate::clean;
+use crate::clean::{self, ExternalCrate};
 use crate::config::RenderOptions;
 use crate::error::Error;
 use crate::formats::cache::Cache;
@@ -218,12 +218,13 @@ impl<'tcx> FormatRenderer<'tcx> for JsonRenderer<'tcx> {
                 .cache
                 .extern_locations
                 .iter()
-                .map(|(k, v)| {
+                .map(|(crate_num, external_location)| {
+                    let e = ExternalCrate { crate_num: *crate_num };
                     (
-                        k.as_u32(),
+                        crate_num.as_u32(),
                         types::ExternalCrate {
-                            name: v.0.to_string(),
-                            html_root_url: match &v.2 {
+                            name: e.name(self.tcx).to_string(),
+                            html_root_url: match external_location {
                                 ExternalLocation::Remote(s) => Some(s.clone()),
                                 _ => None,
                             },
