coverage. Adopt nicer code style

Author: zhuyunxing

compiler/rustc_codegen_llvm/src/coverageinfo/ffi.rs

@@ -115,7 +115,7 @@ pub mod mcdc {
 
     /// Must match the layout of `LLVMRustMCDCDecisionParameters`.
     #[repr(C)]
-    #[derive(Clone, Copy, Debug)]
+    #[derive(Clone, Copy, Debug, Default)]
     pub struct DecisionParameters {
         bitmap_idx: u32,
         conditions_num: u16,
@@ -126,19 +126,47 @@ pub mod mcdc {
 
     /// Must match the layout of `LLVMRustMCDCBranchParameters`.
     #[repr(C)]
-    #[derive(Clone, Copy, Debug)]
+    #[derive(Clone, Copy, Debug, Default)]
     pub struct BranchParameters {
         condition_id: LLVMConditionId,
         condition_ids: [LLVMConditionId; 2],
     }
 
-    /// Same layout with `LLVMRustMCDCParameters`
     #[repr(C, u8)]
-    #[derive(Clone, Copy, Debug)]
-    pub enum Parameters {
+    pub enum ParameterTag {
         None,
-        Decision(DecisionParameters),
-        Branch(BranchParameters),
+        Decision,
+        Branch,
+    }
+    /// Same layout with `LLVMRustMCDCParameters`
+    #[repr(C)]
+    #[derive(Clone, Copy, Debug)]
+    pub struct Parameters {
+        tag: ParameterTag,
+        decision_params: DecisionParameters,
+        branch_params: BranchParameters,
+    }
+
+    impl Parameters {
+        pub fn none() -> Self {
+            Self {
+                tag: ParameterTag::None,
+                decision_params: Default::default(),
+                branch_params: Default::default(),
+            }
+        }
+        pub fn decision(decision_params: DecisionParameters) -> Self {
+            Self { tag: ParameterTag::Decision, decision_params, branch_params: Default::default() }
+        }
+        pub fn branch(branch_params: BranchParameters) -> Self {
+            Self { tag: ParameterTag::Branch, decision_params: Default, branch_params }
+        }
+    }
+
+    impl Default for Parameters {
+        fn default() -> Self {
+            Self::none()
+        }
     }
 
     impl From<ConditionInfo> for BranchParameters {
@@ -287,14 +315,9 @@ impl CounterMappingRegion {
         end_line: u32,
         end_col: u32,
     ) -> Self {
-        let mcdc_params = condition_info
-            .map(mcdc::BranchParameters::from)
-            .map(mcdc::Parameters::Branch)
-            .unwrap_or(mcdc::Parameters::None);
-        let kind = match mcdc_params {
-            mcdc::Parameters::None => RegionKind::BranchRegion,
-            mcdc::Parameters::Branch(_) => RegionKind::MCDCBranchRegion,
-            _ => unreachable!("invalid mcdc params for branch"),
+        let (kind, mcdc_params) = match condition_info {
+            Some(info) => (RegionKind::MCDCBranchRegion, mcdc::Parameters::branch(info.into())),
+            None => (RegionKind::BranchRegion, Default::default()),
         };
         Self {
             counter,

compiler/rustc_codegen_llvm/src/coverageinfo/mod.rs

@@ -188,20 +188,20 @@ fn ensure_mcdc_parameters<'ll, 'tcx>(
     instance: Instance<'tcx>,
     function_coverage_info: &FunctionCoverageInfo,
 ) {
-    if bx
-        .coverage_context()
-        .is_some_and(|cx| !cx.mcdc_condition_bitmap_map.borrow().contains_key(&instance))
-    {
-        let fn_name = bx.get_pgo_func_name_var(instance);
-        let hash = bx.const_u64(function_coverage_info.function_source_hash);
-        let bitmap_bytes = bx.const_u32(function_coverage_info.mcdc_bitmap_bytes);
-        let cond_bitmap = bx.mcdc_parameters(fn_name, hash, bitmap_bytes);
-        bx.coverage_context()
-            .unwrap()
-            .mcdc_condition_bitmap_map
-            .borrow_mut()
-            .insert(instance, cond_bitmap);
+    let Some(cx) = bx.coverage_context() else { return };
+    if cx.mcdc_condition_bitmap_map.borrow().contains_key(&instance) {
+        return;
     }
+    
+    let fn_name = bx.get_pgo_func_name_var(instance);
+    let hash = bx.const_u64(function_coverage_info.function_source_hash);
+    let bitmap_bytes = bx.const_u32(function_coverage_info.mcdc_bitmap_bytes);
+    let cond_bitmap = bx.mcdc_parameters(fn_name, hash, bitmap_bytes);
+    bx.coverage_context()
+        .expect("already checked above")
+        .mcdc_condition_bitmap_map
+        .borrow_mut()
+        .insert(instance, cond_bitmap);
 }
 
 /// Calls llvm::createPGOFuncNameVar() with the given function instance's

compiler/rustc_llvm/llvm-wrapper/CoverageMappingWrapper.cpp

@@ -82,16 +82,18 @@ struct LLVMRustMCDCBranchParameters {
   int16_t ConditionIDs[2];
 };
 
-union LLVMRustMCDCParametersPayload {
-  LLVMRustMCDCDecisionParameters DecisionParameters;
-  LLVMRustMCDCBranchParameters BranchParameters;
-};
-
 struct LLVMRustMCDCParameters {
   LLVMRustMCDCParametersTag Tag;
-  LLVMRustMCDCParametersPayload Payload;
+  LLVMRustMCDCDecisionParameters DecisionParameters;
+  LLVMRustMCDCBranchParameters BranchParameters;
 };
 
+// LLVM representations for `MCDCParameters` evolved from LLVM 18 to 19.
+// Look at representations in 18
+// https://github.com/rust-lang/llvm-project/blob/66a2881a/llvm/include/llvm/ProfileData/Coverage/CoverageMapping.h#L253-L263
+// and representations in 19
+// https://github.com/llvm/llvm-project/blob/843cc474faefad1d639f4c44c1cf3ad7dbda76c8/llvm/include/llvm/ProfileData/Coverage/MCDCTypes.h
+#if LLVM_VERSION_GE(18, 0) && LLVM_VERSION_LT(19, 0)
 static coverage::CounterMappingRegion::MCDCParameters
 fromRust(LLVMRustMCDCParameters Params) {
   auto parameter = coverage::CounterMappingRegion::MCDCParameters{};
@@ -117,6 +119,27 @@ fromRust(LLVMRustMCDCParameters Params) {
   }
   report_fatal_error("Bad LLVMRustMCDCParametersTag!");
 }
+#elif LLVM_VERSION_GE(19, 0)
+static coverage::mcdc::MCDCParameters fromRust(LLVMRustMCDCParameters Params) {
+  switch (Params.Tag) {
+  case LLVMRustMCDCParametersTag::None:
+    return coverage::mcdc::MCDCParameters;
+  case LLVMRustMCDCParametersTag::Decision:
+    return coverage::mcdc::DecisionParameters(
+        Params.Payload.DecisionParameters.BitmapIdx,
+        Params.Payload.DecisionParameters.NumConditions);
+  case LLVMRustMCDCParametersTag::Branch:
+    return coverage::mcdc::BranchParameters(
+        static_cast<coverage::mcdc::ConditionID>(
+            Params.Payload.BranchParameters.ConditionID),
+        {static_cast<coverage::CounterMappingRegion::MCDCConditionID>(
+             Params.Payload.BranchParameters.ConditionIDs[0]),
+         static_cast<coverage::CounterMappingRegion::MCDCConditionID>(
+             Params.Payload.BranchParameters.ConditionIDs[1])});
+  }
+  report_fatal_error("Bad LLVMRustMCDCParametersTag!");
+}
+#endif
 
 // FFI equivalent of struct `llvm::coverage::CounterMappingRegion`
 // https://github.com/rust-lang/llvm-project/blob/ea6fa9c2/llvm/include/llvm/ProfileData/Coverage/CoverageMapping.h#L211-L304
@@ -191,7 +214,7 @@ extern "C" void LLVMRustCoverageWriteMappingToBuffer(
            RustMappingRegions, NumMappingRegions)) {
     MappingRegions.emplace_back(
         fromRust(Region.Count), fromRust(Region.FalseCount),
-#if LLVM_VERSION_GE(18, 0) && LLVM_VERSION_LT(19, 0)
+#if LLVM_VERSION_GE(18, 0)
         fromRust(Region.MCDCParameters),
 #endif
         Region.FileID, Region.ExpandedFileID, // File IDs, then region info.

compiler/rustc_middle/src/mir/coverage.rs

@@ -225,7 +225,7 @@ impl MappingKind {
         let two = |a, b| Some(a).into_iter().chain(Some(b));
         match *self {
             Self::Code(term) => one(term),
-            Self::Branch { true_term, false_term, .. } => two(true_term, false_term),
+            Self::Branch { true_term, false_term } => two(true_term, false_term),
             Self::MCDCBranch { true_term, false_term, .. } => two(true_term, false_term),
             Self::MCDCDecision(_) => zero(),
         }

compiler/rustc_mir_transform/src/coverage/mod.rs

@@ -233,12 +233,10 @@ fn inject_mcdc_statements<'tcx>(
         return;
     }
 
-    // Inject test vector update first because inject statement always inject new statement at head.
+    // Inject test vector update first because `inject_statement` always insert new statement at head.
     for (end_bcbs, bitmap_idx) in
         coverage_spans.all_bcb_mappings().filter_map(|mapping| match &mapping.kind {
-            BcbMappingKind::Decision { end_bcbs: end_bcb, bitmap_idx, .. } => {
-                Some((end_bcb, *bitmap_idx))
-            }
+            BcbMappingKind::Decision { end_bcbs, bitmap_idx, .. } => Some((end_bcbs, *bitmap_idx)),
             _ => None,
         })
     {

compiler/rustc_session/src/options.rs

@@ -947,13 +947,12 @@ mod parse {
             match option {
                 "no-branch" => {
                     slot.branch = false;
+                    slot.mcdc = false;
                 }
-                "branch" => {
-                    slot.branch = true;
-                }
+                "branch" => slot.branch = true,
                 "mcdc" => {
                     slot.branch = true;
-                    slot.mcdc = true
+                    slot.mcdc = true;
                 }
                 _ => return false,
             }

src/doc/unstable-book/src/compiler-flags/coverage-options.md

@@ -5,4 +5,4 @@ This option controls details of the coverage instrumentation performed by
 
 Multiple options can be passed, separated by commas. Valid options are:
 
-- `no-branch`, `branch` or `mcdc`: `branch` enables branch coverage instrumentation and `mcdc` further enables modified condition/decision coverage instrumentation. `no-branch` disables branch coverage instrumentation, which is same as do not pass `branch` or `mcdc`
+- `no-branch`, `branch` or `mcdc`: `branch` enables branch coverage instrumentation and `mcdc` further enables modified condition/decision coverage instrumentation. `no-branch` disables branch coverage instrumentation as well as mcdc instrumentation, which is same as do not pass `branch` or `mcdc`.