about summary refs log tree commit diff
diff options
context:
space:
mode:
authorZalathar <Zalathar@users.noreply.github.com>2024-07-04 23:52:49 +1000
committerZalathar <Zalathar@users.noreply.github.com>2024-07-05 13:53:05 +1000
commitf095de4bf105e92b035a7cd64c34bcce56b7cb78 (patch)
tree63008ff6829b5ad83b318db7129fc5b9d1850e0c
parent489233170abcc97f59a1ee2211535f0ff67a0b43 (diff)
downloadrust-f095de4bf105e92b035a7cd64c34bcce56b7cb78.tar.gz
rust-f095de4bf105e92b035a7cd64c34bcce56b7cb78.zip
coverage: Rename `mir::coverage::BranchInfo` to `CoverageInfoHi`
This opens the door to collecting and storing coverage information that is
unrelated to branch coverage or MC/DC.
-rw-r--r--compiler/rustc_middle/src/mir/coverage.rs11
-rw-r--r--compiler/rustc_middle/src/mir/mod.rs13
-rw-r--r--compiler/rustc_middle/src/mir/pretty.rs16
-rw-r--r--compiler/rustc_mir_build/src/build/coverageinfo.rs107
-rw-r--r--compiler/rustc_mir_build/src/build/coverageinfo/mcdc.rs12
-rw-r--r--compiler/rustc_mir_build/src/build/custom/mod.rs2
-rw-r--r--compiler/rustc_mir_build/src/build/matches/mod.rs4
-rw-r--r--compiler/rustc_mir_build/src/build/mod.rs8
-rw-r--r--compiler/rustc_mir_transform/src/coverage/mappings.rs22
9 files changed, 110 insertions, 85 deletions
diff --git a/compiler/rustc_middle/src/mir/coverage.rs b/compiler/rustc_middle/src/mir/coverage.rs
index da25fbb0a82..beaaadd497d 100644
--- a/compiler/rustc_middle/src/mir/coverage.rs
+++ b/compiler/rustc_middle/src/mir/coverage.rs
@@ -103,7 +103,7 @@ pub enum CoverageKind {
     SpanMarker,
 
     /// Marks its enclosing basic block with an ID that can be referred to by
-    /// side data in [`BranchInfo`].
+    /// side data in [`CoverageInfoHi`].
     ///
     /// Should be erased before codegen (at some point after `InstrumentCoverage`).
     BlockMarker { id: BlockMarkerId },
@@ -274,10 +274,15 @@ pub struct FunctionCoverageInfo {
     pub mcdc_num_condition_bitmaps: usize,
 }
 
-/// Branch information recorded during THIR-to-MIR lowering, and stored in MIR.
+/// Coverage information for a function, recorded during MIR building and
+/// attached to the corresponding `mir::Body`. Used by the `InstrumentCoverage`
+/// MIR pass.
+///
+/// ("Hi" indicates that this is "high-level" information collected at the
+/// THIR/MIR boundary, before the MIR-based coverage instrumentation pass.)
 #[derive(Clone, Debug)]
 #[derive(TyEncodable, TyDecodable, Hash, HashStable, TypeFoldable, TypeVisitable)]
-pub struct BranchInfo {
+pub struct CoverageInfoHi {
     /// 1 more than the highest-numbered [`CoverageKind::BlockMarker`] that was
     /// injected into the MIR body. This makes it possible to allocate per-ID
     /// data structures without having to scan the entire body first.
diff --git a/compiler/rustc_middle/src/mir/mod.rs b/compiler/rustc_middle/src/mir/mod.rs
index ef88b253864..83e3898cebf 100644
--- a/compiler/rustc_middle/src/mir/mod.rs
+++ b/compiler/rustc_middle/src/mir/mod.rs
@@ -430,11 +430,12 @@ pub struct Body<'tcx> {
 
     pub tainted_by_errors: Option<ErrorGuaranteed>,
 
-    /// Branch coverage information collected during MIR building, to be used by
-    /// the `InstrumentCoverage` pass.
+    /// Coverage information collected from THIR/MIR during MIR building,
+    /// to be used by the `InstrumentCoverage` pass.
     ///
-    /// Only present if branch coverage is enabled and this function is eligible.
-    pub coverage_branch_info: Option<Box<coverage::BranchInfo>>,
+    /// Only present if coverage is enabled and this function is eligible.
+    /// Boxed to limit space overhead in non-coverage builds.
+    pub coverage_info_hi: Option<Box<coverage::CoverageInfoHi>>,
 
     /// Per-function coverage information added by the `InstrumentCoverage`
     /// pass, to be used in conjunction with the coverage statements injected
@@ -484,7 +485,7 @@ impl<'tcx> Body<'tcx> {
             is_polymorphic: false,
             injection_phase: None,
             tainted_by_errors,
-            coverage_branch_info: None,
+            coverage_info_hi: None,
             function_coverage_info: None,
         };
         body.is_polymorphic = body.has_non_region_param();
@@ -515,7 +516,7 @@ impl<'tcx> Body<'tcx> {
             is_polymorphic: false,
             injection_phase: None,
             tainted_by_errors: None,
-            coverage_branch_info: None,
+            coverage_info_hi: None,
             function_coverage_info: None,
         };
         body.is_polymorphic = body.has_non_region_param();
diff --git a/compiler/rustc_middle/src/mir/pretty.rs b/compiler/rustc_middle/src/mir/pretty.rs
index 4453ce44b03..5b2c603ce25 100644
--- a/compiler/rustc_middle/src/mir/pretty.rs
+++ b/compiler/rustc_middle/src/mir/pretty.rs
@@ -473,8 +473,8 @@ pub fn write_mir_intro<'tcx>(
     // Add an empty line before the first block is printed.
     writeln!(w)?;
 
-    if let Some(branch_info) = &body.coverage_branch_info {
-        write_coverage_branch_info(branch_info, w)?;
+    if let Some(coverage_info_hi) = &body.coverage_info_hi {
+        write_coverage_info_hi(coverage_info_hi, w)?;
     }
     if let Some(function_coverage_info) = &body.function_coverage_info {
         write_function_coverage_info(function_coverage_info, w)?;
@@ -483,12 +483,16 @@ pub fn write_mir_intro<'tcx>(
     Ok(())
 }
 
-fn write_coverage_branch_info(
-    branch_info: &coverage::BranchInfo,
+fn write_coverage_info_hi(
+    coverage_info_hi: &coverage::CoverageInfoHi,
     w: &mut dyn io::Write,
 ) -> io::Result<()> {
-    let coverage::BranchInfo { branch_spans, mcdc_branch_spans, mcdc_decision_spans, .. } =
-        branch_info;
+    let coverage::CoverageInfoHi {
+        num_block_markers: _,
+        branch_spans,
+        mcdc_branch_spans,
+        mcdc_decision_spans,
+    } = coverage_info_hi;
 
     for coverage::BranchSpan { span, true_marker, false_marker } in branch_spans {
         writeln!(
diff --git a/compiler/rustc_mir_build/src/build/coverageinfo.rs b/compiler/rustc_mir_build/src/build/coverageinfo.rs
index 876faca5172..204ee45bfa2 100644
--- a/compiler/rustc_mir_build/src/build/coverageinfo.rs
+++ b/compiler/rustc_mir_build/src/build/coverageinfo.rs
@@ -2,7 +2,7 @@ use std::assert_matches::assert_matches;
 use std::collections::hash_map::Entry;
 
 use rustc_data_structures::fx::FxHashMap;
-use rustc_middle::mir::coverage::{BlockMarkerId, BranchSpan, CoverageKind};
+use rustc_middle::mir::coverage::{BlockMarkerId, BranchSpan, CoverageInfoHi, CoverageKind};
 use rustc_middle::mir::{self, BasicBlock, SourceInfo, UnOp};
 use rustc_middle::thir::{ExprId, ExprKind, Pat, Thir};
 use rustc_middle::ty::TyCtxt;
@@ -13,16 +13,25 @@ use crate::build::{Builder, CFG};
 
 mod mcdc;
 
-pub(crate) struct BranchInfoBuilder {
+/// Collects coverage-related information during MIR building, to eventually be
+/// turned into a function's [`CoverageInfoHi`] when MIR building is complete.
+pub(crate) struct CoverageInfoBuilder {
     /// Maps condition expressions to their enclosing `!`, for better instrumentation.
     nots: FxHashMap<ExprId, NotInfo>,
 
     markers: BlockMarkerGen,
-    branch_spans: Vec<BranchSpan>,
 
+    /// Present if branch coverage is enabled.
+    branch_info: Option<BranchInfo>,
+    /// Present if MC/DC coverage is enabled.
     mcdc_info: Option<MCDCInfoBuilder>,
 }
 
+#[derive(Default)]
+struct BranchInfo {
+    branch_spans: Vec<BranchSpan>,
+}
+
 #[derive(Clone, Copy)]
 struct NotInfo {
     /// When visiting the associated expression as a branch condition, treat this
@@ -62,20 +71,20 @@ impl BlockMarkerGen {
     }
 }
 
-impl BranchInfoBuilder {
-    /// Creates a new branch info builder, but only if branch coverage instrumentation
+impl CoverageInfoBuilder {
+    /// Creates a new coverage info builder, but only if coverage instrumentation
     /// is enabled and `def_id` represents a function that is eligible for coverage.
     pub(crate) fn new_if_enabled(tcx: TyCtxt<'_>, def_id: LocalDefId) -> Option<Self> {
-        if tcx.sess.instrument_coverage_branch() && tcx.is_eligible_for_coverage(def_id) {
-            Some(Self {
-                nots: FxHashMap::default(),
-                markers: BlockMarkerGen::default(),
-                branch_spans: vec![],
-                mcdc_info: tcx.sess.instrument_coverage_mcdc().then(MCDCInfoBuilder::new),
-            })
-        } else {
-            None
+        if !tcx.sess.instrument_coverage() || !tcx.is_eligible_for_coverage(def_id) {
+            return None;
         }
+
+        Some(Self {
+            nots: FxHashMap::default(),
+            markers: BlockMarkerGen::default(),
+            branch_info: tcx.sess.instrument_coverage_branch().then(BranchInfo::default),
+            mcdc_info: tcx.sess.instrument_coverage_mcdc().then(MCDCInfoBuilder::new),
+        })
     }
 
     /// Unary `!` expressions inside an `if` condition are lowered by lowering
@@ -88,6 +97,12 @@ impl BranchInfoBuilder {
     pub(crate) fn visit_unary_not(&mut self, thir: &Thir<'_>, unary_not: ExprId) {
         assert_matches!(thir[unary_not].kind, ExprKind::Unary { op: UnOp::Not, .. });
 
+        // The information collected by this visitor is only needed when branch
+        // coverage or higher is enabled.
+        if self.branch_info.is_none() {
+            return;
+        }
+
         self.visit_with_not_info(
             thir,
             unary_not,
@@ -137,40 +152,40 @@ impl BranchInfoBuilder {
                 false_block,
                 inject_block_marker,
             );
-        } else {
-            let true_marker = self.markers.inject_block_marker(cfg, source_info, true_block);
-            let false_marker = self.markers.inject_block_marker(cfg, source_info, false_block);
-
-            self.branch_spans.push(BranchSpan {
-                span: source_info.span,
-                true_marker,
-                false_marker,
-            });
+            return;
         }
+
+        // Bail out if branch coverage is not enabled.
+        let Some(branch_info) = self.branch_info.as_mut() else { return };
+
+        let true_marker = self.markers.inject_block_marker(cfg, source_info, true_block);
+        let false_marker = self.markers.inject_block_marker(cfg, source_info, false_block);
+
+        branch_info.branch_spans.push(BranchSpan {
+            span: source_info.span,
+            true_marker,
+            false_marker,
+        });
     }
 
-    pub(crate) fn into_done(self) -> Option<Box<mir::coverage::BranchInfo>> {
-        let Self {
-            nots: _,
-            markers: BlockMarkerGen { num_block_markers },
-            branch_spans,
-            mcdc_info,
-        } = self;
+    pub(crate) fn into_done(self) -> Box<CoverageInfoHi> {
+        let Self { nots: _, markers: BlockMarkerGen { num_block_markers }, branch_info, mcdc_info } =
+            self;
 
-        if num_block_markers == 0 {
-            assert!(branch_spans.is_empty());
-            return None;
-        }
+        let branch_spans =
+            branch_info.map(|branch_info| branch_info.branch_spans).unwrap_or_default();
 
         let (mcdc_decision_spans, mcdc_branch_spans) =
             mcdc_info.map(MCDCInfoBuilder::into_done).unwrap_or_default();
 
-        Some(Box::new(mir::coverage::BranchInfo {
+        // For simplicity, always return an info struct (without Option), even
+        // if there's nothing interesting in it.
+        Box::new(CoverageInfoHi {
             num_block_markers,
             branch_spans,
             mcdc_branch_spans,
             mcdc_decision_spans,
-        }))
+        })
     }
 }
 
@@ -184,7 +199,7 @@ impl<'tcx> Builder<'_, 'tcx> {
         block: &mut BasicBlock,
     ) {
         // Bail out if condition coverage is not enabled for this function.
-        let Some(branch_info) = self.coverage_branch_info.as_mut() else { return };
+        let Some(coverage_info) = self.coverage_info.as_mut() else { return };
         if !self.tcx.sess.instrument_coverage_condition() {
             return;
         };
@@ -224,7 +239,7 @@ impl<'tcx> Builder<'_, 'tcx> {
         );
 
         // Separate path for handling branches when MC/DC is enabled.
-        branch_info.register_two_way_branch(
+        coverage_info.register_two_way_branch(
             self.tcx,
             &mut self.cfg,
             source_info,
@@ -247,12 +262,12 @@ impl<'tcx> Builder<'_, 'tcx> {
         mut then_block: BasicBlock,
         mut else_block: BasicBlock,
     ) {
-        // Bail out if branch coverage is not enabled for this function.
-        let Some(branch_info) = self.coverage_branch_info.as_mut() else { return };
+        // Bail out if coverage is not enabled for this function.
+        let Some(coverage_info) = self.coverage_info.as_mut() else { return };
 
         // If this condition expression is nested within one or more `!` expressions,
         // replace it with the enclosing `!` collected by `visit_unary_not`.
-        if let Some(&NotInfo { enclosing_not, is_flipped }) = branch_info.nots.get(&expr_id) {
+        if let Some(&NotInfo { enclosing_not, is_flipped }) = coverage_info.nots.get(&expr_id) {
             expr_id = enclosing_not;
             if is_flipped {
                 std::mem::swap(&mut then_block, &mut else_block);
@@ -261,7 +276,7 @@ impl<'tcx> Builder<'_, 'tcx> {
 
         let source_info = SourceInfo { span: self.thir[expr_id].span, scope: self.source_scope };
 
-        branch_info.register_two_way_branch(
+        coverage_info.register_two_way_branch(
             self.tcx,
             &mut self.cfg,
             source_info,
@@ -280,13 +295,11 @@ impl<'tcx> Builder<'_, 'tcx> {
         true_block: BasicBlock,
         false_block: BasicBlock,
     ) {
-        // Bail out if branch coverage is not enabled for this function.
-        let Some(branch_info) = self.coverage_branch_info.as_mut() else { return };
-
-        // FIXME(#124144) This may need special handling when MC/DC is enabled.
+        // Bail out if coverage is not enabled for this function.
+        let Some(coverage_info) = self.coverage_info.as_mut() else { return };
 
         let source_info = SourceInfo { span: pattern.span, scope: self.source_scope };
-        branch_info.register_two_way_branch(
+        coverage_info.register_two_way_branch(
             self.tcx,
             &mut self.cfg,
             source_info,
diff --git a/compiler/rustc_mir_build/src/build/coverageinfo/mcdc.rs b/compiler/rustc_mir_build/src/build/coverageinfo/mcdc.rs
index f97e9ef60a2..3aa6e708476 100644
--- a/compiler/rustc_mir_build/src/build/coverageinfo/mcdc.rs
+++ b/compiler/rustc_mir_build/src/build/coverageinfo/mcdc.rs
@@ -250,24 +250,24 @@ impl MCDCInfoBuilder {
 
 impl Builder<'_, '_> {
     pub(crate) fn visit_coverage_branch_operation(&mut self, logical_op: LogicalOp, span: Span) {
-        if let Some(branch_info) = self.coverage_branch_info.as_mut()
-            && let Some(mcdc_info) = branch_info.mcdc_info.as_mut()
+        if let Some(coverage_info) = self.coverage_info.as_mut()
+            && let Some(mcdc_info) = coverage_info.mcdc_info.as_mut()
         {
             mcdc_info.state.record_conditions(logical_op, span);
         }
     }
 
     pub(crate) fn mcdc_increment_depth_if_enabled(&mut self) {
-        if let Some(branch_info) = self.coverage_branch_info.as_mut()
-            && let Some(mcdc_info) = branch_info.mcdc_info.as_mut()
+        if let Some(coverage_info) = self.coverage_info.as_mut()
+            && let Some(mcdc_info) = coverage_info.mcdc_info.as_mut()
         {
             mcdc_info.state.decision_ctx_stack.push(MCDCDecisionCtx::default());
         };
     }
 
     pub(crate) fn mcdc_decrement_depth_if_enabled(&mut self) {
-        if let Some(branch_info) = self.coverage_branch_info.as_mut()
-            && let Some(mcdc_info) = branch_info.mcdc_info.as_mut()
+        if let Some(coverage_info) = self.coverage_info.as_mut()
+            && let Some(mcdc_info) = coverage_info.mcdc_info.as_mut()
         {
             if mcdc_info.state.decision_ctx_stack.pop().is_none() {
                 bug!("Unexpected empty decision stack");
diff --git a/compiler/rustc_mir_build/src/build/custom/mod.rs b/compiler/rustc_mir_build/src/build/custom/mod.rs
index a0a512a2eff..f6ebcbcbdc9 100644
--- a/compiler/rustc_mir_build/src/build/custom/mod.rs
+++ b/compiler/rustc_mir_build/src/build/custom/mod.rs
@@ -62,7 +62,7 @@ pub(super) fn build_custom_mir<'tcx>(
         tainted_by_errors: None,
         injection_phase: None,
         pass_count: 0,
-        coverage_branch_info: None,
+        coverage_info_hi: None,
         function_coverage_info: None,
     };
 
diff --git a/compiler/rustc_mir_build/src/build/matches/mod.rs b/compiler/rustc_mir_build/src/build/matches/mod.rs
index efed52231e3..e435e2f9288 100644
--- a/compiler/rustc_mir_build/src/build/matches/mod.rs
+++ b/compiler/rustc_mir_build/src/build/matches/mod.rs
@@ -160,8 +160,8 @@ impl<'a, 'tcx> Builder<'a, 'tcx> {
                 // Improve branch coverage instrumentation by noting conditions
                 // nested within one or more `!` expressions.
                 // (Skipped if branch coverage is not enabled.)
-                if let Some(branch_info) = this.coverage_branch_info.as_mut() {
-                    branch_info.visit_unary_not(this.thir, expr_id);
+                if let Some(coverage_info) = this.coverage_info.as_mut() {
+                    coverage_info.visit_unary_not(this.thir, expr_id);
                 }
 
                 let local_scope = this.local_scope();
diff --git a/compiler/rustc_mir_build/src/build/mod.rs b/compiler/rustc_mir_build/src/build/mod.rs
index 601e5d4d3dc..0f9746cb719 100644
--- a/compiler/rustc_mir_build/src/build/mod.rs
+++ b/compiler/rustc_mir_build/src/build/mod.rs
@@ -218,8 +218,8 @@ struct Builder<'a, 'tcx> {
     lint_level_roots_cache: GrowableBitSet<hir::ItemLocalId>,
 
     /// Collects additional coverage information during MIR building.
-    /// Only present if branch coverage is enabled and this function is eligible.
-    coverage_branch_info: Option<coverageinfo::BranchInfoBuilder>,
+    /// Only present if coverage is enabled and this function is eligible.
+    coverage_info: Option<coverageinfo::CoverageInfoBuilder>,
 }
 
 type CaptureMap<'tcx> = SortedIndexMultiMap<usize, HirId, Capture<'tcx>>;
@@ -773,7 +773,7 @@ impl<'a, 'tcx> Builder<'a, 'tcx> {
             unit_temp: None,
             var_debug_info: vec![],
             lint_level_roots_cache: GrowableBitSet::new_empty(),
-            coverage_branch_info: coverageinfo::BranchInfoBuilder::new_if_enabled(tcx, def),
+            coverage_info: coverageinfo::CoverageInfoBuilder::new_if_enabled(tcx, def),
         };
 
         assert_eq!(builder.cfg.start_new_block(), START_BLOCK);
@@ -802,7 +802,7 @@ impl<'a, 'tcx> Builder<'a, 'tcx> {
             self.coroutine,
             None,
         );
-        body.coverage_branch_info = self.coverage_branch_info.and_then(|b| b.into_done());
+        body.coverage_info_hi = self.coverage_info.map(|b| b.into_done());
         body
     }
 
diff --git a/compiler/rustc_mir_transform/src/coverage/mappings.rs b/compiler/rustc_mir_transform/src/coverage/mappings.rs
index 235992ac547..25297245172 100644
--- a/compiler/rustc_mir_transform/src/coverage/mappings.rs
+++ b/compiler/rustc_mir_transform/src/coverage/mappings.rs
@@ -3,7 +3,9 @@ use std::collections::BTreeSet;
 use rustc_data_structures::graph::DirectedGraph;
 use rustc_index::bit_set::BitSet;
 use rustc_index::IndexVec;
-use rustc_middle::mir::coverage::{BlockMarkerId, BranchSpan, ConditionInfo, CoverageKind};
+use rustc_middle::mir::coverage::{
+    BlockMarkerId, BranchSpan, ConditionInfo, CoverageInfoHi, CoverageKind,
+};
 use rustc_middle::mir::{self, BasicBlock, StatementKind};
 use rustc_middle::ty::TyCtxt;
 use rustc_span::Span;
@@ -157,12 +159,12 @@ impl ExtractedMappings {
 }
 
 fn resolve_block_markers(
-    branch_info: &mir::coverage::BranchInfo,
+    coverage_info_hi: &CoverageInfoHi,
     mir_body: &mir::Body<'_>,
 ) -> IndexVec<BlockMarkerId, Option<BasicBlock>> {
     let mut block_markers = IndexVec::<BlockMarkerId, Option<BasicBlock>>::from_elem_n(
         None,
-        branch_info.num_block_markers,
+        coverage_info_hi.num_block_markers,
     );
 
     // Fill out the mapping from block marker IDs to their enclosing blocks.
@@ -188,11 +190,11 @@ pub(super) fn extract_branch_pairs(
     hir_info: &ExtractedHirInfo,
     basic_coverage_blocks: &CoverageGraph,
 ) -> Vec<BranchPair> {
-    let Some(branch_info) = mir_body.coverage_branch_info.as_deref() else { return vec![] };
+    let Some(coverage_info_hi) = mir_body.coverage_info_hi.as_deref() else { return vec![] };
 
-    let block_markers = resolve_block_markers(branch_info, mir_body);
+    let block_markers = resolve_block_markers(coverage_info_hi, mir_body);
 
-    branch_info
+    coverage_info_hi
         .branch_spans
         .iter()
         .filter_map(|&BranchSpan { span: raw_span, true_marker, false_marker }| {
@@ -222,9 +224,9 @@ pub(super) fn extract_mcdc_mappings(
     mcdc_branches: &mut impl Extend<MCDCBranch>,
     mcdc_decisions: &mut impl Extend<MCDCDecision>,
 ) {
-    let Some(branch_info) = mir_body.coverage_branch_info.as_deref() else { return };
+    let Some(coverage_info_hi) = mir_body.coverage_info_hi.as_deref() else { return };
 
-    let block_markers = resolve_block_markers(branch_info, mir_body);
+    let block_markers = resolve_block_markers(coverage_info_hi, mir_body);
 
     let bcb_from_marker =
         |marker: BlockMarkerId| basic_coverage_blocks.bcb_from_bb(block_markers[marker]?);
@@ -243,7 +245,7 @@ pub(super) fn extract_mcdc_mappings(
             Some((span, true_bcb, false_bcb))
         };
 
-    mcdc_branches.extend(branch_info.mcdc_branch_spans.iter().filter_map(
+    mcdc_branches.extend(coverage_info_hi.mcdc_branch_spans.iter().filter_map(
         |&mir::coverage::MCDCBranchSpan {
              span: raw_span,
              condition_info,
@@ -257,7 +259,7 @@ pub(super) fn extract_mcdc_mappings(
         },
     ));
 
-    mcdc_decisions.extend(branch_info.mcdc_decision_spans.iter().filter_map(
+    mcdc_decisions.extend(coverage_info_hi.mcdc_decision_spans.iter().filter_map(
         |decision: &mir::coverage::MCDCDecisionSpan| {
             let span = unexpand_into_body_span(decision.span, body_span)?;