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`

4 files changed, 74 insertions, 57 deletions

diff --git a/src/librustdoc/html/markdown.rs b/src/librustdoc/html/markdown.rs
index f8ca259fb9a..ccc51c243ad 100644
--- a/src/librustdoc/html/markdown.rs
+++ b/src/librustdoc/html/markdown.rs
@@ -1373,10 +1373,6 @@ impl IdMap {
         }
     }
 
-    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(),
diff --git a/src/librustdoc/html/markdown/tests.rs b/src/librustdoc/html/markdown/tests.rs
index 59ca841715c..e2ce9ad23f4 100644
--- a/src/librustdoc/html/markdown/tests.rs
+++ b/src/librustdoc/html/markdown/tests.rs
@@ -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]
diff --git a/src/librustdoc/html/render/context.rs b/src/librustdoc/html/render/context.rs
index 976168a9ac4..864fbccbcc4 100644
--- a/src/librustdoc/html/render/context.rs
+++ b/src/librustdoc/html/render/context.rs
@@ -3,8 +3,7 @@ use std::collections::BTreeMap;
 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 @@ use crate::html::{layout, sources};
 /// 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 @@ impl<'tcx> Context<'tcx> {
             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 @@ impl<'tcx> Context<'tcx> {
                     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 @@ impl<'tcx> FormatRenderer<'tcx> for Context<'tcx> {
             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 @@ impl<'tcx> FormatRenderer<'tcx> for Context<'tcx> {
             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 @@ impl<'tcx> FormatRenderer<'tcx> for Context<'tcx> {
         } 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 @@ impl<'tcx> FormatRenderer<'tcx> for Context<'tcx> {
             &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 @@ impl<'tcx> FormatRenderer<'tcx> for Context<'tcx> {
         }
 
         // 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 @@ impl<'tcx> FormatRenderer<'tcx> for Context<'tcx> {
             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),
diff --git a/src/librustdoc/html/render/mod.rs b/src/librustdoc/html/render/mod.rs
index 9b9ec2581cf..2331f4d20a8 100644
--- a/src/librustdoc/html/render/mod.rs
+++ b/src/librustdoc/html/render/mod.rs
@@ -42,6 +42,7 @@ use std::fmt;
 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 @@ crate fn ensure_trailing_slash(v: &str) -> impl fmt::Display + '_ {
     })
 }
 
+/// 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 @@ crate struct SharedContext<'tcx> {
     /// 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 struct SharedContext<'tcx> {
     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<'_> {