coverage: Store the instrumentor's expressions in a separate list

This will make it easier to store the entire expression list in
`FunctionCoverageData`.

Author: Zalathar

compiler/rustc_middle/src/mir/coverage.rs

@@ -135,6 +135,14 @@ impl Op {
     }
 }
 
+#[derive(Clone, Debug)]
+#[derive(TyEncodable, TyDecodable, Hash, HashStable, TypeFoldable, TypeVisitable)]
+pub struct Expression {
+    pub lhs: CovTerm,
+    pub op: Op,
+    pub rhs: CovTerm,
+}
+
 #[derive(Clone, Debug, PartialEq, Eq, PartialOrd, Ord)]
 #[derive(TyEncodable, TyDecodable, Hash, HashStable, TypeFoldable, TypeVisitable)]
 pub struct Mapping {

compiler/rustc_mir_transform/src/coverage/counters.rs

@@ -21,7 +21,7 @@ use std::fmt::{self, Debug};
 #[derive(Clone)]
 pub(super) enum BcbCounter {
     Counter { id: CounterId },
-    Expression { id: ExpressionId, lhs: CovTerm, op: Op, rhs: CovTerm },
+    Expression { id: ExpressionId },
 }
 
 impl BcbCounter {
@@ -41,17 +41,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()),
         }
     }
 }
@@ -60,7 +50,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>>,
@@ -75,6 +64,7 @@ pub(super) struct CoverageCounters {
     /// BCB/edge, but are needed as operands to more complex expressions.
     /// These are always [`BcbCounter::Expression`].
     pub(super) intermediate_expressions: Vec<BcbCounter>,
+    pub(crate) expressions: IndexVec<ExpressionId, Expression>,
 
     pub debug_counters: DebugCounters,
 }
@@ -85,12 +75,11 @@ 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(),
 
             debug_counters: DebugCounters::new(),
         }
@@ -134,12 +123,12 @@ impl CoverageCounters {
     where
         F: Fn() -> Option<String>,
     {
-        let id = self.next_expression();
-        let expression = BcbCounter::Expression { id, lhs, op, rhs };
+        let id = self.expressions.push(Expression { lhs, op, rhs });
         if self.debug_counters.is_enabled() {
-            self.debug_counters.add_counter(&expression, (debug_block_label_fn)());
+            let expression = self.expressions[id].clone();
+            self.debug_counters.add_expression(id, expression, (debug_block_label_fn)());
         }
-        expression
+        BcbCounter::Expression { id }
     }
 
     /// Counter IDs start from one and go up.
@@ -149,20 +138,12 @@ 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(crate) fn num_counters(&self) -> usize {
         self.next_counter_id.as_usize()
     }
 
     pub(crate) fn num_expressions(&self) -> usize {
-        self.next_expression_id.as_usize()
+        self.expressions.len()
     }
 
     fn set_bcb_counter(

compiler/rustc_mir_transform/src/coverage/debug.rs

@@ -118,6 +118,7 @@ use rustc_middle::mir::generic_graphviz::GraphvizWriter;
 use rustc_middle::mir::spanview::{self, SpanViewable};
 
 use rustc_data_structures::fx::FxHashMap;
+use rustc_index::IndexVec;
 use rustc_middle::mir::coverage::*;
 use rustc_middle::mir::{self, BasicBlock};
 use rustc_middle::ty::TyCtxt;
@@ -259,12 +260,13 @@ impl Default for ExpressionFormat {
 /// `DebugCounters` supports a recursive rendering of `Expression` counters, so they can be
 /// presented as nested expressions such as `(bcb3 - (bcb0 + bcb1))`.
 pub(super) struct DebugCounters {
+    expressions: IndexVec<ExpressionId, Expression>,
     some_counters: Option<FxHashMap<CovTerm, DebugCounter>>,
 }
 
 impl DebugCounters {
     pub fn new() -> Self {
-        Self { some_counters: None }
+        Self { expressions: IndexVec::new(), some_counters: None }
     }
 
     pub fn enable(&mut self) {
@@ -285,6 +287,17 @@ impl DebugCounters {
         }
     }
 
+    pub fn add_expression(
+        &mut self,
+        id: ExpressionId,
+        expression: Expression,
+        some_block_label: Option<String>,
+    ) {
+        assert_eq!(id, self.expressions.next_index());
+        self.expressions.push(expression);
+        self.add_counter(&BcbCounter::Expression { id }, some_block_label);
+    }
+
     pub fn format_counter(&self, counter_kind: &BcbCounter) -> String {
         match *counter_kind {
             BcbCounter::Counter { .. } => {
@@ -298,8 +311,9 @@ impl DebugCounters {
 
     fn format_counter_kind(&self, counter_kind: &BcbCounter) -> String {
         let counter_format = &debug_options().counter_format;
-        if let BcbCounter::Expression { id, lhs, op, rhs } = *counter_kind {
+        if let BcbCounter::Expression { id } = *counter_kind {
             if counter_format.operation {
+                let Expression { lhs, op, rhs } = self.expressions[id];
                 return format!(
                     "{}{} {} {}",
                     if counter_format.id || self.some_counters.is_none() {
@@ -500,9 +514,14 @@ impl UsedExpressions {
         self.some_used_expression_operands.is_some()
     }
 
-    pub fn add_expression_operands(&mut self, expression: &BcbCounter) {
+    pub fn add_expression_operands(
+        &mut self,
+        expression: &BcbCounter,
+        coverage_counters: &CoverageCounters,
+    ) {
         if let Some(used_expression_operands) = self.some_used_expression_operands.as_mut() {
-            if let BcbCounter::Expression { id, lhs, rhs, .. } = *expression {
+            if let BcbCounter::Expression { id } = *expression {
+                let Expression { lhs, op: _, rhs } = coverage_counters.expressions[id];
                 used_expression_operands.entry(lhs).or_insert_with(Vec::new).push(id);
                 used_expression_operands.entry(rhs).or_insert_with(Vec::new).push(id);
             }
@@ -556,6 +575,7 @@ impl UsedExpressions {
             BasicCoverageBlock,
             BcbCounter,
         )],
+        coverage_counters: &CoverageCounters,
     ) {
         if self.is_enabled() {
             let mut not_validated = bcb_counters_without_direct_coverage_spans
@@ -568,7 +588,7 @@ impl UsedExpressions {
                 validating_count = to_validate.len();
                 for counter_kind in to_validate {
                     if self.expression_is_used(counter_kind) {
-                        self.add_expression_operands(counter_kind);
+                        self.add_expression_operands(counter_kind, coverage_counters);
                     } else {
                         not_validated.push(counter_kind);
                     }

compiler/rustc_mir_transform/src/coverage/mod.rs

@@ -213,7 +213,8 @@ impl<'a, 'tcx> Instrumentor<'a, 'tcx> {
             // BCB) to the `debug_used_expressions` map.
             if debug_used_expressions.is_enabled() {
                 for intermediate_expression in &self.coverage_counters.intermediate_expressions {
-                    debug_used_expressions.add_expression_operands(intermediate_expression);
+                    debug_used_expressions
+                        .add_expression_operands(intermediate_expression, &self.coverage_counters);
                 }
             }
 
@@ -311,7 +312,7 @@ impl<'a, 'tcx> Instrumentor<'a, 'tcx> {
             let counter_kind = self.coverage_counters.take_bcb_counter(bcb).unwrap_or_else(|| {
                 bug!("Every BasicCoverageBlock should have a Counter or Expression");
             });
-            debug_used_expressions.add_expression_operands(&counter_kind);
+            debug_used_expressions.add_expression_operands(&counter_kind, &self.coverage_counters);
             graphviz_data.add_bcb_coverage_spans_with_counter(bcb, bcb_spans, &counter_kind);
 
             let term = counter_kind.as_term();
@@ -359,7 +360,8 @@ impl<'a, 'tcx> Instrumentor<'a, 'tcx> {
         // If debug is enabled, validate that every BCB or edge counter not directly associated
         // with a coverage span is at least indirectly associated (it is a dependency of a BCB
         // counter that _is_ associated with a coverage span).
-        debug_used_expressions.validate(&bcb_counters_without_direct_coverage_spans);
+        debug_used_expressions
+            .validate(&bcb_counters_without_direct_coverage_spans, &self.coverage_counters);
 
         for (edge_from_bcb, target_bcb, counter_kind) in bcb_counters_without_direct_coverage_spans
         {
@@ -440,7 +442,8 @@ impl<'a, 'tcx> Instrumentor<'a, 'tcx> {
     fn make_mir_coverage_kind(&self, counter_kind: &BcbCounter) -> CoverageKind {
         match *counter_kind {
             BcbCounter::Counter { id } => CoverageKind::Counter { id },
-            BcbCounter::Expression { id, lhs, op, rhs } => {
+            BcbCounter::Expression { id } => {
+                let Expression { lhs, op, rhs } = self.coverage_counters.expressions[id];
                 CoverageKind::Expression { id, lhs, op, rhs }
             }
         }