coverage. Group MCDC decisions and conditions until instrument mappin…

…gs and support multiple branch markers
rust-lang · ZhuUx · May 21, 2024 · Jul 9, 2024 · Jul 9, 2024 · Jul 25, 2024
commit bc11434a0943acc43788216934bb14c724c75358
diff --git a/compiler/rustc_middle/src/mir/coverage.rs b/compiler/rustc_middle/src/mir/coverage.rs
@@ -274,8 +274,10 @@ pub struct CoverageInfoHi {
     /// data structures without having to scan the entire body first.
     pub num_block_markers: usize,
     pub branch_spans: Vec<BranchSpan>,
-    pub mcdc_branch_spans: Vec<MCDCBranchSpan>,
-    pub mcdc_decision_spans: Vec<MCDCDecisionSpan>,
+    /// Branch spans generated by mcdc. Because of some limits mcdc builder give up generating
+    /// decisions including them so that they are handled as normal branch spans.
+    pub mcdc_degraded_spans: Vec<MCDCBranchSpan>,
+    pub mcdc_spans: Vec<(MCDCDecisionSpan, Vec<MCDCBranchSpan>)>,
 }
 
 #[derive(Clone, Debug)]
@@ -308,12 +310,12 @@ impl Default for ConditionInfo {
 #[derive(TyEncodable, TyDecodable, Hash, HashStable, TypeFoldable, TypeVisitable)]
 pub struct MCDCBranchSpan {
     pub span: Span,
-    /// If `None`, this actually represents a normal branch span inserted for
-    /// code that was too complex for MC/DC.
-    pub condition_info: Option<ConditionInfo>,
-    pub true_marker: BlockMarkerId,
-    pub false_marker: BlockMarkerId,
-    pub decision_depth: u16,
+    pub condition_info: ConditionInfo,
+    // For boolean decisions and most pattern matching decisions there is only
+    // one true marker and one false marker in each branch. But for matching decisions
+    // with `|` there can be several.
+    pub true_markers: Vec<BlockMarkerId>,
+    pub false_markers: Vec<BlockMarkerId>,
 }
 
 #[derive(Copy, Clone, Debug)]
@@ -327,13 +329,12 @@ pub struct DecisionInfo {
 #[derive(TyEncodable, TyDecodable, Hash, HashStable, TypeFoldable, TypeVisitable)]
 pub struct MCDCDecisionSpan {
     pub span: Span,
-    pub num_conditions: usize,
     pub end_markers: Vec<BlockMarkerId>,
     pub decision_depth: u16,
 }
 
 impl MCDCDecisionSpan {
     pub fn new(span: Span) -> Self {
-        Self { span, num_conditions: 0, end_markers: Vec::new(), decision_depth: 0 }
+        Self { span, end_markers: Vec::new(), decision_depth: 0 }
     }
 }
diff --git a/compiler/rustc_middle/src/mir/pretty.rs b/compiler/rustc_middle/src/mir/pretty.rs
@@ -490,8 +490,8 @@ fn write_coverage_info_hi(
     let coverage::CoverageInfoHi {
         num_block_markers: _,
         branch_spans,
-        mcdc_branch_spans,
-        mcdc_decision_spans,
+        mcdc_degraded_spans,
+        mcdc_spans,
     } = coverage_info_hi;
 
     // Only add an extra trailing newline if we printed at least one thing.
@@ -505,29 +505,30 @@ fn write_coverage_info_hi(
         did_print = true;
     }
 
-    for coverage::MCDCBranchSpan {
-        span,
-        condition_info,
-        true_marker,
-        false_marker,
-        decision_depth,
-    } in mcdc_branch_spans
-    {
+    for coverage::MCDCBranchSpan { span, true_markers, false_markers, .. } in mcdc_degraded_spans {
         writeln!(
             w,
-            "{INDENT}coverage mcdc branch {{ condition_id: {:?}, true: {true_marker:?}, false: {false_marker:?}, depth: {decision_depth:?} }} => {span:?}",
-            condition_info.map(|info| info.condition_id)
+            "{INDENT}coverage branch {{ true: {true_markers:?}, false: {false_markers:?} }} => {span:?}",
         )?;
         did_print = true;
     }
 
-    for coverage::MCDCDecisionSpan { span, num_conditions, end_markers, decision_depth } in
-        mcdc_decision_spans
+    for (coverage::MCDCDecisionSpan { span, end_markers, decision_depth }, conditions) in mcdc_spans
     {
+        let num_conditions = conditions.len();
         writeln!(
             w,
             "{INDENT}coverage mcdc decision {{ num_conditions: {num_conditions:?}, end: {end_markers:?}, depth: {decision_depth:?} }} => {span:?}"
         )?;
+        for coverage::MCDCBranchSpan { span, condition_info, true_markers, false_markers } in
+            conditions
+        {
+            writeln!(
+                w,
+                "{INDENT}coverage mcdc branch {{ condition_id: {:?}, true: {true_markers:?}, false: {false_markers:?} }} => {span:?}",
+                condition_info.condition_id
+            )?;
+        }
         did_print = true;
     }
 

diff --git a/compiler/rustc_mir_build/src/build/coverageinfo.rs b/compiler/rustc_mir_build/src/build/coverageinfo.rs
@@ -175,21 +175,16 @@ impl CoverageInfoBuilder {
         let branch_spans =
             branch_info.map(|branch_info| branch_info.branch_spans).unwrap_or_default();
 
-        let (mut mcdc_branch_spans, mcdc_spans) =
+        let (mcdc_degraded_spans, mcdc_spans) =
             mcdc_info.map(MCDCInfoBuilder::into_done).unwrap_or_default();
-        let mut mcdc_decision_spans = Vec::with_capacity(mcdc_spans.len());
-        for (decision, conditions) in mcdc_spans {
-            mcdc_branch_spans.extend(conditions);
-            mcdc_decision_spans.push(decision);
-        }
 
         // For simplicity, always return an info struct (without Option), even
         // if there's nothing interesting in it.
         Box::new(CoverageInfoHi {
             num_block_markers,
             branch_spans,
-            mcdc_branch_spans,
-            mcdc_decision_spans,
+            mcdc_degraded_spans,
+            mcdc_spans,
         })
     }
 }

diff --git a/compiler/rustc_mir_build/src/build/coverageinfo/mcdc.rs b/compiler/rustc_mir_build/src/build/coverageinfo/mcdc.rs
@@ -34,6 +34,7 @@ struct BooleanDecisionCtx {
     /// To construct condition evaluation tree.
     decision_stack: VecDeque<ConditionInfo>,
     conditions: Vec<MCDCBranchSpan>,
+    condition_id_counter: usize,
 }
 
 impl BooleanDecisionCtx {
@@ -43,9 +44,15 @@ impl BooleanDecisionCtx {
             decision_info: MCDCDecisionSpan::new(Span::default()),
             decision_stack: VecDeque::new(),
             conditions: vec![],
+            condition_id_counter: 0,
         }
     }
 
+    fn next_condition_id(&mut self) -> ConditionId {
+        self.condition_id_counter += 1;
+        ConditionId::from_usize(self.condition_id_counter)
+    }
+
     // At first we assign ConditionIds for each sub expression.
     // If the sub expression is composite, re-assign its ConditionId to its LHS and generate a new ConditionId for its RHS.
     //
@@ -89,14 +96,12 @@ impl BooleanDecisionCtx {
     fn record_conditions(&mut self, op: LogicalOp) {
         let parent_condition = self.decision_stack.pop_back().unwrap_or_default();
         let lhs_id = if parent_condition.condition_id == ConditionId::NONE {
-            self.decision_info.num_conditions += 1;
-            ConditionId::from(self.decision_info.num_conditions)
+            ConditionId::from(self.next_condition_id())
         } else {
             parent_condition.condition_id
         };
 
-        self.decision_info.num_conditions += 1;
-        let rhs_condition_id = ConditionId::from(self.decision_info.num_conditions);
+        let rhs_condition_id = self.next_condition_id();
 
         let (lhs, rhs) = match op {
             LogicalOp::And => {
@@ -147,13 +152,10 @@ impl BooleanDecisionCtx {
 
         self.conditions.push(MCDCBranchSpan {
             span,
-            condition_info: Some(condition_info),
-            true_marker,
-            false_marker,
-            decision_depth: 0,
+            condition_info,
+            true_markers: vec![true_marker],
+            false_markers: vec![false_marker],
         });
-        // In case this decision had only one condition
-        self.decision_info.num_conditions = self.decision_info.num_conditions.max(1);
     }
 
     fn is_finished(&self) -> bool {
@@ -247,7 +249,6 @@ struct MCDCTargetInfo {
 impl MCDCTargetInfo {
     fn set_depth(&mut self, depth: u16) {
         self.decision.decision_depth = depth;
-        self.conditions.iter_mut().for_each(|branch| branch.decision_depth = depth);
     }
 }
 
@@ -341,7 +342,9 @@ impl MCDCInfoBuilder {
     }
 
     fn append_normal_branches(&mut self, mut branches: Vec<MCDCBranchSpan>) {
-        branches.iter_mut().for_each(|branch| branch.condition_info = None);
+        branches
+            .iter_mut()
+            .for_each(|branch| branch.condition_info.condition_id = ConditionId::NONE);
         self.normal_branch_spans.extend(branches);
     }
 

diff --git a/compiler/rustc_mir_transform/src/coverage/counters.rs b/compiler/rustc_mir_transform/src/coverage/counters.rs
@@ -101,7 +101,12 @@ impl CoverageCounters {
         BcbCounter::Counter { id }
     }
 
-    fn make_expression(&mut self, lhs: BcbCounter, op: Op, rhs: BcbCounter) -> BcbCounter {
+    pub(super) fn make_expression(
+        &mut self,
+        lhs: BcbCounter,
+        op: Op,
+        rhs: BcbCounter,
+    ) -> BcbCounter {
         let new_expr = BcbExpression { lhs, op, rhs };
         *self
             .expressions_memo
@@ -159,7 +164,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)
     }