From: Tyson Nottingham <tgnottingham@gmail.com>
Date: Tue, 27 Oct 2020 22:47:29 +0000 (-0700)
Subject: rustc_span: represent line bounds with Range
X-Git-Url: https://git.lizzy.rs/?a=commitdiff_plain;h=47dad31a04724b969d260c32f136bda47fcba63f;p=rust.git

rustc_span: represent line bounds with Range
---

diff --git a/compiler/rustc_span/src/caching_source_map_view.rs b/compiler/rustc_span/src/caching_source_map_view.rs
index 2393ae21613..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 @@ pub fn new(source_map: &'sm SourceMap) -> 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 @@ pub fn byte_pos_to_line_and_col(
 
         // Check if the position is in one of the cached lines
         for cache_entry in self.line_cache.iter_mut() {
-            if line_contains((cache_entry.line_start, cache_entry.line_end), pos) {
+            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,
                 ));
             }
         }
@@ -95,30 +106,13 @@ pub fn byte_pos_to_line_and_col(
         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 line_contains(line_bounds: (BytePos, BytePos), pos: BytePos) -> bool {
-    // This condition will be false in one case where we'd rather it wasn't. 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
-    // below. 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.
-    //
-    // Supposing we only use this function to check whether or not the line 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_bounds.0 <= pos && pos < line_bounds.1
-}
-
 #[inline]
 fn file_contains(file: &SourceFile, pos: BytePos) -> bool {
     // `SourceMap::lookup_source_file_idx` and `SourceFile::contains` both consider the position
diff --git a/compiler/rustc_span/src/lib.rs b/compiler/rustc_span/src/lib.rs
index 21a38283f45..54b0040dc09 100644
--- a/compiler/rustc_span/src/lib.rs
+++ b/compiler/rustc_span/src/lib.rs
@@ -52,7 +52,7 @@
 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,16 +1426,16 @@ pub fn lookup_line(&self, pos: BytePos) -> Option<usize> {
         if line_index >= 0 { Some(line_index as usize) } else { None }
     }
 
-    pub fn line_bounds(&self, line_index: usize) -> (BytePos, BytePos) {
+    pub fn line_bounds(&self, line_index: usize) -> Range<BytePos> {
         if self.is_empty() {
-            return (self.start_pos, self.end_pos);
+            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]
         }
     }