diff options
| -rw-r--r-- | compiler/rustc_span/src/caching_source_map_view.rs | 42 | ||||
| -rw-r--r-- | compiler/rustc_span/src/lib.rs | 21 |
2 files changed, 46 insertions, 17 deletions
diff --git a/compiler/rustc_span/src/caching_source_map_view.rs b/compiler/rustc_span/src/caching_source_map_view.rs index 68b0bd1a574..15dd00fb483 100644 --- a/compiler/rustc_span/src/caching_source_map_view.rs +++ b/compiler/rustc_span/src/caching_source_map_view.rs @@ -1,13 +1,25 @@ use crate::source_map::SourceMap; use crate::{BytePos, SourceFile}; use rustc_data_structures::sync::Lrc; +use std::ops::Range; #[derive(Clone)] struct CacheEntry { time_stamp: usize, line_number: usize, - line_start: BytePos, - line_end: BytePos, + // The line's byte position range in the `SourceMap`. This range will fail to contain a valid + // position in certain edge cases. Spans often start/end one past something, and when that + // something is the last character of a file (this can happen when a file doesn't end in a + // newline, for example), we'd still like for the position to be considered within the last + // line. However, it isn't according to the exclusive upper bound of this range. We cannot + // change the upper bound to be inclusive, because for most lines, the upper bound is the same + // as the lower bound of the next line, so there would be an ambiguity. + // + // Since the containment aspect of this range is only used to see whether or not the cache + // entry contains a position, the only ramification of the above is that we will get cache + // misses for these rare positions. A line lookup for the position via `SourceMap::lookup_line` + // after a cache miss will produce the last line number, as desired. + line: Range<BytePos>, file: Lrc<SourceFile>, file_index: usize, } @@ -26,8 +38,7 @@ impl<'sm> CachingSourceMapView<'sm> { let entry = CacheEntry { time_stamp: 0, line_number: 0, - line_start: BytePos(0), - line_end: BytePos(0), + line: BytePos(0)..BytePos(0), file: first_file, file_index: 0, }; @@ -47,13 +58,13 @@ impl<'sm> CachingSourceMapView<'sm> { // Check if the position is in one of the cached lines for cache_entry in self.line_cache.iter_mut() { - if pos >= cache_entry.line_start && pos < cache_entry.line_end { + if cache_entry.line.contains(&pos) { cache_entry.time_stamp = self.time_stamp; return Some(( cache_entry.file.clone(), cache_entry.line_number, - pos - cache_entry.line_start, + pos - cache_entry.line.start, )); } } @@ -69,13 +80,13 @@ impl<'sm> CachingSourceMapView<'sm> { let cache_entry = &mut self.line_cache[oldest]; // If the entry doesn't point to the correct file, fix it up - if pos < cache_entry.file.start_pos || pos >= cache_entry.file.end_pos { + if !file_contains(&cache_entry.file, pos) { let file_valid; if self.source_map.files().len() > 0 { let file_index = self.source_map.lookup_source_file_idx(pos); let file = self.source_map.files()[file_index].clone(); - if pos >= file.start_pos && pos < file.end_pos { + if file_contains(&file, pos) { cache_entry.file = file; cache_entry.file_index = file_index; file_valid = true; @@ -95,10 +106,19 @@ impl<'sm> CachingSourceMapView<'sm> { let line_bounds = cache_entry.file.line_bounds(line_index); cache_entry.line_number = line_index + 1; - cache_entry.line_start = line_bounds.0; - cache_entry.line_end = line_bounds.1; + cache_entry.line = line_bounds; cache_entry.time_stamp = self.time_stamp; - Some((cache_entry.file.clone(), cache_entry.line_number, pos - cache_entry.line_start)) + Some((cache_entry.file.clone(), cache_entry.line_number, pos - cache_entry.line.start)) } } + +#[inline] +fn file_contains(file: &SourceFile, pos: BytePos) -> bool { + // `SourceMap::lookup_source_file_idx` and `SourceFile::contains` both consider the position + // one past the end of a file to belong to it. Normally, that's what we want. But for the + // purposes of converting a byte position to a line and column number, we can't come up with a + // line and column number if the file is empty, because an empty file doesn't contain any + // lines. So for our purposes, we don't consider empty files to contain any byte position. + file.contains(pos) && !file.is_empty() +} diff --git a/compiler/rustc_span/src/lib.rs b/compiler/rustc_span/src/lib.rs index 79363c3a5ca..0e3027273ab 100644 --- a/compiler/rustc_span/src/lib.rs +++ b/compiler/rustc_span/src/lib.rs @@ -52,7 +52,7 @@ use std::cell::RefCell; use std::cmp::{self, Ordering}; use std::fmt; use std::hash::Hash; -use std::ops::{Add, Sub}; +use std::ops::{Add, Range, Sub}; use std::path::{Path, PathBuf}; use std::str::FromStr; @@ -1426,24 +1426,33 @@ impl SourceFile { if line_index >= 0 { Some(line_index as usize) } else { None } } - pub fn line_bounds(&self, line_index: usize) -> (BytePos, BytePos) { - if self.start_pos == self.end_pos { - return (self.start_pos, self.end_pos); + pub fn line_bounds(&self, line_index: usize) -> Range<BytePos> { + if self.is_empty() { + return self.start_pos..self.end_pos; } assert!(line_index < self.lines.len()); if line_index == (self.lines.len() - 1) { - (self.lines[line_index], self.end_pos) + self.lines[line_index]..self.end_pos } else { - (self.lines[line_index], self.lines[line_index + 1]) + self.lines[line_index]..self.lines[line_index + 1] } } + /// Returns whether or not the file contains the given `SourceMap` byte + /// position. The position one past the end of the file is considered to be + /// contained by the file. This implies that files for which `is_empty` + /// returns true still contain one byte position according to this function. #[inline] pub fn contains(&self, byte_pos: BytePos) -> bool { byte_pos >= self.start_pos && byte_pos <= self.end_pos } + #[inline] + pub fn is_empty(&self) -> bool { + self.start_pos == self.end_pos + } + /// Calculates the original byte position relative to the start of the file /// based on the given byte position. pub fn original_relative_byte_pos(&self, pos: BytePos) -> BytePos { |