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.

Sign up for GitHub

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

Change treatment of zero-length filemaps #26816

Merged
merged 4 commits into from
Jul 21, 2015
Merged

Change treatment of zero-length filemaps #26816

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
4 commits
Select commit Hold shift + click to select a range
007246c
Allow for space between each filemap in the codemap
nrc Jul 2, 2015
0e907fa
Provide a filemap ctor with line info
nrc Jul 2, 2015
bf34187
Test
nrc Jul 3, 2015
f47d20a
Use a span from the correct file for the inner span of a module
nrc Jul 6, 2015
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
4 changes: 4 additions & 0 deletions src/librustdoc/clean/mod.rs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -1947,6 +1947,10 @@ impl Span {

impl Clean<Span> for syntax::codemap::Span {
fn clean(&self, cx: &DocContext) -> Span {
if *self == DUMMY_SP {
return Span::empty();
}

let cm = cx.sess().codemap();
let filename = cm.span_to_filename(*self);
let lo = cm.lookup_char_pos(self.lo);
Expand Down
194 changes: 107 additions & 87 deletions src/libsyntax/codemap.rs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -115,6 +115,10 @@ impl Sub for CharPos {
/// are *absolute* positions from the beginning of the codemap, not positions
/// relative to FileMaps. Methods on the CodeMap can be used to relate spans back
/// to the original source.
/// You must be careful if the span crosses more than one file - you will not be
/// able to use many of the functions on spans in codemap and you cannot assume
/// that the length of the span = hi - lo; there may be space in the BytePos
/// range between files.
#[derive(Clone, Copy, Hash)]
pub struct Span {
pub lo: BytePos,
Expand Down Expand Up @@ -339,7 +343,7 @@ pub struct MultiByteChar {
pub bytes: usize,
}

/// A single source in the CodeMap
/// A single source in the CodeMap.
pub struct FileMap {
/// The name of the file that the source came from, source that doesn't
/// originate from files has names between angle brackets by convention,
Expand Down Expand Up @@ -508,6 +512,9 @@ impl FileMap {
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],
Expand Down Expand Up @@ -598,27 +605,27 @@ impl CodeMap {
Ok(self.new_filemap(path.to_str().unwrap().to_string(), src))
}

fn next_start_pos(&self) -> usize {
let files = self.files.borrow();
match files.last() {
None => 0,
Copy link
Contributor

Choose a reason for hiding this comment

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

While you're messing with this anyway, maybe this should be 1, rather than 0, so that DUMMY_SP doesn't point into an actual file?

Copy link
Member Author

Choose a reason for hiding this comment

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

This is a good idea, but I couldn't make it work :-( Basically there are a bunch of cases where we have a DUMMY_SP and try and treat it like a proper one. Might be interesting to fix this all properly, but I don't have time.

// Add one so there is some space between files. This lets us distinguish
// positions in the codemap, even in the presence of zero-length files.
Some(last) => last.end_pos.to_usize() + 1,
}
}

/// 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> {
let start_pos = self.next_start_pos();
let mut files = self.files.borrow_mut();
let start_pos = match files.last() {
None => 0,
Some(last) => last.end_pos.to_usize(),
};

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

// Append '\n' in case it's not already there.
// This is a workaround to prevent CodeMap.lookup_filemap_idx from
// accidentally overflowing into the next filemap in case the last byte
// of span is also the last byte of filemap, which leads to incorrect
// results from CodeMap.span_to_*.
if !src.is_empty() && !src.ends_with("\n") {
src.push('\n');
}

let end_pos = start_pos + src.len();

let filemap = Rc::new(FileMap {
Expand All @@ -635,6 +642,21 @@ impl CodeMap {
filemap
}

/// Creates a new filemap and sets its line information.
pub fn new_filemap_and_lines(&self, filename: &str, src: &str) -> Rc<FileMap> {
let fm = self.new_filemap(filename.to_string(), src.to_owned());
let mut byte_pos: u32 = 0;
for line in src.lines() {
// register the start of this line
fm.next_line(BytePos(byte_pos));

// update byte_pos to include this line and the \n at the end
byte_pos += line.len() as u32 + 1;
}
fm
}


/// Allocates a new FileMap representing a source file from an external
/// crate. The source code of such an "imported filemap" is not available,
/// but we still know enough to generate accurate debuginfo location
Expand All @@ -645,11 +667,8 @@ impl CodeMap {
mut file_local_lines: Vec<BytePos>,
mut file_local_multibyte_chars: Vec<MultiByteChar>)
-> Rc<FileMap> {
let start_pos = self.next_start_pos();
let mut files = self.files.borrow_mut();
let start_pos = match files.last() {
None => 0,
Some(last) => last.end_pos.to_usize(),
};

let end_pos = Pos::from_usize(start_pos + source_len);
let start_pos = Pos::from_usize(start_pos);
Expand Down Expand Up @@ -686,39 +705,61 @@ impl CodeMap {

/// Lookup source information about a BytePos
pub fn lookup_char_pos(&self, pos: BytePos) -> Loc {
let FileMapAndLine {fm: f, line: a} = self.lookup_line(pos);
let line = a + 1; // Line numbers start at 1
let chpos = self.bytepos_to_file_charpos(pos);
let linebpos = (*f.lines.borrow())[a];
let linechpos = self.bytepos_to_file_charpos(linebpos);
debug!("byte pos {:?} is on the line at byte pos {:?}",
pos, linebpos);
debug!("char pos {:?} is on the line at char pos {:?}",
chpos, linechpos);
debug!("byte is on line: {}", line);
assert!(chpos >= linechpos);
Loc {
file: f,
line: line,
col: chpos - linechpos
match self.lookup_line(pos) {
Ok(FileMapAndLine { fm: f, line: a }) => {
let line = a + 1; // Line numbers start at 1
let linebpos = (*f.lines.borrow())[a];
let linechpos = self.bytepos_to_file_charpos(linebpos);
debug!("byte pos {:?} is on the line at byte pos {:?}",
pos, linebpos);
debug!("char pos {:?} is on the line at char pos {:?}",
chpos, linechpos);
debug!("byte is on line: {}", line);
assert!(chpos >= linechpos);
Loc {
file: f,
line: line,
col: chpos - linechpos,
}
}
Err(f) => {
Loc {
file: f,
line: 0,
col: chpos,
}
}
}
}

fn lookup_line(&self, pos: BytePos) -> FileMapAndLine {
// If the relevant filemap is empty, we don't return a line number.
fn lookup_line(&self, pos: BytePos) -> Result<FileMapAndLine, Rc<FileMap>> {
let idx = self.lookup_filemap_idx(pos);

let files = self.files.borrow();
let f = (*files)[idx].clone();

let len = f.lines.borrow().len();
if len == 0 {
return Err(f);
}

let mut a = 0;
{
let lines = f.lines.borrow();
let mut b = lines.len();
while b - a > 1 {
let m = (a + b) / 2;
if (*lines)[m] > pos { b = m; } else { a = m; }
if (*lines)[m] > pos {
b = m;
} else {
a = m;
}
}
assert!(a <= lines.len());
}
FileMapAndLine {fm: f, line: a}
Ok(FileMapAndLine { fm: f, line: a })
}

pub fn lookup_char_pos_adj(&self, pos: BytePos) -> LocWithOpt {
Expand Down Expand Up @@ -853,7 +894,7 @@ impl CodeMap {
FileMapAndBytePos {fm: fm, pos: offset}
}

/// Converts an absolute BytePos to a CharPos relative to the filemap and above.
/// Converts an absolute BytePos to a CharPos relative to the filemap.
pub fn bytepos_to_file_charpos(&self, bpos: BytePos) -> CharPos {
let idx = self.lookup_filemap_idx(bpos);
let files = self.files.borrow();
Expand All @@ -880,12 +921,15 @@ impl CodeMap {
CharPos(bpos.to_usize() - map.start_pos.to_usize() - total_extra_bytes)
}

// Return the index of the filemap (in self.files) which contains pos.
fn lookup_filemap_idx(&self, pos: BytePos) -> usize {
let files = self.files.borrow();
let files = &*files;
let len = files.len();
let count = files.len();

// Binary search for the filemap.
let mut a = 0;
let mut b = len;
let mut b = count;
while b - a > 1 {
let m = (a + b) / 2;
if files[m].start_pos > pos {
Expand All @@ -894,26 +938,8 @@ impl CodeMap {
a = m;
}
}
// There can be filemaps with length 0. These have the same start_pos as
// the previous filemap, but are not the filemaps we want (because they
// are length 0, they cannot contain what we are looking for). So,
// rewind until we find a useful filemap.
loop {
let lines = files[a].lines.borrow();
let lines = lines;
if !lines.is_empty() {
break;
}
if a == 0 {
panic!("position {} does not resolve to a source location",
pos.to_usize());
}
a -= 1;
}
if a >= len {
panic!("position {} does not resolve to a source location",
pos.to_usize())
}

assert!(a < count, "position {} does not resolve to a source location", pos.to_usize());

return a;
}
Expand Down Expand Up @@ -1027,10 +1053,13 @@ mod tests {
let fm = cm.new_filemap("blork.rs".to_string(),
"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."));
// TESTING BROKEN BEHAVIOR:
// TESTING BROKEN BEHAVIOR: line break declared before actual line break.
fm.next_line(BytePos(10));
assert_eq!(fm.get_line(1), Some("."));
fm.next_line(BytePos(12));
assert_eq!(fm.get_line(2), Some("second line"));
}

#[test]
Expand All @@ -1056,9 +1085,9 @@ mod tests {

fm1.next_line(BytePos(0));
fm1.next_line(BytePos(12));
fm2.next_line(BytePos(24));
fm3.next_line(BytePos(24));
fm3.next_line(BytePos(34));
fm2.next_line(fm2.start_pos);
fm3.next_line(fm3.start_pos);
fm3.next_line(fm3.start_pos + BytePos(12));

cm
}
Expand All @@ -1068,11 +1097,15 @@ mod tests {
// Test lookup_byte_offset
let cm = init_code_map();

let fmabp1 = cm.lookup_byte_offset(BytePos(22));
let fmabp1 = cm.lookup_byte_offset(BytePos(23));
assert_eq!(fmabp1.fm.name, "blork.rs");
assert_eq!(fmabp1.pos, BytePos(22));
assert_eq!(fmabp1.pos, BytePos(23));

let fmabp1 = cm.lookup_byte_offset(BytePos(24));
assert_eq!(fmabp1.fm.name, "empty.rs");
assert_eq!(fmabp1.pos, BytePos(0));

let fmabp2 = cm.lookup_byte_offset(BytePos(24));
let fmabp2 = cm.lookup_byte_offset(BytePos(25));
assert_eq!(fmabp2.fm.name, "blork2.rs");
assert_eq!(fmabp2.pos, BytePos(0));
}
Expand All @@ -1085,7 +1118,7 @@ mod tests {
let cp1 = cm.bytepos_to_file_charpos(BytePos(22));
assert_eq!(cp1, CharPos(22));

let cp2 = cm.bytepos_to_file_charpos(BytePos(24));
let cp2 = cm.bytepos_to_file_charpos(BytePos(25));
assert_eq!(cp2, CharPos(0));
}

Expand All @@ -1099,7 +1132,7 @@ mod tests {
assert_eq!(loc1.line, 2);
assert_eq!(loc1.col, CharPos(10));

let loc2 = cm.lookup_char_pos(BytePos(24));
let loc2 = cm.lookup_char_pos(BytePos(25));
assert_eq!(loc2.file.name, "blork2.rs");
assert_eq!(loc2.line, 1);
assert_eq!(loc2.col, CharPos(0));
Expand All @@ -1115,18 +1148,18 @@ mod tests {
"first line€€.\n€ second line".to_string());

fm1.next_line(BytePos(0));
fm1.next_line(BytePos(22));
fm2.next_line(BytePos(40));
fm2.next_line(BytePos(58));
fm1.next_line(BytePos(28));
fm2.next_line(fm2.start_pos);
fm2.next_line(fm2.start_pos + BytePos(20));

fm1.record_multibyte_char(BytePos(3), 3);
fm1.record_multibyte_char(BytePos(9), 3);
fm1.record_multibyte_char(BytePos(12), 3);
fm1.record_multibyte_char(BytePos(15), 3);
fm1.record_multibyte_char(BytePos(18), 3);
fm2.record_multibyte_char(BytePos(50), 3);
fm2.record_multibyte_char(BytePos(53), 3);
fm2.record_multibyte_char(BytePos(58), 3);
fm2.record_multibyte_char(fm2.start_pos + BytePos(10), 3);
fm2.record_multibyte_char(fm2.start_pos + BytePos(13), 3);
fm2.record_multibyte_char(fm2.start_pos + BytePos(18), 3);

cm
}
Expand Down Expand Up @@ -1172,27 +1205,14 @@ mod tests {
Span { lo: BytePos(left_index), hi: BytePos(right_index + 1), expn_id: NO_EXPANSION }
}

fn new_filemap_and_lines(cm: &CodeMap, filename: &str, input: &str) -> Rc<FileMap> {
let fm = cm.new_filemap(filename.to_string(), input.to_string());
let mut byte_pos: u32 = 0;
for line in input.lines() {
// register the start of this line
fm.next_line(BytePos(byte_pos));

// update byte_pos to include this line and the \n at the end
byte_pos += line.len() as u32 + 1;
}
fm
}

/// Test span_to_snippet and span_to_lines for a span coverting 3
/// lines in the middle of a file.
#[test]
fn span_to_snippet_and_lines_spanning_multiple_lines() {
let cm = CodeMap::new();
let inputtext = "aaaaa\nbbbbBB\nCCC\nDDDDDddddd\neee\n";
let selection = " \n ^~\n~~~\n~~~~~ \n \n";
new_filemap_and_lines(&cm, "blork.rs", inputtext);
cm.new_filemap_and_lines("blork.rs", inputtext);
let span = span_from_selection(inputtext, selection);

// check that we are extracting the text we thought we were extracting
Expand Down
Loading