From 3f67118ae7dcd7c35ca18f23cac787254004bc62 Mon Sep 17 00:00:00 2001 From: Zalathar Date: Sat, 30 Dec 2023 22:36:11 +1100 Subject: [PATCH] coverage: Make `coverage_counters` a local variable This avoids the awkwardness of having to create it in the pass's constructor, and then mutate it later to actually create the counters. --- .../src/coverage/counters.rs | 30 ++++++++--------- .../rustc_mir_transform/src/coverage/mod.rs | 32 +++++++++++-------- .../rustc_mir_transform/src/coverage/tests.rs | 6 ++-- 3 files changed, 37 insertions(+), 31 deletions(-) diff --git a/compiler/rustc_mir_transform/src/coverage/counters.rs b/compiler/rustc_mir_transform/src/coverage/counters.rs index d995d562521da..8c11dea5d4ee6 100644 --- a/compiler/rustc_mir_transform/src/coverage/counters.rs +++ b/compiler/rustc_mir_transform/src/coverage/counters.rs @@ -61,27 +61,27 @@ pub(super) struct CoverageCounters { } impl CoverageCounters { - pub(super) fn new(basic_coverage_blocks: &CoverageGraph) -> Self { + /// Makes [`BcbCounter`] `Counter`s and `Expressions` for the `BasicCoverageBlock`s directly or + /// indirectly associated with coverage spans, and accumulates additional `Expression`s + /// representing intermediate values. + pub(super) fn make_bcb_counters( + basic_coverage_blocks: &CoverageGraph, + bcb_has_coverage_spans: impl Fn(BasicCoverageBlock) -> bool, + ) -> Self { let num_bcbs = basic_coverage_blocks.num_nodes(); - Self { + let mut this = Self { next_counter_id: CounterId::START, bcb_counters: IndexVec::from_elem_n(None, num_bcbs), bcb_edge_counters: FxIndexMap::default(), bcb_has_incoming_edge_counters: BitSet::new_empty(num_bcbs), expressions: IndexVec::new(), - } - } + }; - /// Makes [`BcbCounter`] `Counter`s and `Expressions` for the `BasicCoverageBlock`s directly or - /// indirectly associated with coverage spans, and accumulates additional `Expression`s - /// representing intermediate values. - pub fn make_bcb_counters( - &mut self, - basic_coverage_blocks: &CoverageGraph, - bcb_has_coverage_spans: impl Fn(BasicCoverageBlock) -> bool, - ) { - MakeBcbCounters::new(self, basic_coverage_blocks).make_bcb_counters(bcb_has_coverage_spans) + MakeBcbCounters::new(&mut this, basic_coverage_blocks) + .make_bcb_counters(bcb_has_coverage_spans); + + this } fn make_counter(&mut self) -> BcbCounter { @@ -189,8 +189,8 @@ impl CoverageCounters { .map(|(&(from_bcb, to_bcb), counter_kind)| (from_bcb, to_bcb, counter_kind)) } - pub(super) fn take_expressions(&mut self) -> IndexVec { - std::mem::take(&mut self.expressions) + pub(super) fn into_expressions(self) -> IndexVec { + self.expressions } } diff --git a/compiler/rustc_mir_transform/src/coverage/mod.rs b/compiler/rustc_mir_transform/src/coverage/mod.rs index 95562344ec813..d2ec5d7d7cca0 100644 --- a/compiler/rustc_mir_transform/src/coverage/mod.rs +++ b/compiler/rustc_mir_transform/src/coverage/mod.rs @@ -70,7 +70,6 @@ struct Instrumentor<'a, 'tcx> { mir_body: &'a mut mir::Body<'tcx>, hir_info: ExtractedHirInfo, basic_coverage_blocks: CoverageGraph, - coverage_counters: CoverageCounters, } impl<'a, 'tcx> Instrumentor<'a, 'tcx> { @@ -80,9 +79,8 @@ impl<'a, 'tcx> Instrumentor<'a, 'tcx> { debug!(?hir_info, "instrumenting {:?}", mir_body.source.def_id()); let basic_coverage_blocks = CoverageGraph::from_mir(mir_body); - let coverage_counters = CoverageCounters::new(&basic_coverage_blocks); - Self { tcx, mir_body, hir_info, basic_coverage_blocks, coverage_counters } + Self { tcx, mir_body, hir_info, basic_coverage_blocks } } fn inject_counters(&'a mut self) { @@ -103,16 +101,18 @@ impl<'a, 'tcx> Instrumentor<'a, 'tcx> { // and all `Expression` dependencies (operands) are also generated, for any other // `BasicCoverageBlock`s not already associated with a coverage span. let bcb_has_coverage_spans = |bcb| coverage_spans.bcb_has_coverage_spans(bcb); - self.coverage_counters - .make_bcb_counters(&self.basic_coverage_blocks, bcb_has_coverage_spans); + let coverage_counters = CoverageCounters::make_bcb_counters( + &self.basic_coverage_blocks, + bcb_has_coverage_spans, + ); - let mappings = self.create_mappings(&coverage_spans); - self.inject_coverage_statements(bcb_has_coverage_spans); + let mappings = self.create_mappings(&coverage_spans, &coverage_counters); + self.inject_coverage_statements(bcb_has_coverage_spans, &coverage_counters); self.mir_body.function_coverage_info = Some(Box::new(FunctionCoverageInfo { function_source_hash: self.hir_info.function_source_hash, - num_counters: self.coverage_counters.num_counters(), - expressions: self.coverage_counters.take_expressions(), + num_counters: coverage_counters.num_counters(), + expressions: coverage_counters.into_expressions(), mappings, })); } @@ -122,7 +122,11 @@ impl<'a, 'tcx> Instrumentor<'a, 'tcx> { /// /// Precondition: All BCBs corresponding to those spans have been given /// coverage counters. - fn create_mappings(&self, coverage_spans: &CoverageSpans) -> Vec { + fn create_mappings( + &self, + coverage_spans: &CoverageSpans, + coverage_counters: &CoverageCounters, + ) -> Vec { let source_map = self.tcx.sess.source_map(); let body_span = self.hir_info.body_span; @@ -135,8 +139,7 @@ impl<'a, 'tcx> Instrumentor<'a, 'tcx> { .bcbs_with_coverage_spans() // For each BCB with spans, get a coverage term for its counter. .map(|(bcb, spans)| { - let term = self - .coverage_counters + let term = coverage_counters .bcb_counter(bcb) .expect("all BCBs with spans were given counters") .as_term(); @@ -157,9 +160,10 @@ impl<'a, 'tcx> Instrumentor<'a, 'tcx> { fn inject_coverage_statements( &mut self, bcb_has_coverage_spans: impl Fn(BasicCoverageBlock) -> bool, + coverage_counters: &CoverageCounters, ) { // Process the counters associated with BCB nodes. - for (bcb, counter_kind) in self.coverage_counters.bcb_node_counters() { + for (bcb, counter_kind) in coverage_counters.bcb_node_counters() { let do_inject = match counter_kind { // Counter-increment statements always need to be injected. BcbCounter::Counter { .. } => true, @@ -178,7 +182,7 @@ impl<'a, 'tcx> Instrumentor<'a, 'tcx> { } // Process the counters associated with BCB edges. - for (from_bcb, to_bcb, counter_kind) in self.coverage_counters.bcb_edge_counters() { + for (from_bcb, to_bcb, counter_kind) in coverage_counters.bcb_edge_counters() { let do_inject = match counter_kind { // Counter-increment statements always need to be injected. BcbCounter::Counter { .. } => true, diff --git a/compiler/rustc_mir_transform/src/coverage/tests.rs b/compiler/rustc_mir_transform/src/coverage/tests.rs index 931bc8e58ffc1..d9a3c0cb162f3 100644 --- a/compiler/rustc_mir_transform/src/coverage/tests.rs +++ b/compiler/rustc_mir_transform/src/coverage/tests.rs @@ -630,8 +630,10 @@ fn test_make_bcb_counters() { // 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 mut coverage_counters = counters::CoverageCounters::new(&basic_coverage_blocks); - coverage_counters.make_bcb_counters(&basic_coverage_blocks, bcb_has_coverage_spans); + let coverage_counters = counters::CoverageCounters::make_bcb_counters( + &basic_coverage_blocks, + bcb_has_coverage_spans, + ); assert_eq!(coverage_counters.num_expressions(), 0); assert_eq!(