diff --git a/compiler/rustc_mir_build/src/build/coverageinfo.rs b/compiler/rustc_mir_build/src/build/coverageinfo.rs index e3929f10534..f0c0abfccec 100644 --- a/compiler/rustc_mir_build/src/build/coverageinfo.rs +++ b/compiler/rustc_mir_build/src/build/coverageinfo.rs @@ -23,9 +23,7 @@ pub(crate) struct BranchInfoBuilder { markers: BlockMarkerGen, branch_spans: Vec, - mcdc_branch_spans: Vec, - mcdc_decision_spans: Vec, - mcdc_state: Option, + mcdc_info: Option, } #[derive(Clone, Copy)] @@ -76,9 +74,7 @@ pub(crate) fn new_if_enabled(tcx: TyCtxt<'_>, def_id: LocalDefId) -> Option, expr_id: ExprId, not_info: No } } - fn fetch_mcdc_condition_info( - &mut self, - tcx: TyCtxt<'_>, - true_marker: BlockMarkerId, - false_marker: BlockMarkerId, - ) -> Option<(u16, ConditionInfo)> { - let mcdc_state = self.mcdc_state.as_mut()?; - let decision_depth = mcdc_state.decision_depth(); - let (mut condition_info, decision_result) = - mcdc_state.take_condition(true_marker, false_marker); - // take_condition() returns Some for decision_result when the decision stack - // 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 { - 0 => { - unreachable!("Decision with no condition is not expected"); - } - 1..=MAX_CONDITIONS_NUM_IN_DECISION => { - self.mcdc_decision_spans.push(decision); - } - _ => { - // Do not generate mcdc mappings and statements for decisions with too many conditions. - let rebase_idx = self.mcdc_branch_spans.len() - decision.conditions_num + 1; - for branch in &mut self.mcdc_branch_spans[rebase_idx..] { - branch.condition_info = None; - } - - // ConditionInfo of this branch shall also be reset. - condition_info = None; - - tcx.dcx().emit_warn(MCDCExceedsConditionNumLimit { - span: decision.span, - conditions_num: decision.conditions_num, - max_conditions_num: MAX_CONDITIONS_NUM_IN_DECISION, - }); - } - } - } - condition_info.map(|cond_info| (decision_depth, cond_info)) - } - fn add_two_way_branch<'tcx>( &mut self, cfg: &mut CFG<'tcx>, @@ -185,9 +139,7 @@ pub(crate) fn into_done(self) -> Option> { nots: _, markers: BlockMarkerGen { num_block_markers }, branch_spans, - mcdc_branch_spans, - mcdc_decision_spans, - mcdc_state: _, + mcdc_info, } = self; if num_block_markers == 0 { @@ -195,6 +147,9 @@ pub(crate) fn into_done(self) -> Option> { return None; } + let (mcdc_decision_spans, mcdc_branch_spans) = + mcdc_info.map(MCDCInfoBuilder::into_done).unwrap_or_default(); + Some(Box::new(mir::coverage::BranchInfo { num_block_markers, branch_spans, @@ -221,20 +176,23 @@ struct MCDCState { } impl MCDCState { - fn new_if_enabled(tcx: TyCtxt<'_>) -> Option { - tcx.sess - .instrument_coverage_mcdc() - .then(|| Self { decision_ctx_stack: vec![MCDCDecisionCtx::default()] }) + fn new() -> Self { + Self { decision_ctx_stack: vec![MCDCDecisionCtx::default()] } } /// Decision depth is given as a u16 to reduce the size of the `CoverageKind`, /// as it is very unlikely that the depth ever reaches 2^16. #[inline] fn decision_depth(&self) -> u16 { - u16::try_from( - self.decision_ctx_stack.len().checked_sub(1).expect("Unexpected empty decision stack"), - ) - .expect("decision depth did not fit in u16, this is likely to be an instrumentation error") + match u16::try_from(self.decision_ctx_stack.len()) + .expect( + "decision depth did not fit in u16, this is likely to be an instrumentation error", + ) + .checked_sub(1) + { + Some(d) => d, + None => bug!("Unexpected empty decision stack"), + } } // At first we assign ConditionIds for each sub expression. @@ -279,8 +237,9 @@ fn decision_depth(&self) -> u16 { // - If the op is OR, the "true_next" of LHS and RHS should be the parent's "true_next". While "false_next" of the LHS is the RHS, the "false next" of RHS is the parent's "false_next". fn record_conditions(&mut self, op: LogicalOp, span: Span) { let decision_depth = self.decision_depth(); - let decision_ctx = - self.decision_ctx_stack.last_mut().expect("Unexpected empty decision_ctx_stack"); + let Some(decision_ctx) = self.decision_ctx_stack.last_mut() else { + bug!("Unexpected empty decision_ctx_stack") + }; let decision = match decision_ctx.processing_decision.as_mut() { Some(decision) => { decision.span = decision.span.to(span); @@ -343,8 +302,9 @@ fn take_condition( true_marker: BlockMarkerId, false_marker: BlockMarkerId, ) -> (Option, Option) { - let decision_ctx = - self.decision_ctx_stack.last_mut().expect("Unexpected empty decision_ctx_stack"); + let Some(decision_ctx) = self.decision_ctx_stack.last_mut() else { + bug!("Unexpected empty decision_ctx_stack") + }; let Some(condition_info) = decision_ctx.decision_stack.pop_back() else { return (None, None); }; @@ -366,6 +326,74 @@ fn take_condition( } } +struct MCDCInfoBuilder { + branch_spans: Vec, + decision_spans: Vec, + state: MCDCState, +} + +impl MCDCInfoBuilder { + fn new() -> Self { + Self { branch_spans: vec![], decision_spans: vec![], state: MCDCState::new() } + } + + fn visit_evaluated_condition( + &mut self, + tcx: TyCtxt<'_>, + source_info: SourceInfo, + true_block: BasicBlock, + false_block: BasicBlock, + mut inject_block_marker: impl FnMut(SourceInfo, BasicBlock) -> BlockMarkerId, + ) { + let true_marker = inject_block_marker(source_info, true_block); + let false_marker = inject_block_marker(source_info, false_block); + + let decision_depth = self.state.decision_depth(); + let (mut condition_info, decision_result) = + self.state.take_condition(true_marker, false_marker); + // take_condition() returns Some for decision_result when the decision stack + // 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 { + 0 => { + unreachable!("Decision with no condition is not expected"); + } + 1..=MAX_CONDITIONS_NUM_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; + 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 { + span: decision.span, + conditions_num: decision.conditions_num, + max_conditions_num: MAX_CONDITIONS_NUM_IN_DECISION, + }); + } + } + } + self.branch_spans.push(MCDCBranchSpan { + span: source_info.span, + condition_info, + true_marker, + false_marker, + decision_depth, + }); + } + + fn into_done(self) -> (Vec, Vec) { + (self.decision_spans, self.branch_spans) + } +} + impl Builder<'_, '_> { /// If branch coverage is enabled, inject marker statements into `then_block` /// and `else_block`, and record their IDs in the table of branch spans. @@ -390,23 +418,17 @@ pub(crate) fn visit_coverage_branch_condition( let source_info = SourceInfo { span: self.thir[expr_id].span, scope: self.source_scope }; // Separate path for handling branches when MC/DC is enabled. - if branch_info.mcdc_state.is_some() { - let mut inject_block_marker = - |block| branch_info.markers.inject_block_marker(&mut self.cfg, source_info, block); - let true_marker = inject_block_marker(then_block); - let false_marker = inject_block_marker(else_block); - let (decision_depth, condition_info) = branch_info - .fetch_mcdc_condition_info(self.tcx, true_marker, false_marker) - .map_or((0, None), |(decision_depth, condition_info)| { - (decision_depth, Some(condition_info)) - }); - branch_info.mcdc_branch_spans.push(MCDCBranchSpan { - span: source_info.span, - condition_info, - true_marker, - false_marker, - decision_depth, - }); + if let Some(mcdc_info) = branch_info.mcdc_info.as_mut() { + let inject_block_marker = |source_info, block| { + branch_info.markers.inject_block_marker(&mut self.cfg, source_info, block) + }; + mcdc_info.visit_evaluated_condition( + self.tcx, + source_info, + then_block, + else_block, + inject_block_marker, + ); return; } @@ -415,25 +437,27 @@ pub(crate) fn visit_coverage_branch_condition( pub(crate) fn visit_coverage_branch_operation(&mut self, logical_op: LogicalOp, span: Span) { if let Some(branch_info) = self.coverage_branch_info.as_mut() - && let Some(mcdc_state) = branch_info.mcdc_state.as_mut() + && let Some(mcdc_info) = branch_info.mcdc_info.as_mut() { - mcdc_state.record_conditions(logical_op, span); + mcdc_info.state.record_conditions(logical_op, span); } } pub(crate) fn mcdc_increment_depth_if_enabled(&mut self) { if let Some(branch_info) = self.coverage_branch_info.as_mut() - && let Some(mcdc_state) = branch_info.mcdc_state.as_mut() + && let Some(mcdc_info) = branch_info.mcdc_info.as_mut() { - mcdc_state.decision_ctx_stack.push(MCDCDecisionCtx::default()); + mcdc_info.state.decision_ctx_stack.push(MCDCDecisionCtx::default()); }; } pub(crate) fn mcdc_decrement_depth_if_enabled(&mut self) { if let Some(branch_info) = self.coverage_branch_info.as_mut() - && let Some(mcdc_state) = branch_info.mcdc_state.as_mut() + && let Some(mcdc_info) = branch_info.mcdc_info.as_mut() { - mcdc_state.decision_ctx_stack.pop().expect("Unexpected empty decision stack"); + if mcdc_info.state.decision_ctx_stack.pop().is_none() { + bug!("Unexpected empty decision stack"); + } }; } }