diff --git a/compiler/rustc_mir_transform/src/coverage/spans.rs b/compiler/rustc_mir_transform/src/coverage/spans.rs index 934e77e7deb0a..98fb1d8e1c940 100644 --- a/compiler/rustc_mir_transform/src/coverage/spans.rs +++ b/compiler/rustc_mir_transform/src/coverage/spans.rs @@ -61,7 +61,7 @@ pub(super) fn generate_coverage_spans( hir_info, basic_coverage_blocks, ); - let coverage_spans = SpansRefiner::refine_sorted_spans(basic_coverage_blocks, sorted_spans); + let coverage_spans = SpansRefiner::refine_sorted_spans(sorted_spans); mappings.extend(coverage_spans.into_iter().map(|RefinedCovspan { bcb, span, .. }| { // Each span produced by the generator represents an ordinary code region. BcbMapping { kind: BcbMappingKind::Code(bcb), span } @@ -88,8 +88,6 @@ pub(super) fn generate_coverage_spans( #[derive(Debug)] struct CurrCovspan { - /// This is used as the basis for [`PrevCovspan::original_span`], so it must - /// not be modified. span: Span, bcb: BasicCoverageBlock, is_closure: bool, @@ -102,7 +100,7 @@ impl CurrCovspan { fn into_prev(self) -> PrevCovspan { let Self { span, bcb, is_closure } = self; - PrevCovspan { original_span: span, span, bcb, merged_spans: vec![span], is_closure } + PrevCovspan { span, bcb, merged_spans: vec![span], is_closure } } fn into_refined(self) -> RefinedCovspan { @@ -115,7 +113,6 @@ impl CurrCovspan { #[derive(Debug)] struct PrevCovspan { - original_span: Span, span: Span, bcb: BasicCoverageBlock, /// List of all the original spans from MIR that have been merged into this @@ -135,42 +132,17 @@ impl PrevCovspan { self.merged_spans.push(other.span); } - fn cutoff_statements_at(&mut self, cutoff_pos: BytePos) { + fn cutoff_statements_at(mut self, cutoff_pos: BytePos) -> Option { self.merged_spans.retain(|span| span.hi() <= cutoff_pos); if let Some(max_hi) = self.merged_spans.iter().map(|span| span.hi()).max() { self.span = self.span.with_hi(max_hi); } - } - - fn into_dup(self) -> DuplicateCovspan { - let Self { original_span, span, bcb, merged_spans: _, is_closure } = self; - // Only unmodified spans end up in `pending_dups`. - debug_assert_eq!(original_span, span); - DuplicateCovspan { span, bcb, is_closure } - } - - fn refined_copy(&self) -> RefinedCovspan { - let &Self { original_span: _, span, bcb, merged_spans: _, is_closure } = self; - RefinedCovspan { span, bcb, is_closure } - } - fn into_refined(self) -> RefinedCovspan { - self.refined_copy() + if self.merged_spans.is_empty() { None } else { Some(self.into_refined()) } } -} - -#[derive(Debug)] -struct DuplicateCovspan { - span: Span, - bcb: BasicCoverageBlock, - is_closure: bool, -} -impl DuplicateCovspan { - /// Returns a copy of this covspan, as a [`RefinedCovspan`]. - /// Should only be called in places that would otherwise clone this covspan. fn refined_copy(&self) -> RefinedCovspan { - let &Self { span, bcb, is_closure } = self; + let &Self { span, bcb, merged_spans: _, is_closure } = self; RefinedCovspan { span, bcb, is_closure } } @@ -205,10 +177,7 @@ impl RefinedCovspan { /// * Merge spans that represent continuous (both in source code and control flow), non-branching /// execution /// * Carve out (leave uncovered) any span that will be counted by another MIR (notably, closures) -struct SpansRefiner<'a> { - /// The BasicCoverageBlock Control Flow Graph (BCB CFG). - basic_coverage_blocks: &'a CoverageGraph, - +struct SpansRefiner { /// The initial set of coverage spans, sorted by `Span` (`lo` and `hi`) and by relative /// dominance between the `BasicCoverageBlock`s of equal `Span`s. sorted_spans_iter: std::vec::IntoIter, @@ -223,36 +192,22 @@ struct SpansRefiner<'a> { /// If that `curr` was discarded, `prev` retains its value from the previous iteration. some_prev: Option, - /// One or more coverage spans with the same `Span` but different `BasicCoverageBlock`s, and - /// no `BasicCoverageBlock` in this list dominates another `BasicCoverageBlock` in the list. - /// If a new `curr` span also fits this criteria (compared to an existing list of - /// `pending_dups`), that `curr` moves to `prev` before possibly being added to - /// the `pending_dups` list, on the next iteration. As a result, if `prev` and `pending_dups` - /// have the same `Span`, the criteria for `pending_dups` holds for `prev` as well: a `prev` - /// with a matching `Span` does not dominate any `pending_dup` and no `pending_dup` dominates a - /// `prev` with a matching `Span`) - pending_dups: Vec, - /// The final coverage spans to add to the coverage map. A `Counter` or `Expression` /// will also be injected into the MIR for each BCB that has associated spans. refined_spans: Vec, } -impl<'a> SpansRefiner<'a> { +impl SpansRefiner { /// Takes the initial list of (sorted) spans extracted from MIR, and "refines" /// them by merging compatible adjacent spans, removing redundant spans, /// and carving holes in spans when they overlap in unwanted ways. - fn refine_sorted_spans( - basic_coverage_blocks: &'a CoverageGraph, - sorted_spans: Vec, - ) -> Vec { + fn refine_sorted_spans(sorted_spans: Vec) -> Vec { + let sorted_spans_len = sorted_spans.len(); let this = Self { - basic_coverage_blocks, sorted_spans_iter: sorted_spans.into_iter(), some_curr: None, some_prev: None, - pending_dups: Vec::new(), - refined_spans: Vec::with_capacity(basic_coverage_blocks.num_nodes() * 2), + refined_spans: Vec::with_capacity(sorted_spans_len), }; this.to_refined_spans() @@ -292,21 +247,11 @@ impl<'a> SpansRefiner<'a> { self.take_curr(); // Discards curr. } else if curr.is_closure { self.carve_out_span_for_closure(); - } else if prev.original_span == prev.span && prev.span == curr.span { - // Prev and curr have the same span, and prev's span hasn't - // been modified by other spans. - self.update_pending_dups(); } else { self.cutoff_prev_at_overlapping_curr(); } } - // Drain any remaining dups into the output. - for dup in self.pending_dups.drain(..) { - debug!(" ...adding at least one pending dup={:?}", dup); - self.refined_spans.push(dup.into_refined()); - } - // There is usually a final span remaining in `prev` after the loop ends, // so add it to the output as well. if let Some(prev) = self.some_prev.take() { @@ -359,36 +304,6 @@ impl<'a> SpansRefiner<'a> { self.some_prev.take().unwrap_or_else(|| bug!("some_prev is None (take_prev)")) } - /// If there are `pending_dups` but `prev` is not a matching dup (`prev.span` doesn't match the - /// `pending_dups` spans), then one of the following two things happened during the previous - /// iteration: - /// * the previous `curr` span (which is now `prev`) was not a duplicate of the pending_dups - /// (in which case there should be at least two spans in `pending_dups`); or - /// * the `span` of `prev` was modified by `curr_mut().merge_from(prev)` (in which case - /// `pending_dups` could have as few as one span) - /// In either case, no more spans will match the span of `pending_dups`, so - /// add the `pending_dups` if they don't overlap `curr`, and clear the list. - fn maybe_flush_pending_dups(&mut self) { - let Some(last_dup) = self.pending_dups.last() else { return }; - if last_dup.span == self.prev().span { - return; - } - - debug!( - " SAME spans, but pending_dups are NOT THE SAME, so BCBs matched on \ - previous iteration, or prev started a new disjoint span" - ); - if last_dup.span.hi() <= self.curr().span.lo() { - for dup in self.pending_dups.drain(..) { - debug!(" ...adding at least one pending={:?}", dup); - self.refined_spans.push(dup.into_refined()); - } - } else { - self.pending_dups.clear(); - } - assert!(self.pending_dups.is_empty()); - } - /// Advance `prev` to `curr` (if any), and `curr` to the next coverage span in sorted order. fn next_coverage_span(&mut self) -> bool { if let Some(curr) = self.some_curr.take() { @@ -408,7 +323,6 @@ impl<'a> SpansRefiner<'a> { ); } else { self.some_curr = Some(CurrCovspan::new(curr.span, curr.bcb, curr.is_closure)); - self.maybe_flush_pending_dups(); return true; } } @@ -433,13 +347,6 @@ impl<'a> SpansRefiner<'a> { let mut pre_closure = self.prev().refined_copy(); pre_closure.span = pre_closure.span.with_hi(left_cutoff); debug!(" prev overlaps a closure. Adding span for pre_closure={:?}", pre_closure); - - for mut dup in self.pending_dups.iter().map(DuplicateCovspan::refined_copy) { - dup.span = dup.span.with_hi(left_cutoff); - debug!(" ...and at least one pre_closure dup={:?}", dup); - self.refined_spans.push(dup); - } - self.refined_spans.push(pre_closure); } @@ -448,58 +355,9 @@ impl<'a> SpansRefiner<'a> { self.prev_mut().span = self.prev().span.with_lo(right_cutoff); debug!(" Mutated prev.span to start after the closure. prev={:?}", self.prev()); - for dup in &mut self.pending_dups { - debug!(" ...and at least one overlapping dup={:?}", dup); - dup.span = dup.span.with_lo(right_cutoff); - } - // Prevent this curr from becoming prev. let closure_covspan = self.take_curr().into_refined(); self.refined_spans.push(closure_covspan); // since self.prev() was already updated - } else { - self.pending_dups.clear(); - } - } - - /// Called if `curr.span` equals `prev.original_span` (and potentially equal to all - /// `pending_dups` spans, if any). Keep in mind, `prev.span()` may have been changed. - /// If prev.span() was merged into other spans (with matching BCB, for instance), - /// `prev.span.hi()` will be greater than (further right of) `prev.original_span.hi()`. - /// If prev.span() was split off to the right of a closure, prev.span().lo() will be - /// greater than prev.original_span.lo(). The actual span of `prev.original_span` is - /// not as important as knowing that `prev()` **used to have the same span** as `curr()`, - /// which means their sort order is still meaningful for determining the dominator - /// relationship. - /// - /// When two coverage spans have the same `Span`, dominated spans can be discarded; but if - /// neither coverage span dominates the other, both (or possibly more than two) are held, - /// until their disposition is determined. In this latter case, the `prev` dup is moved into - /// `pending_dups` so the new `curr` dup can be moved to `prev` for the next iteration. - fn update_pending_dups(&mut self) { - let prev_bcb = self.prev().bcb; - let curr_bcb = self.curr().bcb; - - // Equal coverage spans are ordered by dominators before dominated (if any), so it should be - // impossible for `curr` to dominate any previous coverage span. - debug_assert!(!self.basic_coverage_blocks.dominates(curr_bcb, prev_bcb)); - - // `prev` is a duplicate of `curr`, so add it to the list of pending dups. - // If it dominates `curr`, it will be removed by the subsequent discard step. - let prev = self.take_prev().into_dup(); - debug!(?prev, "adding prev to pending dups"); - self.pending_dups.push(prev); - - let initial_pending_count = self.pending_dups.len(); - if initial_pending_count > 0 { - self.pending_dups - .retain(|dup| !self.basic_coverage_blocks.dominates(dup.bcb, curr_bcb)); - - let n_discarded = initial_pending_count - self.pending_dups.len(); - if n_discarded > 0 { - debug!( - " discarded {n_discarded} of {initial_pending_count} pending_dups that dominated curr", - ); - } } } @@ -516,19 +374,13 @@ impl<'a> SpansRefiner<'a> { if it has statements that end before curr; prev={:?}", self.prev() ); - if self.pending_dups.is_empty() { - let curr_span = self.curr().span; - self.prev_mut().cutoff_statements_at(curr_span.lo()); - if self.prev().merged_spans.is_empty() { - debug!(" ... no non-overlapping statements to add"); - } else { - debug!(" ... adding modified prev={:?}", self.prev()); - let prev = self.take_prev().into_refined(); - self.refined_spans.push(prev); - } + + let curr_span = self.curr().span; + if let Some(prev) = self.take_prev().cutoff_statements_at(curr_span.lo()) { + debug!("after cutoff, adding {prev:?}"); + self.refined_spans.push(prev); } else { - // with `pending_dups`, `prev` cannot have any statements that don't overlap - self.pending_dups.clear(); + debug!("prev was eliminated by cutoff"); } } } diff --git a/compiler/rustc_mir_transform/src/coverage/spans/from_mir.rs b/compiler/rustc_mir_transform/src/coverage/spans/from_mir.rs index 2db358379fe51..b91ab811918a7 100644 --- a/compiler/rustc_mir_transform/src/coverage/spans/from_mir.rs +++ b/compiler/rustc_mir_transform/src/coverage/spans/from_mir.rs @@ -52,14 +52,19 @@ 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 both spans are equal, sort the BCBs in dominator order, - // so that dominating BCBs come before other BCBs they dominate. - .then_with(|| basic_coverage_blocks.cmp_in_dominator_order(a.bcb, b.bcb)) - // If two spans are otherwise identical, put closure spans first, - // as this seems to be what the refinement step expects. + // 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()) + // 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 + // 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.) + initial_spans.dedup_by(|b, a| a.span.source_equal(b.span)); + initial_spans } diff --git a/tests/coverage/closure_macro.cov-map b/tests/coverage/closure_macro.cov-map index 571e5564b659c..e43ed1f76f366 100644 --- a/tests/coverage/closure_macro.cov-map +++ b/tests/coverage/closure_macro.cov-map @@ -7,18 +7,17 @@ Number of file 0 mappings: 1 - Code(Counter(0)) at (prev + 29, 1) to (start + 2, 2) Function name: closure_macro::main -Raw bytes (43): 0x[01, 01, 02, 01, 05, 05, 02, 07, 01, 21, 01, 01, 21, 02, 02, 09, 00, 0f, 05, 00, 12, 00, 13, 02, 00, 12, 00, 13, 05, 00, 54, 00, 55, 02, 02, 09, 02, 0b, 07, 03, 01, 00, 02] +Raw bytes (38): 0x[01, 01, 02, 01, 05, 05, 02, 06, 01, 21, 01, 01, 21, 02, 02, 09, 00, 12, 02, 00, 0f, 00, 54, 05, 00, 54, 00, 55, 02, 02, 09, 02, 0b, 07, 03, 01, 00, 02] Number of files: 1 - file 0 => global file 1 Number of expressions: 2 - expression 0 operands: lhs = Counter(0), rhs = Counter(1) - expression 1 operands: lhs = Counter(1), rhs = Expression(0, Sub) -Number of file 0 mappings: 7 +Number of file 0 mappings: 6 - Code(Counter(0)) at (prev + 33, 1) to (start + 1, 33) -- Code(Expression(0, Sub)) at (prev + 2, 9) to (start + 0, 15) +- Code(Expression(0, Sub)) at (prev + 2, 9) to (start + 0, 18) = (c0 - c1) -- Code(Counter(1)) at (prev + 0, 18) to (start + 0, 19) -- Code(Expression(0, Sub)) at (prev + 0, 18) to (start + 0, 19) +- Code(Expression(0, Sub)) at (prev + 0, 15) to (start + 0, 84) = (c0 - c1) - Code(Counter(1)) at (prev + 0, 84) to (start + 0, 85) - Code(Expression(0, Sub)) at (prev + 2, 9) to (start + 2, 11) diff --git a/tests/coverage/closure_macro_async.cov-map b/tests/coverage/closure_macro_async.cov-map index 49ec767eab33e..212b67a8a3e1d 100644 --- a/tests/coverage/closure_macro_async.cov-map +++ b/tests/coverage/closure_macro_async.cov-map @@ -15,18 +15,17 @@ Number of file 0 mappings: 1 - Code(Counter(0)) at (prev + 35, 1) to (start + 0, 43) Function name: closure_macro_async::test::{closure#0} -Raw bytes (43): 0x[01, 01, 02, 01, 05, 05, 02, 07, 01, 23, 2b, 01, 21, 02, 02, 09, 00, 0f, 05, 00, 12, 00, 13, 02, 00, 12, 00, 13, 05, 00, 54, 00, 55, 02, 02, 09, 02, 0b, 07, 03, 01, 00, 02] +Raw bytes (38): 0x[01, 01, 02, 01, 05, 05, 02, 06, 01, 23, 2b, 01, 21, 02, 02, 09, 00, 12, 02, 00, 0f, 00, 54, 05, 00, 54, 00, 55, 02, 02, 09, 02, 0b, 07, 03, 01, 00, 02] Number of files: 1 - file 0 => global file 1 Number of expressions: 2 - expression 0 operands: lhs = Counter(0), rhs = Counter(1) - expression 1 operands: lhs = Counter(1), rhs = Expression(0, Sub) -Number of file 0 mappings: 7 +Number of file 0 mappings: 6 - Code(Counter(0)) at (prev + 35, 43) to (start + 1, 33) -- Code(Expression(0, Sub)) at (prev + 2, 9) to (start + 0, 15) +- Code(Expression(0, Sub)) at (prev + 2, 9) to (start + 0, 18) = (c0 - c1) -- Code(Counter(1)) at (prev + 0, 18) to (start + 0, 19) -- Code(Expression(0, Sub)) at (prev + 0, 18) to (start + 0, 19) +- Code(Expression(0, Sub)) at (prev + 0, 15) to (start + 0, 84) = (c0 - c1) - Code(Counter(1)) at (prev + 0, 84) to (start + 0, 85) - Code(Expression(0, Sub)) at (prev + 2, 9) to (start + 2, 11)