Output Branch Regions from Coverage Instrumentation

Author: Swatinem

compiler/rustc_mir_transform/src/coverage/counters.rs

@@ -69,8 +69,31 @@ impl CoverageCounters {
         &mut self,
         basic_coverage_blocks: &CoverageGraph,
         coverage_spans: &[CoverageSpan],
-    ) -> Result<(), Error> {
-        MakeBcbCounters::new(self, basic_coverage_blocks).make_bcb_counters(coverage_spans)
+    ) -> Result<Vec<(BasicCoverageBlock, CoverageKind)>, Error> {
+        let mut make_counters = MakeBcbCounters::new(self, basic_coverage_blocks);
+        make_counters.make_bcb_counters(coverage_spans)?;
+
+        let branches: Vec<_> = basic_coverage_blocks
+            .iter_enumerated()
+            .filter_map(|(bcb, _data)| {
+                let branches = make_counters.bcb_branches(bcb);
+                if branches.len() != 2 {
+                    return None;
+                }
+                let mut branch_counters =
+                    branches.iter().filter_map(|branch| make_counters.branch_counter(branch));
+
+                // FIXME(swatinem): figure out what the correct ordering here is?
+                // In simple testing, the `false` (aka `0`) branch of `SwitchInt` seems to be first.
+                let false_ = branch_counters.next()?.as_operand();
+                let true_ = branch_counters.next()?.as_operand();
+
+                let branch = CoverageKind::Branch { true_, false_ };
+                Some((bcb, branch))
+            })
+            .collect();
+
+        Ok(branches)
     }
 
     fn make_counter<F>(&mut self, debug_block_label_fn: F) -> CoverageKind

compiler/rustc_mir_transform/src/coverage/mod.rs

@@ -202,11 +202,11 @@ impl<'a, 'tcx> Instrumentor<'a, 'tcx> {
         //
         // Intermediate expressions (used to compute other `Expression` values), which have no
         // direct association with any `BasicCoverageBlock`, are accumulated inside `coverage_counters`.
-        let result = self
+        let mut result = self
             .coverage_counters
             .make_bcb_counters(&mut self.basic_coverage_blocks, &coverage_spans);
 
-        if let Ok(()) = result {
+        if let Ok(branches) = result.as_mut() {
             // If debugging, add any intermediate expressions (which are not associated with any
             // BCB) to the `debug_used_expressions` map.
             if debug_used_expressions.is_enabled() {
@@ -231,6 +231,8 @@ impl<'a, 'tcx> Instrumentor<'a, 'tcx> {
                 &mut debug_used_expressions,
             );
 
+            self.inject_branch_counters(std::mem::take(branches));
+
             ////////////////////////////////////////////////////
             // For any remaining `BasicCoverageBlock` counters (that were not associated with
             // any `CoverageSpan`), inject `Coverage` statements (_without_ code region `Span`s)
@@ -275,6 +277,27 @@ impl<'a, 'tcx> Instrumentor<'a, 'tcx> {
         }
     }
 
+    fn inject_branch_counters(&mut self, branches: Vec<(BasicCoverageBlock, CoverageKind)>) {
+        let tcx = self.tcx;
+        let source_map = tcx.sess.source_map();
+        let body_span = self.body_span;
+        let file_name = Symbol::intern(&self.source_file.name.prefer_remapped().to_string_lossy());
+
+        for branch in branches {
+            let bcb_data = self.bcb_data(branch.0);
+            let terminator = bcb_data.terminator(self.mir_body);
+
+            let span = terminator.source_info.span;
+
+            // FIXME(swatinem): figure out what a good span for the conditional is.
+            // For trivial code examples, the `terminator` seems to be sufficient.
+            let code_region =
+                Some(make_code_region(source_map, file_name, &self.source_file, span, body_span));
+
+            inject_statement(self.mir_body, branch.1, bcb_data.last_bb(), code_region);
+        }
+    }
+
     /// Inject a counter for each `CoverageSpan`. There can be multiple `CoverageSpan`s for a given
     /// BCB, but only one actual counter needs to be incremented per BCB. `bb_counters` maps each
     /// `bcb` to its `Counter`, when injected. Subsequent `CoverageSpan`s for a BCB that already has

compiler/rustc_mir_transform/src/coverage/tests.rs

@@ -676,7 +676,7 @@ fn test_make_bcb_counters() {
             }
         }
         let mut coverage_counters = counters::CoverageCounters::new(0, &basic_coverage_blocks);
-        let () = coverage_counters
+        let _ = coverage_counters
             .make_bcb_counters(&mut basic_coverage_blocks, &coverage_spans)
             .expect("should be Ok");
         assert_eq!(coverage_counters.intermediate_expressions.len(), 0);

src/tools/compiletest/src/runtest.rs

@@ -18,7 +18,6 @@ use crate::ColorConfig;
 use regex::{Captures, Regex};
 use rustfix::{apply_suggestions, get_suggestions_from_json, Filter};
 
-use std::borrow::Cow;
 use std::collections::hash_map::DefaultHasher;
 use std::collections::{HashMap, HashSet};
 use std::env;
@@ -564,7 +563,12 @@ impl<'test> TestCx<'test> {
 
         // Run `llvm-cov show` to produce a coverage report in text format.
         let proc_res = self.run_llvm_tool("llvm-cov", |cmd| {
-            cmd.args(["show", "--format=text", "--show-line-counts-or-regions"]);
+            cmd.args([
+                "show",
+                "--format=text",
+                "--show-line-counts-or-regions",
+                "--show-branches=count",
+            ]);
 
             cmd.arg("--Xdemangler");
             cmd.arg(self.config.rust_demangler_path.as_ref().unwrap());
@@ -723,7 +727,7 @@ impl<'test> TestCx<'test> {
 
     /// Replace line numbers in coverage reports with the placeholder `LL`,
     /// so that the tests are less sensitive to lines being added/removed.
-    fn anonymize_coverage_line_numbers(coverage: &str) -> Cow<'_, str> {
+    fn anonymize_coverage_line_numbers(line: &str) -> String {
         // The coverage reporter prints line numbers at the start of a line.
         // They are truncated or left-padded to occupy exactly 5 columns.
         // (`LineNumberColumnWidth` in `SourceCoverageViewText.cpp`.)
@@ -733,7 +737,10 @@ impl<'test> TestCx<'test> {
         // have an additional prefix of `  |` for each nesting level.
         static LINE_NUMBER_RE: Lazy<Regex> =
             Lazy::new(|| Regex::new(r"(?m:^)(?<prefix>(?:  \|)*) *[0-9]+\|").unwrap());
-        LINE_NUMBER_RE.replace_all(coverage, "$prefix   LL|")
+        static BRANCH_LINE_RE: Lazy<Regex> =
+            Lazy::new(|| Regex::new(r"(?m:^)(?<prefix>(?:  \|)*)  Branch \([0-9]+:").unwrap());
+        let line = LINE_NUMBER_RE.replace_all(line, "$prefix   LL|");
+        BRANCH_LINE_RE.replace_all(&line, "$prefix  Branch (LL:").into_owned()
     }
 
     /// Coverage reports can describe multiple source files, separated by

src/tools/coverage-dump/src/covfun.rs

@@ -65,7 +65,9 @@ pub(crate) fn dump_covfun_mappings(
                 // If the mapping contains expressions, also print the resolved
                 // form of those expressions
                 kind.for_each_operand(|label, operand| {
-                    if matches!(operand, Operand::Expression { .. }) {
+                    if matches!(operand, Operand::Expression { .. })
+                        || matches!(kind, MappingKind::Branch { .. })
+                    {
                         let pad = if label.is_empty() { "" } else { " " };
                         let resolved = expression_resolver.format_operand(operand);
                         println!("    {label}{pad}= {resolved}");
@@ -207,7 +209,6 @@ impl Operand {
     }
 }
 
-#[derive(Debug)]
 enum MappingKind {
     Code(Operand),
     Gap(Operand),
@@ -216,6 +217,18 @@ enum MappingKind {
     Branch { true_: Operand, false_: Operand },
 }
 
+impl Debug for MappingKind {
+    fn fmt(&self, f: &mut fmt::Formatter<'_>) -> fmt::Result {
+        match self {
+            Self::Code(operand) => f.debug_tuple("Code").field(operand).finish(),
+            Self::Gap(operand) => f.debug_tuple("Gap").field(operand).finish(),
+            Self::Expansion(expansion) => f.debug_tuple("Expansion").field(expansion).finish(),
+            Self::Skip => write!(f, "Skip"),
+            Self::Branch { .. } => f.debug_tuple("Branch").finish(),
+        }
+    }
+}
+
 impl MappingKind {
     /// Visits each operand directly contained in this mapping, along with
     /// a string label (possibly empty).
@@ -226,8 +239,8 @@ impl MappingKind {
             Self::Expansion(_) => (),
             Self::Skip => (),
             Self::Branch { true_, false_ } => {
-                func("true_ ", true_);
-                func("false_", false_);
+                func("true ", true_);
+                func("false", false_);
             }
         }
     }