Auto merge of #26816 - nrc:zero-codemap, r=@jroesch

See commits for details

Author: bors

src/librustdoc/clean/mod.rs

@@ -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);

src/libsyntax/codemap.rs

@@ -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,
@@ -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,
@@ -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],
@@ -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,
+            // 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 {
@@ -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
@@ -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);
@@ -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 {
@@ -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();
@@ -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 {
@@ -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;
     }
@@ -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]
@@ -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
     }
@@ -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));
     }
@@ -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));
     }
 
@@ -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));
@@ -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
     }
@@ -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