coverage. Implement branch coverage in mcdc for pattern match

Author: zhuyunxing

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 {

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)
         )?;
     }

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));
+        }
+    }
 }

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,

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
     }

compiler/rustc_mir_transform/src/coverage/mod.rs

@@ -11,6 +11,7 @@ use self::counters::{CounterIncrementSite, CoverageCounters};
 use self::graph::{BasicCoverageBlock, CoverageGraph};
 use self::spans::{BcbBranchPair, BcbMapping, BcbMappingKind, CoverageSpans};
 
+use crate::coverage::counters::BcbCounter;
 use crate::MirPass;
 
 use rustc_middle::mir::coverage::*;
@@ -83,10 +84,10 @@ fn instrument_function_for_coverage<'tcx>(tcx: TyCtxt<'tcx>, mir_body: &mut mir:
     // and all `Expression` dependencies (operands) are also generated, for any other
     // `BasicCoverageBlock`s not already associated with a coverage span.
     let bcb_has_coverage_spans = |bcb| coverage_spans.bcb_has_coverage_spans(bcb);
-    let coverage_counters =
+    let mut coverage_counters =
         CoverageCounters::make_bcb_counters(&basic_coverage_blocks, bcb_has_coverage_spans);
 
-    let mappings = create_mappings(tcx, &hir_info, &coverage_spans, &coverage_counters);
+    let mappings = create_mappings(tcx, &hir_info, &coverage_spans, &mut 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");
@@ -120,7 +121,7 @@ fn create_mappings<'tcx>(
     tcx: TyCtxt<'tcx>,
     hir_info: &ExtractedHirInfo,
     coverage_spans: &CoverageSpans,
-    coverage_counters: &CoverageCounters,
+    coverage_counters: &mut CoverageCounters,
 ) -> Vec<Mapping> {
     let source_map = tcx.sess.source_map();
     let body_span = hir_info.body_span;
@@ -160,6 +161,9 @@ fn create_mappings<'tcx>(
                     false_term: term_for_bcb(false_bcb),
                     mcdc_params,
                 },
+                BcbMappingKind::MCDCMatchBranch { .. } => {
+                    return None;
+                }
                 BcbMappingKind::MCDCDecision { bitmap_idx, conditions_num, .. } => {
                     MappingKind::MCDCDecision(DecisionInfo { bitmap_idx, conditions_num })
                 }
@@ -179,9 +183,63 @@ fn create_mappings<'tcx>(
         },
     ));
 
+    mappings.extend(create_mcdc_match_mappings(coverage_spans, coverage_counters, |span| {
+        make_code_region(source_map, file_name, span, body_span)
+    }));
+
     mappings
 }
 
+fn create_mcdc_match_mappings(
+    coverage_spans: &CoverageSpans,
+    coverage_counters: &mut CoverageCounters,
+    get_code_region: impl Fn(Span) -> Option<CodeRegion>,
+) -> Vec<Mapping> {
+    coverage_spans
+        .mappings
+        .iter()
+        .filter_map(|mapping| match mapping.kind {
+            BcbMappingKind::MCDCMatchBranch {
+                ref true_bcbs,
+                ref test_bcbs,
+                prebinding_bcbs: ref prebingding_bcbs,
+                ..
+            } => {
+                let used_block = true_bcbs[0];
+                let mut calc_exprs = |bcbs: &[BasicCoverageBlock]| {
+                    bcbs.iter()
+                        .map(|bcb| coverage_counters.bcb_counter(*bcb))
+                        .collect::<Option<Vec<_>>>()
+                        .and_then(|counters| {
+                            counters.into_iter().fold(None, |acc, counter| {
+                                Some(coverage_counters.make_sum_expression(acc, counter))
+                            })
+                        })
+                };
+                let true_expr = calc_exprs(true_bcbs).expect("true blocks must not be empty");
+                let test_expr = calc_exprs(test_bcbs).expect("test blocks must not be empty");
+                let prebinding_expr = calc_exprs(prebingding_bcbs);
+                let true_term = true_expr.as_term();
+                let exclude_expr =
+                    coverage_counters.make_sum_expression(prebinding_expr, true_expr);
+                let false_expr =
+                    coverage_counters.make_expression(test_expr, Op::Subtract, exclude_expr);
+                let false_term = false_expr.as_term();
+                if let BcbCounter::Expression { id } = true_expr {
+                    coverage_counters.set_bcb_branch_sum_expr(used_block, id);
+                }
+                if let BcbCounter::Expression { id } = false_expr {
+                    coverage_counters.set_bcb_branch_sum_expr(used_block, id);
+                }
+
+                let kind = MappingKind::Branch { true_term, false_term };
+                get_code_region(mapping.span).map(|code_region| Mapping { kind, code_region })
+            }
+            _ => None,
+        })
+        .collect()
+}
+
 /// For each BCB node or BCB edge that has an associated coverage counter,
 /// inject any necessary coverage statements into MIR.
 fn inject_coverage_statements<'tcx>(
@@ -232,6 +290,16 @@ fn inject_coverage_statements<'tcx>(
             basic_coverage_blocks[bcb].leader_bb(),
         );
     }
+
+    for (bcb, expressions) in coverage_counters.bcb_nodes_with_branch_sum_expressions() {
+        expressions.into_iter().for_each(|&id| {
+            inject_statement(
+                mir_body,
+                CoverageKind::ExpressionUsed { id },
+                basic_coverage_blocks[bcb].leader_bb(),
+            );
+        });
+    }
 }
 
 /// For each conditions inject statements to update condition bitmap after it has been evaluated.

compiler/rustc_mir_transform/src/coverage/spans.rs

@@ -27,6 +27,13 @@ pub(super) enum BcbMappingKind {
         /// for code that was too complex for MC/DC.
         condition_info: Option<ConditionInfo>,
     },
+    MCDCMatchBranch {
+        true_bcbs: Vec<BasicCoverageBlock>,
+        test_bcbs: Vec<BasicCoverageBlock>,
+        prebinding_bcbs: Vec<BasicCoverageBlock>,
+        #[allow(unused)]
+        condition_info: Option<ConditionInfo>,
+    },
     /// Associates a mcdc decision with its join BCB.
     MCDCDecision { end_bcbs: BTreeSet<BasicCoverageBlock>, bitmap_idx: u32, conditions_num: u16 },
 }
@@ -126,6 +133,19 @@ pub(super) fn generate_coverage_spans(
                 insert(true_bcb);
                 insert(false_bcb);
             }
+            BcbMappingKind::MCDCMatchBranch {
+                ref true_bcbs,
+                ref test_bcbs,
+                prebinding_bcbs: ref prebingding_bcbs,
+                ..
+            } => {
+                true_bcbs
+                    .iter()
+                    .chain(test_bcbs.iter())
+                    .chain(prebingding_bcbs)
+                    .copied()
+                    .for_each(&mut insert);
+            }
             BcbMappingKind::MCDCDecision { bitmap_idx, conditions_num, .. } => {
                 // `bcb_has_mappings` is used for inject coverage counters
                 // but they are not needed for decision BCBs.

compiler/rustc_mir_transform/src/coverage/spans/from_mir.rs

@@ -2,7 +2,7 @@ use rustc_data_structures::captures::Captures;
 use rustc_data_structures::fx::FxHashSet;
 use rustc_index::IndexVec;
 use rustc_middle::mir::coverage::{
-    BlockMarkerId, BranchSpan, CoverageKind, MCDCBranchSpan, MCDCDecisionSpan,
+    BlockMarkerId, BranchSpan, CoverageKind, MCDCBranchSpan, MCDCDecisionSpan, MCDCMatchBranchSpan,
 };
 use rustc_middle::mir::{
     self, AggregateKind, BasicBlock, FakeReadCause, Rvalue, Statement, StatementKind, Terminator,
@@ -462,6 +462,31 @@ pub(super) fn extract_mcdc_mappings(
                 },
             )
         };
+    let mcdc_match_branch_filter_map = |&MCDCMatchBranchSpan {
+                                            span: raw_span,
+                                            ref test_markers,
+                                            ref true_markers,
+                                            ref substract_markers,
+                                            condition_info,
+                                        }| {
+        if !raw_span.ctxt().outer_expn_data().is_root() {
+            return None;
+        }
+        let (span, _) = unexpand_into_body_span_with_visible_macro(raw_span, body_span)?;
+        let test_bcbs = test_markers.iter().copied().map(bcb_from_marker).collect::<Option<_>>()?;
+        let true_bcbs = true_markers.iter().copied().map(bcb_from_marker).collect::<Option<_>>()?;
+        let prebinding_bcbs =
+            substract_markers.iter().copied().map(bcb_from_marker).collect::<Option<_>>()?;
+        Some(BcbMapping {
+            kind: BcbMappingKind::MCDCMatchBranch {
+                test_bcbs,
+                true_bcbs,
+                prebinding_bcbs,
+                condition_info,
+            },
+            span,
+        })
+    };
 
     let mut next_bitmap_idx = 0;
 
@@ -471,8 +496,10 @@ pub(super) fn extract_mcdc_mappings(
         let end_bcbs = decision
             .end_markers
             .iter()
-            .map(|&marker| bcb_from_marker(marker))
-            .collect::<Option<_>>()?;
+            .copied()
+            .map(bcb_from_marker)
+            .collect::<Option<_>>()
+            .expect("all markers must be located in a bcb");
 
         let bitmap_idx = next_bitmap_idx;
         next_bitmap_idx += (1_u32 << decision.conditions_num).div_ceil(8);
@@ -489,6 +516,7 @@ pub(super) fn extract_mcdc_mappings(
 
     std::iter::empty()
         .chain(branch_info.mcdc_branch_spans.iter().filter_map(mcdc_branch_filter_map))
+        .chain(branch_info.mcdc_match_branch_spans.iter().filter_map(mcdc_match_branch_filter_map))
         .chain(branch_info.mcdc_decision_spans.iter().filter_map(decision_filter_map))
         .collect::<Vec<_>>()
 }