Auto merge of #79957 - jyn514:smaller-span, r=GuillaumeGomez

[rustdoc] Calculate span information on demand instead of storing it ahead of time

This brings `size_of<clean::types::Span>()` down from over 100 bytes (!!) to only 12, the same as rustc. It brings `Item` down even more, from `784` to `680`.

~~TODO: I need to figure out how to do this for the JSON backend too. That uses `From` impls everywhere, which don't allow passing in the `Session` as an argument. `@P1n3appl3,` `@tmandry,` maybe one of you have ideas?~~ Figured it out, fortunately only two functions needed to be changed. I like the `convert_x()` format better than `From` everywhere but I'm open to feedback.

Helps with #79103
This commit is contained in:
bors 2020-12-12 19:41:56 +00:00
commit 7efc097c4f
11 changed files with 116 additions and 95 deletions

View File

@ -118,7 +118,7 @@ impl<'a, 'tcx> AutoTraitFinder<'a, 'tcx> {
};
Some(Item {
source: Span::empty(),
source: Span::dummy(),
name: None,
attrs: Default::default(),
visibility: Inherited,

View File

@ -479,7 +479,7 @@ fn build_module(cx: &DocContext<'_>, did: DefId, visited: &mut FxHashSet<DefId>)
items.push(clean::Item {
name: None,
attrs: clean::Attributes::default(),
source: clean::Span::empty(),
source: clean::Span::dummy(),
def_id: DefId::local(CRATE_DEF_INDEX),
visibility: clean::Public,
stability: None,

View File

@ -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,8 @@ 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();
}
// 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,
}
fn clean(&self, _cx: &DocContext<'_>) -> Span {
Span::from_rustc_span(*self)
}
}

View File

@ -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,32 +1610,41 @@ crate enum VariantKind {
Struct(VariantStruct),
}
/// Small wrapper around `rustc_span::Span` that adds helper methods and enforces calling `source_callsite`.
#[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,
}
crate struct Span(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 from_rustc_span(sp: rustc_span::Span) -> Self {
// Get the macro invocation instead of the definition,
// in case the span is result of a macro expansion.
// (See rust-lang/rust#39726)
Self(sp.source_callsite())
}
crate fn dummy() -> Self {
Self(rustc_span::DUMMY_SP)
}
crate fn span(&self) -> rustc_span::Span {
self.original
self.0
}
crate fn filename(&self, sess: &Session) -> FileName {
sess.source_map().span_to_filename(self.0)
}
crate fn lo(&self, sess: &Session) -> Loc {
sess.source_map().lookup_char_pos(self.0.lo())
}
crate fn hi(&self, sess: &Session) -> Loc {
sess.source_map().lookup_char_pos(self.0.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
}
}

View File

@ -1,5 +1,7 @@
use std::sync::Arc;
use rustc_data_structures::sync::Lrc;
use rustc_session::Session;
use rustc_span::edition::Edition;
use crate::clean;
@ -19,6 +21,7 @@ crate trait FormatRenderer: Clone {
render_info: RenderInfo,
edition: Edition,
cache: &mut Cache,
sess: Lrc<Session>,
) -> Result<(Self, clean::Crate), Error>;
/// Renders a single non-module item. This means no recursive sub-item rendering is required.
@ -49,6 +52,7 @@ crate fn run_format<T: FormatRenderer>(
render_info: RenderInfo,
diag: &rustc_errors::Handler,
edition: Edition,
sess: Lrc<Session>,
) -> Result<(), Error> {
let (krate, mut cache) = Cache::from_krate(
render_info.clone(),
@ -59,7 +63,7 @@ crate fn run_format<T: FormatRenderer>(
);
let (mut format_renderer, mut krate) =
T::init(krate, options, render_info, edition, &mut cache)?;
T::init(krate, options, render_info, edition, &mut cache, sess)?;
let cache = Arc::new(cache);
// Freeze the cache now that the index has been built. Put an Arc into TLS for future

View File

@ -52,10 +52,12 @@ use rustc_ast_pretty::pprust;
use rustc_attr::StabilityLevel;
use rustc_data_structures::flock;
use rustc_data_structures::fx::{FxHashMap, FxHashSet};
use rustc_data_structures::sync::Lrc;
use rustc_hir as hir;
use rustc_hir::def_id::{DefId, LOCAL_CRATE};
use rustc_hir::Mutability;
use rustc_middle::middle::stability;
use rustc_session::Session;
use rustc_span::edition::Edition;
use rustc_span::hygiene::MacroKind;
use rustc_span::source_map::FileName;
@ -121,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,
@ -171,6 +174,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 {
@ -381,6 +388,7 @@ impl FormatRenderer for Context {
_render_info: RenderInfo,
edition: Edition,
cache: &mut Cache,
sess: Lrc<Session>,
) -> Result<(Context, clean::Crate), Error> {
// need to save a copy of the options for rendering the index page
let md_opts = options.clone();
@ -453,6 +461,7 @@ impl FormatRenderer for Context {
}
let (sender, receiver) = channel();
let mut scx = SharedContext {
sess,
collapsed: krate.collapsed,
src_root,
include_sources,
@ -1629,24 +1638,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();
@ -1665,11 +1674,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),

View File

@ -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 {

View File

@ -6,14 +6,16 @@ use std::convert::From;
use rustc_ast::ast;
use rustc_span::def_id::{DefId, CRATE_DEF_INDEX};
use rustc_span::Pos;
use crate::clean;
use crate::doctree;
use crate::formats::item_type::ItemType;
use crate::json::types::*;
use crate::json::JsonRenderer;
impl From<clean::Item> for Option<Item> {
fn from(item: clean::Item) -> Self {
impl JsonRenderer {
pub(super) fn convert_item(&self, item: clean::Item) -> Option<Item> {
let item_type = ItemType::from(&item);
let clean::Item {
source,
@ -32,7 +34,7 @@ impl From<clean::Item> for Option<Item> {
id: def_id.into(),
crate_id: def_id.krate.as_u32(),
name,
source: source.into(),
source: self.convert_span(source),
visibility: visibility.into(),
docs: attrs.collapsed_doc_value().unwrap_or_default(),
links: attrs
@ -53,22 +55,23 @@ impl From<clean::Item> for Option<Item> {
}),
}
}
}
impl From<clean::Span> for Option<Span> {
fn from(span: clean::Span) -> Self {
let clean::Span { loline, locol, hiline, hicol, .. } = span;
match span.filename {
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),
}),
fn convert_span(&self, span: clean::Span) -> Option<Span> {
match span.filename(&self.sess) {
rustc_span::FileName::Real(name) => {
let hi = span.hi(&self.sess);
let lo = span.lo(&self.sess);
Some(Span {
filename: match name {
rustc_span::RealFileName::Named(path) => path,
rustc_span::RealFileName::Devirtualized { local_path, virtual_name: _ } => {
local_path
}
},
begin: (lo.line, lo.col.to_usize()),
end: (hi.line, hi.col.to_usize()),
})
}
_ => None,
}
}

View File

@ -13,6 +13,8 @@ use std::path::PathBuf;
use std::rc::Rc;
use rustc_data_structures::fx::FxHashMap;
use rustc_data_structures::sync::Lrc;
use rustc_session::Session;
use rustc_span::edition::Edition;
use crate::clean;
@ -24,6 +26,7 @@ use crate::html::render::cache::ExternalLocation;
#[derive(Clone)]
crate struct JsonRenderer {
sess: Lrc<Session>,
/// A mapping of IDs that contains all local items for this crate which gets output as a top
/// level field of the JSON blob.
index: Rc<RefCell<FxHashMap<types::Id, types::Item>>>,
@ -124,10 +127,12 @@ impl FormatRenderer for JsonRenderer {
_render_info: RenderInfo,
_edition: Edition,
_cache: &mut Cache,
sess: Lrc<Session>,
) -> Result<(Self, clean::Crate), Error> {
debug!("Initializing json renderer");
Ok((
JsonRenderer {
sess,
index: Rc::new(RefCell::new(FxHashMap::default())),
out_path: options.output,
},
@ -143,7 +148,7 @@ impl FormatRenderer for JsonRenderer {
item.kind.inner_items().for_each(|i| self.item(i.clone(), cache).unwrap());
let id = item.def_id;
if let Some(mut new_item) = item.into(): Option<types::Item> {
if let Some(mut new_item) = self.convert_item(item) {
if let types::ItemEnum::TraitItem(ref mut t) = new_item.inner {
t.implementors = self.get_trait_implementors(id, cache)
} else if let types::ItemEnum::StructItem(ref mut s) = new_item.inner {

View File

@ -62,9 +62,11 @@ use std::default::Default;
use std::env;
use std::process;
use rustc_data_structures::sync::Lrc;
use rustc_errors::ErrorReported;
use rustc_session::config::{make_crate_type_option, ErrorOutputType, RustcOptGroup};
use rustc_session::getopts;
use rustc_session::Session;
use rustc_session::{early_error, early_warn};
#[macro_use]
@ -484,8 +486,9 @@ fn run_renderer<T: formats::FormatRenderer>(
render_info: config::RenderInfo,
diag: &rustc_errors::Handler,
edition: rustc_span::edition::Edition,
sess: Lrc<Session>,
) -> MainResult {
match formats::run_format::<T>(krate, renderopts, render_info, &diag, edition) {
match formats::run_format::<T>(krate, renderopts, render_info, &diag, edition, sess) {
Ok(_) => Ok(()),
Err(e) => {
let mut msg = diag.struct_err(&format!("couldn't generate documentation: {}", e.error));
@ -553,12 +556,15 @@ 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", || {
run_renderer::<html::render::Context>(krate, renderopts, renderinfo, &diag, edition)
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", || {
run_renderer::<json::JsonRenderer>(krate, renderopts, renderinfo, &diag, edition)
Some(config::OutputFormat::Json) => sess_time.time("render_json", || {
run_renderer::<json::JsonRenderer>(krate, renderopts, renderinfo, &diag, edition, sess)
}),
}
}

View File

@ -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),