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.

Author: Zalathar

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<std::vec::IntoIter<CoverageSpan>>,
+    sorted_spans_iter: std::vec::IntoIter<CoverageSpan>,
 
     /// 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<CoverageSpan>,
 }
 
-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<CoverageSpan> {
-        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!(

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<CoverageSpan> {
-        let mut initial_spans =
-            Vec::<CoverageSpan>::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<CoverageSpan> {
+    let mut initial_spans = Vec::<CoverageSpan>::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<CoverageSpan> {
-        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<CoverageSpan> {
+    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,