coverage. Introduce MCDCBranchMarker to trace branch blocks of condit…

…ions
rust-lang · Jun 20, 2024 · 8712975 · 8712975
1 parent c002a8f
commit 8712975
Show file tree

Hide file tree

Showing 7 changed files with 276 additions and 191 deletions.
diff --git a/compiler/rustc_middle/src/mir/coverage.rs b/compiler/rustc_middle/src/mir/coverage.rs
@@ -291,8 +291,8 @@ pub struct BranchInfo {
     /// 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>,
+    pub mcdc_degraded_spans: Vec<MCDCBranchSpan>,
+    pub mcdc_spans: Vec<(MCDCDecisionSpan, Vec<MCDCBranchSpan>)>,
 }
 
 #[derive(Clone, Debug)]
@@ -325,12 +325,16 @@ 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,
+    pub markers: MCDCBranchMarkers,
+}
+
+#[derive(Clone, Debug)]
+#[derive(TyEncodable, TyDecodable, Hash, HashStable, TypeFoldable, TypeVisitable)]
+pub enum MCDCBranchMarkers {
+    /// The first indicates true branch, the second indicates the false branch.
+    Boolean(BlockMarkerId, BlockMarkerId),
+    PatternMatching,
 }
 
 #[derive(Copy, Clone, Debug)]
@@ -344,7 +348,6 @@ 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,
 }
diff --git a/compiler/rustc_middle/src/mir/pretty.rs b/compiler/rustc_middle/src/mir/pretty.rs
@@ -487,8 +487,7 @@ fn write_coverage_branch_info(
     branch_info: &coverage::BranchInfo,
     w: &mut dyn io::Write,
 ) -> io::Result<()> {
-    let coverage::BranchInfo { branch_spans, mcdc_branch_spans, mcdc_decision_spans, .. } =
-        branch_info;
+    let coverage::BranchInfo { branch_spans, mcdc_degraded_spans, mcdc_spans, .. } = branch_info;
 
     for coverage::BranchSpan { span, true_marker, false_marker } in branch_spans {
         writeln!(
@@ -497,32 +496,30 @@ fn write_coverage_branch_info(
         )?;
     }
 
-    for coverage::MCDCBranchSpan {
-        span,
-        condition_info,
-        true_marker,
-        false_marker,
-        decision_depth,
-    } in mcdc_branch_spans
-    {
+    for coverage::MCDCBranchSpan { span, 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 mcdc degraded branch {{ markers: {markers:?} }} => {span:?}",
         )?;
     }
 
-    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, markers } in conditions {
+            writeln!(
+                w,
+                "{INDENT}coverage mcdc branch {{ condition_id: {:?}, markers: {markers:?} }} => {span:?}",
+                condition_info.condition_id
+            )?;
+        }
     }
 
-    if !branch_spans.is_empty() || !mcdc_branch_spans.is_empty() || !mcdc_decision_spans.is_empty()
-    {
+    if !branch_spans.is_empty() || !mcdc_degraded_spans.is_empty() || !mcdc_spans.is_empty() {
         writeln!(w)?;
     }
 

diff --git a/compiler/rustc_mir_build/src/build/coverageinfo.rs b/compiler/rustc_mir_build/src/build/coverageinfo.rs
@@ -162,18 +162,13 @@ impl BranchInfoBuilder {
             return None;
         }
 
-        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);
-        }
         Some(Box::new(mir::coverage::BranchInfo {
             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
@@ -3,7 +3,8 @@ use std::collections::VecDeque;
 use rustc_data_structures::fx::FxIndexMap;
 use rustc_middle::bug;
 use rustc_middle::mir::coverage::{
-    BlockMarkerId, ConditionId, ConditionInfo, DecisionId, MCDCBranchSpan, MCDCDecisionSpan,
+    BlockMarkerId, ConditionId, ConditionInfo, DecisionId, MCDCBranchMarkers, MCDCBranchSpan,
+    MCDCDecisionSpan,
 };
 use rustc_middle::mir::BasicBlock;
 use rustc_middle::thir::LogicalOp;
@@ -29,6 +30,7 @@ struct BooleanDecisionCtx {
     /// To construct condition evaluation tree.
     decision_stack: VecDeque<ConditionInfo>,
     conditions: Vec<MCDCBranchSpan>,
+    condition_id_counter: usize,
 }
 
 impl BooleanDecisionCtx {
@@ -37,15 +39,20 @@ impl BooleanDecisionCtx {
             id,
             decision_info: MCDCDecisionSpan {
                 span: Span::default(),
-                num_conditions: 0,
                 end_markers: vec![],
                 decision_depth: 0,
             },
             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,9 @@ impl BooleanDecisionCtx {
 
         self.conditions.push(MCDCBranchSpan {
             span,
-            condition_info: Some(condition_info),
-            true_marker,
-            false_marker,
-            decision_depth: 0,
+            condition_info,
+            markers: MCDCBranchMarkers::Boolean(true_marker, 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 {
@@ -250,7 +251,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);
     }
 }
 
@@ -298,7 +298,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)
     }