Skip to content

Commit 44c8f55

Browse files
committed
coverage: Rename is_closure to is_hole
When refining covspans, we don't specifically care which ones represent closures; we just want to know which ones represent "holes" that should be carved out of other spans and then discarded. (Closures are currently the only source of hole spans, but in the future we might want to also create hole spans for nested items and inactive `#[cfg(..)]` regions.)
1 parent 8bd33e3 commit 44c8f55

File tree

2 files changed

+59
-60
lines changed

2 files changed

+59
-60
lines changed

compiler/rustc_mir_transform/src/coverage/spans.rs

+43-49
Original file line numberDiff line numberDiff line change
@@ -90,23 +90,23 @@ pub(super) fn generate_coverage_spans(
9090
struct CurrCovspan {
9191
span: Span,
9292
bcb: BasicCoverageBlock,
93-
is_closure: bool,
93+
is_hole: bool,
9494
}
9595

9696
impl CurrCovspan {
97-
fn new(span: Span, bcb: BasicCoverageBlock, is_closure: bool) -> Self {
98-
Self { span, bcb, is_closure }
97+
fn new(span: Span, bcb: BasicCoverageBlock, is_hole: bool) -> Self {
98+
Self { span, bcb, is_hole }
9999
}
100100

101101
fn into_prev(self) -> PrevCovspan {
102-
let Self { span, bcb, is_closure } = self;
103-
PrevCovspan { span, bcb, merged_spans: vec![span], is_closure }
102+
let Self { span, bcb, is_hole } = self;
103+
PrevCovspan { span, bcb, merged_spans: vec![span], is_hole }
104104
}
105105

106106
fn into_refined(self) -> RefinedCovspan {
107-
// This is only called in cases where `curr` is a closure span that has
107+
// This is only called in cases where `curr` is a hole span that has
108108
// been carved out of `prev`.
109-
debug_assert!(self.is_closure);
109+
debug_assert!(self.is_hole);
110110
self.into_prev().into_refined()
111111
}
112112
}
@@ -118,12 +118,12 @@ struct PrevCovspan {
118118
/// List of all the original spans from MIR that have been merged into this
119119
/// span. Mainly used to precisely skip over gaps when truncating a span.
120120
merged_spans: Vec<Span>,
121-
is_closure: bool,
121+
is_hole: bool,
122122
}
123123

124124
impl PrevCovspan {
125125
fn is_mergeable(&self, other: &CurrCovspan) -> bool {
126-
self.bcb == other.bcb && !self.is_closure && !other.is_closure
126+
self.bcb == other.bcb && !self.is_hole && !other.is_hole
127127
}
128128

129129
fn merge_from(&mut self, other: &CurrCovspan) {
@@ -142,8 +142,8 @@ impl PrevCovspan {
142142
}
143143

144144
fn refined_copy(&self) -> RefinedCovspan {
145-
let &Self { span, bcb, merged_spans: _, is_closure } = self;
146-
RefinedCovspan { span, bcb, is_closure }
145+
let &Self { span, bcb, merged_spans: _, is_hole } = self;
146+
RefinedCovspan { span, bcb, is_hole }
147147
}
148148

149149
fn into_refined(self) -> RefinedCovspan {
@@ -156,12 +156,12 @@ impl PrevCovspan {
156156
struct RefinedCovspan {
157157
span: Span,
158158
bcb: BasicCoverageBlock,
159-
is_closure: bool,
159+
is_hole: bool,
160160
}
161161

162162
impl RefinedCovspan {
163163
fn is_mergeable(&self, other: &Self) -> bool {
164-
self.bcb == other.bcb && !self.is_closure && !other.is_closure
164+
self.bcb == other.bcb && !self.is_hole && !other.is_hole
165165
}
166166

167167
fn merge_from(&mut self, other: &Self) {
@@ -176,7 +176,8 @@ impl RefinedCovspan {
176176
/// * Remove duplicate source code coverage regions
177177
/// * Merge spans that represent continuous (both in source code and control flow), non-branching
178178
/// execution
179-
/// * Carve out (leave uncovered) any span that will be counted by another MIR (notably, closures)
179+
/// * Carve out (leave uncovered) any "hole" spans that need to be left blank
180+
/// (e.g. closures that will be counted by their own MIR body)
180181
struct SpansRefiner {
181182
/// The initial set of coverage spans, sorted by `Span` (`lo` and `hi`) and by relative
182183
/// dominance between the `BasicCoverageBlock`s of equal `Span`s.
@@ -228,7 +229,7 @@ impl SpansRefiner {
228229
let curr = self.curr();
229230

230231
if prev.is_mergeable(curr) {
231-
debug!(" same bcb (and neither is a closure), merge with prev={prev:?}");
232+
debug!(?prev, "curr will be merged into prev");
232233
let curr = self.take_curr();
233234
self.prev_mut().merge_from(&curr);
234235
} else if prev.span.hi() <= curr.span.lo() {
@@ -237,15 +238,13 @@ impl SpansRefiner {
237238
);
238239
let prev = self.take_prev().into_refined();
239240
self.refined_spans.push(prev);
240-
} else if prev.is_closure {
241+
} else if prev.is_hole {
241242
// drop any equal or overlapping span (`curr`) and keep `prev` to test again in the
242243
// next iter
243-
debug!(
244-
" curr overlaps a closure (prev). Drop curr and keep prev for next iter. prev={prev:?}",
245-
);
244+
debug!(?prev, "prev (a hole) overlaps curr, so discarding curr");
246245
self.take_curr(); // Discards curr.
247-
} else if curr.is_closure {
248-
self.carve_out_span_for_closure();
246+
} else if curr.is_hole {
247+
self.carve_out_span_for_hole();
249248
} else {
250249
self.cutoff_prev_at_overlapping_curr();
251250
}
@@ -269,10 +268,9 @@ impl SpansRefiner {
269268
}
270269
});
271270

272-
// Remove spans derived from closures, originally added to ensure the coverage
273-
// regions for the current function leave room for the closure's own coverage regions
274-
// (injected separately, from the closure's own MIR).
275-
self.refined_spans.retain(|covspan| !covspan.is_closure);
271+
// Discard hole spans, since their purpose was to carve out chunks from
272+
// other spans, but we don't want the holes themselves in the final mappings.
273+
self.refined_spans.retain(|covspan| !covspan.is_hole);
276274
self.refined_spans
277275
}
278276

@@ -315,47 +313,43 @@ impl SpansRefiner {
315313
{
316314
// Skip curr because prev has already advanced beyond the end of curr.
317315
// This can only happen if a prior iteration updated `prev` to skip past
318-
// a region of code, such as skipping past a closure.
319-
debug!(
320-
" prev.span starts after curr.span, so curr will be dropped (skipping past \
321-
closure?); prev={prev:?}",
322-
);
316+
// a region of code, such as skipping past a hole.
317+
debug!(?prev, "prev.span starts after curr.span, so curr will be dropped");
323318
} else {
324-
self.some_curr = Some(CurrCovspan::new(curr.span, curr.bcb, curr.is_closure));
319+
self.some_curr = Some(CurrCovspan::new(curr.span, curr.bcb, curr.is_hole));
325320
return true;
326321
}
327322
}
328323
false
329324
}
330325

331-
/// If `prev`s span extends left of the closure (`curr`), carve out the closure's span from
332-
/// `prev`'s span. (The closure's coverage counters will be injected when processing the
333-
/// closure's own MIR.) Add the portion of the span to the left of the closure; and if the span
334-
/// extends to the right of the closure, update `prev` to that portion of the span.
335-
fn carve_out_span_for_closure(&mut self) {
326+
/// If `prev`s span extends left of the hole (`curr`), carve out the hole's span from
327+
/// `prev`'s span. Add the portion of the span to the left of the hole; and if the span
328+
/// extends to the right of the hole, update `prev` to that portion of the span.
329+
fn carve_out_span_for_hole(&mut self) {
336330
let prev = self.prev();
337331
let curr = self.curr();
338332

339333
let left_cutoff = curr.span.lo();
340334
let right_cutoff = curr.span.hi();
341-
let has_pre_closure_span = prev.span.lo() < right_cutoff;
342-
let has_post_closure_span = prev.span.hi() > right_cutoff;
343-
344-
if has_pre_closure_span {
345-
let mut pre_closure = self.prev().refined_copy();
346-
pre_closure.span = pre_closure.span.with_hi(left_cutoff);
347-
debug!(" prev overlaps a closure. Adding span for pre_closure={:?}", pre_closure);
348-
self.refined_spans.push(pre_closure);
335+
let has_pre_hole_span = prev.span.lo() < right_cutoff;
336+
let has_post_hole_span = prev.span.hi() > right_cutoff;
337+
338+
if has_pre_hole_span {
339+
let mut pre_hole = prev.refined_copy();
340+
pre_hole.span = pre_hole.span.with_hi(left_cutoff);
341+
debug!(?pre_hole, "prev overlaps a hole; adding pre-hole span");
342+
self.refined_spans.push(pre_hole);
349343
}
350344

351-
if has_post_closure_span {
352-
// Mutate `prev.span` to start after the closure (and discard curr).
345+
if has_post_hole_span {
346+
// Mutate `prev.span` to start after the hole (and discard curr).
353347
self.prev_mut().span = self.prev().span.with_lo(right_cutoff);
354-
debug!(" Mutated prev.span to start after the closure. prev={:?}", self.prev());
348+
debug!(prev=?self.prev(), "mutated prev to start after the hole");
355349

356350
// Prevent this curr from becoming prev.
357-
let closure_covspan = self.take_curr().into_refined();
358-
self.refined_spans.push(closure_covspan); // since self.prev() was already updated
351+
let hole_covspan = self.take_curr().into_refined();
352+
self.refined_spans.push(hole_covspan); // since self.prev() was already updated
359353
}
360354
}
361355

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

+16-11
Original file line numberDiff line numberDiff line change
@@ -52,14 +52,14 @@ pub(super) fn mir_to_initial_sorted_coverage_spans(
5252
// - Span A extends further left, or
5353
// - Both have the same start and span A extends further right
5454
.then_with(|| Ord::cmp(&a.span.hi(), &b.span.hi()).reverse())
55-
// If two spans have the same lo & hi, put closure spans first,
56-
// as they take precedence over non-closure spans.
57-
.then_with(|| Ord::cmp(&a.is_closure, &b.is_closure).reverse())
55+
// If two spans have the same lo & hi, put hole spans first,
56+
// as they take precedence over non-hole spans.
57+
.then_with(|| Ord::cmp(&a.is_hole, &b.is_hole).reverse())
5858
// After deduplication, we want to keep only the most-dominated BCB.
5959
.then_with(|| basic_coverage_blocks.cmp_in_dominator_order(a.bcb, b.bcb).reverse())
6060
});
6161

62-
// Among covspans with the same span, keep only one. Closure spans take
62+
// Among covspans with the same span, keep only one. Hole spans take
6363
// precedence, otherwise keep the one with the most-dominated BCB.
6464
// (Ideally we should try to preserve _all_ non-dominating BCBs, but that
6565
// requires a lot more complexity in the span refiner, for little benefit.)
@@ -78,8 +78,8 @@ pub(super) fn mir_to_initial_sorted_coverage_spans(
7878
fn remove_unwanted_macro_spans(initial_spans: &mut Vec<SpanFromMir>) {
7979
let mut seen_macro_spans = FxHashSet::default();
8080
initial_spans.retain(|covspan| {
81-
// Ignore (retain) closure spans and non-macro-expansion spans.
82-
if covspan.is_closure || covspan.visible_macro.is_none() {
81+
// Ignore (retain) hole spans and non-macro-expansion spans.
82+
if covspan.is_hole || covspan.visible_macro.is_none() {
8383
return true;
8484
}
8585

@@ -96,7 +96,7 @@ fn split_visible_macro_spans(initial_spans: &mut Vec<SpanFromMir>) {
9696
let mut extra_spans = vec![];
9797

9898
initial_spans.retain(|covspan| {
99-
if covspan.is_closure {
99+
if covspan.is_hole {
100100
return true;
101101
}
102102

@@ -112,7 +112,7 @@ fn split_visible_macro_spans(initial_spans: &mut Vec<SpanFromMir>) {
112112
return true;
113113
}
114114

115-
assert!(!covspan.is_closure);
115+
assert!(!covspan.is_hole);
116116
extra_spans.push(SpanFromMir::new(before, covspan.visible_macro, covspan.bcb, false));
117117
extra_spans.push(SpanFromMir::new(after, covspan.visible_macro, covspan.bcb, false));
118118
false // Discard the original covspan that we just split.
@@ -148,6 +148,8 @@ fn bcb_to_initial_coverage_spans<'a, 'tcx>(
148148
let expn_span = filtered_statement_span(statement)?;
149149
let (span, visible_macro) = unexpand(expn_span)?;
150150

151+
// A statement that looks like the assignment of a closure expression
152+
// is treated as a "hole" span, to be carved out of other spans.
151153
Some(SpanFromMir::new(span, visible_macro, bcb, is_closure_like(statement)))
152154
});
153155

@@ -336,7 +338,10 @@ pub(super) struct SpanFromMir {
336338
pub(super) span: Span,
337339
visible_macro: Option<Symbol>,
338340
pub(super) bcb: BasicCoverageBlock,
339-
pub(super) is_closure: bool,
341+
/// If true, this covspan represents a "hole" that should be carved out
342+
/// from other spans, e.g. because it represents a closure expression that
343+
/// will be instrumented separately as its own function.
344+
pub(super) is_hole: bool,
340345
}
341346

342347
impl SpanFromMir {
@@ -348,8 +353,8 @@ impl SpanFromMir {
348353
span: Span,
349354
visible_macro: Option<Symbol>,
350355
bcb: BasicCoverageBlock,
351-
is_closure: bool,
356+
is_hole: bool,
352357
) -> Self {
353-
Self { span, visible_macro, bcb, is_closure }
358+
Self { span, visible_macro, bcb, is_hole }
354359
}
355360
}

0 commit comments

Comments
 (0)