From 49ba9c4940b9d0f8d943ad992c7b79027d12e8bc Mon Sep 17 00:00:00 2001 From: Zalathar Date: Sun, 1 Oct 2023 21:07:42 +1100 Subject: [PATCH] coverage: Disconnect span extraction from `CoverageSpansGenerator` By performal initial span extraction in a separate free function, we can remove some accidental complexity from the main generator code. --- .../rustc_mir_transform/src/coverage/spans.rs | 29 ++-- .../src/coverage/spans/from_mir.rs | 143 +++++++++--------- 2 files changed, 85 insertions(+), 87 deletions(-) diff --git a/compiler/rustc_mir_transform/src/coverage/spans.rs b/compiler/rustc_mir_transform/src/coverage/spans.rs index 20b2180495dae..be4984e899bc6 100644 --- a/compiler/rustc_mir_transform/src/coverage/spans.rs +++ b/compiler/rustc_mir_transform/src/coverage/spans.rs @@ -203,13 +203,7 @@ impl CoverageSpan { /// * 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 CoverageSpansGenerator<'a, 'tcx> { - /// The MIR, used to look up `BasicBlockData`. - mir_body: &'a mir::Body<'tcx>, - - /// A `Span` covering the signature of function for the MIR. - fn_sig_span: Span, - +struct CoverageSpansGenerator<'a> { /// A `Span` covering the function body of the MIR (typically from left curly brace to right /// curly brace). body_span: Span, @@ -219,7 +213,7 @@ struct CoverageSpansGenerator<'a, 'tcx> { /// The initial set of `CoverageSpan`s, sorted by `Span` (`lo` and `hi`) and by relative /// dominance between the `BasicCoverageBlock`s of equal `Span`s. - sorted_spans_iter: Option>, + sorted_spans_iter: std::vec::IntoIter, /// The current `CoverageSpan` to compare to its `prev`, to possibly merge, discard, force the /// discard of the `prev` (and or `pending_dups`), or keep both (with `prev` moved to @@ -259,7 +253,7 @@ struct CoverageSpansGenerator<'a, 'tcx> { refined_spans: Vec, } -impl<'a, 'tcx> CoverageSpansGenerator<'a, 'tcx> { +impl<'a> CoverageSpansGenerator<'a> { /// Generate a minimal set of `CoverageSpan`s, each representing a contiguous code region to be /// counted. /// @@ -282,17 +276,22 @@ impl<'a, 'tcx> CoverageSpansGenerator<'a, 'tcx> { /// Note the resulting vector of `CoverageSpan`s may not be fully sorted (and does not need /// to be). pub(super) fn generate_coverage_spans( - mir_body: &'a mir::Body<'tcx>, + mir_body: &mir::Body<'_>, fn_sig_span: Span, // Ensured to be same SourceFile and SyntaxContext as `body_span` body_span: Span, basic_coverage_blocks: &'a CoverageGraph, ) -> Vec { - let mut coverage_spans = Self { + let sorted_spans = from_mir::mir_to_initial_sorted_coverage_spans( mir_body, fn_sig_span, body_span, basic_coverage_blocks, - sorted_spans_iter: None, + ); + + let coverage_spans = Self { + body_span, + basic_coverage_blocks, + sorted_spans_iter: sorted_spans.into_iter(), refined_spans: Vec::with_capacity(basic_coverage_blocks.num_nodes() * 2), some_curr: None, curr_original_span: Span::with_root_ctxt(BytePos(0), BytePos(0)), @@ -302,10 +301,6 @@ impl<'a, 'tcx> CoverageSpansGenerator<'a, 'tcx> { pending_dups: Vec::new(), }; - let sorted_spans = coverage_spans.mir_to_initial_sorted_coverage_spans(); - - coverage_spans.sorted_spans_iter = Some(sorted_spans.into_iter()); - coverage_spans.to_refined_spans() } @@ -510,7 +505,7 @@ impl<'a, 'tcx> CoverageSpansGenerator<'a, 'tcx> { self.some_prev = Some(curr); self.prev_original_span = self.curr_original_span; } - while let Some(curr) = self.sorted_spans_iter.as_mut().unwrap().next() { + while let Some(curr) = self.sorted_spans_iter.next() { debug!("FOR curr={:?}", curr); if self.some_prev.is_some() && self.prev_starts_after_next(&curr) { debug!( 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 79f5b0bf1a9c6..add41330aafd8 100644 --- a/compiler/rustc_mir_transform/src/coverage/spans/from_mir.rs +++ b/compiler/rustc_mir_transform/src/coverage/spans/from_mir.rs @@ -3,86 +3,89 @@ use rustc_middle::mir::{ }; use rustc_span::Span; -use crate::coverage::graph::{BasicCoverageBlock, BasicCoverageBlockData}; -use crate::coverage::spans::{CoverageSpan, CoverageSpansGenerator}; +use crate::coverage::graph::{BasicCoverageBlock, BasicCoverageBlockData, CoverageGraph}; +use crate::coverage::spans::CoverageSpan; -impl<'a, 'tcx> CoverageSpansGenerator<'a, 'tcx> { - pub(super) fn mir_to_initial_sorted_coverage_spans(&self) -> Vec { - let mut initial_spans = - Vec::::with_capacity(self.mir_body.basic_blocks.len() * 2); - for (bcb, bcb_data) in self.basic_coverage_blocks.iter_enumerated() { - initial_spans.extend(self.bcb_to_initial_coverage_spans(bcb, bcb_data)); - } +pub(super) fn mir_to_initial_sorted_coverage_spans( + mir_body: &mir::Body<'_>, + fn_sig_span: Span, + body_span: Span, + basic_coverage_blocks: &CoverageGraph, +) -> Vec { + let mut initial_spans = Vec::::with_capacity(mir_body.basic_blocks.len() * 2); + for (bcb, bcb_data) in basic_coverage_blocks.iter_enumerated() { + initial_spans.extend(bcb_to_initial_coverage_spans(mir_body, body_span, bcb, bcb_data)); + } - if initial_spans.is_empty() { - // This can happen if, for example, the function is unreachable (contains only a - // `BasicBlock`(s) with an `Unreachable` terminator). - return initial_spans; - } + if initial_spans.is_empty() { + // This can happen if, for example, the function is unreachable (contains only a + // `BasicBlock`(s) with an `Unreachable` terminator). + return initial_spans; + } - initial_spans.push(CoverageSpan::for_fn_sig(self.fn_sig_span)); + initial_spans.push(CoverageSpan::for_fn_sig(fn_sig_span)); - initial_spans.sort_by(|a, b| { - // First sort by span start. - Ord::cmp(&a.span.lo(), &b.span.lo()) - // If span starts are the same, sort by span end in reverse order. - // This ensures that if spans A and B are adjacent in the list, - // and they overlap but are not equal, then either: - // - 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(|| self.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. - .then_with(|| Ord::cmp(&a.is_closure, &b.is_closure).reverse()) - }); + initial_spans.sort_by(|a, b| { + // First sort by span start. + Ord::cmp(&a.span.lo(), &b.span.lo()) + // If span starts are the same, sort by span end in reverse order. + // This ensures that if spans A and B are adjacent in the list, + // and they overlap but are not equal, then either: + // - 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. + .then_with(|| Ord::cmp(&a.is_closure, &b.is_closure).reverse()) + }); - initial_spans - } + initial_spans +} - // Generate a set of `CoverageSpan`s from the filtered set of `Statement`s and `Terminator`s of - // the `BasicBlock`(s) in the given `BasicCoverageBlockData`. One `CoverageSpan` is generated - // for each `Statement` and `Terminator`. (Note that subsequent stages of coverage analysis will - // merge some `CoverageSpan`s, at which point a `CoverageSpan` may represent multiple - // `Statement`s and/or `Terminator`s.) - fn bcb_to_initial_coverage_spans( - &self, - bcb: BasicCoverageBlock, - bcb_data: &'a BasicCoverageBlockData, - ) -> Vec { - bcb_data - .basic_blocks - .iter() - .flat_map(|&bb| { - let data = &self.mir_body[bb]; - data.statements - .iter() - .enumerate() - .filter_map(move |(index, statement)| { - filtered_statement_span(statement).map(|span| { - CoverageSpan::for_statement( - statement, - function_source_span(span, self.body_span), - span, - bcb, - bb, - index, - ) - }) - }) - .chain(filtered_terminator_span(data.terminator()).map(|span| { - CoverageSpan::for_terminator( - function_source_span(span, self.body_span), +// Generate a set of `CoverageSpan`s from the filtered set of `Statement`s and `Terminator`s of +// the `BasicBlock`(s) in the given `BasicCoverageBlockData`. One `CoverageSpan` is generated +// for each `Statement` and `Terminator`. (Note that subsequent stages of coverage analysis will +// merge some `CoverageSpan`s, at which point a `CoverageSpan` may represent multiple +// `Statement`s and/or `Terminator`s.) +fn bcb_to_initial_coverage_spans( + mir_body: &mir::Body<'_>, + body_span: Span, + bcb: BasicCoverageBlock, + bcb_data: &BasicCoverageBlockData, +) -> Vec { + bcb_data + .basic_blocks + .iter() + .flat_map(|&bb| { + let data = &mir_body[bb]; + data.statements + .iter() + .enumerate() + .filter_map(move |(index, statement)| { + filtered_statement_span(statement).map(|span| { + CoverageSpan::for_statement( + statement, + function_source_span(span, body_span), span, bcb, bb, + index, ) - })) - }) - .collect() - } + }) + }) + .chain(filtered_terminator_span(data.terminator()).map(|span| { + CoverageSpan::for_terminator( + function_source_span(span, body_span), + span, + bcb, + bb, + ) + })) + }) + .collect() } /// If the MIR `Statement` has a span contributive to computing coverage spans,