coverage: Restrict empty-span expansion to only cover { and }

Author: Zalathar

compiler/rustc_mir_transform/src/coverage/mod.rs

@@ -22,7 +22,7 @@ use rustc_middle::mir::{
 use rustc_middle::ty::TyCtxt;
 use rustc_span::def_id::LocalDefId;
 use rustc_span::source_map::SourceMap;
-use rustc_span::{BytePos, Pos, RelativeBytePos, SourceFile, Span};
+use rustc_span::{BytePos, Pos, SourceFile, Span};
 use tracing::{debug, debug_span, trace};
 
 use crate::coverage::counters::{CounterIncrementSite, CoverageCounters};
@@ -391,6 +391,41 @@ fn inject_statement(mir_body: &mut mir::Body<'_>, counter_kind: CoverageKind, bb
     data.statements.insert(0, statement);
 }
 
+fn ensure_non_empty_span(
+    source_map: &SourceMap,
+    hir_info: &ExtractedHirInfo,
+    span: Span,
+) -> Option<Span> {
+    if !span.is_empty() {
+        return Some(span);
+    }
+
+    let lo = span.lo();
+    let hi = span.hi();
+
+    // The span is empty, so try to expand it to cover an adjacent '{' or '}',
+    // but only within the bounds of the body span.
+    let try_next = hi < hir_info.body_span.hi();
+    let try_prev = hir_info.body_span.lo() < lo;
+    if !(try_next || try_prev) {
+        return None;
+    }
+
+    source_map
+        .span_to_source(span, |src, start, end| try {
+            // We're only checking for specific ASCII characters, so we don't
+            // have to worry about multi-byte code points.
+            if try_next && src.as_bytes()[end] == b'{' {
+                Some(span.with_hi(hi + BytePos(1)))
+            } else if try_prev && src.as_bytes()[start - 1] == b'}' {
+                Some(span.with_lo(lo - BytePos(1)))
+            } else {
+                None
+            }
+        })
+        .ok()?
+}
+
 /// Converts the span into its start line and column, and end line and column.
 ///
 /// Line numbers and column numbers are 1-based. Unlike most column numbers emitted by
@@ -407,44 +442,23 @@ fn make_source_region(
     file: &SourceFile,
     span: Span,
 ) -> Option<SourceRegion> {
+    let span = ensure_non_empty_span(source_map, hir_info, span)?;
+
     let lo = span.lo();
     let hi = span.hi();
 
     // Column numbers need to be in bytes, so we can't use the more convenient
     // `SourceMap` methods for looking up file coordinates.
-    let rpos_and_line_and_byte_column = |pos: BytePos| -> Option<(RelativeBytePos, usize, usize)> {
+    let line_and_byte_column = |pos: BytePos| -> Option<(usize, usize)> {
         let rpos = file.relative_position(pos);
         let line_index = file.lookup_line(rpos)?;
         let line_start = file.lines()[line_index];
         // Line numbers and column numbers are 1-based, so add 1 to each.
-        Some((rpos, line_index + 1, (rpos - line_start).to_usize() + 1))
+        Some((line_index + 1, (rpos - line_start).to_usize() + 1))
     };
 
-    let (lo_rpos, mut start_line, mut start_col) = rpos_and_line_and_byte_column(lo)?;
-    let (hi_rpos, mut end_line, mut end_col) = rpos_and_line_and_byte_column(hi)?;
-
-    // If the span is empty, try to expand it horizontally by one character's
-    // worth of bytes, so that it is more visible in `llvm-cov` reports.
-    // We do this after resolving line/column numbers, so that empty spans at the
-    // end of a line get an extra column instead of wrapping to the next line.
-    let body_span = hir_info.body_span;
-    if span.is_empty()
-        && body_span.contains(span)
-        && let Some(src) = &file.src
-    {
-        // Prefer to expand the end position, if it won't go outside the body span.
-        if hi < body_span.hi() {
-            let hi_rpos = hi_rpos.to_usize();
-            let nudge_bytes = src.ceil_char_boundary(hi_rpos + 1) - hi_rpos;
-            end_col += nudge_bytes;
-        } else if lo > body_span.lo() {
-            let lo_rpos = lo_rpos.to_usize();
-            let nudge_bytes = lo_rpos - src.floor_char_boundary(lo_rpos - 1);
-            // Subtract the nudge, but don't go below column 1.
-            start_col = start_col.saturating_sub(nudge_bytes).max(1);
-        }
-        // If neither nudge could be applied, stick with the empty span coordinates.
-    }
+    let (mut start_line, start_col) = line_and_byte_column(lo)?;
+    let (mut end_line, end_col) = line_and_byte_column(hi)?;
 
     // Apply an offset so that code in doctests has correct line numbers.
     // FIXME(#79417): Currently we have no way to offset doctest _columns_.

compiler/rustc_mir_transform/src/lib.rs

@@ -9,7 +9,6 @@
 #![feature(let_chains)]
 #![feature(map_try_insert)]
 #![feature(never_type)]
-#![feature(round_char_boundary)]
 #![feature(try_blocks)]
 #![feature(yeet_expr)]
 #![warn(unreachable_pub)]

compiler/rustc_span/src/source_map.rs

@@ -534,7 +534,7 @@ impl SourceMap {
     /// Extracts the source surrounding the given `Span` using the `extract_source` function. The
     /// extract function takes three arguments: a string slice containing the source, an index in
     /// the slice for the beginning of the span and an index in the slice for the end of the span.
-    fn span_to_source<F, T>(&self, sp: Span, extract_source: F) -> Result<T, SpanSnippetError>
+    pub fn span_to_source<F, T>(&self, sp: Span, extract_source: F) -> Result<T, SpanSnippetError>
     where
         F: Fn(&str, usize, usize) -> Result<T, SpanSnippetError>,
     {

tests/coverage-run-rustdoc/doctest.coverage

@@ -45,11 +45,11 @@ $DIR/doctest.rs:
    LL|      1|//!     if *res.as_ref().unwrap_err() == *res.as_ref().unwrap_err() {
    LL|      1|//!         println!("{:?}", res);
    LL|      1|//!     }
-                   ^0
+                  ^0
    LL|      1|//!     if *res.as_ref().unwrap_err() == *res.as_ref().unwrap_err() {
    LL|      1|//!         res = Ok(1);
    LL|      1|//!     }
-                   ^0
+                  ^0
    LL|      1|//!     res = Ok(0);
    LL|       |//! }
    LL|       |//! // need to be explicit because rustdoc cant infer the return type

tests/coverage/abort.cov-map

@@ -1,5 +1,5 @@
 Function name: abort::main
-Raw bytes (89): 0x[01, 01, 0a, 01, 27, 05, 09, 03, 0d, 22, 11, 03, 0d, 03, 0d, 22, 15, 03, 0d, 03, 0d, 05, 09, 0d, 01, 0d, 01, 01, 1b, 03, 02, 0b, 00, 18, 22, 01, 0c, 00, 19, 11, 00, 1a, 02, 0a, 0e, 02, 0a, 00, 0b, 22, 02, 0c, 00, 19, 15, 00, 1a, 00, 31, 1a, 00, 31, 00, 32, 22, 04, 0c, 00, 19, 05, 00, 1a, 00, 31, 09, 00, 31, 00, 32, 27, 01, 09, 00, 17, 0d, 02, 05, 01, 02]
+Raw bytes (89): 0x[01, 01, 0a, 01, 27, 05, 09, 03, 0d, 22, 11, 03, 0d, 03, 0d, 22, 15, 03, 0d, 03, 0d, 05, 09, 0d, 01, 0d, 01, 01, 1b, 03, 02, 0b, 00, 18, 22, 01, 0c, 00, 19, 11, 00, 1a, 02, 0a, 0e, 02, 09, 00, 0a, 22, 02, 0c, 00, 19, 15, 00, 1a, 00, 31, 1a, 00, 30, 00, 31, 22, 04, 0c, 00, 19, 05, 00, 1a, 00, 31, 09, 00, 30, 00, 31, 27, 01, 09, 00, 17, 0d, 02, 05, 01, 02]
 Number of files: 1
 - file 0 => global file 1
 Number of expressions: 10
@@ -20,17 +20,17 @@ Number of file 0 mappings: 13
 - Code(Expression(8, Sub)) at (prev + 1, 12) to (start + 0, 25)
     = ((c0 + (c1 + c2)) - c3)
 - Code(Counter(4)) at (prev + 0, 26) to (start + 2, 10)
-- Code(Expression(3, Sub)) at (prev + 2, 10) to (start + 0, 11)
+- Code(Expression(3, Sub)) at (prev + 2, 9) to (start + 0, 10)
     = (((c0 + (c1 + c2)) - c3) - c4)
 - Code(Expression(8, Sub)) at (prev + 2, 12) to (start + 0, 25)
     = ((c0 + (c1 + c2)) - c3)
 - Code(Counter(5)) at (prev + 0, 26) to (start + 0, 49)
-- Code(Expression(6, Sub)) at (prev + 0, 49) to (start + 0, 50)
+- Code(Expression(6, Sub)) at (prev + 0, 48) to (start + 0, 49)
     = (((c0 + (c1 + c2)) - c3) - c5)
 - Code(Expression(8, Sub)) at (prev + 4, 12) to (start + 0, 25)
     = ((c0 + (c1 + c2)) - c3)
 - Code(Counter(1)) at (prev + 0, 26) to (start + 0, 49)
-- Code(Counter(2)) at (prev + 0, 49) to (start + 0, 50)
+- Code(Counter(2)) at (prev + 0, 48) to (start + 0, 49)
 - Code(Expression(9, Add)) at (prev + 1, 9) to (start + 0, 23)
     = (c1 + c2)
 - Code(Counter(3)) at (prev + 2, 5) to (start + 1, 2)

tests/coverage/abort.coverage

@@ -18,12 +18,12 @@
    LL|      6|        }
    LL|       |        // See discussion (below the `Notes` section) on coverage results for the closing brace.
    LL|     10|        if countdown < 5 { might_abort(false); } // Counts for different regions on one line.
-                                       ^4                     ^6
+                                       ^4                    ^6
    LL|       |        // For the following example, the closing brace is the last character on the line.
    LL|       |        // This shows the character after the closing brace is highlighted, even if that next
    LL|       |        // character is a newline.
    LL|     10|        if countdown < 5 { might_abort(false); }
-                                       ^4                     ^6
+                                       ^4                    ^6
    LL|     10|        countdown -= 1;
    LL|       |    }
    LL|      1|    Ok(())

tests/coverage/assert.cov-map

@@ -1,5 +1,5 @@
 Function name: assert::main
-Raw bytes (65): 0x[01, 01, 08, 01, 1b, 05, 1f, 09, 0d, 03, 11, 16, 05, 03, 11, 05, 1f, 09, 0d, 09, 01, 09, 01, 01, 1b, 03, 02, 0b, 00, 18, 16, 01, 0c, 00, 1a, 05, 00, 1b, 02, 0a, 12, 02, 13, 00, 20, 09, 00, 21, 02, 0a, 0d, 02, 0a, 00, 0b, 1b, 01, 09, 00, 17, 11, 02, 05, 01, 02]
+Raw bytes (65): 0x[01, 01, 08, 01, 1b, 05, 1f, 09, 0d, 03, 11, 16, 05, 03, 11, 05, 1f, 09, 0d, 09, 01, 09, 01, 01, 1b, 03, 02, 0b, 00, 18, 16, 01, 0c, 00, 1a, 05, 00, 1b, 02, 0a, 12, 02, 13, 00, 20, 09, 00, 21, 02, 0a, 0d, 02, 09, 00, 0a, 1b, 01, 09, 00, 17, 11, 02, 05, 01, 02]
 Number of files: 1
 - file 0 => global file 1
 Number of expressions: 8
@@ -21,7 +21,7 @@ Number of file 0 mappings: 9
 - Code(Expression(4, Sub)) at (prev + 2, 19) to (start + 0, 32)
     = (((c0 + (c1 + (c2 + c3))) - c4) - c1)
 - Code(Counter(2)) at (prev + 0, 33) to (start + 2, 10)
-- Code(Counter(3)) at (prev + 2, 10) to (start + 0, 11)
+- Code(Counter(3)) at (prev + 2, 9) to (start + 0, 10)
 - Code(Expression(6, Add)) at (prev + 1, 9) to (start + 0, 23)
     = (c1 + (c2 + c3))
 - Code(Counter(4)) at (prev + 2, 5) to (start + 1, 2)

tests/coverage/async2.cov-map

@@ -8,14 +8,14 @@ Number of file 0 mappings: 1
 Highest counter ID seen: c0
 
 Function name: async2::async_func::{closure#0}
-Raw bytes (24): 0x[01, 01, 00, 04, 01, 10, 17, 03, 09, 05, 03, 0a, 02, 06, 00, 02, 06, 00, 07, 01, 01, 01, 00, 02]
+Raw bytes (24): 0x[01, 01, 00, 04, 01, 10, 17, 03, 09, 05, 03, 0a, 02, 06, 00, 02, 05, 00, 06, 01, 01, 01, 00, 02]
 Number of files: 1
 - file 0 => global file 1
 Number of expressions: 0
 Number of file 0 mappings: 4
 - Code(Counter(0)) at (prev + 16, 23) to (start + 3, 9)
 - Code(Counter(1)) at (prev + 3, 10) to (start + 2, 6)
-- Code(Zero) at (prev + 2, 6) to (start + 0, 7)
+- Code(Zero) at (prev + 2, 5) to (start + 0, 6)
 - Code(Counter(0)) at (prev + 1, 1) to (start + 0, 2)
 Highest counter ID seen: c1
 
@@ -47,14 +47,14 @@ Number of file 0 mappings: 1
 Highest counter ID seen: c0
 
 Function name: async2::non_async_func
-Raw bytes (24): 0x[01, 01, 00, 04, 01, 08, 01, 03, 09, 05, 03, 0a, 02, 06, 00, 02, 06, 00, 07, 01, 01, 01, 00, 02]
+Raw bytes (24): 0x[01, 01, 00, 04, 01, 08, 01, 03, 09, 05, 03, 0a, 02, 06, 00, 02, 05, 00, 06, 01, 01, 01, 00, 02]
 Number of files: 1
 - file 0 => global file 1
 Number of expressions: 0
 Number of file 0 mappings: 4
 - Code(Counter(0)) at (prev + 8, 1) to (start + 3, 9)
 - Code(Counter(1)) at (prev + 3, 10) to (start + 2, 6)
-- Code(Zero) at (prev + 2, 6) to (start + 0, 7)
+- Code(Zero) at (prev + 2, 5) to (start + 0, 6)
 - Code(Counter(0)) at (prev + 1, 1) to (start + 0, 2)
 Highest counter ID seen: c1
 

tests/coverage/async2.coverage

@@ -11,7 +11,7 @@
    LL|      1|    if b {
    LL|      1|        println!("non_async_func println in block");
    LL|      1|    }
-                   ^0
+                  ^0
    LL|      1|}
    LL|       |
    LL|      1|async fn async_func() {
@@ -20,7 +20,7 @@
    LL|      1|    if b {
    LL|      1|        println!("async_func println in block");
    LL|      1|    }
-                   ^0
+                  ^0
    LL|      1|}
    LL|       |
    LL|      1|async fn async_func_just_println() {

tests/coverage/branch/if.cov-map

@@ -25,7 +25,7 @@ Number of file 0 mappings: 8
 Highest counter ID seen: c4
 
 Function name: if::branch_not
-Raw bytes (116): 0x[01, 01, 07, 05, 09, 05, 0d, 05, 0d, 05, 11, 05, 11, 05, 15, 05, 15, 12, 01, 0c, 01, 01, 10, 05, 03, 08, 00, 09, 20, 09, 02, 00, 08, 00, 09, 09, 01, 09, 00, 11, 02, 01, 06, 00, 07, 05, 01, 08, 00, 0a, 20, 0a, 0d, 00, 08, 00, 0a, 0a, 00, 0b, 02, 06, 0d, 02, 06, 00, 07, 05, 01, 08, 00, 0b, 20, 11, 12, 00, 08, 00, 0b, 11, 00, 0c, 02, 06, 12, 02, 06, 00, 07, 05, 01, 08, 00, 0c, 20, 1a, 15, 00, 08, 00, 0c, 1a, 00, 0d, 02, 06, 15, 02, 06, 00, 07, 05, 01, 01, 00, 02]
+Raw bytes (116): 0x[01, 01, 07, 05, 09, 05, 0d, 05, 0d, 05, 11, 05, 11, 05, 15, 05, 15, 12, 01, 0c, 01, 01, 10, 05, 03, 08, 00, 09, 20, 09, 02, 00, 08, 00, 09, 09, 01, 09, 00, 11, 02, 01, 05, 00, 06, 05, 01, 08, 00, 0a, 20, 0a, 0d, 00, 08, 00, 0a, 0a, 00, 0b, 02, 06, 0d, 02, 05, 00, 06, 05, 01, 08, 00, 0b, 20, 11, 12, 00, 08, 00, 0b, 11, 00, 0c, 02, 06, 12, 02, 05, 00, 06, 05, 01, 08, 00, 0c, 20, 1a, 15, 00, 08, 00, 0c, 1a, 00, 0d, 02, 06, 15, 02, 05, 00, 06, 05, 01, 01, 00, 02]
 Number of files: 1
 - file 0 => global file 1
 Number of expressions: 7
@@ -43,34 +43,34 @@ Number of file 0 mappings: 18
     true  = c2
     false = (c1 - c2)
 - Code(Counter(2)) at (prev + 1, 9) to (start + 0, 17)
-- Code(Expression(0, Sub)) at (prev + 1, 6) to (start + 0, 7)
+- Code(Expression(0, Sub)) at (prev + 1, 5) to (start + 0, 6)
     = (c1 - c2)
 - Code(Counter(1)) at (prev + 1, 8) to (start + 0, 10)
 - Branch { true: Expression(2, Sub), false: Counter(3) } at (prev + 0, 8) to (start + 0, 10)
     true  = (c1 - c3)
     false = c3
 - Code(Expression(2, Sub)) at (prev + 0, 11) to (start + 2, 6)
     = (c1 - c3)
-- Code(Counter(3)) at (prev + 2, 6) to (start + 0, 7)
+- Code(Counter(3)) at (prev + 2, 5) to (start + 0, 6)
 - Code(Counter(1)) at (prev + 1, 8) to (start + 0, 11)
 - Branch { true: Counter(4), false: Expression(4, Sub) } at (prev + 0, 8) to (start + 0, 11)
     true  = c4
     false = (c1 - c4)
 - Code(Counter(4)) at (prev + 0, 12) to (start + 2, 6)
-- Code(Expression(4, Sub)) at (prev + 2, 6) to (start + 0, 7)
+- Code(Expression(4, Sub)) at (prev + 2, 5) to (start + 0, 6)
     = (c1 - c4)
 - Code(Counter(1)) at (prev + 1, 8) to (start + 0, 12)
 - Branch { true: Expression(6, Sub), false: Counter(5) } at (prev + 0, 8) to (start + 0, 12)
     true  = (c1 - c5)
     false = c5
 - Code(Expression(6, Sub)) at (prev + 0, 13) to (start + 2, 6)
     = (c1 - c5)
-- Code(Counter(5)) at (prev + 2, 6) to (start + 0, 7)
+- Code(Counter(5)) at (prev + 2, 5) to (start + 0, 6)
 - Code(Counter(1)) at (prev + 1, 1) to (start + 0, 2)
 Highest counter ID seen: c5
 
 Function name: if::branch_not_as
-Raw bytes (90): 0x[01, 01, 05, 05, 09, 05, 0d, 05, 0d, 05, 11, 05, 11, 0e, 01, 1d, 01, 01, 10, 05, 03, 08, 00, 14, 20, 02, 09, 00, 08, 00, 14, 02, 00, 15, 02, 06, 09, 02, 06, 00, 07, 05, 01, 08, 00, 15, 20, 0d, 0a, 00, 08, 00, 15, 0d, 00, 16, 02, 06, 0a, 02, 06, 00, 07, 05, 01, 08, 00, 16, 20, 12, 11, 00, 08, 00, 16, 12, 00, 17, 02, 06, 11, 02, 06, 00, 07, 05, 01, 01, 00, 02]
+Raw bytes (90): 0x[01, 01, 05, 05, 09, 05, 0d, 05, 0d, 05, 11, 05, 11, 0e, 01, 1d, 01, 01, 10, 05, 03, 08, 00, 14, 20, 02, 09, 00, 08, 00, 14, 02, 00, 15, 02, 06, 09, 02, 05, 00, 06, 05, 01, 08, 00, 15, 20, 0d, 0a, 00, 08, 00, 15, 0d, 00, 16, 02, 06, 0a, 02, 05, 00, 06, 05, 01, 08, 00, 16, 20, 12, 11, 00, 08, 00, 16, 12, 00, 17, 02, 06, 11, 02, 05, 00, 06, 05, 01, 01, 00, 02]
 Number of files: 1
 - file 0 => global file 1
 Number of expressions: 5
@@ -87,21 +87,21 @@ Number of file 0 mappings: 14
     false = c2
 - Code(Expression(0, Sub)) at (prev + 0, 21) to (start + 2, 6)
     = (c1 - c2)
-- Code(Counter(2)) at (prev + 2, 6) to (start + 0, 7)
+- Code(Counter(2)) at (prev + 2, 5) to (start + 0, 6)
 - Code(Counter(1)) at (prev + 1, 8) to (start + 0, 21)
 - Branch { true: Counter(3), false: Expression(2, Sub) } at (prev + 0, 8) to (start + 0, 21)
     true  = c3
     false = (c1 - c3)
 - Code(Counter(3)) at (prev + 0, 22) to (start + 2, 6)
-- Code(Expression(2, Sub)) at (prev + 2, 6) to (start + 0, 7)
+- Code(Expression(2, Sub)) at (prev + 2, 5) to (start + 0, 6)
     = (c1 - c3)
 - Code(Counter(1)) at (prev + 1, 8) to (start + 0, 22)
 - Branch { true: Expression(4, Sub), false: Counter(4) } at (prev + 0, 8) to (start + 0, 22)
     true  = (c1 - c4)
     false = c4
 - Code(Expression(4, Sub)) at (prev + 0, 23) to (start + 2, 6)
     = (c1 - c4)
-- Code(Counter(4)) at (prev + 2, 6) to (start + 0, 7)
+- Code(Counter(4)) at (prev + 2, 5) to (start + 0, 6)
 - Code(Counter(1)) at (prev + 1, 1) to (start + 0, 2)
 Highest counter ID seen: c4