coverage: Replace impossible coverage::Error with assertions

Historically, these errors existed so that the coverage debug code could dump
additional information before reporting a compiler bug. That debug code was
removed by #115962, so we can now simplify these methods by making them panic
when they detect a bug.

compiler/rustc_mir_transform/src/coverage/counters.rs

@@ -1,5 +1,3 @@
-use super::Error;
-
 use super::graph;
 
 use graph::{BasicCoverageBlock, BcbBranch, CoverageGraph, TraverseCoverageGraphWithLoops};
@@ -81,7 +79,7 @@ impl CoverageCounters {
         &mut self,
         basic_coverage_blocks: &CoverageGraph,
         bcb_has_coverage_spans: impl Fn(BasicCoverageBlock) -> bool,
-    ) -> Result<(), Error> {
+    ) {
         MakeBcbCounters::new(self, basic_coverage_blocks).make_bcb_counters(bcb_has_coverage_spans)
     }
 
@@ -111,11 +109,7 @@ impl CoverageCounters {
         self.expressions.len()
     }
 
-    fn set_bcb_counter(
+    fn set_bcb_counter(&mut self, bcb: BasicCoverageBlock, counter_kind: BcbCounter) -> CovTerm {
-        &mut self,
-        bcb: BasicCoverageBlock,
-        counter_kind: BcbCounter,
-    ) -> Result<CovTerm, Error> {
         assert!(
             // If the BCB has an edge counter (to be injected into a new `BasicBlock`), it can also
             // have an expression (to be injected into an existing `BasicBlock` represented by this
@@ -126,12 +120,12 @@ impl CoverageCounters {
 
         let term = counter_kind.as_term();
         if let Some(replaced) = self.bcb_counters[bcb].replace(counter_kind) {
-            Error::from_string(format!(
+            bug!(
                 "attempt to set a BasicCoverageBlock coverage counter more than once; \
                 {bcb:?} already had counter {replaced:?}",
-            ))
+            );
         } else {
-            Ok(term)
+            term
         }
     }
 
@@ -140,26 +134,26 @@ impl CoverageCounters {
         from_bcb: BasicCoverageBlock,
         to_bcb: BasicCoverageBlock,
         counter_kind: BcbCounter,
-    ) -> Result<CovTerm, Error> {
+    ) -> CovTerm {
         // If the BCB has an edge counter (to be injected into a new `BasicBlock`), it can also
         // have an expression (to be injected into an existing `BasicBlock` represented by this
         // `BasicCoverageBlock`).
         if let Some(node_counter) = self.bcb_counter(to_bcb) && !node_counter.is_expression() {
-            return Error::from_string(format!(
+            bug!(
                 "attempt to add an incoming edge counter from {from_bcb:?} \
                 when the target BCB already has {node_counter:?}"
-            ));
+            );
         }
 
         self.bcb_has_incoming_edge_counters.insert(to_bcb);
         let term = counter_kind.as_term();
         if let Some(replaced) = self.bcb_edge_counters.insert((from_bcb, to_bcb), counter_kind) {
-            Error::from_string(format!(
+            bug!(
                 "attempt to set an edge counter more than once; from_bcb: \
                 {from_bcb:?} already had counter {replaced:?}",
-            ))
+            );
         } else {
-            Ok(term)
+            term
         }
     }
 
@@ -213,14 +207,7 @@ impl<'a> MakeBcbCounters<'a> {
     /// One way to predict which branch executes the least is by considering loops. A loop is exited
     /// at a branch, so the branch that jumps to a `BasicCoverageBlock` outside the loop is almost
     /// always executed less than the branch that does not exit the loop.
-    ///
+    fn make_bcb_counters(&mut self, bcb_has_coverage_spans: impl Fn(BasicCoverageBlock) -> bool) {
-    /// Returns any non-code-span expressions created to represent intermediate values (such as to
-    /// add two counters so the result can be subtracted from another counter), or an Error with
-    /// message for subsequent debugging.
-    fn make_bcb_counters(
-        &mut self,
-        bcb_has_coverage_spans: impl Fn(BasicCoverageBlock) -> bool,
-    ) -> Result<(), Error> {
         debug!("make_bcb_counters(): adding a counter or expression to each BasicCoverageBlock");
 
         // Walk the `CoverageGraph`. For each `BasicCoverageBlock` node with an associated
@@ -237,10 +224,10 @@ impl<'a> MakeBcbCounters<'a> {
         while let Some(bcb) = traversal.next() {
             if bcb_has_coverage_spans(bcb) {
                 debug!("{:?} has at least one coverage span. Get or make its counter", bcb);
-                let branching_counter_operand = self.get_or_make_counter_operand(bcb)?;
+                let branching_counter_operand = self.get_or_make_counter_operand(bcb);
 
                 if self.bcb_needs_branch_counters(bcb) {
-                    self.make_branch_counters(&traversal, bcb, branching_counter_operand)?;
+                    self.make_branch_counters(&traversal, bcb, branching_counter_operand);
                 }
             } else {
                 debug!(
@@ -251,14 +238,11 @@ impl<'a> MakeBcbCounters<'a> {
             }
         }
 
-        if traversal.is_complete() {
+        assert!(
-            Ok(())
+            traversal.is_complete(),
-        } else {
-            Error::from_string(format!(
             "`TraverseCoverageGraphWithLoops` missed some `BasicCoverageBlock`s: {:?}",
             traversal.unvisited(),
-            ))
+        );
-        }
     }
 
     fn make_branch_counters(
@@ -266,7 +250,7 @@ impl<'a> MakeBcbCounters<'a> {
         traversal: &TraverseCoverageGraphWithLoops<'_>,
         branching_bcb: BasicCoverageBlock,
         branching_counter_operand: CovTerm,
-    ) -> Result<(), Error> {
+    ) {
         let branches = self.bcb_branches(branching_bcb);
         debug!(
             "{:?} has some branch(es) without counters:\n  {}",
@@ -299,10 +283,10 @@ impl<'a> MakeBcbCounters<'a> {
                         counter",
                         branch, branching_bcb
                     );
-                    self.get_or_make_counter_operand(branch.target_bcb)?
+                    self.get_or_make_counter_operand(branch.target_bcb)
                 } else {
                     debug!("  {:?} has multiple incoming edges, so adding an edge counter", branch);
-                    self.get_or_make_edge_counter_operand(branching_bcb, branch.target_bcb)?
+                    self.get_or_make_edge_counter_operand(branching_bcb, branch.target_bcb)
                 };
                 if let Some(sumup_counter_operand) =
                     some_sumup_counter_operand.replace(branch_counter_operand)
@@ -337,19 +321,18 @@ impl<'a> MakeBcbCounters<'a> {
         debug!("{:?} gets an expression: {:?}", expression_branch, expression);
         let bcb = expression_branch.target_bcb;
         if expression_branch.is_only_path_to_target() {
-            self.coverage_counters.set_bcb_counter(bcb, expression)?;
+            self.coverage_counters.set_bcb_counter(bcb, expression);
         } else {
-            self.coverage_counters.set_bcb_edge_counter(branching_bcb, bcb, expression)?;
+            self.coverage_counters.set_bcb_edge_counter(branching_bcb, bcb, expression);
         }
-        Ok(())
     }
 
     #[instrument(level = "debug", skip(self))]
-    fn get_or_make_counter_operand(&mut self, bcb: BasicCoverageBlock) -> Result<CovTerm, Error> {
+    fn get_or_make_counter_operand(&mut self, bcb: BasicCoverageBlock) -> CovTerm {
         // If the BCB already has a counter, return it.
         if let Some(counter_kind) = &self.coverage_counters.bcb_counters[bcb] {
             debug!("{bcb:?} already has a counter: {counter_kind:?}");
-            return Ok(counter_kind.as_term());
+            return counter_kind.as_term();
         }
 
         // A BCB with only one incoming edge gets a simple `Counter` (via `make_counter()`).
@@ -378,10 +361,10 @@ impl<'a> MakeBcbCounters<'a> {
 
         let mut predecessors = self.bcb_predecessors(bcb).to_owned().into_iter();
         let first_edge_counter_operand =
-            self.get_or_make_edge_counter_operand(predecessors.next().unwrap(), bcb)?;
+            self.get_or_make_edge_counter_operand(predecessors.next().unwrap(), bcb);
         let mut some_sumup_edge_counter_operand = None;
         for predecessor in predecessors {
-            let edge_counter_operand = self.get_or_make_edge_counter_operand(predecessor, bcb)?;
+            let edge_counter_operand = self.get_or_make_edge_counter_operand(predecessor, bcb);
             if let Some(sumup_edge_counter_operand) =
                 some_sumup_edge_counter_operand.replace(edge_counter_operand)
             {
@@ -411,7 +394,7 @@ impl<'a> MakeBcbCounters<'a> {
         &mut self,
         from_bcb: BasicCoverageBlock,
         to_bcb: BasicCoverageBlock,
-    ) -> Result<CovTerm, Error> {
+    ) -> CovTerm {
         // If the source BCB has only one successor (assumed to be the given target), an edge
         // counter is unnecessary. Just get or make a counter for the source BCB.
         let successors = self.bcb_successors(from_bcb).iter();
@@ -424,7 +407,7 @@ impl<'a> MakeBcbCounters<'a> {
             self.coverage_counters.bcb_edge_counters.get(&(from_bcb, to_bcb))
         {
             debug!("Edge {from_bcb:?}->{to_bcb:?} already has a counter: {counter_kind:?}");
-            return Ok(counter_kind.as_term());
+            return counter_kind.as_term();
         }
 
         // Make a new counter to count this edge.

compiler/rustc_mir_transform/src/coverage/mod.rs

@@ -26,18 +26,6 @@ use rustc_span::def_id::DefId;
 use rustc_span::source_map::SourceMap;
 use rustc_span::{ExpnKind, SourceFile, Span, Symbol};
 
-/// A simple error message wrapper for `coverage::Error`s.
-#[derive(Debug)]
-struct Error {
-    message: String,
-}
-
-impl Error {
-    pub fn from_string<T>(message: String) -> Result<T, Error> {
-        Err(Self { message })
-    }
-}
-
 /// Inserts `StatementKind::Coverage` statements that either instrument the binary with injected
 /// counters, via intrinsic `llvm.instrprof.increment`, and/or inject metadata used during codegen
 /// to construct the coverage map.
@@ -167,10 +155,7 @@ impl<'a, 'tcx> Instrumentor<'a, 'tcx> {
         // `BasicCoverageBlock`s not already associated with a coverage span.
         let bcb_has_coverage_spans = |bcb| coverage_spans.bcb_has_coverage_spans(bcb);
         self.coverage_counters
-            .make_bcb_counters(&mut self.basic_coverage_blocks, bcb_has_coverage_spans)
+            .make_bcb_counters(&self.basic_coverage_blocks, bcb_has_coverage_spans);
-            .unwrap_or_else(|e| {
-                bug!("Error processing: {:?}: {:?}", self.mir_body.source.def_id(), e.message)
-            });
 
         let mappings = self.create_mappings_and_inject_coverage_statements(&coverage_spans);
 

compiler/rustc_mir_transform/src/coverage/tests.rs

@@ -647,15 +647,13 @@ fn test_traverse_coverage_with_loops() {
 fn test_make_bcb_counters() {
     rustc_span::create_default_session_globals_then(|| {
         let mir_body = goto_switchint();
-        let mut basic_coverage_blocks = graph::CoverageGraph::from_mir(&mir_body);
+        let basic_coverage_blocks = graph::CoverageGraph::from_mir(&mir_body);
         // Historically this test would use `spans` internals to set up fake
         // coverage spans for BCBs 1 and 2. Now we skip that step and just tell
         // BCB counter construction that those BCBs have spans.
         let bcb_has_coverage_spans = |bcb: BasicCoverageBlock| (1..=2).contains(&bcb.as_usize());
         let mut coverage_counters = counters::CoverageCounters::new(&basic_coverage_blocks);
-        coverage_counters
+        coverage_counters.make_bcb_counters(&basic_coverage_blocks, bcb_has_coverage_spans);
-            .make_bcb_counters(&mut basic_coverage_blocks, bcb_has_coverage_spans)
-            .expect("should be Ok");
         assert_eq!(coverage_counters.num_expressions(), 0);
 
         let_bcb!(1);