mcdc-coverage(instrumentation): Rename Id names, Start instrumentation implementation

Author: RenjiSann

compiler/rustc_middle/src/mir/coverage.rs

@@ -15,6 +15,23 @@ rustc_index::newtype_index! {
     pub struct BlockMarkerId {}
 }
 
+// DecisionMarkerId and ConditionMarkerId are used between THIR and MIR representations.
+// DecisionId and ConditionId are used between MIR representation and LLVM-IR.
+
+rustc_index::newtype_index! {
+    #[derive(HashStable)]
+    #[encodable]
+    #[debug_format = "DecisionMarkerId({})"]
+    pub struct DecisionMarkerId {}
+}
+
+rustc_index::newtype_index! {
+    #[derive(HashStable)]
+    #[encodable]
+    #[debug_format = "ConditionMarkerId({})"]
+    pub struct ConditionMarkerId {}
+}
+
 rustc_index::newtype_index! {
     #[derive(HashStable)]
     #[encodable]
@@ -115,27 +132,31 @@ pub enum CoverageKind {
     /// conditions in the same decision will reference this id.
     ///
     /// Has no effect during codegen.
-    MCDCDecisionEntryMarker { id: DecisionId },
+    MCDCDecisionEntryMarker { decm_id: DecisionMarkerId },
 
     /// Marks one of the basic blocks following the decision referenced with `id`.
     /// the outcome bool designates which branch of the decision it is:
     /// `true` for the then block, `false` for the else block.
     ///
     /// Has no effect during codegen.
-    MCDCDecisionOutputMarker { id: DecisionId, outcome: bool },
+    MCDCDecisionOutputMarker { decm_id: DecisionMarkerId, outcome: bool },
 
     /// Marks a basic block where a condition evaluation occurs
     /// The block may end with a SwitchInt where the 2 successors BBs have a
     /// MCDCConditionOutcomeMarker statement with a matching ID.
     ///
     /// Has no effect during codegen.
-    MCDCConditionEntryMarker { decision_id: DecisionId, id: ConditionId },
+    MCDCConditionEntryMarker { decm_id: DecisionMarkerId, condm_id: ConditionMarkerId },
 
     /// Marks a basic block that is branched from a condition evaluation.
     /// The block may have a predecessor with a matching ID.
     ///
     /// Has no effect during codegen.
-    MCDCConditionOutputMarker { decision_id: DecisionId, id: ConditionId, outcome: bool },
+    MCDCConditionOutputMarker {
+        decm_id: DecisionMarkerId,
+        condm_id: ConditionMarkerId,
+        outcome: bool,
+    },
 
     /// Marks the point in MIR control flow represented by a coverage counter.
     ///
@@ -178,20 +199,20 @@ impl Debug for CoverageKind {
         match self {
             SpanMarker => write!(fmt, "SpanMarker"),
             BlockMarker { id } => write!(fmt, "BlockMarker({:?})", id.index()),
-            MCDCDecisionEntryMarker { id } => {
+            MCDCDecisionEntryMarker { decm_id: id } => {
                 write!(fmt, "MCDCDecisionEntryMarker({:?})", id.index())
             }
-            MCDCDecisionOutputMarker { id, outcome } => {
+            MCDCDecisionOutputMarker { decm_id: id, outcome } => {
                 write!(fmt, "MCDCDecisionOutputMarker({:?}, {})", id.index(), outcome)
             }
-            MCDCConditionEntryMarker { decision_id, id } => {
+            MCDCConditionEntryMarker { decm_id: decision_id, condm_id: id } => {
                 write!(fmt, "MCDCConditionEntryMarker({:?}, {:?})", decision_id.index(), id.index())
             }
-            MCDCConditionOutputMarker { decision_id, id, outcome } => {
+            MCDCConditionOutputMarker { decm_id: decision_marker_id, condm_id: id, outcome } => {
                 write!(
                     fmt,
                     "MCDCConditionOutcomeMarker({:?}, {:?}, {})",
-                    decision_id.index(),
+                    decision_marker_id.index(),
                     id.index(),
                     outcome
                 )
@@ -323,7 +344,7 @@ pub struct BranchInfo {
 
     /// Associate a span for every decision in the function body.
     /// Empty if MCDC coverage is disabled.
-    pub decision_spans: IndexVec<DecisionId, DecisionSpan>,
+    pub decision_spans: IndexVec<DecisionMarkerId, DecisionSpan>,
 }
 
 #[derive(Clone, Debug)]
@@ -342,7 +363,7 @@ pub struct BranchSpan {
     pub span: Span,
     /// ID of Decision structure the branch is part of. Only used in
     /// the MCDC coverage.
-    pub decision_id: DecisionId,
+    pub decision_id: DecisionMarkerId,
     pub true_marker: BlockMarkerId,
     pub false_marker: BlockMarkerId,
 }

compiler/rustc_middle/src/ty/structural_impls.rs

@@ -406,6 +406,8 @@ TrivialTypeTraversalImpls! {
     ::rustc_hir::MatchSource,
     ::rustc_target::asm::InlineAsmRegOrRegClass,
     crate::mir::coverage::BlockMarkerId,
+    crate::mir::coverage::DecisionMarkerId,
+    crate::mir::coverage::ConditionMarkerId,
     crate::mir::coverage::DecisionId,
     crate::mir::coverage::ConditionId,
     crate::mir::coverage::CounterId,

compiler/rustc_mir_build/src/build/coverageinfo.rs

@@ -4,7 +4,7 @@ use std::collections::hash_map::Entry;
 use rustc_data_structures::fx::FxHashMap;
 use rustc_index::IndexVec;
 use rustc_middle::mir::coverage::{
-    BlockMarkerId, BranchSpan, ConditionId, CoverageKind, DecisionId, DecisionSpan,
+    BlockMarkerId, BranchSpan, ConditionMarkerId, CoverageKind, DecisionMarkerId, DecisionSpan,
 };
 use rustc_middle::mir::{self, BasicBlock, UnOp};
 use rustc_middle::thir::{ExprId, ExprKind, Thir};
@@ -19,12 +19,12 @@ struct MCDCInfoBuilder {
     /// ID of the current decision.
     /// Do not use directly. Use the function instead, as it will hide
     /// the decision in the scope of nested decisions.
-    current_decision_id: Option<DecisionId>,
+    current_decision_id: Option<DecisionMarkerId>,
     /// Track the nesting level of decision to avoid MCDC instrumentation of
     /// nested decisions.
     nested_decision_level: u32,
     /// Vector for storing all the decisions with their span
-    decision_spans: IndexVec<DecisionId, Span>,
+    decision_spans: IndexVec<DecisionMarkerId, Span>,
 
     next_condition_id: u32,
 }
@@ -52,7 +52,7 @@ impl MCDCInfoBuilder {
         self.nested_decision_level > 1
     }
 
-    pub fn current_decision_id(&self) -> Option<DecisionId> {
+    pub fn current_decision_id(&self) -> Option<DecisionMarkerId> {
         if self.in_nested_condition() { None } else { self.current_decision_id }
     }
 
@@ -235,7 +235,7 @@ impl Builder<'_, '_> {
                 .mcdc_info
                 .as_ref()
                 .and_then(|mcdc_info| mcdc_info.current_decision_id())
-                .unwrap_or(DecisionId::from_u32(0)),
+                .unwrap_or(DecisionMarkerId::from_u32(0)),
             true_marker,
             false_marker,
         });
@@ -268,7 +268,7 @@ impl Builder<'_, '_> {
         let marker_statement = mir::Statement {
             source_info,
             kind: mir::StatementKind::Coverage(CoverageKind::MCDCDecisionEntryMarker {
-                id: decision_id,
+                decm_id: decision_id,
             }),
         };
         self.cfg.push(block, marker_statement);
@@ -290,43 +290,31 @@ impl Builder<'_, '_> {
     /// If MCDC is enabled and the current decision is being instrumented,
     /// inject an `MCDCDecisionOutputMarker` to the given basic block.
     /// `outcome` should be true for the then block and false for the else block.
-    pub(crate) fn mcdc_decision_outcome_block(
-        &mut self,
-        bb: BasicBlock,
-        outcome: bool,
-    ) -> BasicBlock {
+    pub(crate) fn mcdc_decision_outcome_block(&mut self, bb: BasicBlock, outcome: bool) {
         // Get the MCDCInfoBuilder object, which existence implies that MCDC is enabled.
         let Some(BranchInfoBuilder { mcdc_info: Some(mcdc_info), .. }) =
             self.coverage_branch_info.as_mut()
         else {
-            return bb;
+            return;
         };
 
         let Some(decision_id) = mcdc_info.current_decision_id() else {
             // Decision is not instrumented
-            return bb;
+            return;
         };
 
         let span = mcdc_info.decision_spans[decision_id];
         let source_info = self.source_info(span);
         let marker_statement = mir::Statement {
             source_info,
             kind: mir::StatementKind::Coverage(CoverageKind::MCDCDecisionOutputMarker {
-                id: decision_id,
+                decm_id: decision_id,
                 outcome,
             }),
         };
 
         // Insert statements at the beginning of the following basic block
         self.cfg.block_data_mut(bb).statements.insert(0, marker_statement);
-
-        // Create a new block to return
-        let new_bb = self.cfg.start_new_block();
-
-        // Set bb -> new_bb
-        self.cfg.goto(bb, source_info, new_bb);
-
-        new_bb
     }
 
     /// Add markers on the condition's basic blocks to ease the later MCDC instrumentation.
@@ -344,13 +332,12 @@ impl Builder<'_, '_> {
             return;
         };
 
-        let Some(decision_id) = mcdc_info.current_decision_id() else {
+        let Some(decm_id) = mcdc_info.current_decision_id() else {
             // If current_decision_id() is None, the decision is not instrumented.
             return;
         };
 
-
-        let id = ConditionId::from_u32(mcdc_info.next_condition_id());
+        let condm_id = ConditionMarkerId::from_u32(mcdc_info.next_condition_id());
         let span = self.thir[condition_expr].span;
         let source_info = self.source_info(span);
 
@@ -362,15 +349,15 @@ impl Builder<'_, '_> {
 
         inject_statement(
             condition_block,
-            CoverageKind::MCDCConditionEntryMarker { decision_id, id },
+            CoverageKind::MCDCConditionEntryMarker { decm_id, condm_id },
         );
         inject_statement(
             then_block,
-            CoverageKind::MCDCConditionOutputMarker { decision_id, id, outcome: true },
+            CoverageKind::MCDCConditionOutputMarker { decm_id, condm_id, outcome: true },
         );
         inject_statement(
             else_block,
-            CoverageKind::MCDCConditionOutputMarker { decision_id, id, outcome: false },
+            CoverageKind::MCDCConditionOutputMarker { decm_id, condm_id, outcome: false },
         );
     }
 }

compiler/rustc_mir_build/src/build/expr/into.rs

@@ -92,7 +92,7 @@ impl<'a, 'tcx> Builder<'a, 'tcx> {
                                 ));
 
                                 // If MCDC is enabled, inject an outcome marker.
-                                let then_blk = this.mcdc_decision_outcome_block(then_blk, true);
+                                this.mcdc_decision_outcome_block(then_blk, true);
 
                                 // Lower the `then` arm into its block.
                                 this.expr_into_dest(destination, then_blk, then)
@@ -101,7 +101,7 @@ impl<'a, 'tcx> Builder<'a, 'tcx> {
                         // If MCDC is enabled and decision was instrumented, exit the
                         // decision scope and inject an MCDC decision output marker
                         // in the then and else blocks.
-                        let else_block = this.mcdc_decision_outcome_block(else_block, false);
+                        this.mcdc_decision_outcome_block(else_block, false);
                         this.end_mcdc_decision_coverage();
 
                         // Pack `(then_block, else_block)` into `BlockAnd<BasicBlock>`.