1
Fork 0

Don't share id_map and deref_id_map

All the tests passed, so it doesn't seem they need to be shared.
Plus they should be item/page-specific.

I'm not sure why they were shared before. I think the reason `id_map`
worked as a shared value before is that it is cleared before rendering
each item (in `render_item`). And then I'm guessing `deref_id_map`
worked because it's a hashmap keyed by `DefId`, so there was no overlap
(though I'm guessing we could have had issues in the future).

Note that `id_map` currently still has to be cleared because otherwise
child items would inherit the `id_map` of their parent. I'm hoping to
figure out a way to stop cloning `Context`, but until then we have to
reset `id_map`.
This commit is contained in:
Camelid 2021-02-22 15:43:43 -08:00
parent 3bc879e76b
commit 4c51a66d67
2 changed files with 20 additions and 21 deletions

View file

@ -6,7 +6,7 @@ use std::rc::Rc;
use std::sync::mpsc::channel; use std::sync::mpsc::channel;
use rustc_data_structures::fx::FxHashMap; use rustc_data_structures::fx::FxHashMap;
use rustc_hir::def_id::LOCAL_CRATE; use rustc_hir::def_id::{DefId, LOCAL_CRATE};
use rustc_middle::ty::TyCtxt; use rustc_middle::ty::TyCtxt;
use rustc_session::Session; use rustc_session::Session;
use rustc_span::edition::Edition; use rustc_span::edition::Edition;
@ -30,7 +30,7 @@ use crate::formats::item_type::ItemType;
use crate::formats::FormatRenderer; use crate::formats::FormatRenderer;
use crate::html::escape::Escape; use crate::html::escape::Escape;
use crate::html::format::Buffer; use crate::html::format::Buffer;
use crate::html::markdown::{self, plain_text_summary, ErrorCodes}; use crate::html::markdown::{self, plain_text_summary, ErrorCodes, IdMap};
use crate::html::{layout, sources}; use crate::html::{layout, sources};
/// Major driving force in all rustdoc rendering. This contains information /// Major driving force in all rustdoc rendering. This contains information
@ -52,6 +52,11 @@ crate struct Context<'tcx> {
/// real location of an item. This is used to allow external links to /// real location of an item. This is used to allow external links to
/// publicly reused items to redirect to the right location. /// publicly reused items to redirect to the right location.
pub(super) render_redirect_pages: bool, pub(super) render_redirect_pages: bool,
/// The map used to ensure all generated 'id=' attributes are unique.
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: RefCell<FxHashMap<DefId, String>>,
/// Shared mutable state. /// Shared mutable state.
/// ///
/// Issue for improving the situation: [#82381][] /// Issue for improving the situation: [#82381][]
@ -72,7 +77,7 @@ crate struct Context<'tcx> {
// `Context` is cloned a lot, so we don't want the size to grow unexpectedly. // `Context` is cloned a lot, so we don't want the size to grow unexpectedly.
#[cfg(target_arch = "x86_64")] #[cfg(target_arch = "x86_64")]
rustc_data_structures::static_assert_size!(Context<'_>, 72); rustc_data_structures::static_assert_size!(Context<'_>, 152);
impl<'tcx> Context<'tcx> { impl<'tcx> Context<'tcx> {
pub(super) fn path(&self, filename: &str) -> PathBuf { pub(super) fn path(&self, filename: &str) -> PathBuf {
@ -95,7 +100,7 @@ impl<'tcx> Context<'tcx> {
} }
pub(super) fn derive_id(&self, id: String) -> String { pub(super) fn derive_id(&self, id: String) -> String {
let mut map = self.shared.id_map.borrow_mut(); let mut map = self.id_map.borrow_mut();
map.derive(id) map.derive(id)
} }
@ -153,8 +158,8 @@ impl<'tcx> Context<'tcx> {
}; };
{ {
self.shared.id_map.borrow_mut().reset(); self.id_map.borrow_mut().reset();
self.shared.id_map.borrow_mut().populate(&INITIAL_IDS); self.id_map.borrow_mut().populate(&INITIAL_IDS);
} }
if !self.render_redirect_pages { if !self.render_redirect_pages {
@ -387,8 +392,6 @@ impl<'tcx> FormatRenderer<'tcx> for Context<'tcx> {
edition, edition,
codes: ErrorCodes::from(unstable_features.is_nightly_build()), codes: ErrorCodes::from(unstable_features.is_nightly_build()),
playground, playground,
id_map: RefCell::new(id_map),
deref_id_map: RefCell::new(FxHashMap::default()),
all: RefCell::new(AllTypes::new()), all: RefCell::new(AllTypes::new()),
errors: receiver, errors: receiver,
redirections: if generate_redirect_map { Some(Default::default()) } else { None }, redirections: if generate_redirect_map { Some(Default::default()) } else { None },
@ -418,6 +421,8 @@ impl<'tcx> FormatRenderer<'tcx> for Context<'tcx> {
current: Vec::new(), current: Vec::new(),
dst, dst,
render_redirect_pages: false, render_redirect_pages: false,
id_map: RefCell::new(id_map),
deref_id_map: RefCell::new(FxHashMap::default()),
shared: Rc::new(scx), shared: Rc::new(scx),
cache: Rc::new(cache), cache: Rc::new(cache),
}; };

View file

@ -70,7 +70,7 @@ use crate::html::format::{
PrintWithSpace, WhereClause, PrintWithSpace, WhereClause,
}; };
use crate::html::layout; use crate::html::layout;
use crate::html::markdown::{self, ErrorCodes, IdMap, Markdown, MarkdownHtml, MarkdownSummaryLine}; use crate::html::markdown::{self, ErrorCodes, Markdown, MarkdownHtml, MarkdownSummaryLine};
/// A pair of name and its optional document. /// A pair of name and its optional document.
crate type NameDoc = (String, Option<String>); crate type NameDoc = (String, Option<String>);
@ -121,11 +121,6 @@ crate struct SharedContext<'tcx> {
crate edition: Edition, crate edition: Edition,
codes: ErrorCodes, codes: ErrorCodes,
playground: Option<markdown::Playground>, playground: Option<markdown::Playground>,
/// The map used to ensure all generated 'id=' attributes are unique.
id_map: RefCell<IdMap>,
/// Tracks section IDs for `Deref` targets so they match in both the main
/// body and the sidebar.
deref_id_map: RefCell<FxHashMap<DefId, String>>,
all: RefCell<AllTypes>, all: RefCell<AllTypes>,
/// Storage for the errors produced while generating documentation so they /// Storage for the errors produced while generating documentation so they
/// can be printed together at the end. /// can be printed together at the end.
@ -650,7 +645,7 @@ fn render_markdown(
prefix: &str, prefix: &str,
is_hidden: bool, is_hidden: bool,
) { ) {
let mut ids = cx.shared.id_map.borrow_mut(); let mut ids = cx.id_map.borrow_mut();
write!( write!(
w, w,
"<div class=\"docblock{}\">{}{}</div>", "<div class=\"docblock{}\">{}{}</div>",
@ -810,7 +805,7 @@ fn short_item_info(
if let Some(note) = note { if let Some(note) = note {
let note = note.as_str(); let note = note.as_str();
let mut ids = cx.shared.id_map.borrow_mut(); let mut ids = cx.id_map.borrow_mut();
let html = MarkdownHtml( let html = MarkdownHtml(
&note, &note,
&mut ids, &mut ids,
@ -849,7 +844,7 @@ fn short_item_info(
message.push_str(&format!(" ({})", feature)); message.push_str(&format!(" ({})", feature));
if let Some(unstable_reason) = reason { if let Some(unstable_reason) = reason {
let mut ids = cx.shared.id_map.borrow_mut(); let mut ids = cx.id_map.borrow_mut();
message = format!( message = format!(
"<details><summary>{}</summary>{}</details>", "<details><summary>{}</summary>{}</details>",
message, message,
@ -1189,8 +1184,7 @@ fn render_assoc_items(
type_.print(cx.cache()) type_.print(cx.cache())
))); )));
debug!("Adding {} to deref id map", type_.print(cx.cache())); debug!("Adding {} to deref id map", type_.print(cx.cache()));
cx.shared cx.deref_id_map
.deref_id_map
.borrow_mut() .borrow_mut()
.insert(type_.def_id_full(cx.cache()).unwrap(), id.clone()); .insert(type_.def_id_full(cx.cache()).unwrap(), id.clone());
write!( write!(
@ -1497,7 +1491,7 @@ fn render_impl(
} }
if let Some(ref dox) = cx.shared.maybe_collapsed_doc_value(&i.impl_item) { if let Some(ref dox) = cx.shared.maybe_collapsed_doc_value(&i.impl_item) {
let mut ids = cx.shared.id_map.borrow_mut(); let mut ids = cx.id_map.borrow_mut();
write!( write!(
w, w,
"<div class=\"docblock\">{}</div>", "<div class=\"docblock\">{}</div>",
@ -2046,7 +2040,7 @@ fn sidebar_deref_methods(cx: &Context<'_>, out: &mut Buffer, impl_: &Impl, v: &V
.flat_map(|i| get_methods(i.inner_impl(), true, &mut used_links, deref_mut, c)) .flat_map(|i| get_methods(i.inner_impl(), true, &mut used_links, deref_mut, c))
.collect::<Vec<_>>(); .collect::<Vec<_>>();
if !ret.is_empty() { if !ret.is_empty() {
let deref_id_map = cx.shared.deref_id_map.borrow(); let deref_id_map = cx.deref_id_map.borrow();
let id = deref_id_map let id = deref_id_map
.get(&real_target.def_id_full(cx.cache()).unwrap()) .get(&real_target.def_id_full(cx.cache()).unwrap())
.expect("Deref section without derived id"); .expect("Deref section without derived id");