Skip to content

Implement lazy loading of external crates' sources. #42593

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

Merged
merged 12 commits into from
Jun 18, 2017
Merged
Show file tree
Hide file tree
Changes from 1 commit
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
Prev Previous commit
Next Next commit
Added consumption logic for external sources in FileMap
We now fetch source lines from the `external_src` member as a secondary
fallback if no regular source is present, that is, if the file map
belongs to an external crate and the source has been fetched from disk.
  • Loading branch information
ibabushkin committed Jun 11, 2017
commit a5b8851e220d7a80222ea04d242603dd3392d17b
6 changes: 4 additions & 2 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 @@ -911,7 +912,8 @@ 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 annotated_file.file.src.is_none()
&& annotated_file.file.external_src.borrow().is_absent() {
Copy link
Member

Choose a reason for hiding this comment

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

This could be a single method on FileMap. I'd even put the attempt to load the external source in that method.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

We can't do that, since said method would have to be defined in a place where the FileLoader doesn't exist.

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
11 changes: 6 additions & 5 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 Down Expand Up @@ -122,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 @@ -184,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 @@ -206,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
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
46 changes: 28 additions & 18 deletions src/libsyntax_pos/lib.rs
Original file line number Diff line number Diff line change
Expand Up @@ -33,6 +33,7 @@
#![cfg_attr(stage0, feature(rustc_private))]
#![cfg_attr(stage0, feature(staged_api))]

use std::borrow::Cow;
use std::cell::{Cell, RefCell};
use std::ops::{Add, Sub};
use std::rc::Rc;
Expand Down Expand Up @@ -605,24 +606,33 @@ impl FileMap {

/// get a line from the list of pre-computed line-beginnings.
Copy link
Member

Choose a reason for hiding this comment

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

Heh, I can see where you got the bad style from though (this is very old code).

/// line-number here is 0-based.
pub fn get_line(&self, line_number: usize) -> Option<&str> {
match self.src {
Some(ref src) => {
let lines = self.lines.borrow();
lines.get(line_number).map(|&line| {
let begin: BytePos = line - self.start_pos;
let begin = begin.to_usize();
// We can't use `lines.get(line_number+1)` because we might
// be parsing when we call this function and thus the current
// line is the last one we have line info for.
let slice = &src[begin..];
match slice.find('\n') {
Some(e) => &slice[..e],
None => slice
}
})
pub fn get_line(&self, line_number: usize) -> Option<Cow<str>> {
fn get_until_newline(src: &str, begin: usize) -> &str {
// We can't use `lines.get(line_number+1)` because we might
// be parsing when we call this function and thus the current
// line is the last one we have line info for.
let slice = &src[begin..];
match slice.find('\n') {
Some(e) => &slice[..e],
None => slice
}
None => None
}

let lines = self.lines.borrow();
let line = if let Some(line) = lines.get(line_number) {
line
} else {
return None;
};
let begin: BytePos = *line - self.start_pos;
let begin = begin.to_usize();

if let Some(ref src) = self.src {
Some(Cow::from(get_until_newline(src, begin)))
} else if let Some(src) = self.external_src.borrow().get_source() {
Some(Cow::Owned(String::from(get_until_newline(src, begin))))
} else {
None
}
}

Expand All @@ -641,7 +651,7 @@ impl FileMap {
}

pub fn is_imported(&self) -> bool {
self.src.is_none() // TODO: change to something more sensible
self.src.is_none()
}

pub fn byte_length(&self) -> u32 {
Expand Down