From 4fa95b3a078f261267293f3308dd62889167c0bd Mon Sep 17 00:00:00 2001 From: Joshua Nelson Date: Fri, 11 Dec 2020 23:01:26 -0500 Subject: [PATCH] Calculate span info on-demand instead of ahead of time This should *vastly* reduce memory usage. --- src/librustdoc/clean/mod.rs | 24 ++--------- src/librustdoc/clean/types.rs | 41 +++++++++++-------- src/librustdoc/html/render/mod.rs | 25 ++++++----- src/librustdoc/html/sources.rs | 17 ++++++-- src/librustdoc/json/conversions.rs | 11 +++-- src/librustdoc/lib.rs | 5 ++- .../passes/calculate_doc_coverage.rs | 15 +++---- 7 files changed, 69 insertions(+), 69 deletions(-) diff --git a/src/librustdoc/clean/mod.rs b/src/librustdoc/clean/mod.rs index 16274430902..a21e9d9820c 100644 --- a/src/librustdoc/clean/mod.rs +++ b/src/librustdoc/clean/mod.rs @@ -25,7 +25,7 @@ use rustc_middle::ty::{self, AdtKind, Lift, Ty, TyCtxt}; use rustc_mir::const_eval::{is_const_fn, is_min_const_fn, is_unstable_const_fn}; use rustc_span::hygiene::{AstPass, MacroKind}; use rustc_span::symbol::{kw, sym, Ident, Symbol}; -use rustc_span::{self, ExpnKind, Pos}; +use rustc_span::{self, ExpnKind}; use rustc_typeck::hir_ty_to_ty; use std::collections::hash_map::Entry; @@ -1881,29 +1881,11 @@ impl Clean for hir::VariantData<'_> { } impl Clean for rustc_span::Span { - fn clean(&self, cx: &DocContext<'_>) -> Span { - if self.is_dummy() { - return Span::empty(); - } - + fn clean(&self, _cx: &DocContext<'_>) -> Span { // Get the macro invocation instead of the definition, // in case the span is result of a macro expansion. // (See rust-lang/rust#39726) - let span = self.source_callsite(); - - let sm = cx.sess().source_map(); - let filename = sm.span_to_filename(span); - let lo = sm.lookup_char_pos(span.lo()); - let hi = sm.lookup_char_pos(span.hi()); - Span { - filename, - cnum: lo.file.cnum, - loline: lo.line, - locol: lo.col.to_usize(), - hiline: hi.line, - hicol: hi.col.to_usize(), - original: span, - } + Span { original: self.source_callsite() } } } diff --git a/src/librustdoc/clean/types.rs b/src/librustdoc/clean/types.rs index d4796b7ed66..a8626af8832 100644 --- a/src/librustdoc/clean/types.rs +++ b/src/librustdoc/clean/types.rs @@ -17,15 +17,16 @@ use rustc_data_structures::fx::{FxHashMap, FxHashSet}; use rustc_feature::UnstableFeatures; use rustc_hir as hir; use rustc_hir::def::Res; -use rustc_hir::def_id::{CrateNum, DefId, LOCAL_CRATE}; +use rustc_hir::def_id::{CrateNum, DefId}; use rustc_hir::lang_items::LangItem; use rustc_hir::Mutability; use rustc_index::vec::IndexVec; use rustc_middle::ty::{AssocKind, TyCtxt}; +use rustc_session::Session; use rustc_span::hygiene::MacroKind; use rustc_span::source_map::DUMMY_SP; use rustc_span::symbol::{kw, sym, Ident, Symbol, SymbolStr}; -use rustc_span::{self, FileName}; +use rustc_span::{self, FileName, Loc}; use rustc_target::abi::VariantIdx; use rustc_target::spec::abi::Abi; use smallvec::{smallvec, SmallVec}; @@ -1609,33 +1610,37 @@ crate enum VariantKind { Struct(VariantStruct), } +/// Small wrapper around `rustc_span::Span` that adds helper methods. #[derive(Clone, Debug)] crate struct Span { - crate filename: FileName, - crate cnum: CrateNum, - crate loline: usize, - crate locol: usize, - crate hiline: usize, - crate hicol: usize, crate original: rustc_span::Span, } impl Span { - crate fn empty() -> Span { - Span { - filename: FileName::Anon(0), - cnum: LOCAL_CRATE, - loline: 0, - locol: 0, - hiline: 0, - hicol: 0, - original: rustc_span::DUMMY_SP, - } + crate fn empty() -> Self { + Self { original: rustc_span::DUMMY_SP } } crate fn span(&self) -> rustc_span::Span { self.original } + + crate fn filename(&self, sess: &Session) -> FileName { + sess.source_map().span_to_filename(self.original) + } + + crate fn lo(&self, sess: &Session) -> Loc { + sess.source_map().lookup_char_pos(self.original.lo()) + } + + crate fn hi(&self, sess: &Session) -> Loc { + sess.source_map().lookup_char_pos(self.original.hi()) + } + + crate fn cnum(&self, sess: &Session) -> CrateNum { + // FIXME: is there a time when the lo and hi crate would be different? + self.lo(sess).file.cnum + } } #[derive(Clone, PartialEq, Eq, Debug, Hash)] diff --git a/src/librustdoc/html/render/mod.rs b/src/librustdoc/html/render/mod.rs index ff8fd208eea..b91f1a6c0e3 100644 --- a/src/librustdoc/html/render/mod.rs +++ b/src/librustdoc/html/render/mod.rs @@ -103,7 +103,6 @@ crate fn ensure_trailing_slash(v: &str) -> impl fmt::Display + '_ { /// rustdoc tree). #[derive(Clone)] crate struct Context { - crate sess: Lrc, /// Current hierarchy of components leading down to what's currently being /// rendered crate current: Vec, @@ -124,6 +123,7 @@ crate struct Context { } crate struct SharedContext { + crate sess: Lrc, /// The path to the crate root source minus the file name. /// Used for simplifying paths to the highlighted source code files. crate src_root: PathBuf, @@ -176,6 +176,10 @@ impl Context { let filename = format!("{}{}.{}", base, self.shared.resource_suffix, ext,); self.dst.join(&filename) } + + fn sess(&self) -> &Session { + &self.shared.sess + } } impl SharedContext { @@ -459,6 +463,7 @@ impl FormatRenderer for Context { } let (sender, receiver) = channel(); let mut scx = SharedContext { + sess, collapsed: krate.collapsed, src_root, include_sources, @@ -498,7 +503,6 @@ impl FormatRenderer for Context { let cache = Arc::new(cache); let mut cx = Context { - sess, current: Vec::new(), dst, render_redirect_pages: false, @@ -1636,24 +1640,24 @@ impl Context { /// of their crate documentation isn't known. fn src_href(&self, item: &clean::Item, cache: &Cache) -> Option { let mut root = self.root_path(); - let mut path = String::new(); + let cnum = item.source.cnum(self.sess()); // We can safely ignore synthetic `SourceFile`s. - let file = match item.source.filename { + let file = match item.source.filename(self.sess()) { FileName::Real(ref path) => path.local_path().to_path_buf(), _ => return None, }; let file = &file; - let (krate, path) = if item.source.cnum == LOCAL_CRATE { + let (krate, path) = if cnum == LOCAL_CRATE { if let Some(path) = self.shared.local_sources.get(file) { (&self.shared.layout.krate, path) } else { return None; } } else { - let (krate, src_root) = match *cache.extern_locations.get(&item.source.cnum)? { + let (krate, src_root) = match *cache.extern_locations.get(&cnum)? { (ref name, ref src, ExternalLocation::Local) => (name, src), (ref name, ref src, ExternalLocation::Remote(ref s)) => { root = s.to_string(); @@ -1672,11 +1676,10 @@ impl Context { (krate, &path) }; - let lines = if item.source.loline == item.source.hiline { - item.source.loline.to_string() - } else { - format!("{}-{}", item.source.loline, item.source.hiline) - }; + let loline = item.source.lo(self.sess()).line; + let hiline = item.source.hi(self.sess()).line; + let lines = + if loline == hiline { loline.to_string() } else { format!("{}-{}", loline, hiline) }; Some(format!( "{root}src/{krate}/{path}#{lines}", root = Escape(&root), diff --git a/src/librustdoc/html/sources.rs b/src/librustdoc/html/sources.rs index e7b5a90d84d..ef9e9f350fb 100644 --- a/src/librustdoc/html/sources.rs +++ b/src/librustdoc/html/sources.rs @@ -7,6 +7,7 @@ use crate::html::highlight; use crate::html::layout; use crate::html::render::{SharedContext, BASIC_KEYWORDS}; use rustc_hir::def_id::LOCAL_CRATE; +use rustc_session::Session; use rustc_span::source_map::FileName; use std::ffi::OsStr; use std::fs; @@ -34,37 +35,45 @@ struct SourceCollector<'a> { impl<'a> DocFolder for SourceCollector<'a> { fn fold_item(&mut self, item: clean::Item) -> Option { + // If we're not rendering sources, there's nothing to do. // If we're including source files, and we haven't seen this file yet, // then we need to render it out to the filesystem. if self.scx.include_sources // skip all synthetic "files" - && item.source.filename.is_real() + && item.source.filename(self.sess()).is_real() // skip non-local files - && item.source.cnum == LOCAL_CRATE + && item.source.cnum(self.sess()) == LOCAL_CRATE { + let filename = item.source.filename(self.sess()); // If it turns out that we couldn't read this file, then we probably // can't read any of the files (generating html output from json or // something like that), so just don't include sources for the // entire crate. The other option is maintaining this mapping on a // per-file basis, but that's probably not worth it... - self.scx.include_sources = match self.emit_source(&item.source.filename) { + self.scx.include_sources = match self.emit_source(&filename) { Ok(()) => true, Err(e) => { println!( "warning: source code was requested to be rendered, \ but processing `{}` had an error: {}", - item.source.filename, e + filename, e ); println!(" skipping rendering of source code"); false } }; } + // FIXME: if `include_sources` isn't set and DocFolder didn't require consuming the crate by value, + // we could return None here without having to walk the rest of the crate. Some(self.fold_item_recur(item)) } } impl<'a> SourceCollector<'a> { + fn sess(&self) -> &Session { + &self.scx.sess + } + /// Renders the given filename into its corresponding HTML source file. fn emit_source(&mut self, filename: &FileName) -> Result<(), Error> { let p = match *filename { diff --git a/src/librustdoc/json/conversions.rs b/src/librustdoc/json/conversions.rs index afb8dfa6766..9ae0a16ac35 100644 --- a/src/librustdoc/json/conversions.rs +++ b/src/librustdoc/json/conversions.rs @@ -56,9 +56,12 @@ impl From for Option { } impl From for Option { + #[allow(unreachable_code)] fn from(span: clean::Span) -> Self { - let clean::Span { loline, locol, hiline, hicol, .. } = span; - match span.filename { + // TODO: this should actually work + // Unfortunately this requires rethinking the whole framework, + // since this now needs a context and not just .into(). + match span.filename(todo!()) { rustc_span::FileName::Real(name) => Some(Span { filename: match name { rustc_span::RealFileName::Named(path) => path, @@ -66,8 +69,8 @@ impl From for Option { local_path } }, - begin: (loline, locol), - end: (hiline, hicol), + begin: todo!(), + end: todo!(), }), _ => None, } diff --git a/src/librustdoc/lib.rs b/src/librustdoc/lib.rs index fbab5735ee7..e094ead12bd 100644 --- a/src/librustdoc/lib.rs +++ b/src/librustdoc/lib.rs @@ -555,13 +555,14 @@ fn main_options(options: config::Options) -> MainResult { info!("going to format"); let (error_format, edition, debugging_options) = diag_opts; let diag = core::new_handler(error_format, None, &debugging_options); + let sess_time = sess.clone(); match output_format { - None | Some(config::OutputFormat::Html) => sess.time("render_html", || { + None | Some(config::OutputFormat::Html) => sess_time.time("render_html", || { run_renderer::( krate, renderopts, renderinfo, &diag, edition, sess, ) }), - Some(config::OutputFormat::Json) => sess.time("render_json", || { + Some(config::OutputFormat::Json) => sess_time.time("render_json", || { run_renderer::(krate, renderopts, renderinfo, &diag, edition, sess) }), } diff --git a/src/librustdoc/passes/calculate_doc_coverage.rs b/src/librustdoc/passes/calculate_doc_coverage.rs index 3f9978c8fca..52f6a97089b 100644 --- a/src/librustdoc/passes/calculate_doc_coverage.rs +++ b/src/librustdoc/passes/calculate_doc_coverage.rs @@ -216,13 +216,9 @@ impl<'a, 'b> fold::DocFolder for CoverageCalculator<'a, 'b> { return Some(i); } clean::ImplItem(ref impl_) => { + let filename = i.source.filename(self.ctx.sess()); if let Some(ref tr) = impl_.trait_ { - debug!( - "impl {:#} for {:#} in {}", - tr.print(), - impl_.for_.print(), - i.source.filename - ); + debug!("impl {:#} for {:#} in {}", tr.print(), impl_.for_.print(), filename,); // don't count trait impls, the missing-docs lint doesn't so we shouldn't // either @@ -231,7 +227,7 @@ impl<'a, 'b> fold::DocFolder for CoverageCalculator<'a, 'b> { // inherent impls *can* be documented, and those docs show up, but in most // cases it doesn't make sense, as all methods on a type are in one single // impl block - debug!("impl {:#} in {}", impl_.for_.print(), i.source.filename); + debug!("impl {:#} in {}", impl_.for_.print(), filename); } } _ => { @@ -251,6 +247,7 @@ impl<'a, 'b> fold::DocFolder for CoverageCalculator<'a, 'b> { None, ); + let filename = i.source.filename(self.ctx.sess()); let has_doc_example = tests.found_tests != 0; let hir_id = self.ctx.tcx.hir().local_def_id_to_hir_id(i.def_id.expect_local()); let (level, source) = self.ctx.tcx.lint_level_at_node(MISSING_DOCS, hir_id); @@ -258,8 +255,8 @@ impl<'a, 'b> fold::DocFolder for CoverageCalculator<'a, 'b> { // unless the user had an explicit `allow` let should_have_docs = level != lint::Level::Allow || matches!(source, LintSource::Default); - debug!("counting {:?} {:?} in {}", i.type_(), i.name, i.source.filename); - self.items.entry(i.source.filename.clone()).or_default().count_item( + debug!("counting {:?} {:?} in {}", i.type_(), i.name, filename); + self.items.entry(filename).or_default().count_item( has_docs, has_doc_example, should_have_doc_example(self.ctx, &i),