Skip to content

Commit

Permalink
coverage: Rename is_closure to is_hole
Browse files Browse the repository at this point in the history
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.)
  • Loading branch information
Zalathar committed Feb 23, 2024
1 parent f380bd7 commit 303c333
Show file tree
Hide file tree
Showing 2 changed files with 49 additions and 52 deletions.
76 changes: 35 additions & 41 deletions compiler/rustc_mir_transform/src/coverage/spans.rs
Original file line number Diff line number Diff line change
Expand Up @@ -90,17 +90,17 @@ pub(super) fn generate_coverage_spans(
struct CurrCovspan {
span: Span,
bcb: BasicCoverageBlock,
is_closure: bool,
is_hole: bool,
}

impl CurrCovspan {
fn new(span: Span, bcb: BasicCoverageBlock, is_closure: bool) -> Self {
Self { span, bcb, is_closure }
fn new(span: Span, bcb: BasicCoverageBlock, is_hole: bool) -> Self {
Self { span, bcb, is_hole }
}

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

Expand All @@ -111,12 +111,12 @@ struct PrevCovspan {
/// List of all the original spans from MIR that have been merged into this
/// span. Mainly used to precisely skip over gaps when truncating a span.
merged_spans: Vec<Span>,
is_closure: bool,
is_hole: bool,
}

impl PrevCovspan {
fn is_mergeable(&self, other: &CurrCovspan) -> bool {
self.bcb == other.bcb && !self.is_closure && !other.is_closure
self.bcb == other.bcb && !self.is_hole && !other.is_hole
}

fn merge_from(&mut self, other: &CurrCovspan) {
Expand All @@ -135,8 +135,8 @@ impl PrevCovspan {
}

fn refined_copy(&self) -> Option<RefinedCovspan> {
let &Self { span, bcb, merged_spans: _, is_closure } = self;
(!is_closure).then_some(RefinedCovspan { span, bcb })
let &Self { span, bcb, merged_spans: _, is_hole } = self;
(!is_hole).then_some(RefinedCovspan { span, bcb })
}

fn into_refined(self) -> Option<RefinedCovspan> {
Expand Down Expand Up @@ -209,7 +209,7 @@ impl SpansRefiner {
let curr = self.curr();

if prev.is_mergeable(curr) {
debug!(" same bcb (and neither is a closure), merge with prev={prev:?}");
debug!(?prev, "curr will be merged into prev");
let curr = self.take_curr();
self.prev_mut().merge_from(&curr);
} else if prev.span.hi() <= curr.span.lo() {
Expand All @@ -218,15 +218,13 @@ impl SpansRefiner {
);
let prev = self.take_prev().into_refined();
self.refined_spans.extend(prev);
} else if prev.is_closure {
} else if prev.is_hole {
// drop any equal or overlapping span (`curr`) and keep `prev` to test again in the
// next iter
debug!(
" curr overlaps a closure (prev). Drop curr and keep prev for next iter. prev={prev:?}",
);
debug!(?prev, "prev (a hole) overlaps curr, so discarding curr");
self.take_curr(); // Discards curr.
} else if curr.is_closure {
self.carve_out_span_for_closure();
} else if curr.is_hole {
self.carve_out_span_for_hole();
} else {
self.cutoff_prev_at_overlapping_curr();
}
Expand Down Expand Up @@ -281,48 +279,44 @@ impl SpansRefiner {
{
// Skip curr because prev has already advanced beyond the end of curr.
// This can only happen if a prior iteration updated `prev` to skip past
// a region of code, such as skipping past a closure.
debug!(
" prev.span starts after curr.span, so curr will be dropped (skipping past \
closure?); prev={prev:?}",
);
// a region of code, such as skipping past a hole.
debug!(?prev, "prev.span starts after curr.span, so curr will be dropped");
} else {
self.some_curr = Some(CurrCovspan::new(curr.span, curr.bcb, curr.is_closure));
self.some_curr = Some(CurrCovspan::new(curr.span, curr.bcb, curr.is_hole));
return true;
}
}
false
}

/// If `prev`s span extends left of the closure (`curr`), carve out the closure's span from
/// `prev`'s span. (The closure's coverage counters will be injected when processing the
/// closure's own MIR.) Add the portion of the span to the left of the closure; and if the span
/// extends to the right of the closure, update `prev` to that portion of the span.
fn carve_out_span_for_closure(&mut self) {
/// If `prev`s span extends left of the hole (`curr`), carve out the hole's span from
/// `prev`'s span. Add the portion of the span to the left of the hole; and if the span
/// extends to the right of the hole, update `prev` to that portion of the span.
fn carve_out_span_for_hole(&mut self) {
let prev = self.prev();
let curr = self.curr();
assert!(!prev.is_closure && curr.is_closure);
assert!(!prev.is_hole && curr.is_hole);

let left_cutoff = curr.span.lo();
let right_cutoff = curr.span.hi();
let has_pre_closure_span = prev.span.lo() < right_cutoff;
let has_post_closure_span = prev.span.hi() > right_cutoff;

if has_pre_closure_span {
let mut pre_closure = prev.refined_copy().expect("prev is not a closure span");
pre_closure.span = pre_closure.span.with_hi(left_cutoff);
debug!(" prev overlaps a closure. Adding span for pre_closure={:?}", pre_closure);
self.refined_spans.push(pre_closure);
let has_pre_hole_span = prev.span.lo() < right_cutoff;
let has_post_hole_span = prev.span.hi() > right_cutoff;

if has_pre_hole_span {
let mut pre_hole = prev.refined_copy().expect("prev is not a hole span");
pre_hole.span = pre_hole.span.with_hi(left_cutoff);
debug!(?pre_hole, "prev overlaps a hole; adding pre-hole span");
self.refined_spans.push(pre_hole);
}

if has_post_closure_span {
// Mutate `prev.span` to start after the closure (and discard curr).
if has_post_hole_span {
// Mutate `prev.span` to start after the hole (and discard curr).
self.prev_mut().span = self.prev().span.with_lo(right_cutoff);
debug!(" Mutated prev.span to start after the closure. prev={:?}", self.prev());
debug!(prev=?self.prev(), "mutated prev to start after the hole");

// Discard this curr, since it's a closure span.
// Discard this curr, since it's a hole span.
let curr = self.take_curr();
assert!(curr.is_closure);
assert!(curr.is_hole);
}
}

Expand Down
25 changes: 14 additions & 11 deletions compiler/rustc_mir_transform/src/coverage/spans/from_mir.rs
Original file line number Diff line number Diff line change
Expand Up @@ -52,14 +52,14 @@ pub(super) fn mir_to_initial_sorted_coverage_spans(
// - Span A extends further left, or
// - Both have the same start and span A extends further right
.then_with(|| Ord::cmp(&a.span.hi(), &b.span.hi()).reverse())
// If two spans have the same lo & hi, put closure spans first,
// as they take precedence over non-closure spans.
.then_with(|| Ord::cmp(&a.is_closure, &b.is_closure).reverse())
// If two spans have the same lo & hi, put hole spans first,
// as they take precedence over non-hole spans.
.then_with(|| Ord::cmp(&a.is_hole, &b.is_hole).reverse())
// After deduplication, we want to keep only the most-dominated BCB.
.then_with(|| basic_coverage_blocks.cmp_in_dominator_order(a.bcb, b.bcb).reverse())
});

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

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

initial_spans.retain(|covspan| {
if covspan.is_closure {
if covspan.is_hole {
return true;
}

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

assert!(!covspan.is_closure);
assert!(!covspan.is_hole);
extra_spans.push(SpanFromMir::new(before, covspan.visible_macro, covspan.bcb, false));
extra_spans.push(SpanFromMir::new(after, covspan.visible_macro, covspan.bcb, false));
false // Discard the original covspan that we just split.
Expand Down Expand Up @@ -336,7 +336,10 @@ pub(super) struct SpanFromMir {
pub(super) span: Span,
visible_macro: Option<Symbol>,
pub(super) bcb: BasicCoverageBlock,
pub(super) is_closure: bool,
/// If true, this covspan represents a "hole" that should be carved out
/// from other spans, e.g. because it represents a closure expression that
/// will be instrumented separately as its own function.
pub(super) is_hole: bool,
}

impl SpanFromMir {
Expand All @@ -348,8 +351,8 @@ impl SpanFromMir {
span: Span,
visible_macro: Option<Symbol>,
bcb: BasicCoverageBlock,
is_closure: bool,
is_hole: bool,
) -> Self {
Self { span, visible_macro, bcb, is_closure }
Self { span, visible_macro, bcb, is_hole }
}
}

0 comments on commit 303c333

Please sign in to comment.