Skip to content

Commit

Permalink
coverage: Restrict empty-span expansion to only cover { and }
Browse files Browse the repository at this point in the history
  • Loading branch information
Zalathar committed Nov 8, 2024
1 parent 996bdab commit 3c30fe3
Show file tree
Hide file tree
Showing 50 changed files with 200 additions and 189 deletions.
70 changes: 42 additions & 28 deletions compiler/rustc_mir_transform/src/coverage/mod.rs
Original file line number Diff line number Diff line change
Expand Up @@ -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};
Expand Down Expand Up @@ -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
Expand All @@ -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_.
Expand Down
1 change: 0 additions & 1 deletion compiler/rustc_mir_transform/src/lib.rs
Original file line number Diff line number Diff line change
Expand Up @@ -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)]
Expand Down
2 changes: 1 addition & 1 deletion compiler/rustc_span/src/source_map.rs
Original file line number Diff line number Diff line change
Expand Up @@ -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>,
{
Expand Down
4 changes: 2 additions & 2 deletions tests/coverage-run-rustdoc/doctest.coverage
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand Down
8 changes: 4 additions & 4 deletions tests/coverage/abort.cov-map
Original file line number Diff line number Diff line change
@@ -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
Expand All @@ -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)
Expand Down
4 changes: 2 additions & 2 deletions tests/coverage/abort.coverage
Original file line number Diff line number Diff line change
Expand Up @@ -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(())
Expand Down
4 changes: 2 additions & 2 deletions tests/coverage/assert.cov-map
Original file line number Diff line number Diff line change
@@ -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
Expand All @@ -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)
Expand Down
8 changes: 4 additions & 4 deletions tests/coverage/async2.cov-map
Original file line number Diff line number Diff line change
Expand Up @@ -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

Expand Down Expand Up @@ -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

4 changes: 2 additions & 2 deletions tests/coverage/async2.coverage
Original file line number Diff line number Diff line change
Expand Up @@ -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() {
Expand All @@ -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() {
Expand Down
18 changes: 9 additions & 9 deletions tests/coverage/branch/if.cov-map
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand All @@ -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
Expand All @@ -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

Expand Down
4 changes: 2 additions & 2 deletions tests/coverage/branch/if.coverage
Original file line number Diff line number Diff line change
Expand Up @@ -30,7 +30,7 @@
------------------
LL| 2| say("not not a");
LL| 2| }
^1
^1
LL| 3| if !!!a {
------------------
| Branch (LL:8): [True: 1, False: 2]
Expand All @@ -54,7 +54,7 @@
------------------
LL| 2| say("not not (a as bool)");
LL| 2| }
^1
^1
LL| 3| if !!!(a as bool) {
------------------
| Branch (LL:8): [True: 1, False: 2]
Expand Down
Loading

0 comments on commit 3c30fe3

Please sign in to comment.