Make coverage counter IDs count up from 0, not 1

Operand types are now tracked explicitly, so there is no need to reserve ID 0
for the special always-zero counter.

As part of the renumbering, this change fixes an off-by-one error in the way
counters were counted by the `coverageinfo` query. As a result, functions
should now have exactly the number of counters they actually need, instead of
always having an extra counter that is never used.

12 files changed, 43 insertions, 61 deletions

diff --git a/compiler/rustc_codegen_llvm/src/coverageinfo/ffi.rs b/compiler/rustc_codegen_llvm/src/coverageinfo/ffi.rs
index 1791ce4b315..6ca61aa43d7 100644
--- a/compiler/rustc_codegen_llvm/src/coverageinfo/ffi.rs
+++ b/compiler/rustc_codegen_llvm/src/coverageinfo/ffi.rs
@@ -1,4 +1,4 @@
-use rustc_middle::mir::coverage::{CounterValueReference, MappedExpressionIndex};
+use rustc_middle::mir::coverage::{CounterId, MappedExpressionIndex};
 
 /// Must match the layout of `LLVMRustCounterKind`.
 #[derive(Copy, Clone, Debug)]
@@ -36,11 +36,9 @@ impl Counter {
         Self { kind: CounterKind::Zero, id: 0 }
     }
 
-    /// Constructs a new `Counter` of kind `CounterValueReference`, and converts
-    /// the given 1-based counter_id to the required 0-based equivalent for
-    /// the `Counter` encoding.
-    pub fn counter_value_reference(counter_id: CounterValueReference) -> Self {
-        Self { kind: CounterKind::CounterValueReference, id: counter_id.zero_based_index() }
+    /// Constructs a new `Counter` of kind `CounterValueReference`.
+    pub fn counter_value_reference(counter_id: CounterId) -> Self {
+        Self { kind: CounterKind::CounterValueReference, id: counter_id.as_u32() }
     }
 
     /// Constructs a new `Counter` of kind `Expression`.
diff --git a/compiler/rustc_codegen_llvm/src/coverageinfo/map_data.rs b/compiler/rustc_codegen_llvm/src/coverageinfo/map_data.rs
index 4fada329367..7e981af0a53 100644
--- a/compiler/rustc_codegen_llvm/src/coverageinfo/map_data.rs
+++ b/compiler/rustc_codegen_llvm/src/coverageinfo/map_data.rs
@@ -3,7 +3,7 @@ pub use super::ffi::*;
 use rustc_index::{IndexSlice, IndexVec};
 use rustc_middle::bug;
 use rustc_middle::mir::coverage::{
-    CodeRegion, CounterValueReference, ExpressionId, MappedExpressionIndex, Op, Operand,
+    CodeRegion, CounterId, ExpressionId, MappedExpressionIndex, Op, Operand,
 };
 use rustc_middle::ty::Instance;
 use rustc_middle::ty::TyCtxt;
@@ -32,7 +32,7 @@ pub struct FunctionCoverage<'tcx> {
     instance: Instance<'tcx>,
     source_hash: u64,
     is_used: bool,
-    counters: IndexVec<CounterValueReference, Option<CodeRegion>>,
+    counters: IndexVec<CounterId, Option<CodeRegion>>,
     expressions: IndexVec<ExpressionId, Option<Expression>>,
     unreachable_regions: Vec<CodeRegion>,
 }
@@ -80,7 +80,7 @@ impl<'tcx> FunctionCoverage<'tcx> {
     }
 
     /// Adds a code region to be counted by an injected counter intrinsic.
-    pub fn add_counter(&mut self, id: CounterValueReference, region: CodeRegion) {
+    pub fn add_counter(&mut self, id: CounterId, region: CodeRegion) {
         if let Some(previous_region) = self.counters[id].replace(region.clone()) {
             assert_eq!(previous_region, region, "add_counter: code region for id changed");
         }
@@ -192,18 +192,7 @@ impl<'tcx> FunctionCoverage<'tcx> {
         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) => {
-                debug_assert!(
-                    id.index() > 0,
-                    "Operand::Counter indexes for counters are 1-based, but this id={}",
-                    id.index()
-                );
-                // Note: Some codegen-injected Counters may be only referenced by `Expression`s,
-                // and may not have their own `CodeRegion`s,
-                let index = CounterValueReference::from(id.index());
-                // Note, the conversion to LLVM `Counter` adjusts the index to be zero-based.
-                Some(Counter::counter_value_reference(index))
-            }
+            Operand::Counter(id) => Some(Counter::counter_value_reference(id)),
             Operand::Expression(id) => {
                 self.expressions
                     .get(id)
diff --git a/compiler/rustc_codegen_llvm/src/coverageinfo/mod.rs b/compiler/rustc_codegen_llvm/src/coverageinfo/mod.rs
index 5e0c5df2bcb..c1017ab8d7c 100644
--- a/compiler/rustc_codegen_llvm/src/coverageinfo/mod.rs
+++ b/compiler/rustc_codegen_llvm/src/coverageinfo/mod.rs
@@ -16,9 +16,7 @@ use rustc_hir as hir;
 use rustc_hir::def_id::DefId;
 use rustc_llvm::RustString;
 use rustc_middle::bug;
-use rustc_middle::mir::coverage::{
-    CodeRegion, CounterValueReference, CoverageKind, ExpressionId, Op, Operand,
-};
+use rustc_middle::mir::coverage::{CodeRegion, CounterId, CoverageKind, ExpressionId, Op, Operand};
 use rustc_middle::mir::Coverage;
 use rustc_middle::ty;
 use rustc_middle::ty::layout::{FnAbiOf, HasTyCtxt};
@@ -33,7 +31,7 @@ mod ffi;
 pub(crate) mod map_data;
 pub mod mapgen;
 
-const UNUSED_FUNCTION_COUNTER_ID: CounterValueReference = CounterValueReference::START;
+const UNUSED_FUNCTION_COUNTER_ID: CounterId = CounterId::START;
 
 const VAR_ALIGN_BYTES: usize = 8;
 
@@ -125,7 +123,7 @@ impl<'tcx> CoverageInfoBuilderMethods<'tcx> for Builder<'_, '_, 'tcx> {
                     let fn_name = bx.get_pgo_func_name_var(instance);
                     let hash = bx.const_u64(function_source_hash);
                     let num_counters = bx.const_u32(coverageinfo.num_counters);
-                    let index = bx.const_u32(id.zero_based_index());
+                    let index = bx.const_u32(id.as_u32());
                     debug!(
                         "codegen intrinsic instrprof.increment(fn_name={:?}, hash={:?}, num_counters={:?}, index={:?})",
                         fn_name, hash, num_counters, index,
@@ -178,7 +176,7 @@ impl<'tcx> Builder<'_, '_, 'tcx> {
     fn add_coverage_counter(
         &mut self,
         instance: Instance<'tcx>,
-        id: CounterValueReference,
+        id: CounterId,
         region: CodeRegion,
     ) -> bool {
         if let Some(coverage_context) = self.coverage_context() {
diff --git a/compiler/rustc_middle/src/mir/coverage.rs b/compiler/rustc_middle/src/mir/coverage.rs
index bcec46d9149..d7d6e3a0086 100644
--- a/compiler/rustc_middle/src/mir/coverage.rs
+++ b/compiler/rustc_middle/src/mir/coverage.rs
@@ -6,22 +6,22 @@ use rustc_span::Symbol;
 use std::fmt::{self, Debug, Formatter};
 
 rustc_index::newtype_index! {
+    /// ID of a coverage counter. Values ascend from 0.
+    ///
+    /// Note that LLVM handles counter IDs as `uint32_t`, so there is no need
+    /// to use a larger representation on the Rust side.
     #[derive(HashStable)]
     #[max = 0xFFFF_FFFF]
-    #[debug_format = "CounterValueReference({})"]
-    pub struct CounterValueReference {}
+    #[debug_format = "CounterId({})"]
+    pub struct CounterId {}
 }
 
-impl CounterValueReference {
-    /// Counters start at 1 for historical reasons.
-    pub const START: Self = Self::from_u32(1);
+impl CounterId {
+    pub const START: Self = Self::from_u32(0);
 
-    /// Returns explicitly-requested zero-based version of the counter id, used
-    /// during codegen. LLVM expects zero-based indexes.
-    pub fn zero_based_index(self) -> u32 {
-        let one_based_index = self.as_u32();
-        debug_assert!(one_based_index > 0);
-        one_based_index - 1
+    #[inline(always)]
+    pub fn next_id(self) -> Self {
+        Self::from_u32(self.as_u32() + 1)
     }
 }
 
@@ -63,7 +63,7 @@ rustc_index::newtype_index! {
 #[derive(TyEncodable, TyDecodable, Hash, HashStable, TypeFoldable, TypeVisitable)]
 pub enum Operand {
     Zero,
-    Counter(CounterValueReference),
+    Counter(CounterId),
     Expression(ExpressionId),
 }
 
@@ -83,7 +83,7 @@ pub enum CoverageKind {
         function_source_hash: u64,
         /// ID of this counter within its enclosing function.
         /// Expressions in the same function can refer to it as an operand.
-        id: CounterValueReference,
+        id: CounterId,
     },
     Expression {
         /// ID of this coverage-counter expression within its enclosing function.
diff --git a/compiler/rustc_middle/src/ty/structural_impls.rs b/compiler/rustc_middle/src/ty/structural_impls.rs
index 15cf4c46788..c6b6f0e8990 100644
--- a/compiler/rustc_middle/src/ty/structural_impls.rs
+++ b/compiler/rustc_middle/src/ty/structural_impls.rs
@@ -470,7 +470,7 @@ TrivialTypeTraversalAndLiftImpls! {
     ::rustc_hir::Unsafety,
     ::rustc_target::asm::InlineAsmRegOrRegClass,
     ::rustc_target::spec::abi::Abi,
-    crate::mir::coverage::CounterValueReference,
+    crate::mir::coverage::CounterId,
     crate::mir::coverage::ExpressionId,
     crate::mir::coverage::MappedExpressionIndex,
     crate::mir::Local,
diff --git a/compiler/rustc_mir_transform/src/coverage/counters.rs b/compiler/rustc_mir_transform/src/coverage/counters.rs
index cd2539a5e66..97bdb878ab1 100644
--- a/compiler/rustc_mir_transform/src/coverage/counters.rs
+++ b/compiler/rustc_mir_transform/src/coverage/counters.rs
@@ -16,7 +16,7 @@ use rustc_middle::mir::coverage::*;
 /// `Coverage` statements.
 pub(super) struct CoverageCounters {
     function_source_hash: u64,
-    next_counter_id: u32,
+    next_counter_id: CounterId,
     next_expression_id: ExpressionId,
     pub debug_counters: DebugCounters,
 }
@@ -25,7 +25,7 @@ impl CoverageCounters {
     pub fn new(function_source_hash: u64) -> Self {
         Self {
             function_source_hash,
-            next_counter_id: CounterValueReference::START.as_u32(),
+            next_counter_id: CounterId::START,
             next_expression_id: ExpressionId::START,
             debug_counters: DebugCounters::new(),
         }
@@ -93,10 +93,10 @@ impl CoverageCounters {
     }
 
     /// Counter IDs start from one and go up.
-    fn next_counter(&mut self) -> CounterValueReference {
+    fn next_counter(&mut self) -> CounterId {
         let next = self.next_counter_id;
-        self.next_counter_id += 1;
-        CounterValueReference::from(next)
+        self.next_counter_id = next.next_id();
+        next
     }
 
     /// Expression IDs start from 0 and go up.
diff --git a/compiler/rustc_mir_transform/src/coverage/query.rs b/compiler/rustc_mir_transform/src/coverage/query.rs
index c1a896dbd34..aa205655f9d 100644
--- a/compiler/rustc_mir_transform/src/coverage/query.rs
+++ b/compiler/rustc_mir_transform/src/coverage/query.rs
@@ -43,11 +43,9 @@ struct CoverageVisitor {
 }
 
 impl CoverageVisitor {
-    /// Updates `num_counters` to the maximum encountered zero-based counter_id plus 1. Note the
-    /// final computed number of counters should be the number of all `CoverageKind::Counter`
-    /// statements in the MIR *plus one* for the implicit `ZERO` counter.
+    /// Updates `num_counters` to the maximum encountered counter ID plus 1.
     #[inline(always)]
-    fn update_num_counters(&mut self, counter_id: CounterValueReference) {
+    fn update_num_counters(&mut self, counter_id: CounterId) {
         let counter_id = counter_id.as_u32();
         self.info.num_counters = std::cmp::max(self.info.num_counters, counter_id + 1);
     }
@@ -103,8 +101,7 @@ fn coverageinfo<'tcx>(tcx: TyCtxt<'tcx>, instance_def: ty::InstanceDef<'tcx>) ->
     let mir_body = tcx.instance_mir(instance_def);
 
     let mut coverage_visitor = CoverageVisitor {
-        // num_counters always has at least the `ZERO` counter.
-        info: CoverageInfo { num_counters: 1, num_expressions: 0 },
+        info: CoverageInfo { num_counters: 0, num_expressions: 0 },
         add_missing_operands: false,
     };
 
diff --git a/compiler/rustc_mir_transform/src/coverage/tests.rs b/compiler/rustc_mir_transform/src/coverage/tests.rs
index 25891d3ca0f..248a192f8f5 100644
--- a/compiler/rustc_mir_transform/src/coverage/tests.rs
+++ b/compiler/rustc_mir_transform/src/coverage/tests.rs
@@ -683,7 +683,7 @@ fn test_make_bcb_counters() {
 
         let_bcb!(1);
         assert_eq!(
-            1, // coincidentally, bcb1 has a `Counter` with id = 1
+            0, // bcb1 has a `Counter` with id = 0
             match basic_coverage_blocks[bcb1].counter().expect("should have a counter") {
                 CoverageKind::Counter { id, .. } => id,
                 _ => panic!("expected a Counter"),
@@ -693,7 +693,7 @@ fn test_make_bcb_counters() {
 
         let_bcb!(2);
         assert_eq!(
-            2, // coincidentally, bcb2 has a `Counter` with id = 2
+            1, // bcb2 has a `Counter` with id = 1
             match basic_coverage_blocks[bcb2].counter().expect("should have a counter") {
                 CoverageKind::Counter { id, .. } => id,
                 _ => panic!("expected a Counter"),
diff --git a/tests/mir-opt/coverage_graphviz.bar.InstrumentCoverage.0.dot b/tests/mir-opt/coverage_graphviz.bar.InstrumentCoverage.0.dot
index 03df5c9504b..3b90aaeae42 100644
--- a/tests/mir-opt/coverage_graphviz.bar.InstrumentCoverage.0.dot
+++ b/tests/mir-opt/coverage_graphviz.bar.InstrumentCoverage.0.dot
@@ -2,5 +2,5 @@ digraph Cov_0_4 {
     graph [fontname="Courier, monospace"];
     node [fontname="Courier, monospace"];
     edge [fontname="Courier, monospace"];
-    bcb0__Cov_0_4 [shape="none", label=<<table border="0" cellborder="1" cellspacing="0"><tr><td bgcolor="gray" align="center" colspan="1">bcb0</td></tr><tr><td align="left" balign="left"></td></tr><tr><td align="left" balign="left">Counter(bcb0) at 18:1-20:2<br align="left"/>    19:5-19:9: @0[0]: Coverage::Counter(1) for $DIR/coverage_graphviz.rs:18:1 - 20:2<br align="left"/>    20:2-20:2: @0.Return: return</td></tr><tr><td align="left" balign="left">bb0: Return</td></tr></table>>];
+    bcb0__Cov_0_4 [shape="none", label=<<table border="0" cellborder="1" cellspacing="0"><tr><td bgcolor="gray" align="center" colspan="1">bcb0</td></tr><tr><td align="left" balign="left"></td></tr><tr><td align="left" balign="left">Counter(bcb0) at 18:1-20:2<br align="left"/>    19:5-19:9: @0[0]: Coverage::Counter(0) for $DIR/coverage_graphviz.rs:18:1 - 20:2<br align="left"/>    20:2-20:2: @0.Return: return</td></tr><tr><td align="left" balign="left">bb0: Return</td></tr></table>>];
 }
diff --git a/tests/mir-opt/coverage_graphviz.main.InstrumentCoverage.0.dot b/tests/mir-opt/coverage_graphviz.main.InstrumentCoverage.0.dot
index affd0fb18d5..19c220e2e1d 100644
--- a/tests/mir-opt/coverage_graphviz.main.InstrumentCoverage.0.dot
+++ b/tests/mir-opt/coverage_graphviz.main.InstrumentCoverage.0.dot
@@ -2,7 +2,7 @@ digraph Cov_0_3 {
     graph [fontname="Courier, monospace"];
     node [fontname="Courier, monospace"];
     edge [fontname="Courier, monospace"];
-    bcb3__Cov_0_3 [shape="none", label=<<table border="0" cellborder="1" cellspacing="0"><tr><td bgcolor="gray" align="center" colspan="1">bcb3</td></tr><tr><td align="left" balign="left">Counter(bcb3) at 13:10-13:10<br align="left"/>    13:10-13:10: @5[0]: Coverage::Counter(2) for $DIR/coverage_graphviz.rs:13:10 - 13:11</td></tr><tr><td align="left" balign="left">bb5: Goto</td></tr></table>>];
+    bcb3__Cov_0_3 [shape="none", label=<<table border="0" cellborder="1" cellspacing="0"><tr><td bgcolor="gray" align="center" colspan="1">bcb3</td></tr><tr><td align="left" balign="left">Counter(bcb3) at 13:10-13:10<br align="left"/>    13:10-13:10: @5[0]: Coverage::Counter(1) for $DIR/coverage_graphviz.rs:13:10 - 13:11</td></tr><tr><td align="left" balign="left">bb5: Goto</td></tr></table>>];
     bcb2__Cov_0_3 [shape="none", label=<<table border="0" cellborder="1" cellspacing="0"><tr><td bgcolor="gray" align="center" colspan="1">bcb2</td></tr><tr><td align="left" balign="left">Expression(bcb1:(bcb0 + bcb3) - bcb3) at 12:13-12:18<br align="left"/>    12:13-12:18: @4[0]: Coverage::Expression(2) = Expression(1) + Zero for $DIR/coverage_graphviz.rs:15:1 - 15:2<br align="left"/>Expression(bcb2:(bcb1:(bcb0 + bcb3) - bcb3) + 0) at 15:2-15:2<br align="left"/>    15:2-15:2: @4.Return: return</td></tr><tr><td align="left" balign="left">bb4: Return</td></tr></table>>];
     bcb1__Cov_0_3 [shape="none", label=<<table border="0" cellborder="1" cellspacing="0"><tr><td bgcolor="gray" align="center" colspan="1">bcb1</td></tr><tr><td align="left" balign="left">Expression(bcb0 + bcb3) at 10:5-11:17<br align="left"/>    11:12-11:17: @2.Call: _2 = bar() -&gt; [return: bb3, unwind: bb6]</td></tr><tr><td align="left" balign="left">bb1: FalseUnwind<br align="left"/>bb2: Call</td></tr><tr><td align="left" balign="left">bb3: SwitchInt</td></tr></table>>];
     bcb0__Cov_0_3 [shape="none", label=<<table border="0" cellborder="1" cellspacing="0"><tr><td bgcolor="gray" align="center" colspan="1">bcb0</td></tr><tr><td align="left" balign="left"></td></tr><tr><td align="left" balign="left">Counter(bcb0) at 9:1-9:11<br align="left"/>    </td></tr><tr><td align="left" balign="left">bb0: Goto</td></tr></table>>];
diff --git a/tests/mir-opt/instrument_coverage.bar.InstrumentCoverage.diff b/tests/mir-opt/instrument_coverage.bar.InstrumentCoverage.diff
index 0aece766bd5..afcfde09c02 100644
--- a/tests/mir-opt/instrument_coverage.bar.InstrumentCoverage.diff
+++ b/tests/mir-opt/instrument_coverage.bar.InstrumentCoverage.diff
@@ -5,7 +5,7 @@
       let mut _0: bool;
   
       bb0: {
-+         Coverage::Counter(1) for /the/src/instrument_coverage.rs:20:1 - 22:2;
++         Coverage::Counter(0) for /the/src/instrument_coverage.rs:20:1 - 22:2;
           _0 = const true;
           return;
       }
diff --git a/tests/mir-opt/instrument_coverage.main.InstrumentCoverage.diff b/tests/mir-opt/instrument_coverage.main.InstrumentCoverage.diff
index e87ed5268e0..e17c6ddc56e 100644
--- a/tests/mir-opt/instrument_coverage.main.InstrumentCoverage.diff
+++ b/tests/mir-opt/instrument_coverage.main.InstrumentCoverage.diff
@@ -8,12 +8,12 @@
       let mut _3: !;
   
       bb0: {
-+         Coverage::Counter(1) for /the/src/instrument_coverage.rs:11:1 - 11:11;
++         Coverage::Counter(0) for /the/src/instrument_coverage.rs:11:1 - 11:11;
           goto -> bb1;
       }
   
       bb1: {
-+         Coverage::Expression(0) = Counter(1) + Counter(2) for /the/src/instrument_coverage.rs:12:5 - 13:17;
++         Coverage::Expression(0) = Counter(0) + Counter(1) for /the/src/instrument_coverage.rs:12:5 - 13:17;
           falseUnwind -> [real: bb2, unwind: bb6];
       }
   
@@ -28,14 +28,14 @@
   
       bb4: {
 +         Coverage::Expression(2) = Expression(1) + Zero for /the/src/instrument_coverage.rs:17:1 - 17:2;
-+         Coverage::Expression(1) = Expression(0) - Counter(2) for /the/src/instrument_coverage.rs:14:13 - 14:18;
++         Coverage::Expression(1) = Expression(0) - Counter(1) for /the/src/instrument_coverage.rs:14:13 - 14:18;
           _0 = const ();
           StorageDead(_2);
           return;
       }
   
       bb5: {
-+         Coverage::Counter(2) for /the/src/instrument_coverage.rs:15:10 - 15:11;
++         Coverage::Counter(1) for /the/src/instrument_coverage.rs:15:10 - 15:11;
           _1 = const ();
           StorageDead(_2);
           goto -> bb1;