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

compiler/rustc_codegen_llvm/src/coverageinfo/ffi.rs

@@ -138,7 +138,7 @@ pub(crate) struct CoverageSpan {
 
 impl CoverageSpan {
     pub(crate) fn from_source_region(file_id: u32, code_region: &SourceRegion) -> Self {
-        let &SourceRegion { file_name: _, start_line, start_col, end_line, end_col } = code_region;
+        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}");

compiler/rustc_codegen_llvm/src/coverageinfo/map_data.rs

@@ -6,7 +6,6 @@ use rustc_middle::mir::coverage::{
     SourceRegion,
 };
 use rustc_middle::ty::Instance;
-use rustc_span::Symbol;
 use tracing::{debug, instrument};
 
 use crate::coverageinfo::ffi::{Counter, CounterExpression, ExprKind};
@@ -180,7 +179,7 @@ impl<'tcx> FunctionCoverageCollector<'tcx> {
 }
 
 pub(crate) struct FunctionCoverage<'tcx> {
-    function_coverage_info: &'tcx FunctionCoverageInfo,
+    pub(crate) function_coverage_info: &'tcx FunctionCoverageInfo,
     is_used: bool,
 
     counters_seen: BitSet<CounterId>,
@@ -199,11 +198,6 @@ impl<'tcx> FunctionCoverage<'tcx> {
         if self.is_used { self.function_coverage_info.function_source_hash } else { 0 }
     }
 
-    /// Returns an iterator over all filenames used by this function's mappings.
-    pub(crate) fn all_file_names(&self) -> impl Iterator<Item = Symbol> + Captures<'_> {
-        self.function_coverage_info.mappings.iter().map(|mapping| mapping.source_region.file_name)
-    }
-
     /// Convert this function's coverage expression data into a form that can be
     /// passed through FFI to LLVM.
     pub(crate) fn counter_expressions(

compiler/rustc_codegen_llvm/src/coverageinfo/mapgen.rs

@@ -11,8 +11,10 @@ use rustc_index::IndexVec;
 use rustc_middle::mir::coverage::MappingKind;
 use rustc_middle::ty::{self, TyCtxt};
 use rustc_middle::{bug, mir};
-use rustc_span::Symbol;
+use rustc_session::RemapFileNameExt;
+use rustc_session::config::RemapPathScopeComponents;
 use rustc_span::def_id::DefIdSet;
+use rustc_span::{Span, Symbol};
 use rustc_target::spec::HasTargetSpec;
 use tracing::debug;
 
@@ -69,8 +71,10 @@ pub(crate) fn finalize(cx: &CodegenCx<'_, '_>) {
         .map(|(instance, function_coverage)| (instance, function_coverage.into_finished()))
         .collect::<Vec<_>>();
 
-    let all_file_names =
-        function_coverage_entries.iter().flat_map(|(_, fn_cov)| fn_cov.all_file_names());
+    let all_file_names = function_coverage_entries
+        .iter()
+        .map(|(_, fn_cov)| fn_cov.function_coverage_info.body_span)
+        .map(|span| span_file_name(tcx, span));
     let global_file_table = GlobalFileTable::new(all_file_names);
 
     // Encode all filenames referenced by coverage mappings in this CGU.
@@ -95,7 +99,7 @@ pub(crate) fn finalize(cx: &CodegenCx<'_, '_>) {
         let is_used = function_coverage.is_used();
 
         let coverage_mapping_buffer =
-            encode_mappings_for_function(&global_file_table, &function_coverage);
+            encode_mappings_for_function(tcx, &global_file_table, &function_coverage);
 
         if coverage_mapping_buffer.is_empty() {
             if function_coverage.is_used() {
@@ -232,12 +236,20 @@ impl VirtualFileMapping {
     }
 }
 
+fn span_file_name(tcx: TyCtxt<'_>, span: Span) -> Symbol {
+    let source_file = tcx.sess.source_map().lookup_source_file(span.lo());
+    let name =
+        source_file.name.for_scope(tcx.sess, RemapPathScopeComponents::MACRO).to_string_lossy();
+    Symbol::intern(&name)
+}
+
 /// Using the expressions and counter regions collected for a single function,
 /// generate the variable-sized payload of its corresponding `__llvm_covfun`
 /// entry. The payload is returned as a vector of bytes.
 ///
 /// Newly-encountered filenames will be added to the global file table.
 fn encode_mappings_for_function(
+    tcx: TyCtxt<'_>,
     global_file_table: &GlobalFileTable,
     function_coverage: &FunctionCoverage<'_>,
 ) -> Vec<u8> {
@@ -254,53 +266,45 @@ fn encode_mappings_for_function(
     let mut mcdc_branch_regions = vec![];
     let mut mcdc_decision_regions = vec![];
 
-    // Group mappings into runs with the same filename, preserving the order
-    // yielded by `FunctionCoverage`.
-    // Prepare file IDs for each filename, and prepare the mapping data so that
-    // we can pass it through FFI to LLVM.
-    for (file_name, counter_regions_for_file) in
-        &counter_regions.group_by(|(_, region)| region.file_name)
-    {
-        // Look up the global file ID for this filename.
-        let global_file_id = global_file_table.global_file_id_for_file_name(file_name);
-
-        // Associate that global file ID with a local file ID for this 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
-        // form suitable for FFI.
-        for (mapping_kind, region) in counter_regions_for_file {
-            debug!("Adding counter {mapping_kind:?} to map for {region:?}");
-            let span = ffi::CoverageSpan::from_source_region(local_file_id.as_u32(), region);
-            match mapping_kind {
-                MappingKind::Code(term) => {
-                    code_regions
-                        .push(ffi::CodeRegion { span, counter: ffi::Counter::from_term(term) });
-                }
-                MappingKind::Branch { true_term, false_term } => {
-                    branch_regions.push(ffi::BranchRegion {
-                        span,
-                        true_counter: ffi::Counter::from_term(true_term),
-                        false_counter: ffi::Counter::from_term(false_term),
-                    });
-                }
-                MappingKind::MCDCBranch { true_term, false_term, mcdc_params } => {
-                    mcdc_branch_regions.push(ffi::MCDCBranchRegion {
-                        span,
-                        true_counter: ffi::Counter::from_term(true_term),
-                        false_counter: ffi::Counter::from_term(false_term),
-                        mcdc_branch_params: ffi::mcdc::BranchParameters::from(mcdc_params),
-                    });
-                }
-                MappingKind::MCDCDecision(mcdc_decision_params) => {
-                    mcdc_decision_regions.push(ffi::MCDCDecisionRegion {
-                        span,
-                        mcdc_decision_params: ffi::mcdc::DecisionParameters::from(
-                            mcdc_decision_params,
-                        ),
-                    });
-                }
+    // 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);
+
+    // Look up the global file ID for that filename.
+    let global_file_id = global_file_table.global_file_id_for_file_name(file_name);
+
+    // Associate that global file ID with a local file ID for this 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
+    // form suitable for FFI.
+    for (mapping_kind, region) in counter_regions {
+        debug!("Adding counter {mapping_kind:?} to map for {region:?}");
+        let span = ffi::CoverageSpan::from_source_region(local_file_id.as_u32(), region);
+        match mapping_kind {
+            MappingKind::Code(term) => {
+                code_regions.push(ffi::CodeRegion { span, counter: ffi::Counter::from_term(term) });
+            }
+            MappingKind::Branch { true_term, false_term } => {
+                branch_regions.push(ffi::BranchRegion {
+                    span,
+                    true_counter: ffi::Counter::from_term(true_term),
+                    false_counter: ffi::Counter::from_term(false_term),
+                });
+            }
+            MappingKind::MCDCBranch { true_term, false_term, mcdc_params } => {
+                mcdc_branch_regions.push(ffi::MCDCBranchRegion {
+                    span,
+                    true_counter: ffi::Counter::from_term(true_term),
+                    false_counter: ffi::Counter::from_term(false_term),
+                    mcdc_branch_params: ffi::mcdc::BranchParameters::from(mcdc_params),
+                });
+            }
+            MappingKind::MCDCDecision(mcdc_decision_params) => {
+                mcdc_decision_regions.push(ffi::MCDCDecisionRegion {
+                    span,
+                    mcdc_decision_params: ffi::mcdc::DecisionParameters::from(mcdc_decision_params),
+                });
             }
         }
     }

compiler/rustc_middle/src/mir/coverage.rs

@@ -4,7 +4,7 @@ use std::fmt::{self, Debug, Formatter};
 
 use rustc_index::IndexVec;
 use rustc_macros::{HashStable, TyDecodable, TyEncodable, TypeFoldable, TypeVisitable};
-use rustc_span::{Span, Symbol};
+use rustc_span::Span;
 
 rustc_index::newtype_index! {
     /// Used by [`CoverageKind::BlockMarker`] to mark blocks during THIR-to-MIR
@@ -158,7 +158,6 @@ impl Debug for CoverageKind {
 #[derive(Clone, TyEncodable, TyDecodable, Hash, HashStable, PartialEq, Eq, PartialOrd, Ord)]
 #[derive(TypeFoldable, TypeVisitable)]
 pub struct SourceRegion {
-    pub file_name: Symbol,
     pub start_line: u32,
     pub start_col: u32,
     pub end_line: u32,
@@ -167,11 +166,8 @@ pub struct SourceRegion {
 
 impl Debug for SourceRegion {
     fn fmt(&self, fmt: &mut Formatter<'_>) -> fmt::Result {
-        write!(
-            fmt,
-            "{}:{}:{} - {}:{}",
-            self.file_name, self.start_line, self.start_col, self.end_line, self.end_col
-        )
+        let &Self { start_line, start_col, end_line, end_col } = self;
+        write!(fmt, "{start_line}:{start_col} - {end_line}:{end_col}")
     }
 }
 
@@ -246,6 +242,7 @@ pub struct Mapping {
 #[derive(TyEncodable, TyDecodable, Hash, HashStable, TypeFoldable, TypeVisitable)]
 pub struct FunctionCoverageInfo {
     pub function_source_hash: u64,
+    pub body_span: Span,
     pub num_counters: usize,
     pub mcdc_bitmap_bits: usize,
     pub expressions: IndexVec<ExpressionId, Expression>,

compiler/rustc_middle/src/mir/pretty.rs

@@ -596,8 +596,10 @@ fn write_function_coverage_info(
     function_coverage_info: &coverage::FunctionCoverageInfo,
     w: &mut dyn io::Write,
 ) -> io::Result<()> {
-    let coverage::FunctionCoverageInfo { expressions, mappings, .. } = function_coverage_info;
+    let coverage::FunctionCoverageInfo { body_span, expressions, mappings, .. } =
+        function_coverage_info;
 
+    writeln!(w, "{INDENT}coverage body span: {body_span:?}")?;
     for (id, expression) in expressions.iter_enumerated() {
         writeln!(w, "{INDENT}coverage {id:?} => {expression:?};")?;
     }

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, Span, Symbol};
+use rustc_span::{BytePos, Pos, RelativeBytePos, SourceFile, Span};
 use tracing::{debug, debug_span, trace};
 
 use crate::coverage::counters::{CounterIncrementSite, CoverageCounters};
@@ -122,6 +122,7 @@ fn instrument_function_for_coverage<'tcx>(tcx: TyCtxt<'tcx>, mir_body: &mut mir:
 
     mir_body.function_coverage_info = Some(Box::new(FunctionCoverageInfo {
         function_source_hash: hir_info.function_source_hash,
+        body_span: hir_info.body_span,
         num_counters: coverage_counters.num_counters(),
         mcdc_bitmap_bits: extracted_mappings.mcdc_bitmap_bits,
         expressions: coverage_counters.into_expressions(),
@@ -142,19 +143,11 @@ fn create_mappings<'tcx>(
     coverage_counters: &CoverageCounters,
 ) -> Vec<Mapping> {
     let source_map = tcx.sess.source_map();
-    let body_span = hir_info.body_span;
-
-    let source_file = source_map.lookup_source_file(body_span.lo());
-
-    use rustc_session::RemapFileNameExt;
-    use rustc_session::config::RemapPathScopeComponents;
-    let file_name = Symbol::intern(
-        &source_file.name.for_scope(tcx.sess, RemapPathScopeComponents::MACRO).to_string_lossy(),
-    );
+    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_name, span);
+    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 {
@@ -398,7 +391,7 @@ fn inject_statement(mir_body: &mut mir::Body<'_>, counter_kind: CoverageKind, bb
     data.statements.insert(0, statement);
 }
 
-/// Convert the Span into its file name, start line and column, and end line and column.
+/// 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
@@ -411,18 +404,12 @@ fn inject_statement(mir_body: &mut mir::Body<'_>, counter_kind: CoverageKind, bb
 fn make_source_region(
     source_map: &SourceMap,
     hir_info: &ExtractedHirInfo,
-    file_name: Symbol,
+    file: &SourceFile,
     span: Span,
 ) -> Option<SourceRegion> {
     let lo = span.lo();
     let hi = span.hi();
 
-    let file = source_map.lookup_source_file(lo);
-    if !file.contains(hi) {
-        debug!(?span, ?file, ?lo, ?hi, "span crosses multiple files; skipping");
-        return None;
-    }
-
     // 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)> {
@@ -465,7 +452,6 @@ fn make_source_region(
     end_line = source_map.doctest_offset_line(&file.name, end_line);
 
     check_source_region(SourceRegion {
-        file_name,
         start_line: start_line as u32,
         start_col: start_col as u32,
         end_line: end_line as u32,
@@ -478,7 +464,7 @@ fn make_source_region(
 /// 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 { file_name: _, start_line, start_col, end_line, end_col } = source_region;
+    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.

tests/mir-opt/coverage/branch_match_arms.main.InstrumentCoverage.diff

@@ -26,18 +26,19 @@
           debug a => _9;
       }
 
++     coverage body span: $DIR/branch_match_arms.rs:14:11: 21:2 (#0)
 +     coverage ExpressionId(0) => Expression { lhs: Counter(1), op: Add, rhs: Counter(2) };
 +     coverage ExpressionId(1) => Expression { lhs: Expression(0), op: Add, rhs: Counter(3) };
 +     coverage ExpressionId(2) => Expression { lhs: Counter(0), op: Subtract, rhs: Expression(1) };
 +     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)) => $DIR/branch_match_arms.rs:14:1 - 15:21;
-+     coverage Code(Counter(3)) => $DIR/branch_match_arms.rs:16:17 - 16:33;
-+     coverage Code(Counter(2)) => $DIR/branch_match_arms.rs:17:17 - 17:33;
-+     coverage Code(Counter(1)) => $DIR/branch_match_arms.rs:18:17 - 18:33;
-+     coverage Code(Expression(2)) => $DIR/branch_match_arms.rs:19:17 - 19:33;
-+     coverage Code(Expression(5)) => $DIR/branch_match_arms.rs:21:1 - 21: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;
 + 
       bb0: {
 +         Coverage::CounterIncrement(0);

tests/mir-opt/coverage/instrument_coverage.bar.InstrumentCoverage.diff

@@ -4,7 +4,8 @@
   fn bar() -> bool {
       let mut _0: bool;
 
-+     coverage Code(Counter(0)) => $DIR/instrument_coverage.rs:19:1 - 21:2;
++     coverage body span: $DIR/instrument_coverage.rs:19:18: 21:2 (#0)
++     coverage Code(Counter(0)) => 19:1 - 21:2;
 + 
       bb0: {
 +         Coverage::CounterIncrement(0);

tests/mir-opt/coverage/instrument_coverage.main.InstrumentCoverage.diff

@@ -7,12 +7,13 @@
       let mut _2: bool;
       let mut _3: !;
 
++     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)) => $DIR/instrument_coverage.rs:10:1 - 10:11;
-+     coverage Code(Expression(0)) => $DIR/instrument_coverage.rs:12:12 - 12:17;
-+     coverage Code(Counter(0)) => $DIR/instrument_coverage.rs:13:13 - 13:18;
-+     coverage Code(Counter(1)) => $DIR/instrument_coverage.rs:14:10 - 14:11;
-+     coverage Code(Counter(0)) => $DIR/instrument_coverage.rs:16:1 - 16:2;
++     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:10 - 14:11;
++     coverage Code(Counter(0)) => 16:1 - 16:2;
 + 
       bb0: {
 +         Coverage::CounterIncrement(0);

tests/mir-opt/coverage/instrument_coverage_cleanup.main.CleanupPostBorrowck.diff

@@ -7,12 +7,13 @@
 
       coverage branch { true: BlockMarkerId(0), false: BlockMarkerId(1) } => $DIR/instrument_coverage_cleanup.rs:14:8: 14:36 (#0)
 
+      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)) => $DIR/instrument_coverage_cleanup.rs:13:1 - 14:36;
-      coverage Code(Expression(0)) => $DIR/instrument_coverage_cleanup.rs:14:37 - 14:39;
-      coverage Code(Counter(1)) => $DIR/instrument_coverage_cleanup.rs:14:39 - 14:40;
-      coverage Code(Counter(0)) => $DIR/instrument_coverage_cleanup.rs:15:1 - 15:2;
-      coverage Branch { true_term: Expression(0), false_term: Counter(1) } => $DIR/instrument_coverage_cleanup.rs:14:8 - 14:36;
+      coverage Code(Counter(0)) => 13:1 - 14:36;
+      coverage Code(Expression(0)) => 14:37 - 14:39;
+      coverage Code(Counter(1)) => 14:39 - 14:40;
+      coverage Code(Counter(0)) => 15:1 - 15:2;
+      coverage Branch { true_term: Expression(0), false_term: Counter(1) } => 14:8 - 14:36;
 
       bb0: {
           Coverage::CounterIncrement(0);