From 3fe671d87361ca34abcf7d4e7f109f2b7c1fc049 Mon Sep 17 00:00:00 2001 From: Justus K Date: Tue, 6 Jul 2021 18:07:46 +0200 Subject: [PATCH] rustdoc: Unify external_paths and paths cache map The `formats::cache::Cache` fields `paths` and `external_paths` are now unified to a single `paths` map, which makes it easier to manage the paths and avoids using local paths as extern and vice versa. The `paths` map maps a `DefId` to an enum which can be either `Local` or `Extern` so there can't be any misusing. --- src/librustdoc/clean/inline.rs | 4 +- src/librustdoc/formats/cache.rs | 51 ++++++++++++++-------- src/librustdoc/html/format.rs | 9 ++-- src/librustdoc/html/render/cache.rs | 6 +-- src/librustdoc/html/render/context.rs | 6 ++- src/librustdoc/html/render/mod.rs | 9 +++- src/librustdoc/html/render/print_item.rs | 12 +++-- src/librustdoc/html/render/write_shared.rs | 13 +++--- src/librustdoc/json/mod.rs | 14 +++--- 9 files changed, 69 insertions(+), 55 deletions(-) diff --git a/src/librustdoc/clean/inline.rs b/src/librustdoc/clean/inline.rs index 175c7facfdbe..2e6e32dbda9b 100644 --- a/src/librustdoc/clean/inline.rs +++ b/src/librustdoc/clean/inline.rs @@ -18,7 +18,7 @@ use crate::clean::{ self, utils, Attributes, AttributesExt, GetDefId, ItemId, NestedAttributesExt, Type, }; use crate::core::DocContext; -use crate::formats::item_type::ItemType; +use crate::formats::{cache::CachedPath, item_type::ItemType}; use super::{Clean, Visibility}; @@ -189,7 +189,7 @@ crate fn record_extern_fqn(cx: &mut DocContext<'_>, did: DefId, kind: ItemType) if did.is_local() { cx.cache.exact_paths.insert(did, fqn); } else { - cx.cache.external_paths.insert(did, (fqn, kind)); + cx.cache.paths.insert(did, CachedPath::Extern(fqn, kind)); } } diff --git a/src/librustdoc/formats/cache.rs b/src/librustdoc/formats/cache.rs index e7d6e5ac2c24..2798434bc6fa 100644 --- a/src/librustdoc/formats/cache.rs +++ b/src/librustdoc/formats/cache.rs @@ -16,6 +16,24 @@ use crate::html::markdown::short_markdown_summary; use crate::html::render::cache::{get_index_search_type, ExternalLocation}; use crate::html::render::IndexItem; +/// A path that is either local, or external. It consists of a fully qualified name, +/// and a type description. +#[derive(Debug, Clone)] +crate enum CachedPath { + Local(Vec, ItemType), + Extern(Vec, ItemType), +} + +impl CachedPath { + crate fn is_local(&self) -> bool { + matches!(self, CachedPath::Local(..)) + } + + crate fn is_extern(&self) -> bool { + matches!(self, CachedPath::Extern(..)) + } +} + /// 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, @@ -35,16 +53,12 @@ crate struct Cache { /// found on that implementation. crate impls: FxHashMap>, - /// Maintains a mapping of local crate `DefId`s to the fully qualified name - /// and "short type description" of that node. This is used when generating - /// URLs when a type is being linked to. External paths are not located in - /// this map because the `External` type itself has all the information - /// necessary. - crate paths: FxHashMap, ItemType)>, - - /// Similar to `paths`, but only holds external paths. This is only used for - /// generating explicit hyperlinks to other crates. - crate external_paths: FxHashMap, ItemType)>, + /// Maintain a mapping of `DefId`s to the fully qualified name and "shorty type description" of + /// that node. This map contains local and external cached paths that are differentiated using + /// the [`CachedPath`] enum. + /// + /// This was previously known as `extern_paths` and `paths`. + crate paths: FxHashMap, /// Maps local `DefId`s of exported types to fully qualified paths. /// Unlike 'paths', this mapping ignores any renames that occur @@ -156,7 +170,7 @@ impl Cache { 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, e.location(extern_url, &dst, tcx)); - self.external_paths.insert(did, (vec![name.to_string()], ItemType::Module)); + self.paths.insert(did, CachedPath::Extern(vec![name.to_string()], ItemType::Module)); } // Cache where all known primitives have their documentation located. @@ -267,15 +281,15 @@ impl<'a, 'tcx> DocFolder for CacheBuilder<'a, 'tcx> { // for where the type was defined. On the other // hand, `paths` always has the right // information if present. - Some(&( + Some(CachedPath::Local( ref fqp, ItemType::Trait | ItemType::Struct | ItemType::Union | ItemType::Enum, )) => Some(&fqp[..fqp.len() - 1]), - Some(..) => Some(&*self.cache.stack), - None => None, + Some(CachedPath::Local(..)) => Some(&*self.cache.stack), + _ => None, }; ((Some(*last), path), true) } @@ -353,15 +367,16 @@ impl<'a, 'tcx> DocFolder for CacheBuilder<'a, 'tcx> { { self.cache.paths.insert( item.def_id.expect_def_id(), - (self.cache.stack.clone(), item.type_()), + CachedPath::Local(self.cache.stack.clone(), item.type_()), ); } } } clean::PrimitiveItem(..) => { - self.cache - .paths - .insert(item.def_id.expect_def_id(), (self.cache.stack.clone(), item.type_())); + self.cache.paths.insert( + item.def_id.expect_def_id(), + CachedPath::Local(self.cache.stack.clone(), item.type_()), + ); } clean::ExternCrateItem { .. } diff --git a/src/librustdoc/html/format.rs b/src/librustdoc/html/format.rs index c08fe47696bf..5ee7fc1dbaa7 100644 --- a/src/librustdoc/html/format.rs +++ b/src/librustdoc/html/format.rs @@ -21,7 +21,7 @@ use rustc_target::spec::abi::Abi; use crate::clean::{ self, utils::find_nearest_parent_module, ExternalCrate, GetDefId, ItemId, PrimitiveType, }; -use crate::formats::item_type::ItemType; +use crate::formats::{cache::CachedPath, item_type::ItemType}; use crate::html::escape::Escape; use crate::html::render::cache::ExternalLocation; use crate::html::render::Context; @@ -483,13 +483,12 @@ crate fn href(did: DefId, cx: &Context<'_>) -> Option<(String, ItemType, Vec (fqp, shortty, { + let (fqp, shortty, mut url_parts) = match *cache.paths.get(&did)? { + CachedPath::Local(ref fqp, shortty) => (fqp, shortty, { let module_fqp = to_module_fqp(shortty, fqp); href_relative_parts(module_fqp, relative_to) }), - None => { - let &(ref fqp, shortty) = cache.external_paths.get(&did)?; + CachedPath::Extern(ref fqp, shortty) => { let module_fqp = to_module_fqp(shortty, fqp); ( fqp, diff --git a/src/librustdoc/html/render/cache.rs b/src/librustdoc/html/render/cache.rs index 0734d2670ccf..63ae57618aa4 100644 --- a/src/librustdoc/html/render/cache.rs +++ b/src/librustdoc/html/render/cache.rs @@ -9,7 +9,7 @@ use crate::clean; use crate::clean::types::{ FnDecl, FnRetTy, GenericBound, Generics, GetDefId, Type, WherePredicate, }; -use crate::formats::cache::Cache; +use crate::formats::cache::{Cache, CachedPath}; use crate::formats::item_type::ItemType; use crate::html::markdown::short_markdown_summary; use crate::html::render::{IndexItem, IndexItemFunctionType, RenderType, TypeWithKind}; @@ -33,7 +33,7 @@ crate fn build_index<'tcx>(krate: &clean::Crate, cache: &mut Cache, tcx: TyCtxt< // Attach all orphan items to the type's definition if the type // has since been learned. for &(did, ref item) in &cache.orphan_impl_items { - if let Some(&(ref fqp, _)) = cache.paths.get(&did) { + if let Some(CachedPath::Local(fqp, _)) = cache.paths.get(&did) { let desc = item .doc_value() .map_or_else(String::new, |s| short_markdown_summary(&s, &item.link_names(&cache))); @@ -90,7 +90,7 @@ crate fn build_index<'tcx>(krate: &clean::Crate, cache: &mut Cache, tcx: TyCtxt< defid_to_pathid.insert(defid, pathid); lastpathid += 1; - if let Some(&(ref fqp, short)) = paths.get(&defid) { + if let Some(&CachedPath::Local(ref fqp, short)) = paths.get(&defid) { crate_paths.push((short, fqp.last().unwrap().clone())); Some(pathid) } else { diff --git a/src/librustdoc/html/render/context.rs b/src/librustdoc/html/render/context.rs index 61057ff515b1..3808bdc59b9f 100644 --- a/src/librustdoc/html/render/context.rs +++ b/src/librustdoc/html/render/context.rs @@ -24,7 +24,7 @@ use crate::clean::ExternalCrate; use crate::config::RenderOptions; use crate::docfs::{DocFS, PathError}; use crate::error::Error; -use crate::formats::cache::Cache; +use crate::formats::cache::{Cache, CachedPath}; use crate::formats::item_type::ItemType; use crate::formats::FormatRenderer; use crate::html::escape::Escape; @@ -230,7 +230,9 @@ impl<'tcx> Context<'tcx> { &self.shared.style_files, ) } else { - if let Some(&(ref names, ty)) = self.cache.paths.get(&it.def_id.expect_def_id()) { + if let Some(&CachedPath::Local(ref names, ty)) = + self.cache.paths.get(&it.def_id.expect_def_id()) + { let mut path = String::new(); for name in &names[..names.len() - 1] { path.push_str(name); diff --git a/src/librustdoc/html/render/mod.rs b/src/librustdoc/html/render/mod.rs index 97ee682c11c4..079643e6970d 100644 --- a/src/librustdoc/html/render/mod.rs +++ b/src/librustdoc/html/render/mod.rs @@ -56,7 +56,7 @@ use serde::{Serialize, Serializer}; use crate::clean::{self, GetDefId, ItemId, RenderedLink, SelfTy}; use crate::docfs::PathError; use crate::error::Error; -use crate::formats::cache::Cache; +use crate::formats::cache::{Cache, CachedPath}; use crate::formats::item_type::ItemType; use crate::formats::{AssocItemRender, Impl, RenderMode}; use crate::html::escape::Escape; @@ -2339,7 +2339,12 @@ fn collect_paths_for_type(first_ty: clean::Type, cache: &Cache) -> Vec { match ty { clean::Type::ResolvedPath { did, .. } => { - let get_extern = || cache.external_paths.get(&did).map(|s| s.0.clone()); + let get_extern = || { + cache.paths.get(&did).and_then(|p| match p { + CachedPath::Extern(a, _) => Some(a.clone()), + CachedPath::Local(..) => None, + }) + }; let fqp = cache.exact_paths.get(&did).cloned().or_else(get_extern); if let Some(path) = fqp { diff --git a/src/librustdoc/html/render/print_item.rs b/src/librustdoc/html/render/print_item.rs index eeac9d1a9db7..f4e004605381 100644 --- a/src/librustdoc/html/render/print_item.rs +++ b/src/librustdoc/html/render/print_item.rs @@ -19,7 +19,7 @@ use super::{ Context, }; use crate::clean::{self, GetDefId}; -use crate::formats::item_type::ItemType; +use crate::formats::{cache::CachedPath, item_type::ItemType}; use crate::formats::{AssocItemRender, Impl, RenderMode}; use crate::html::escape::Escape; use crate::html::format::{ @@ -696,7 +696,7 @@ fn item_trait(w: &mut Buffer, cx: &Context<'_>, it: &clean::Item, t: &clean::Tra i.inner_impl() .for_ .def_id_full(cx.cache()) - .map_or(true, |d| cx.cache.paths.contains_key(&d)) + .map_or(true, |d| cx.cache.paths.get(&d).map_or(false, CachedPath::is_local)) }); let (mut synthetic, mut concrete): (Vec<&&Impl>, Vec<&&Impl>) = @@ -784,11 +784,9 @@ fn item_trait(w: &mut Buffer, cx: &Context<'_>, it: &clean::Item, t: &clean::Tra src=\"{root_path}/implementors/{path}/{ty}.{name}.js\" async>\ ", root_path = vec![".."; cx.current.len()].join("/"), - path = if it.def_id.is_local() { - cx.current.join("/") - } else { - let (ref path, _) = cx.cache.external_paths[&it.def_id.expect_def_id()]; - path[..path.len() - 1].join("/") + path = match cx.cache.paths[&it.def_id.expect_def_id()] { + CachedPath::Extern(_, _) => cx.current.join("/"), + CachedPath::Local(ref path, _) => path[..path.len() - 1].join("/"), }, ty = it.type_(), name = *it.name.as_ref().unwrap() diff --git a/src/librustdoc/html/render/write_shared.rs b/src/librustdoc/html/render/write_shared.rs index 94a902a2d052..037040cdba97 100644 --- a/src/librustdoc/html/render/write_shared.rs +++ b/src/librustdoc/html/render/write_shared.rs @@ -16,6 +16,7 @@ use crate::clean::Crate; use crate::config::{EmitType, RenderOptions}; use crate::docfs::PathError; use crate::error::Error; +use crate::formats::cache::CachedPath; use crate::html::{layout, static_files}; static FILES_UNVERSIONED: Lazy> = Lazy::new(|| { @@ -488,12 +489,9 @@ pub(super) fn write_shared( // // FIXME: this is a vague explanation for why this can't be a `get`, in // theory it should be... - let &(ref remote_path, remote_item_type) = match cx.cache.paths.get(&did) { - Some(p) => p, - None => match cx.cache.external_paths.get(&did) { - Some(p) => p, - None => continue, - }, + let (remote_path, remote_item_type) = match cx.cache.paths.get(&did) { + Some(&CachedPath::Local(ref a, b) | &CachedPath::Extern(ref a, b)) => (a, b), + None => continue, }; #[derive(Serialize)] @@ -528,7 +526,8 @@ pub(super) fn write_shared( // Only create a js file if we have impls to add to it. If the trait is // documented locally though we always create the file to avoid dead // links. - if implementors.is_empty() && !cx.cache.paths.contains_key(&did) { + if implementors.is_empty() && cx.cache.paths.get(&did).map_or(false, CachedPath::is_extern) + { continue; } diff --git a/src/librustdoc/json/mod.rs b/src/librustdoc/json/mod.rs index 8bdf1a598123..d589a58e67f2 100644 --- a/src/librustdoc/json/mod.rs +++ b/src/librustdoc/json/mod.rs @@ -22,7 +22,7 @@ use crate::clean; use crate::clean::ExternalCrate; use crate::config::RenderOptions; use crate::error::Error; -use crate::formats::cache::Cache; +use crate::formats::cache::{Cache, CachedPath}; use crate::formats::FormatRenderer; use crate::html::render::cache::ExternalLocation; use crate::json::conversions::{from_item_id, IntoWithTcx}; @@ -99,13 +99,10 @@ impl JsonRenderer<'tcx> { .cache .paths .get(&id) - .unwrap_or_else(|| { - self.cache - .external_paths - .get(&id) - .expect("Trait should either be in local or external paths") + .map(|p| match p { + CachedPath::Local(a, _) | CachedPath::Extern(a, _) => a, }) - .0 + .unwrap() .last() .map(Clone::clone), visibility: types::Visibility::Public, @@ -204,8 +201,7 @@ impl<'tcx> FormatRenderer<'tcx> for JsonRenderer<'tcx> { .paths .clone() .into_iter() - .chain(self.cache.external_paths.clone().into_iter()) - .map(|(k, (path, kind))| { + .map(|(k, CachedPath::Local(path, kind) | CachedPath::Extern(path, kind))| { ( from_item_id(k.into()), types::ItemSummary {