coverage: Treat each match arm as a "branch" for branch coverage

Author: Zalathar

compiler/rustc_mir_build/src/build/coverageinfo.rs

@@ -28,6 +28,12 @@ struct NotInfo {
     is_flipped: bool,
 }
 
+pub(crate) struct MatchArm {
+    pub(crate) source_info: SourceInfo,
+    pub(crate) has_guard: bool,
+    pub(crate) pre_binding_block: Option<BasicBlock>,
+}
+
 impl BranchInfoBuilder {
     /// Creates a new branch info builder, but only if branch coverage instrumentation
     /// is enabled and `def_id` represents a function that is eligible for coverage.
@@ -99,6 +105,29 @@ impl BranchInfoBuilder {
         ]);
     }
 
+    pub(crate) fn add_match_arms(&mut self, cfg: &mut CFG<'_>, arms: Vec<MatchArm>) {
+        // Match expressions with 0-1 arms don't have any branches for their arms.
+        if arms.len() < 2 {
+            return;
+        }
+
+        // FIXME(#124118) The current implementation of branch coverage for
+        // match arms can't handle or-patterns or guards.
+        if arms.iter().any(|arm| arm.has_guard || arm.pre_binding_block.is_none()) {
+            return;
+        }
+
+        let branch_arms = arms
+            .iter()
+            .map(|&MatchArm { source_info, pre_binding_block, .. }| {
+                let marker = self.inject_block_marker(cfg, source_info, pre_binding_block.unwrap());
+                BranchArm { span: source_info.span, marker }
+            })
+            .collect::<Vec<_>>();
+
+        self.branch_arm_lists.push(branch_arms);
+    }
+
     fn next_block_marker_id(&mut self) -> BlockMarkerId {
         let id = BlockMarkerId::from_usize(self.num_block_markers);
         self.num_block_markers += 1;

compiler/rustc_mir_build/src/build/matches/mod.rs

@@ -5,6 +5,7 @@
 //! This also includes code for pattern bindings in `let` statements and
 //! function parameters.
 
+use crate::build::coverageinfo;
 use crate::build::expr::as_place::PlaceBuilder;
 use crate::build::scope::DropKind;
 use crate::build::ForGuard::{self, OutsideGuard, RefWithinGuard};
@@ -315,6 +316,10 @@ impl<'a, 'tcx> Builder<'a, 'tcx> {
             &mut candidates,
         );
 
+        // Record branch coverage info for this match.
+        // (Does nothing if branch coverage is not enabled.)
+        self.visit_coverage_match_expr(&candidates);
+
         self.lower_match_arms(
             destination,
             scrutinee_place,
@@ -364,6 +369,26 @@ impl<'a, 'tcx> Builder<'a, 'tcx> {
             .collect()
     }
 
+    /// If branch coverage is enabled, inject marker statements into each match
+    /// arm, and record their IDs in the table of branches.
+    ///
+    /// Unlike some of the other branch coverage visitor methods, this one needs
+    /// access to private details of [`Candidate`], so having it here is easier.
+    fn visit_coverage_match_expr(&mut self, candidates: &[&mut Candidate<'_, 'tcx>]) {
+        // Bail out if branch coverage is not enabled for this function.
+        let Some(branch_info) = self.coverage_branch_info.as_mut() else { return };
+
+        let arms = candidates
+            .iter()
+            .map(|c| coverageinfo::MatchArm {
+                source_info: SourceInfo { span: c.extra_data.span, scope: self.source_scope },
+                has_guard: c.has_guard,
+                pre_binding_block: c.pre_binding_block,
+            })
+            .collect::<Vec<_>>();
+        branch_info.add_match_arms(&mut self.cfg, arms);
+    }
+
     /// Create the decision tree for the match expression, starting from `block`.
     ///
     /// Modifies `candidates` to store the bindings and type ascriptions for

tests/coverage/branch/match-arms.cov-map

@@ -36,25 +36,49 @@ Number of file 0 mappings: 12
     = ((((((((Zero + c2) + c3) + c4) + c5) + c6) + c7) + c8) + c9)
 
 Function name: match_arms::match_arms
-Raw bytes (51): 0x[01, 01, 06, 05, 07, 0b, 11, 09, 0d, 13, 02, 17, 09, 11, 0d, 07, 01, 18, 01, 01, 10, 05, 03, 0b, 00, 10, 11, 01, 11, 00, 21, 0d, 01, 11, 00, 21, 09, 01, 11, 00, 21, 02, 01, 11, 00, 21, 0f, 03, 05, 01, 02]
+Raw bytes (102): 0x[01, 01, 15, 0d, 17, 09, 4a, 05, 4f, 53, 11, 09, 0d, 09, 4a, 05, 4f, 53, 11, 09, 0d, 05, 4f, 53, 11, 09, 0d, 05, 4f, 53, 11, 09, 0d, 43, 4a, 47, 09, 11, 0d, 05, 4f, 53, 11, 09, 0d, 0a, 01, 18, 01, 01, 10, 05, 03, 0b, 00, 10, 20, 11, 03, 01, 09, 00, 13, 11, 00, 11, 00, 21, 20, 0d, 17, 01, 09, 00, 13, 0d, 00, 11, 00, 21, 20, 09, 4a, 01, 09, 00, 13, 09, 00, 11, 00, 21, 4a, 01, 11, 00, 21, 3f, 03, 05, 01, 02]
 Number of files: 1
 - file 0 => global file 1
-Number of expressions: 6
-- expression 0 operands: lhs = Counter(1), rhs = Expression(1, Add)
-- expression 1 operands: lhs = Expression(2, Add), rhs = Counter(4)
-- expression 2 operands: lhs = Counter(2), rhs = Counter(3)
-- expression 3 operands: lhs = Expression(4, Add), rhs = Expression(0, Sub)
-- expression 4 operands: lhs = Expression(5, Add), rhs = Counter(2)
-- expression 5 operands: lhs = Counter(4), rhs = Counter(3)
-Number of file 0 mappings: 7
+Number of expressions: 21
+- expression 0 operands: lhs = Counter(3), rhs = Expression(5, Add)
+- expression 1 operands: lhs = Counter(2), rhs = Expression(18, Sub)
+- expression 2 operands: lhs = Counter(1), rhs = Expression(19, Add)
+- expression 3 operands: lhs = Expression(20, Add), rhs = Counter(4)
+- expression 4 operands: lhs = Counter(2), rhs = Counter(3)
+- expression 5 operands: lhs = Counter(2), rhs = Expression(18, Sub)
+- expression 6 operands: lhs = Counter(1), rhs = Expression(19, Add)
+- expression 7 operands: lhs = Expression(20, Add), rhs = Counter(4)
+- expression 8 operands: lhs = Counter(2), rhs = Counter(3)
+- expression 9 operands: lhs = Counter(1), rhs = Expression(19, Add)
+- expression 10 operands: lhs = Expression(20, Add), rhs = Counter(4)
+- expression 11 operands: lhs = Counter(2), rhs = Counter(3)
+- expression 12 operands: lhs = Counter(1), rhs = Expression(19, Add)
+- expression 13 operands: lhs = Expression(20, Add), rhs = Counter(4)
+- expression 14 operands: lhs = Counter(2), rhs = Counter(3)
+- expression 15 operands: lhs = Expression(16, Add), rhs = Expression(18, Sub)
+- expression 16 operands: lhs = Expression(17, Add), rhs = Counter(2)
+- expression 17 operands: lhs = Counter(4), rhs = Counter(3)
+- expression 18 operands: lhs = Counter(1), rhs = Expression(19, Add)
+- expression 19 operands: lhs = Expression(20, Add), rhs = Counter(4)
+- expression 20 operands: lhs = Counter(2), rhs = Counter(3)
+Number of file 0 mappings: 10
 - Code(Counter(0)) at (prev + 24, 1) to (start + 1, 16)
 - Code(Counter(1)) at (prev + 3, 11) to (start + 0, 16)
-- Code(Counter(4)) at (prev + 1, 17) to (start + 0, 33)
-- Code(Counter(3)) at (prev + 1, 17) to (start + 0, 33)
-- Code(Counter(2)) at (prev + 1, 17) to (start + 0, 33)
-- Code(Expression(0, Sub)) at (prev + 1, 17) to (start + 0, 33)
+- Branch { true: Counter(4), false: Expression(0, Add) } at (prev + 1, 9) to (start + 0, 19)
+    true  = c4
+    false = (c3 + (c2 + (c1 - ((c2 + c3) + c4))))
+- Code(Counter(4)) at (prev + 0, 17) to (start + 0, 33)
+- Branch { true: Counter(3), false: Expression(5, Add) } at (prev + 1, 9) to (start + 0, 19)
+    true  = c3
+    false = (c2 + (c1 - ((c2 + c3) + c4)))
+- Code(Counter(3)) at (prev + 0, 17) to (start + 0, 33)
+- Branch { true: Counter(2), false: Expression(18, Sub) } at (prev + 1, 9) to (start + 0, 19)
+    true  = c2
+    false = (c1 - ((c2 + c3) + c4))
+- Code(Counter(2)) at (prev + 0, 17) to (start + 0, 33)
+- Code(Expression(18, Sub)) at (prev + 1, 17) to (start + 0, 33)
     = (c1 - ((c2 + c3) + c4))
-- Code(Expression(3, Add)) at (prev + 3, 5) to (start + 1, 2)
+- Code(Expression(15, Add)) at (prev + 3, 5) to (start + 1, 2)
     = (((c4 + c3) + c2) + (c1 - ((c2 + c3) + c4)))
 
 Function name: match_arms::or_patterns

tests/coverage/branch/match-arms.coverage

@@ -26,8 +26,17 @@
    LL|       |
    LL|     15|    match value {
    LL|      8|        Enum::D(d) => consume(d),
+  ------------------
+  |  Branch (LL:9): [True: 8, False: 7]
+  ------------------
    LL|      4|        Enum::C(c) => consume(c),
+  ------------------
+  |  Branch (LL:9): [True: 4, False: 3]
+  ------------------
    LL|      2|        Enum::B(b) => consume(b),
+  ------------------
+  |  Branch (LL:9): [True: 2, False: 1]
+  ------------------
    LL|      1|        Enum::A(a) => consume(a),
    LL|       |    }
    LL|       |
@@ -101,5 +110,5 @@
    LL|       |    }
    LL|       |}
    LL|       |
-   LL|       |// FIXME(#124118) Actually instrument match arms for branch coverage.
+   LL|       |// FIXME(#124118) Support match expressions with guards or or-patterns.
 

tests/coverage/branch/match-arms.rs

@@ -87,4 +87,4 @@ fn main() {
     }
 }
 
-// FIXME(#124118) Actually instrument match arms for branch coverage.
+// FIXME(#124118) Support match expressions with guards or or-patterns.

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

@@ -26,18 +26,31 @@
           debug a => _9;
       }
 
+      coverage branches {
+          BlockMarkerId(0) => $DIR/branch_match_arms.rs:17:9: 17:19 (#0)
+          BlockMarkerId(1) => $DIR/branch_match_arms.rs:18:9: 18:19 (#0)
+          BlockMarkerId(2) => $DIR/branch_match_arms.rs:19:9: 19:19 (#0)
+          BlockMarkerId(3) => $DIR/branch_match_arms.rs:20:9: 20:19 (#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 ExpressionId(6) => Expression { lhs: Counter(1), op: Add, rhs: Expression(2) };
++     coverage ExpressionId(7) => Expression { lhs: Counter(2), op: Add, rhs: Expression(6) };
++     coverage ExpressionId(8) => Expression { lhs: Counter(3), op: Add, rhs: Expression(7) };
 +     coverage Code(Counter(0)) => $DIR/branch_match_arms.rs:15:1 - 16:21;
 +     coverage Code(Counter(3)) => $DIR/branch_match_arms.rs:17:17 - 17:33;
 +     coverage Code(Counter(2)) => $DIR/branch_match_arms.rs:18:17 - 18:33;
 +     coverage Code(Counter(1)) => $DIR/branch_match_arms.rs:19:17 - 19:33;
 +     coverage Code(Expression(2)) => $DIR/branch_match_arms.rs:20:17 - 20:33;
 +     coverage Code(Expression(5)) => $DIR/branch_match_arms.rs:22:1 - 22:2;
++     coverage Branch { true_term: Counter(1), false_term: Expression(2) } => $DIR/branch_match_arms.rs:19:9 - 19:19;
++     coverage Branch { true_term: Counter(2), false_term: Expression(6) } => $DIR/branch_match_arms.rs:18:9 - 18:19;
++     coverage Branch { true_term: Counter(3), false_term: Expression(7) } => $DIR/branch_match_arms.rs:17:9 - 17:19;
 + 
       bb0: {
 +         Coverage::CounterIncrement(0);
@@ -55,21 +68,25 @@
 
       bb2: {
 +         Coverage::CounterIncrement(3);
+          Coverage::BlockMarker(0);
           falseEdge -> [real: bb6, imaginary: bb3];
       }
 
       bb3: {
 +         Coverage::CounterIncrement(2);
+          Coverage::BlockMarker(1);
           falseEdge -> [real: bb8, imaginary: bb4];
       }
 
       bb4: {
 +         Coverage::CounterIncrement(1);
+          Coverage::BlockMarker(2);
           falseEdge -> [real: bb10, imaginary: bb5];
       }
 
       bb5: {
 +         Coverage::ExpressionUsed(2);
+          Coverage::BlockMarker(3);
           StorageLive(_9);
           _9 = ((_1 as A).0: u32);
           StorageLive(_10);