Auto merge of #82356 - camelid:render-cleanup, r=GuillaumeGomez

rustdoc: Cleanup `html::render::Context`

- Move most shared fields to `SharedContext` (except for `cache`, which
  isn't mutated anyway)
- Replace a use of `Arc` with `Rc`
- Make a bunch of fields private
- Add static size assertion for `Context`
- Don't share `id_map` and `deref_id_map`
This commit is contained in:
bors 2021-03-09 04:33:43 +00:00
commit 4b9f5cc4c1
6 changed files with 84 additions and 60 deletions

View File

@ -9,7 +9,7 @@
/// 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
/// module, and cleanup/finalizing output.
crate trait FormatRenderer<'tcx>: Clone {
crate trait FormatRenderer<'tcx>: Sized {
/// Gives a description of the renderer. Used for performance profiling.
fn descr() -> &'static str;
@ -23,6 +23,9 @@ fn init(
tcx: TyCtxt<'tcx>,
) -> Result<(Self, clean::Crate), Error>;
/// Make a new renderer to render a child of the item currently being rendered.
fn make_child_renderer(&self) -> Self;
/// Renders a single non-module item. This means no recursive sub-item rendering is required.
fn item(&mut self, item: clean::Item) -> Result<(), Error>;
@ -67,7 +70,7 @@ fn after_krate(
item.name = Some(krate.name);
// Render the crate documentation
let mut work = vec![(format_renderer.clone(), item)];
let mut work = vec![(format_renderer.make_child_renderer(), item)];
let unknown = rustc_span::Symbol::intern("<unknown item>");
while let Some((mut cx, item)) = work.pop() {
@ -87,7 +90,7 @@ fn after_krate(
};
for it in module.items {
debug!("Adding {:?} to worklist", it.name);
work.push((cx.clone(), it));
work.push((cx.make_child_renderer(), it));
}
cx.mod_item_out(&name)?;

View File

@ -1373,10 +1373,6 @@ pub fn new() -> Self {
}
}
crate fn reset(&mut self) {
self.map = init_id_map();
}
crate fn derive<S: AsRef<str> + ToString>(&mut self, candidate: S) -> String {
let id = match self.map.get_mut(candidate.as_ref()) {
None => candidate.to_string(),

View File

@ -1,7 +1,6 @@
use super::{plain_text_summary, short_markdown_summary};
use super::{ErrorCodes, IdMap, Ignore, LangString, Markdown, MarkdownHtml};
use rustc_span::edition::{Edition, DEFAULT_EDITION};
use std::cell::RefCell;
#[test]
fn test_unique_id() {
@ -38,15 +37,9 @@ fn test_unique_id() {
"assoc_type.Item-1",
];
let map = RefCell::new(IdMap::new());
let test = || {
let mut map = map.borrow_mut();
let actual: Vec<String> = input.iter().map(|s| map.derive(s.to_string())).collect();
assert_eq!(&actual[..], expected);
};
test();
map.borrow_mut().reset();
test();
let mut map = IdMap::new();
let actual: Vec<String> = input.iter().map(|s| map.derive(s.to_string())).collect();
assert_eq!(&actual[..], expected);
}
#[test]

View File

@ -3,8 +3,7 @@
use std::io;
use std::path::PathBuf;
use std::rc::Rc;
use std::sync::mpsc::{channel, Receiver};
use std::sync::Arc;
use std::sync::mpsc::channel;
use rustc_data_structures::fx::FxHashMap;
use rustc_hir::def_id::{DefId, LOCAL_CRATE};
@ -41,35 +40,44 @@
/// It is intended that this context is a lightweight object which can be fairly
/// easily cloned because it is cloned per work-job (about once per item in the
/// rustdoc tree).
#[derive(Clone)]
crate struct Context<'tcx> {
/// Current hierarchy of components leading down to what's currently being
/// rendered
crate current: Vec<String>,
pub(super) current: Vec<String>,
/// The current destination folder of where HTML artifacts should be placed.
/// This changes as the context descends into the module hierarchy.
crate dst: PathBuf,
pub(super) dst: PathBuf,
/// A flag, which when `true`, will render pages which redirect to the
/// real location of an item. This is used to allow external links to
/// publicly reused items to redirect to the right location.
crate render_redirect_pages: bool,
/// `None` by default, depends on the `generate-redirect-map` option flag. If this field is set
/// to `Some(...)`, it'll store redirections and then generate a JSON file at the top level of
/// the crate.
crate redirections: Option<Rc<RefCell<FxHashMap<String, String>>>>,
pub(super) render_redirect_pages: bool,
/// The map used to ensure all generated 'id=' attributes are unique.
pub(super) id_map: Rc<RefCell<IdMap>>,
pub(super) id_map: RefCell<IdMap>,
/// Tracks section IDs for `Deref` targets so they match in both the main
/// body and the sidebar.
pub(super) deref_id_map: Rc<RefCell<FxHashMap<DefId, String>>>,
crate shared: Arc<SharedContext<'tcx>>,
all: Rc<RefCell<AllTypes>>,
/// Storage for the errors produced while generating documentation so they
/// can be printed together at the end.
crate errors: Rc<Receiver<String>>,
crate cache: Rc<Cache>,
pub(super) deref_id_map: RefCell<FxHashMap<DefId, String>>,
/// Shared mutable state.
///
/// Issue for improving the situation: [#82381][]
///
/// [#82381]: https://github.com/rust-lang/rust/issues/82381
pub(super) shared: Rc<SharedContext<'tcx>>,
/// The [`Cache`] used during rendering.
///
/// Ideally the cache would be in [`SharedContext`], but it's mutated
/// between when the `SharedContext` is created and when `Context`
/// is created, so more refactoring would be needed.
///
/// It's immutable once in `Context`, so it's not as bad that it's not in
/// `SharedContext`.
// FIXME: move `cache` to `SharedContext`
pub(super) cache: Rc<Cache>,
}
// `Context` is cloned a lot, so we don't want the size to grow unexpectedly.
#[cfg(target_arch = "x86_64")]
rustc_data_structures::static_assert_size!(Context<'_>, 152);
impl<'tcx> Context<'tcx> {
pub(super) fn path(&self, filename: &str) -> PathBuf {
// We use splitn vs Path::extension here because we might get a filename
@ -148,11 +156,6 @@ fn render_item(&self, it: &clean::Item, pushname: bool) -> String {
static_extra_scripts: &[],
};
{
self.id_map.borrow_mut().reset();
self.id_map.borrow_mut().populate(&INITIAL_IDS);
}
if !self.render_redirect_pages {
layout::render(
&self.shared.layout,
@ -169,7 +172,7 @@ fn render_item(&self, it: &clean::Item, pushname: bool) -> String {
path.push('/');
}
path.push_str(&item_path(ty, names.last().unwrap()));
match self.redirections {
match self.shared.redirections {
Some(ref redirections) => {
let mut current_path = String::new();
for name in &self.current {
@ -383,6 +386,9 @@ fn init(
edition,
codes: ErrorCodes::from(unstable_features.is_nightly_build()),
playground,
all: RefCell::new(AllTypes::new()),
errors: receiver,
redirections: if generate_redirect_map { Some(Default::default()) } else { None },
};
// Add the default themes to the `Vec` of stylepaths
@ -409,24 +415,36 @@ fn init(
current: Vec::new(),
dst,
render_redirect_pages: false,
id_map: Rc::new(RefCell::new(id_map)),
deref_id_map: Rc::new(RefCell::new(FxHashMap::default())),
shared: Arc::new(scx),
all: Rc::new(RefCell::new(AllTypes::new())),
errors: Rc::new(receiver),
id_map: RefCell::new(id_map),
deref_id_map: RefCell::new(FxHashMap::default()),
shared: Rc::new(scx),
cache: Rc::new(cache),
redirections: if generate_redirect_map { Some(Default::default()) } else { None },
};
CURRENT_DEPTH.with(|s| s.set(0));
// Write shared runs within a flock; disable thread dispatching of IO temporarily.
Arc::get_mut(&mut cx.shared).unwrap().fs.set_sync_only(true);
Rc::get_mut(&mut cx.shared).unwrap().fs.set_sync_only(true);
write_shared(&cx, &krate, index, &md_opts)?;
Arc::get_mut(&mut cx.shared).unwrap().fs.set_sync_only(false);
Rc::get_mut(&mut cx.shared).unwrap().fs.set_sync_only(false);
Ok((cx, krate))
}
fn make_child_renderer(&self) -> Self {
let mut id_map = IdMap::new();
id_map.populate(&INITIAL_IDS);
Self {
current: self.current.clone(),
dst: self.dst.clone(),
render_redirect_pages: self.render_redirect_pages,
id_map: RefCell::new(id_map),
deref_id_map: RefCell::new(FxHashMap::default()),
shared: Rc::clone(&self.shared),
cache: Rc::clone(&self.cache),
}
}
fn after_krate(
&mut self,
krate: &clean::Crate,
@ -464,7 +482,7 @@ fn after_krate(
} else {
String::new()
};
let all = self.all.replace(AllTypes::new());
let all = self.shared.all.replace(AllTypes::new());
let v = layout::render(
&self.shared.layout,
&page,
@ -494,7 +512,7 @@ fn after_krate(
&style_files,
);
self.shared.fs.write(&settings_file, v.as_bytes())?;
if let Some(redirections) = self.redirections.take() {
if let Some(ref redirections) = self.shared.redirections {
if !redirections.borrow().is_empty() {
let redirect_map_path =
self.dst.join(&*krate.name.as_str()).join("redirect-map.json");
@ -505,8 +523,8 @@ fn after_krate(
}
// Flush pending errors.
Arc::get_mut(&mut self.shared).unwrap().fs.close();
let nb_errors = self.errors.iter().map(|err| diag.struct_err(&err).emit()).count();
Rc::get_mut(&mut self.shared).unwrap().fs.close();
let nb_errors = self.shared.errors.iter().map(|err| diag.struct_err(&err).emit()).count();
if nb_errors > 0 {
Err(Error::new(io::Error::new(io::ErrorKind::Other, "I/O error"), ""))
} else {
@ -585,13 +603,13 @@ fn item(&mut self, item: clean::Item) -> Result<(), Error> {
self.shared.fs.write(&joint_dst, buf.as_bytes())?;
if !self.render_redirect_pages {
self.all.borrow_mut().append(full_path(self, &item), &item_type);
self.shared.all.borrow_mut().append(full_path(self, &item), &item_type);
}
// If the item is a macro, redirect from the old macro URL (with !)
// to the new one (without).
if item_type == ItemType::Macro {
let redir_name = format!("{}.{}!.html", item_type, name);
if let Some(ref redirections) = self.redirections {
if let Some(ref redirections) = self.shared.redirections {
let crate_name = &self.shared.layout.krate;
redirections.borrow_mut().insert(
format!("{}/{}", crate_name, redir_name),

View File

@ -42,6 +42,7 @@
use std::path::{Path, PathBuf};
use std::str;
use std::string::ToString;
use std::sync::mpsc::Receiver;
use itertools::Itertools;
use rustc_ast_pretty::pprust;
@ -81,6 +82,7 @@
})
}
/// Shared mutable state used in [`Context`] and elsewhere.
crate struct SharedContext<'tcx> {
crate tcx: TyCtxt<'tcx>,
/// The path to the crate root source minus the file name.
@ -96,16 +98,16 @@
/// The local file sources we've emitted and their respective url-paths.
crate local_sources: FxHashMap<PathBuf, String>,
/// Whether the collapsed pass ran
crate collapsed: bool,
collapsed: bool,
/// The base-URL of the issue tracker for when an item has been tagged with
/// an issue number.
crate issue_tracker_base_url: Option<String>,
issue_tracker_base_url: Option<String>,
/// The directories that have already been created in this doc run. Used to reduce the number
/// of spurious `create_dir_all` calls.
crate created_dirs: RefCell<FxHashSet<PathBuf>>,
created_dirs: RefCell<FxHashSet<PathBuf>>,
/// This flag indicates whether listings of modules (in the side bar and documentation itself)
/// should be ordered alphabetically or in order of appearance (in the source code).
crate sort_modules_alphabetically: bool,
sort_modules_alphabetically: bool,
/// Additional CSS files to be added to the generated docs.
crate style_files: Vec<StylePath>,
/// Suffix to be added on resource files (if suffix is "-v2" then "light.css" becomes
@ -118,8 +120,16 @@
crate fs: DocFS,
/// The default edition used to parse doctests.
crate edition: Edition,
crate codes: ErrorCodes,
codes: ErrorCodes,
playground: Option<markdown::Playground>,
all: RefCell<AllTypes>,
/// Storage for the errors produced while generating documentation so they
/// can be printed together at the end.
errors: Receiver<String>,
/// `None` by default, depends on the `generate-redirect-map` option flag. If this field is set
/// to `Some(...)`, it'll store redirections and then generate a JSON file at the top level of
/// the crate.
redirections: Option<RefCell<FxHashMap<String, String>>>,
}
impl SharedContext<'_> {

View File

@ -148,6 +148,10 @@ fn init(
))
}
fn make_child_renderer(&self) -> Self {
self.clone()
}
/// Inserts an item into the index. This should be used rather than directly calling insert on
/// the hashmap because certain items (traits and types) need to have their mappings for trait
/// implementations filled out before they're inserted.