Skip to content

Commit

Permalink
coverage: Treat each match arm as a "branch" for branch coverage
Browse files Browse the repository at this point in the history
  • Loading branch information
Zalathar committed Apr 20, 2024
1 parent e2a4fa1 commit d642bc8
Show file tree
Hide file tree
Showing 6 changed files with 120 additions and 16 deletions.
29 changes: 29 additions & 0 deletions compiler/rustc_mir_build/src/build/coverageinfo.rs
Expand Up @@ -28,6 +28,12 @@ struct NotInfo {
is_flipped: bool,
}

pub(crate) struct MatchArm {
pub(crate) source_info: SourceInfo,
pub(crate) has_guard: bool,
pub(crate) pre_binding_block: Option<BasicBlock>,
}

impl BranchInfoBuilder {
/// Creates a new branch info builder, but only if branch coverage instrumentation
/// is enabled and `def_id` represents a function that is eligible for coverage.
Expand Down Expand Up @@ -99,6 +105,29 @@ impl BranchInfoBuilder {
]);
}

pub(crate) fn add_match_arms(&mut self, cfg: &mut CFG<'_>, arms: Vec<MatchArm>) {
// Match expressions with 0-1 arms don't have any branches for their arms.
if arms.len() < 2 {
return;
}

// FIXME(#124118) The current implementation of branch coverage for
// match arms can't handle or-patterns or guards.
if arms.iter().any(|arm| arm.has_guard || arm.pre_binding_block.is_none()) {
return;
}

let branch_arms = arms
.iter()
.map(|&MatchArm { source_info, pre_binding_block, .. }| {
let marker = self.inject_block_marker(cfg, source_info, pre_binding_block.unwrap());
BranchArm { span: source_info.span, marker }
})
.collect::<Vec<_>>();

self.branch_arm_lists.push(branch_arms);
}

fn next_block_marker_id(&mut self) -> BlockMarkerId {
let id = BlockMarkerId::from_usize(self.num_block_markers);
self.num_block_markers += 1;
Expand Down
25 changes: 25 additions & 0 deletions compiler/rustc_mir_build/src/build/matches/mod.rs
Expand Up @@ -5,6 +5,7 @@
//! This also includes code for pattern bindings in `let` statements and
//! function parameters.

use crate::build::coverageinfo;
use crate::build::expr::as_place::PlaceBuilder;
use crate::build::scope::DropKind;
use crate::build::ForGuard::{self, OutsideGuard, RefWithinGuard};
Expand Down Expand Up @@ -315,6 +316,10 @@ impl<'a, 'tcx> Builder<'a, 'tcx> {
&mut candidates,
);

// Record branch coverage info for this match.
// (Does nothing if branch coverage is not enabled.)
self.visit_coverage_match_expr(&candidates);

self.lower_match_arms(
destination,
scrutinee_place,
Expand Down Expand Up @@ -364,6 +369,26 @@ impl<'a, 'tcx> Builder<'a, 'tcx> {
.collect()
}

/// If branch coverage is enabled, inject marker statements into each match
/// arm, and record their IDs in the table of branches.
///
/// Unlike some of the other branch coverage visitor methods, this one needs
/// access to private details of [`Candidate`], so having it here is easier.
fn visit_coverage_match_expr(&mut self, candidates: &[&mut Candidate<'_, 'tcx>]) {
// Bail out if branch coverage is not enabled for this function.
let Some(branch_info) = self.coverage_branch_info.as_mut() else { return };

let arms = candidates
.iter()
.map(|c| coverageinfo::MatchArm {
source_info: SourceInfo { span: c.extra_data.span, scope: self.source_scope },
has_guard: c.has_guard,
pre_binding_block: c.pre_binding_block,
})
.collect::<Vec<_>>();
branch_info.add_match_arms(&mut self.cfg, arms);
}

/// Create the decision tree for the match expression, starting from `block`.
///
/// Modifies `candidates` to store the bindings and type ascriptions for
Expand Down
52 changes: 38 additions & 14 deletions tests/coverage/branch/match-arms.cov-map
Expand Up @@ -36,25 +36,49 @@ Number of file 0 mappings: 12
= ((((((((Zero + c2) + c3) + c4) + c5) + c6) + c7) + c8) + c9)

Function name: match_arms::match_arms
Raw bytes (51): 0x[01, 01, 06, 05, 07, 0b, 11, 09, 0d, 13, 02, 17, 09, 11, 0d, 07, 01, 18, 01, 01, 10, 05, 03, 0b, 00, 10, 11, 01, 11, 00, 21, 0d, 01, 11, 00, 21, 09, 01, 11, 00, 21, 02, 01, 11, 00, 21, 0f, 03, 05, 01, 02]
Raw bytes (102): 0x[01, 01, 15, 0d, 17, 09, 4a, 05, 4f, 53, 11, 09, 0d, 09, 4a, 05, 4f, 53, 11, 09, 0d, 05, 4f, 53, 11, 09, 0d, 05, 4f, 53, 11, 09, 0d, 43, 4a, 47, 09, 11, 0d, 05, 4f, 53, 11, 09, 0d, 0a, 01, 18, 01, 01, 10, 05, 03, 0b, 00, 10, 20, 11, 03, 01, 09, 00, 13, 11, 00, 11, 00, 21, 20, 0d, 17, 01, 09, 00, 13, 0d, 00, 11, 00, 21, 20, 09, 4a, 01, 09, 00, 13, 09, 00, 11, 00, 21, 4a, 01, 11, 00, 21, 3f, 03, 05, 01, 02]
Number of files: 1
- file 0 => global file 1
Number of expressions: 6
- expression 0 operands: lhs = Counter(1), rhs = Expression(1, Add)
- expression 1 operands: lhs = Expression(2, Add), rhs = Counter(4)
- expression 2 operands: lhs = Counter(2), rhs = Counter(3)
- expression 3 operands: lhs = Expression(4, Add), rhs = Expression(0, Sub)
- expression 4 operands: lhs = Expression(5, Add), rhs = Counter(2)
- expression 5 operands: lhs = Counter(4), rhs = Counter(3)
Number of file 0 mappings: 7
Number of expressions: 21
- expression 0 operands: lhs = Counter(3), rhs = Expression(5, Add)
- expression 1 operands: lhs = Counter(2), rhs = Expression(18, Sub)
- expression 2 operands: lhs = Counter(1), rhs = Expression(19, Add)
- expression 3 operands: lhs = Expression(20, Add), rhs = Counter(4)
- expression 4 operands: lhs = Counter(2), rhs = Counter(3)
- expression 5 operands: lhs = Counter(2), rhs = Expression(18, Sub)
- expression 6 operands: lhs = Counter(1), rhs = Expression(19, Add)
- expression 7 operands: lhs = Expression(20, Add), rhs = Counter(4)
- expression 8 operands: lhs = Counter(2), rhs = Counter(3)
- expression 9 operands: lhs = Counter(1), rhs = Expression(19, Add)
- expression 10 operands: lhs = Expression(20, Add), rhs = Counter(4)
- expression 11 operands: lhs = Counter(2), rhs = Counter(3)
- expression 12 operands: lhs = Counter(1), rhs = Expression(19, Add)
- expression 13 operands: lhs = Expression(20, Add), rhs = Counter(4)
- expression 14 operands: lhs = Counter(2), rhs = Counter(3)
- expression 15 operands: lhs = Expression(16, Add), rhs = Expression(18, Sub)
- expression 16 operands: lhs = Expression(17, Add), rhs = Counter(2)
- expression 17 operands: lhs = Counter(4), rhs = Counter(3)
- expression 18 operands: lhs = Counter(1), rhs = Expression(19, Add)
- expression 19 operands: lhs = Expression(20, Add), rhs = Counter(4)
- expression 20 operands: lhs = Counter(2), rhs = Counter(3)
Number of file 0 mappings: 10
- Code(Counter(0)) at (prev + 24, 1) to (start + 1, 16)
- Code(Counter(1)) at (prev + 3, 11) to (start + 0, 16)
- Code(Counter(4)) at (prev + 1, 17) to (start + 0, 33)
- Code(Counter(3)) at (prev + 1, 17) to (start + 0, 33)
- Code(Counter(2)) at (prev + 1, 17) to (start + 0, 33)
- Code(Expression(0, Sub)) at (prev + 1, 17) to (start + 0, 33)
- Branch { true: Counter(4), false: Expression(0, Add) } at (prev + 1, 9) to (start + 0, 19)
true = c4
false = (c3 + (c2 + (c1 - ((c2 + c3) + c4))))
- Code(Counter(4)) at (prev + 0, 17) to (start + 0, 33)
- Branch { true: Counter(3), false: Expression(5, Add) } at (prev + 1, 9) to (start + 0, 19)
true = c3
false = (c2 + (c1 - ((c2 + c3) + c4)))
- Code(Counter(3)) at (prev + 0, 17) to (start + 0, 33)
- Branch { true: Counter(2), false: Expression(18, Sub) } at (prev + 1, 9) to (start + 0, 19)
true = c2
false = (c1 - ((c2 + c3) + c4))
- Code(Counter(2)) at (prev + 0, 17) to (start + 0, 33)
- Code(Expression(18, Sub)) at (prev + 1, 17) to (start + 0, 33)
= (c1 - ((c2 + c3) + c4))
- Code(Expression(3, Add)) at (prev + 3, 5) to (start + 1, 2)
- Code(Expression(15, Add)) at (prev + 3, 5) to (start + 1, 2)
= (((c4 + c3) + c2) + (c1 - ((c2 + c3) + c4)))

Function name: match_arms::or_patterns
Expand Down
11 changes: 10 additions & 1 deletion tests/coverage/branch/match-arms.coverage
Expand Up @@ -26,8 +26,17 @@
LL| |
LL| 15| match value {
LL| 8| Enum::D(d) => consume(d),
------------------
| Branch (LL:9): [True: 8, False: 7]
------------------
LL| 4| Enum::C(c) => consume(c),
------------------
| Branch (LL:9): [True: 4, False: 3]
------------------
LL| 2| Enum::B(b) => consume(b),
------------------
| Branch (LL:9): [True: 2, False: 1]
------------------
LL| 1| Enum::A(a) => consume(a),
LL| | }
LL| |
Expand Down Expand Up @@ -101,5 +110,5 @@
LL| | }
LL| |}
LL| |
LL| |// FIXME(#124118) Actually instrument match arms for branch coverage.
LL| |// FIXME(#124118) Support match expressions with guards or or-patterns.

2 changes: 1 addition & 1 deletion tests/coverage/branch/match-arms.rs
Expand Up @@ -87,4 +87,4 @@ fn main() {
}
}

// FIXME(#124118) Actually instrument match arms for branch coverage.
// FIXME(#124118) Support match expressions with guards or or-patterns.
Expand Up @@ -26,18 +26,31 @@
debug a => _9;
}

coverage branches {
BlockMarkerId(0) => $DIR/branch_match_arms.rs:17:9: 17:19 (#0)
BlockMarkerId(1) => $DIR/branch_match_arms.rs:18:9: 18:19 (#0)
BlockMarkerId(2) => $DIR/branch_match_arms.rs:19:9: 19:19 (#0)
BlockMarkerId(3) => $DIR/branch_match_arms.rs:20:9: 20:19 (#0)
}

+ coverage ExpressionId(0) => Expression { lhs: Counter(1), op: Add, rhs: Counter(2) };
+ coverage ExpressionId(1) => Expression { lhs: Expression(0), op: Add, rhs: Counter(3) };
+ coverage ExpressionId(2) => Expression { lhs: Counter(0), op: Subtract, rhs: Expression(1) };
+ coverage ExpressionId(3) => Expression { lhs: Counter(3), op: Add, rhs: Counter(2) };
+ coverage ExpressionId(4) => Expression { lhs: Expression(3), op: Add, rhs: Counter(1) };
+ coverage ExpressionId(5) => Expression { lhs: Expression(4), op: Add, rhs: Expression(2) };
+ coverage ExpressionId(6) => Expression { lhs: Counter(1), op: Add, rhs: Expression(2) };
+ coverage ExpressionId(7) => Expression { lhs: Counter(2), op: Add, rhs: Expression(6) };
+ coverage ExpressionId(8) => Expression { lhs: Counter(3), op: Add, rhs: Expression(7) };
+ coverage Code(Counter(0)) => $DIR/branch_match_arms.rs:15:1 - 16:21;
+ coverage Code(Counter(3)) => $DIR/branch_match_arms.rs:17:17 - 17:33;
+ coverage Code(Counter(2)) => $DIR/branch_match_arms.rs:18:17 - 18:33;
+ coverage Code(Counter(1)) => $DIR/branch_match_arms.rs:19:17 - 19:33;
+ coverage Code(Expression(2)) => $DIR/branch_match_arms.rs:20:17 - 20:33;
+ coverage Code(Expression(5)) => $DIR/branch_match_arms.rs:22:1 - 22:2;
+ coverage Branch { true_term: Counter(1), false_term: Expression(2) } => $DIR/branch_match_arms.rs:19:9 - 19:19;
+ coverage Branch { true_term: Counter(2), false_term: Expression(6) } => $DIR/branch_match_arms.rs:18:9 - 18:19;
+ coverage Branch { true_term: Counter(3), false_term: Expression(7) } => $DIR/branch_match_arms.rs:17:9 - 17:19;
+
bb0: {
+ Coverage::CounterIncrement(0);
Expand All @@ -55,21 +68,25 @@

bb2: {
+ Coverage::CounterIncrement(3);
Coverage::BlockMarker(0);
falseEdge -> [real: bb6, imaginary: bb3];
}

bb3: {
+ Coverage::CounterIncrement(2);
Coverage::BlockMarker(1);
falseEdge -> [real: bb8, imaginary: bb4];
}

bb4: {
+ Coverage::CounterIncrement(1);
Coverage::BlockMarker(2);
falseEdge -> [real: bb10, imaginary: bb5];
}

bb5: {
+ Coverage::ExpressionUsed(2);
Coverage::BlockMarker(3);
StorageLive(_9);
_9 = ((_1 as A).0: u32);
StorageLive(_10);
Expand Down

0 comments on commit d642bc8

Please sign in to comment.