coverage: Store expression data in function coverage info

Even though expression details are now stored in the info structure, we still
need to inject `ExpressionUsed` statements into MIR, because if one is missing
during codegen then we know that it was optimized out and we can remap all of
its associated code regions to zero.

3 files changed, 19 insertions, 72 deletions

diff --git a/compiler/rustc_mir_transform/src/coverage/counters.rs b/compiler/rustc_mir_transform/src/coverage/counters.rs
index 4140d402c75..a83ccf8fc3c 100644
--- a/compiler/rustc_mir_transform/src/coverage/counters.rs
+++ b/compiler/rustc_mir_transform/src/coverage/counters.rs
@@ -19,7 +19,7 @@ const NESTED_INDENT: &str = "    ";
 #[derive(Clone)]
 pub(super) enum BcbCounter {
     Counter { id: CounterId },
-    Expression { id: ExpressionId, lhs: CovTerm, op: Op, rhs: CovTerm },
+    Expression { id: ExpressionId },
 }
 
 impl BcbCounter {
@@ -39,17 +39,7 @@ impl Debug for BcbCounter {
     fn fmt(&self, fmt: &mut fmt::Formatter<'_>) -> fmt::Result {
         match self {
             Self::Counter { id, .. } => write!(fmt, "Counter({:?})", id.index()),
-            Self::Expression { id, lhs, op, rhs } => write!(
-                fmt,
-                "Expression({:?}) = {:?} {} {:?}",
-                id.index(),
-                lhs,
-                match op {
-                    Op::Add => "+",
-                    Op::Subtract => "-",
-                },
-                rhs,
-            ),
+            Self::Expression { id } => write!(fmt, "Expression({:?})", id.index()),
         }
     }
 }
@@ -58,7 +48,6 @@ impl Debug for BcbCounter {
 /// associated with nodes/edges in the BCB graph.
 pub(super) struct CoverageCounters {
     next_counter_id: CounterId,
-    next_expression_id: ExpressionId,
 
     /// Coverage counters/expressions that are associated with individual BCBs.
     bcb_counters: IndexVec<BasicCoverageBlock, Option<BcbCounter>>,
@@ -69,10 +58,9 @@ pub(super) struct CoverageCounters {
     /// Only used by debug assertions, to verify that BCBs with incoming edge
     /// counters do not have their own physical counters (expressions are allowed).
     bcb_has_incoming_edge_counters: BitSet<BasicCoverageBlock>,
-    /// Expression nodes that are not directly associated with any particular
-    /// BCB/edge, but are needed as operands to more complex expressions.
-    /// These are always [`BcbCounter::Expression`].
-    pub(super) intermediate_expressions: Vec<BcbCounter>,
+    /// Table of expression data, associating each expression ID with its
+    /// corresponding operator (+ or -) and its LHS/RHS operands.
+    expressions: IndexVec<ExpressionId, Expression>,
 }
 
 impl CoverageCounters {
@@ -81,12 +69,10 @@ impl CoverageCounters {
 
         Self {
             next_counter_id: CounterId::START,
-            next_expression_id: ExpressionId::START,
-
             bcb_counters: IndexVec::from_elem_n(None, num_bcbs),
             bcb_edge_counters: FxHashMap::default(),
             bcb_has_incoming_edge_counters: BitSet::new_empty(num_bcbs),
-            intermediate_expressions: Vec::new(),
+            expressions: IndexVec::new(),
         }
     }
 
@@ -107,8 +93,8 @@ impl CoverageCounters {
     }
 
     fn make_expression(&mut self, lhs: CovTerm, op: Op, rhs: CovTerm) -> BcbCounter {
-        let id = self.next_expression();
-        BcbCounter::Expression { id, lhs, op, rhs }
+        let id = self.expressions.push(Expression { lhs, op, rhs });
+        BcbCounter::Expression { id }
     }
 
     /// Counter IDs start from one and go up.
@@ -118,20 +104,13 @@ impl CoverageCounters {
         next
     }
 
-    /// Expression IDs start from 0 and go up.
-    /// (Counter IDs and Expression IDs are distinguished by the `Operand` enum.)
-    fn next_expression(&mut self) -> ExpressionId {
-        let next = self.next_expression_id;
-        self.next_expression_id = self.next_expression_id + 1;
-        next
-    }
-
     pub(super) fn num_counters(&self) -> usize {
         self.next_counter_id.as_usize()
     }
 
+    #[cfg(test)]
     pub(super) fn num_expressions(&self) -> usize {
-        self.next_expression_id.as_usize()
+        self.expressions.len()
     }
 
     fn set_bcb_counter(
@@ -207,6 +186,10 @@ impl CoverageCounters {
     ) -> impl Iterator<Item = ((BasicCoverageBlock, BasicCoverageBlock), BcbCounter)> + '_ {
         self.bcb_edge_counters.drain()
     }
+
+    pub(super) fn take_expressions(&mut self) -> IndexVec<ExpressionId, Expression> {
+        std::mem::take(&mut self.expressions)
+    }
 }
 
 /// Traverse the `CoverageGraph` and add either a `Counter` or `Expression` to every BCB, to be
@@ -333,7 +316,6 @@ impl<'a> MakeBcbCounters<'a> {
                     );
                     debug!("  [new intermediate expression: {:?}]", intermediate_expression);
                     let intermediate_expression_operand = intermediate_expression.as_term();
-                    self.coverage_counters.intermediate_expressions.push(intermediate_expression);
                     some_sumup_counter_operand.replace(intermediate_expression_operand);
                 }
             }
@@ -446,7 +428,6 @@ impl<'a> MakeBcbCounters<'a> {
                     intermediate_expression
                 );
                 let intermediate_expression_operand = intermediate_expression.as_term();
-                self.coverage_counters.intermediate_expressions.push(intermediate_expression);
                 some_sumup_edge_counter_operand.replace(intermediate_expression_operand);
             }
         }
diff --git a/compiler/rustc_mir_transform/src/coverage/mod.rs b/compiler/rustc_mir_transform/src/coverage/mod.rs
index 078123a7b7f..df4dccf0f0b 100644
--- a/compiler/rustc_mir_transform/src/coverage/mod.rs
+++ b/compiler/rustc_mir_transform/src/coverage/mod.rs
@@ -167,9 +167,6 @@ impl<'a, 'tcx> Instrumentor<'a, 'tcx> {
         // every coverage span has a `Counter` or `Expression` assigned to its `BasicCoverageBlock`
         // and all `Expression` dependencies (operands) are also generated, for any other
         // `BasicCoverageBlock`s not already associated with a coverage span.
-        //
-        // Intermediate expressions (used to compute other `Expression` values), which have no
-        // direct association with any `BasicCoverageBlock`, are accumulated inside `coverage_counters`.
         let bcb_has_coverage_spans = |bcb| coverage_spans.bcb_has_coverage_spans(bcb);
         let result = self
             .coverage_counters
@@ -195,29 +192,16 @@ impl<'a, 'tcx> Instrumentor<'a, 'tcx> {
             // are in fact counted, even though they don't directly contribute to counting
             // their own independent code region's coverage.
             self.inject_indirect_counters();
-
-            // Intermediate expressions will be injected as the final step, after generating
-            // debug output, if any.
-            ////////////////////////////////////////////////////
         };
 
         if let Err(e) = result {
             bug!("Error processing: {:?}: {:?}", self.mir_body.source.def_id(), e.message)
         };
 
-        ////////////////////////////////////////////////////
-        // Finally, inject the intermediate expressions collected along the way.
-        for intermediate_expression in &self.coverage_counters.intermediate_expressions {
-            inject_intermediate_expression(
-                self.mir_body,
-                self.make_mir_coverage_kind(intermediate_expression),
-            );
-        }
-
         self.mir_body.function_coverage_info = Some(Box::new(FunctionCoverageInfo {
             function_source_hash: self.function_source_hash,
             num_counters: self.coverage_counters.num_counters(),
-            num_expressions: self.coverage_counters.num_expressions(),
+            expressions: self.coverage_counters.take_expressions(),
             mappings: std::mem::take(&mut self.mappings),
         }));
     }
@@ -304,10 +288,8 @@ impl<'a, 'tcx> Instrumentor<'a, 'tcx> {
                         inject_to_bb,
                     );
                 }
-                BcbCounter::Expression { .. } => inject_intermediate_expression(
-                    self.mir_body,
-                    self.make_mir_coverage_kind(&counter_kind),
-                ),
+                // Experessions with no associated spans don't need to inject a statement.
+                BcbCounter::Expression { .. } => {}
             }
         }
     }
@@ -330,9 +312,7 @@ impl<'a, 'tcx> Instrumentor<'a, 'tcx> {
     fn make_mir_coverage_kind(&self, counter_kind: &BcbCounter) -> CoverageKind {
         match *counter_kind {
             BcbCounter::Counter { id } => CoverageKind::CounterIncrement { id },
-            BcbCounter::Expression { id, lhs, op, rhs } => {
-                CoverageKind::Expression { id, lhs, op, rhs }
-            }
+            BcbCounter::Expression { id } => CoverageKind::ExpressionUsed { id },
         }
     }
 }
@@ -371,20 +351,6 @@ fn inject_statement(mir_body: &mut mir::Body<'_>, counter_kind: CoverageKind, bb
     data.statements.insert(0, statement);
 }
 
-// Non-code expressions are injected into the coverage map, without generating executable code.
-fn inject_intermediate_expression(mir_body: &mut mir::Body<'_>, expression: CoverageKind) {
-    debug_assert!(matches!(expression, CoverageKind::Expression { .. }));
-    debug!("  injecting non-code expression {:?}", expression);
-    let inject_in_bb = mir::START_BLOCK;
-    let data = &mut mir_body[inject_in_bb];
-    let source_info = data.terminator().source_info;
-    let statement = Statement {
-        source_info,
-        kind: StatementKind::Coverage(Box::new(Coverage { kind: expression })),
-    };
-    data.statements.push(statement);
-}
-
 /// Convert the Span into its file name, start line and column, and end line and column
 fn make_code_region(
     source_map: &SourceMap,
diff --git a/compiler/rustc_mir_transform/src/coverage/tests.rs b/compiler/rustc_mir_transform/src/coverage/tests.rs
index 487d2282364..795cbce963d 100644
--- a/compiler/rustc_mir_transform/src/coverage/tests.rs
+++ b/compiler/rustc_mir_transform/src/coverage/tests.rs
@@ -656,7 +656,7 @@ fn test_make_bcb_counters() {
         coverage_counters
             .make_bcb_counters(&mut basic_coverage_blocks, bcb_has_coverage_spans)
             .expect("should be Ok");
-        assert_eq!(coverage_counters.intermediate_expressions.len(), 0);
+        assert_eq!(coverage_counters.num_expressions(), 0);
 
         let_bcb!(1);
         assert_eq!(