coverage: Split out MC/DC branches from BcbMappingKind

Author: Zalathar

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

@@ -3,9 +3,7 @@ use std::collections::BTreeSet;
 use rustc_data_structures::graph::DirectedGraph;
 use rustc_index::bit_set::BitSet;
 use rustc_index::IndexVec;
-use rustc_middle::mir::coverage::{
-    BlockMarkerId, BranchSpan, ConditionInfo, CoverageKind, MCDCBranchSpan,
-};
+use rustc_middle::mir::coverage::{BlockMarkerId, BranchSpan, ConditionInfo, CoverageKind};
 use rustc_middle::mir::{self, BasicBlock, StatementKind};
 use rustc_span::Span;
 
@@ -15,23 +13,13 @@ use crate::coverage::spans::{
 };
 use crate::coverage::ExtractedHirInfo;
 
-#[derive(Clone, Debug)]
+#[derive(Clone, Copy, Debug)]
 pub(super) enum BcbMappingKind {
     /// Associates an ordinary executable code span with its corresponding BCB.
     Code(BasicCoverageBlock),
-
-    // Ordinary branch mappings are stored separately, so they don't have a
-    // variant in this enum.
     //
-    /// Associates a mcdc branch span with condition info besides fields for normal branch.
-    MCDCBranch {
-        true_bcb: BasicCoverageBlock,
-        false_bcb: BasicCoverageBlock,
-        /// If `None`, this actually represents a normal branch mapping inserted
-        /// for code that was too complex for MC/DC.
-        condition_info: Option<ConditionInfo>,
-        decision_depth: u16,
-    },
+    // Branch and MC/DC mappings are more complex, so they are represented
+    // separately.
 }
 
 #[derive(Debug)]
@@ -50,6 +38,18 @@ pub(super) struct BcbBranchPair {
     pub(super) false_bcb: BasicCoverageBlock,
 }
 
+/// Associates an MC/DC branch span with condition info besides fields for normal branch.
+#[derive(Debug)]
+pub(super) struct MCDCBranch {
+    pub(super) span: Span,
+    pub(super) true_bcb: BasicCoverageBlock,
+    pub(super) false_bcb: BasicCoverageBlock,
+    /// If `None`, this actually represents a normal branch mapping inserted
+    /// for code that was too complex for MC/DC.
+    pub(super) condition_info: Option<ConditionInfo>,
+    pub(super) decision_depth: u16,
+}
+
 /// Associates an MC/DC decision with its join BCBs.
 #[derive(Debug)]
 pub(super) struct MCDCDecision {
@@ -65,6 +65,7 @@ pub(super) struct CoverageSpans {
     pub(super) mappings: Vec<BcbMapping>,
     pub(super) branch_pairs: Vec<BcbBranchPair>,
     test_vector_bitmap_bytes: u32,
+    pub(super) mcdc_branches: Vec<MCDCBranch>,
     pub(super) mcdc_decisions: Vec<MCDCDecision>,
 }
 
@@ -89,6 +90,7 @@ pub(super) fn generate_coverage_spans(
 ) -> Option<CoverageSpans> {
     let mut mappings = vec![];
     let mut branch_pairs = vec![];
+    let mut mcdc_branches = vec![];
     let mut mcdc_decisions = vec![];
 
     if hir_info.is_async_fn {
@@ -104,15 +106,20 @@ pub(super) fn generate_coverage_spans(
 
         branch_pairs.extend(extract_branch_pairs(mir_body, hir_info, basic_coverage_blocks));
 
-        mappings.extend(extract_mcdc_mappings(
+        extract_mcdc_mappings(
             mir_body,
             hir_info.body_span,
             basic_coverage_blocks,
+            &mut mcdc_branches,
             &mut mcdc_decisions,
-        ));
+        );
     }
 
-    if mappings.is_empty() && branch_pairs.is_empty() && mcdc_decisions.is_empty() {
+    if mappings.is_empty()
+        && branch_pairs.is_empty()
+        && mcdc_branches.is_empty()
+        && mcdc_decisions.is_empty()
+    {
         return None;
     }
 
@@ -122,19 +129,19 @@ pub(super) fn generate_coverage_spans(
         bcb_has_mappings.insert(bcb);
     };
 
-    for BcbMapping { kind, span: _ } in &mappings {
-        match *kind {
+    for &BcbMapping { kind, span: _ } in &mappings {
+        match kind {
             BcbMappingKind::Code(bcb) => insert(bcb),
-            BcbMappingKind::MCDCBranch { true_bcb, false_bcb, .. } => {
-                insert(true_bcb);
-                insert(false_bcb);
-            }
         }
     }
     for &BcbBranchPair { true_bcb, false_bcb, .. } in &branch_pairs {
         insert(true_bcb);
         insert(false_bcb);
     }
+    for &MCDCBranch { true_bcb, false_bcb, .. } in &mcdc_branches {
+        insert(true_bcb);
+        insert(false_bcb);
+    }
 
     // Determine the length of the test vector bitmap.
     let test_vector_bitmap_bytes = mcdc_decisions
@@ -150,6 +157,7 @@ pub(super) fn generate_coverage_spans(
         mappings,
         branch_pairs,
         test_vector_bitmap_bytes,
+        mcdc_branches,
         mcdc_decisions,
     })
 }
@@ -217,11 +225,10 @@ pub(super) fn extract_mcdc_mappings(
     mir_body: &mir::Body<'_>,
     body_span: Span,
     basic_coverage_blocks: &CoverageGraph,
+    mcdc_branches: &mut impl Extend<MCDCBranch>,
     mcdc_decisions: &mut impl Extend<MCDCDecision>,
-) -> Vec<BcbMapping> {
-    let Some(branch_info) = mir_body.coverage_branch_info.as_deref() else {
-        return vec![];
-    };
+) {
+    let Some(branch_info) = mir_body.coverage_branch_info.as_deref() else { return };
 
     let block_markers = resolve_block_markers(branch_info, mir_body);
 
@@ -242,25 +249,19 @@ pub(super) fn extract_mcdc_mappings(
             Some((span, true_bcb, false_bcb))
         };
 
-    let mcdc_branch_filter_map = |&MCDCBranchSpan {
-                                      span: raw_span,
-                                      true_marker,
-                                      false_marker,
-                                      condition_info,
-                                      decision_depth,
-                                  }| {
-        check_branch_bcb(raw_span, true_marker, false_marker).map(|(span, true_bcb, false_bcb)| {
-            BcbMapping {
-                kind: BcbMappingKind::MCDCBranch {
-                    true_bcb,
-                    false_bcb,
-                    condition_info,
-                    decision_depth,
-                },
-                span,
-            }
-        })
-    };
+    mcdc_branches.extend(branch_info.mcdc_branch_spans.iter().filter_map(
+        |&mir::coverage::MCDCBranchSpan {
+             span: raw_span,
+             condition_info,
+             true_marker,
+             false_marker,
+             decision_depth,
+         }| {
+            let (span, true_bcb, false_bcb) =
+                check_branch_bcb(raw_span, true_marker, false_marker)?;
+            Some(MCDCBranch { span, true_bcb, false_bcb, condition_info, decision_depth })
+        },
+    ));
 
     let mut next_bitmap_idx = 0;
 
@@ -286,8 +287,4 @@ pub(super) fn extract_mcdc_mappings(
             })
         },
     ));
-
-    std::iter::empty()
-        .chain(branch_info.mcdc_branch_spans.iter().filter_map(mcdc_branch_filter_map))
-        .collect::<Vec<_>>()
 }

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

@@ -150,27 +150,11 @@ fn create_mappings<'tcx>(
     let mut mappings = Vec::new();
 
     mappings.extend(coverage_spans.mappings.iter().filter_map(
-        |BcbMapping { kind: bcb_mapping_kind, span }| {
-            let kind = match *bcb_mapping_kind {
+        |&BcbMapping { kind: bcb_mapping_kind, span }| {
+            let kind = match bcb_mapping_kind {
                 BcbMappingKind::Code(bcb) => MappingKind::Code(term_for_bcb(bcb)),
-                BcbMappingKind::MCDCBranch {
-                    true_bcb, false_bcb, condition_info: None, ..
-                } => MappingKind::Branch {
-                    true_term: term_for_bcb(true_bcb),
-                    false_term: term_for_bcb(false_bcb),
-                },
-                BcbMappingKind::MCDCBranch {
-                    true_bcb,
-                    false_bcb,
-                    condition_info: Some(mcdc_params),
-                    ..
-                } => MappingKind::MCDCBranch {
-                    true_term: term_for_bcb(true_bcb),
-                    false_term: term_for_bcb(false_bcb),
-                    mcdc_params,
-                },
             };
-            let code_region = make_code_region(source_map, file_name, *span, body_span)?;
+            let code_region = make_code_region(source_map, file_name, span, body_span)?;
             Some(Mapping { kind, code_region })
         },
     ));
@@ -185,6 +169,19 @@ fn create_mappings<'tcx>(
         },
     ));
 
+    mappings.extend(coverage_spans.mcdc_branches.iter().filter_map(
+        |&mappings::MCDCBranch { span, true_bcb, false_bcb, condition_info, decision_depth: _ }| {
+            let code_region = make_code_region(source_map, file_name, span, body_span)?;
+            let true_term = term_for_bcb(true_bcb);
+            let false_term = term_for_bcb(false_bcb);
+            let kind = match condition_info {
+                Some(mcdc_params) => MappingKind::MCDCBranch { true_term, false_term, mcdc_params },
+                None => MappingKind::Branch { true_term, false_term },
+            };
+            Some(Mapping { kind, code_region })
+        },
+    ));
+
     mappings.extend(coverage_spans.mcdc_decisions.iter().filter_map(
         |&mappings::MCDCDecision { span, bitmap_idx, conditions_num, .. }| {
             let code_region = make_code_region(source_map, file_name, span, body_span)?;
@@ -278,24 +275,22 @@ fn inject_mcdc_statements<'tcx>(
         }
     }
 
-    for (true_bcb, false_bcb, condition_id, decision_depth) in
-        coverage_spans.mappings.iter().filter_map(|mapping| match mapping.kind {
-            BcbMappingKind::MCDCBranch { true_bcb, false_bcb, condition_info, decision_depth } => {
-                Some((true_bcb, false_bcb, condition_info?.condition_id, decision_depth))
-            }
-            _ => None,
-        })
+    for &mappings::MCDCBranch { span: _, true_bcb, false_bcb, condition_info, decision_depth } in
+        &coverage_spans.mcdc_branches
     {
+        let Some(condition_info) = condition_info else { continue };
+        let id = condition_info.condition_id;
+
         let true_bb = basic_coverage_blocks[true_bcb].leader_bb();
         inject_statement(
             mir_body,
-            CoverageKind::CondBitmapUpdate { id: condition_id, value: true, decision_depth },
+            CoverageKind::CondBitmapUpdate { id, value: true, decision_depth },
             true_bb,
         );
         let false_bb = basic_coverage_blocks[false_bcb].leader_bb();
         inject_statement(
             mir_body,
-            CoverageKind::CondBitmapUpdate { id: condition_id, value: false, decision_depth },
+            CoverageKind::CondBitmapUpdate { id, value: false, decision_depth },
             false_bb,
         );
     }