diff --git a/compiler/rustc_mir_transform/src/coverage/counters.rs b/compiler/rustc_mir_transform/src/coverage/counters.rs index b5968517d77..a8b0f4a8d6d 100644 --- a/compiler/rustc_mir_transform/src/coverage/counters.rs +++ b/compiler/rustc_mir_transform/src/coverage/counters.rs @@ -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!( diff --git a/compiler/rustc_mir_transform/src/coverage/graph.rs b/compiler/rustc_mir_transform/src/coverage/graph.rs index fd74a2a97e2..360dccb240d 100644 --- a/compiler/rustc_mir_transform/src/coverage/graph.rs +++ b/compiler/rustc_mir_transform/src/coverage/graph.rs @@ -14,16 +14,16 @@ use std::ops::{Index, IndexMut}; /// A coverage-specific simplification of the MIR control flow graph (CFG). The `CoverageGraph`s /// nodes are `BasicCoverageBlock`s, which encompass one or more MIR `BasicBlock`s. #[derive(Debug)] -pub(super) struct CoverageGraph { +pub(crate) struct CoverageGraph { bcbs: IndexVec, bb_to_bcb: IndexVec>, - pub successors: IndexVec>, - pub predecessors: IndexVec>, + pub(crate) successors: IndexVec>, + pub(crate) predecessors: IndexVec>, dominators: Option>, } impl CoverageGraph { - pub fn from_mir(mir_body: &mir::Body<'_>) -> Self { + pub(crate) fn from_mir(mir_body: &mir::Body<'_>) -> Self { let (bcbs, bb_to_bcb) = Self::compute_basic_coverage_blocks(mir_body); // Pre-transform MIR `BasicBlock` successors and predecessors into the BasicCoverageBlock @@ -135,24 +135,28 @@ impl CoverageGraph { } #[inline(always)] - pub fn iter_enumerated( + pub(crate) fn iter_enumerated( &self, ) -> impl Iterator { self.bcbs.iter_enumerated() } #[inline(always)] - pub fn bcb_from_bb(&self, bb: BasicBlock) -> Option { + pub(crate) fn bcb_from_bb(&self, bb: BasicBlock) -> Option { if bb.index() < self.bb_to_bcb.len() { self.bb_to_bcb[bb] } else { None } } #[inline(always)] - pub fn dominates(&self, dom: BasicCoverageBlock, node: BasicCoverageBlock) -> bool { + pub(crate) fn dominates(&self, dom: BasicCoverageBlock, node: BasicCoverageBlock) -> bool { self.dominators.as_ref().unwrap().dominates(dom, node) } #[inline(always)] - pub fn cmp_in_dominator_order(&self, a: BasicCoverageBlock, b: BasicCoverageBlock) -> Ordering { + pub(crate) fn cmp_in_dominator_order( + &self, + a: BasicCoverageBlock, + b: BasicCoverageBlock, + ) -> Ordering { self.dominators.as_ref().unwrap().cmp_in_dominator_order(a, b) } @@ -166,7 +170,7 @@ impl CoverageGraph { /// /// FIXME: That assumption might not be true for [`TerminatorKind::Yield`]? #[inline(always)] - pub(super) fn bcb_has_multiple_in_edges(&self, bcb: BasicCoverageBlock) -> bool { + pub(crate) fn bcb_has_multiple_in_edges(&self, bcb: BasicCoverageBlock) -> bool { // Even though bcb0 conceptually has an extra virtual in-edge due to // being the entry point, we've already asserted that it has no _other_ // in-edges, so there's no possibility of it having _multiple_ in-edges. @@ -212,7 +216,7 @@ impl graph::StartNode for CoverageGraph { impl graph::Successors for CoverageGraph { #[inline] fn successors(&self, node: Self::Node) -> impl Iterator { - self.successors[node].iter().cloned() + self.successors[node].iter().copied() } } @@ -227,7 +231,7 @@ rustc_index::newtype_index! { /// A node in the control-flow graph of CoverageGraph. #[orderable] #[debug_format = "bcb{}"] - pub(super) struct BasicCoverageBlock { + pub(crate) struct BasicCoverageBlock { const START_BCB = 0; } } @@ -259,23 +263,23 @@ rustc_index::newtype_index! { /// queries (`dominates()`, `predecessors`, `successors`, etc.) have branch (control flow) /// significance. #[derive(Debug, Clone)] -pub(super) struct BasicCoverageBlockData { - pub basic_blocks: Vec, +pub(crate) struct BasicCoverageBlockData { + pub(crate) basic_blocks: Vec, } impl BasicCoverageBlockData { - pub fn from(basic_blocks: Vec) -> Self { + fn from(basic_blocks: Vec) -> Self { assert!(basic_blocks.len() > 0); Self { basic_blocks } } #[inline(always)] - pub fn leader_bb(&self) -> BasicBlock { + pub(crate) fn leader_bb(&self) -> BasicBlock { self.basic_blocks[0] } #[inline(always)] - pub fn last_bb(&self) -> BasicBlock { + pub(crate) fn last_bb(&self) -> BasicBlock { *self.basic_blocks.last().unwrap() } } @@ -364,7 +368,7 @@ fn bcb_filtered_successors<'a, 'tcx>(terminator: &'a Terminator<'tcx>) -> Covera /// CoverageGraph outside all loops. This supports traversing the BCB CFG in a way that /// ensures a loop is completely traversed before processing Blocks after the end of the loop. #[derive(Debug)] -pub(super) struct TraversalContext { +struct TraversalContext { /// BCB with one or more incoming loop backedges, indicating which loop /// this context is for. /// @@ -375,7 +379,7 @@ pub(super) struct TraversalContext { worklist: VecDeque, } -pub(super) struct TraverseCoverageGraphWithLoops<'a> { +pub(crate) struct TraverseCoverageGraphWithLoops<'a> { basic_coverage_blocks: &'a CoverageGraph, backedges: IndexVec>, @@ -384,7 +388,7 @@ pub(super) struct TraverseCoverageGraphWithLoops<'a> { } impl<'a> TraverseCoverageGraphWithLoops<'a> { - pub(super) fn new(basic_coverage_blocks: &'a CoverageGraph) -> Self { + pub(crate) fn new(basic_coverage_blocks: &'a CoverageGraph) -> Self { let backedges = find_loop_backedges(basic_coverage_blocks); let worklist = VecDeque::from([basic_coverage_blocks.start_node()]); @@ -400,7 +404,7 @@ impl<'a> TraverseCoverageGraphWithLoops<'a> { /// For each loop on the loop context stack (top-down), yields a list of BCBs /// within that loop that have an outgoing edge back to the loop header. - pub(super) fn reloop_bcbs_per_loop(&self) -> impl Iterator { + pub(crate) fn reloop_bcbs_per_loop(&self) -> impl Iterator { self.context_stack .iter() .rev() @@ -408,39 +412,38 @@ impl<'a> TraverseCoverageGraphWithLoops<'a> { .map(|header_bcb| self.backedges[header_bcb].as_slice()) } - pub(super) fn next(&mut self) -> Option { + pub(crate) fn next(&mut self) -> Option { debug!( "TraverseCoverageGraphWithLoops::next - context_stack: {:?}", self.context_stack.iter().rev().collect::>() ); while let Some(context) = self.context_stack.last_mut() { - if let Some(bcb) = context.worklist.pop_front() { - if !self.visited.insert(bcb) { - debug!("Already visited: {bcb:?}"); - continue; - } - debug!("Visiting {bcb:?}"); - - if self.backedges[bcb].len() > 0 { - debug!("{bcb:?} is a loop header! Start a new TraversalContext..."); - self.context_stack.push(TraversalContext { - loop_header: Some(bcb), - worklist: VecDeque::new(), - }); - } - self.add_successors_to_worklists(bcb); - return Some(bcb); - } else { - // Strip contexts with empty worklists from the top of the stack + let Some(bcb) = context.worklist.pop_front() else { + // This stack level is exhausted; pop it and try the next one. self.context_stack.pop(); + continue; + }; + + if !self.visited.insert(bcb) { + debug!("Already visited: {bcb:?}"); + continue; } + debug!("Visiting {bcb:?}"); + + if self.backedges[bcb].len() > 0 { + debug!("{bcb:?} is a loop header! Start a new TraversalContext..."); + self.context_stack + .push(TraversalContext { loop_header: Some(bcb), worklist: VecDeque::new() }); + } + self.add_successors_to_worklists(bcb); + return Some(bcb); } None } - pub fn add_successors_to_worklists(&mut self, bcb: BasicCoverageBlock) { + fn add_successors_to_worklists(&mut self, bcb: BasicCoverageBlock) { let successors = &self.basic_coverage_blocks.successors[bcb]; debug!("{:?} has {} successors:", bcb, successors.len()); @@ -494,11 +497,11 @@ impl<'a> TraverseCoverageGraphWithLoops<'a> { } } - pub fn is_complete(&self) -> bool { + pub(crate) fn is_complete(&self) -> bool { self.visited.count() == self.visited.domain_size() } - pub fn unvisited(&self) -> Vec { + pub(crate) fn unvisited(&self) -> Vec { let mut unvisited_set: BitSet = BitSet::new_filled(self.visited.domain_size()); unvisited_set.subtract(&self.visited); @@ -506,7 +509,7 @@ impl<'a> TraverseCoverageGraphWithLoops<'a> { } } -pub(super) fn find_loop_backedges( +fn find_loop_backedges( basic_coverage_blocks: &CoverageGraph, ) -> IndexVec> { let num_bcbs = basic_coverage_blocks.num_nodes(); diff --git a/compiler/rustc_mir_transform/src/coverage/tests.rs b/compiler/rustc_mir_transform/src/coverage/tests.rs index ca64688e6b8..048547dc9f5 100644 --- a/compiler/rustc_mir_transform/src/coverage/tests.rs +++ b/compiler/rustc_mir_transform/src/coverage/tests.rs @@ -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; @@ -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::>() - ); - 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::(), - 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::(), - 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::(), - 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() - ); - }); -}