Calculate span info on-demand instead of ahead of time

This should *vastly* reduce memory usage.

Author: jyn514

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<VariantKind> for hir::VariantData<'_> {
 }
 
 impl Clean<Span> 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() }
     }
 }
 

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)]

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<Session>,
     /// Current hierarchy of components leading down to what's currently being
     /// rendered
     crate current: Vec<String>,
@@ -124,6 +123,7 @@ crate struct Context {
 }
 
 crate struct SharedContext {
+    crate sess: Lrc<Session>,
     /// 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<String> {
         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),

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<clean::Item> {
+        // 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 {

src/librustdoc/json/conversions.rs

@@ -56,18 +56,21 @@ impl From<clean::Item> for Option<Item> {
 }
 
 impl From<clean::Span> for Option<Span> {
+    #[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,
                     rustc_span::RealFileName::Devirtualized { local_path, virtual_name: _ } => {
                         local_path
                     }
                 },
-                begin: (loline, locol),
-                end: (hiline, hicol),
+                begin: todo!(),
+                end: todo!(),
             }),
             _ => None,
         }

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::<html::render::Context>(
                 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::<json::JsonRenderer>(krate, renderopts, renderinfo, &diag, edition, sess)
         }),
     }

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,15 +247,16 @@ 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);
                 // `missing_docs` is allow-by-default, so don't treat this as ignoring the item
                 // 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),