Skip to content

Commit 3fe671d

Browse files
committed
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.
1 parent d04ec47 commit 3fe671d

File tree

9 files changed

+69
-55
lines changed

9 files changed

+69
-55
lines changed

src/librustdoc/clean/inline.rs

+2-2
Original file line numberDiff line numberDiff line change
@@ -18,7 +18,7 @@ use crate::clean::{
1818
self, utils, Attributes, AttributesExt, GetDefId, ItemId, NestedAttributesExt, Type,
1919
};
2020
use crate::core::DocContext;
21-
use crate::formats::item_type::ItemType;
21+
use crate::formats::{cache::CachedPath, item_type::ItemType};
2222

2323
use super::{Clean, Visibility};
2424

@@ -189,7 +189,7 @@ crate fn record_extern_fqn(cx: &mut DocContext<'_>, did: DefId, kind: ItemType)
189189
if did.is_local() {
190190
cx.cache.exact_paths.insert(did, fqn);
191191
} else {
192-
cx.cache.external_paths.insert(did, (fqn, kind));
192+
cx.cache.paths.insert(did, CachedPath::Extern(fqn, kind));
193193
}
194194
}
195195

src/librustdoc/formats/cache.rs

+33-18
Original file line numberDiff line numberDiff line change
@@ -16,6 +16,24 @@ use crate::html::markdown::short_markdown_summary;
1616
use crate::html::render::cache::{get_index_search_type, ExternalLocation};
1717
use crate::html::render::IndexItem;
1818

19+
/// A path that is either local, or external. It consists of a fully qualified name,
20+
/// and a type description.
21+
#[derive(Debug, Clone)]
22+
crate enum CachedPath {
23+
Local(Vec<String>, ItemType),
24+
Extern(Vec<String>, ItemType),
25+
}
26+
27+
impl CachedPath {
28+
crate fn is_local(&self) -> bool {
29+
matches!(self, CachedPath::Local(..))
30+
}
31+
32+
crate fn is_extern(&self) -> bool {
33+
matches!(self, CachedPath::Extern(..))
34+
}
35+
}
36+
1937
/// This cache is used to store information about the [`clean::Crate`] being
2038
/// rendered in order to provide more useful documentation. This contains
2139
/// information like all implementors of a trait, all traits a type implements,
@@ -35,16 +53,12 @@ crate struct Cache {
3553
/// found on that implementation.
3654
crate impls: FxHashMap<DefId, Vec<Impl>>,
3755

38-
/// Maintains a mapping of local crate `DefId`s to the fully qualified name
39-
/// and "short type description" of that node. This is used when generating
40-
/// URLs when a type is being linked to. External paths are not located in
41-
/// this map because the `External` type itself has all the information
42-
/// necessary.
43-
crate paths: FxHashMap<DefId, (Vec<String>, ItemType)>,
44-
45-
/// Similar to `paths`, but only holds external paths. This is only used for
46-
/// generating explicit hyperlinks to other crates.
47-
crate external_paths: FxHashMap<DefId, (Vec<String>, ItemType)>,
56+
/// Maintain a mapping of `DefId`s to the fully qualified name and "shorty type description" of
57+
/// that node. This map contains local and external cached paths that are differentiated using
58+
/// the [`CachedPath`] enum.
59+
///
60+
/// This was previously known as `extern_paths` and `paths`.
61+
crate paths: FxHashMap<DefId, CachedPath>,
4862

4963
/// Maps local `DefId`s of exported types to fully qualified paths.
5064
/// Unlike 'paths', this mapping ignores any renames that occur
@@ -156,7 +170,7 @@ impl Cache {
156170
let extern_url = extern_html_root_urls.get(&*name.as_str()).map(|u| &**u);
157171
let did = DefId { krate: n, index: CRATE_DEF_INDEX };
158172
self.extern_locations.insert(n, e.location(extern_url, &dst, tcx));
159-
self.external_paths.insert(did, (vec![name.to_string()], ItemType::Module));
173+
self.paths.insert(did, CachedPath::Extern(vec![name.to_string()], ItemType::Module));
160174
}
161175

162176
// Cache where all known primitives have their documentation located.
@@ -267,15 +281,15 @@ impl<'a, 'tcx> DocFolder for CacheBuilder<'a, 'tcx> {
267281
// for where the type was defined. On the other
268282
// hand, `paths` always has the right
269283
// information if present.
270-
Some(&(
284+
Some(CachedPath::Local(
271285
ref fqp,
272286
ItemType::Trait
273287
| ItemType::Struct
274288
| ItemType::Union
275289
| ItemType::Enum,
276290
)) => Some(&fqp[..fqp.len() - 1]),
277-
Some(..) => Some(&*self.cache.stack),
278-
None => None,
291+
Some(CachedPath::Local(..)) => Some(&*self.cache.stack),
292+
_ => None,
279293
};
280294
((Some(*last), path), true)
281295
}
@@ -353,15 +367,16 @@ impl<'a, 'tcx> DocFolder for CacheBuilder<'a, 'tcx> {
353367
{
354368
self.cache.paths.insert(
355369
item.def_id.expect_def_id(),
356-
(self.cache.stack.clone(), item.type_()),
370+
CachedPath::Local(self.cache.stack.clone(), item.type_()),
357371
);
358372
}
359373
}
360374
}
361375
clean::PrimitiveItem(..) => {
362-
self.cache
363-
.paths
364-
.insert(item.def_id.expect_def_id(), (self.cache.stack.clone(), item.type_()));
376+
self.cache.paths.insert(
377+
item.def_id.expect_def_id(),
378+
CachedPath::Local(self.cache.stack.clone(), item.type_()),
379+
);
365380
}
366381

367382
clean::ExternCrateItem { .. }

src/librustdoc/html/format.rs

+4-5
Original file line numberDiff line numberDiff line change
@@ -21,7 +21,7 @@ use rustc_target::spec::abi::Abi;
2121
use crate::clean::{
2222
self, utils::find_nearest_parent_module, ExternalCrate, GetDefId, ItemId, PrimitiveType,
2323
};
24-
use crate::formats::item_type::ItemType;
24+
use crate::formats::{cache::CachedPath, item_type::ItemType};
2525
use crate::html::escape::Escape;
2626
use crate::html::render::cache::ExternalLocation;
2727
use crate::html::render::Context;
@@ -483,13 +483,12 @@ crate fn href(did: DefId, cx: &Context<'_>) -> Option<(String, ItemType, Vec<Str
483483
return None;
484484
}
485485

486-
let (fqp, shortty, mut url_parts) = match cache.paths.get(&did) {
487-
Some(&(ref fqp, shortty)) => (fqp, shortty, {
486+
let (fqp, shortty, mut url_parts) = match *cache.paths.get(&did)? {
487+
CachedPath::Local(ref fqp, shortty) => (fqp, shortty, {
488488
let module_fqp = to_module_fqp(shortty, fqp);
489489
href_relative_parts(module_fqp, relative_to)
490490
}),
491-
None => {
492-
let &(ref fqp, shortty) = cache.external_paths.get(&did)?;
491+
CachedPath::Extern(ref fqp, shortty) => {
493492
let module_fqp = to_module_fqp(shortty, fqp);
494493
(
495494
fqp,

src/librustdoc/html/render/cache.rs

+3-3
Original file line numberDiff line numberDiff line change
@@ -9,7 +9,7 @@ use crate::clean;
99
use crate::clean::types::{
1010
FnDecl, FnRetTy, GenericBound, Generics, GetDefId, Type, WherePredicate,
1111
};
12-
use crate::formats::cache::Cache;
12+
use crate::formats::cache::{Cache, CachedPath};
1313
use crate::formats::item_type::ItemType;
1414
use crate::html::markdown::short_markdown_summary;
1515
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<
3333
// Attach all orphan items to the type's definition if the type
3434
// has since been learned.
3535
for &(did, ref item) in &cache.orphan_impl_items {
36-
if let Some(&(ref fqp, _)) = cache.paths.get(&did) {
36+
if let Some(CachedPath::Local(fqp, _)) = cache.paths.get(&did) {
3737
let desc = item
3838
.doc_value()
3939
.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<
9090
defid_to_pathid.insert(defid, pathid);
9191
lastpathid += 1;
9292

93-
if let Some(&(ref fqp, short)) = paths.get(&defid) {
93+
if let Some(&CachedPath::Local(ref fqp, short)) = paths.get(&defid) {
9494
crate_paths.push((short, fqp.last().unwrap().clone()));
9595
Some(pathid)
9696
} else {

src/librustdoc/html/render/context.rs

+4-2
Original file line numberDiff line numberDiff line change
@@ -24,7 +24,7 @@ use crate::clean::ExternalCrate;
2424
use crate::config::RenderOptions;
2525
use crate::docfs::{DocFS, PathError};
2626
use crate::error::Error;
27-
use crate::formats::cache::Cache;
27+
use crate::formats::cache::{Cache, CachedPath};
2828
use crate::formats::item_type::ItemType;
2929
use crate::formats::FormatRenderer;
3030
use crate::html::escape::Escape;
@@ -230,7 +230,9 @@ impl<'tcx> Context<'tcx> {
230230
&self.shared.style_files,
231231
)
232232
} else {
233-
if let Some(&(ref names, ty)) = self.cache.paths.get(&it.def_id.expect_def_id()) {
233+
if let Some(&CachedPath::Local(ref names, ty)) =
234+
self.cache.paths.get(&it.def_id.expect_def_id())
235+
{
234236
let mut path = String::new();
235237
for name in &names[..names.len() - 1] {
236238
path.push_str(name);

src/librustdoc/html/render/mod.rs

+7-2
Original file line numberDiff line numberDiff line change
@@ -56,7 +56,7 @@ use serde::{Serialize, Serializer};
5656
use crate::clean::{self, GetDefId, ItemId, RenderedLink, SelfTy};
5757
use crate::docfs::PathError;
5858
use crate::error::Error;
59-
use crate::formats::cache::Cache;
59+
use crate::formats::cache::{Cache, CachedPath};
6060
use crate::formats::item_type::ItemType;
6161
use crate::formats::{AssocItemRender, Impl, RenderMode};
6262
use crate::html::escape::Escape;
@@ -2339,7 +2339,12 @@ fn collect_paths_for_type(first_ty: clean::Type, cache: &Cache) -> Vec<String> {
23392339

23402340
match ty {
23412341
clean::Type::ResolvedPath { did, .. } => {
2342-
let get_extern = || cache.external_paths.get(&did).map(|s| s.0.clone());
2342+
let get_extern = || {
2343+
cache.paths.get(&did).and_then(|p| match p {
2344+
CachedPath::Extern(a, _) => Some(a.clone()),
2345+
CachedPath::Local(..) => None,
2346+
})
2347+
};
23432348
let fqp = cache.exact_paths.get(&did).cloned().or_else(get_extern);
23442349

23452350
if let Some(path) = fqp {

src/librustdoc/html/render/print_item.rs

+5-7
Original file line numberDiff line numberDiff line change
@@ -19,7 +19,7 @@ use super::{
1919
Context,
2020
};
2121
use crate::clean::{self, GetDefId};
22-
use crate::formats::item_type::ItemType;
22+
use crate::formats::{cache::CachedPath, item_type::ItemType};
2323
use crate::formats::{AssocItemRender, Impl, RenderMode};
2424
use crate::html::escape::Escape;
2525
use crate::html::format::{
@@ -696,7 +696,7 @@ fn item_trait(w: &mut Buffer, cx: &Context<'_>, it: &clean::Item, t: &clean::Tra
696696
i.inner_impl()
697697
.for_
698698
.def_id_full(cx.cache())
699-
.map_or(true, |d| cx.cache.paths.contains_key(&d))
699+
.map_or(true, |d| cx.cache.paths.get(&d).map_or(false, CachedPath::is_local))
700700
});
701701

702702
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
784784
src=\"{root_path}/implementors/{path}/{ty}.{name}.js\" async>\
785785
</script>",
786786
root_path = vec![".."; cx.current.len()].join("/"),
787-
path = if it.def_id.is_local() {
788-
cx.current.join("/")
789-
} else {
790-
let (ref path, _) = cx.cache.external_paths[&it.def_id.expect_def_id()];
791-
path[..path.len() - 1].join("/")
787+
path = match cx.cache.paths[&it.def_id.expect_def_id()] {
788+
CachedPath::Extern(_, _) => cx.current.join("/"),
789+
CachedPath::Local(ref path, _) => path[..path.len() - 1].join("/"),
792790
},
793791
ty = it.type_(),
794792
name = *it.name.as_ref().unwrap()

src/librustdoc/html/render/write_shared.rs

+6-7
Original file line numberDiff line numberDiff line change
@@ -16,6 +16,7 @@ use crate::clean::Crate;
1616
use crate::config::{EmitType, RenderOptions};
1717
use crate::docfs::PathError;
1818
use crate::error::Error;
19+
use crate::formats::cache::CachedPath;
1920
use crate::html::{layout, static_files};
2021

2122
static FILES_UNVERSIONED: Lazy<FxHashMap<&str, &[u8]>> = Lazy::new(|| {
@@ -488,12 +489,9 @@ pub(super) fn write_shared(
488489
//
489490
// FIXME: this is a vague explanation for why this can't be a `get`, in
490491
// theory it should be...
491-
let &(ref remote_path, remote_item_type) = match cx.cache.paths.get(&did) {
492-
Some(p) => p,
493-
None => match cx.cache.external_paths.get(&did) {
494-
Some(p) => p,
495-
None => continue,
496-
},
492+
let (remote_path, remote_item_type) = match cx.cache.paths.get(&did) {
493+
Some(&CachedPath::Local(ref a, b) | &CachedPath::Extern(ref a, b)) => (a, b),
494+
None => continue,
497495
};
498496

499497
#[derive(Serialize)]
@@ -528,7 +526,8 @@ pub(super) fn write_shared(
528526
// Only create a js file if we have impls to add to it. If the trait is
529527
// documented locally though we always create the file to avoid dead
530528
// links.
531-
if implementors.is_empty() && !cx.cache.paths.contains_key(&did) {
529+
if implementors.is_empty() && cx.cache.paths.get(&did).map_or(false, CachedPath::is_extern)
530+
{
532531
continue;
533532
}
534533

src/librustdoc/json/mod.rs

+5-9
Original file line numberDiff line numberDiff line change
@@ -22,7 +22,7 @@ use crate::clean;
2222
use crate::clean::ExternalCrate;
2323
use crate::config::RenderOptions;
2424
use crate::error::Error;
25-
use crate::formats::cache::Cache;
25+
use crate::formats::cache::{Cache, CachedPath};
2626
use crate::formats::FormatRenderer;
2727
use crate::html::render::cache::ExternalLocation;
2828
use crate::json::conversions::{from_item_id, IntoWithTcx};
@@ -99,13 +99,10 @@ impl JsonRenderer<'tcx> {
9999
.cache
100100
.paths
101101
.get(&id)
102-
.unwrap_or_else(|| {
103-
self.cache
104-
.external_paths
105-
.get(&id)
106-
.expect("Trait should either be in local or external paths")
102+
.map(|p| match p {
103+
CachedPath::Local(a, _) | CachedPath::Extern(a, _) => a,
107104
})
108-
.0
105+
.unwrap()
109106
.last()
110107
.map(Clone::clone),
111108
visibility: types::Visibility::Public,
@@ -204,8 +201,7 @@ impl<'tcx> FormatRenderer<'tcx> for JsonRenderer<'tcx> {
204201
.paths
205202
.clone()
206203
.into_iter()
207-
.chain(self.cache.external_paths.clone().into_iter())
208-
.map(|(k, (path, kind))| {
204+
.map(|(k, CachedPath::Local(path, kind) | CachedPath::Extern(path, kind))| {
209205
(
210206
from_item_id(k.into()),
211207
types::ItemSummary {

0 commit comments

Comments
 (0)