coverage. Implement branch coverage in mcdc for pattern match

Author: zhuyunxing

Diff for: compiler/rustc_middle/src/mir/coverage.rs

@@ -281,6 +281,7 @@ pub struct BranchInfo {
     pub num_block_markers: usize,
     pub branch_spans: Vec<BranchSpan>,
     pub mcdc_branch_spans: Vec<MCDCBranchSpan>,
+    pub mcdc_match_branch_spans: Vec<MCDCMatchBranchSpan>,
     pub mcdc_decision_spans: Vec<MCDCDecisionSpan>,
 }
 
@@ -321,6 +322,16 @@ pub struct MCDCBranchSpan {
     pub false_marker: BlockMarkerId,
 }
 
+#[derive(Clone, Debug)]
+#[derive(TyEncodable, TyDecodable, Hash, HashStable, TypeFoldable, TypeVisitable)]
+pub struct MCDCMatchBranchSpan {
+    pub span: Span,
+    pub condition_info: Option<ConditionInfo>,
+    pub test_markers: Vec<BlockMarkerId>,
+    pub true_markers: Vec<BlockMarkerId>,
+    pub substract_markers: Vec<BlockMarkerId>,
+}
+
 #[derive(Copy, Clone, Debug)]
 #[derive(TyEncodable, TyDecodable, Hash, HashStable, TypeFoldable, TypeVisitable)]
 pub struct DecisionInfo {

Diff for: compiler/rustc_middle/src/mir/pretty.rs

@@ -485,12 +485,16 @@ fn write_coverage_branch_info(
         )?;
     }
 
-    for coverage::MCDCBranchSpan { span, condition_info, true_marker, false_marker } in
-        mcdc_branch_spans
+    for coverage::MCDCBranchSpan {
+        span,
+        condition_info,
+        true_marker: true_markers,
+        false_marker: false_markers,
+    } in mcdc_branch_spans
     {
         writeln!(
             w,
-            "{INDENT}coverage mcdc branch {{ condition_id: {:?}, true: {true_marker:?}, false: {false_marker:?} }} => {span:?}",
+            "{INDENT}coverage mcdc branch {{ condition_id: {:?}, true: {true_markers:?}, false: {false_markers:?} }} => {span:?}",
             condition_info.map(|info| info.condition_id)
         )?;
     }

Diff for: compiler/rustc_mir_build/src/build/coverageinfo.rs

@@ -1,13 +1,13 @@
 use std::assert_matches::assert_matches;
 use std::collections::hash_map::Entry;
-use std::collections::VecDeque;
+use std::collections::{BTreeMap, VecDeque};
 
 use rustc_data_structures::fx::FxHashMap;
 use rustc_middle::mir::coverage::{
     BlockMarkerId, BranchSpan, ConditionId, ConditionInfo, CoverageKind, MCDCBranchSpan,
-    MCDCDecisionSpan,
+    MCDCDecisionSpan, MCDCMatchBranchSpan,
 };
-use rustc_middle::mir::{self, BasicBlock, SourceInfo, UnOp};
+use rustc_middle::mir::{self, BasicBlock, SourceInfo, StatementKind, UnOp};
 use rustc_middle::thir::{ExprId, ExprKind, LogicalOp, Thir};
 use rustc_middle::ty::TyCtxt;
 use rustc_span::def_id::LocalDefId;
@@ -22,7 +22,8 @@ pub(crate) struct BranchInfoBuilder {
 
     num_block_markers: usize,
     branch_spans: Vec<BranchSpan>,
-
+    pattern_match_branch_records: BTreeMap<Span, MCDCMatchBranchSpan>,
+    markers_map: BTreeMap<usize, BasicBlock>,
     mcdc_branch_spans: Vec<MCDCBranchSpan>,
     mcdc_decision_spans: Vec<MCDCDecisionSpan>,
     mcdc_state: Option<MCDCState>,
@@ -47,6 +48,8 @@ impl BranchInfoBuilder {
                 nots: FxHashMap::default(),
                 num_block_markers: 0,
                 branch_spans: vec![],
+                pattern_match_branch_records: BTreeMap::new(),
+                markers_map: Default::default(),
                 mcdc_branch_spans: vec![],
                 mcdc_decision_spans: vec![],
                 mcdc_state: MCDCState::new_if_enabled(tcx),
@@ -175,6 +178,8 @@ impl BranchInfoBuilder {
             nots: _,
             num_block_markers,
             branch_spans,
+            pattern_match_branch_records,
+            markers_map: _,
             mcdc_branch_spans,
             mcdc_decision_spans,
             mcdc_state: _,
@@ -184,11 +189,24 @@ impl BranchInfoBuilder {
             assert!(branch_spans.is_empty());
             return None;
         }
+        let mut mcdc_match_branch_spans =
+            pattern_match_branch_records.into_values().collect::<Vec<_>>();
+        mcdc_match_branch_spans.sort_by(|a, b| {
+            if a.span.contains(b.span) { std::cmp::Ordering::Less } else { a.span.cmp(&b.span) }
+        });
+        for idx in 0..(mcdc_match_branch_spans.len() - 1) {
+            if mcdc_match_branch_spans[idx].span.contains(mcdc_match_branch_spans[idx + 1].span) {
+                let refined_span =
+                    mcdc_match_branch_spans[idx].span.until(mcdc_match_branch_spans[idx + 1].span);
+                mcdc_match_branch_spans[idx].span = refined_span;
+            }
+        }
 
         Some(Box::new(mir::coverage::BranchInfo {
             num_block_markers,
             branch_spans,
             mcdc_branch_spans,
+            mcdc_match_branch_spans,
             mcdc_decision_spans,
         }))
     }
@@ -385,4 +403,74 @@ impl Builder<'_, '_> {
             mcdc_state.record_conditions(logical_op, span);
         }
     }
+
+    pub(crate) fn visit_coverage_mcdc_match_arms(
+        &mut self,
+        test_block: BasicBlock,
+        targets: impl Iterator<Item = (Span, BasicBlock)>,
+    ) {
+        if !self.tcx.sess.instrument_coverage_mcdc() {
+            return;
+        }
+        let mut markers_map = BTreeMap::<usize, BasicBlock>::new();
+        let mut get_marker_id = |span, block| {
+            self.cfg
+                .block_data_mut(block)
+                .statements
+                .iter()
+                .find_map(|statement| match statement.kind {
+                    StatementKind::Coverage(CoverageKind::BlockMarker { id }) => Some(id),
+                    _ => None,
+                })
+                .unwrap_or_else(|| {
+                    let source_info = self.source_info(span);
+                    let id = self
+                        .coverage_branch_info
+                        .as_mut()
+                        .expect("Checked at entry of the function")
+                        .next_block_marker_id();
+
+                    let marker_statement = mir::Statement {
+                        source_info,
+                        kind: mir::StatementKind::Coverage(CoverageKind::BlockMarker { id }),
+                    };
+                    markers_map.insert(id.as_usize(), block);
+                    self.cfg.push(block, marker_statement);
+                    id
+                })
+        };
+        let test_block_id = get_marker_id(Span::default(), test_block);
+
+        let mut targets = targets
+            .map(|(span, block)| {
+                let marker_id = get_marker_id(span, block);
+                (span, marker_id)
+            })
+            .collect::<Vec<_>>();
+
+        let branch_records = self
+            .coverage_branch_info
+            .as_mut()
+            .map(|branch_info| {
+                branch_info.markers_map.extend(markers_map);
+                &mut branch_info.pattern_match_branch_records
+            })
+            .expect("Checked at entry of the function");
+
+        while let Some((span, matched_block)) = targets.pop() {
+            // By here we do not know the span of the target yet, so just record the
+            // basic blocks and insert BlockMarkerId later.
+            let record = branch_records.entry(span).or_insert_with(|| MCDCMatchBranchSpan {
+                span,
+                condition_info: None,
+                test_markers: vec![],
+                true_markers: vec![],
+                substract_markers: vec![],
+            });
+            record.test_markers.push(test_block_id);
+            record.true_markers.push(matched_block);
+
+            record.substract_markers.extend(targets.iter().map(|(_, blk)| *blk));
+        }
+    }
 }

Diff for: compiler/rustc_mir_build/src/build/matches/mod.rs

@@ -1857,6 +1857,24 @@ impl<'a, 'tcx> Builder<'a, 'tcx> {
         // Extract the match-pair from the highest priority candidate and build a test from it.
         let (match_place, test) = self.pick_test(candidates);
 
+        // Patterns like `Some(A) | Some(B)` need spans for their target branches separately.
+        // Keep it in original order so that true arms of the laters won't be counted as false arms of the formers.
+        let mut coverage_targets: Vec<_> = candidates
+            .iter()
+            .filter_map(|candidate| {
+                candidate.match_pairs.first().map(|matched_pair| {
+                    (candidate.extra_data.span, (matched_pair.pattern.span, None))
+                })
+            })
+            .collect();
+
+        let mut set_coverage_matched_blk = |pattern_span: Span, blk: BasicBlock| {
+            *coverage_targets
+                .iter_mut()
+                .find_map(|(span, (_, blk_opt))| (*span == pattern_span).then_some(blk_opt))
+                .expect("target_candidates only are part of candidates") = Some(blk);
+        };
+
         // For each of the N possible test outcomes, build the vector of candidates that applies if
         // the test has that particular outcome.
         let (remaining_candidates, target_candidates) =
@@ -1883,6 +1901,10 @@ impl<'a, 'tcx> Builder<'a, 'tcx> {
             .into_iter()
             .map(|(branch, mut candidates)| {
                 let candidate_start = self.cfg.start_new_block();
+                set_coverage_matched_blk(
+                    candidates.first().expect("candidates must be not empty").extra_data.span,
+                    candidate_start,
+                );
                 self.match_candidates(
                     span,
                     scrutinee_span,
@@ -1894,6 +1916,12 @@ impl<'a, 'tcx> Builder<'a, 'tcx> {
             })
             .collect();
 
+        self.visit_coverage_mcdc_match_arms(
+            start_block,
+            coverage_targets.into_iter().filter_map(|(_, (span, matched_target_block))| {
+                matched_target_block.map(|blk| (span, blk))
+            }),
+        );
         // Perform the test, branching to one of N blocks.
         self.perform_test(
             span,

Diff for: compiler/rustc_mir_transform/src/coverage/counters.rs

@@ -1,4 +1,7 @@
-use std::fmt::{self, Debug};
+use std::{
+    collections::BTreeMap,
+    fmt::{self, Debug},
+};
 
 use rustc_data_structures::captures::Captures;
 use rustc_data_structures::fx::FxHashMap;
@@ -58,6 +61,8 @@ pub(super) struct CoverageCounters {
     /// Table of expression data, associating each expression ID with its
     /// corresponding operator (+ or -) and its LHS/RHS operands.
     expressions: IndexVec<ExpressionId, Expression>,
+
+    branch_sum_expressions: BTreeMap<BasicCoverageBlock, Vec<ExpressionId>>,
 }
 
 impl CoverageCounters {
@@ -75,6 +80,7 @@ impl CoverageCounters {
             bcb_counters: IndexVec::from_elem_n(None, num_bcbs),
             bcb_edge_counters: FxHashMap::default(),
             expressions: IndexVec::new(),
+            branch_sum_expressions: BTreeMap::new(),
         };
 
         MakeBcbCounters::new(&mut this, basic_coverage_blocks)
@@ -88,7 +94,7 @@ impl CoverageCounters {
         BcbCounter::Counter { id }
     }
 
-    fn make_expression(&mut self, lhs: BcbCounter, op: Op, rhs: BcbCounter) -> BcbCounter {
+    pub fn make_expression(&mut self, lhs: BcbCounter, op: Op, rhs: BcbCounter) -> BcbCounter {
         let expression = Expression { lhs: lhs.as_term(), op, rhs: rhs.as_term() };
         let id = self.expressions.push(expression);
         BcbCounter::Expression { id }
@@ -97,7 +103,11 @@ impl CoverageCounters {
     /// Variant of `make_expression` that makes `lhs` optional and assumes [`Op::Add`].
     ///
     /// This is useful when using [`Iterator::fold`] to build an arbitrary-length sum.
-    fn make_sum_expression(&mut self, lhs: Option<BcbCounter>, rhs: BcbCounter) -> BcbCounter {
+    pub(super) fn make_sum_expression(
+        &mut self,
+        lhs: Option<BcbCounter>,
+        rhs: BcbCounter,
+    ) -> BcbCounter {
         let Some(lhs) = lhs else { return rhs };
         self.make_expression(lhs, Op::Add, rhs)
     }
@@ -138,6 +148,10 @@ impl CoverageCounters {
         }
     }
 
+    pub(super) fn set_bcb_branch_sum_expr(&mut self, bcb: BasicCoverageBlock, expr: ExpressionId) {
+        self.branch_sum_expressions.entry(bcb).or_default().push(expr);
+    }
+
     pub(super) fn bcb_counter(&self, bcb: BasicCoverageBlock) -> Option<BcbCounter> {
         self.bcb_counters[bcb]
     }
@@ -164,6 +178,12 @@ impl CoverageCounters {
         })
     }
 
+    pub(super) fn bcb_nodes_with_branch_sum_expressions(
+        &self,
+    ) -> impl Iterator<Item = (BasicCoverageBlock, &[ExpressionId])> {
+        self.branch_sum_expressions.iter().map(|(bcb, exprs)| (*bcb, exprs.as_slice()))
+    }
+
     pub(super) fn into_expressions(self) -> IndexVec<ExpressionId, Expression> {
         self.expressions
     }