Skip to content

Commit

Permalink
Auto merge of #42593 - ibabushkin:on-demand-external-source, r=eddyb
Browse files Browse the repository at this point in the history
Implement lazy loading of external crates' sources. Fixes #38875

Fixes #38875. This is a follow-up to #42507. When a (now correctly translated) span from an external crate is referenced in a error, warning or info message, we still don't have the source code being referenced.
Since stuffing the source in the serialized metadata of an rlib is extremely wasteful, the following scheme has been implemented:

* File maps now contain a source hash that gets serialized as well.
* When a span is rendered in a message, the source hash in the corresponding file map(s) is used to try and load the source from the corresponding file on disk. If the file is not found or the hashes don't match, the failed attempt is recorded (and not retried).
* The machinery fetching source lines from file maps is augmented to use the lazily loaded external source as a secondary fallback for file maps belonging to external crates.

This required a small change to the expected stderr of one UI test (it now renders a span, where previously was none).

Further work can be done based on this - some of the machinery previously used to hide external spans is possibly obsolete and the hashing code can be reused in different places as well.

r? @eddyb
  • Loading branch information
bors committed Jun 18, 2017
2 parents 78d8416 + bd4fe45 commit 28cc0c5
Show file tree
Hide file tree
Showing 14 changed files with 257 additions and 81 deletions.
1 change: 1 addition & 0 deletions src/Cargo.lock

Some generated files are not rendered by default. Learn more about how customized files appear on GitHub.

4 changes: 4 additions & 0 deletions src/librustc/ich/impls_syntax.rs
Original file line number Diff line number Diff line change
Expand Up @@ -336,6 +336,8 @@ impl<'a, 'gcx, 'tcx> HashStable<StableHashingContext<'a, 'gcx, 'tcx>> for FileMa
crate_of_origin,
// Do not hash the source as it is not encoded
src: _,
src_hash,
external_src: _,
start_pos,
end_pos: _,
ref lines,
Expand All @@ -350,6 +352,8 @@ impl<'a, 'gcx, 'tcx> HashStable<StableHashingContext<'a, 'gcx, 'tcx>> for FileMa
index: CRATE_DEF_INDEX,
}.hash_stable(hcx, hasher);

src_hash.hash_stable(hcx, hasher);

// We only hash the relative position within this filemap
let lines = lines.borrow();
lines.len().hash_stable(hcx, hasher);
Expand Down
11 changes: 11 additions & 0 deletions src/librustc_data_structures/stable_hasher.rs
Original file line number Diff line number Diff line change
Expand Up @@ -78,6 +78,17 @@ impl StableHasherResult for [u8; 20] {
}
}

impl StableHasherResult for u128 {
fn finish(mut hasher: StableHasher<Self>) -> Self {
let hash_bytes: &[u8] = hasher.finalize();
assert!(hash_bytes.len() >= mem::size_of::<u128>());

unsafe {
::std::ptr::read_unaligned(hash_bytes.as_ptr() as *const u128)
}
}
}

impl StableHasherResult for u64 {
fn finish(mut hasher: StableHasher<Self>) -> Self {
hasher.state.finalize();
Expand Down
12 changes: 7 additions & 5 deletions src/librustc_errors/emitter.rs
Original file line number Diff line number Diff line change
Expand Up @@ -17,6 +17,7 @@ use RenderSpan::*;
use snippet::{Annotation, AnnotationType, Line, MultilineAnnotation, StyledString, Style};
use styled_buffer::StyledBuffer;

use std::borrow::Cow;
use std::io::prelude::*;
use std::io;
use std::rc::Rc;
Expand Down Expand Up @@ -131,7 +132,7 @@ impl EmitterWriter {
}
}

fn preprocess_annotations(&self, msp: &MultiSpan) -> Vec<FileWithAnnotatedLines> {
fn preprocess_annotations(&mut self, msp: &MultiSpan) -> Vec<FileWithAnnotatedLines> {
fn add_annotation_to_file(file_vec: &mut Vec<FileWithAnnotatedLines>,
file: Rc<FileMap>,
line_index: usize,
Expand Down Expand Up @@ -175,6 +176,7 @@ impl EmitterWriter {
if span_label.span == DUMMY_SP {
continue;
}

let lo = cm.lookup_char_pos(span_label.span.lo);
let mut hi = cm.lookup_char_pos(span_label.span.hi);

Expand Down Expand Up @@ -890,10 +892,10 @@ impl EmitterWriter {
let mut annotated_files = self.preprocess_annotations(msp);

// Make sure our primary file comes first
let primary_lo = if let (Some(ref cm), Some(ref primary_span)) =
let (primary_lo, cm) = if let (Some(cm), Some(ref primary_span)) =
(self.cm.as_ref(), msp.primary_span().as_ref()) {
if primary_span != &&DUMMY_SP {
cm.lookup_char_pos(primary_span.lo)
(cm.lookup_char_pos(primary_span.lo), cm)
} else {
emit_to_destination(&buffer.render(), level, &mut self.dst)?;
return Ok(());
Expand All @@ -911,7 +913,7 @@ impl EmitterWriter {
// Print out the annotate source lines that correspond with the error
for annotated_file in annotated_files {
// we can't annotate anything if the source is unavailable.
if annotated_file.file.src.is_none() {
if !cm.ensure_filemap_source_present(annotated_file.file.clone()) {
continue;
}

Expand Down Expand Up @@ -1012,7 +1014,7 @@ impl EmitterWriter {
} else if line_idx_delta == 2 {
let unannotated_line = annotated_file.file
.get_line(annotated_file.lines[line_idx].line_index)
.unwrap_or("");
.unwrap_or_else(|| Cow::from(""));

let last_buffer_line_num = buffer.num_lines();

Expand Down
14 changes: 8 additions & 6 deletions src/librustc_errors/lib.rs
Original file line number Diff line number Diff line change
Expand Up @@ -37,6 +37,7 @@ use self::Level::*;

use emitter::{Emitter, EmitterWriter};

use std::borrow::Cow;
use std::cell::{RefCell, Cell};
use std::{error, fmt};
use std::rc::Rc;
Expand All @@ -49,7 +50,7 @@ pub mod registry;
pub mod styled_buffer;
mod lock;

use syntax_pos::{BytePos, Loc, FileLinesResult, FileName, MultiSpan, Span, NO_EXPANSION};
use syntax_pos::{BytePos, Loc, FileLinesResult, FileMap, FileName, MultiSpan, Span, NO_EXPANSION};

#[derive(Clone, Debug, PartialEq, RustcEncodable, RustcDecodable)]
pub enum RenderSpan {
Expand Down Expand Up @@ -103,6 +104,7 @@ pub trait CodeMapper {
fn span_to_filename(&self, sp: Span) -> FileName;
fn merge_spans(&self, sp_lhs: Span, sp_rhs: Span) -> Option<Span>;
fn call_span_if_macro(&self, sp: Span) -> Span;
fn ensure_filemap_source_present(&self, file_map: Rc<FileMap>) -> bool;
}

impl CodeSuggestion {
Expand All @@ -121,7 +123,7 @@ impl CodeSuggestion {
use syntax_pos::{CharPos, Loc, Pos};

fn push_trailing(buf: &mut String,
line_opt: Option<&str>,
line_opt: Option<&Cow<str>>,
lo: &Loc,
hi_opt: Option<&Loc>) {
let (lo, hi_opt) = (lo.col.to_usize(), hi_opt.map(|hi| hi.col.to_usize()));
Expand Down Expand Up @@ -183,13 +185,13 @@ impl CodeSuggestion {
let cur_lo = cm.lookup_char_pos(sp.lo);
for (buf, substitute) in bufs.iter_mut().zip(substitutes) {
if prev_hi.line == cur_lo.line {
push_trailing(buf, prev_line, &prev_hi, Some(&cur_lo));
push_trailing(buf, prev_line.as_ref(), &prev_hi, Some(&cur_lo));
} else {
push_trailing(buf, prev_line, &prev_hi, None);
push_trailing(buf, prev_line.as_ref(), &prev_hi, None);
// push lines between the previous and current span (if any)
for idx in prev_hi.line..(cur_lo.line - 1) {
if let Some(line) = fm.get_line(idx) {
buf.push_str(line);
buf.push_str(line.as_ref());
buf.push('\n');
}
}
Expand All @@ -205,7 +207,7 @@ impl CodeSuggestion {
for buf in &mut bufs {
// if the replacement already ends with a newline, don't print the next line
if !buf.ends_with('\n') {
push_trailing(buf, prev_line, &prev_hi, None);
push_trailing(buf, prev_line.as_ref(), &prev_hi, None);
}
// remove trailing newline
buf.pop();
Expand Down
4 changes: 3 additions & 1 deletion src/librustc_metadata/decoder.rs
Original file line number Diff line number Diff line change
Expand Up @@ -765,7 +765,7 @@ impl<'a, 'tcx> CrateMetadata {
assert!(!self.is_proc_macro(id));
let ast = self.entry(id).ast.unwrap();
let def_id = self.local_def_id(id);
let body = ast.decode(self).body.decode(self);
let body = ast.decode((self, tcx)).body.decode((self, tcx));
tcx.hir.intern_inlined_body(def_id, body)
}

Expand Down Expand Up @@ -1149,6 +1149,7 @@ impl<'a, 'tcx> CrateMetadata {
// containing the information we need.
let syntax_pos::FileMap { name,
name_was_remapped,
src_hash,
start_pos,
end_pos,
lines,
Expand All @@ -1174,6 +1175,7 @@ impl<'a, 'tcx> CrateMetadata {
let local_version = local_codemap.new_imported_filemap(name,
name_was_remapped,
self.cnum.as_u32(),
src_hash,
source_length,
lines,
multibyte_chars);
Expand Down
85 changes: 39 additions & 46 deletions src/libsyntax/codemap.rs
Original file line number Diff line number Diff line change
Expand Up @@ -158,29 +158,13 @@ impl CodeMap {

/// Creates a new filemap without setting its line information. If you don't
/// intend to set the line information yourself, you should use new_filemap_and_lines.
pub fn new_filemap(&self, filename: FileName, mut src: String) -> Rc<FileMap> {
pub fn new_filemap(&self, filename: FileName, src: String) -> Rc<FileMap> {
let start_pos = self.next_start_pos();
let mut files = self.files.borrow_mut();

// Remove utf-8 BOM if any.
if src.starts_with("\u{feff}") {
src.drain(..3);
}

let end_pos = start_pos + src.len();

let (filename, was_remapped) = self.path_mapping.map_prefix(filename);

let filemap = Rc::new(FileMap {
name: filename,
name_was_remapped: was_remapped,
crate_of_origin: 0,
src: Some(Rc::new(src)),
start_pos: Pos::from_usize(start_pos),
end_pos: Pos::from_usize(end_pos),
lines: RefCell::new(Vec::new()),
multibyte_chars: RefCell::new(Vec::new()),
});
let filemap =
Rc::new(FileMap::new(filename, was_remapped, src, Pos::from_usize(start_pos)));

files.push(filemap.clone());

Expand Down Expand Up @@ -210,6 +194,7 @@ impl CodeMap {
filename: FileName,
name_was_remapped: bool,
crate_of_origin: u32,
src_hash: u128,
source_len: usize,
mut file_local_lines: Vec<BytePos>,
mut file_local_multibyte_chars: Vec<MultiByteChar>)
Expand All @@ -233,6 +218,8 @@ impl CodeMap {
name_was_remapped: name_was_remapped,
crate_of_origin: crate_of_origin,
src: None,
src_hash: src_hash,
external_src: RefCell::new(ExternalSource::AbsentOk),
start_pos: start_pos,
end_pos: end_pos,
lines: RefCell::new(file_local_lines),
Expand Down Expand Up @@ -428,30 +415,31 @@ impl CodeMap {
local_end.fm.start_pos)
}));
} else {
match local_begin.fm.src {
Some(ref src) => {
let start_index = local_begin.pos.to_usize();
let end_index = local_end.pos.to_usize();
let source_len = (local_begin.fm.end_pos -
local_begin.fm.start_pos).to_usize();

if start_index > end_index || end_index > source_len {
return Err(SpanSnippetError::MalformedForCodemap(
MalformedCodemapPositions {
name: local_begin.fm.name.clone(),
source_len: source_len,
begin_pos: local_begin.pos,
end_pos: local_end.pos,
}));
}

return Ok((&src[start_index..end_index]).to_string())
}
None => {
return Err(SpanSnippetError::SourceNotAvailable {
filename: local_begin.fm.name.clone()
});
}
self.ensure_filemap_source_present(local_begin.fm.clone());

let start_index = local_begin.pos.to_usize();
let end_index = local_end.pos.to_usize();
let source_len = (local_begin.fm.end_pos -
local_begin.fm.start_pos).to_usize();

if start_index > end_index || end_index > source_len {
return Err(SpanSnippetError::MalformedForCodemap(
MalformedCodemapPositions {
name: local_begin.fm.name.clone(),
source_len: source_len,
begin_pos: local_begin.pos,
end_pos: local_end.pos,
}));
}

if let Some(ref src) = local_begin.fm.src {
return Ok((&src[start_index..end_index]).to_string());
} else if let Some(src) = local_begin.fm.external_src.borrow().get_source() {
return Ok((&src[start_index..end_index]).to_string());
} else {
return Err(SpanSnippetError::SourceNotAvailable {
filename: local_begin.fm.name.clone()
});
}
}
}
Expand Down Expand Up @@ -572,6 +560,10 @@ impl CodeMapper for CodeMap {
}
sp
}
fn ensure_filemap_source_present(&self, file_map: Rc<FileMap>) -> bool {
let src = self.file_loader.read_file(Path::new(&file_map.name)).ok();
return file_map.add_external_src(src)
}
}

#[derive(Clone)]
Expand Down Expand Up @@ -617,6 +609,7 @@ impl FilePathMapping {
#[cfg(test)]
mod tests {
use super::*;
use std::borrow::Cow;
use std::rc::Rc;

#[test]
Expand All @@ -626,12 +619,12 @@ mod tests {
"first line.\nsecond line".to_string());
fm.next_line(BytePos(0));
// Test we can get lines with partial line info.
assert_eq!(fm.get_line(0), Some("first line."));
assert_eq!(fm.get_line(0), Some(Cow::from("first line.")));
// TESTING BROKEN BEHAVIOR: line break declared before actual line break.
fm.next_line(BytePos(10));
assert_eq!(fm.get_line(1), Some("."));
assert_eq!(fm.get_line(1), Some(Cow::from(".")));
fm.next_line(BytePos(12));
assert_eq!(fm.get_line(2), Some("second line"));
assert_eq!(fm.get_line(2), Some(Cow::from("second line")));
}

#[test]
Expand Down
2 changes: 1 addition & 1 deletion src/libsyntax/json.rs
Original file line number Diff line number Diff line change
Expand Up @@ -314,7 +314,7 @@ impl DiagnosticSpanLine {
h_end: usize)
-> DiagnosticSpanLine {
DiagnosticSpanLine {
text: fm.get_line(index).unwrap_or("").to_owned(),
text: fm.get_line(index).map_or(String::new(), |l| l.into_owned()),
highlight_start: h_start,
highlight_end: h_end,
}
Expand Down
1 change: 1 addition & 0 deletions src/libsyntax_pos/Cargo.toml
Original file line number Diff line number Diff line change
Expand Up @@ -10,3 +10,4 @@ crate-type = ["dylib"]

[dependencies]
serialize = { path = "../libserialize" }
rustc_data_structures = { path = "../librustc_data_structures" }
Loading

0 comments on commit 28cc0c5

Please sign in to comment.