Skip to content

Commit cfd7cf5

Browse files
borsbors
committed
Auto merge of rust-lang#127199 - Zalathar:hir-holes, r=oli-obk
coverage: Extract hole spans from HIR instead of MIR This makes it possible to treat more kinds of nested item/code as holes, instead of being restricted to closures. (It also potentially opens up the possibility of using HIR holes to modify branch or MC/DC spans, though we currently don't actually do this.) Thus, this new implementation treats the following as holes: - Closures (as before, including `async` and coroutines) - All nested items - Inline `const` (because why not) This gives more accurate coverage reports, because lines occupied by holes don't show the execution count from the enclosing function. Fixes rust-lang#126626.
2 parents 59a4f02 + 63c04f0 commit cfd7cf5

20 files changed

+348
-150
lines changed

‎compiler/rustc_mir_transform/src/coverage/mod.rs

+77-3
Original file line numberDiff line numberDiff line change
@@ -8,6 +8,10 @@ mod spans;
88
mod tests;
99
mod unexpand;
1010

11+
use rustc_hir as hir;
12+
use rustc_hir::intravisit::{walk_expr, Visitor};
13+
use rustc_middle::hir::map::Map;
14+
use rustc_middle::hir::nested_filter;
1115
use rustc_middle::mir::coverage::{
1216
CodeRegion, CoverageKind, DecisionInfo, FunctionCoverageInfo, Mapping, MappingKind,
1317
};
@@ -465,6 +469,9 @@ struct ExtractedHirInfo {
465469
/// Must have the same context and filename as the body span.
466470
fn_sig_span_extended: Option<Span>,
467471
body_span: Span,
472+
/// "Holes" are regions within the body span that should not be included in
473+
/// coverage spans for this function (e.g. closures and nested items).
474+
hole_spans: Vec<Span>,
468475
}
469476

470477
fn extract_hir_info<'tcx>(tcx: TyCtxt<'tcx>, def_id: LocalDefId) -> ExtractedHirInfo {
@@ -480,7 +487,7 @@ fn extract_hir_info<'tcx>(tcx: TyCtxt<'tcx>, def_id: LocalDefId) -> ExtractedHir
480487

481488
let mut body_span = hir_body.value.span;
482489

483-
use rustc_hir::{Closure, Expr, ExprKind, Node};
490+
use hir::{Closure, Expr, ExprKind, Node};
484491
// Unexpand a closure's body span back to the context of its declaration.
485492
// This helps with closure bodies that consist of just a single bang-macro,
486493
// and also with closure bodies produced by async desugaring.
@@ -507,11 +514,78 @@ fn extract_hir_info<'tcx>(tcx: TyCtxt<'tcx>, def_id: LocalDefId) -> ExtractedHir
507514

508515
let function_source_hash = hash_mir_source(tcx, hir_body);
509516

510-
ExtractedHirInfo { function_source_hash, is_async_fn, fn_sig_span_extended, body_span }
517+
let hole_spans = extract_hole_spans_from_hir(tcx, body_span, hir_body);
518+
519+
ExtractedHirInfo {
520+
function_source_hash,
521+
is_async_fn,
522+
fn_sig_span_extended,
523+
body_span,
524+
hole_spans,
525+
}
511526
}
512527

513-
fn hash_mir_source<'tcx>(tcx: TyCtxt<'tcx>, hir_body: &'tcx rustc_hir::Body<'tcx>) -> u64 {
528+
fn hash_mir_source<'tcx>(tcx: TyCtxt<'tcx>, hir_body: &'tcx hir::Body<'tcx>) -> u64 {
514529
// FIXME(cjgillot) Stop hashing HIR manually here.
515530
let owner = hir_body.id().hir_id.owner;
516531
tcx.hir_owner_nodes(owner).opt_hash_including_bodies.unwrap().to_smaller_hash().as_u64()
517532
}
533+
534+
fn extract_hole_spans_from_hir<'tcx>(
535+
tcx: TyCtxt<'tcx>,
536+
body_span: Span, // Usually `hir_body.value.span`, but not always
537+
hir_body: &hir::Body<'tcx>,
538+
) -> Vec<Span> {
539+
struct HolesVisitor<'hir, F> {
540+
hir: Map<'hir>,
541+
visit_hole_span: F,
542+
}
543+
544+
impl<'hir, F: FnMut(Span)> Visitor<'hir> for HolesVisitor<'hir, F> {
545+
/// - We need `NestedFilter::INTRA = true` so that `visit_item` will be called.
546+
/// - Bodies of nested items don't actually get visited, because of the
547+
/// `visit_item` override.
548+
/// - For nested bodies that are not part of an item, we do want to visit any
549+
/// items contained within them.
550+
type NestedFilter = nested_filter::All;
551+
552+
fn nested_visit_map(&mut self) -> Self::Map {
553+
self.hir
554+
}
555+
556+
fn visit_item(&mut self, item: &'hir hir::Item<'hir>) {
557+
(self.visit_hole_span)(item.span);
558+
// Having visited this item, we don't care about its children,
559+
// so don't call `walk_item`.
560+
}
561+
562+
// We override `visit_expr` instead of the more specific expression
563+
// visitors, so that we have direct access to the expression span.
564+
fn visit_expr(&mut self, expr: &'hir hir::Expr<'hir>) {
565+
match expr.kind {
566+
hir::ExprKind::Closure(_) | hir::ExprKind::ConstBlock(_) => {
567+
(self.visit_hole_span)(expr.span);
568+
// Having visited this expression, we don't care about its
569+
// children, so don't call `walk_expr`.
570+
}
571+
572+
// For other expressions, recursively visit as normal.
573+
_ => walk_expr(self, expr),
574+
}
575+
}
576+
}
577+
578+
let mut hole_spans = vec![];
579+
let mut visitor = HolesVisitor {
580+
hir: tcx.hir(),
581+
visit_hole_span: |hole_span| {
582+
// Discard any holes that aren't directly visible within the body span.
583+
if body_span.contains(hole_span) && body_span.eq_ctxt(hole_span) {
584+
hole_spans.push(hole_span);
585+
}
586+
},
587+
};
588+
589+
visitor.visit_body(hir_body);
590+
hole_spans
591+
}

‎compiler/rustc_mir_transform/src/coverage/spans.rs

+4-3
Original file line numberDiff line numberDiff line change
@@ -8,7 +8,7 @@ use rustc_span::Span;
88
use crate::coverage::graph::{BasicCoverageBlock, CoverageGraph};
99
use crate::coverage::mappings;
1010
use crate::coverage::spans::from_mir::{
11-
extract_covspans_and_holes_from_mir, ExtractedCovspans, Hole, SpanFromMir,
11+
extract_covspans_from_mir, ExtractedCovspans, Hole, SpanFromMir,
1212
};
1313
use crate::coverage::ExtractedHirInfo;
1414

@@ -20,8 +20,8 @@ pub(super) fn extract_refined_covspans(
2020
basic_coverage_blocks: &CoverageGraph,
2121
code_mappings: &mut impl Extend<mappings::CodeMapping>,
2222
) {
23-
let ExtractedCovspans { mut covspans, mut holes } =
24-
extract_covspans_and_holes_from_mir(mir_body, hir_info, basic_coverage_blocks);
23+
let ExtractedCovspans { mut covspans } =
24+
extract_covspans_from_mir(mir_body, hir_info, basic_coverage_blocks);
2525

2626
// First, perform the passes that need macro information.
2727
covspans.sort_by(|a, b| basic_coverage_blocks.cmp_in_dominator_order(a.bcb, b.bcb));
@@ -45,6 +45,7 @@ pub(super) fn extract_refined_covspans(
4545
covspans.dedup_by(|b, a| a.span.source_equal(b.span));
4646

4747
// Sort the holes, and merge overlapping/adjacent holes.
48+
let mut holes = hir_info.hole_spans.iter().map(|&span| Hole { span }).collect::<Vec<_>>();
4849
holes.sort_by(|a, b| compare_spans(a.span, b.span));
4950
holes.dedup_by(|b, a| a.merge_if_overlapping_or_adjacent(b));
5051

‎compiler/rustc_mir_transform/src/coverage/spans/from_mir.rs

+6-35
Original file line numberDiff line numberDiff line change
@@ -1,8 +1,7 @@
11
use rustc_middle::bug;
22
use rustc_middle::mir::coverage::CoverageKind;
33
use rustc_middle::mir::{
4-
self, AggregateKind, FakeReadCause, Rvalue, Statement, StatementKind, Terminator,
5-
TerminatorKind,
4+
self, FakeReadCause, Statement, StatementKind, Terminator, TerminatorKind,
65
};
76
use rustc_span::{Span, Symbol};
87

@@ -15,43 +14,34 @@ use crate::coverage::ExtractedHirInfo;
1514

1615
pub(crate) struct ExtractedCovspans {
1716
pub(crate) covspans: Vec<SpanFromMir>,
18-
pub(crate) holes: Vec<Hole>,
1917
}
2018

2119
/// Traverses the MIR body to produce an initial collection of coverage-relevant
2220
/// spans, each associated with a node in the coverage graph (BCB) and possibly
2321
/// other metadata.
24-
pub(crate) fn extract_covspans_and_holes_from_mir(
22+
pub(crate) fn extract_covspans_from_mir(
2523
mir_body: &mir::Body<'_>,
2624
hir_info: &ExtractedHirInfo,
2725
basic_coverage_blocks: &CoverageGraph,
2826
) -> ExtractedCovspans {
2927
let &ExtractedHirInfo { body_span, .. } = hir_info;
3028

3129
let mut covspans = vec![];
32-
let mut holes = vec![];
3330

3431
for (bcb, bcb_data) in basic_coverage_blocks.iter_enumerated() {
35-
bcb_to_initial_coverage_spans(
36-
mir_body,
37-
body_span,
38-
bcb,
39-
bcb_data,
40-
&mut covspans,
41-
&mut holes,
42-
);
32+
bcb_to_initial_coverage_spans(mir_body, body_span, bcb, bcb_data, &mut covspans);
4333
}
4434

4535
// Only add the signature span if we found at least one span in the body.
46-
if !covspans.is_empty() || !holes.is_empty() {
36+
if !covspans.is_empty() {
4737
// If there is no usable signature span, add a fake one (before refinement)
4838
// to avoid an ugly gap between the body start and the first real span.
4939
// FIXME: Find a more principled way to solve this problem.
5040
let fn_sig_span = hir_info.fn_sig_span_extended.unwrap_or_else(|| body_span.shrink_to_lo());
5141
covspans.push(SpanFromMir::for_fn_sig(fn_sig_span));
5242
}
5343

54-
ExtractedCovspans { covspans, holes }
44+
ExtractedCovspans { covspans }
5545
}
5646

5747
// Generate a set of coverage spans from the filtered set of `Statement`s and `Terminator`s of
@@ -65,7 +55,6 @@ fn bcb_to_initial_coverage_spans<'a, 'tcx>(
6555
bcb: BasicCoverageBlock,
6656
bcb_data: &'a BasicCoverageBlockData,
6757
initial_covspans: &mut Vec<SpanFromMir>,
68-
holes: &mut Vec<Hole>,
6958
) {
7059
for &bb in &bcb_data.basic_blocks {
7160
let data = &mir_body[bb];
@@ -81,13 +70,7 @@ fn bcb_to_initial_coverage_spans<'a, 'tcx>(
8170
let expn_span = filtered_statement_span(statement)?;
8271
let (span, visible_macro) = unexpand(expn_span)?;
8372

84-
// A statement that looks like the assignment of a closure expression
85-
// is treated as a "hole" span, to be carved out of other spans.
86-
if is_closure_like(statement) {
87-
holes.push(Hole { span });
88-
} else {
89-
initial_covspans.push(SpanFromMir::new(span, visible_macro, bcb));
90-
}
73+
initial_covspans.push(SpanFromMir::new(span, visible_macro, bcb));
9174
Some(())
9275
};
9376
for statement in data.statements.iter() {
@@ -105,18 +88,6 @@ fn bcb_to_initial_coverage_spans<'a, 'tcx>(
10588
}
10689
}
10790

108-
fn is_closure_like(statement: &Statement<'_>) -> bool {
109-
match statement.kind {
110-
StatementKind::Assign(box (_, Rvalue::Aggregate(box ref agg_kind, _))) => match agg_kind {
111-
AggregateKind::Closure(_, _)
112-
| AggregateKind::Coroutine(_, _)
113-
| AggregateKind::CoroutineClosure(..) => true,
114-
_ => false,
115-
},
116-
_ => false,
117-
}
118-
}
119-
12091
/// If the MIR `Statement` has a span contributive to computing coverage spans,
12192
/// return it; otherwise return `None`.
12293
fn filtered_statement_span(statement: &Statement<'_>) -> Option<Span> {

‎tests/coverage-run-rustdoc/doctest.coverage

+4-4
Original file line numberDiff line numberDiff line change
@@ -34,10 +34,10 @@ $DIR/doctest.rs:
3434
LL| |//!
3535
LL| |//! doctest returning a result:
3636
LL| 1|//! ```
37-
LL| 1|//! #[derive(Debug, PartialEq)]
38-
LL| 1|//! struct SomeError {
39-
LL| 1|//! msg: String,
40-
LL| 1|//! }
37+
LL| |//! #[derive(Debug, PartialEq)]
38+
LL| |//! struct SomeError {
39+
LL| |//! msg: String,
40+
LL| |//! }
4141
LL| 1|//! let mut res = Err(SomeError { msg: String::from("a message") });
4242
LL| 1|//! if res.is_ok() {
4343
LL| 0|//! res?;

‎tests/coverage/async.cov-map

+21-20
Original file line numberDiff line numberDiff line change
@@ -167,15 +167,16 @@ Number of file 0 mappings: 14
167167
= ((c6 + c7) + c8)
168168

169169
Function name: async::j
170-
Raw bytes (53): 0x[01, 01, 02, 07, 0d, 05, 09, 09, 01, 35, 01, 13, 0c, 05, 14, 09, 00, 0a, 01, 00, 0e, 00, 1b, 05, 00, 1f, 00, 27, 09, 01, 09, 00, 0a, 11, 00, 0e, 00, 1a, 09, 00, 1e, 00, 20, 0d, 01, 0e, 00, 10, 03, 02, 01, 00, 02]
170+
Raw bytes (58): 0x[01, 01, 02, 07, 0d, 05, 09, 0a, 01, 35, 01, 00, 0d, 01, 0b, 0b, 00, 0c, 05, 01, 09, 00, 0a, 01, 00, 0e, 00, 1b, 05, 00, 1f, 00, 27, 09, 01, 09, 00, 0a, 11, 00, 0e, 00, 1a, 09, 00, 1e, 00, 20, 0d, 01, 0e, 00, 10, 03, 02, 01, 00, 02]
171171
Number of files: 1
172172
- file 0 => global file 1
173173
Number of expressions: 2
174174
- expression 0 operands: lhs = Expression(1, Add), rhs = Counter(3)
175175
- expression 1 operands: lhs = Counter(1), rhs = Counter(2)
176-
Number of file 0 mappings: 9
177-
- Code(Counter(0)) at (prev + 53, 1) to (start + 19, 12)
178-
- Code(Counter(1)) at (prev + 20, 9) to (start + 0, 10)
176+
Number of file 0 mappings: 10
177+
- Code(Counter(0)) at (prev + 53, 1) to (start + 0, 13)
178+
- Code(Counter(0)) at (prev + 11, 11) to (start + 0, 12)
179+
- Code(Counter(1)) at (prev + 1, 9) to (start + 0, 10)
179180
- Code(Counter(0)) at (prev + 0, 14) to (start + 0, 27)
180181
- Code(Counter(1)) at (prev + 0, 31) to (start + 0, 39)
181182
- Code(Counter(2)) at (prev + 1, 9) to (start + 0, 10)
@@ -186,48 +187,48 @@ Number of file 0 mappings: 9
186187
= ((c1 + c2) + c3)
187188

188189
Function name: async::j::c
189-
Raw bytes (26): 0x[01, 01, 01, 01, 05, 04, 01, 37, 05, 01, 12, 05, 02, 0d, 00, 0e, 02, 0a, 0d, 00, 0e, 01, 02, 05, 00, 06]
190+
Raw bytes (26): 0x[01, 01, 01, 01, 05, 04, 01, 37, 05, 01, 12, 05, 02, 0d, 00, 0e, 02, 02, 0d, 00, 0e, 01, 02, 05, 00, 06]
190191
Number of files: 1
191192
- file 0 => global file 1
192193
Number of expressions: 1
193194
- expression 0 operands: lhs = Counter(0), rhs = Counter(1)
194195
Number of file 0 mappings: 4
195196
- Code(Counter(0)) at (prev + 55, 5) to (start + 1, 18)
196197
- Code(Counter(1)) at (prev + 2, 13) to (start + 0, 14)
197-
- Code(Expression(0, Sub)) at (prev + 10, 13) to (start + 0, 14)
198+
- Code(Expression(0, Sub)) at (prev + 2, 13) to (start + 0, 14)
198199
= (c0 - c1)
199200
- Code(Counter(0)) at (prev + 2, 5) to (start + 0, 6)
200201

201202
Function name: async::j::d
202-
Raw bytes (9): 0x[01, 01, 00, 01, 01, 46, 05, 00, 17]
203+
Raw bytes (9): 0x[01, 01, 00, 01, 01, 3e, 05, 00, 17]
203204
Number of files: 1
204205
- file 0 => global file 1
205206
Number of expressions: 0
206207
Number of file 0 mappings: 1
207-
- Code(Counter(0)) at (prev + 70, 5) to (start + 0, 23)
208+
- Code(Counter(0)) at (prev + 62, 5) to (start + 0, 23)
208209

209210
Function name: async::j::f
210-
Raw bytes (9): 0x[01, 01, 00, 01, 01, 47, 05, 00, 17]
211+
Raw bytes (9): 0x[01, 01, 00, 01, 01, 3f, 05, 00, 17]
211212
Number of files: 1
212213
- file 0 => global file 1
213214
Number of expressions: 0
214215
Number of file 0 mappings: 1
215-
- Code(Counter(0)) at (prev + 71, 5) to (start + 0, 23)
216+
- Code(Counter(0)) at (prev + 63, 5) to (start + 0, 23)
216217

217218
Function name: async::k (unused)
218-
Raw bytes (29): 0x[01, 01, 00, 05, 00, 4f, 01, 01, 0c, 00, 02, 0e, 00, 10, 00, 01, 0e, 00, 10, 00, 01, 0e, 00, 10, 00, 02, 01, 00, 02]
219+
Raw bytes (29): 0x[01, 01, 00, 05, 00, 47, 01, 01, 0c, 00, 02, 0e, 00, 10, 00, 01, 0e, 00, 10, 00, 01, 0e, 00, 10, 00, 02, 01, 00, 02]
219220
Number of files: 1
220221
- file 0 => global file 1
221222
Number of expressions: 0
222223
Number of file 0 mappings: 5
223-
- Code(Zero) at (prev + 79, 1) to (start + 1, 12)
224+
- Code(Zero) at (prev + 71, 1) to (start + 1, 12)
224225
- Code(Zero) at (prev + 2, 14) to (start + 0, 16)
225226
- Code(Zero) at (prev + 1, 14) to (start + 0, 16)
226227
- Code(Zero) at (prev + 1, 14) to (start + 0, 16)
227228
- Code(Zero) at (prev + 2, 1) to (start + 0, 2)
228229

229230
Function name: async::l
230-
Raw bytes (37): 0x[01, 01, 04, 01, 07, 05, 09, 0f, 02, 09, 05, 05, 01, 57, 01, 01, 0c, 02, 02, 0e, 00, 10, 05, 01, 0e, 00, 10, 09, 01, 0e, 00, 10, 0b, 02, 01, 00, 02]
231+
Raw bytes (37): 0x[01, 01, 04, 01, 07, 05, 09, 0f, 02, 09, 05, 05, 01, 4f, 01, 01, 0c, 02, 02, 0e, 00, 10, 05, 01, 0e, 00, 10, 09, 01, 0e, 00, 10, 0b, 02, 01, 00, 02]
231232
Number of files: 1
232233
- file 0 => global file 1
233234
Number of expressions: 4
@@ -236,7 +237,7 @@ Number of expressions: 4
236237
- expression 2 operands: lhs = Expression(3, Add), rhs = Expression(0, Sub)
237238
- expression 3 operands: lhs = Counter(2), rhs = Counter(1)
238239
Number of file 0 mappings: 5
239-
- Code(Counter(0)) at (prev + 87, 1) to (start + 1, 12)
240+
- Code(Counter(0)) at (prev + 79, 1) to (start + 1, 12)
240241
- Code(Expression(0, Sub)) at (prev + 2, 14) to (start + 0, 16)
241242
= (c0 - (c1 + c2))
242243
- Code(Counter(1)) at (prev + 1, 14) to (start + 0, 16)
@@ -245,26 +246,26 @@ Number of file 0 mappings: 5
245246
= ((c2 + c1) + (c0 - (c1 + c2)))
246247

247248
Function name: async::m
248-
Raw bytes (9): 0x[01, 01, 00, 01, 01, 5f, 01, 00, 19]
249+
Raw bytes (9): 0x[01, 01, 00, 01, 01, 57, 01, 00, 19]
249250
Number of files: 1
250251
- file 0 => global file 1
251252
Number of expressions: 0
252253
Number of file 0 mappings: 1
253-
- Code(Counter(0)) at (prev + 95, 1) to (start + 0, 25)
254+
- Code(Counter(0)) at (prev + 87, 1) to (start + 0, 25)
254255

255256
Function name: async::m::{closure#0} (unused)
256-
Raw bytes (9): 0x[01, 01, 00, 01, 00, 5f, 19, 00, 22]
257+
Raw bytes (9): 0x[01, 01, 00, 01, 00, 57, 19, 00, 22]
257258
Number of files: 1
258259
- file 0 => global file 1
259260
Number of expressions: 0
260261
Number of file 0 mappings: 1
261-
- Code(Zero) at (prev + 95, 25) to (start + 0, 34)
262+
- Code(Zero) at (prev + 87, 25) to (start + 0, 34)
262263

263264
Function name: async::main
264-
Raw bytes (9): 0x[01, 01, 00, 01, 01, 61, 01, 08, 02]
265+
Raw bytes (9): 0x[01, 01, 00, 01, 01, 59, 01, 08, 02]
265266
Number of files: 1
266267
- file 0 => global file 1
267268
Number of expressions: 0
268269
Number of file 0 mappings: 1
269-
- Code(Counter(0)) at (prev + 97, 1) to (start + 8, 2)
270+
- Code(Counter(0)) at (prev + 89, 1) to (start + 8, 2)
270271

0 commit comments

Comments
 (0)