coverage. Change assign statements cl does not support on c++17 and fix code according to reviews

Author: Lambdaris

compiler/rustc_codegen_gcc/src/builder.rs

@@ -25,7 +25,7 @@ use rustc_middle::ty::layout::{
     FnAbiError, FnAbiOfHelpers, FnAbiRequest, HasParamEnv, HasTyCtxt, LayoutError, LayoutOfHelpers,
     TyAndLayout,
 };
-use rustc_middle::ty::{ParamEnv, Ty, TyCtxt, Instance};
+use rustc_middle::ty::{Instance, ParamEnv, Ty, TyCtxt};
 use rustc_span::def_id::DefId;
 use rustc_span::Span;
 use rustc_target::abi::{
@@ -1737,6 +1737,41 @@ impl<'a, 'gcc, 'tcx> BuilderMethods<'a, 'tcx> for Builder<'a, 'gcc, 'tcx> {
     ) {
         unimplemented!();
     }
+
+    fn mcdc_parameters(
+        &mut self,
+        _fn_name: Self::Value,
+        _hash: Self::Value,
+        _bitmap_bytes: Self::Value,
+    ) -> Self::Value {
+        unimplemented!();
+    }
+
+    fn mcdc_tvbitmap_update(
+        &mut self,
+        _fn_name: Self::Value,
+        _hash: Self::Value,
+        _bitmap_bytes: Self::Value,
+        _bitmap_index: Self::Value,
+        _mcdc_temp: Self::Value,
+    ) {
+        unimplemented!();
+    }
+
+    fn mcdc_condbitmap_update(
+        &mut self,
+        _fn_name: Self::Value,
+        _hash: Self::Value,
+        _cond_loc: Self::Value,
+        _mcdc_temp: Self::Value,
+        _bool_value: Self::Value,
+    ) {
+        unimplemented!();
+    }
+
+    fn mcdc_condbitmap_reset(&mut self, _mcdc_temp: Self::Value) {
+        unimplemented!();
+    }
 }
 
 impl<'a, 'gcc, 'tcx> Builder<'a, 'gcc, 'tcx> {

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,38 +82,64 @@ 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{};
   switch (Params.Tag) {
   case LLVMRustMCDCParametersTag::None:
-    return coverage::CounterMappingRegion::MCDCParameters{};
+    return parameter;
   case LLVMRustMCDCParametersTag::Decision:
-    return coverage::CounterMappingRegion::MCDCParameters{
-        .BitmapIdx =
-            static_cast<unsigned>(Params.Payload.DecisionParameters.BitmapIdx),
-        .NumConditions = static_cast<unsigned>(
-            Params.Payload.DecisionParameters.NumConditions)};
+    parameter.BitmapIdx =
+        static_cast<unsigned>(Params.Payload.DecisionParameters.BitmapIdx),
+    parameter.NumConditions =
+        static_cast<unsigned>(Params.Payload.DecisionParameters.NumConditions);
+    return parameter;
   case LLVMRustMCDCParametersTag::Branch:
-    return coverage::CounterMappingRegion::MCDCParameters{
-        .ID = static_cast<coverage::CounterMappingRegion::MCDCConditionID>(
-            Params.Payload.BranchParameters.ConditionID),
-        .FalseID = static_cast<coverage::CounterMappingRegion::MCDCConditionID>(
+    parameter.ID = static_cast<coverage::CounterMappingRegion::MCDCConditionID>(
+        Params.Payload.BranchParameters.ConditionID),
+    parameter.FalseID =
+        static_cast<coverage::CounterMappingRegion::MCDCConditionID>(
             Params.Payload.BranchParameters.ConditionIDs[0]),
-        .TrueID = static_cast<coverage::CounterMappingRegion::MCDCConditionID>(
-            Params.Payload.BranchParameters.ConditionIDs[1])};
+    parameter.TrueID =
+        static_cast<coverage::CounterMappingRegion::MCDCConditionID>(
+            Params.Payload.BranchParameters.ConditionIDs[1]);
+    return parameter;
+  }
+  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
@@ -188,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

@@ -142,27 +142,24 @@ fn create_mappings<'tcx>(
 
     mappings.extend(coverage_spans.all_bcb_mappings().filter_map(
         |BcbMapping { kind: bcb_mapping_kind, span }| {
-            let kind = match bcb_mapping_kind {
-                BcbMappingKind::Code(bcb) => MappingKind::Code(term_for_bcb(*bcb)),
+            let kind = match *bcb_mapping_kind {
+                BcbMappingKind::Code(bcb) => MappingKind::Code(term_for_bcb(bcb)),
                 BcbMappingKind::Branch { true_bcb, false_bcb, condition_info } => {
                     if condition_info.condition_id == ConditionId::NONE {
                         MappingKind::Branch {
-                            true_term: term_for_bcb(*true_bcb),
-                            false_term: term_for_bcb(*false_bcb),
+                            true_term: term_for_bcb(true_bcb),
+                            false_term: term_for_bcb(false_bcb),
                         }
                     } else {
                         MappingKind::MCDCBranch {
-                            true_term: term_for_bcb(*true_bcb),
-                            false_term: term_for_bcb(*false_bcb),
-                            mcdc_params: *condition_info,
+                            true_term: term_for_bcb(true_bcb),
+                            false_term: term_for_bcb(false_bcb),
+                            mcdc_params: condition_info,
                         }
                     }
                 }
                 BcbMappingKind::Decision { bitmap_idx, conditions_num, .. } => {
-                    MappingKind::MCDCDecision(DecisionInfo {
-                        bitmap_idx: *bitmap_idx,
-                        conditions_num: *conditions_num,
-                    })
+                    MappingKind::MCDCDecision(DecisionInfo { bitmap_idx, conditions_num })
                 }
             };
             let code_region = make_code_region(source_map, file_name, *span, body_span)?;
@@ -236,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_mir_transform/src/coverage/spans.rs

@@ -100,18 +100,18 @@ pub(super) fn generate_coverage_spans(
     };
     let mut test_vector_bitmap_bytes = 0;
     for BcbMapping { kind, span: _ } in &mappings {
-        match kind {
-            BcbMappingKind::Code(bcb) => insert(*bcb),
+        match *kind {
+            BcbMappingKind::Code(bcb) => insert(bcb),
             BcbMappingKind::Branch { true_bcb, false_bcb, .. } => {
-                insert(*true_bcb);
-                insert(*false_bcb);
+                insert(true_bcb);
+                insert(false_bcb);
             }
             BcbMappingKind::Decision { bitmap_idx, conditions_num, .. } => {
                 // `bcb_has_mappings` is used for inject coverage counters
                 // but they are not needed for decision BCBs.
                 // While the length of test vector bitmap should be calculated here.
                 test_vector_bitmap_bytes = test_vector_bitmap_bytes
-                    .max(bitmap_idx + (1_u32 << *conditions_num as u32).div_ceil(8));
+                    .max(bitmap_idx + (1_u32 << conditions_num as u32).div_ceil(8));
             }
         }
     }

compiler/rustc_session/src/options.rs

@@ -945,32 +945,20 @@ mod parse {
     pub(crate) fn parse_coverage_options(slot: &mut CoverageOptions, v: Option<&str>) -> bool {
         let Some(v) = v else { return true };
 
-        let set_branch_option = |slot: &mut CoverageOptions, option: &str| {
+        for option in v.split(',') {
             match option {
-                "no-branch" => slot.branch = false,
+                "no-branch" => {
+                    slot.branch = false;
+                    slot.mcdc = false;
+                }
                 "branch" => slot.branch = true,
                 "mcdc" => {
+                    slot.branch = true;
                     slot.mcdc = true;
-                    slot.branch = true
                 }
                 _ => return false,
             }
-            true
-        };
-
-        // Once an option is parsed we removed it from the array so that conflicting options such as "branch,no-branch" could be detected.
-        let mut parsers_set: [Option<&dyn Fn(&mut CoverageOptions, &str) -> bool>; 1] =
-            [Some(&set_branch_option)];
-
-        for option in v.split(',') {
-            if !parsers_set
-                .iter_mut()
-                .any(|p| p.is_some_and(|parser| parser(slot, option)).then(|| p.take()).is_some())
-            {
-                return false;
-            }
         }
-
         true
     }
 

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`.

src/tools/coverage-dump/src/covfun.rs

@@ -75,8 +75,6 @@ pub(crate) fn dump_covfun_mappings(
                         println!("    true  = {}", expression_resolver.format_term(r#true));
                         println!("    false = {}", expression_resolver.format_term(r#false));
                     }
-
-                    MappingKind::MCDCDecision { .. } => {}
                     _ => (),
                 }
             }