about summary refs log tree commit diff
path: root/compiler/rustc_mir_transform/src/coverage/mod.rs
diff options
context:
space:
mode:
authorGuillaume Gomez <guillaume1.gomez@gmail.com>2025-05-06 19:27:39 +0200
committerGitHub <noreply@github.com>2025-05-06 19:27:39 +0200
commit3b82d4640aaeec3cd66b96bc1c7206d69d30b2de (patch)
tree59069572434167343fdea35246000a28d33fb31d /compiler/rustc_mir_transform/src/coverage/mod.rs
parent8929f8a180f09b25ff405f1f20416b97720d90df (diff)
parent77a7ae4e9fcbf3be3988c98015d7f52137bb237c (diff)
downloadrust-3b82d4640aaeec3cd66b96bc1c7206d69d30b2de.tar.gz
rust-3b82d4640aaeec3cd66b96bc1c7206d69d30b2de.zip
Rollup merge of #139966 - Zalathar:span-merge, r=oli-obk
coverage: Only merge adjacent coverage spans

For a long time, coverage instrumentation has automatically “merged” spans with the same control-flow into a smaller number of larger spans, even when the spans being merged are not overlapping or adjacent. This causes any source text between the original spans to be included in the merged span, which is then associated with an execution count when shown in coverage reports.

That approach causes a number of problems:
- The intervening source text can contain all sorts of things that shouldn't really be marked as executable code (e.g. nested items, parts of macro invocations, long comments). In some cases we have complicated workarounds (e.g. bucketing to avoid merging spans across nested items), but in other cases there isn't much we can do.
- Merging can have aesthetically weird effects, such as including unbalanced parentheses, because the merging process doesn't really understand what it's doing at a source code level.
- It generally leads to an accumulation of piled-on heuristics and special cases that give decent-looking results, but are fiendishly difficult to modify or replace.

Therefore, this PR aims to abolish the merging of non-adjacent coverage spans.

The big tradeoff here is that the resulting coverage metadata (embedded in the instrumented binary) tends to become larger, because the overall number of distinct spans has increased. That's unfortunate, but I see it as the inevitable cost of cleaning up the messes and inaccuracies that were caused by the old approach. And the resulting spans do tend to be more accurate to the program's actual control-flow.

---

The `.coverage` snapshot changes give an indication of how this PR will affect user-visible coverage reports. In many cases the changes to reporting are minor or even nonexistent, despite substantial changes to the metadata (as indicated by `.cov-map` snapshots).

---

try-job: aarch64-gnu
Diffstat (limited to 'compiler/rustc_mir_transform/src/coverage/mod.rs')
-rw-r--r--compiler/rustc_mir_transform/src/coverage/mod.rs32
1 files changed, 11 insertions, 21 deletions
diff --git a/compiler/rustc_mir_transform/src/coverage/mod.rs b/compiler/rustc_mir_transform/src/coverage/mod.rs
index f2e8f9e1bcd..702c62eddc7 100644
--- a/compiler/rustc_mir_transform/src/coverage/mod.rs
+++ b/compiler/rustc_mir_transform/src/coverage/mod.rs
@@ -268,9 +268,9 @@ fn inject_statement(mir_body: &mut mir::Body<'_>, counter_kind: CoverageKind, bb
 struct ExtractedHirInfo {
     function_source_hash: u64,
     is_async_fn: bool,
-    /// The span of the function's signature, extended to the start of `body_span`.
+    /// The span of the function's signature, if available.
     /// Must have the same context and filename as the body span.
-    fn_sig_span_extended: Option<Span>,
+    fn_sig_span: Option<Span>,
     body_span: Span,
     /// "Holes" are regions within the function body (or its expansions) that
     /// should not be included in coverage spans for this function
@@ -308,30 +308,20 @@ fn extract_hir_info<'tcx>(tcx: TyCtxt<'tcx>, def_id: LocalDefId) -> ExtractedHir
 
     // The actual signature span is only used if it has the same context and
     // filename as the body, and precedes the body.
-    let fn_sig_span_extended = maybe_fn_sig
-        .map(|fn_sig| fn_sig.span)
-        .filter(|&fn_sig_span| {
-            let source_map = tcx.sess.source_map();
-            let file_idx = |span: Span| source_map.lookup_source_file_idx(span.lo());
-
-            fn_sig_span.eq_ctxt(body_span)
-                && fn_sig_span.hi() <= body_span.lo()
-                && file_idx(fn_sig_span) == file_idx(body_span)
-        })
-        // If so, extend it to the start of the body span.
-        .map(|fn_sig_span| fn_sig_span.with_hi(body_span.lo()));
+    let fn_sig_span = maybe_fn_sig.map(|fn_sig| fn_sig.span).filter(|&fn_sig_span| {
+        let source_map = tcx.sess.source_map();
+        let file_idx = |span: Span| source_map.lookup_source_file_idx(span.lo());
+
+        fn_sig_span.eq_ctxt(body_span)
+            && fn_sig_span.hi() <= body_span.lo()
+            && file_idx(fn_sig_span) == file_idx(body_span)
+    });
 
     let function_source_hash = hash_mir_source(tcx, hir_body);
 
     let hole_spans = extract_hole_spans_from_hir(tcx, hir_body);
 
-    ExtractedHirInfo {
-        function_source_hash,
-        is_async_fn,
-        fn_sig_span_extended,
-        body_span,
-        hole_spans,
-    }
+    ExtractedHirInfo { function_source_hash, is_async_fn, fn_sig_span, body_span, hole_spans }
 }
 
 fn hash_mir_source<'tcx>(tcx: TyCtxt<'tcx>, hir_body: &'tcx hir::Body<'tcx>) -> u64 {