From 354507e61f303d6c86fa0f832e1081d40c2d89c2 Mon Sep 17 00:00:00 2001 From: QuietMisdreavus Date: Sat, 1 Sep 2018 21:20:39 -0500 Subject: [PATCH] shuffle ownership of `external_traits` constraints: - clean/inline.rs needs this map to fill in traits when inlining - fold.rs needs this map to allow passes to fold trait items - html/render.rs needs this map to seed the Cache.traits map of all known traits The first two are the real problem, since `DocFolder` only operates on `clean::Crate` but `clean/inline.rs` only sees the `DocContext`. The introduction of early passes means that these two now exist at the same time, so they need to share ownership of the map. Even better, the use of `Crate` in a rustc thread pool means that it needs to be Sync, so it can't use `Lrc` to manually activate thread-safety. `parking_lot` is reused from elsewhere in the tree to allow use of its `ReentrantMutex`, as the relevant parts of rustdoc are still single-threaded and this allows for easier use in that context. --- src/Cargo.lock | 1 + src/librustdoc/Cargo.toml | 1 + src/librustdoc/clean/inline.rs | 14 ++++++++++---- src/librustdoc/clean/mod.rs | 6 ++++-- src/librustdoc/core.rs | 6 +++--- src/librustdoc/fold.rs | 15 ++++++++------- src/librustdoc/html/render.rs | 2 +- src/librustdoc/lib.rs | 1 + src/librustdoc/passes/strip_hidden.rs | 9 ++------- 9 files changed, 31 insertions(+), 24 deletions(-) diff --git a/src/Cargo.lock b/src/Cargo.lock index 5ac838cadc2..377cfe74868 100644 --- a/src/Cargo.lock +++ b/src/Cargo.lock @@ -2443,6 +2443,7 @@ name = "rustdoc" version = "0.0.0" dependencies = [ "minifier 0.0.19 (registry+https://github.com/rust-lang/crates.io-index)", + "parking_lot 0.6.4 (registry+https://github.com/rust-lang/crates.io-index)", "pulldown-cmark 0.1.2 (registry+https://github.com/rust-lang/crates.io-index)", "tempfile 3.0.3 (registry+https://github.com/rust-lang/crates.io-index)", ] diff --git a/src/librustdoc/Cargo.toml b/src/librustdoc/Cargo.toml index e163fc68cbd..845bfad7807 100644 --- a/src/librustdoc/Cargo.toml +++ b/src/librustdoc/Cargo.toml @@ -11,3 +11,4 @@ path = "lib.rs" pulldown-cmark = { version = "0.1.2", default-features = false } minifier = "0.0.19" tempfile = "3" +parking_lot = "0.6.4" diff --git a/src/librustdoc/clean/inline.rs b/src/librustdoc/clean/inline.rs index 257fcea7842..1ea130cf16a 100644 --- a/src/librustdoc/clean/inline.rs +++ b/src/librustdoc/clean/inline.rs @@ -539,10 +539,13 @@ pub fn record_extern_trait(cx: &DocContext, did: DefId) { return; } - if cx.external_traits.borrow().contains_key(&did) || - cx.active_extern_traits.borrow().contains(&did) { - return; + let external_traits = cx.external_traits.lock(); + if external_traits.borrow().contains_key(&did) || + cx.active_extern_traits.borrow().contains(&did) + { + return; + } } cx.active_extern_traits.borrow_mut().push(did); @@ -550,6 +553,9 @@ pub fn record_extern_trait(cx: &DocContext, did: DefId) { debug!("record_extern_trait: {:?}", did); let trait_ = build_external_trait(cx, did); - cx.external_traits.borrow_mut().insert(did, trait_); + { + let external_traits = cx.external_traits.lock(); + external_traits.borrow_mut().insert(did, trait_); + } cx.active_extern_traits.borrow_mut().remove_item(&did); } diff --git a/src/librustdoc/clean/mod.rs b/src/librustdoc/clean/mod.rs index a5f13abe443..a982933f6c1 100644 --- a/src/librustdoc/clean/mod.rs +++ b/src/librustdoc/clean/mod.rs @@ -55,6 +55,8 @@ use std::cell::RefCell; use std::sync::Arc; use std::u32; +use parking_lot::ReentrantMutex; + use core::{self, DocContext}; use doctree; use visit_ast; @@ -136,7 +138,7 @@ pub struct Crate { pub primitives: Vec<(DefId, PrimitiveType, Attributes)>, // These are later on moved into `CACHEKEY`, leaving the map empty. // Only here so that they can be filtered through the rustdoc passes. - pub external_traits: FxHashMap, + pub external_traits: Arc>>>, pub masked_crates: FxHashSet, } @@ -214,7 +216,7 @@ impl<'a, 'tcx, 'rcx, 'cstore> Clean for visit_ast::RustdocVisitor<'a, 'tc module: Some(module), externs, primitives, - external_traits: Default::default(), + external_traits: cx.external_traits.clone(), masked_crates, } } diff --git a/src/librustdoc/core.rs b/src/librustdoc/core.rs index a270e0c9ba5..db6e0183804 100644 --- a/src/librustdoc/core.rs +++ b/src/librustdoc/core.rs @@ -36,11 +36,13 @@ use syntax::symbol::keywords; use syntax_pos::DUMMY_SP; use errors; use errors::emitter::{Emitter, EmitterWriter}; +use parking_lot::ReentrantMutex; use std::cell::RefCell; use std::mem; use rustc_data_structures::sync::{self, Lrc}; use std::rc::Rc; +use std::sync::Arc; use std::path::PathBuf; use visit_ast::RustdocVisitor; @@ -66,7 +68,7 @@ pub struct DocContext<'a, 'tcx: 'a, 'rcx: 'a, 'cstore: 'rcx> { /// Later on moved into `html::render::CACHE_KEY` pub renderinfo: RefCell, /// Later on moved through `clean::Crate` into `html::render::CACHE_KEY` - pub external_traits: RefCell>, + pub external_traits: Arc>>>, /// Used while populating `external_traits` to ensure we don't process the same trait twice at /// the same time. pub active_extern_traits: RefCell>, @@ -599,8 +601,6 @@ pub fn run_core(search_paths: SearchPaths, ctxt.sess().abort_if_errors(); - krate.external_traits = ctxt.external_traits.into_inner(); - (krate, ctxt.renderinfo.into_inner(), passes) }), &sess) }) diff --git a/src/librustdoc/fold.rs b/src/librustdoc/fold.rs index 6d96bc8e360..b8e27c53170 100644 --- a/src/librustdoc/fold.rs +++ b/src/librustdoc/fold.rs @@ -8,8 +8,6 @@ // option. This file may not be copied, modified, or distributed // except according to those terms. -use std::mem; - use clean::*; pub struct StripItem(pub Item); @@ -116,11 +114,14 @@ pub trait DocFolder : Sized { fn fold_crate(&mut self, mut c: Crate) -> Crate { c.module = c.module.take().and_then(|module| self.fold_item(module)); - let traits = mem::replace(&mut c.external_traits, Default::default()); - c.external_traits.extend(traits.into_iter().map(|(k, mut v)| { - v.items = v.items.into_iter().filter_map(|i| self.fold_item(i)).collect(); - (k, v) - })); + { + let guard = c.external_traits.lock(); + let traits = guard.replace(Default::default()); + guard.borrow_mut().extend(traits.into_iter().map(|(k, mut v)| { + v.items = v.items.into_iter().filter_map(|i| self.fold_item(i)).collect(); + (k, v) + })); + } c } } diff --git a/src/librustdoc/html/render.rs b/src/librustdoc/html/render.rs index 3793bb089b0..3e1720f8b8a 100644 --- a/src/librustdoc/html/render.rs +++ b/src/librustdoc/html/render.rs @@ -606,7 +606,7 @@ pub fn run(mut krate: clean::Crate, crate_version: krate.version.take(), orphan_impl_items: Vec::new(), orphan_trait_impls: Vec::new(), - traits: mem::replace(&mut krate.external_traits, FxHashMap()), + traits: krate.external_traits.lock().replace(FxHashMap()), deref_trait_did, deref_mut_trait_did, owned_box_did, diff --git a/src/librustdoc/lib.rs b/src/librustdoc/lib.rs index e7bf7fdf3f6..5607c97a496 100644 --- a/src/librustdoc/lib.rs +++ b/src/librustdoc/lib.rs @@ -49,6 +49,7 @@ extern crate rustc_errors as errors; extern crate pulldown_cmark; extern crate tempfile; extern crate minifier; +extern crate parking_lot; extern crate serialize as rustc_serialize; // used by deriving diff --git a/src/librustdoc/passes/strip_hidden.rs b/src/librustdoc/passes/strip_hidden.rs index eab4022a338..24dd4cc13bf 100644 --- a/src/librustdoc/passes/strip_hidden.rs +++ b/src/librustdoc/passes/strip_hidden.rs @@ -24,13 +24,9 @@ pub const STRIP_HIDDEN: Pass = "strips all doc(hidden) items from the output"); /// Strip items marked `#[doc(hidden)]` -pub fn strip_hidden(mut krate: clean::Crate, cx: &DocContext) -> clean::Crate { +pub fn strip_hidden(krate: clean::Crate, _: &DocContext) -> clean::Crate { let mut retained = DefIdSet(); - // as an early pass, the external traits haven't been swapped in, so we need to do that ahead - // of time - mem::swap(&mut krate.external_traits, &mut cx.external_traits.borrow_mut()); - // strip all #[doc(hidden)] items let krate = { let mut stripper = Stripper{ retained: &mut retained, update_retained: true }; @@ -39,8 +35,7 @@ pub fn strip_hidden(mut krate: clean::Crate, cx: &DocContext) -> clean::Crate { // strip all impls referencing stripped items let mut stripper = ImplStripper { retained: &retained }; - let mut krate = stripper.fold_crate(krate); - mem::swap(&mut krate.external_traits, &mut cx.external_traits.borrow_mut()); + let krate = stripper.fold_crate(krate); krate }