Skip to content

rustdoc: Replace String with Symbol in Cache.paths et al. #91876

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Closed
wants to merge 2 commits into from
Closed
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
10 changes: 4 additions & 6 deletions src/librustdoc/clean/inline.rs
Original file line number Diff line number Diff line change
Expand Up @@ -162,13 +162,11 @@ crate fn load_attrs<'hir>(cx: &DocContext<'hir>, did: DefId) -> Attrs<'hir> {
/// These names are used later on by HTML rendering to generate things like
/// source links back to the original item.
crate fn record_extern_fqn(cx: &mut DocContext<'_>, did: DefId, kind: ItemType) {
let crate_name = cx.tcx.crate_name(did.krate).to_string();
let crate_name = cx.tcx.crate_name(did.krate);

let relative = cx.tcx.def_path(did).data.into_iter().filter_map(|elem| {
// extern blocks have an empty name
let s = elem.data.to_string();
if !s.is_empty() { Some(s) } else { None }
});
// FIXME: use def_id_to_path instead
let relative =
cx.tcx.def_path(did).data.into_iter().filter_map(|elem| elem.data.get_opt_name());
let fqn = if let ItemType::Macro = kind {
// Check to see if it is a macro 2.0 or built-in macro
if matches!(
Expand Down
17 changes: 17 additions & 0 deletions src/librustdoc/clean/utils.rs
Original file line number Diff line number Diff line change
Expand Up @@ -536,3 +536,20 @@ pub(super) fn display_macro_source(
}
}
}

crate fn join_path_segments(segments: &[Symbol]) -> String {
join_path_segments_with_sep(segments, "::")
}

crate fn join_path_segments_with_sep(segments: &[Symbol], sep: &str) -> String {
let mut out = String::new();
// This can't use an iterator intersperse because then it would be borrowing from
// temporary SymbolStrs due to the lifetimes on SymbolStr's Deref impl.
for (i, s) in segments.iter().enumerate() {
if i > 0 {
out.push_str(sep);
}
out.push_str(&s.as_str())
}
out
}
23 changes: 12 additions & 11 deletions src/librustdoc/formats/cache.rs
Original file line number Diff line number Diff line change
Expand Up @@ -5,7 +5,9 @@ use rustc_hir::def_id::{CrateNum, DefId, CRATE_DEF_INDEX};
use rustc_middle::middle::privacy::AccessLevels;
use rustc_middle::ty::TyCtxt;
use rustc_span::symbol::sym;
use rustc_span::Symbol;

use crate::clean::utils::join_path_segments;
use crate::clean::{self, ExternalCrate, ItemId, PrimitiveType};
use crate::core::DocContext;
use crate::fold::DocFolder;
Expand Down Expand Up @@ -39,11 +41,11 @@ crate struct Cache {
/// 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<DefId, (Vec<String>, ItemType)>,
crate paths: FxHashMap<DefId, (Vec<Symbol>, ItemType)>,

/// Similar to `paths`, but only holds external paths. This is only used for
/// generating explicit hyperlinks to other crates.
crate external_paths: FxHashMap<DefId, (Vec<String>, ItemType)>,
crate external_paths: FxHashMap<DefId, (Vec<Symbol>, ItemType)>,

/// Maps local `DefId`s of exported types to fully qualified paths.
/// Unlike 'paths', this mapping ignores any renames that occur
Expand All @@ -55,7 +57,7 @@ crate struct Cache {
/// to the path used if the corresponding type is inlined. By
/// doing this, we can detect duplicate impls on a trait page, and only display
/// the impl for the inlined type.
crate exact_paths: FxHashMap<DefId, Vec<String>>,
crate exact_paths: FxHashMap<DefId, Vec<Symbol>>,

/// This map contains information about all known traits of this crate.
/// Implementations of a crate should inherit the documentation of the
Expand Down Expand Up @@ -92,7 +94,7 @@ crate struct Cache {
crate masked_crates: FxHashSet<CrateNum>,

// Private fields only used when initially crawling a crate to build a cache
stack: Vec<String>,
stack: Vec<Symbol>,
parent_stack: Vec<DefId>,
parent_is_trait_impl: bool,
stripped_mod: bool,
Expand Down Expand Up @@ -156,7 +158,7 @@ impl Cache {
let dst = &render_options.output;
let location = e.location(extern_url, extern_url_takes_precedence, dst, tcx);
cx.cache.extern_locations.insert(e.crate_num, location);
cx.cache.external_paths.insert(e.def_id(), (vec![name.to_string()], ItemType::Module));
cx.cache.external_paths.insert(e.def_id(), (vec![name], ItemType::Module));
}

// FIXME: avoid this clone (requires implementing Default manually)
Expand All @@ -165,10 +167,9 @@ impl Cache {
let crate_name = tcx.crate_name(def_id.krate);
// Recall that we only allow primitive modules to be at the root-level of the crate.
// If that restriction is ever lifted, this will have to include the relative paths instead.
cx.cache.external_paths.insert(
def_id,
(vec![crate_name.to_string(), prim.as_sym().to_string()], ItemType::Primitive),
);
cx.cache
.external_paths
.insert(def_id, (vec![crate_name, prim.as_sym()], ItemType::Primitive));
}

krate = CacheBuilder { tcx, cache: &mut cx.cache }.fold_crate(krate);
Expand Down Expand Up @@ -300,7 +301,7 @@ impl<'a, 'tcx> DocFolder for CacheBuilder<'a, 'tcx> {
self.cache.search_index.push(IndexItem {
ty: item.type_(),
name: s.to_string(),
path: path.join("::"),
path: join_path_segments(path),
desc,
parent,
parent_idx: None,
Expand All @@ -321,7 +322,7 @@ impl<'a, 'tcx> DocFolder for CacheBuilder<'a, 'tcx> {
// Keep track of the fully qualified path for this item.
let pushed = match item.name {
Some(n) if !n.is_empty() => {
self.cache.stack.push(n.to_string());
self.cache.stack.push(n);
true
}
_ => false,
Expand Down
48 changes: 27 additions & 21 deletions src/librustdoc/html/format.rs
Original file line number Diff line number Diff line change
Expand Up @@ -19,14 +19,18 @@ use rustc_middle::ty;
use rustc_middle::ty::DefIdTree;
use rustc_middle::ty::TyCtxt;
use rustc_span::def_id::CRATE_DEF_INDEX;
use rustc_span::Symbol;
use rustc_target::spec::abi::Abi;

use crate::clean::utils::join_path_segments;
use crate::clean::{self, utils::find_nearest_parent_module, ExternalCrate, ItemId, PrimitiveType};
use crate::formats::item_type::ItemType;
use crate::html::escape::Escape;
use crate::html::render::cache::ExternalLocation;
use crate::html::render::Context;

use super::url_parts_builder::UrlPartsBuilder;

crate trait Print {
fn print(self, buffer: &mut Buffer);
}
Expand Down Expand Up @@ -503,7 +507,7 @@ crate fn href_with_root_path(
did: DefId,
cx: &Context<'_>,
root_path: Option<&str>,
) -> Result<(String, ItemType, Vec<String>), HrefError> {
) -> Result<(String, ItemType, Vec<Symbol>), HrefError> {
let tcx = cx.tcx();
let def_kind = tcx.def_kind(did);
let did = match def_kind {
Expand All @@ -515,7 +519,7 @@ crate fn href_with_root_path(
};
let cache = cx.cache();
let relative_to = &cx.current;
fn to_module_fqp(shortty: ItemType, fqp: &[String]) -> &[String] {
fn to_module_fqp(shortty: ItemType, fqp: &[Symbol]) -> &[Symbol] {
if shortty == ItemType::Module { fqp } else { &fqp[..fqp.len() - 1] }
}

Expand Down Expand Up @@ -544,9 +548,9 @@ crate fn href_with_root_path(
ExternalLocation::Remote(ref s) => {
is_remote = true;
let s = s.trim_end_matches('/');
let mut s = vec![s];
s.extend(module_fqp[..].iter().map(String::as_str));
s
let mut builder = UrlPartsBuilder::singleton(s);
builder.extend(module_fqp.iter().copied());
builder
}
ExternalLocation::Local => href_relative_parts(module_fqp, relative_to),
ExternalLocation::Unknown => return Err(HrefError::DocumentationNotBuilt),
Expand All @@ -560,50 +564,52 @@ crate fn href_with_root_path(
if !is_remote {
if let Some(root_path) = root_path {
let root = root_path.trim_end_matches('/');
url_parts.insert(0, root);
url_parts.push_front(root);
}
}
debug!(?url_parts);
let last = &fqp.last().unwrap()[..];
let filename;
let last = &*fqp.last().unwrap().as_str();
match shortty {
ItemType::Module => {
url_parts.push("index.html");
}
_ => {
filename = format!("{}.{}.html", shortty.as_str(), last);
let filename = format!("{}.{}.html", shortty.as_str(), last);
url_parts.push(&filename);
}
}
Ok((url_parts.join("/"), shortty, fqp.to_vec()))
Ok((url_parts.finish(), shortty, fqp.to_vec()))
}

crate fn href(did: DefId, cx: &Context<'_>) -> Result<(String, ItemType, Vec<String>), HrefError> {
crate fn href(did: DefId, cx: &Context<'_>) -> Result<(String, ItemType, Vec<Symbol>), HrefError> {
href_with_root_path(did, cx, None)
}

/// Both paths should only be modules.
/// This is because modules get their own directories; that is, `std::vec` and `std::vec::Vec` will
/// both need `../iter/trait.Iterator.html` to get at the iterator trait.
crate fn href_relative_parts<'a>(fqp: &'a [String], relative_to_fqp: &'a [String]) -> Vec<&'a str> {
crate fn href_relative_parts(fqp: &[Symbol], relative_to_fqp: &[String]) -> UrlPartsBuilder {
for (i, (f, r)) in fqp.iter().zip(relative_to_fqp.iter()).enumerate() {
// e.g. linking to std::iter from std::vec (`dissimilar_part_count` will be 1)
if f != r {
if &*f.as_str() != r {
let dissimilar_part_count = relative_to_fqp.len() - i;
let fqp_module = fqp[i..fqp.len()].iter().map(String::as_str);
return iter::repeat("..").take(dissimilar_part_count).chain(fqp_module).collect();
let fqp_module = &fqp[i..fqp.len()];
let mut builder: UrlPartsBuilder =
iter::repeat("..").take(dissimilar_part_count).collect();
builder.extend(fqp_module.iter().copied());
return builder;
}
}
// e.g. linking to std::sync::atomic from std::sync
if relative_to_fqp.len() < fqp.len() {
fqp[relative_to_fqp.len()..fqp.len()].iter().map(String::as_str).collect()
fqp[relative_to_fqp.len()..fqp.len()].iter().copied().collect()
// e.g. linking to std::sync from std::sync::atomic
} else if fqp.len() < relative_to_fqp.len() {
let dissimilar_part_count = relative_to_fqp.len() - fqp.len();
iter::repeat("..").take(dissimilar_part_count).collect()
// linking to the same module
} else {
Vec::new()
UrlPartsBuilder::new()
}
}

Expand All @@ -630,8 +636,8 @@ fn resolved_path<'cx>(
if let Ok((_, _, fqp)) = href(did, cx) {
format!(
"{}::{}",
fqp[..fqp.len() - 1].join("::"),
anchor(did, fqp.last().unwrap(), cx)
join_path_segments(&fqp[..fqp.len() - 1]),
anchor(did, &fqp.last().unwrap().as_str(), cx)
)
} else {
last.name.to_string()
Expand Down Expand Up @@ -742,7 +748,7 @@ crate fn anchor<'a, 'cx: 'a>(
short_ty,
url,
short_ty,
fqp.join("::"),
join_path_segments(&fqp),
text
)
} else {
Expand Down Expand Up @@ -960,7 +966,7 @@ fn fmt_type<'cx>(
url = url,
shortty = ItemType::AssocType,
name = name,
path = path.join("::")
path = join_path_segments(path),
)?;
}
_ => write!(f, "{}", name)?,
Expand Down
1 change: 1 addition & 0 deletions src/librustdoc/html/mod.rs
Original file line number Diff line number Diff line change
Expand Up @@ -9,6 +9,7 @@ crate mod render;
crate mod sources;
crate mod static_files;
crate mod toc;
mod url_parts_builder;

#[cfg(test)]
mod tests;
5 changes: 3 additions & 2 deletions src/librustdoc/html/render/cache.rs
Original file line number Diff line number Diff line change
Expand Up @@ -8,6 +8,7 @@ use serde::ser::{Serialize, SerializeStruct, Serializer};

use crate::clean;
use crate::clean::types::{FnDecl, FnRetTy, GenericBound, Generics, Type, WherePredicate};
use crate::clean::utils::join_path_segments;
use crate::formats::cache::Cache;
use crate::formats::item_type::ItemType;
use crate::html::markdown::short_markdown_summary;
Expand Down Expand Up @@ -38,7 +39,7 @@ crate fn build_index<'tcx>(krate: &clean::Crate, cache: &mut Cache, tcx: TyCtxt<
cache.search_index.push(IndexItem {
ty: item.type_(),
name: item.name.unwrap().to_string(),
path: fqp[..fqp.len() - 1].join("::"),
path: join_path_segments(&fqp[..fqp.len() - 1]),
desc,
parent: Some(did),
parent_idx: None,
Expand Down Expand Up @@ -90,7 +91,7 @@ crate fn build_index<'tcx>(krate: &clean::Crate, cache: &mut Cache, tcx: TyCtxt<
lastpathid += 1;

if let Some(&(ref fqp, short)) = paths.get(&defid) {
crate_paths.push((short, fqp.last().unwrap().clone()));
crate_paths.push((short, fqp.last().unwrap().to_string()));
Some(pathid)
} else {
None
Expand Down
6 changes: 3 additions & 3 deletions src/librustdoc/html/render/context.rs
Original file line number Diff line number Diff line change
Expand Up @@ -237,18 +237,18 @@ impl<'tcx> Context<'tcx> {
if let Some(&(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);
path.push_str(&name.as_str());
path.push('/');
}
path.push_str(&item_path(ty, names.last().unwrap()));
path.push_str(&item_path(ty, &names.last().unwrap().as_str()));
match self.shared.redirections {
Some(ref redirections) => {
let mut current_path = String::new();
for name in &self.current {
current_path.push_str(name);
current_path.push('/');
}
current_path.push_str(&item_path(ty, names.last().unwrap()));
current_path.push_str(&item_path(ty, &names.last().unwrap().as_str()));
redirections.borrow_mut().insert(current_path, path);
}
None => return layout::redirect(&format!("{}{}", self.root_path(), path)),
Expand Down
3 changes: 2 additions & 1 deletion src/librustdoc/html/render/mod.rs
Original file line number Diff line number Diff line change
Expand Up @@ -63,6 +63,7 @@ use rustc_span::{
use serde::ser::SerializeSeq;
use serde::{Serialize, Serializer};

use crate::clean::utils::join_path_segments;
use crate::clean::{self, ItemId, RenderedLink, SelfTy};
use crate::error::Error;
use crate::formats::cache::Cache;
Expand Down Expand Up @@ -2524,7 +2525,7 @@ fn collect_paths_for_type(first_ty: clean::Type, cache: &Cache) -> Vec<String> {
let fqp = cache.exact_paths.get(&did).cloned().or_else(get_extern);

if let Some(path) = fqp {
out.push(path.join("::"));
out.push(join_path_segments(&path));
}
};

Expand Down
3 changes: 2 additions & 1 deletion src/librustdoc/html/render/print_item.rs
Original file line number Diff line number Diff line change
Expand Up @@ -22,6 +22,7 @@ use super::{
ImplRenderingParameters,
};
use crate::clean;
use crate::clean::utils::join_path_segments_with_sep;
use crate::formats::item_type::ItemType;
use crate::formats::{AssocItemRender, Impl, RenderMode};
use crate::html::escape::Escape;
Expand Down Expand Up @@ -874,7 +875,7 @@ fn item_trait(w: &mut Buffer, cx: &Context<'_>, it: &clean::Item, t: &clean::Tra
cx.current.join("/")
} else {
let (ref path, _) = cache.external_paths[&it.def_id.expect_def_id()];
path[..path.len() - 1].join("/")
join_path_segments_with_sep(&path[..path.len() - 1], "/")
},
ty = it.type_(),
name = *it.name.as_ref().unwrap()
Expand Down
2 changes: 1 addition & 1 deletion src/librustdoc/html/render/write_shared.rs
Original file line number Diff line number Diff line change
Expand Up @@ -569,7 +569,7 @@ pub(super) fn write_shared(

let mut mydst = dst.clone();
for part in &remote_path[..remote_path.len() - 1] {
mydst.push(part);
mydst.push(&*part.as_str());
}
cx.shared.ensure_dir(&mydst)?;
mydst.push(&format!("{}.{}.js", remote_item_type, remote_path[remote_path.len() - 1]));
Expand Down
Loading