Skip to content

Commit

Permalink
Rollup merge of #118690 - Zalathar:test-macros, r=cjgillot
Browse files Browse the repository at this point in the history
coverage: Avoid unnecessary macros in unit tests

These macros don't provide enough value to justify their complexity, when they can just as easily be functions instead.

---

`@rustbot` label +A-code-coverage
  • Loading branch information
matthiaskrgr authored Dec 8, 2023
2 parents fa724cc + 47e6e5e commit 0c121b5
Show file tree
Hide file tree
Showing 5 changed files with 33 additions and 80 deletions.
5 changes: 0 additions & 5 deletions Cargo.lock
Original file line number Diff line number Diff line change
Expand Up @@ -795,10 +795,6 @@ dependencies = [
"rustc-demangle",
]

[[package]]
name = "coverage_test_macros"
version = "0.0.0"

[[package]]
name = "cpufeatures"
version = "0.2.8"
Expand Down Expand Up @@ -4266,7 +4262,6 @@ dependencies = [
name = "rustc_mir_transform"
version = "0.0.0"
dependencies = [
"coverage_test_macros",
"either",
"itertools",
"rustc_arena",
Expand Down
5 changes: 0 additions & 5 deletions compiler/rustc_mir_transform/Cargo.toml
Original file line number Diff line number Diff line change
Expand Up @@ -27,8 +27,3 @@ rustc_trait_selection = { path = "../rustc_trait_selection" }
smallvec = { version = "1.8.1", features = ["union", "may_dangle"] }
tracing = "0.1"
# tidy-alphabetical-end

[dev-dependencies]
# tidy-alphabetical-start
coverage_test_macros = { path = "src/coverage/test_macros" }
# tidy-alphabetical-end

This file was deleted.

This file was deleted.

90 changes: 33 additions & 57 deletions compiler/rustc_mir_transform/src/coverage/tests.rs
Original file line number Diff line number Diff line change
Expand Up @@ -27,8 +27,6 @@
use super::counters;
use super::graph::{self, BasicCoverageBlock};

use coverage_test_macros::let_bcb;

use itertools::Itertools;
use rustc_data_structures::graph::WithNumNodes;
use rustc_data_structures::graph::WithSuccessors;
Expand All @@ -37,6 +35,10 @@ use rustc_middle::mir::*;
use rustc_middle::ty;
use rustc_span::{self, BytePos, Pos, Span, DUMMY_SP};

fn bcb(index: u32) -> BasicCoverageBlock {
BasicCoverageBlock::from_u32(index)
}

// All `TEMP_BLOCK` targets should be replaced before calling `to_body() -> mir::Body`.
const TEMP_BLOCK: BasicBlock = BasicBlock::MAX;

Expand Down Expand Up @@ -300,12 +302,15 @@ fn goto_switchint<'a>() -> Body<'a> {
mir_body
}

macro_rules! assert_successors {
($basic_coverage_blocks:ident, $i:ident, [$($successor:ident),*]) => {
let mut successors = $basic_coverage_blocks.successors[$i].clone();
successors.sort_unstable();
assert_eq!(successors, vec![$($successor),*]);
}
#[track_caller]
fn assert_successors(
basic_coverage_blocks: &graph::CoverageGraph,
bcb: BasicCoverageBlock,
expected_successors: &[BasicCoverageBlock],
) {
let mut successors = basic_coverage_blocks.successors[bcb].clone();
successors.sort_unstable();
assert_eq!(successors, expected_successors);
}

#[test]
Expand Down Expand Up @@ -334,13 +339,9 @@ fn test_covgraph_goto_switchint() {
basic_coverage_blocks.iter_enumerated().collect::<Vec<_>>()
);

let_bcb!(0);
let_bcb!(1);
let_bcb!(2);

assert_successors!(basic_coverage_blocks, bcb0, [bcb1, bcb2]);
assert_successors!(basic_coverage_blocks, bcb1, []);
assert_successors!(basic_coverage_blocks, bcb2, []);
assert_successors(&basic_coverage_blocks, bcb(0), &[bcb(1), bcb(2)]);
assert_successors(&basic_coverage_blocks, bcb(1), &[]);
assert_successors(&basic_coverage_blocks, bcb(2), &[]);
}

/// Create a mock `Body` with a loop.
Expand Down Expand Up @@ -418,15 +419,10 @@ fn test_covgraph_switchint_then_loop_else_return() {
basic_coverage_blocks.iter_enumerated().collect::<Vec<_>>()
);

let_bcb!(0);
let_bcb!(1);
let_bcb!(2);
let_bcb!(3);

assert_successors!(basic_coverage_blocks, bcb0, [bcb1]);
assert_successors!(basic_coverage_blocks, bcb1, [bcb2, bcb3]);
assert_successors!(basic_coverage_blocks, bcb2, []);
assert_successors!(basic_coverage_blocks, bcb3, [bcb1]);
assert_successors(&basic_coverage_blocks, bcb(0), &[bcb(1)]);
assert_successors(&basic_coverage_blocks, bcb(1), &[bcb(2), bcb(3)]);
assert_successors(&basic_coverage_blocks, bcb(2), &[]);
assert_successors(&basic_coverage_blocks, bcb(3), &[bcb(1)]);
}

/// Create a mock `Body` with nested loops.
Expand Down Expand Up @@ -546,21 +542,13 @@ fn test_covgraph_switchint_loop_then_inner_loop_else_break() {
basic_coverage_blocks.iter_enumerated().collect::<Vec<_>>()
);

let_bcb!(0);
let_bcb!(1);
let_bcb!(2);
let_bcb!(3);
let_bcb!(4);
let_bcb!(5);
let_bcb!(6);

assert_successors!(basic_coverage_blocks, bcb0, [bcb1]);
assert_successors!(basic_coverage_blocks, bcb1, [bcb2, bcb3]);
assert_successors!(basic_coverage_blocks, bcb2, []);
assert_successors!(basic_coverage_blocks, bcb3, [bcb4]);
assert_successors!(basic_coverage_blocks, bcb4, [bcb5, bcb6]);
assert_successors!(basic_coverage_blocks, bcb5, [bcb1]);
assert_successors!(basic_coverage_blocks, bcb6, [bcb4]);
assert_successors(&basic_coverage_blocks, bcb(0), &[bcb(1)]);
assert_successors(&basic_coverage_blocks, bcb(1), &[bcb(2), bcb(3)]);
assert_successors(&basic_coverage_blocks, bcb(2), &[]);
assert_successors(&basic_coverage_blocks, bcb(3), &[bcb(4)]);
assert_successors(&basic_coverage_blocks, bcb(4), &[bcb(5), bcb(6)]);
assert_successors(&basic_coverage_blocks, bcb(5), &[bcb(1)]);
assert_successors(&basic_coverage_blocks, bcb(6), &[bcb(4)]);
}

#[test]
Expand Down Expand Up @@ -595,10 +583,7 @@ fn test_find_loop_backedges_one() {
backedges
);

let_bcb!(1);
let_bcb!(3);

assert_eq!(backedges[bcb1], vec![bcb3]);
assert_eq!(backedges[bcb(1)], &[bcb(3)]);
}

#[test]
Expand All @@ -613,13 +598,8 @@ fn test_find_loop_backedges_two() {
backedges
);

let_bcb!(1);
let_bcb!(4);
let_bcb!(5);
let_bcb!(6);

assert_eq!(backedges[bcb1], vec![bcb5]);
assert_eq!(backedges[bcb4], vec![bcb6]);
assert_eq!(backedges[bcb(1)], &[bcb(5)]);
assert_eq!(backedges[bcb(4)], &[bcb(6)]);
}

#[test]
Expand All @@ -632,13 +612,11 @@ fn test_traverse_coverage_with_loops() {
traversed_in_order.push(bcb);
}

let_bcb!(6);

// bcb0 is visited first. Then bcb1 starts the first loop, and all remaining nodes, *except*
// bcb6 are inside the first loop.
assert_eq!(
*traversed_in_order.last().expect("should have elements"),
bcb6,
bcb(6),
"bcb6 should not be visited until all nodes inside the first loop have been visited"
);
}
Expand All @@ -656,20 +634,18 @@ fn test_make_bcb_counters() {
coverage_counters.make_bcb_counters(&basic_coverage_blocks, bcb_has_coverage_spans);
assert_eq!(coverage_counters.num_expressions(), 0);

let_bcb!(1);
assert_eq!(
0, // bcb1 has a `Counter` with id = 0
match coverage_counters.bcb_counter(bcb1).expect("should have a counter") {
match coverage_counters.bcb_counter(bcb(1)).expect("should have a counter") {
counters::BcbCounter::Counter { id, .. } => id,
_ => panic!("expected a Counter"),
}
.as_u32()
);

let_bcb!(2);
assert_eq!(
1, // bcb2 has a `Counter` with id = 1
match coverage_counters.bcb_counter(bcb2).expect("should have a counter") {
match coverage_counters.bcb_counter(bcb(2)).expect("should have a counter") {
counters::BcbCounter::Counter { id, .. } => id,
_ => panic!("expected a Counter"),
}
Expand Down

0 comments on commit 0c121b5

Please sign in to comment.