Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

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

Merged
merged 4 commits into from
Dec 12, 2020
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
2 changes: 1 addition & 1 deletion src/librustdoc/clean/auto_trait.rs
Original file line number Diff line number Diff line change
Expand Up @@ -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,
Expand Down
2 changes: 1 addition & 1 deletion src/librustdoc/clean/inline.rs
Original file line number Diff line number Diff line change
Expand Up @@ -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,
Expand Down
27 changes: 3 additions & 24 deletions src/librustdoc/clean/mod.rs
Original file line number Diff line number Diff line change
Expand Up @@ -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;
Expand Down Expand Up @@ -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)
}
}

Expand Down
54 changes: 32 additions & 22 deletions src/librustdoc/clean/types.rs
Original file line number Diff line number Diff line change
Expand Up @@ -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};
Expand Down Expand Up @@ -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?
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

In case of proc-macro maybe?

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Yeah, as a follow-up I could add a debug_assert maybe. But I think the callsites should probably be using item.def_id.is_local() instead, I think that would be more reliable (and allow deleting this code).

self.lo(sess).file.cnum
}
}

Expand Down
6 changes: 5 additions & 1 deletion src/librustdoc/formats/renderer.rs
Original file line number Diff line number Diff line change
@@ -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;
Expand All @@ -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.
Expand Down Expand Up @@ -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(),
Expand All @@ -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
Expand Down
26 changes: 17 additions & 9 deletions src/librustdoc/html/render/mod.rs
Original file line number Diff line number Diff line change
Expand Up @@ -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;
Expand Down Expand Up @@ -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,
Expand Down Expand Up @@ -173,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 {
Expand Down Expand Up @@ -383,6 +390,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();
Expand Down Expand Up @@ -455,6 +463,7 @@ impl FormatRenderer for Context {
}
let (sender, receiver) = channel();
let mut scx = SharedContext {
sess,
collapsed: krate.collapsed,
src_root,
include_sources,
Expand Down Expand Up @@ -1631,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();
Expand All @@ -1667,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),
Expand Down
17 changes: 13 additions & 4 deletions src/librustdoc/html/sources.rs
Original file line number Diff line number Diff line change
Expand Up @@ -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;
Expand Down Expand Up @@ -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 {
Expand Down
39 changes: 21 additions & 18 deletions src/librustdoc/json/conversions.rs
Original file line number Diff line number Diff line change
Expand Up @@ -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,
Expand All @@ -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
Expand All @@ -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,
}
}
Expand Down
Loading