Rollup merge of #125756 - Zalathar:branch-on-bool, r=oli-obk
coverage: Optionally instrument the RHS of lazy logical operators (This is an updated version of #124644 and #124402. Fixes #124120.) When `||` or `&&` is used outside of a branching context (such as the condition of an `if`), the rightmost value does not directly influence any branching decision, so branch coverage instrumentation does not treat it as its own true-or-false branch. That is a correct and useful interpretation of “branch coverage”, but might be undesirable in some contexts, as described at #124120. This PR therefore adds a new coverage level `-Zcoverage-options=condition` that behaves like branch coverage, but also adds additional branch instrumentation to the right-hand-side of lazy boolean operators. --- As discussed at https://github.com/rust-lang/rust/issues/124120#issuecomment-2092394586, this is mainly intended as an intermediate step towards fully-featured MC/DC instrumentation. It's likely that we'll eventually want to remove this coverage level (rather than stabilize it), either because it has been incorporated into MC/DC instrumentation, or because it's getting in the way of future MC/DC work. The main appeal of landing it now is so that work on tracking conditions can proceed concurrently with other MC/DC-related work. ````@rustbot```` label +A-code-coverage
This commit is contained in:
commit
7667a91778
11 changed files with 411 additions and 8 deletions
|
@ -157,6 +157,63 @@ impl BranchInfoBuilder {
|
|||
}
|
||||
|
||||
impl<'tcx> Builder<'_, 'tcx> {
|
||||
/// If condition coverage is enabled, inject extra blocks and marker statements
|
||||
/// that will let us track the value of the condition in `place`.
|
||||
pub(crate) fn visit_coverage_standalone_condition(
|
||||
&mut self,
|
||||
mut expr_id: ExprId, // Expression giving the span of the condition
|
||||
place: mir::Place<'tcx>, // Already holds the boolean condition value
|
||||
block: &mut BasicBlock,
|
||||
) {
|
||||
// Bail out if condition coverage is not enabled for this function.
|
||||
let Some(branch_info) = self.coverage_branch_info.as_mut() else { return };
|
||||
if !self.tcx.sess.instrument_coverage_condition() {
|
||||
return;
|
||||
};
|
||||
|
||||
// Remove any wrappers, so that we can inspect the real underlying expression.
|
||||
while let ExprKind::Use { source: inner } | ExprKind::Scope { value: inner, .. } =
|
||||
self.thir[expr_id].kind
|
||||
{
|
||||
expr_id = inner;
|
||||
}
|
||||
// If the expression is a lazy logical op, it will naturally get branch
|
||||
// coverage as part of its normal lowering, so we can disregard it here.
|
||||
if let ExprKind::LogicalOp { .. } = self.thir[expr_id].kind {
|
||||
return;
|
||||
}
|
||||
|
||||
let source_info = SourceInfo { span: self.thir[expr_id].span, scope: self.source_scope };
|
||||
|
||||
// Using the boolean value that has already been stored in `place`, set up
|
||||
// control flow in the shape of a diamond, so that we can place separate
|
||||
// marker statements in the true and false blocks. The coverage MIR pass
|
||||
// will use those markers to inject coverage counters as appropriate.
|
||||
//
|
||||
// block
|
||||
// / \
|
||||
// true_block false_block
|
||||
// (marker) (marker)
|
||||
// \ /
|
||||
// join_block
|
||||
|
||||
let true_block = self.cfg.start_new_block();
|
||||
let false_block = self.cfg.start_new_block();
|
||||
self.cfg.terminate(
|
||||
*block,
|
||||
source_info,
|
||||
mir::TerminatorKind::if_(mir::Operand::Copy(place), true_block, false_block),
|
||||
);
|
||||
|
||||
branch_info.add_two_way_branch(&mut self.cfg, source_info, true_block, false_block);
|
||||
|
||||
let join_block = self.cfg.start_new_block();
|
||||
self.cfg.goto(true_block, source_info, join_block);
|
||||
self.cfg.goto(false_block, source_info, join_block);
|
||||
// Any subsequent codegen in the caller should use the new join block.
|
||||
*block = join_block;
|
||||
}
|
||||
|
||||
/// If branch coverage is enabled, inject marker statements into `then_block`
|
||||
/// and `else_block`, and record their IDs in the table of branch spans.
|
||||
pub(crate) fn visit_coverage_branch_condition(
|
||||
|
|
|
@ -183,9 +183,13 @@ impl<'a, 'tcx> Builder<'a, 'tcx> {
|
|||
const_: Const::from_bool(this.tcx, constant),
|
||||
},
|
||||
);
|
||||
let rhs = unpack!(this.expr_into_dest(destination, continuation, rhs));
|
||||
let mut rhs_block = unpack!(this.expr_into_dest(destination, continuation, rhs));
|
||||
// Instrument the lowered RHS's value for condition coverage.
|
||||
// (Does nothing if condition coverage is not enabled.)
|
||||
this.visit_coverage_standalone_condition(rhs, destination, &mut rhs_block);
|
||||
|
||||
let target = this.cfg.start_new_block();
|
||||
this.cfg.goto(rhs, source_info, target);
|
||||
this.cfg.goto(rhs_block, source_info, target);
|
||||
this.cfg.goto(short_circuit, source_info, target);
|
||||
target.unit()
|
||||
}
|
||||
|
|
Loading…
Add table
Add a link
Reference in a new issue