!! (WIP) Only CoverageKind::Mappings can have code regions

Author: Zalathar

compiler/rustc_codegen_llvm/src/coverageinfo/map_data.rs

@@ -71,28 +71,20 @@ impl<'tcx> FunctionCoverage<'tcx> {
         }
     }
 
-    /// Adds code regions to be counted by an injected counter intrinsic.
+    /// Notes that a counter increment has been encountered with the given ID.
     #[instrument(level = "debug", skip(self))]
-    pub(crate) fn add_counter(&mut self, id: CounterId, code_regions: &[CodeRegion]) {
-        if self.counters_seen.insert(id) {
-            self.add_mappings(CovTerm::Counter(id), code_regions);
-        }
+    pub(crate) fn counter_seen(&mut self, id: CounterId) {
+        self.counters_seen.insert(id);
     }
 
-    /// Adds information about a coverage expression, along with zero or more
-    /// code regions mapped to that expression.
-    ///
-    /// Both counters and "counter expressions" (or simply, "expressions") can be operands in other
-    /// expressions. These are tracked as separate variants of `CovTerm`, so there is no ambiguity
-    /// between operands that are counter IDs and operands that are expression IDs.
+    /// Adds information about a coverage expression.
     #[instrument(level = "debug", skip(self))]
-    pub(crate) fn add_counter_expression(
+    pub(crate) fn add_expression_data(
         &mut self,
         expression_id: ExpressionId,
         lhs: CovTerm,
         op: Op,
         rhs: CovTerm,
-        code_regions: &[CodeRegion],
     ) {
         debug_assert!(
             expression_id.as_usize() < self.expressions.len(),
@@ -106,10 +98,7 @@ impl<'tcx> FunctionCoverage<'tcx> {
         let expression = Expression { lhs, op, rhs };
         let slot = &mut self.expressions[expression_id];
         match slot {
-            None => {
-                *slot = Some(expression);
-                self.add_mappings(CovTerm::Expression(expression_id), code_regions);
-            }
+            None => *slot = Some(expression),
             // If this expression ID slot has already been filled, it should
             // contain identical information.
             Some(ref previous_expression) => assert_eq!(

compiler/rustc_codegen_llvm/src/coverageinfo/mod.rs

@@ -110,15 +110,15 @@ impl<'tcx> CoverageInfoBuilderMethods<'tcx> for Builder<'_, '_, 'tcx> {
             .entry(instance)
             .or_insert_with(|| FunctionCoverage::new(bx.tcx(), instance));
 
-        let Coverage { kind, code_regions } = coverage;
+        let Coverage { kind } = coverage;
         match *kind {
             CoverageKind::Counter { function_source_hash, id } => {
                 debug!(
                     "ensuring function source hash is set for instance={:?}; function_source_hash={}",
                     instance, function_source_hash,
                 );
                 func_coverage.set_function_source_hash(function_source_hash);
-                func_coverage.add_counter(id, code_regions);
+                func_coverage.counter_seen(id);
                 // We need to explicitly drop the `RefMut` before calling into `instrprof_increment`,
                 // as that needs an exclusive borrow.
                 drop(coverage_map);
@@ -136,12 +136,9 @@ impl<'tcx> CoverageInfoBuilderMethods<'tcx> for Builder<'_, '_, 'tcx> {
                 bx.instrprof_increment(fn_name, hash, num_counters, index);
             }
             CoverageKind::Expression { id, lhs, op, rhs } => {
-                func_coverage.add_counter_expression(id, lhs, op, rhs, code_regions);
+                func_coverage.add_expression_data(id, lhs, op, rhs);
             }
-            CoverageKind::Unreachable => {
-                func_coverage.add_unreachable_regions(code_regions);
-            }
-            CoverageKind::Mappings { term } => {
+            CoverageKind::Mappings { term, ref code_regions } => {
                 func_coverage.add_mappings(term, code_regions);
             }
         }

compiler/rustc_middle/src/mir/coverage.rs

@@ -76,9 +76,9 @@ pub enum CoverageKind {
         op: Op,
         rhs: CovTerm,
     },
-    Unreachable,
     Mappings {
         term: CovTerm,
+        code_regions: Vec<CodeRegion>,
     },
 }
 
@@ -98,8 +98,11 @@ impl Debug for CoverageKind {
                 },
                 rhs,
             ),
-            Unreachable => write!(fmt, "Unreachable"),
-            Mappings { term } => fmt.debug_struct("Mappings").field("term", term).finish(),
+            Mappings { term, code_regions } => fmt
+                .debug_struct("Mappings")
+                .field("term", term)
+                .field("code_regions", code_regions)
+                .finish(),
         }
     }
 }

compiler/rustc_middle/src/mir/mod.rs

@@ -1473,12 +1473,8 @@ impl Debug for Statement<'_> {
             AscribeUserType(box (ref place, ref c_ty), ref variance) => {
                 write!(fmt, "AscribeUserType({place:?}, {variance:?}, {c_ty:?})")
             }
-            Coverage(box self::Coverage { ref kind, ref code_regions }) => {
-                if code_regions.is_empty() {
-                    write!(fmt, "Coverage::{kind:?}")
-                } else {
-                    write!(fmt, "Coverage::{kind:?} for {code_regions:?}")
-                }
+            Coverage(box self::Coverage { ref kind }) => {
+                write!(fmt, "Coverage::{kind:?}")
             }
             Intrinsic(box ref intrinsic) => write!(fmt, "{intrinsic}"),
             ConstEvalCounter => write!(fmt, "ConstEvalCounter"),

compiler/rustc_middle/src/mir/syntax.rs

@@ -5,7 +5,7 @@
 
 use super::{BasicBlock, Constant, Local, SwitchTargets, UserTypeProjection};
 
-use crate::mir::coverage::{CodeRegion, CoverageKind};
+use crate::mir::coverage::CoverageKind;
 use crate::traits::Reveal;
 use crate::ty::adjustment::PointerCoercion;
 use crate::ty::GenericArgsRef;
@@ -523,7 +523,6 @@ pub enum FakeReadCause {
 #[derive(TypeFoldable, TypeVisitable)]
 pub struct Coverage {
     pub kind: CoverageKind,
-    pub code_regions: Vec<CodeRegion>,
 }
 
 #[derive(Clone, Debug, PartialEq, TyEncodable, TyDecodable, Hash, HashStable)]

compiler/rustc_mir_transform/src/coverage/mod.rs

@@ -316,15 +316,13 @@ impl<'a, 'tcx> Instrumentor<'a, 'tcx> {
 
             inject_statement(
                 self.mir_body,
-                CoverageKind::Mappings { term: counter_kind.as_term() },
+                CoverageKind::Mappings { term: counter_kind.as_term(), code_regions },
                 mir::START_BLOCK,
-                code_regions,
             );
             inject_statement(
                 self.mir_body,
                 self.make_mir_coverage_kind(&counter_kind),
                 self.bcb_leader_bb(bcb),
-                Vec::new(),
             );
         }
     }
@@ -407,7 +405,6 @@ impl<'a, 'tcx> Instrumentor<'a, 'tcx> {
                         self.mir_body,
                         self.make_mir_coverage_kind(&counter_kind),
                         inject_to_bb,
-                        Vec::new(),
                     );
                 }
                 BcbCounter::Expression { .. } => inject_intermediate_expression(
@@ -473,18 +470,13 @@ fn inject_edge_counter_basic_block(
     new_bb
 }
 
-fn inject_statement(
-    mir_body: &mut mir::Body<'_>,
-    counter_kind: CoverageKind,
-    bb: BasicBlock,
-    code_regions: Vec<CodeRegion>,
-) {
-    debug!("  injecting statement {counter_kind:?} for {bb:?} at code regions: {code_regions:?}");
+fn inject_statement(mir_body: &mut mir::Body<'_>, counter_kind: CoverageKind, bb: BasicBlock) {
+    debug!("  injecting statement {counter_kind:?} for {bb:?}");
     let data = &mut mir_body[bb];
     let source_info = data.terminator().source_info;
     let statement = Statement {
         source_info,
-        kind: StatementKind::Coverage(Box::new(Coverage { kind: counter_kind, code_regions })),
+        kind: StatementKind::Coverage(Box::new(Coverage { kind: counter_kind })),
     };
     data.statements.insert(0, statement);
 }
@@ -498,10 +490,7 @@ fn inject_intermediate_expression(mir_body: &mut mir::Body<'_>, expression: Cove
     let source_info = data.terminator().source_info;
     let statement = Statement {
         source_info,
-        kind: StatementKind::Coverage(Box::new(Coverage {
-            kind: expression,
-            code_regions: Vec::new(),
-        })),
+        kind: StatementKind::Coverage(Box::new(Coverage { kind: expression })),
     };
     data.statements.push(statement);
 }

compiler/rustc_mir_transform/src/coverage/query.rs

@@ -68,8 +68,7 @@ impl CoverageVisitor {
                 self.update_from_term(lhs);
                 self.update_from_term(rhs);
             }
-            CoverageKind::Unreachable => {}
-            CoverageKind::Mappings { term } => self.update_from_term(term),
+            CoverageKind::Mappings { term, .. } => self.update_from_term(term),
         }
     }
 }
@@ -94,8 +93,11 @@ fn coverageinfo<'tcx>(tcx: TyCtxt<'tcx>, instance_def: ty::InstanceDef<'tcx>) ->
 fn covered_code_regions(tcx: TyCtxt<'_>, def_id: DefId) -> Vec<&CodeRegion> {
     let body = mir_body(tcx, def_id);
     all_coverage_in_mir_body(body)
-        // Coverage statements have a list of code regions (possibly empty).
-        .flat_map(|coverage| coverage.code_regions.as_slice())
+        // Only `Mappings` statements have a list of code regions.
+        .flat_map(|coverage| match coverage.kind {
+            CoverageKind::Mappings { ref code_regions, .. } => code_regions.as_slice(),
+            _ => &[],
+        })
         .collect()
 }
 

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

@@ -6,7 +6,7 @@
 
       bb0: {
 +         Coverage::Counter(0);
-+         Coverage::Mappings { term: Counter(0) } for [/the/src/instrument_coverage.rs:20:1 - 22:2];
++         Coverage::Mappings { term: Counter(0), code_regions: [/the/src/instrument_coverage.rs:20:1 - 22:2] };
           _0 = const true;
           return;
       }

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

@@ -8,11 +8,11 @@
       let mut _3: !;
 
       bb0: {
-+         Coverage::Mappings { term: Counter(1) } for [/the/src/instrument_coverage.rs:15:10 - 15:11];
-+         Coverage::Mappings { term: Expression(1) } for [/the/src/instrument_coverage.rs:14:13 - 14:18, /the/src/instrument_coverage.rs:17:1 - 17:2];
-+         Coverage::Mappings { term: Expression(0) } for [/the/src/instrument_coverage.rs:12:5 - 13:17];
++         Coverage::Mappings { term: Counter(1), code_regions: [/the/src/instrument_coverage.rs:15:10 - 15:11] };
++         Coverage::Mappings { term: Expression(1), code_regions: [/the/src/instrument_coverage.rs:14:13 - 14:18, /the/src/instrument_coverage.rs:17:1 - 17:2] };
++         Coverage::Mappings { term: Expression(0), code_regions: [/the/src/instrument_coverage.rs:12:5 - 13:17] };
 +         Coverage::Counter(0);
-+         Coverage::Mappings { term: Counter(0) } for [/the/src/instrument_coverage.rs:11:1 - 11:11];
++         Coverage::Mappings { term: Counter(0), code_regions: [/the/src/instrument_coverage.rs:11:1 - 11:11] };
           goto -> bb1;
       }