coverage: Rename conditions_num to num_conditions

This field counts the number of conditions that contribute to a particular decision, but the name "conditions num" sounds like an ID instead of a count, so "num conditions" is clearer.
rust-lang · May 1, 2024 · 15ca582 · 15ca582
1 parent b3e14bd
commit 15ca582
Show file tree

Hide file tree

Showing 10 changed files with 40 additions and 35 deletions.
diff --git a/compiler/rustc_codegen_llvm/src/coverageinfo/ffi.rs b/compiler/rustc_codegen_llvm/src/coverageinfo/ffi.rs
@@ -118,7 +118,7 @@ pub mod mcdc {
     #[derive(Clone, Copy, Debug, Default)]
     pub struct DecisionParameters {
         bitmap_idx: u32,
-        conditions_num: u16,
+        num_conditions: u16,
     }
 
     // ConditionId in llvm is `unsigned int` at 18 while `int16_t` at [19](https://github.com/llvm/llvm-project/pull/81257)
@@ -178,7 +178,7 @@ pub mod mcdc {
 
     impl From<DecisionInfo> for DecisionParameters {
         fn from(value: DecisionInfo) -> Self {
-            Self { bitmap_idx: value.bitmap_idx, conditions_num: value.conditions_num }
+            Self { bitmap_idx: value.bitmap_idx, num_conditions: value.num_conditions }
         }
     }
 }

diff --git a/compiler/rustc_middle/src/mir/coverage.rs b/compiler/rustc_middle/src/mir/coverage.rs
@@ -329,14 +329,14 @@ pub struct MCDCBranchSpan {
 #[derive(TyEncodable, TyDecodable, Hash, HashStable, TypeFoldable, TypeVisitable)]
 pub struct DecisionInfo {
     pub bitmap_idx: u32,
-    pub conditions_num: u16,
+    pub num_conditions: u16,
 }
 
 #[derive(Clone, Debug)]
 #[derive(TyEncodable, TyDecodable, Hash, HashStable, TypeFoldable, TypeVisitable)]
 pub struct MCDCDecisionSpan {
     pub span: Span,
-    pub conditions_num: usize,
+    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
@@ -511,12 +511,12 @@ fn write_coverage_branch_info(
         )?;
     }
 
-    for coverage::MCDCDecisionSpan { span, conditions_num, end_markers, decision_depth } in
+    for coverage::MCDCDecisionSpan { span, num_conditions, end_markers, decision_depth } in
         mcdc_decision_spans
     {
         writeln!(
             w,
-            "{INDENT}coverage mcdc decision {{ conditions_num: {conditions_num:?}, end: {end_markers:?}, depth: {decision_depth:?} }} => {span:?}"
+            "{INDENT}coverage mcdc decision {{ num_conditions: {num_conditions:?}, end: {end_markers:?}, depth: {decision_depth:?} }} => {span:?}"
         )?;
     }
 

diff --git a/compiler/rustc_mir_build/messages.ftl b/compiler/rustc_mir_build/messages.ftl
@@ -97,7 +97,9 @@ mir_build_deref_raw_pointer_requires_unsafe_unsafe_op_in_unsafe_fn_allowed =
     .note = raw pointers may be null, dangling or unaligned; they can violate aliasing rules and cause data races: all of these are undefined behavior
     .label = dereference of raw pointer
 
-mir_build_exceeds_mcdc_condition_num_limit =  Conditions number of the decision ({$conditions_num}) exceeds limit ({$max_conditions_num}). MCDC analysis will not count this expression.
+mir_build_exceeds_mcdc_condition_limit =
+    number of conditions in decision ({$num_conditions}) exceeds limit ({$limit})
+    .note = this decision will not be instrumented for MC/DC coverage
 
 mir_build_extern_static_requires_unsafe =
     use of extern static is unsafe and requires unsafe block

diff --git a/compiler/rustc_mir_build/src/build/coverageinfo/mcdc.rs b/compiler/rustc_mir_build/src/build/coverageinfo/mcdc.rs
@@ -9,12 +9,12 @@ use rustc_middle::ty::TyCtxt;
 use rustc_span::Span;
 
 use crate::build::Builder;
-use crate::errors::MCDCExceedsConditionNumLimit;
+use crate::errors::MCDCExceedsConditionLimit;
 
 /// The MCDC bitmap scales exponentially (2^n) based on the number of conditions seen,
-/// So llvm sets a maximum value prevents the bitmap footprint from growing too large without the user's knowledge.
-/// This limit may be relaxed if the [upstream change](https://github.com/llvm/llvm-project/pull/82448) is merged.
-const MAX_CONDITIONS_NUM_IN_DECISION: usize = 6;
+/// So LLVM imposes a limit to prevent the bitmap footprint from growing too large without the user's knowledge.
+/// This limit may be relaxed if [upstream change #82448](https://github.com/llvm/llvm-project/pull/82448) is merged.
+const MAX_CONDITIONS_IN_DECISION: usize = 6;
 
 #[derive(Default)]
 struct MCDCDecisionCtx {
@@ -99,22 +99,22 @@ impl MCDCState {
             }
             None => decision_ctx.processing_decision.insert(MCDCDecisionSpan {
                 span,
-                conditions_num: 0,
+                num_conditions: 0,
                 end_markers: vec![],
                 decision_depth,
             }),
         };
 
         let parent_condition = decision_ctx.decision_stack.pop_back().unwrap_or_default();
         let lhs_id = if parent_condition.condition_id == ConditionId::NONE {
-            decision.conditions_num += 1;
-            ConditionId::from(decision.conditions_num)
+            decision.num_conditions += 1;
+            ConditionId::from(decision.num_conditions)
         } else {
             parent_condition.condition_id
         };
 
-        decision.conditions_num += 1;
-        let rhs_condition_id = ConditionId::from(decision.conditions_num);
+        decision.num_conditions += 1;
+        let rhs_condition_id = ConditionId::from(decision.num_conditions);
 
         let (lhs, rhs) = match op {
             LogicalOp::And => {
@@ -207,27 +207,27 @@ impl MCDCInfoBuilder {
         // is empty, i.e. when all the conditions of the decision were instrumented,
         // and the decision is "complete".
         if let Some(decision) = decision_result {
-            match decision.conditions_num {
+            match decision.num_conditions {
                 0 => {
                     unreachable!("Decision with no condition is not expected");
                 }
-                1..=MAX_CONDITIONS_NUM_IN_DECISION => {
+                1..=MAX_CONDITIONS_IN_DECISION => {
                     self.decision_spans.push(decision);
                 }
                 _ => {
                     // Do not generate mcdc mappings and statements for decisions with too many conditions.
-                    let rebase_idx = self.branch_spans.len() - decision.conditions_num + 1;
+                    let rebase_idx = self.branch_spans.len() - decision.num_conditions + 1;
                     for branch in &mut self.branch_spans[rebase_idx..] {
                         branch.condition_info = None;
                     }
 
                     // ConditionInfo of this branch shall also be reset.
                     condition_info = None;
 
-                    tcx.dcx().emit_warn(MCDCExceedsConditionNumLimit {
+                    tcx.dcx().emit_warn(MCDCExceedsConditionLimit {
                         span: decision.span,
-                        conditions_num: decision.conditions_num,
-                        max_conditions_num: MAX_CONDITIONS_NUM_IN_DECISION,
+                        num_conditions: decision.num_conditions,
+                        limit: MAX_CONDITIONS_IN_DECISION,
                     });
                 }
             }

diff --git a/compiler/rustc_mir_build/src/errors.rs b/compiler/rustc_mir_build/src/errors.rs
@@ -819,12 +819,13 @@ pub struct NontrivialStructuralMatch<'tcx> {
 }
 
 #[derive(Diagnostic)]
-#[diag(mir_build_exceeds_mcdc_condition_num_limit)]
-pub(crate) struct MCDCExceedsConditionNumLimit {
+#[diag(mir_build_exceeds_mcdc_condition_limit)]
+#[note]
+pub(crate) struct MCDCExceedsConditionLimit {
     #[primary_span]
     pub span: Span,
-    pub conditions_num: usize,
-    pub max_conditions_num: usize,
+    pub num_conditions: usize,
+    pub limit: usize,
 }
 
 #[derive(Diagnostic)]

diff --git a/compiler/rustc_mir_transform/src/coverage/mappings.rs b/compiler/rustc_mir_transform/src/coverage/mappings.rs
@@ -36,7 +36,7 @@ pub(super) enum BcbMappingKind {
     MCDCDecision {
         end_bcbs: BTreeSet<BasicCoverageBlock>,
         bitmap_idx: u32,
-        conditions_num: u16,
+        num_conditions: u16,
         decision_depth: u16,
     },
 }
@@ -119,12 +119,12 @@ pub(super) fn generate_coverage_spans(
                 insert(true_bcb);
                 insert(false_bcb);
             }
-            BcbMappingKind::MCDCDecision { bitmap_idx, conditions_num, .. } => {
+            BcbMappingKind::MCDCDecision { bitmap_idx, num_conditions, .. } => {
                 // `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 << num_conditions as u32).div_ceil(8));
             }
         }
     }
@@ -255,13 +255,13 @@ pub(super) fn extract_mcdc_mappings(
             .collect::<Option<_>>()?;
 
         let bitmap_idx = next_bitmap_idx;
-        next_bitmap_idx += (1_u32 << decision.conditions_num).div_ceil(8);
+        next_bitmap_idx += (1_u32 << decision.num_conditions).div_ceil(8);
 
         Some(BcbMapping {
             kind: BcbMappingKind::MCDCDecision {
                 end_bcbs,
                 bitmap_idx,
-                conditions_num: decision.conditions_num as u16,
+                num_conditions: decision.num_conditions as u16,
                 decision_depth: decision.decision_depth,
             },
             span,

diff --git a/compiler/rustc_mir_transform/src/coverage/mod.rs b/compiler/rustc_mir_transform/src/coverage/mod.rs
@@ -172,8 +172,8 @@ fn create_mappings<'tcx>(
                     false_term: term_for_bcb(false_bcb),
                     mcdc_params,
                 },
-                BcbMappingKind::MCDCDecision { bitmap_idx, conditions_num, .. } => {
-                    MappingKind::MCDCDecision(DecisionInfo { bitmap_idx, conditions_num })
+                BcbMappingKind::MCDCDecision { bitmap_idx, num_conditions, .. } => {
+                    MappingKind::MCDCDecision(DecisionInfo { bitmap_idx, num_conditions })
                 }
             };
             let code_region = make_code_region(source_map, file_name, *span, body_span)?;

diff --git a/tests/ui/instrument-coverage/mcdc-condition-limit.bad.stderr b/tests/ui/instrument-coverage/mcdc-condition-limit.bad.stderr
@@ -1,8 +1,10 @@
-warning: Conditions number of the decision (7) exceeds limit (6). MCDC analysis will not count this expression.
+warning: number of conditions in decision (7) exceeds limit (6)
   --> $DIR/mcdc-condition-limit.rs:18:8
    |
 LL |     if a && b && c && d && e && f && g {
    |        ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^
+   |
+   = note: this decision will not be instrumented for MC/DC coverage
 
 warning: 1 warning emitted
 
diff --git a/tests/ui/instrument-coverage/mcdc-condition-limit.rs b/tests/ui/instrument-coverage/mcdc-condition-limit.rs
@@ -15,7 +15,7 @@ fn main() {
 #[cfg(bad)]
 fn main() {
     let [a, b, c, d, e, f, g] = <[bool; 7]>::default();
-    if a && b && c && d && e && f && g { //[bad]~ WARNING Conditions number of the decision
+    if a && b && c && d && e && f && g { //[bad]~ WARNING number of conditions in decision
         core::hint::black_box("hello");
     }
 }