Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

coverage: Several small improvements to graph code #126538

Merged
merged 4 commits into from
Jun 17, 2024
Merged
Show file tree
Hide file tree
Changes from 1 commit
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
Next Next commit
coverage: Remove some old low-value unit tests for graph traversal
These tests might have originally been useful as an implementation aid, but now
they don't provide enough value to justify the burden of updating them as the
underlying code changes.

The code they test is still exercised by the main end-to-end coverage tests.
  • Loading branch information
Zalathar committed Jun 16, 2024
commit 5eb30f06993ea1dab0926d81f6d407d894b593e8
5 changes: 0 additions & 5 deletions compiler/rustc_mir_transform/src/coverage/counters.rs
Original file line number Diff line number Diff line change
Expand Up @@ -168,11 +168,6 @@ impl CoverageCounters {
self.counter_increment_sites.len()
}

#[cfg(test)]
pub(super) fn num_expressions(&self) -> usize {
self.expressions.len()
}

fn set_bcb_counter(&mut self, bcb: BasicCoverageBlock, counter_kind: BcbCounter) -> BcbCounter {
if let Some(replaced) = self.bcb_counters[bcb].replace(counter_kind) {
bug!(
Expand Down
106 changes: 0 additions & 106 deletions compiler/rustc_mir_transform/src/coverage/tests.rs
Original file line number Diff line number Diff line change
Expand Up @@ -24,7 +24,6 @@
//! globals is comparatively simpler. The easiest way is to wrap the test in a closure argument
//! to: `rustc_span::create_default_session_globals_then(|| { test_here(); })`.

use super::counters;
use super::graph::{self, BasicCoverageBlock};

use itertools::Itertools;
Expand Down Expand Up @@ -551,108 +550,3 @@ fn test_covgraph_switchint_loop_then_inner_loop_else_break() {
assert_successors(&basic_coverage_blocks, bcb(5), &[bcb(1)]);
assert_successors(&basic_coverage_blocks, bcb(6), &[bcb(4)]);
}

#[test]
fn test_find_loop_backedges_none() {
let mir_body = goto_switchint();
let basic_coverage_blocks = graph::CoverageGraph::from_mir(&mir_body);
if false {
eprintln!(
"basic_coverage_blocks = {:?}",
basic_coverage_blocks.iter_enumerated().collect::<Vec<_>>()
);
eprintln!("successors = {:?}", basic_coverage_blocks.successors);
}
let backedges = graph::find_loop_backedges(&basic_coverage_blocks);
assert_eq!(
backedges.iter_enumerated().map(|(_bcb, backedges)| backedges.len()).sum::<usize>(),
0,
"backedges: {:?}",
backedges
);
}

#[test]
fn test_find_loop_backedges_one() {
let mir_body = switchint_then_loop_else_return();
let basic_coverage_blocks = graph::CoverageGraph::from_mir(&mir_body);
let backedges = graph::find_loop_backedges(&basic_coverage_blocks);
assert_eq!(
backedges.iter_enumerated().map(|(_bcb, backedges)| backedges.len()).sum::<usize>(),
1,
"backedges: {:?}",
backedges
);

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

#[test]
fn test_find_loop_backedges_two() {
let mir_body = switchint_loop_then_inner_loop_else_break();
let basic_coverage_blocks = graph::CoverageGraph::from_mir(&mir_body);
let backedges = graph::find_loop_backedges(&basic_coverage_blocks);
assert_eq!(
backedges.iter_enumerated().map(|(_bcb, backedges)| backedges.len()).sum::<usize>(),
2,
"backedges: {:?}",
backedges
);

assert_eq!(backedges[bcb(1)], &[bcb(5)]);
assert_eq!(backedges[bcb(4)], &[bcb(6)]);
}

#[test]
fn test_traverse_coverage_with_loops() {
let mir_body = switchint_loop_then_inner_loop_else_break();
let basic_coverage_blocks = graph::CoverageGraph::from_mir(&mir_body);
let mut traversed_in_order = Vec::new();
let mut traversal = graph::TraverseCoverageGraphWithLoops::new(&basic_coverage_blocks);
while let Some(bcb) = traversal.next() {
traversed_in_order.push(bcb);
}

// 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"),
bcb(6),
"bcb6 should not be visited until all nodes inside the first loop have been visited"
);
}

#[test]
fn test_make_bcb_counters() {
rustc_span::create_default_session_globals_then(|| {
let mir_body = goto_switchint();
let basic_coverage_blocks = graph::CoverageGraph::from_mir(&mir_body);
// Historically this test would use `spans` internals to set up fake
// coverage spans for BCBs 1 and 2. Now we skip that step and just tell
// BCB counter construction that those BCBs have spans.
let bcb_has_coverage_spans = |bcb: BasicCoverageBlock| (1..=2).contains(&bcb.as_usize());
let coverage_counters = counters::CoverageCounters::make_bcb_counters(
&basic_coverage_blocks,
bcb_has_coverage_spans,
);
assert_eq!(coverage_counters.num_expressions(), 0);

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

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