coverage. Split structures for mcdc branches and normal branches

Author: zhuyunxing

compiler/rustc_codegen_llvm/src/coverageinfo/ffi.rs

@@ -250,23 +250,24 @@ impl CounterMappingRegion {
             MappingKind::Branch { true_term, false_term } => Self::branch_region(
                 Counter::from_term(true_term),
                 Counter::from_term(false_term),
-                None,
-                local_file_id,
-                start_line,
-                start_col,
-                end_line,
-                end_col,
-            ),
-            MappingKind::MCDCBranch { true_term, false_term, mcdc_params } => Self::branch_region(
-                Counter::from_term(true_term),
-                Counter::from_term(false_term),
-                Some(mcdc_params),
                 local_file_id,
                 start_line,
                 start_col,
                 end_line,
                 end_col,
             ),
+            MappingKind::MCDCBranch { true_term, false_term, mcdc_params } => {
+                Self::mcdc_branch_region(
+                    Counter::from_term(true_term),
+                    Counter::from_term(false_term),
+                    mcdc_params,
+                    local_file_id,
+                    start_line,
+                    start_col,
+                    end_line,
+                    end_col,
+                )
+            }
             MappingKind::MCDCDecision(decision_info) => Self::decision_region(
                 decision_info,
                 local_file_id,
@@ -303,28 +304,47 @@ impl CounterMappingRegion {
     pub(crate) fn branch_region(
         counter: Counter,
         false_counter: Counter,
-        condition_info: Option<ConditionInfo>,
         file_id: u32,
         start_line: u32,
         start_col: u32,
         end_line: u32,
         end_col: u32,
     ) -> Self {
-        let (kind, mcdc_params) = match condition_info {
-            Some(info) => (RegionKind::MCDCBranchRegion, mcdc::Parameters::branch(info.into())),
-            None => (RegionKind::BranchRegion, mcdc::Parameters::none()),
-        };
         Self {
             counter,
             false_counter,
-            mcdc_params,
+            mcdc_params: mcdc::Parameters::none(),
+            file_id,
+            expanded_file_id: 0,
+            start_line,
+            start_col,
+            end_line,
+            end_col,
+            kind: RegionKind::BranchRegion,
+        }
+    }
+
+    pub(crate) fn mcdc_branch_region(
+        counter: Counter,
+        false_counter: Counter,
+        condition_info: ConditionInfo,
+        file_id: u32,
+        start_line: u32,
+        start_col: u32,
+        end_line: u32,
+        end_col: u32,
+    ) -> Self {
+        Self {
+            counter,
+            false_counter,
+            mcdc_params: mcdc::Parameters::branch(condition_info.into()),
             file_id,
             expanded_file_id: 0,
             start_line,
             start_col,
             end_line,
             end_col,
-            kind,
+            kind: RegionKind::MCDCBranchRegion,
         }
     }
 

compiler/rustc_middle/src/mir/coverage.rs

@@ -278,14 +278,14 @@ pub struct BranchInfo {
     /// data structures without having to scan the entire body first.
     pub num_block_markers: usize,
     pub branch_spans: Vec<BranchSpan>,
-    pub decision_spans: Vec<DecisionSpan>,
+    pub mcdc_branch_spans: Vec<MCDCBranchSpan>,
+    pub mcdc_decision_spans: Vec<MCDCDecisionSpan>,
 }
 
 #[derive(Clone, Debug)]
 #[derive(TyEncodable, TyDecodable, Hash, HashStable, TypeFoldable, TypeVisitable)]
 pub struct BranchSpan {
     pub span: Span,
-    pub condition_info: ConditionInfo,
     pub true_marker: BlockMarkerId,
     pub false_marker: BlockMarkerId,
 }
@@ -308,6 +308,15 @@ impl Default for ConditionInfo {
     }
 }
 
+#[derive(Clone, Debug)]
+#[derive(TyEncodable, TyDecodable, Hash, HashStable, TypeFoldable, TypeVisitable)]
+pub struct MCDCBranchSpan {
+    pub span: Span,
+    pub condition_info: ConditionInfo,
+    pub true_marker: BlockMarkerId,
+    pub false_marker: BlockMarkerId,
+}
+
 #[derive(Copy, Clone, Debug)]
 #[derive(TyEncodable, TyDecodable, Hash, HashStable, TypeFoldable, TypeVisitable)]
 pub struct DecisionInfo {
@@ -317,8 +326,8 @@ pub struct DecisionInfo {
 
 #[derive(Clone, Debug)]
 #[derive(TyEncodable, TyDecodable, Hash, HashStable, TypeFoldable, TypeVisitable)]
-pub struct DecisionSpan {
+pub struct MCDCDecisionSpan {
     pub span: Span,
     pub conditions_num: usize,
-    pub end_marker: Vec<BlockMarkerId>,
+    pub end_markers: Vec<BlockMarkerId>,
 }

compiler/rustc_middle/src/mir/pretty.rs

@@ -475,23 +475,35 @@ fn write_coverage_branch_info(
     branch_info: &coverage::BranchInfo,
     w: &mut dyn io::Write,
 ) -> io::Result<()> {
-    let coverage::BranchInfo { branch_spans, .. } = branch_info;
+    let coverage::BranchInfo { branch_spans, mcdc_branch_spans, mcdc_decision_spans, .. } =
+        branch_info;
 
-    for coverage::BranchSpan { span, true_marker, false_marker, condition_info } in branch_spans {
-        if condition_info.condition_id == coverage::ConditionId::NONE {
-            writeln!(
-                w,
-                "{INDENT}coverage branch {{ true: {true_marker:?}, false: {false_marker:?} }} => {span:?}",
-            )?;
-        } else {
-            let id = condition_info.condition_id;
-            writeln!(
-                w,
-                "{INDENT}coverage branch {{ condition_id: {id:?}, true: {true_marker:?}, false: {false_marker:?} }} => {span:?}",
-            )?;
-        }
+    for coverage::BranchSpan { span, true_marker, false_marker } in branch_spans {
+        writeln!(
+            w,
+            "{INDENT}coverage branch {{ true: {true_marker:?}, false: {false_marker:?} }} => {span:?}",
+        )?;
     }
-    if !branch_spans.is_empty() {
+
+    for coverage::MCDCBranchSpan { span, condition_info, true_marker, false_marker } in
+        mcdc_branch_spans
+    {
+        writeln!(
+            w,
+            "{INDENT}coverage mcdc branch {{ condition_id: {:?}, true: {true_marker:?}, false: {false_marker:?} }} => {span:?}",
+            condition_info.condition_id
+        )?;
+    }
+
+    for coverage::MCDCDecisionSpan { span, conditions_num, end_markers } in mcdc_decision_spans {
+        writeln!(
+            w,
+            "{INDENT}coverage mcdc decision {{ conditions_num: {conditions_num:?}, end: {end_markers:?} }} => {span:?}"
+        )?;
+    }
+
+    if !branch_spans.is_empty() || !mcdc_branch_spans.is_empty() || !mcdc_decision_spans.is_empty()
+    {
         writeln!(w)?;
     }
 

compiler/rustc_mir_build/src/build/coverageinfo.rs

@@ -4,7 +4,8 @@ use std::collections::VecDeque;
 
 use rustc_data_structures::fx::FxHashMap;
 use rustc_middle::mir::coverage::{
-    BlockMarkerId, BranchSpan, ConditionId, ConditionInfo, CoverageKind, DecisionSpan,
+    BlockMarkerId, BranchSpan, ConditionId, ConditionInfo, CoverageKind, MCDCBranchSpan,
+    MCDCDecisionSpan,
 };
 use rustc_middle::mir::{self, BasicBlock, UnOp};
 use rustc_middle::thir::{ExprId, ExprKind, LogicalOp, Thir};
@@ -21,7 +22,8 @@ pub(crate) struct BranchInfoBuilder {
 
     num_block_markers: usize,
     branch_spans: Vec<BranchSpan>,
-    decision_spans: Vec<DecisionSpan>,
+    mcdc_branch_spans: Vec<MCDCBranchSpan>,
+    mcdc_decision_spans: Vec<MCDCDecisionSpan>,
     mcdc_state: Option<MCDCState>,
 }
 
@@ -44,7 +46,8 @@ impl BranchInfoBuilder {
                 nots: FxHashMap::default(),
                 num_block_markers: 0,
                 branch_spans: vec![],
-                decision_spans: vec![],
+                mcdc_branch_spans: vec![],
+                mcdc_decision_spans: vec![],
                 mcdc_state: MCDCState::new_if_enabled(tcx),
             })
         } else {
@@ -103,10 +106,8 @@ impl BranchInfoBuilder {
         tcx: TyCtxt<'_>,
         true_marker: BlockMarkerId,
         false_marker: BlockMarkerId,
-    ) -> ConditionInfo {
-        let Some(mcdc_state) = self.mcdc_state.as_mut() else {
-            return ConditionInfo::default();
-        };
+    ) -> Option<ConditionInfo> {
+        let mcdc_state = self.mcdc_state.as_mut()?;
         let (mut condition_info, decision_result) =
             mcdc_state.take_condition(true_marker, false_marker);
         if let Some(decision) = decision_result {
@@ -115,17 +116,22 @@ impl BranchInfoBuilder {
                     unreachable!("Decision with no condition is not expected");
                 }
                 1..=MAX_CONDITIONS_NUM_IN_DECISION => {
-                    self.decision_spans.push(decision);
+                    self.mcdc_decision_spans.push(decision);
                 }
                 _ => {
                     // Do not generate mcdc mappings and statements for decisions with too many conditions.
-                    for branch in
-                        self.branch_spans.iter_mut().rev().take(decision.conditions_num - 1)
-                    {
-                        branch.condition_info = ConditionInfo::default();
-                    }
+                    let rebase_idx = self.mcdc_branch_spans.len() - decision.conditions_num + 1;
+                    let to_normal_branches = self.mcdc_branch_spans.split_off(rebase_idx);
+                    self.branch_spans.extend(to_normal_branches.into_iter().map(
+                        |MCDCBranchSpan { span, true_marker, false_marker, .. }| BranchSpan {
+                            span,
+                            true_marker,
+                            false_marker,
+                        },
+                    ));
+
                     // ConditionInfo of this branch shall also be reset.
-                    condition_info = ConditionInfo::default();
+                    condition_info = None;
 
                     tcx.dcx().emit_warn(MCDCExceedsConditionNumLimit {
                         span: decision.span,
@@ -145,7 +151,14 @@ impl BranchInfoBuilder {
     }
 
     pub(crate) fn into_done(self) -> Option<Box<mir::coverage::BranchInfo>> {
-        let Self { nots: _, num_block_markers, branch_spans, decision_spans, .. } = self;
+        let Self {
+            nots: _,
+            num_block_markers,
+            branch_spans,
+            mcdc_branch_spans,
+            mcdc_decision_spans,
+            ..
+        } = self;
 
         if num_block_markers == 0 {
             assert!(branch_spans.is_empty());
@@ -155,7 +168,8 @@ impl BranchInfoBuilder {
         Some(Box::new(mir::coverage::BranchInfo {
             num_block_markers,
             branch_spans,
-            decision_spans,
+            mcdc_branch_spans,
+            mcdc_decision_spans,
         }))
     }
 }
@@ -168,7 +182,7 @@ const MAX_CONDITIONS_NUM_IN_DECISION: usize = 6;
 struct MCDCState {
     /// To construct condition evaluation tree.
     decision_stack: VecDeque<ConditionInfo>,
-    processing_decision: Option<DecisionSpan>,
+    processing_decision: Option<MCDCDecisionSpan>,
 }
 
 impl MCDCState {
@@ -224,10 +238,10 @@ impl MCDCState {
                 decision.span = decision.span.to(span);
                 decision
             }
-            None => self.processing_decision.insert(DecisionSpan {
+            None => self.processing_decision.insert(MCDCDecisionSpan {
                 span,
                 conditions_num: 0,
-                end_marker: vec![],
+                end_markers: vec![],
             }),
         };
 
@@ -279,24 +293,24 @@ impl MCDCState {
         &mut self,
         true_marker: BlockMarkerId,
         false_marker: BlockMarkerId,
-    ) -> (ConditionInfo, Option<DecisionSpan>) {
+    ) -> (Option<ConditionInfo>, Option<MCDCDecisionSpan>) {
         let Some(condition_info) = self.decision_stack.pop_back() else {
-            return (ConditionInfo::default(), None);
+            return (None, None);
         };
         let Some(decision) = self.processing_decision.as_mut() else {
             bug!("Processing decision should have been created before any conditions are taken");
         };
         if condition_info.true_next_id == ConditionId::NONE {
-            decision.end_marker.push(true_marker);
+            decision.end_markers.push(true_marker);
         }
         if condition_info.false_next_id == ConditionId::NONE {
-            decision.end_marker.push(false_marker);
+            decision.end_markers.push(false_marker);
         }
 
         if self.decision_stack.is_empty() {
-            (condition_info, self.processing_decision.take())
+            (Some(condition_info), self.processing_decision.take())
         } else {
-            (condition_info, None)
+            (Some(condition_info), None)
         }
     }
 }
@@ -341,14 +355,22 @@ impl Builder<'_, '_> {
         let true_marker = inject_branch_marker(then_block);
         let false_marker = inject_branch_marker(else_block);
 
-        let condition_info = branch_info.fetch_condition_info(self.tcx, true_marker, false_marker);
-
-        branch_info.branch_spans.push(BranchSpan {
-            span: source_info.span,
-            condition_info,
-            true_marker,
-            false_marker,
-        });
+        if let Some(condition_info) =
+            branch_info.fetch_condition_info(self.tcx, true_marker, false_marker)
+        {
+            branch_info.mcdc_branch_spans.push(MCDCBranchSpan {
+                span: source_info.span,
+                condition_info,
+                true_marker,
+                false_marker,
+            });
+        } else {
+            branch_info.branch_spans.push(BranchSpan {
+                span: source_info.span,
+                true_marker,
+                false_marker,
+            });
+        }
     }
 
     pub(crate) fn visit_coverage_branch_operation(&mut self, logical_op: LogicalOp, span: Span) {