Skip to content

Commit 3043b70

Browse files
committed
Modify the codemap code to use more slices and to information about
columns within a line, not just the line numbers. Also try to clarify and use the term `line_index` when 0-based.
1 parent 8710c76 commit 3043b70

File tree

2 files changed

+133
-40
lines changed

2 files changed

+133
-40
lines changed

src/libsyntax/codemap.rs

Lines changed: 92 additions & 13 deletions
Original file line numberDiff line numberDiff line change
@@ -47,7 +47,7 @@ pub struct BytePos(pub u32);
4747
/// A character offset. Because of multibyte utf8 characters, a byte offset
4848
/// is not equivalent to a character offset. The CodeMap will convert BytePos
4949
/// values to CharPos values as necessary.
50-
#[derive(Copy, Clone, PartialEq, Hash, PartialOrd, Debug)]
50+
#[derive(Copy, Clone, PartialEq, Eq, Hash, PartialOrd, Debug)]
5151
pub struct CharPos(pub usize);
5252

5353
// FIXME: Lots of boilerplate in these impls, but so far my attempts to fix
@@ -299,9 +299,21 @@ impl ExpnId {
299299

300300
pub type FileName = String;
301301

302+
#[derive(Copy, Clone, Debug, PartialEq, Eq)]
303+
pub struct LineInfo {
304+
/// Index of line, starting from 0.
305+
pub line_index: usize,
306+
307+
/// Column in line where span begins, starting from 0.
308+
pub start_col: CharPos,
309+
310+
/// Column in line where span ends, starting from 0, exclusive.
311+
pub end_col: CharPos,
312+
}
313+
302314
pub struct FileLines {
303315
pub file: Rc<FileMap>,
304-
pub lines: Vec<usize>
316+
pub lines: Vec<LineInfo>
305317
}
306318

307319
/// Identifies an offset of a multi-byte character in a FileMap
@@ -467,9 +479,9 @@ impl FileMap {
467479
lines.push(pos);
468480
}
469481

470-
/// get a line from the list of pre-computed line-beginnings
471-
///
472-
pub fn get_line(&self, line_number: usize) -> Option<String> {
482+
/// get a line from the list of pre-computed line-beginnings.
483+
/// line-number here is 0-based.
484+
pub fn get_line(&self, line_number: usize) -> Option<&str> {
473485
match self.src {
474486
Some(ref src) => {
475487
let lines = self.lines.borrow();
@@ -480,7 +492,7 @@ impl FileMap {
480492
match slice.find('\n') {
481493
Some(e) => &slice[..e],
482494
None => slice
483-
}.to_string()
495+
}
484496
})
485497
}
486498
None => None
@@ -649,10 +661,29 @@ impl CodeMap {
649661
pub fn span_to_lines(&self, sp: Span) -> FileLines {
650662
let lo = self.lookup_char_pos(sp.lo);
651663
let hi = self.lookup_char_pos(sp.hi);
652-
let mut lines = Vec::new();
653-
for i in lo.line - 1..hi.line {
654-
lines.push(i);
655-
};
664+
let mut lines = Vec::with_capacity(hi.line - lo.line + 1);
665+
666+
// The span starts partway through the first line,
667+
// but after that it starts from offset 0.
668+
let mut start_col = lo.col;
669+
670+
// For every line but the last, it extends from `start_col`
671+
// and to the end of the line. Be careful because the line
672+
// numbers in Loc are 1-based, so we subtract 1 to get 0-based
673+
// lines.
674+
for line_index in lo.line-1 .. hi.line-1 {
675+
let line_len = lo.file.get_line(line_index).map(|s| s.len()).unwrap_or(0);
676+
lines.push(LineInfo { line_index: line_index,
677+
start_col: start_col,
678+
end_col: CharPos::from_usize(line_len) });
679+
start_col = CharPos::from_usize(0);
680+
}
681+
682+
// For the last line, it extends from `start_col` to `hi.col`:
683+
lines.push(LineInfo { line_index: hi.line - 1,
684+
start_col: start_col,
685+
end_col: hi.col });
686+
656687
FileLines {file: lo.file, lines: lines}
657688
}
658689

@@ -907,17 +938,18 @@ pub struct MalformedCodemapPositions {
907938
#[cfg(test)]
908939
mod test {
909940
use super::*;
941+
use std::rc::Rc;
910942

911943
#[test]
912944
fn t1 () {
913945
let cm = CodeMap::new();
914946
let fm = cm.new_filemap("blork.rs".to_string(),
915947
"first line.\nsecond line".to_string());
916948
fm.next_line(BytePos(0));
917-
assert_eq!(fm.get_line(0), Some("first line.".to_string()));
949+
assert_eq!(fm.get_line(0), Some("first line."));
918950
// TESTING BROKEN BEHAVIOR:
919951
fm.next_line(BytePos(10));
920-
assert_eq!(fm.get_line(1), Some(".".to_string()));
952+
assert_eq!(fm.get_line(1), Some("."));
921953
}
922954

923955
#[test]
@@ -1045,7 +1077,54 @@ mod test {
10451077

10461078
assert_eq!(file_lines.file.name, "blork.rs");
10471079
assert_eq!(file_lines.lines.len(), 1);
1048-
assert_eq!(file_lines.lines[0], 1);
1080+
assert_eq!(file_lines.lines[0].line_index, 1);
1081+
}
1082+
1083+
/// Given a string like " ^~~~~~~~~~~~ ", produces a span
1084+
/// coverting that range. The idea is that the string has the same
1085+
/// length as the input, and we uncover the byte positions. Note
1086+
/// that this can span lines and so on.
1087+
fn span_from_selection(input: &str, selection: &str) -> Span {
1088+
assert_eq!(input.len(), selection.len());
1089+
let left_index = selection.find('^').unwrap() as u32;
1090+
let right_index = selection.rfind('~').unwrap() as u32;
1091+
Span { lo: BytePos(left_index), hi: BytePos(right_index + 1), expn_id: NO_EXPANSION }
1092+
}
1093+
1094+
fn new_filemap_and_lines(cm: &CodeMap, filename: &str, input: &str) -> Rc<FileMap> {
1095+
let fm = cm.new_filemap(filename.to_string(), input.to_string());
1096+
let mut byte_pos: u32 = 0;
1097+
for line in input.lines() {
1098+
// register the start of this line
1099+
fm.next_line(BytePos(byte_pos));
1100+
1101+
// update byte_pos to include this line and the \n at the end
1102+
byte_pos += line.len() as u32 + 1;
1103+
}
1104+
fm
1105+
}
1106+
1107+
/// Test span_to_snippet and span_to_lines for a span coverting 3
1108+
/// lines in the middle of a file.
1109+
#[test]
1110+
fn span_to_snippet_and_lines_spanning_multiple_lines() {
1111+
let cm = CodeMap::new();
1112+
let inputtext = "aaaaa\nbbbbBB\nCCC\nDDDDDddddd\neee\n";
1113+
let selection = " \n ^~\n~~~\n~~~~~ \n \n";
1114+
new_filemap_and_lines(&cm, "blork.rs", inputtext);
1115+
let span = span_from_selection(inputtext, selection);
1116+
1117+
// check that we are extracting the text we thought we were extracting
1118+
assert_eq!(&cm.span_to_snippet(span).unwrap(), "BB\nCCC\nDDDDD");
1119+
1120+
// check that span_to_lines gives us the complete result with the lines/cols we expected
1121+
let lines = cm.span_to_lines(span);
1122+
let expected = vec![
1123+
LineInfo { line_index: 1, start_col: CharPos(4), end_col: CharPos(6) },
1124+
LineInfo { line_index: 2, start_col: CharPos(0), end_col: CharPos(3) },
1125+
LineInfo { line_index: 3, start_col: CharPos(0), end_col: CharPos(5) }
1126+
];
1127+
assert_eq!(lines.lines, expected);
10491128
}
10501129

10511130
#[test]

src/libsyntax/diagnostic.rs

Lines changed: 41 additions & 27 deletions
Original file line numberDiff line numberDiff line change
@@ -483,25 +483,39 @@ fn highlight_lines(err: &mut EmitterWriter,
483483
cm: &codemap::CodeMap,
484484
sp: Span,
485485
lvl: Level,
486-
lines: codemap::FileLines) -> io::Result<()> {
486+
lines: codemap::FileLines)
487+
-> io::Result<()>
488+
{
487489
let fm = &*lines.file;
488490

489-
let mut elided = false;
490-
let mut display_lines = &lines.lines[..];
491-
if display_lines.len() > MAX_LINES {
492-
display_lines = &display_lines[0..MAX_LINES];
493-
elided = true;
494-
}
491+
let line_strings: Option<Vec<&str>> =
492+
lines.lines.iter()
493+
.map(|info| fm.get_line(info.line_index))
494+
.collect();
495+
496+
let line_strings = match line_strings {
497+
None => { return Ok(()); }
498+
Some(line_strings) => line_strings
499+
};
500+
501+
// Display only the first MAX_LINES lines.
502+
let all_lines = lines.lines.len();
503+
let display_lines = cmp::min(all_lines, MAX_LINES);
504+
let display_line_infos = &lines.lines[..display_lines];
505+
let display_line_strings = &line_strings[..display_lines];
506+
495507
// Print the offending lines
496-
for &line_number in display_lines {
497-
if let Some(line) = fm.get_line(line_number) {
498-
try!(write!(&mut err.dst, "{}:{} {}\n", fm.name,
499-
line_number + 1, line));
500-
}
508+
for (line_info, line) in display_line_infos.iter().zip(display_line_strings.iter()) {
509+
try!(write!(&mut err.dst, "{}:{} {}\n",
510+
fm.name,
511+
line_info.line_index + 1,
512+
line));
501513
}
502-
if elided {
503-
let last_line = display_lines[display_lines.len() - 1];
504-
let s = format!("{}:{} ", fm.name, last_line + 1);
514+
515+
// If we elided something, put an ellipsis.
516+
if display_lines < all_lines {
517+
let last_line_index = display_line_infos.last().unwrap().line_index;
518+
let s = format!("{}:{} ", fm.name, last_line_index + 1);
505519
try!(write!(&mut err.dst, "{0:1$}...\n", "", s.len()));
506520
}
507521

@@ -510,7 +524,7 @@ fn highlight_lines(err: &mut EmitterWriter,
510524
if lines.lines.len() == 1 {
511525
let lo = cm.lookup_char_pos(sp.lo);
512526
let mut digits = 0;
513-
let mut num = (lines.lines[0] + 1) / 10;
527+
let mut num = (lines.lines[0].line_index + 1) / 10;
514528

515529
// how many digits must be indent past?
516530
while num > 0 { num /= 10; digits += 1; }
@@ -522,7 +536,7 @@ fn highlight_lines(err: &mut EmitterWriter,
522536
for _ in 0..skip {
523537
s.push(' ');
524538
}
525-
if let Some(orig) = fm.get_line(lines.lines[0]) {
539+
if let Some(orig) = fm.get_line(lines.lines[0].line_index) {
526540
let mut col = skip;
527541
let mut lastc = ' ';
528542
let mut iter = orig.chars().enumerate();
@@ -597,32 +611,32 @@ fn end_highlight_lines(w: &mut EmitterWriter,
597611

598612
let lines = &lines.lines[..];
599613
if lines.len() > MAX_LINES {
600-
if let Some(line) = fm.get_line(lines[0]) {
614+
if let Some(line) = fm.get_line(lines[0].line_index) {
601615
try!(write!(&mut w.dst, "{}:{} {}\n", fm.name,
602-
lines[0] + 1, line));
616+
lines[0].line_index + 1, line));
603617
}
604618
try!(write!(&mut w.dst, "...\n"));
605-
let last_line_number = lines[lines.len() - 1];
606-
if let Some(last_line) = fm.get_line(last_line_number) {
619+
let last_line_index = lines[lines.len() - 1].line_index;
620+
if let Some(last_line) = fm.get_line(last_line_index) {
607621
try!(write!(&mut w.dst, "{}:{} {}\n", fm.name,
608-
last_line_number + 1, last_line));
622+
last_line_index + 1, last_line));
609623
}
610624
} else {
611-
for &line_number in lines {
612-
if let Some(line) = fm.get_line(line_number) {
625+
for line_info in lines {
626+
if let Some(line) = fm.get_line(line_info.line_index) {
613627
try!(write!(&mut w.dst, "{}:{} {}\n", fm.name,
614-
line_number + 1, line));
628+
line_info.line_index + 1, line));
615629
}
616630
}
617631
}
618-
let last_line_start = format!("{}:{} ", fm.name, lines[lines.len()-1]+1);
632+
let last_line_start = format!("{}:{} ", fm.name, lines[lines.len()-1].line_index + 1);
619633
let hi = cm.lookup_char_pos(sp.hi);
620634
let skip = last_line_start.width(false);
621635
let mut s = String::new();
622636
for _ in 0..skip {
623637
s.push(' ');
624638
}
625-
if let Some(orig) = fm.get_line(lines[0]) {
639+
if let Some(orig) = fm.get_line(lines[0].line_index) {
626640
let iter = orig.chars().enumerate();
627641
for (pos, ch) in iter {
628642
// Span seems to use half-opened interval, so subtract 1

0 commit comments

Comments
 (0)