about summary refs log tree commit diff
diff options
context:
space:
mode:
authorZalathar <Zalathar@users.noreply.github.com>2024-11-24 18:50:08 +1100
committerZalathar <Zalathar@users.noreply.github.com>2024-11-24 23:46:39 +1100
commitb9fb1a69d2fe1c146cbdf181cf9e0eaf15935799 (patch)
treef7201981d931b6a5e099bc29ea45cc1c3b0c5de8
parent87fe7def1284e1c25dc358c693a9add93db5a953 (diff)
downloadrust-b9fb1a69d2fe1c146cbdf181cf9e0eaf15935799.tar.gz
rust-b9fb1a69d2fe1c146cbdf181cf9e0eaf15935799.zip
coverage: Store coverage source regions as `Span` until codegen
-rw-r--r--compiler/rustc_codegen_llvm/src/coverageinfo/ffi.rs28
-rw-r--r--compiler/rustc_codegen_llvm/src/coverageinfo/map_data.rs14
-rw-r--r--compiler/rustc_codegen_llvm/src/coverageinfo/mapgen.rs26
-rw-r--r--compiler/rustc_codegen_llvm/src/coverageinfo/mapgen/spans.rs124
-rw-r--r--compiler/rustc_codegen_llvm/src/lib.rs1
-rw-r--r--compiler/rustc_middle/src/mir/coverage.rs18
-rw-r--r--compiler/rustc_middle/src/mir/pretty.rs4
-rw-r--r--compiler/rustc_mir_transform/src/coverage/mod.rs166
-rw-r--r--tests/mir-opt/coverage/branch_match_arms.main.InstrumentCoverage.diff12
-rw-r--r--tests/mir-opt/coverage/instrument_coverage.bar.InstrumentCoverage.diff2
-rw-r--r--tests/mir-opt/coverage/instrument_coverage.main.InstrumentCoverage.diff10
-rw-r--r--tests/mir-opt/coverage/instrument_coverage_cleanup.main.CleanupPostBorrowck.diff10
-rw-r--r--tests/mir-opt/coverage/instrument_coverage_cleanup.main.InstrumentCoverage.diff10
13 files changed, 199 insertions, 226 deletions
diff --git a/compiler/rustc_codegen_llvm/src/coverageinfo/ffi.rs b/compiler/rustc_codegen_llvm/src/coverageinfo/ffi.rs
index 7aa03908f0f..19d6726002c 100644
--- a/compiler/rustc_codegen_llvm/src/coverageinfo/ffi.rs
+++ b/compiler/rustc_codegen_llvm/src/coverageinfo/ffi.rs
@@ -1,6 +1,4 @@
-use rustc_middle::mir::coverage::{CounterId, CovTerm, ExpressionId, SourceRegion};
-
-use crate::coverageinfo::mapgen::LocalFileId;
+use rustc_middle::mir::coverage::{CounterId, CovTerm, ExpressionId};
 
 /// Must match the layout of `LLVMRustCounterKind`.
 #[derive(Copy, Clone, Debug)]
@@ -126,30 +124,16 @@ pub(crate) struct CoverageSpan {
     /// Local index into the function's local-to-global file ID table.
     /// The value at that index is itself an index into the coverage filename
     /// table in the CGU's `__llvm_covmap` section.
-    file_id: u32,
+    pub(crate) file_id: u32,
 
     /// 1-based starting line of the source code span.
-    start_line: u32,
+    pub(crate) start_line: u32,
     /// 1-based starting column of the source code span.
-    start_col: u32,
+    pub(crate) start_col: u32,
     /// 1-based ending line of the source code span.
-    end_line: u32,
+    pub(crate) end_line: u32,
     /// 1-based ending column of the source code span. High bit must be unset.
-    end_col: u32,
-}
-
-impl CoverageSpan {
-    pub(crate) fn from_source_region(
-        local_file_id: LocalFileId,
-        code_region: &SourceRegion,
-    ) -> Self {
-        let file_id = local_file_id.as_u32();
-        let &SourceRegion { start_line, start_col, end_line, end_col } = code_region;
-        // Internally, LLVM uses the high bit of `end_col` to distinguish between
-        // code regions and gap regions, so it can't be used by the column number.
-        assert!(end_col & (1u32 << 31) == 0, "high bit of `end_col` must be unset: {end_col:#X}");
-        Self { file_id, start_line, start_col, end_line, end_col }
-    }
+    pub(crate) end_col: u32,
 }
 
 /// Must match the layout of `LLVMRustCoverageCodeRegion`.
diff --git a/compiler/rustc_codegen_llvm/src/coverageinfo/map_data.rs b/compiler/rustc_codegen_llvm/src/coverageinfo/map_data.rs
index e3c0df27883..95746b88ced 100644
--- a/compiler/rustc_codegen_llvm/src/coverageinfo/map_data.rs
+++ b/compiler/rustc_codegen_llvm/src/coverageinfo/map_data.rs
@@ -3,9 +3,9 @@ use rustc_data_structures::fx::FxIndexSet;
 use rustc_index::bit_set::BitSet;
 use rustc_middle::mir::coverage::{
     CounterId, CovTerm, Expression, ExpressionId, FunctionCoverageInfo, Mapping, MappingKind, Op,
-    SourceRegion,
 };
 use rustc_middle::ty::Instance;
+use rustc_span::Span;
 use tracing::{debug, instrument};
 
 use crate::coverageinfo::ffi::{Counter, CounterExpression, ExprKind};
@@ -220,16 +220,16 @@ impl<'tcx> FunctionCoverage<'tcx> {
         })
     }
 
-    /// Converts this function's coverage mappings into an intermediate form
-    /// that will be used by `mapgen` when preparing for FFI.
-    pub(crate) fn counter_regions(
+    /// Yields all this function's coverage mappings, after simplifying away
+    /// unused counters and counter expressions.
+    pub(crate) fn mapping_spans(
         &self,
-    ) -> impl Iterator<Item = (MappingKind, &SourceRegion)> + ExactSizeIterator {
+    ) -> impl Iterator<Item = (MappingKind, Span)> + ExactSizeIterator + Captures<'_> {
         self.function_coverage_info.mappings.iter().map(move |mapping| {
-            let Mapping { kind, source_region } = mapping;
+            let &Mapping { ref kind, span } = mapping;
             let kind =
                 kind.map_terms(|term| if self.is_zero_term(term) { CovTerm::Zero } else { term });
-            (kind, source_region)
+            (kind, span)
         })
     }
 
diff --git a/compiler/rustc_codegen_llvm/src/coverageinfo/mapgen.rs b/compiler/rustc_codegen_llvm/src/coverageinfo/mapgen.rs
index 0c79cd5c262..e8dfed1f06a 100644
--- a/compiler/rustc_codegen_llvm/src/coverageinfo/mapgen.rs
+++ b/compiler/rustc_codegen_llvm/src/coverageinfo/mapgen.rs
@@ -1,3 +1,5 @@
+mod spans;
+
 use std::ffi::CString;
 use std::iter;
 
@@ -201,7 +203,7 @@ rustc_index::newtype_index! {
     /// An index into a function's list of global file IDs. That underlying list
     /// of local-to-global mappings will be embedded in the function's record in
     /// the `__llvm_covfun` linker section.
-    pub(crate) struct LocalFileId {}
+    struct LocalFileId {}
 }
 
 /// Holds a mapping from "local" (per-function) file IDs to "global" (per-CGU)
@@ -244,11 +246,13 @@ fn encode_mappings_for_function(
     global_file_table: &GlobalFileTable,
     function_coverage: &FunctionCoverage<'_>,
 ) -> Vec<u8> {
-    let counter_regions = function_coverage.counter_regions();
-    if counter_regions.is_empty() {
+    let mapping_spans = function_coverage.mapping_spans();
+    if mapping_spans.is_empty() {
         return Vec::new();
     }
 
+    let fn_cov_info = function_coverage.function_coverage_info;
+
     let expressions = function_coverage.counter_expressions().collect::<Vec<_>>();
 
     let mut virtual_file_mapping = VirtualFileMapping::default();
@@ -258,7 +262,9 @@ fn encode_mappings_for_function(
     let mut mcdc_decision_regions = vec![];
 
     // Currently a function's mappings must all be in the same file as its body span.
-    let file_name = span_file_name(tcx, function_coverage.function_coverage_info.body_span);
+    let file_name = span_file_name(tcx, fn_cov_info.body_span);
+    let source_map = tcx.sess.source_map();
+    let source_file = source_map.lookup_source_file(fn_cov_info.body_span.lo());
 
     // Look up the global file ID for that filename.
     let global_file_id = global_file_table.global_file_id_for_file_name(file_name);
@@ -267,11 +273,15 @@ fn encode_mappings_for_function(
     let local_file_id = virtual_file_mapping.local_id_for_global(global_file_id);
     debug!("  file id: {local_file_id:?} => {global_file_id:?} = '{file_name:?}'");
 
-    // For each counter/region pair in this function+file, convert it to a
+    let make_cov_span = |span| {
+        spans::make_coverage_span(local_file_id, source_map, fn_cov_info, &source_file, span)
+    };
+
+    // For each coverage mapping span in this function+file, convert it to a
     // form suitable for FFI.
-    for (mapping_kind, region) in counter_regions {
-        debug!("Adding counter {mapping_kind:?} to map for {region:?}");
-        let cov_span = ffi::CoverageSpan::from_source_region(local_file_id, region);
+    for (mapping_kind, span) in mapping_spans {
+        debug!("Adding counter {mapping_kind:?} to map for {span:?}");
+        let Some(cov_span) = make_cov_span(span) else { continue };
         match mapping_kind {
             MappingKind::Code(term) => {
                 code_regions
diff --git a/compiler/rustc_codegen_llvm/src/coverageinfo/mapgen/spans.rs b/compiler/rustc_codegen_llvm/src/coverageinfo/mapgen/spans.rs
new file mode 100644
index 00000000000..4a7721879fd
--- /dev/null
+++ b/compiler/rustc_codegen_llvm/src/coverageinfo/mapgen/spans.rs
@@ -0,0 +1,124 @@
+use rustc_middle::mir::coverage::FunctionCoverageInfo;
+use rustc_span::source_map::SourceMap;
+use rustc_span::{BytePos, Pos, SourceFile, Span};
+use tracing::debug;
+
+use crate::coverageinfo::ffi;
+use crate::coverageinfo::mapgen::LocalFileId;
+
+/// 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
+/// the compiler, these column numbers are denoted in **bytes**, because that's what
+/// LLVM's `llvm-cov` tool expects to see in coverage maps.
+///
+/// Returns `None` if the conversion failed for some reason. This shouldn't happen,
+/// but it's hard to rule out entirely (especially in the presence of complex macros
+/// or other expansions), and if it does happen then skipping a span or function is
+/// better than an ICE or `llvm-cov` failure that the user might have no way to avoid.
+pub(crate) fn make_coverage_span(
+    file_id: LocalFileId,
+    source_map: &SourceMap,
+    fn_cov_info: &FunctionCoverageInfo,
+    file: &SourceFile,
+    span: Span,
+) -> Option<ffi::CoverageSpan> {
+    let span = ensure_non_empty_span(source_map, fn_cov_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 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((line_index + 1, (rpos - line_start).to_usize() + 1))
+    };
+
+    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_.
+    start_line = source_map.doctest_offset_line(&file.name, start_line);
+    end_line = source_map.doctest_offset_line(&file.name, end_line);
+
+    check_coverage_span(ffi::CoverageSpan {
+        file_id: file_id.as_u32(),
+        start_line: start_line as u32,
+        start_col: start_col as u32,
+        end_line: end_line as u32,
+        end_col: end_col as u32,
+    })
+}
+
+fn ensure_non_empty_span(
+    source_map: &SourceMap,
+    fn_cov_info: &FunctionCoverageInfo,
+    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 < fn_cov_info.body_span.hi();
+    let try_prev = fn_cov_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()?
+}
+
+/// If `llvm-cov` sees a source region that is improperly ordered (end < start),
+/// it will immediately exit with a fatal error. To prevent that from happening,
+/// discard regions that are improperly ordered, or might be interpreted in a
+/// way that makes them improperly ordered.
+fn check_coverage_span(cov_span: ffi::CoverageSpan) -> Option<ffi::CoverageSpan> {
+    let ffi::CoverageSpan { file_id: _, start_line, start_col, end_line, end_col } = cov_span;
+
+    // Line/column coordinates are supposed to be 1-based. If we ever emit
+    // coordinates of 0, `llvm-cov` might misinterpret them.
+    let all_nonzero = [start_line, start_col, end_line, end_col].into_iter().all(|x| x != 0);
+    // Coverage mappings use the high bit of `end_col` to indicate that a
+    // region is actually a "gap" region, so make sure it's unset.
+    let end_col_has_high_bit_unset = (end_col & (1 << 31)) == 0;
+    // If a region is improperly ordered (end < start), `llvm-cov` will exit
+    // with a fatal error, which is inconvenient for users and hard to debug.
+    let is_ordered = (start_line, start_col) <= (end_line, end_col);
+
+    if all_nonzero && end_col_has_high_bit_unset && is_ordered {
+        Some(cov_span)
+    } else {
+        debug!(
+            ?cov_span,
+            ?all_nonzero,
+            ?end_col_has_high_bit_unset,
+            ?is_ordered,
+            "Skipping source region that would be misinterpreted or rejected by LLVM"
+        );
+        // If this happens in a debug build, ICE to make it easier to notice.
+        debug_assert!(false, "Improper source region: {cov_span:?}");
+        None
+    }
+}
diff --git a/compiler/rustc_codegen_llvm/src/lib.rs b/compiler/rustc_codegen_llvm/src/lib.rs
index 3dfb86d422d..9f398107fc6 100644
--- a/compiler/rustc_codegen_llvm/src/lib.rs
+++ b/compiler/rustc_codegen_llvm/src/lib.rs
@@ -17,6 +17,7 @@
 #![feature(iter_intersperse)]
 #![feature(let_chains)]
 #![feature(rustdoc_internals)]
+#![feature(try_blocks)]
 #![warn(unreachable_pub)]
 // tidy-alphabetical-end
 
diff --git a/compiler/rustc_middle/src/mir/coverage.rs b/compiler/rustc_middle/src/mir/coverage.rs
index 11a4e7f89a7..33f9a2bdca6 100644
--- a/compiler/rustc_middle/src/mir/coverage.rs
+++ b/compiler/rustc_middle/src/mir/coverage.rs
@@ -155,22 +155,6 @@ impl Debug for CoverageKind {
     }
 }
 
-#[derive(Clone, TyEncodable, TyDecodable, Hash, HashStable, PartialEq, Eq, PartialOrd, Ord)]
-#[derive(TypeFoldable, TypeVisitable)]
-pub struct SourceRegion {
-    pub start_line: u32,
-    pub start_col: u32,
-    pub end_line: u32,
-    pub end_col: u32,
-}
-
-impl Debug for SourceRegion {
-    fn fmt(&self, fmt: &mut Formatter<'_>) -> fmt::Result {
-        let &Self { start_line, start_col, end_line, end_col } = self;
-        write!(fmt, "{start_line}:{start_col} - {end_line}:{end_col}")
-    }
-}
-
 #[derive(Copy, Clone, Debug, PartialEq, Eq, Hash, HashStable)]
 #[derive(TyEncodable, TyDecodable, TypeFoldable, TypeVisitable)]
 pub enum Op {
@@ -232,7 +216,7 @@ impl MappingKind {
 #[derive(TyEncodable, TyDecodable, Hash, HashStable, TypeFoldable, TypeVisitable)]
 pub struct Mapping {
     pub kind: MappingKind,
-    pub source_region: SourceRegion,
+    pub span: Span,
 }
 
 /// Stores per-function coverage information attached to a `mir::Body`,
diff --git a/compiler/rustc_middle/src/mir/pretty.rs b/compiler/rustc_middle/src/mir/pretty.rs
index 2bfcd0a6227..ece468947c2 100644
--- a/compiler/rustc_middle/src/mir/pretty.rs
+++ b/compiler/rustc_middle/src/mir/pretty.rs
@@ -603,8 +603,8 @@ fn write_function_coverage_info(
     for (id, expression) in expressions.iter_enumerated() {
         writeln!(w, "{INDENT}coverage {id:?} => {expression:?};")?;
     }
-    for coverage::Mapping { kind, source_region } in mappings {
-        writeln!(w, "{INDENT}coverage {kind:?} => {source_region:?};")?;
+    for coverage::Mapping { kind, span } in mappings {
+        writeln!(w, "{INDENT}coverage {kind:?} => {span:?};")?;
     }
     writeln!(w)?;
 
diff --git a/compiler/rustc_mir_transform/src/coverage/mod.rs b/compiler/rustc_mir_transform/src/coverage/mod.rs
index e8b3d80be02..02fd289b6ef 100644
--- a/compiler/rustc_mir_transform/src/coverage/mod.rs
+++ b/compiler/rustc_mir_transform/src/coverage/mod.rs
@@ -13,16 +13,15 @@ use rustc_hir::intravisit::{Visitor, walk_expr};
 use rustc_middle::hir::map::Map;
 use rustc_middle::hir::nested_filter;
 use rustc_middle::mir::coverage::{
-    CoverageKind, DecisionInfo, FunctionCoverageInfo, Mapping, MappingKind, SourceRegion,
+    CoverageKind, DecisionInfo, FunctionCoverageInfo, Mapping, MappingKind,
 };
 use rustc_middle::mir::{
     self, BasicBlock, BasicBlockData, SourceInfo, Statement, StatementKind, Terminator,
     TerminatorKind,
 };
 use rustc_middle::ty::TyCtxt;
+use rustc_span::Span;
 use rustc_span::def_id::LocalDefId;
-use rustc_span::source_map::SourceMap;
-use rustc_span::{BytePos, Pos, SourceFile, Span};
 use tracing::{debug, debug_span, trace};
 
 use crate::coverage::counters::{CounterIncrementSite, CoverageCounters};
@@ -97,7 +96,7 @@ fn instrument_function_for_coverage<'tcx>(tcx: TyCtxt<'tcx>, mir_body: &mut mir:
     let coverage_counters =
         CoverageCounters::make_bcb_counters(&basic_coverage_blocks, &bcbs_with_counter_mappings);
 
-    let mappings = create_mappings(tcx, &hir_info, &extracted_mappings, &coverage_counters);
+    let mappings = create_mappings(&extracted_mappings, &coverage_counters);
     if mappings.is_empty() {
         // No spans could be converted into valid mappings, so skip this function.
         debug!("no spans could be converted into valid mappings; skipping");
@@ -136,18 +135,12 @@ fn instrument_function_for_coverage<'tcx>(tcx: TyCtxt<'tcx>, mir_body: &mut mir:
 ///
 /// Precondition: All BCBs corresponding to those spans have been given
 /// coverage counters.
-fn create_mappings<'tcx>(
-    tcx: TyCtxt<'tcx>,
-    hir_info: &ExtractedHirInfo,
+fn create_mappings(
     extracted_mappings: &ExtractedMappings,
     coverage_counters: &CoverageCounters,
 ) -> Vec<Mapping> {
-    let source_map = tcx.sess.source_map();
-    let file = source_map.lookup_source_file(hir_info.body_span.lo());
-
     let term_for_bcb =
         |bcb| coverage_counters.term_for_bcb(bcb).expect("all BCBs with spans were given counters");
-    let region_for_span = |span: Span| make_source_region(source_map, hir_info, &file, span);
 
     // Fully destructure the mappings struct to make sure we don't miss any kinds.
     let ExtractedMappings {
@@ -160,22 +153,20 @@ fn create_mappings<'tcx>(
     } = extracted_mappings;
     let mut mappings = Vec::new();
 
-    mappings.extend(code_mappings.iter().filter_map(
+    mappings.extend(code_mappings.iter().map(
         // Ordinary code mappings are the simplest kind.
         |&mappings::CodeMapping { span, bcb }| {
-            let source_region = region_for_span(span)?;
             let kind = MappingKind::Code(term_for_bcb(bcb));
-            Some(Mapping { kind, source_region })
+            Mapping { kind, span }
         },
     ));
 
-    mappings.extend(branch_pairs.iter().filter_map(
+    mappings.extend(branch_pairs.iter().map(
         |&mappings::BranchPair { span, true_bcb, false_bcb }| {
             let true_term = term_for_bcb(true_bcb);
             let false_term = term_for_bcb(false_bcb);
             let kind = MappingKind::Branch { true_term, false_term };
-            let source_region = region_for_span(span)?;
-            Some(Mapping { kind, source_region })
+            Mapping { kind, span }
         },
     ));
 
@@ -183,7 +174,7 @@ fn create_mappings<'tcx>(
         |bcb| coverage_counters.term_for_bcb(bcb).expect("all BCBs with spans were given counters");
 
     // MCDC branch mappings are appended with their decisions in case decisions were ignored.
-    mappings.extend(mcdc_degraded_branches.iter().filter_map(
+    mappings.extend(mcdc_degraded_branches.iter().map(
         |&mappings::MCDCBranch {
              span,
              true_bcb,
@@ -192,10 +183,9 @@ fn create_mappings<'tcx>(
              true_index: _,
              false_index: _,
          }| {
-            let source_region = region_for_span(span)?;
             let true_term = term_for_bcb(true_bcb);
             let false_term = term_for_bcb(false_bcb);
-            Some(Mapping { kind: MappingKind::Branch { true_term, false_term }, source_region })
+            Mapping { kind: MappingKind::Branch { true_term, false_term }, span }
         },
     ));
 
@@ -203,7 +193,7 @@ fn create_mappings<'tcx>(
         let num_conditions = branches.len() as u16;
         let conditions = branches
             .into_iter()
-            .filter_map(
+            .map(
                 |&mappings::MCDCBranch {
                      span,
                      true_bcb,
@@ -212,31 +202,29 @@ fn create_mappings<'tcx>(
                      true_index: _,
                      false_index: _,
                  }| {
-                    let source_region = region_for_span(span)?;
                     let true_term = term_for_bcb(true_bcb);
                     let false_term = term_for_bcb(false_bcb);
-                    Some(Mapping {
+                    Mapping {
                         kind: MappingKind::MCDCBranch {
                             true_term,
                             false_term,
                             mcdc_params: condition_info,
                         },
-                        source_region,
-                    })
+                        span,
+                    }
                 },
             )
             .collect::<Vec<_>>();
 
-        if conditions.len() == num_conditions as usize
-            && let Some(source_region) = region_for_span(decision.span)
-        {
+        if conditions.len() == num_conditions as usize {
             // LLVM requires end index for counter mapping regions.
             let kind = MappingKind::MCDCDecision(DecisionInfo {
                 bitmap_idx: (decision.bitmap_idx + decision.num_test_vectors) as u32,
                 num_conditions,
             });
             mappings.extend(
-                std::iter::once(Mapping { kind, source_region }).chain(conditions.into_iter()),
+                std::iter::once(Mapping { kind, span: decision.span })
+                    .chain(conditions.into_iter()),
             );
         } else {
             mappings.extend(conditions.into_iter().map(|mapping| {
@@ -245,10 +233,7 @@ fn create_mappings<'tcx>(
                 else {
                     unreachable!("all mappings here are MCDCBranch as shown above");
                 };
-                Mapping {
-                    kind: MappingKind::Branch { true_term, false_term },
-                    source_region: mapping.source_region,
-                }
+                Mapping { kind: MappingKind::Branch { true_term, false_term }, span: mapping.span }
             }))
         }
     }
@@ -391,121 +376,6 @@ 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
-/// the compiler, these column numbers are denoted in **bytes**, because that's what
-/// LLVM's `llvm-cov` tool expects to see in coverage maps.
-///
-/// Returns `None` if the conversion failed for some reason. This shouldn't happen,
-/// but it's hard to rule out entirely (especially in the presence of complex macros
-/// or other expansions), and if it does happen then skipping a span or function is
-/// better than an ICE or `llvm-cov` failure that the user might have no way to avoid.
-fn make_source_region(
-    source_map: &SourceMap,
-    hir_info: &ExtractedHirInfo,
-    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 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((line_index + 1, (rpos - line_start).to_usize() + 1))
-    };
-
-    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_.
-    start_line = source_map.doctest_offset_line(&file.name, start_line);
-    end_line = source_map.doctest_offset_line(&file.name, end_line);
-
-    check_source_region(SourceRegion {
-        start_line: start_line as u32,
-        start_col: start_col as u32,
-        end_line: end_line as u32,
-        end_col: end_col as u32,
-    })
-}
-
-/// If `llvm-cov` sees a source region that is improperly ordered (end < start),
-/// it will immediately exit with a fatal error. To prevent that from happening,
-/// discard regions that are improperly ordered, or might be interpreted in a
-/// way that makes them improperly ordered.
-fn check_source_region(source_region: SourceRegion) -> Option<SourceRegion> {
-    let SourceRegion { start_line, start_col, end_line, end_col } = source_region;
-
-    // Line/column coordinates are supposed to be 1-based. If we ever emit
-    // coordinates of 0, `llvm-cov` might misinterpret them.
-    let all_nonzero = [start_line, start_col, end_line, end_col].into_iter().all(|x| x != 0);
-    // Coverage mappings use the high bit of `end_col` to indicate that a
-    // region is actually a "gap" region, so make sure it's unset.
-    let end_col_has_high_bit_unset = (end_col & (1 << 31)) == 0;
-    // If a region is improperly ordered (end < start), `llvm-cov` will exit
-    // with a fatal error, which is inconvenient for users and hard to debug.
-    let is_ordered = (start_line, start_col) <= (end_line, end_col);
-
-    if all_nonzero && end_col_has_high_bit_unset && is_ordered {
-        Some(source_region)
-    } else {
-        debug!(
-            ?source_region,
-            ?all_nonzero,
-            ?end_col_has_high_bit_unset,
-            ?is_ordered,
-            "Skipping source region that would be misinterpreted or rejected by LLVM"
-        );
-        // If this happens in a debug build, ICE to make it easier to notice.
-        debug_assert!(false, "Improper source region: {source_region:?}");
-        None
-    }
-}
-
 /// Function information extracted from HIR by the coverage instrumentor.
 #[derive(Debug)]
 struct ExtractedHirInfo {
diff --git a/tests/mir-opt/coverage/branch_match_arms.main.InstrumentCoverage.diff b/tests/mir-opt/coverage/branch_match_arms.main.InstrumentCoverage.diff
index cbb11d50f79..39d52ba698a 100644
--- a/tests/mir-opt/coverage/branch_match_arms.main.InstrumentCoverage.diff
+++ b/tests/mir-opt/coverage/branch_match_arms.main.InstrumentCoverage.diff
@@ -33,12 +33,12 @@
 +     coverage ExpressionId(3) => Expression { lhs: Counter(3), op: Add, rhs: Counter(2) };
 +     coverage ExpressionId(4) => Expression { lhs: Expression(3), op: Add, rhs: Counter(1) };
 +     coverage ExpressionId(5) => Expression { lhs: Expression(4), op: Add, rhs: Expression(2) };
-+     coverage Code(Counter(0)) => 14:1 - 15:21;
-+     coverage Code(Counter(3)) => 16:17 - 16:33;
-+     coverage Code(Counter(2)) => 17:17 - 17:33;
-+     coverage Code(Counter(1)) => 18:17 - 18:33;
-+     coverage Code(Expression(2)) => 19:17 - 19:33;
-+     coverage Code(Expression(5)) => 21:1 - 21:2;
++     coverage Code(Counter(0)) => $DIR/branch_match_arms.rs:14:1: 15:21 (#0);
++     coverage Code(Counter(3)) => $DIR/branch_match_arms.rs:16:17: 16:33 (#0);
++     coverage Code(Counter(2)) => $DIR/branch_match_arms.rs:17:17: 17:33 (#0);
++     coverage Code(Counter(1)) => $DIR/branch_match_arms.rs:18:17: 18:33 (#0);
++     coverage Code(Expression(2)) => $DIR/branch_match_arms.rs:19:17: 19:33 (#0);
++     coverage Code(Expression(5)) => $DIR/branch_match_arms.rs:21:2: 21:2 (#0);
 + 
       bb0: {
 +         Coverage::CounterIncrement(0);
diff --git a/tests/mir-opt/coverage/instrument_coverage.bar.InstrumentCoverage.diff b/tests/mir-opt/coverage/instrument_coverage.bar.InstrumentCoverage.diff
index 2efb1fd0a17..148ff86354b 100644
--- a/tests/mir-opt/coverage/instrument_coverage.bar.InstrumentCoverage.diff
+++ b/tests/mir-opt/coverage/instrument_coverage.bar.InstrumentCoverage.diff
@@ -5,7 +5,7 @@
       let mut _0: bool;
   
 +     coverage body span: $DIR/instrument_coverage.rs:19:18: 21:2 (#0)
-+     coverage Code(Counter(0)) => 19:1 - 21:2;
++     coverage Code(Counter(0)) => $DIR/instrument_coverage.rs:19:1: 21:2 (#0);
 + 
       bb0: {
 +         Coverage::CounterIncrement(0);
diff --git a/tests/mir-opt/coverage/instrument_coverage.main.InstrumentCoverage.diff b/tests/mir-opt/coverage/instrument_coverage.main.InstrumentCoverage.diff
index a179824d6c7..b480d1ac13a 100644
--- a/tests/mir-opt/coverage/instrument_coverage.main.InstrumentCoverage.diff
+++ b/tests/mir-opt/coverage/instrument_coverage.main.InstrumentCoverage.diff
@@ -9,11 +9,11 @@
   
 +     coverage body span: $DIR/instrument_coverage.rs:10:11: 16:2 (#0)
 +     coverage ExpressionId(0) => Expression { lhs: Counter(0), op: Add, rhs: Counter(1) };
-+     coverage Code(Counter(0)) => 10:1 - 10:11;
-+     coverage Code(Expression(0)) => 12:12 - 12:17;
-+     coverage Code(Counter(0)) => 13:13 - 13:18;
-+     coverage Code(Counter(1)) => 14:9 - 14:10;
-+     coverage Code(Counter(0)) => 16:1 - 16:2;
++     coverage Code(Counter(0)) => $DIR/instrument_coverage.rs:10:1: 10:11 (#0);
++     coverage Code(Expression(0)) => $DIR/instrument_coverage.rs:12:12: 12:17 (#0);
++     coverage Code(Counter(0)) => $DIR/instrument_coverage.rs:13:13: 13:18 (#0);
++     coverage Code(Counter(1)) => $DIR/instrument_coverage.rs:14:10: 14:10 (#0);
++     coverage Code(Counter(0)) => $DIR/instrument_coverage.rs:16:2: 16:2 (#0);
 + 
       bb0: {
 +         Coverage::CounterIncrement(0);
diff --git a/tests/mir-opt/coverage/instrument_coverage_cleanup.main.CleanupPostBorrowck.diff b/tests/mir-opt/coverage/instrument_coverage_cleanup.main.CleanupPostBorrowck.diff
index 082539369f7..2c7ec6e85eb 100644
--- a/tests/mir-opt/coverage/instrument_coverage_cleanup.main.CleanupPostBorrowck.diff
+++ b/tests/mir-opt/coverage/instrument_coverage_cleanup.main.CleanupPostBorrowck.diff
@@ -9,11 +9,11 @@
   
       coverage body span: $DIR/instrument_coverage_cleanup.rs:13:11: 15:2 (#0)
       coverage ExpressionId(0) => Expression { lhs: Counter(0), op: Subtract, rhs: Counter(1) };
-      coverage Code(Counter(0)) => 13:1 - 14:36;
-      coverage Code(Expression(0)) => 14:37 - 14:39;
-      coverage Code(Counter(1)) => 14:38 - 14:39;
-      coverage Code(Counter(0)) => 15:1 - 15:2;
-      coverage Branch { true_term: Expression(0), false_term: Counter(1) } => 14:8 - 14:36;
+      coverage Code(Counter(0)) => $DIR/instrument_coverage_cleanup.rs:13:1: 14:36 (#0);
+      coverage Code(Expression(0)) => $DIR/instrument_coverage_cleanup.rs:14:37: 14:39 (#0);
+      coverage Code(Counter(1)) => $DIR/instrument_coverage_cleanup.rs:14:39: 14:39 (#0);
+      coverage Code(Counter(0)) => $DIR/instrument_coverage_cleanup.rs:15:2: 15:2 (#0);
+      coverage Branch { true_term: Expression(0), false_term: Counter(1) } => $DIR/instrument_coverage_cleanup.rs:14:8: 14:36 (#0);
   
       bb0: {
           Coverage::CounterIncrement(0);
diff --git a/tests/mir-opt/coverage/instrument_coverage_cleanup.main.InstrumentCoverage.diff b/tests/mir-opt/coverage/instrument_coverage_cleanup.main.InstrumentCoverage.diff
index 8635818c6a7..c08265eb0e9 100644
--- a/tests/mir-opt/coverage/instrument_coverage_cleanup.main.InstrumentCoverage.diff
+++ b/tests/mir-opt/coverage/instrument_coverage_cleanup.main.InstrumentCoverage.diff
@@ -9,11 +9,11 @@
   
 +     coverage body span: $DIR/instrument_coverage_cleanup.rs:13:11: 15:2 (#0)
 +     coverage ExpressionId(0) => Expression { lhs: Counter(0), op: Subtract, rhs: Counter(1) };
-+     coverage Code(Counter(0)) => 13:1 - 14:36;
-+     coverage Code(Expression(0)) => 14:37 - 14:39;
-+     coverage Code(Counter(1)) => 14:38 - 14:39;
-+     coverage Code(Counter(0)) => 15:1 - 15:2;
-+     coverage Branch { true_term: Expression(0), false_term: Counter(1) } => 14:8 - 14:36;
++     coverage Code(Counter(0)) => $DIR/instrument_coverage_cleanup.rs:13:1: 14:36 (#0);
++     coverage Code(Expression(0)) => $DIR/instrument_coverage_cleanup.rs:14:37: 14:39 (#0);
++     coverage Code(Counter(1)) => $DIR/instrument_coverage_cleanup.rs:14:39: 14:39 (#0);
++     coverage Code(Counter(0)) => $DIR/instrument_coverage_cleanup.rs:15:2: 15:2 (#0);
++     coverage Branch { true_term: Expression(0), false_term: Counter(1) } => $DIR/instrument_coverage_cleanup.rs:14:8: 14:36 (#0);
 + 
       bb0: {
 +         Coverage::CounterIncrement(0);