Make coverage expression IDs count up from 0, not down from `u32::MAX`

Operand types are now tracked explicitly, so there is no need for expression
IDs to avoid counter IDs by descending from `u32::MAX`. Instead they can just
count up from 0, and can be used directly as indices when necessary.

2 files changed, 16 insertions, 37 deletions

diff --git a/compiler/rustc_codegen_llvm/src/coverageinfo/map_data.rs b/compiler/rustc_codegen_llvm/src/coverageinfo/map_data.rs
index 0d4d8ca61e9..4fada329367 100644
--- a/compiler/rustc_codegen_llvm/src/coverageinfo/map_data.rs
+++ b/compiler/rustc_codegen_llvm/src/coverageinfo/map_data.rs
@@ -3,8 +3,7 @@ pub use super::ffi::*;
 use rustc_index::{IndexSlice, IndexVec};
 use rustc_middle::bug;
 use rustc_middle::mir::coverage::{
-    CodeRegion, CounterValueReference, InjectedExpressionId, InjectedExpressionIndex,
-    MappedExpressionIndex, Op, Operand,
+    CodeRegion, CounterValueReference, ExpressionId, MappedExpressionIndex, Op, Operand,
 };
 use rustc_middle::ty::Instance;
 use rustc_middle::ty::TyCtxt;
@@ -19,8 +18,7 @@ pub struct Expression {
 
 /// Collects all of the coverage regions associated with (a) injected counters, (b) counter
 /// expressions (additions or subtraction), and (c) unreachable regions (always counted as zero),
-/// for a given Function. Counters and counter expressions have non-overlapping `id`s because they
-/// can both be operands in an expression. This struct also stores the `function_source_hash`,
+/// for a given Function. This struct also stores the `function_source_hash`,
 /// computed during instrumentation, and forwarded with counters.
 ///
 /// Note, it may be important to understand LLVM's definitions of `unreachable` regions versus "gap
@@ -35,7 +33,7 @@ pub struct FunctionCoverage<'tcx> {
     source_hash: u64,
     is_used: bool,
     counters: IndexVec<CounterValueReference, Option<CodeRegion>>,
-    expressions: IndexVec<InjectedExpressionIndex, Option<Expression>>,
+    expressions: IndexVec<ExpressionId, Option<Expression>>,
     unreachable_regions: Vec<CodeRegion>,
 }
 
@@ -89,22 +87,11 @@ impl<'tcx> FunctionCoverage<'tcx> {
     }
 
     /// Both counters and "counter expressions" (or simply, "expressions") can be operands in other
-    /// expressions. Expression IDs start from `u32::MAX` and go down, so the range of expression
-    /// IDs will not overlap with the range of counter IDs. Counters and expressions can be added in
-    /// any order, and expressions can still be assigned contiguous (though descending) IDs, without
-    /// knowing what the last counter ID will be.
-    ///
-    /// When storing the expression data in the `expressions` vector in the `FunctionCoverage`
-    /// struct, its vector index is computed, from the given expression ID, by subtracting from
-    /// `u32::MAX`.
-    ///
-    /// Since the expression operands (`lhs` and `rhs`) can reference either counters or
-    /// expressions, an operand that references an expression also uses its original ID, descending
-    /// from `u32::MAX`. Theses operands are translated only during code generation, after all
-    /// counters and expressions have been added.
+    /// expressions. These are tracked as separate variants of `Operand`, so there is no ambiguity
+    /// between operands that are counter IDs and operands that are expression IDs.
     pub fn add_counter_expression(
         &mut self,
-        expression_id: InjectedExpressionId,
+        expression_id: ExpressionId,
         lhs: Operand,
         op: Op,
         rhs: Operand,
@@ -114,16 +101,15 @@ impl<'tcx> FunctionCoverage<'tcx> {
             "add_counter_expression({:?}, lhs={:?}, op={:?}, rhs={:?} at {:?}",
             expression_id, lhs, op, rhs, region
         );
-        let expression_index = self.expression_index(expression_id);
         debug_assert!(
-            expression_index.as_usize() < self.expressions.len(),
-            "expression_index {} is out of range for expressions.len() = {}
+            expression_id.as_usize() < self.expressions.len(),
+            "expression_id {} is out of range for expressions.len() = {}
             for {:?}",
-            expression_index.as_usize(),
+            expression_id.as_usize(),
             self.expressions.len(),
             self,
         );
-        if let Some(previous_expression) = self.expressions[expression_index].replace(Expression {
+        if let Some(previous_expression) = self.expressions[expression_id].replace(Expression {
             lhs,
             op,
             rhs,
@@ -190,7 +176,7 @@ impl<'tcx> FunctionCoverage<'tcx> {
         //
         // Expressions will be returned from this function in a sequential vector (array) of
         // `CounterExpression`, so the expression IDs must be mapped from their original,
-        // potentially sparse set of indexes, originally in reverse order from `u32::MAX`.
+        // potentially sparse set of indexes.
         //
         // An `Expression` as an operand will have already been encountered as an `Expression` with
         // operands, so its new_index will already have been generated (as a 1-up index value).
@@ -203,7 +189,7 @@ impl<'tcx> FunctionCoverage<'tcx> {
         // `expression_index`s lower than the referencing `Expression`. Therefore, it is
         // reasonable to look up the new index of an expression operand while the `new_indexes`
         // vector is only complete up to the current `ExpressionIndex`.
-        type NewIndexes = IndexSlice<InjectedExpressionIndex, Option<MappedExpressionIndex>>;
+        type NewIndexes = IndexSlice<ExpressionId, Option<MappedExpressionIndex>>;
         let id_to_counter = |new_indexes: &NewIndexes, operand: Operand| match operand {
             Operand::Zero => Some(Counter::zero()),
             Operand::Counter(id) => {
@@ -219,15 +205,14 @@ impl<'tcx> FunctionCoverage<'tcx> {
                 Some(Counter::counter_value_reference(index))
             }
             Operand::Expression(id) => {
-                let index = self.expression_index(id);
                 self.expressions
-                    .get(index)
+                    .get(id)
                     .expect("expression id is out of range")
                     .as_ref()
                     // If an expression was optimized out, assume it would have produced a count
                     // of zero. This ensures that expressions dependent on optimized-out
                     // expressions are still valid.
-                    .map_or(Some(Counter::zero()), |_| new_indexes[index].map(Counter::expression))
+                    .map_or(Some(Counter::zero()), |_| new_indexes[id].map(Counter::expression))
             }
         };
 
@@ -334,10 +319,4 @@ impl<'tcx> FunctionCoverage<'tcx> {
     fn unreachable_regions(&self) -> impl Iterator<Item = (Counter, &CodeRegion)> {
         self.unreachable_regions.iter().map(|region| (Counter::zero(), region))
     }
-
-    fn expression_index(&self, id: InjectedExpressionId) -> InjectedExpressionIndex {
-        debug_assert!(id.as_usize() >= self.counters.len());
-        let id_descending_from_max = id.as_u32();
-        InjectedExpressionIndex::from(u32::MAX - id_descending_from_max)
-    }
 }
diff --git a/compiler/rustc_codegen_llvm/src/coverageinfo/mod.rs b/compiler/rustc_codegen_llvm/src/coverageinfo/mod.rs
index d6a32497f7f..5e0c5df2bcb 100644
--- a/compiler/rustc_codegen_llvm/src/coverageinfo/mod.rs
+++ b/compiler/rustc_codegen_llvm/src/coverageinfo/mod.rs
@@ -17,7 +17,7 @@ use rustc_hir::def_id::DefId;
 use rustc_llvm::RustString;
 use rustc_middle::bug;
 use rustc_middle::mir::coverage::{
-    CodeRegion, CounterValueReference, CoverageKind, InjectedExpressionId, Op, Operand,
+    CodeRegion, CounterValueReference, CoverageKind, ExpressionId, Op, Operand,
 };
 use rustc_middle::mir::Coverage;
 use rustc_middle::ty;
@@ -202,7 +202,7 @@ impl<'tcx> Builder<'_, '_, 'tcx> {
     fn add_coverage_counter_expression(
         &mut self,
         instance: Instance<'tcx>,
-        id: InjectedExpressionId,
+        id: ExpressionId,
         lhs: Operand,
         op: Op,
         rhs: Operand,