From 65ea7b794718306c7b2945f63f3dd01919fc73c0 Mon Sep 17 00:00:00 2001 From: Mark Rousskov Date: Sat, 10 Aug 2019 13:25:50 -0400 Subject: [PATCH 01/14] rustdoc: Replace HirVec with slices in doctree --- src/librustdoc/doctree.rs | 42 +++++++++++++++++++-------------------- 1 file changed, 21 insertions(+), 21 deletions(-) diff --git a/src/librustdoc/doctree.rs b/src/librustdoc/doctree.rs index 90dcf1be76c0d..6e453561f6da2 100644 --- a/src/librustdoc/doctree.rs +++ b/src/librustdoc/doctree.rs @@ -13,7 +13,7 @@ use rustc::hir::ptr::P; pub struct Module<'hir> { pub name: Option, - pub attrs: &'hir hir::HirVec, + pub attrs: &'hir [ast::Attribute], pub where_outer: Span, pub where_inner: Span, pub extern_crates: Vec>, @@ -41,7 +41,7 @@ pub struct Module<'hir> { impl Module<'hir> { pub fn new( name: Option, - attrs: &'hir hir::HirVec, + attrs: &'hir [ast::Attribute], vis: &'hir hir::Visibility, ) -> Module<'hir> { Module { @@ -89,7 +89,7 @@ pub struct Struct<'hir> { pub struct_type: StructType, pub name: Name, pub generics: &'hir hir::Generics, - pub attrs: &'hir hir::HirVec, + pub attrs: &'hir [ast::Attribute], pub fields: &'hir [hir::StructField], pub whence: Span, } @@ -100,7 +100,7 @@ pub struct Union<'hir> { pub struct_type: StructType, pub name: Name, pub generics: &'hir hir::Generics, - pub attrs: &'hir hir::HirVec, + pub attrs: &'hir [ast::Attribute], pub fields: &'hir [hir::StructField], pub whence: Span, } @@ -109,7 +109,7 @@ pub struct Enum<'hir> { pub vis: &'hir hir::Visibility, pub variants: Vec>, pub generics: &'hir hir::Generics, - pub attrs: &'hir hir::HirVec, + pub attrs: &'hir [ast::Attribute], pub id: hir::HirId, pub whence: Span, pub name: Name, @@ -118,14 +118,14 @@ pub struct Enum<'hir> { pub struct Variant<'hir> { pub name: Name, pub id: hir::HirId, - pub attrs: &'hir hir::HirVec, + pub attrs: &'hir [ast::Attribute], pub def: &'hir hir::VariantData, pub whence: Span, } pub struct Function<'hir> { pub decl: &'hir hir::FnDecl, - pub attrs: &'hir hir::HirVec, + pub attrs: &'hir [ast::Attribute], pub id: hir::HirId, pub name: Name, pub vis: &'hir hir::Visibility, @@ -140,7 +140,7 @@ pub struct Typedef<'hir> { pub gen: &'hir hir::Generics, pub name: Name, pub id: hir::HirId, - pub attrs: &'hir hir::HirVec, + pub attrs: &'hir [ast::Attribute], pub whence: Span, pub vis: &'hir hir::Visibility, } @@ -149,7 +149,7 @@ pub struct OpaqueTy<'hir> { pub opaque_ty: &'hir hir::OpaqueTy, pub name: Name, pub id: hir::HirId, - pub attrs: &'hir hir::HirVec, + pub attrs: &'hir [ast::Attribute], pub whence: Span, pub vis: &'hir hir::Visibility, } @@ -160,7 +160,7 @@ pub struct Static<'hir> { pub mutability: hir::Mutability, pub expr: hir::BodyId, pub name: Name, - pub attrs: &'hir hir::HirVec, + pub attrs: &'hir [ast::Attribute], pub vis: &'hir hir::Visibility, pub id: hir::HirId, pub whence: Span, @@ -170,7 +170,7 @@ pub struct Constant<'hir> { pub type_: &'hir P, pub expr: hir::BodyId, pub name: Name, - pub attrs: &'hir hir::HirVec, + pub attrs: &'hir [ast::Attribute], pub vis: &'hir hir::Visibility, pub id: hir::HirId, pub whence: Span, @@ -182,8 +182,8 @@ pub struct Trait<'hir> { pub name: Name, pub items: Vec<&'hir hir::TraitItem>, pub generics: &'hir hir::Generics, - pub bounds: &'hir hir::HirVec, - pub attrs: &'hir hir::HirVec, + pub bounds: &'hir [hir::GenericBound], + pub attrs: &'hir [ast::Attribute], pub id: hir::HirId, pub whence: Span, pub vis: &'hir hir::Visibility, @@ -192,8 +192,8 @@ pub struct Trait<'hir> { pub struct TraitAlias<'hir> { pub name: Name, pub generics: &'hir hir::Generics, - pub bounds: &'hir hir::HirVec, - pub attrs: &'hir hir::HirVec, + pub bounds: &'hir [hir::GenericBound], + pub attrs: &'hir [ast::Attribute], pub id: hir::HirId, pub whence: Span, pub vis: &'hir hir::Visibility, @@ -208,7 +208,7 @@ pub struct Impl<'hir> { pub trait_: &'hir Option, pub for_: &'hir P, pub items: Vec<&'hir hir::ImplItem>, - pub attrs: &'hir hir::HirVec, + pub attrs: &'hir [ast::Attribute], pub whence: Span, pub vis: &'hir hir::Visibility, pub id: hir::HirId, @@ -219,7 +219,7 @@ pub struct ForeignItem<'hir> { pub id: hir::HirId, pub name: Name, pub kind: &'hir hir::ForeignItemKind, - pub attrs: &'hir hir::HirVec, + pub attrs: &'hir [ast::Attribute], pub whence: Span, } @@ -229,7 +229,7 @@ pub struct Macro<'hir> { pub name: Name, pub hid: hir::HirId, pub def_id: hir::def_id::DefId, - pub attrs: &'hir hir::HirVec, + pub attrs: &'hir [ast::Attribute], pub whence: Span, pub matchers: hir::HirVec, pub imported_from: Option, @@ -240,7 +240,7 @@ pub struct ExternCrate<'hir> { pub cnum: CrateNum, pub path: Option, pub vis: &'hir hir::Visibility, - pub attrs: &'hir hir::HirVec, + pub attrs: &'hir [ast::Attribute], pub whence: Span, } @@ -248,7 +248,7 @@ pub struct Import<'hir> { pub name: Name, pub id: hir::HirId, pub vis: &'hir hir::Visibility, - pub attrs: &'hir hir::HirVec, + pub attrs: &'hir [ast::Attribute], pub path: &'hir hir::Path, pub glob: bool, pub whence: Span, @@ -259,7 +259,7 @@ pub struct ProcMacro<'hir> { pub id: hir::HirId, pub kind: MacroKind, pub helpers: Vec, - pub attrs: &'hir hir::HirVec, + pub attrs: &'hir [ast::Attribute], pub whence: Span, } From 19c85a8f8a3f82bfbc92f3ecd23167c0dbd66b55 Mon Sep 17 00:00:00 2001 From: Mark Rousskov Date: Sat, 10 Aug 2019 13:37:31 -0400 Subject: [PATCH 02/14] Move def_id_to_path to use site in visit_ast --- src/librustdoc/clean/mod.rs | 20 +------------------- src/librustdoc/visit_ast.rs | 19 ++++++++++++++++++- 2 files changed, 19 insertions(+), 20 deletions(-) diff --git a/src/librustdoc/clean/mod.rs b/src/librustdoc/clean/mod.rs index 6f33bdd7f4d2f..0c38e68a764f4 100644 --- a/src/librustdoc/clean/mod.rs +++ b/src/librustdoc/clean/mod.rs @@ -39,7 +39,7 @@ use std::fmt; use std::hash::{Hash, Hasher}; use std::default::Default; use std::{mem, slice, vec}; -use std::iter::{FromIterator, once}; +use std::iter::FromIterator; use std::rc::Rc; use std::cell::RefCell; use std::sync::Arc; @@ -4398,24 +4398,6 @@ impl Clean for hir::TypeBindingKind { } } -pub fn def_id_to_path( - cx: &DocContext<'_>, - did: DefId, - name: Option -) -> Vec { - let crate_name = name.unwrap_or_else(|| cx.tcx.crate_name(did.krate).to_string()); - 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 - } - }); - once(crate_name).chain(relative).collect() -} - pub fn enter_impl_trait(cx: &DocContext<'_>, f: F) -> R where F: FnOnce() -> R, diff --git a/src/librustdoc/visit_ast.rs b/src/librustdoc/visit_ast.rs index f01b9eeb30f23..878f1a476be9c 100644 --- a/src/librustdoc/visit_ast.rs +++ b/src/librustdoc/visit_ast.rs @@ -15,9 +15,26 @@ use syntax_pos::{self, Span}; use std::mem; use crate::core; -use crate::clean::{self, AttributesExt, NestedAttributesExt, def_id_to_path}; +use crate::clean::{self, AttributesExt, NestedAttributesExt}; use crate::doctree::*; +fn def_id_to_path( + cx: &core::DocContext<'_>, + did: DefId, + name: Option +) -> Vec { + let crate_name = name.unwrap_or_else(|| cx.tcx.crate_name(did.krate).to_string()); + 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 + } + }); + std::iter::once(crate_name).chain(relative).collect() +} // Also, is there some reason that this doesn't use the 'visit' // framework from syntax?. From 00d7bc7688c31e22e3f27d3e225087b1f8ea694b Mon Sep 17 00:00:00 2001 From: Mark Rousskov Date: Sat, 10 Aug 2019 13:44:23 -0400 Subject: [PATCH 03/14] Remove crate_name from DocContext tcx.crate_name is the appropriate way to retrieve the crate name. --- src/librustdoc/clean/inline.rs | 5 +---- src/librustdoc/core.rs | 5 +---- src/librustdoc/visit_ast.rs | 11 ++++++----- 3 files changed, 8 insertions(+), 13 deletions(-) diff --git a/src/librustdoc/clean/inline.rs b/src/librustdoc/clean/inline.rs index d2a6dcf19bcca..884cdef3c771f 100644 --- a/src/librustdoc/clean/inline.rs +++ b/src/librustdoc/clean/inline.rs @@ -163,10 +163,7 @@ pub 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. pub fn record_extern_fqn(cx: &DocContext<'_>, did: DefId, kind: clean::TypeKind) { - let mut crate_name = cx.tcx.crate_name(did.krate).to_string(); - if did.is_local() { - crate_name = cx.crate_name.clone().unwrap_or(crate_name); - } + let crate_name = cx.tcx.crate_name(did.krate).to_string(); let relative = cx.tcx.def_path(did).data.into_iter().filter_map(|elem| { // extern blocks have an empty name diff --git a/src/librustdoc/core.rs b/src/librustdoc/core.rs index 5138e4a23a47e..869bec6cb88a5 100644 --- a/src/librustdoc/core.rs +++ b/src/librustdoc/core.rs @@ -46,8 +46,6 @@ pub struct DocContext<'tcx> { pub tcx: TyCtxt<'tcx>, pub resolver: Rc>, - /// The stack of module NodeIds up till this point - pub crate_name: Option, pub cstore: Lrc, /// Later on moved into `html::render::CACHE_KEY` pub renderinfo: RefCell, @@ -332,7 +330,7 @@ pub fn run_core(options: RustdocOptions) -> (clean::Crate, RenderInfo, RenderOpt file_loader: None, diagnostic_output: DiagnosticOutput::Default, stderr: None, - crate_name: crate_name.clone(), + crate_name, lint_caps, }; @@ -372,7 +370,6 @@ pub fn run_core(options: RustdocOptions) -> (clean::Crate, RenderInfo, RenderOpt let ctxt = DocContext { tcx, resolver, - crate_name, cstore: compiler.cstore().clone(), external_traits: Default::default(), active_extern_traits: Default::default(), diff --git a/src/librustdoc/visit_ast.rs b/src/librustdoc/visit_ast.rs index 878f1a476be9c..5fad9038104de 100644 --- a/src/librustdoc/visit_ast.rs +++ b/src/librustdoc/visit_ast.rs @@ -6,6 +6,7 @@ use rustc::hir::def::{Res, DefKind}; use rustc::hir::def_id::{DefId, LOCAL_CRATE}; use rustc::middle::privacy::AccessLevel; use rustc::util::nodemap::{FxHashSet, FxHashMap}; +use rustc::ty::TyCtxt; use syntax::ast; use syntax::ext::base::MacroKind; use syntax::source_map::Spanned; @@ -18,13 +19,13 @@ use crate::core; use crate::clean::{self, AttributesExt, NestedAttributesExt}; use crate::doctree::*; +// FIXME: Should this be replaced with tcx.def_path_str? fn def_id_to_path( - cx: &core::DocContext<'_>, + tcx: TyCtxt<'_>, did: DefId, - name: Option ) -> Vec { - let crate_name = name.unwrap_or_else(|| cx.tcx.crate_name(did.krate).to_string()); - let relative = cx.tcx.def_path(did).data.into_iter().filter_map(|elem| { + let crate_name = tcx.crate_name(did.krate).to_string(); + let relative = 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() { @@ -68,7 +69,7 @@ impl<'a, 'tcx> RustdocVisitor<'a, 'tcx> { // We can't use the entry API, as that keeps the mutable borrow of `self` active // when we try to use `cx`. if self.exact_paths.get(&did).is_none() { - let path = def_id_to_path(self.cx, did, self.cx.crate_name.clone()); + let path = def_id_to_path(self.cx.tcx, did); self.exact_paths.insert(did, path); } } From 8f80a8d7d564b17fe3831c4b5e5404d93a013bf5 Mon Sep 17 00:00:00 2001 From: Mark Rousskov Date: Sat, 10 Aug 2019 13:52:45 -0400 Subject: [PATCH 04/14] Use entry API in store_path --- src/librustdoc/visit_ast.rs | 8 ++------ 1 file changed, 2 insertions(+), 6 deletions(-) diff --git a/src/librustdoc/visit_ast.rs b/src/librustdoc/visit_ast.rs index 5fad9038104de..35b6d9972da06 100644 --- a/src/librustdoc/visit_ast.rs +++ b/src/librustdoc/visit_ast.rs @@ -66,12 +66,8 @@ impl<'a, 'tcx> RustdocVisitor<'a, 'tcx> { } fn store_path(&mut self, did: DefId) { - // We can't use the entry API, as that keeps the mutable borrow of `self` active - // when we try to use `cx`. - if self.exact_paths.get(&did).is_none() { - let path = def_id_to_path(self.cx.tcx, did); - self.exact_paths.insert(did, path); - } + let tcx = self.cx.tcx; + self.exact_paths.entry(did).or_insert_with(|| def_id_to_path(tcx, did)); } pub fn visit(mut self, krate: &'tcx hir::Crate) -> Module<'tcx> { From c57481001ea81665b1ca23368b710a0435d28653 Mon Sep 17 00:00:00 2001 From: Mark Rousskov Date: Sat, 10 Aug 2019 15:20:21 -0400 Subject: [PATCH 05/14] Remove ReentrantMutex This drops the parking_lot dependency; the ReentrantMutex type appeared to be unused (at least, no compilation failures occurred). This is technically a possible change in behavior of its users, as lock() would wait on other threads releasing their guards, but since we didn't actually remove any threading or such in this code, it appears that we never used that behavior (the behavior change is only noticeable if the type previously was used in two threads, in a single thread ReentrantMutex is useless). --- Cargo.lock | 1 - src/librustdoc/Cargo.toml | 1 - src/librustdoc/clean/inline.rs | 6 ++---- src/librustdoc/clean/mod.rs | 4 +--- src/librustdoc/core.rs | 3 +-- src/librustdoc/fold.rs | 8 ++++---- src/librustdoc/html/render.rs | 2 +- 7 files changed, 9 insertions(+), 16 deletions(-) diff --git a/Cargo.lock b/Cargo.lock index 52cfa2cb1f80e..ab6731e4d433d 100644 --- a/Cargo.lock +++ b/Cargo.lock @@ -3252,7 +3252,6 @@ name = "rustdoc" version = "0.0.0" dependencies = [ "minifier 0.0.33 (registry+https://github.com/rust-lang/crates.io-index)", - "parking_lot 0.7.1 (registry+https://github.com/rust-lang/crates.io-index)", "pulldown-cmark 0.5.3 (registry+https://github.com/rust-lang/crates.io-index)", "rustc-rayon 0.2.0 (registry+https://github.com/rust-lang/crates.io-index)", "tempfile 3.0.5 (registry+https://github.com/rust-lang/crates.io-index)", diff --git a/src/librustdoc/Cargo.toml b/src/librustdoc/Cargo.toml index 334dc74c6c8f7..0eb8b73016d10 100644 --- a/src/librustdoc/Cargo.toml +++ b/src/librustdoc/Cargo.toml @@ -13,4 +13,3 @@ pulldown-cmark = { version = "0.5.3", default-features = false } minifier = "0.0.33" rayon = { version = "0.2.0", package = "rustc-rayon" } tempfile = "3" -parking_lot = "0.7" diff --git a/src/librustdoc/clean/inline.rs b/src/librustdoc/clean/inline.rs index 884cdef3c771f..a8336607f7ae9 100644 --- a/src/librustdoc/clean/inline.rs +++ b/src/librustdoc/clean/inline.rs @@ -574,8 +574,7 @@ pub fn record_extern_trait(cx: &DocContext<'_>, did: DefId) { } { - let external_traits = cx.external_traits.lock(); - if external_traits.borrow().contains_key(&did) || + if cx.external_traits.borrow().contains_key(&did) || cx.active_extern_traits.borrow().contains(&did) { return; @@ -588,8 +587,7 @@ pub fn record_extern_trait(cx: &DocContext<'_>, did: DefId) { let trait_ = build_external_trait(cx, did); { - let external_traits = cx.external_traits.lock(); - external_traits.borrow_mut().insert(did, trait_); + cx.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 0c38e68a764f4..ee76bf35cab91 100644 --- a/src/librustdoc/clean/mod.rs +++ b/src/librustdoc/clean/mod.rs @@ -45,8 +45,6 @@ use std::cell::RefCell; use std::sync::Arc; use std::u32; -use parking_lot::ReentrantMutex; - use crate::core::{self, DocContext}; use crate::doctree; use crate::html::render::{cache, ExternalLocation}; @@ -133,7 +131,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: Arc>>>, + pub external_traits: Arc>>, pub masked_crates: FxHashSet, } diff --git a/src/librustdoc/core.rs b/src/librustdoc/core.rs index 869bec6cb88a5..6b524e1206f33 100644 --- a/src/librustdoc/core.rs +++ b/src/librustdoc/core.rs @@ -22,7 +22,6 @@ use syntax::json::JsonEmitter; use syntax::symbol::sym; use errors; use errors::emitter::{Emitter, EmitterWriter}; -use parking_lot::ReentrantMutex; use std::cell::RefCell; use std::mem; @@ -50,7 +49,7 @@ pub struct DocContext<'tcx> { /// 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: Arc>>>, + 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>, diff --git a/src/librustdoc/fold.rs b/src/librustdoc/fold.rs index cfa22bc27b758..5482239c7ce28 100644 --- a/src/librustdoc/fold.rs +++ b/src/librustdoc/fold.rs @@ -105,12 +105,12 @@ pub trait DocFolder : Sized { c.module = c.module.take().and_then(|module| self.fold_item(module)); { - let guard = c.external_traits.lock(); - let traits = guard.replace(Default::default()); - guard.borrow_mut().extend(traits.into_iter().map(|(k, mut v)| { + let mut guard = c.external_traits.borrow_mut(); + let external_traits = std::mem::replace(&mut *guard, Default::default()); + *guard = external_traits.into_iter().map(|(k, mut v)| { v.items = v.items.into_iter().filter_map(|i| self.fold_item(i)).collect(); (k, v) - })); + }).collect(); } c } diff --git a/src/librustdoc/html/render.rs b/src/librustdoc/html/render.rs index b1cee0182052b..211f1e325b9ec 100644 --- a/src/librustdoc/html/render.rs +++ b/src/librustdoc/html/render.rs @@ -659,7 +659,7 @@ pub fn run(mut krate: clean::Crate, crate_version: krate.version.take(), orphan_impl_items: Vec::new(), orphan_trait_impls: Vec::new(), - traits: krate.external_traits.lock().replace(Default::default()), + traits: krate.external_traits.replace(Default::default()), deref_trait_did, deref_mut_trait_did, owned_box_did, From 6be2857a6cc551f63cd79d9d59322fbb2ce9103d Mon Sep 17 00:00:00 2001 From: Mark Rousskov Date: Sat, 10 Aug 2019 16:19:25 -0400 Subject: [PATCH 06/14] Replace Arc with Rc around external_traits --- src/librustdoc/clean/mod.rs | 2 +- src/librustdoc/core.rs | 3 +-- 2 files changed, 2 insertions(+), 3 deletions(-) diff --git a/src/librustdoc/clean/mod.rs b/src/librustdoc/clean/mod.rs index ee76bf35cab91..af4fe62bcbc31 100644 --- a/src/librustdoc/clean/mod.rs +++ b/src/librustdoc/clean/mod.rs @@ -131,7 +131,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: Arc>>, + pub external_traits: Rc>>, pub masked_crates: FxHashSet, } diff --git a/src/librustdoc/core.rs b/src/librustdoc/core.rs index 6b524e1206f33..a14ddc706d5d1 100644 --- a/src/librustdoc/core.rs +++ b/src/librustdoc/core.rs @@ -26,7 +26,6 @@ use errors::emitter::{Emitter, EmitterWriter}; use std::cell::RefCell; use std::mem; use rustc_data_structures::sync::{self, Lrc}; -use std::sync::Arc; use std::rc::Rc; use crate::config::{Options as RustdocOptions, RenderOptions}; @@ -49,7 +48,7 @@ pub struct DocContext<'tcx> { /// 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: Arc>>, + pub external_traits: Rc>>, /// Used while populating `external_traits` to ensure we don't process the same trait twice at /// the same time. pub active_extern_traits: RefCell>, From 00319519bba1fa1c2036aef70607428da9519155 Mon Sep 17 00:00:00 2001 From: Mark Rousskov Date: Sat, 10 Aug 2019 16:43:39 -0400 Subject: [PATCH 07/14] Store typed Passes --- src/librustdoc/clean/mod.rs | 2 + src/librustdoc/config.rs | 16 +++---- src/librustdoc/core.rs | 31 +++++++------- src/librustdoc/html/render.rs | 14 ++---- src/librustdoc/lib.rs | 7 +-- src/librustdoc/passes/collapse_docs.rs | 4 +- src/librustdoc/passes/mod.rs | 59 +++++++++++++------------- 7 files changed, 65 insertions(+), 68 deletions(-) diff --git a/src/librustdoc/clean/mod.rs b/src/librustdoc/clean/mod.rs index af4fe62bcbc31..b281505956d6a 100644 --- a/src/librustdoc/clean/mod.rs +++ b/src/librustdoc/clean/mod.rs @@ -133,6 +133,7 @@ pub struct Crate { // Only here so that they can be filtered through the rustdoc passes. pub external_traits: Rc>>, pub masked_crates: FxHashSet, + pub collapsed: bool, } impl Clean for hir::Crate { @@ -221,6 +222,7 @@ impl Clean for hir::Crate { primitives, external_traits: cx.external_traits.clone(), masked_crates, + collapsed: false, } } } diff --git a/src/librustdoc/config.rs b/src/librustdoc/config.rs index db90bb4524dcf..2be67d707fe14 100644 --- a/src/librustdoc/config.rs +++ b/src/librustdoc/config.rs @@ -220,22 +220,22 @@ impl Options { println!("{:>20} - {}", pass.name, pass.description); } println!("\nDefault passes for rustdoc:"); - for &name in passes::DEFAULT_PASSES { - println!("{:>20}", name); + for pass in passes::DEFAULT_PASSES { + println!("{:>20}", pass.name); } println!("\nPasses run with `--document-private-items`:"); - for &name in passes::DEFAULT_PRIVATE_PASSES { - println!("{:>20}", name); + for pass in passes::DEFAULT_PRIVATE_PASSES { + println!("{:>20}", pass.name); } if nightly_options::is_nightly_build() { println!("\nPasses run with `--show-coverage`:"); - for &name in passes::DEFAULT_COVERAGE_PASSES { - println!("{:>20}", name); + for pass in passes::DEFAULT_COVERAGE_PASSES { + println!("{:>20}", pass.name); } println!("\nPasses run with `--show-coverage --document-private-items`:"); - for &name in passes::PRIVATE_COVERAGE_PASSES { - println!("{:>20}", name); + for pass in passes::PRIVATE_COVERAGE_PASSES { + println!("{:>20}", pass.name); } } diff --git a/src/librustdoc/core.rs b/src/librustdoc/core.rs index a14ddc706d5d1..adbe4b469e8d0 100644 --- a/src/librustdoc/core.rs +++ b/src/librustdoc/core.rs @@ -223,7 +223,7 @@ pub fn new_handler(error_format: ErrorOutputType, ) } -pub fn run_core(options: RustdocOptions) -> (clean::Crate, RenderInfo, RenderOptions, Vec) { +pub fn run_core(options: RustdocOptions) -> (clean::Crate, RenderInfo, RenderOptions) { // Parse, resolve, and typecheck the given crate. let RustdocOptions { @@ -427,8 +427,8 @@ pub fn run_core(options: RustdocOptions) -> (clean::Crate, RenderInfo, RenderOpt }, _ => continue, }; - for p in value.as_str().split_whitespace() { - sink.push(p.to_string()); + for name in value.as_str().split_whitespace() { + sink.push(name.to_string()); } } @@ -439,25 +439,26 @@ pub fn run_core(options: RustdocOptions) -> (clean::Crate, RenderInfo, RenderOpt } } - let mut passes: Vec = - passes::defaults(default_passes).iter().map(|p| p.to_string()).collect(); - passes.extend(manual_passes); + let passes = passes::defaults(default_passes).iter().chain(manual_passes.into_iter() + .flat_map(|name| { + if let Some(pass) = passes::find_pass(&name) { + Some(pass) + } else { + error!("unknown pass {}, skipping", name); + None + } + })); info!("Executing passes"); - for pass_name in &passes { - match passes::find_pass(pass_name).map(|p| p.pass) { - Some(pass) => { - debug!("running pass {}", pass_name); - krate = pass(krate, &ctxt); - } - None => error!("unknown pass {}, skipping", *pass_name), - } + for pass in passes { + debug!("running pass {}", pass.name); + krate = (pass.pass)(krate, &ctxt); } ctxt.sess().abort_if_errors(); - (krate, ctxt.renderinfo.into_inner(), render_options, passes) + (krate, ctxt.renderinfo.into_inner(), render_options) }) }) } diff --git a/src/librustdoc/html/render.rs b/src/librustdoc/html/render.rs index 211f1e325b9ec..eb88c72da9eeb 100644 --- a/src/librustdoc/html/render.rs +++ b/src/librustdoc/html/render.rs @@ -185,8 +185,8 @@ struct SharedContext { pub include_sources: bool, /// The local file sources we've emitted and their respective url-paths. pub local_sources: FxHashMap, - /// All the passes that were run on this crate. - pub passes: FxHashSet, + /// Whether the collapsed pass ran + pub collapsed: bool, /// The base-URL of the issue tracker for when an item has been tagged with /// an issue number. pub issue_tracker_base_url: Option, @@ -229,15 +229,10 @@ impl SharedContext { } impl SharedContext { - /// Returns `true` if the `collapse-docs` pass was run on this crate. - pub fn was_collapsed(&self) -> bool { - self.passes.contains("collapse-docs") - } - /// Based on whether the `collapse-docs` pass was run, return either the `doc_value` or the /// `collapsed_doc_value` of the given item. pub fn maybe_collapsed_doc_value<'a>(&self, item: &'a clean::Item) -> Option> { - if self.was_collapsed() { + if self.collapsed { item.collapsed_doc_value().map(|s| s.into()) } else { item.doc_value().map(|s| s.into()) @@ -526,7 +521,6 @@ pub fn initial_ids() -> Vec { /// Generates the documentation for `crate` into the directory `dst` pub fn run(mut krate: clean::Crate, options: RenderOptions, - passes: FxHashSet, renderinfo: RenderInfo, diag: &errors::Handler, edition: Edition) -> Result<(), Error> { @@ -557,8 +551,8 @@ pub fn run(mut krate: clean::Crate, }; let mut errors = Arc::new(ErrorStorage::new()); let mut scx = SharedContext { + collapsed: krate.collapsed, src_root, - passes, include_sources: true, local_sources: Default::default(), issue_tracker_base_url: None, diff --git a/src/librustdoc/lib.rs b/src/librustdoc/lib.rs index 799b5d923ae89..e63a76614bc99 100644 --- a/src/librustdoc/lib.rs +++ b/src/librustdoc/lib.rs @@ -80,7 +80,6 @@ struct Output { krate: clean::Crate, renderinfo: html::render::RenderInfo, renderopts: config::RenderOptions, - passes: Vec, } pub fn main() { @@ -419,14 +418,13 @@ fn main_options(options: config::Options) -> i32 { return rustc_driver::EXIT_SUCCESS; } - let Output { krate, passes, renderinfo, renderopts } = out; + let Output { krate, renderinfo, renderopts } = out; info!("going to format"); let (error_format, treat_err_as_bug, ui_testing, edition) = diag_opts; let diag = core::new_handler(error_format, None, treat_err_as_bug, ui_testing); match html::render::run( krate, renderopts, - passes.into_iter().collect(), renderinfo, &diag, edition, @@ -459,7 +457,7 @@ where R: 'static + Send, let result = rustc_driver::report_ices_to_stderr_if_any(move || { let crate_name = options.crate_name.clone(); let crate_version = options.crate_version.clone(); - let (mut krate, renderinfo, renderopts, passes) = core::run_core(options); + let (mut krate, renderinfo, renderopts) = core::run_core(options); info!("finished with rustc"); @@ -473,7 +471,6 @@ where R: 'static + Send, krate: krate, renderinfo: renderinfo, renderopts, - passes: passes })).unwrap(); }); diff --git a/src/librustdoc/passes/collapse_docs.rs b/src/librustdoc/passes/collapse_docs.rs index 144ff226c4283..31288345ce57b 100644 --- a/src/librustdoc/passes/collapse_docs.rs +++ b/src/librustdoc/passes/collapse_docs.rs @@ -30,7 +30,9 @@ impl DocFragment { } pub fn collapse_docs(krate: clean::Crate, _: &DocContext<'_>) -> clean::Crate { - Collapser.fold_crate(krate) + let mut krate = Collapser.fold_crate(krate); + krate.collapsed = true; + krate } struct Collapser; diff --git a/src/librustdoc/passes/mod.rs b/src/librustdoc/passes/mod.rs index 4e10b46bc8a6f..641a6df221446 100644 --- a/src/librustdoc/passes/mod.rs +++ b/src/librustdoc/passes/mod.rs @@ -57,8 +57,9 @@ pub struct Pass { pub description: &'static str, } + /// The full list of passes. -pub const PASSES: &'static [Pass] = &[ +pub const PASSES: &[Pass] = &[ CHECK_PRIVATE_ITEMS_DOC_TESTS, STRIP_HIDDEN, UNINDENT_COMMENTS, @@ -73,43 +74,43 @@ pub const PASSES: &'static [Pass] = &[ ]; /// The list of passes run by default. -pub const DEFAULT_PASSES: &[&str] = &[ - "collect-trait-impls", - "collapse-docs", - "unindent-comments", - "check-private-items-doc-tests", - "strip-hidden", - "strip-private", - "collect-intra-doc-links", - "check-code-block-syntax", - "propagate-doc-cfg", +pub const DEFAULT_PASSES: &[Pass] = &[ + COLLECT_TRAIT_IMPLS, + COLLAPSE_DOCS, + UNINDENT_COMMENTS, + CHECK_PRIVATE_ITEMS_DOC_TESTS, + STRIP_HIDDEN, + STRIP_PRIVATE, + COLLECT_INTRA_DOC_LINKS, + CHECK_CODE_BLOCK_SYNTAX, + PROPAGATE_DOC_CFG, ]; /// The list of default passes run with `--document-private-items` is passed to rustdoc. -pub const DEFAULT_PRIVATE_PASSES: &[&str] = &[ - "collect-trait-impls", - "collapse-docs", - "unindent-comments", - "check-private-items-doc-tests", - "strip-priv-imports", - "collect-intra-doc-links", - "check-code-block-syntax", - "propagate-doc-cfg", +pub const DEFAULT_PRIVATE_PASSES: &[Pass] = &[ + COLLECT_TRAIT_IMPLS, + COLLAPSE_DOCS, + UNINDENT_COMMENTS, + CHECK_PRIVATE_ITEMS_DOC_TESTS, + STRIP_PRIV_IMPORTS, + COLLECT_INTRA_DOC_LINKS, + CHECK_CODE_BLOCK_SYNTAX, + PROPAGATE_DOC_CFG, ]; /// The list of default passes run when `--doc-coverage` is passed to rustdoc. -pub const DEFAULT_COVERAGE_PASSES: &'static [&'static str] = &[ - "collect-trait-impls", - "strip-hidden", - "strip-private", - "calculate-doc-coverage", +pub const DEFAULT_COVERAGE_PASSES: &[Pass] = &[ + COLLECT_TRAIT_IMPLS, + STRIP_HIDDEN, + STRIP_PRIVATE, + CALCULATE_DOC_COVERAGE, ]; /// The list of default passes run when `--doc-coverage --document-private-items` is passed to /// rustdoc. -pub const PRIVATE_COVERAGE_PASSES: &'static [&'static str] = &[ - "collect-trait-impls", - "calculate-doc-coverage", +pub const PRIVATE_COVERAGE_PASSES: &[Pass] = &[ + COLLECT_TRAIT_IMPLS, + CALCULATE_DOC_COVERAGE, ]; /// A shorthand way to refer to which set of passes to use, based on the presence of @@ -124,7 +125,7 @@ pub enum DefaultPassOption { } /// Returns the given default set of passes. -pub fn defaults(default_set: DefaultPassOption) -> &'static [&'static str] { +pub fn defaults(default_set: DefaultPassOption) -> &'static [Pass] { match default_set { DefaultPassOption::Default => DEFAULT_PASSES, DefaultPassOption::Private => DEFAULT_PRIVATE_PASSES, From 03474801512806591c17d4477d98b883d74ed455 Mon Sep 17 00:00:00 2001 From: Mark Rousskov Date: Sat, 10 Aug 2019 16:59:21 -0400 Subject: [PATCH 08/14] Don't store all traits in DocContext This is already a query so we're just needlessly copying the data around. --- src/librustdoc/clean/blanket_impl.rs | 3 ++- src/librustdoc/core.rs | 5 +---- src/librustdoc/passes/collect_trait_impls.rs | 4 ++-- 3 files changed, 5 insertions(+), 7 deletions(-) diff --git a/src/librustdoc/clean/blanket_impl.rs b/src/librustdoc/clean/blanket_impl.rs index 64cffaec2eaf3..490d4107c51ab 100644 --- a/src/librustdoc/clean/blanket_impl.rs +++ b/src/librustdoc/clean/blanket_impl.rs @@ -3,6 +3,7 @@ use rustc::traits; use rustc::ty::ToPredicate; use rustc::ty::subst::Subst; use rustc::infer::InferOk; +use rustc::hir::def_id::LOCAL_CRATE; use syntax_pos::DUMMY_SP; use super::*; @@ -27,7 +28,7 @@ impl<'a, 'tcx> BlanketImplFinder<'a, 'tcx> { debug!("get_blanket_impls({:?})", ty); let mut impls = Vec::new(); - for &trait_def_id in self.cx.all_traits.iter() { + for &trait_def_id in self.cx.tcx.all_traits(LOCAL_CRATE).iter() { if !self.cx.renderinfo.borrow().access_levels.is_public(trait_def_id) || self.cx.generated_synthetics .borrow_mut() diff --git a/src/librustdoc/core.rs b/src/librustdoc/core.rs index adbe4b469e8d0..c7695fbd8d261 100644 --- a/src/librustdoc/core.rs +++ b/src/librustdoc/core.rs @@ -68,7 +68,6 @@ pub struct DocContext<'tcx> { /// Auto-trait or blanket impls processed so far, as `(self_ty, trait_def_id)`. // FIXME(eddyb) make this a `ty::TraitRef<'tcx>` set. pub generated_synthetics: RefCell, DefId)>>, - pub all_traits: Vec, pub auto_traits: Vec, } @@ -364,7 +363,6 @@ pub fn run_core(options: RustdocOptions) -> (clean::Crate, RenderInfo, RenderOpt let mut renderinfo = RenderInfo::default(); renderinfo.access_levels = access_levels; - let all_traits = tcx.all_traits(LOCAL_CRATE).to_vec(); let ctxt = DocContext { tcx, resolver, @@ -379,10 +377,9 @@ pub fn run_core(options: RustdocOptions) -> (clean::Crate, RenderInfo, RenderOpt fake_def_ids: Default::default(), all_fake_def_ids: Default::default(), generated_synthetics: Default::default(), - auto_traits: all_traits.iter().cloned().filter(|trait_def_id| { + auto_traits: tcx.all_traits(LOCAL_CRATE).iter().cloned().filter(|trait_def_id| { tcx.trait_is_auto(*trait_def_id) }).collect(), - all_traits, }; debug!("crate: {:?}", tcx.hir().krate()); diff --git a/src/librustdoc/passes/collect_trait_impls.rs b/src/librustdoc/passes/collect_trait_impls.rs index cd488b9df78d2..86e4e9fd95637 100644 --- a/src/librustdoc/passes/collect_trait_impls.rs +++ b/src/librustdoc/passes/collect_trait_impls.rs @@ -4,7 +4,7 @@ use crate::fold::DocFolder; use super::Pass; use rustc::util::nodemap::FxHashSet; -use rustc::hir::def_id::DefId; +use rustc::hir::def_id::{LOCAL_CRATE, DefId}; use syntax::symbol::sym; pub const COLLECT_TRAIT_IMPLS: Pass = Pass { @@ -116,7 +116,7 @@ pub fn collect_trait_impls(krate: Crate, cx: &DocContext<'_>) -> Crate { // `tcx.crates()` doesn't include the local crate, and `tcx.all_trait_implementations` // doesn't work with it anyway, so pull them from the HIR map instead - for &trait_did in cx.all_traits.iter() { + for &trait_did in cx.tcx.all_traits(LOCAL_CRATE).iter() { for &impl_node in cx.tcx.hir().trait_impls(trait_did) { let impl_did = cx.tcx.hir().local_def_id(impl_node); inline::build_impl(cx, impl_did, None, &mut new_items); From eea2f879afbaad07c562ba572482f402bc6c0006 Mon Sep 17 00:00:00 2001 From: Mark Rousskov Date: Sat, 10 Aug 2019 17:10:13 -0400 Subject: [PATCH 09/14] Use a HashSet instead of Vec --- src/librustdoc/clean/inline.rs | 8 +++----- src/librustdoc/core.rs | 2 +- 2 files changed, 4 insertions(+), 6 deletions(-) diff --git a/src/librustdoc/clean/inline.rs b/src/librustdoc/clean/inline.rs index a8336607f7ae9..6f93c95edef08 100644 --- a/src/librustdoc/clean/inline.rs +++ b/src/librustdoc/clean/inline.rs @@ -581,13 +581,11 @@ pub fn record_extern_trait(cx: &DocContext<'_>, did: DefId) { } } - cx.active_extern_traits.borrow_mut().push(did); + cx.active_extern_traits.borrow_mut().insert(did); debug!("record_extern_trait: {:?}", did); let trait_ = build_external_trait(cx, did); - { - cx.external_traits.borrow_mut().insert(did, trait_); - } - cx.active_extern_traits.borrow_mut().remove_item(&did); + cx.external_traits.borrow_mut().insert(did, trait_); + cx.active_extern_traits.borrow_mut().remove(&did); } diff --git a/src/librustdoc/core.rs b/src/librustdoc/core.rs index c7695fbd8d261..87381f224d0bb 100644 --- a/src/librustdoc/core.rs +++ b/src/librustdoc/core.rs @@ -51,7 +51,7 @@ pub struct DocContext<'tcx> { pub external_traits: Rc>>, /// Used while populating `external_traits` to ensure we don't process the same trait twice at /// the same time. - pub active_extern_traits: RefCell>, + pub active_extern_traits: RefCell>, // The current set of type and lifetime substitutions, // for expanding type aliases at the HIR level: From ade8b02828de9653e6aca122f1a0f6d8c48ad29b Mon Sep 17 00:00:00 2001 From: Mark Rousskov Date: Sat, 10 Aug 2019 17:16:01 -0400 Subject: [PATCH 10/14] Remove unnecessary channel --- src/librustdoc/lib.rs | 9 +++------ 1 file changed, 3 insertions(+), 6 deletions(-) diff --git a/src/librustdoc/lib.rs b/src/librustdoc/lib.rs index e63a76614bc99..e30b35937db9f 100644 --- a/src/librustdoc/lib.rs +++ b/src/librustdoc/lib.rs @@ -44,7 +44,6 @@ use std::default::Default; use std::env; use std::panic; use std::process; -use std::sync::mpsc::channel; use rustc::session::{early_warn, early_error}; use rustc::session::config::{ErrorOutputType, RustcOptGroup}; @@ -452,8 +451,6 @@ where R: 'static + Send, // First, parse the crate and extract all relevant information. info!("starting to run rustc"); - let (tx, rx) = channel(); - let result = rustc_driver::report_ices_to_stderr_if_any(move || { let crate_name = options.crate_name.clone(); let crate_version = options.crate_version.clone(); @@ -467,15 +464,15 @@ where R: 'static + Send, krate.version = crate_version; - tx.send(f(Output { + f(Output { krate: krate, renderinfo: renderinfo, renderopts, - })).unwrap(); + }) }); match result { - Ok(()) => rx.recv().unwrap(), + Ok(output) => output, Err(_) => panic::resume_unwind(Box::new(errors::FatalErrorMarker)), } } From dbad77ffdd59e54b3f496cfbdb7909a6bbd03031 Mon Sep 17 00:00:00 2001 From: Mark Rousskov Date: Sat, 10 Aug 2019 18:07:07 -0400 Subject: [PATCH 11/14] Remove thread-local for playground config --- src/librustdoc/config.rs | 2 +- src/librustdoc/externalfiles.rs | 8 +- src/librustdoc/html/markdown.rs | 203 +++++++++++++----------- src/librustdoc/html/render.rs | 29 ++-- src/librustdoc/markdown.rs | 11 +- src/tools/error_index_generator/main.rs | 11 +- 6 files changed, 147 insertions(+), 117 deletions(-) diff --git a/src/librustdoc/config.rs b/src/librustdoc/config.rs index 2be67d707fe14..98ab957ecbb38 100644 --- a/src/librustdoc/config.rs +++ b/src/librustdoc/config.rs @@ -378,7 +378,7 @@ impl Options { &matches.opt_strs("html-after-content"), &matches.opt_strs("markdown-before-content"), &matches.opt_strs("markdown-after-content"), - &diag, &mut id_map, edition) { + &diag, &mut id_map, edition, &None) { Some(eh) => eh, None => return Err(3), }; diff --git a/src/librustdoc/externalfiles.rs b/src/librustdoc/externalfiles.rs index d604ba11d4186..d920b7c4c9169 100644 --- a/src/librustdoc/externalfiles.rs +++ b/src/librustdoc/externalfiles.rs @@ -4,7 +4,7 @@ use std::str; use errors; use crate::syntax::feature_gate::UnstableFeatures; use crate::syntax::edition::Edition; -use crate::html::markdown::{IdMap, ErrorCodes, Markdown}; +use crate::html::markdown::{IdMap, ErrorCodes, Markdown, Playground}; use std::cell::RefCell; @@ -24,7 +24,7 @@ pub struct ExternalHtml { impl ExternalHtml { pub fn load(in_header: &[String], before_content: &[String], after_content: &[String], md_before_content: &[String], md_after_content: &[String], diag: &errors::Handler, - id_map: &mut IdMap, edition: Edition) + id_map: &mut IdMap, edition: Edition, playground: &Option) -> Option { let codes = ErrorCodes::from(UnstableFeatures::from_environment().is_nightly_build()); load_external_files(in_header, diag) @@ -36,7 +36,7 @@ impl ExternalHtml { load_external_files(md_before_content, diag) .map(|m_bc| (ih, format!("{}{}", bc, Markdown(&m_bc, &[], RefCell::new(id_map), - codes, edition)))) + codes, edition, playground)))) ) .and_then(|(ih, bc)| load_external_files(after_content, diag) @@ -46,7 +46,7 @@ impl ExternalHtml { load_external_files(md_after_content, diag) .map(|m_ac| (ih, bc, format!("{}{}", ac, Markdown(&m_ac, &[], RefCell::new(id_map), - codes, edition)))) + codes, edition, playground)))) ) .map(|(ih, bc, ac)| ExternalHtml { diff --git a/src/librustdoc/html/markdown.rs b/src/librustdoc/html/markdown.rs index ef52ce62875c7..73233a2289ccd 100644 --- a/src/librustdoc/html/markdown.rs +++ b/src/librustdoc/html/markdown.rs @@ -17,7 +17,7 @@ //! let s = "My *markdown* _text_"; //! let mut id_map = IdMap::new(); //! let html = format!("{}", Markdown(s, &[], RefCell::new(&mut id_map), -//! ErrorCodes::Yes, Edition::Edition2015)); +//! ErrorCodes::Yes, Edition::Edition2015, None)); //! // ... something using html //! ``` @@ -59,6 +59,7 @@ pub struct Markdown<'a>( pub ErrorCodes, /// Default edition to use when parsing doctests (to add a `fn main`). pub Edition, + pub &'a Option, ); /// A tuple struct like `Markdown` that renders the markdown with a table of contents. pub struct MarkdownWithToc<'a>( @@ -66,9 +67,16 @@ pub struct MarkdownWithToc<'a>( pub RefCell<&'a mut IdMap>, pub ErrorCodes, pub Edition, + pub &'a Option, ); /// A tuple struct like `Markdown` that renders the markdown escaping HTML tags. -pub struct MarkdownHtml<'a>(pub &'a str, pub RefCell<&'a mut IdMap>, pub ErrorCodes, pub Edition); +pub struct MarkdownHtml<'a>( + pub &'a str, + pub RefCell<&'a mut IdMap>, + pub ErrorCodes, + pub Edition, + pub &'a Option, +); /// A tuple struct like `Markdown` that renders only the first paragraph. pub struct MarkdownSummaryLine<'a>(pub &'a str, pub &'a [(String, String)]); @@ -155,30 +163,39 @@ fn slugify(c: char) -> Option { } } -// Information about the playground if a URL has been specified, containing an -// optional crate name and the URL. -thread_local!(pub static PLAYGROUND: RefCell, String)>> = { - RefCell::new(None) -}); +#[derive(Clone, Debug)] +pub struct Playground { + pub crate_name: Option, + pub url: String, +} /// Adds syntax highlighting and playground Run buttons to Rust code blocks. -struct CodeBlocks<'a, I: Iterator>> { +struct CodeBlocks<'p, 'a, I: Iterator>> { inner: I, check_error_codes: ErrorCodes, edition: Edition, + // Information about the playground if a URL has been specified, containing an + // optional crate name and the URL. + playground: &'p Option, } -impl<'a, I: Iterator>> CodeBlocks<'a, I> { - fn new(iter: I, error_codes: ErrorCodes, edition: Edition) -> Self { +impl<'p, 'a, I: Iterator>> CodeBlocks<'p, 'a, I> { + fn new( + iter: I, + error_codes: ErrorCodes, + edition: Edition, + playground: &'p Option, + ) -> Self { CodeBlocks { inner: iter, check_error_codes: error_codes, edition, + playground, } } } -impl<'a, I: Iterator>> Iterator for CodeBlocks<'a, I> { +impl<'a, I: Iterator>> Iterator for CodeBlocks<'_, 'a, I> { type Item = Event<'a>; fn next(&mut self) -> Option { @@ -213,86 +230,86 @@ impl<'a, I: Iterator>> Iterator for CodeBlocks<'a, I> { } let lines = origtext.lines().filter_map(|l| map_line(l).for_html()); let text = lines.collect::>>().join("\n"); - PLAYGROUND.with(|play| { - // insert newline to clearly separate it from the - // previous block so we can shorten the html output - let mut s = String::from("\n"); - let playground_button = play.borrow().as_ref().and_then(|&(ref krate, ref url)| { - if url.is_empty() { - return None; - } - let test = origtext.lines() - .map(|l| map_line(l).for_code()) - .collect::>>().join("\n"); - let krate = krate.as_ref().map(|s| &**s); - let (test, _) = test::make_test(&test, krate, false, - &Default::default(), edition); - let channel = if test.contains("#![feature(") { - "&version=nightly" - } else { - "" - }; - - let edition_string = format!("&edition={}", edition); - - // These characters don't need to be escaped in a URI. - // FIXME: use a library function for percent encoding. - fn dont_escape(c: u8) -> bool { - (b'a' <= c && c <= b'z') || - (b'A' <= c && c <= b'Z') || - (b'0' <= c && c <= b'9') || - c == b'-' || c == b'_' || c == b'.' || - c == b'~' || c == b'!' || c == b'\'' || - c == b'(' || c == b')' || c == b'*' - } - let mut test_escaped = String::new(); - for b in test.bytes() { - if dont_escape(b) { - test_escaped.push(char::from(b)); - } else { - write!(test_escaped, "%{:02X}", b).unwrap(); - } - } - Some(format!( - r#"Run"#, - url, test_escaped, channel, edition_string - )) - }); - - let tooltip = if ignore { - Some(("This example is not tested".to_owned(), "ignore")) - } else if compile_fail { - Some(("This example deliberately fails to compile".to_owned(), "compile_fail")) - } else if explicit_edition { - Some((format!("This code runs with edition {}", edition), "edition")) + // insert newline to clearly separate it from the + // previous block so we can shorten the html output + let mut s = String::from("\n"); + let playground_button = self.playground.as_ref().and_then(|playground| { + let krate = &playground.crate_name; + let url = &playground.url; + if url.is_empty() { + return None; + } + let test = origtext.lines() + .map(|l| map_line(l).for_code()) + .collect::>>().join("\n"); + let krate = krate.as_ref().map(|s| &**s); + let (test, _) = test::make_test(&test, krate, false, + &Default::default(), edition); + let channel = if test.contains("#![feature(") { + "&version=nightly" } else { - None + "" }; - if let Some((s1, s2)) = tooltip { - s.push_str(&highlight::render_with_highlighting( - &text, - Some(&format!("rust-example-rendered{}", - if ignore { " ignore" } - else if compile_fail { " compile_fail" } - else if explicit_edition { " edition " } - else { "" })), - playground_button.as_ref().map(String::as_str), - Some((s1.as_str(), s2)))); - Some(Event::Html(s.into())) - } else { - s.push_str(&highlight::render_with_highlighting( - &text, - Some(&format!("rust-example-rendered{}", - if ignore { " ignore" } - else if compile_fail { " compile_fail" } - else if explicit_edition { " edition " } - else { "" })), - playground_button.as_ref().map(String::as_str), - None)); - Some(Event::Html(s.into())) + let edition_string = format!("&edition={}", edition); + + // These characters don't need to be escaped in a URI. + // FIXME: use a library function for percent encoding. + fn dont_escape(c: u8) -> bool { + (b'a' <= c && c <= b'z') || + (b'A' <= c && c <= b'Z') || + (b'0' <= c && c <= b'9') || + c == b'-' || c == b'_' || c == b'.' || + c == b'~' || c == b'!' || c == b'\'' || + c == b'(' || c == b')' || c == b'*' } - }) + let mut test_escaped = String::new(); + for b in test.bytes() { + if dont_escape(b) { + test_escaped.push(char::from(b)); + } else { + write!(test_escaped, "%{:02X}", b).unwrap(); + } + } + Some(format!( + r#"Run"#, + url, test_escaped, channel, edition_string + )) + }); + + let tooltip = if ignore { + Some(("This example is not tested".to_owned(), "ignore")) + } else if compile_fail { + Some(("This example deliberately fails to compile".to_owned(), "compile_fail")) + } else if explicit_edition { + Some((format!("This code runs with edition {}", edition), "edition")) + } else { + None + }; + + if let Some((s1, s2)) = tooltip { + s.push_str(&highlight::render_with_highlighting( + &text, + Some(&format!("rust-example-rendered{}", + if ignore { " ignore" } + else if compile_fail { " compile_fail" } + else if explicit_edition { " edition " } + else { "" })), + playground_button.as_ref().map(String::as_str), + Some((s1.as_str(), s2)))); + Some(Event::Html(s.into())) + } else { + s.push_str(&highlight::render_with_highlighting( + &text, + Some(&format!("rust-example-rendered{}", + if ignore { " ignore" } + else if compile_fail { " compile_fail" } + else if explicit_edition { " edition " } + else { "" })), + playground_button.as_ref().map(String::as_str), + None)); + Some(Event::Html(s.into())) + } } } @@ -676,7 +693,7 @@ impl LangString { impl<'a> fmt::Display for Markdown<'a> { fn fmt(&self, fmt: &mut fmt::Formatter<'_>) -> fmt::Result { - let Markdown(md, links, ref ids, codes, edition) = *self; + let Markdown(md, links, ref ids, codes, edition, playground) = *self; let mut ids = ids.borrow_mut(); // This is actually common enough to special-case @@ -695,7 +712,7 @@ impl<'a> fmt::Display for Markdown<'a> { let p = HeadingLinks::new(p, None, &mut ids); let p = LinkReplacer::new(p, links); - let p = CodeBlocks::new(p, codes, edition); + let p = CodeBlocks::new(p, codes, edition, playground); let p = Footnotes::new(p); html::push_html(&mut s, p); @@ -705,7 +722,7 @@ impl<'a> fmt::Display for Markdown<'a> { impl<'a> fmt::Display for MarkdownWithToc<'a> { fn fmt(&self, fmt: &mut fmt::Formatter<'_>) -> fmt::Result { - let MarkdownWithToc(md, ref ids, codes, edition) = *self; + let MarkdownWithToc(md, ref ids, codes, edition, playground) = *self; let mut ids = ids.borrow_mut(); let p = Parser::new_ext(md, opts()); @@ -716,7 +733,7 @@ impl<'a> fmt::Display for MarkdownWithToc<'a> { { let p = HeadingLinks::new(p, Some(&mut toc), &mut ids); - let p = CodeBlocks::new(p, codes, edition); + let p = CodeBlocks::new(p, codes, edition, playground); let p = Footnotes::new(p); html::push_html(&mut s, p); } @@ -729,7 +746,7 @@ impl<'a> fmt::Display for MarkdownWithToc<'a> { impl<'a> fmt::Display for MarkdownHtml<'a> { fn fmt(&self, fmt: &mut fmt::Formatter<'_>) -> fmt::Result { - let MarkdownHtml(md, ref ids, codes, edition) = *self; + let MarkdownHtml(md, ref ids, codes, edition, playground) = *self; let mut ids = ids.borrow_mut(); // This is actually common enough to special-case @@ -745,7 +762,7 @@ impl<'a> fmt::Display for MarkdownHtml<'a> { let mut s = String::with_capacity(md.len() * 3 / 2); let p = HeadingLinks::new(p, None, &mut ids); - let p = CodeBlocks::new(p, codes, edition); + let p = CodeBlocks::new(p, codes, edition, playground); let p = Footnotes::new(p); html::push_html(&mut s, p); diff --git a/src/librustdoc/html/render.rs b/src/librustdoc/html/render.rs index eb88c72da9eeb..8a7dfebdbbc1a 100644 --- a/src/librustdoc/html/render.rs +++ b/src/librustdoc/html/render.rs @@ -170,6 +170,7 @@ struct Context { /// The map used to ensure all generated 'id=' attributes are unique. id_map: Rc>, pub shared: Arc, + playground: Option, } struct SharedContext { @@ -574,9 +575,11 @@ pub fn run(mut krate: clean::Crate, }; // If user passed in `--playground-url` arg, we fill in crate name here + let mut playground = None; if let Some(url) = playground_url { - markdown::PLAYGROUND.with(|slot| { - *slot.borrow_mut() = Some((Some(krate.name.clone()), url)); + playground = Some(markdown::Playground { + crate_name: Some(krate.name.clone()), + url, }); } @@ -592,9 +595,9 @@ pub fn run(mut krate: clean::Crate, scx.layout.logo = s.to_string(); } (sym::html_playground_url, Some(s)) => { - markdown::PLAYGROUND.with(|slot| { - let name = krate.name.clone(); - *slot.borrow_mut() = Some((Some(name), s.to_string())); + playground = Some(markdown::Playground { + crate_name: Some(krate.name.clone()), + url: s.to_string(), }); } (sym::issue_tracker_base_url, Some(s)) => { @@ -618,6 +621,7 @@ pub fn run(mut krate: clean::Crate, edition, id_map: Rc::new(RefCell::new(id_map)), shared: Arc::new(scx), + playground, }; // Crawl the crate to build various caches used for the output @@ -2592,7 +2596,7 @@ fn render_markdown(w: &mut fmt::Formatter<'_>, if is_hidden { " hidden" } else { "" }, prefix, Markdown(md_text, &links, RefCell::new(&mut ids), - cx.codes, cx.edition)) + cx.codes, cx.edition, &cx.playground)) } fn document_short( @@ -2957,7 +2961,8 @@ fn short_stability(item: &clean::Item, cx: &Context) -> Vec { if let Some(note) = note { let mut ids = cx.id_map.borrow_mut(); - let html = MarkdownHtml(¬e, RefCell::new(&mut ids), error_codes, cx.edition); + let html = MarkdownHtml( + ¬e, RefCell::new(&mut ids), error_codes, cx.edition, &cx.playground); message.push_str(&format!(": {}", html)); } stability.push(format!("
{}
", message)); @@ -3006,7 +3011,13 @@ fn short_stability(item: &clean::Item, cx: &Context) -> Vec { message = format!( "
{}{}
", message, - MarkdownHtml(&unstable_reason, RefCell::new(&mut ids), error_codes, cx.edition) + MarkdownHtml( + &unstable_reason, + RefCell::new(&mut ids), + error_codes, + cx.edition, + &cx.playground, + ) ); } @@ -4237,7 +4248,7 @@ fn render_impl(w: &mut fmt::Formatter<'_>, cx: &Context, i: &Impl, link: AssocIt let mut ids = cx.id_map.borrow_mut(); write!(w, "
{}
", Markdown(&*dox, &i.impl_item.links(), RefCell::new(&mut ids), - cx.codes, cx.edition))?; + cx.codes, cx.edition, &cx.playground))?; } } diff --git a/src/librustdoc/markdown.rs b/src/librustdoc/markdown.rs index 50a647f244db5..b7dd6c30f09f9 100644 --- a/src/librustdoc/markdown.rs +++ b/src/librustdoc/markdown.rs @@ -60,9 +60,10 @@ pub fn render( }; let playground_url = options.markdown_playground_url .or(options.playground_url); - if let Some(playground) = playground_url { - markdown::PLAYGROUND.with(|s| { *s.borrow_mut() = Some((None, playground)); }); - } + let playground = playground_url.map(|url| markdown::Playground { + crate_name: None, + url, + }); let mut out = match File::create(&output) { Err(e) => { @@ -82,9 +83,9 @@ pub fn render( let mut ids = IdMap::new(); let error_codes = ErrorCodes::from(UnstableFeatures::from_environment().is_nightly_build()); let text = if !options.markdown_no_toc { - MarkdownWithToc(text, RefCell::new(&mut ids), error_codes, edition).to_string() + MarkdownWithToc(text, RefCell::new(&mut ids), error_codes, edition, &playground).to_string() } else { - Markdown(text, &[], RefCell::new(&mut ids), error_codes, edition).to_string() + Markdown(text, &[], RefCell::new(&mut ids), error_codes, edition, &playground).to_string() }; let err = write!( diff --git a/src/tools/error_index_generator/main.rs b/src/tools/error_index_generator/main.rs index c31a5069e4673..33987b0b54213 100644 --- a/src/tools/error_index_generator/main.rs +++ b/src/tools/error_index_generator/main.rs @@ -16,7 +16,7 @@ use std::cell::RefCell; use syntax::edition::DEFAULT_EDITION; use syntax::diagnostics::metadata::{get_metadata_dir, ErrorMetadataMap, ErrorMetadata}; -use rustdoc::html::markdown::{Markdown, IdMap, ErrorCodes, PLAYGROUND}; +use rustdoc::html::markdown::{Markdown, IdMap, ErrorCodes, Playground}; use rustc_serialize::json; enum OutputFormat { @@ -95,9 +95,13 @@ impl Formatter for HTMLFormatter { match info.description { Some(ref desc) => { let mut id_map = self.0.borrow_mut(); + let playground = Playground { + crate_name: None, + url: String::from("https://play.rust-lang.org/"), + }; write!(output, "{}", Markdown(desc, &[], RefCell::new(&mut id_map), - ErrorCodes::Yes, DEFAULT_EDITION))? + ErrorCodes::Yes, DEFAULT_EDITION, &Some(playground)))? }, None => write!(output, "

No description.

\n")?, } @@ -260,9 +264,6 @@ fn parse_args() -> (OutputFormat, PathBuf) { fn main() { env_logger::init(); - PLAYGROUND.with(|slot| { - *slot.borrow_mut() = Some((None, String::from("https://play.rust-lang.org/"))); - }); let (format, dst) = parse_args(); let result = syntax::with_default_globals(move || { main_with_result(format, &dst) From c250b5fd033ebd8257cca8ee537e752355a151c3 Mon Sep 17 00:00:00 2001 From: Mark Rousskov Date: Sat, 10 Aug 2019 18:27:17 -0400 Subject: [PATCH 12/14] Remove fmt::Display impls on Markdown structs These impls prevent ergonomic use of the config (e.g., forcing us to use RefCell) despite all usecases for these structs only using their Display impls once. --- src/librustdoc/externalfiles.rs | 4 +- src/librustdoc/html/markdown.rs | 57 +++++++++++-------------- src/librustdoc/html/render.rs | 10 ++--- src/tools/error_index_generator/main.rs | 2 +- 4 files changed, 34 insertions(+), 39 deletions(-) diff --git a/src/librustdoc/externalfiles.rs b/src/librustdoc/externalfiles.rs index d920b7c4c9169..5d953eec31ec3 100644 --- a/src/librustdoc/externalfiles.rs +++ b/src/librustdoc/externalfiles.rs @@ -36,7 +36,7 @@ impl ExternalHtml { load_external_files(md_before_content, diag) .map(|m_bc| (ih, format!("{}{}", bc, Markdown(&m_bc, &[], RefCell::new(id_map), - codes, edition, playground)))) + codes, edition, playground).to_string()))) ) .and_then(|(ih, bc)| load_external_files(after_content, diag) @@ -46,7 +46,7 @@ impl ExternalHtml { load_external_files(md_after_content, diag) .map(|m_ac| (ih, bc, format!("{}{}", ac, Markdown(&m_ac, &[], RefCell::new(id_map), - codes, edition, playground)))) + codes, edition, playground).to_string()))) ) .map(|(ih, bc, ac)| ExternalHtml { diff --git a/src/librustdoc/html/markdown.rs b/src/librustdoc/html/markdown.rs index 73233a2289ccd..753832305f9ea 100644 --- a/src/librustdoc/html/markdown.rs +++ b/src/librustdoc/html/markdown.rs @@ -1,9 +1,6 @@ //! Markdown formatting for rustdoc. //! -//! This module implements markdown formatting through the pulldown-cmark -//! rust-library. This module exposes all of the -//! functionality through a unit struct, `Markdown`, which has an implementation -//! of `fmt::Display`. Example usage: +//! This module implements markdown formatting through the pulldown-cmark library. //! //! ``` //! #![feature(rustc_private)] @@ -16,8 +13,9 @@ //! //! let s = "My *markdown* _text_"; //! let mut id_map = IdMap::new(); -//! let html = format!("{}", Markdown(s, &[], RefCell::new(&mut id_map), -//! ErrorCodes::Yes, Edition::Edition2015, None)); +//! let md = Markdown(s, &[], RefCell::new(&mut id_map), +//! ErrorCodes::Yes, Edition::Edition2015, None); +//! let html = md.to_string(); //! // ... something using html //! ``` @@ -27,7 +25,7 @@ use rustc_data_structures::fx::FxHashMap; use std::cell::RefCell; use std::collections::VecDeque; use std::default::Default; -use std::fmt::{self, Write}; +use std::fmt::Write; use std::borrow::Cow; use std::ops::Range; use std::str; @@ -46,9 +44,8 @@ fn opts() -> Options { Options::ENABLE_TABLES | Options::ENABLE_FOOTNOTES } -/// A tuple struct that has the `fmt::Display` trait implemented. -/// When formatted, this struct will emit the HTML corresponding to the rendered -/// version of the contained markdown string. +/// When `to_string` is called, this struct will emit the HTML corresponding to +/// the rendered version of the contained markdown string. pub struct Markdown<'a>( pub &'a str, /// A list of link replacements. @@ -691,13 +688,13 @@ impl LangString { } } -impl<'a> fmt::Display for Markdown<'a> { - fn fmt(&self, fmt: &mut fmt::Formatter<'_>) -> fmt::Result { - let Markdown(md, links, ref ids, codes, edition, playground) = *self; +impl Markdown<'_> { + pub fn to_string(self) -> String { + let Markdown(md, links, ids, codes, edition, playground) = self; let mut ids = ids.borrow_mut(); // This is actually common enough to special-case - if md.is_empty() { return Ok(()) } + if md.is_empty() { return String::new(); } let replacer = |_: &str, s: &str| { if let Some(&(_, ref replace)) = links.into_iter().find(|link| &*link.0 == s) { Some((replace.clone(), s.to_owned())) @@ -716,13 +713,13 @@ impl<'a> fmt::Display for Markdown<'a> { let p = Footnotes::new(p); html::push_html(&mut s, p); - fmt.write_str(&s) + s } } -impl<'a> fmt::Display for MarkdownWithToc<'a> { - fn fmt(&self, fmt: &mut fmt::Formatter<'_>) -> fmt::Result { - let MarkdownWithToc(md, ref ids, codes, edition, playground) = *self; +impl MarkdownWithToc<'_> { + pub fn to_string(self) -> String { + let MarkdownWithToc(md, ref ids, codes, edition, playground) = self; let mut ids = ids.borrow_mut(); let p = Parser::new_ext(md, opts()); @@ -738,19 +735,17 @@ impl<'a> fmt::Display for MarkdownWithToc<'a> { html::push_html(&mut s, p); } - write!(fmt, "", toc.into_toc())?; - - fmt.write_str(&s) + format!("{}", toc.into_toc(), s) } } -impl<'a> fmt::Display for MarkdownHtml<'a> { - fn fmt(&self, fmt: &mut fmt::Formatter<'_>) -> fmt::Result { - let MarkdownHtml(md, ref ids, codes, edition, playground) = *self; +impl MarkdownHtml<'_> { + pub fn to_string(self) -> String { + let MarkdownHtml(md, ref ids, codes, edition, playground) = self; let mut ids = ids.borrow_mut(); // This is actually common enough to special-case - if md.is_empty() { return Ok(()) } + if md.is_empty() { return String::new(); } let p = Parser::new_ext(md, opts()); // Treat inline HTML as plain text. @@ -766,15 +761,15 @@ impl<'a> fmt::Display for MarkdownHtml<'a> { let p = Footnotes::new(p); html::push_html(&mut s, p); - fmt.write_str(&s) + s } } -impl<'a> fmt::Display for MarkdownSummaryLine<'a> { - fn fmt(&self, fmt: &mut fmt::Formatter<'_>) -> fmt::Result { - let MarkdownSummaryLine(md, links) = *self; +impl MarkdownSummaryLine<'_> { + pub fn to_string(self) -> String { + let MarkdownSummaryLine(md, links) = self; // This is actually common enough to special-case - if md.is_empty() { return Ok(()) } + if md.is_empty() { return String::new(); } let replacer = |_: &str, s: &str| { if let Some(&(_, ref replace)) = links.into_iter().find(|link| &*link.0 == s) { @@ -790,7 +785,7 @@ impl<'a> fmt::Display for MarkdownSummaryLine<'a> { html::push_html(&mut s, LinkReplacer::new(SummaryLine::new(p), links)); - fmt.write_str(&s) + s } } diff --git a/src/librustdoc/html/render.rs b/src/librustdoc/html/render.rs index 8a7dfebdbbc1a..fc4e25e3d7ff6 100644 --- a/src/librustdoc/html/render.rs +++ b/src/librustdoc/html/render.rs @@ -2596,7 +2596,7 @@ fn render_markdown(w: &mut fmt::Formatter<'_>, if is_hidden { " hidden" } else { "" }, prefix, Markdown(md_text, &links, RefCell::new(&mut ids), - cx.codes, cx.edition, &cx.playground)) + cx.codes, cx.edition, &cx.playground).to_string()) } fn document_short( @@ -2866,7 +2866,7 @@ fn item_module(w: &mut fmt::Formatter<'_>, cx: &Context, ", name = *myitem.name.as_ref().unwrap(), stab_tags = stability_tags(myitem), - docs = MarkdownSummaryLine(doc_value, &myitem.links()), + docs = MarkdownSummaryLine(doc_value, &myitem.links()).to_string(), class = myitem.type_(), add = add, stab = stab.unwrap_or_else(|| String::new()), @@ -2963,7 +2963,7 @@ fn short_stability(item: &clean::Item, cx: &Context) -> Vec { let mut ids = cx.id_map.borrow_mut(); let html = MarkdownHtml( ¬e, RefCell::new(&mut ids), error_codes, cx.edition, &cx.playground); - message.push_str(&format!(": {}", html)); + message.push_str(&format!(": {}", html.to_string())); } stability.push(format!("
{}
", message)); } @@ -3017,7 +3017,7 @@ fn short_stability(item: &clean::Item, cx: &Context) -> Vec { error_codes, cx.edition, &cx.playground, - ) + ).to_string() ); } @@ -4248,7 +4248,7 @@ fn render_impl(w: &mut fmt::Formatter<'_>, cx: &Context, i: &Impl, link: AssocIt let mut ids = cx.id_map.borrow_mut(); write!(w, "
{}
", Markdown(&*dox, &i.impl_item.links(), RefCell::new(&mut ids), - cx.codes, cx.edition, &cx.playground))?; + cx.codes, cx.edition, &cx.playground).to_string())?; } } diff --git a/src/tools/error_index_generator/main.rs b/src/tools/error_index_generator/main.rs index 33987b0b54213..89b545fb7e413 100644 --- a/src/tools/error_index_generator/main.rs +++ b/src/tools/error_index_generator/main.rs @@ -101,7 +101,7 @@ impl Formatter for HTMLFormatter { }; write!(output, "{}", Markdown(desc, &[], RefCell::new(&mut id_map), - ErrorCodes::Yes, DEFAULT_EDITION, &Some(playground)))? + ErrorCodes::Yes, DEFAULT_EDITION, &Some(playground)).to_string())? }, None => write!(output, "

No description.

\n")?, } From 1aa0964b543e0e21cadee2fed6a7725b594e92be Mon Sep 17 00:00:00 2001 From: Mark Rousskov Date: Sat, 10 Aug 2019 18:36:04 -0400 Subject: [PATCH 13/14] Drop RefCell from IdMap in markdown rendering --- src/librustdoc/externalfiles.rs | 6 ++---- src/librustdoc/html/markdown.rs | 19 +++++++------------ src/librustdoc/html/markdown/tests.rs | 12 ++++++------ src/librustdoc/html/render.rs | 9 ++++----- src/librustdoc/markdown.rs | 5 ++--- src/tools/error_index_generator/main.rs | 2 +- 6 files changed, 22 insertions(+), 31 deletions(-) diff --git a/src/librustdoc/externalfiles.rs b/src/librustdoc/externalfiles.rs index 5d953eec31ec3..8254bc800ca46 100644 --- a/src/librustdoc/externalfiles.rs +++ b/src/librustdoc/externalfiles.rs @@ -6,8 +6,6 @@ use crate::syntax::feature_gate::UnstableFeatures; use crate::syntax::edition::Edition; use crate::html::markdown::{IdMap, ErrorCodes, Markdown, Playground}; -use std::cell::RefCell; - #[derive(Clone, Debug)] pub struct ExternalHtml { /// Content that will be included inline in the section of a @@ -35,7 +33,7 @@ impl ExternalHtml { .and_then(|(ih, bc)| load_external_files(md_before_content, diag) .map(|m_bc| (ih, - format!("{}{}", bc, Markdown(&m_bc, &[], RefCell::new(id_map), + format!("{}{}", bc, Markdown(&m_bc, &[], id_map, codes, edition, playground).to_string()))) ) .and_then(|(ih, bc)| @@ -45,7 +43,7 @@ impl ExternalHtml { .and_then(|(ih, bc, ac)| load_external_files(md_after_content, diag) .map(|m_ac| (ih, bc, - format!("{}{}", ac, Markdown(&m_ac, &[], RefCell::new(id_map), + format!("{}{}", ac, Markdown(&m_ac, &[], id_map, codes, edition, playground).to_string()))) ) .map(|(ih, bc, ac)| diff --git a/src/librustdoc/html/markdown.rs b/src/librustdoc/html/markdown.rs index 753832305f9ea..5a7deb651b00d 100644 --- a/src/librustdoc/html/markdown.rs +++ b/src/librustdoc/html/markdown.rs @@ -9,12 +9,10 @@ //! //! use syntax::edition::Edition; //! use rustdoc::html::markdown::{IdMap, Markdown, ErrorCodes}; -//! use std::cell::RefCell; //! //! let s = "My *markdown* _text_"; //! let mut id_map = IdMap::new(); -//! let md = Markdown(s, &[], RefCell::new(&mut id_map), -//! ErrorCodes::Yes, Edition::Edition2015, None); +//! let md = Markdown(s, &[], &mut id_map, ErrorCodes::Yes, Edition::Edition2015, &None); //! let html = md.to_string(); //! // ... something using html //! ``` @@ -51,7 +49,7 @@ pub struct Markdown<'a>( /// A list of link replacements. pub &'a [(String, String)], /// The current list of used header IDs. - pub RefCell<&'a mut IdMap>, + pub &'a mut IdMap, /// Whether to allow the use of explicit error codes in doctest lang strings. pub ErrorCodes, /// Default edition to use when parsing doctests (to add a `fn main`). @@ -61,7 +59,7 @@ pub struct Markdown<'a>( /// A tuple struct like `Markdown` that renders the markdown with a table of contents. pub struct MarkdownWithToc<'a>( pub &'a str, - pub RefCell<&'a mut IdMap>, + pub &'a mut IdMap, pub ErrorCodes, pub Edition, pub &'a Option, @@ -69,7 +67,7 @@ pub struct MarkdownWithToc<'a>( /// A tuple struct like `Markdown` that renders the markdown escaping HTML tags. pub struct MarkdownHtml<'a>( pub &'a str, - pub RefCell<&'a mut IdMap>, + pub &'a mut IdMap, pub ErrorCodes, pub Edition, pub &'a Option, @@ -690,8 +688,7 @@ impl LangString { impl Markdown<'_> { pub fn to_string(self) -> String { - let Markdown(md, links, ids, codes, edition, playground) = self; - let mut ids = ids.borrow_mut(); + let Markdown(md, links, mut ids, codes, edition, playground) = self; // This is actually common enough to special-case if md.is_empty() { return String::new(); } @@ -719,8 +716,7 @@ impl Markdown<'_> { impl MarkdownWithToc<'_> { pub fn to_string(self) -> String { - let MarkdownWithToc(md, ref ids, codes, edition, playground) = self; - let mut ids = ids.borrow_mut(); + let MarkdownWithToc(md, mut ids, codes, edition, playground) = self; let p = Parser::new_ext(md, opts()); @@ -741,8 +737,7 @@ impl MarkdownWithToc<'_> { impl MarkdownHtml<'_> { pub fn to_string(self) -> String { - let MarkdownHtml(md, ref ids, codes, edition, playground) = self; - let mut ids = ids.borrow_mut(); + let MarkdownHtml(md, mut ids, codes, edition, playground) = self; // This is actually common enough to special-case if md.is_empty() { return String::new(); } diff --git a/src/librustdoc/html/markdown/tests.rs b/src/librustdoc/html/markdown/tests.rs index 681f363544a61..a95c29038d46f 100644 --- a/src/librustdoc/html/markdown/tests.rs +++ b/src/librustdoc/html/markdown/tests.rs @@ -73,8 +73,8 @@ fn test_lang_string_parse() { fn test_header() { fn t(input: &str, expect: &str) { let mut map = IdMap::new(); - let output = Markdown(input, &[], RefCell::new(&mut map), - ErrorCodes::Yes, DEFAULT_EDITION).to_string(); + let output = Markdown( + input, &[], &mut map, ErrorCodes::Yes, DEFAULT_EDITION, &None).to_string(); assert_eq!(output, expect, "original: {}", input); } @@ -96,8 +96,8 @@ fn test_header() { fn test_header_ids_multiple_blocks() { let mut map = IdMap::new(); fn t(map: &mut IdMap, input: &str, expect: &str) { - let output = Markdown(input, &[], RefCell::new(map), - ErrorCodes::Yes, DEFAULT_EDITION).to_string(); + let output = Markdown(input, &[], map, + ErrorCodes::Yes, DEFAULT_EDITION, &None).to_string(); assert_eq!(output, expect, "original: {}", input); } @@ -134,8 +134,8 @@ fn test_plain_summary_line() { fn test_markdown_html_escape() { fn t(input: &str, expect: &str) { let mut idmap = IdMap::new(); - let output = MarkdownHtml(input, RefCell::new(&mut idmap), - ErrorCodes::Yes, DEFAULT_EDITION).to_string(); + let output = MarkdownHtml(input, &mut idmap, + ErrorCodes::Yes, DEFAULT_EDITION, &None).to_string(); assert_eq!(output, expect, "original: {}", input); } diff --git a/src/librustdoc/html/render.rs b/src/librustdoc/html/render.rs index fc4e25e3d7ff6..ea97cea942820 100644 --- a/src/librustdoc/html/render.rs +++ b/src/librustdoc/html/render.rs @@ -2595,7 +2595,7 @@ fn render_markdown(w: &mut fmt::Formatter<'_>, write!(w, "
{}{}
", if is_hidden { " hidden" } else { "" }, prefix, - Markdown(md_text, &links, RefCell::new(&mut ids), + Markdown(md_text, &links, &mut ids, cx.codes, cx.edition, &cx.playground).to_string()) } @@ -2961,8 +2961,7 @@ fn short_stability(item: &clean::Item, cx: &Context) -> Vec { if let Some(note) = note { let mut ids = cx.id_map.borrow_mut(); - let html = MarkdownHtml( - ¬e, RefCell::new(&mut ids), error_codes, cx.edition, &cx.playground); + let html = MarkdownHtml(¬e, &mut ids, error_codes, cx.edition, &cx.playground); message.push_str(&format!(": {}", html.to_string())); } stability.push(format!("
{}
", message)); @@ -3013,7 +3012,7 @@ fn short_stability(item: &clean::Item, cx: &Context) -> Vec { message, MarkdownHtml( &unstable_reason, - RefCell::new(&mut ids), + &mut ids, error_codes, cx.edition, &cx.playground, @@ -4247,7 +4246,7 @@ fn render_impl(w: &mut fmt::Formatter<'_>, cx: &Context, i: &Impl, link: AssocIt if let Some(ref dox) = cx.shared.maybe_collapsed_doc_value(&i.impl_item) { let mut ids = cx.id_map.borrow_mut(); write!(w, "
{}
", - Markdown(&*dox, &i.impl_item.links(), RefCell::new(&mut ids), + Markdown(&*dox, &i.impl_item.links(), &mut ids, cx.codes, cx.edition, &cx.playground).to_string())?; } } diff --git a/src/librustdoc/markdown.rs b/src/librustdoc/markdown.rs index b7dd6c30f09f9..eaaae3261c728 100644 --- a/src/librustdoc/markdown.rs +++ b/src/librustdoc/markdown.rs @@ -1,7 +1,6 @@ use std::fs::File; use std::io::prelude::*; use std::path::PathBuf; -use std::cell::RefCell; use errors; use testing; @@ -83,9 +82,9 @@ pub fn render( let mut ids = IdMap::new(); let error_codes = ErrorCodes::from(UnstableFeatures::from_environment().is_nightly_build()); let text = if !options.markdown_no_toc { - MarkdownWithToc(text, RefCell::new(&mut ids), error_codes, edition, &playground).to_string() + MarkdownWithToc(text, &mut ids, error_codes, edition, &playground).to_string() } else { - Markdown(text, &[], RefCell::new(&mut ids), error_codes, edition, &playground).to_string() + Markdown(text, &[], &mut ids, error_codes, edition, &playground).to_string() }; let err = write!( diff --git a/src/tools/error_index_generator/main.rs b/src/tools/error_index_generator/main.rs index 89b545fb7e413..a9d1d9997f6ef 100644 --- a/src/tools/error_index_generator/main.rs +++ b/src/tools/error_index_generator/main.rs @@ -100,7 +100,7 @@ impl Formatter for HTMLFormatter { url: String::from("https://play.rust-lang.org/"), }; write!(output, "{}", - Markdown(desc, &[], RefCell::new(&mut id_map), + Markdown(desc, &[], &mut id_map, ErrorCodes::Yes, DEFAULT_EDITION, &Some(playground)).to_string())? }, None => write!(output, "

No description.

\n")?, From 3b8a24d193a3b2d9e7a888338d0c2bb478892ec2 Mon Sep 17 00:00:00 2001 From: Mark Rousskov Date: Sat, 10 Aug 2019 18:39:50 -0400 Subject: [PATCH 14/14] Reduce nesting in externalfiles implementation Utilize `?` instead of and_then/map operators --- src/librustdoc/externalfiles.rs | 42 +++++++++++---------------------- 1 file changed, 14 insertions(+), 28 deletions(-) diff --git a/src/librustdoc/externalfiles.rs b/src/librustdoc/externalfiles.rs index 8254bc800ca46..56f1191feed0b 100644 --- a/src/librustdoc/externalfiles.rs +++ b/src/librustdoc/externalfiles.rs @@ -25,34 +25,20 @@ impl ExternalHtml { id_map: &mut IdMap, edition: Edition, playground: &Option) -> Option { let codes = ErrorCodes::from(UnstableFeatures::from_environment().is_nightly_build()); - load_external_files(in_header, diag) - .and_then(|ih| - load_external_files(before_content, diag) - .map(|bc| (ih, bc)) - ) - .and_then(|(ih, bc)| - load_external_files(md_before_content, diag) - .map(|m_bc| (ih, - format!("{}{}", bc, Markdown(&m_bc, &[], id_map, - codes, edition, playground).to_string()))) - ) - .and_then(|(ih, bc)| - load_external_files(after_content, diag) - .map(|ac| (ih, bc, ac)) - ) - .and_then(|(ih, bc, ac)| - load_external_files(md_after_content, diag) - .map(|m_ac| (ih, bc, - format!("{}{}", ac, Markdown(&m_ac, &[], id_map, - codes, edition, playground).to_string()))) - ) - .map(|(ih, bc, ac)| - ExternalHtml { - in_header: ih, - before_content: bc, - after_content: ac, - } - ) + let ih = load_external_files(in_header, diag)?; + let bc = load_external_files(before_content, diag)?; + let m_bc = load_external_files(md_before_content, diag)?; + let bc = format!("{}{}", bc, Markdown(&m_bc, &[], id_map, + codes, edition, playground).to_string()); + let ac = load_external_files(after_content, diag)?; + let m_ac = load_external_files(md_after_content, diag)?; + let ac = format!("{}{}", ac, Markdown(&m_ac, &[], id_map, + codes, edition, playground).to_string()); + Some(ExternalHtml { + in_header: ih, + before_content: bc, + after_content: ac, + }) } }