1
Fork 0

Rollup merge of #125754 - Zalathar:conditions-num, r=lqd

coverage: Rename MC/DC `conditions_num` to `num_conditions`

Updated version of #124571, without the other changes that were split out into #125108 and #125700.

This value represents a quantity of conditions, not an ID, so the new spelling is more appropriate.

Some of the code touched by this PR could perhaps use some other changes, but I would prefer to keep this PR as a simple renaming and avoid scope creep.

`@rustbot` label +A-code-coverage
This commit is contained in:
Matthias Krüger 2024-05-30 10:23:09 +02:00 committed by GitHub
commit 479d6cafb7
No known key found for this signature in database
GPG key ID: B5690EEEBB952194
10 changed files with 34 additions and 33 deletions

View file

@ -118,7 +118,7 @@ pub mod mcdc {
#[derive(Clone, Copy, Debug, Default)] #[derive(Clone, Copy, Debug, Default)]
pub struct DecisionParameters { pub struct DecisionParameters {
bitmap_idx: u32, 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) // ConditionId in llvm is `unsigned int` at 18 while `int16_t` at [19](https://github.com/llvm/llvm-project/pull/81257)
@ -177,8 +177,9 @@ pub mod mcdc {
} }
impl From<DecisionInfo> for DecisionParameters { impl From<DecisionInfo> for DecisionParameters {
fn from(value: DecisionInfo) -> Self { fn from(info: DecisionInfo) -> Self {
Self { bitmap_idx: value.bitmap_idx, conditions_num: value.conditions_num } let DecisionInfo { bitmap_idx, num_conditions } = info;
Self { bitmap_idx, num_conditions }
} }
} }
} }

View file

@ -329,14 +329,14 @@ pub struct MCDCBranchSpan {
#[derive(TyEncodable, TyDecodable, Hash, HashStable, TypeFoldable, TypeVisitable)] #[derive(TyEncodable, TyDecodable, Hash, HashStable, TypeFoldable, TypeVisitable)]
pub struct DecisionInfo { pub struct DecisionInfo {
pub bitmap_idx: u32, pub bitmap_idx: u32,
pub conditions_num: u16, pub num_conditions: u16,
} }
#[derive(Clone, Debug)] #[derive(Clone, Debug)]
#[derive(TyEncodable, TyDecodable, Hash, HashStable, TypeFoldable, TypeVisitable)] #[derive(TyEncodable, TyDecodable, Hash, HashStable, TypeFoldable, TypeVisitable)]
pub struct MCDCDecisionSpan { pub struct MCDCDecisionSpan {
pub span: Span, pub span: Span,
pub conditions_num: usize, pub num_conditions: usize,
pub end_markers: Vec<BlockMarkerId>, pub end_markers: Vec<BlockMarkerId>,
pub decision_depth: u16, pub decision_depth: u16,
} }

View file

@ -512,12 +512,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 mcdc_decision_spans
{ {
writeln!( writeln!(
w, 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:?}"
)?; )?;
} }

View file

@ -97,7 +97,7 @@ 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 .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 .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 ({$max_conditions}). MC/DC analysis will not count this expression.
mir_build_extern_static_requires_unsafe = mir_build_extern_static_requires_unsafe =
use of extern static is unsafe and requires unsafe block use of extern static is unsafe and requires unsafe block

View file

@ -10,12 +10,12 @@ use rustc_middle::ty::TyCtxt;
use rustc_span::Span; use rustc_span::Span;
use crate::build::Builder; 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, /// 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. /// 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. /// 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; const MAX_CONDITIONS_IN_DECISION: usize = 6;
#[derive(Default)] #[derive(Default)]
struct MCDCDecisionCtx { struct MCDCDecisionCtx {
@ -100,7 +100,7 @@ impl MCDCState {
} }
None => decision_ctx.processing_decision.insert(MCDCDecisionSpan { None => decision_ctx.processing_decision.insert(MCDCDecisionSpan {
span, span,
conditions_num: 0, num_conditions: 0,
end_markers: vec![], end_markers: vec![],
decision_depth, decision_depth,
}), }),
@ -108,14 +108,14 @@ impl MCDCState {
let parent_condition = decision_ctx.decision_stack.pop_back().unwrap_or_default(); let parent_condition = decision_ctx.decision_stack.pop_back().unwrap_or_default();
let lhs_id = if parent_condition.condition_id == ConditionId::NONE { let lhs_id = if parent_condition.condition_id == ConditionId::NONE {
decision.conditions_num += 1; decision.num_conditions += 1;
ConditionId::from(decision.conditions_num) ConditionId::from(decision.num_conditions)
} else { } else {
parent_condition.condition_id parent_condition.condition_id
}; };
decision.conditions_num += 1; decision.num_conditions += 1;
let rhs_condition_id = ConditionId::from(decision.conditions_num); let rhs_condition_id = ConditionId::from(decision.num_conditions);
let (lhs, rhs) = match op { let (lhs, rhs) = match op {
LogicalOp::And => { LogicalOp::And => {
@ -208,17 +208,17 @@ impl MCDCInfoBuilder {
// is empty, i.e. when all the conditions of the decision were instrumented, // is empty, i.e. when all the conditions of the decision were instrumented,
// and the decision is "complete". // and the decision is "complete".
if let Some(decision) = decision_result { if let Some(decision) = decision_result {
match decision.conditions_num { match decision.num_conditions {
0 => { 0 => {
unreachable!("Decision with no condition is not expected"); unreachable!("Decision with no condition is not expected");
} }
1..=MAX_CONDITIONS_NUM_IN_DECISION => { 1..=MAX_CONDITIONS_IN_DECISION => {
self.decision_spans.push(decision); self.decision_spans.push(decision);
} }
_ => { _ => {
// Do not generate mcdc mappings and statements for decisions with too many conditions. // Do not generate mcdc mappings and statements for decisions with too many conditions.
// Therefore, first erase the condition info of the (N-1) previous branch spans. // Therefore, first erase the condition info of the (N-1) previous branch spans.
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..] { for branch in &mut self.branch_spans[rebase_idx..] {
branch.condition_info = None; branch.condition_info = None;
} }
@ -226,10 +226,10 @@ impl MCDCInfoBuilder {
// Then, erase this last branch span's info too, for a total of N. // Then, erase this last branch span's info too, for a total of N.
condition_info = None; condition_info = None;
tcx.dcx().emit_warn(MCDCExceedsConditionNumLimit { tcx.dcx().emit_warn(MCDCExceedsConditionLimit {
span: decision.span, span: decision.span,
conditions_num: decision.conditions_num, num_conditions: decision.num_conditions,
max_conditions_num: MAX_CONDITIONS_NUM_IN_DECISION, max_conditions: MAX_CONDITIONS_IN_DECISION,
}); });
} }
} }

View file

@ -812,12 +812,12 @@ pub struct NonEmptyNeverPattern<'tcx> {
} }
#[derive(Diagnostic)] #[derive(Diagnostic)]
#[diag(mir_build_exceeds_mcdc_condition_num_limit)] #[diag(mir_build_exceeds_mcdc_condition_limit)]
pub(crate) struct MCDCExceedsConditionNumLimit { pub(crate) struct MCDCExceedsConditionLimit {
#[primary_span] #[primary_span]
pub span: Span, pub span: Span,
pub conditions_num: usize, pub num_conditions: usize,
pub max_conditions_num: usize, pub max_conditions: usize,
} }
#[derive(Diagnostic)] #[derive(Diagnostic)]

View file

@ -48,7 +48,7 @@ pub(super) struct MCDCDecision {
pub(super) span: Span, pub(super) span: Span,
pub(super) end_bcbs: BTreeSet<BasicCoverageBlock>, pub(super) end_bcbs: BTreeSet<BasicCoverageBlock>,
pub(super) bitmap_idx: u32, pub(super) bitmap_idx: u32,
pub(super) conditions_num: u16, pub(super) num_conditions: u16,
pub(super) decision_depth: u16, pub(super) decision_depth: u16,
} }
@ -268,13 +268,13 @@ pub(super) fn extract_mcdc_mappings(
// the bitmap, rounded up to a whole number of bytes. // the bitmap, rounded up to a whole number of bytes.
// The decision's "bitmap index" points to its first byte in the bitmap. // The decision's "bitmap index" points to its first byte in the bitmap.
let bitmap_idx = *mcdc_bitmap_bytes; let bitmap_idx = *mcdc_bitmap_bytes;
*mcdc_bitmap_bytes += (1_u32 << decision.conditions_num).div_ceil(8); *mcdc_bitmap_bytes += (1_u32 << decision.num_conditions).div_ceil(8);
Some(MCDCDecision { Some(MCDCDecision {
span, span,
end_bcbs, end_bcbs,
bitmap_idx, bitmap_idx,
conditions_num: decision.conditions_num as u16, num_conditions: decision.num_conditions as u16,
decision_depth: decision.decision_depth, decision_depth: decision.decision_depth,
}) })
}, },

View file

@ -195,9 +195,9 @@ fn create_mappings<'tcx>(
)); ));
mappings.extend(mcdc_decisions.iter().filter_map( mappings.extend(mcdc_decisions.iter().filter_map(
|&mappings::MCDCDecision { span, bitmap_idx, conditions_num, .. }| { |&mappings::MCDCDecision { span, bitmap_idx, num_conditions, .. }| {
let code_region = region_for_span(span)?; let code_region = region_for_span(span)?;
let kind = MappingKind::MCDCDecision(DecisionInfo { bitmap_idx, conditions_num }); let kind = MappingKind::MCDCDecision(DecisionInfo { bitmap_idx, num_conditions });
Some(Mapping { kind, code_region }) Some(Mapping { kind, code_region })
}, },
)); ));
@ -269,7 +269,7 @@ fn inject_mcdc_statements<'tcx>(
span: _, span: _,
ref end_bcbs, ref end_bcbs,
bitmap_idx, bitmap_idx,
conditions_num: _, num_conditions: _,
decision_depth, decision_depth,
} in &extracted_mappings.mcdc_decisions } in &extracted_mappings.mcdc_decisions
{ {

View file

@ -1,4 +1,4 @@
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). MC/DC analysis will not count this expression.
--> $DIR/mcdc-condition-limit.rs:29:8 --> $DIR/mcdc-condition-limit.rs:29:8
| |
LL | if a && b && c && d && e && f && g { LL | if a && b && c && d && e && f && g {

View file

@ -26,7 +26,7 @@ fn main() {
fn main() { fn main() {
// 7 conditions is too many, so issue a diagnostic. // 7 conditions is too many, so issue a diagnostic.
let [a, b, c, d, e, f, g] = <[bool; 7]>::default(); 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"); core::hint::black_box("hello");
} }
} }