Skip to content

Commit

Permalink
coverage: Remove incorrect assertions from counter allocation
Browse files Browse the repository at this point in the history
These assertions detect situations where a BCB node would have both a physical
counter and one or more in-edge counters/expressions.

For most BCBs that situation would indicate an implementation bug. However,
it's perfectly fine in the case of a BCB having an edge that loops back to
itself.

Given the complexity and risk involved in fixing the assertions, and the fact
that nothing relies on them actually being true, this patch just removes them
instead.
  • Loading branch information
Zalathar committed Mar 20, 2024
1 parent 70206f0 commit 85bec7a
Show file tree
Hide file tree
Showing 3 changed files with 2 additions and 33 deletions.
31 changes: 0 additions & 31 deletions compiler/rustc_mir_transform/src/coverage/counters.rs
Original file line number Diff line number Diff line change
@@ -1,7 +1,6 @@
use rustc_data_structures::captures::Captures;
use rustc_data_structures::fx::FxHashMap;
use rustc_data_structures::graph::WithNumNodes;
use rustc_index::bit_set::BitSet;
use rustc_index::IndexVec;
use rustc_middle::mir::coverage::*;

Expand All @@ -18,10 +17,6 @@ pub(super) enum BcbCounter {
}

impl BcbCounter {
fn is_expression(&self) -> bool {
matches!(self, Self::Expression { .. })
}

pub(super) fn as_term(&self) -> CovTerm {
match *self {
BcbCounter::Counter { id, .. } => CovTerm::Counter(id),
Expand Down Expand Up @@ -60,10 +55,6 @@ pub(super) struct CoverageCounters {
/// We currently don't iterate over this map, but if we do in the future,
/// switch it back to `FxIndexMap` to avoid query stability hazards.
bcb_edge_counters: FxHashMap<(BasicCoverageBlock, BasicCoverageBlock), BcbCounter>,
/// Tracks which BCBs have a counter associated with some incoming edge.
/// Only used by 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>,
/// Table of expression data, associating each expression ID with its
/// corresponding operator (+ or -) and its LHS/RHS operands.
expressions: IndexVec<ExpressionId, Expression>,
Expand All @@ -83,7 +74,6 @@ impl CoverageCounters {
counter_increment_sites: IndexVec::new(),
bcb_counters: IndexVec::from_elem_n(None, num_bcbs),
bcb_edge_counters: FxHashMap::default(),
bcb_has_incoming_edge_counters: BitSet::new_empty(num_bcbs),
expressions: IndexVec::new(),
};

Expand Down Expand Up @@ -122,14 +112,6 @@ impl CoverageCounters {
}

fn set_bcb_counter(&mut self, bcb: BasicCoverageBlock, counter_kind: BcbCounter) -> BcbCounter {
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
// `BasicCoverageBlock`).
counter_kind.is_expression() || !self.bcb_has_incoming_edge_counters.contains(bcb),
"attempt to add a `Counter` to a BCB target with existing incoming edge counters"
);

if let Some(replaced) = self.bcb_counters[bcb].replace(counter_kind) {
bug!(
"attempt to set a BasicCoverageBlock coverage counter more than once; \
Expand All @@ -146,19 +128,6 @@ impl CoverageCounters {
to_bcb: BasicCoverageBlock,
counter_kind: BcbCounter,
) -> BcbCounter {
// 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()
{
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);
if let Some(replaced) = self.bcb_edge_counters.insert((from_bcb, to_bcb), counter_kind) {
bug!(
"attempt to set an edge counter more than once; from_bcb: \
Expand Down
2 changes: 1 addition & 1 deletion tests/coverage/let_else_loop.coverage
Original file line number Diff line number Diff line change
@@ -1,6 +1,6 @@
LL| |#![feature(coverage_attribute)]
LL| |//@ edition: 2021
LL| |//@ ignore-test
LL| |
LL| |// Regression test for <https://github.com/rust-lang/rust/issues/122738>.
LL| |// These code patterns should not trigger an ICE when allocating a physical
LL| |// counter to a node and also one of its in-edges, because that is allowed
Expand Down
2 changes: 1 addition & 1 deletion tests/coverage/let_else_loop.rs
Original file line number Diff line number Diff line change
@@ -1,6 +1,6 @@
#![feature(coverage_attribute)]
//@ edition: 2021
//@ ignore-test

// Regression test for <https://github.com/rust-lang/rust/issues/122738>.
// These code patterns should not trigger an ICE when allocating a physical
// counter to a node and also one of its in-edges, because that is allowed
Expand Down

0 comments on commit 85bec7a

Please sign in to comment.