Skip to content

Commit c5c4e18

Browse files
Rollup merge of #115962 - Zalathar:debug, r=oli-obk
coverage: Remove debug code from the instrumentor The coverage instrumentor has an entire module full of complex code that is only used for debugging. And as I continue to work on coverage, I keep finding that this debug code is constantly causing more trouble than it's worth. It's deeply entangled with current implementation details, such that making any non-trivial change to the instrumentor usually requires major changes to the debug code. And so far I have personally not found any of this debug code to be *useful*. In light of that situation, I'd like to try just ripping all of it out. If I spend any more time dealing with coverage debug code, I want it to be because I'm writing new and useful tools, not dutifully maintaining a boat-anchor that quite plausibly isn't being used by anyone at all. --- r? `@ghost` `@rustbot` label +A-code-coverage --- [Zulip thread](https://rust-lang.zulipchat.com/#narrow/stream/131828-t-compiler/topic/Removing.20debug.20code.20from.20the.20coverage.20instrumentor)
2 parents 4e8f637 + e015f5a commit c5c4e18

File tree

9 files changed

+38
-1087
lines changed

9 files changed

+38
-1087
lines changed

compiler/rustc_mir_transform/src/coverage/counters.rs

+26-75
Original file line numberDiff line numberDiff line change
@@ -1,10 +1,8 @@
11
use super::Error;
22

3-
use super::debug;
43
use super::graph;
54
use super::spans;
65

7-
use debug::{DebugCounters, NESTED_INDENT};
86
use graph::{BasicCoverageBlock, BcbBranch, CoverageGraph, TraverseCoverageGraphWithLoops};
97
use spans::CoverageSpan;
108

@@ -16,6 +14,8 @@ use rustc_middle::mir::coverage::*;
1614

1715
use std::fmt::{self, Debug};
1816

17+
const NESTED_INDENT: &str = " ";
18+
1919
/// The coverage counter or counter expression associated with a particular
2020
/// BCB node or BCB edge.
2121
#[derive(Clone)]
@@ -75,8 +75,6 @@ pub(super) struct CoverageCounters {
7575
/// BCB/edge, but are needed as operands to more complex expressions.
7676
/// These are always [`BcbCounter::Expression`].
7777
pub(super) intermediate_expressions: Vec<BcbCounter>,
78-
79-
pub debug_counters: DebugCounters,
8078
}
8179

8280
impl CoverageCounters {
@@ -91,17 +89,9 @@ impl CoverageCounters {
9189
bcb_edge_counters: FxHashMap::default(),
9290
bcb_has_incoming_edge_counters: BitSet::new_empty(num_bcbs),
9391
intermediate_expressions: Vec::new(),
94-
95-
debug_counters: DebugCounters::new(),
9692
}
9793
}
9894

99-
/// Activate the `DebugCounters` data structures, to provide additional debug formatting
100-
/// features when formatting [`BcbCounter`] (counter) values.
101-
pub fn enable_debug(&mut self) {
102-
self.debug_counters.enable();
103-
}
104-
10595
/// Makes [`BcbCounter`] `Counter`s and `Expressions` for the `BasicCoverageBlock`s directly or
10696
/// indirectly associated with `CoverageSpans`, and accumulates additional `Expression`s
10797
/// representing intermediate values.
@@ -113,44 +103,18 @@ impl CoverageCounters {
113103
MakeBcbCounters::new(self, basic_coverage_blocks).make_bcb_counters(coverage_spans)
114104
}
115105

116-
fn make_counter<F>(&mut self, debug_block_label_fn: F) -> BcbCounter
117-
where
118-
F: Fn() -> Option<String>,
119-
{
120-
let counter = BcbCounter::Counter { id: self.next_counter() };
121-
if self.debug_counters.is_enabled() {
122-
self.debug_counters.add_counter(&counter, (debug_block_label_fn)());
123-
}
124-
counter
106+
fn make_counter(&mut self) -> BcbCounter {
107+
let id = self.next_counter();
108+
BcbCounter::Counter { id }
125109
}
126110

127-
fn make_expression<F>(
128-
&mut self,
129-
lhs: Operand,
130-
op: Op,
131-
rhs: Operand,
132-
debug_block_label_fn: F,
133-
) -> BcbCounter
134-
where
135-
F: Fn() -> Option<String>,
136-
{
111+
fn make_expression(&mut self, lhs: Operand, op: Op, rhs: Operand) -> BcbCounter {
137112
let id = self.next_expression();
138-
let expression = BcbCounter::Expression { id, lhs, op, rhs };
139-
if self.debug_counters.is_enabled() {
140-
self.debug_counters.add_counter(&expression, (debug_block_label_fn)());
141-
}
142-
expression
113+
BcbCounter::Expression { id, lhs, op, rhs }
143114
}
144115

145116
pub fn make_identity_counter(&mut self, counter_operand: Operand) -> BcbCounter {
146-
let some_debug_block_label = if self.debug_counters.is_enabled() {
147-
self.debug_counters.some_block_label(counter_operand).cloned()
148-
} else {
149-
None
150-
};
151-
self.make_expression(counter_operand, Op::Add, Operand::Zero, || {
152-
some_debug_block_label.clone()
153-
})
117+
self.make_expression(counter_operand, Op::Add, Operand::Zero)
154118
}
155119

156120
/// Counter IDs start from one and go up.
@@ -367,12 +331,8 @@ impl<'a> MakeBcbCounters<'a> {
367331
branch_counter_operand,
368332
Op::Add,
369333
sumup_counter_operand,
370-
|| None,
371-
);
372-
debug!(
373-
" [new intermediate expression: {}]",
374-
self.format_counter(&intermediate_expression)
375334
);
335+
debug!(" [new intermediate expression: {:?}]", intermediate_expression);
376336
let intermediate_expression_operand = intermediate_expression.as_operand();
377337
self.coverage_counters.intermediate_expressions.push(intermediate_expression);
378338
some_sumup_counter_operand.replace(intermediate_expression_operand);
@@ -394,9 +354,8 @@ impl<'a> MakeBcbCounters<'a> {
394354
branching_counter_operand,
395355
Op::Subtract,
396356
sumup_counter_operand,
397-
|| Some(format!("{expression_branch:?}")),
398357
);
399-
debug!("{:?} gets an expression: {}", expression_branch, self.format_counter(&expression));
358+
debug!("{:?} gets an expression: {:?}", expression_branch, expression);
400359
let bcb = expression_branch.target_bcb;
401360
if expression_branch.is_only_path_to_target() {
402361
self.coverage_counters.set_bcb_counter(bcb, expression)?;
@@ -418,10 +377,10 @@ impl<'a> MakeBcbCounters<'a> {
418377
// If the BCB already has a counter, return it.
419378
if let Some(counter_kind) = &self.coverage_counters.bcb_counters[bcb] {
420379
debug!(
421-
"{}{:?} already has a counter: {}",
380+
"{}{:?} already has a counter: {:?}",
422381
NESTED_INDENT.repeat(debug_indent_level),
423382
bcb,
424-
self.format_counter(counter_kind),
383+
counter_kind,
425384
);
426385
return Ok(counter_kind.as_operand());
427386
}
@@ -431,22 +390,22 @@ impl<'a> MakeBcbCounters<'a> {
431390
// program results in a tight infinite loop, but it should still compile.
432391
let one_path_to_target = self.bcb_has_one_path_to_target(bcb);
433392
if one_path_to_target || self.bcb_predecessors(bcb).contains(&bcb) {
434-
let counter_kind = self.coverage_counters.make_counter(|| Some(format!("{bcb:?}")));
393+
let counter_kind = self.coverage_counters.make_counter();
435394
if one_path_to_target {
436395
debug!(
437-
"{}{:?} gets a new counter: {}",
396+
"{}{:?} gets a new counter: {:?}",
438397
NESTED_INDENT.repeat(debug_indent_level),
439398
bcb,
440-
self.format_counter(&counter_kind),
399+
counter_kind,
441400
);
442401
} else {
443402
debug!(
444403
"{}{:?} has itself as its own predecessor. It can't be part of its own \
445-
Expression sum, so it will get its own new counter: {}. (Note, the compiled \
404+
Expression sum, so it will get its own new counter: {:?}. (Note, the compiled \
446405
code will generate an infinite loop.)",
447406
NESTED_INDENT.repeat(debug_indent_level),
448407
bcb,
449-
self.format_counter(&counter_kind),
408+
counter_kind,
450409
);
451410
}
452411
return self.coverage_counters.set_bcb_counter(bcb, counter_kind);
@@ -481,12 +440,11 @@ impl<'a> MakeBcbCounters<'a> {
481440
sumup_edge_counter_operand,
482441
Op::Add,
483442
edge_counter_operand,
484-
|| None,
485443
);
486444
debug!(
487-
"{}new intermediate expression: {}",
445+
"{}new intermediate expression: {:?}",
488446
NESTED_INDENT.repeat(debug_indent_level),
489-
self.format_counter(&intermediate_expression)
447+
intermediate_expression
490448
);
491449
let intermediate_expression_operand = intermediate_expression.as_operand();
492450
self.coverage_counters.intermediate_expressions.push(intermediate_expression);
@@ -497,13 +455,12 @@ impl<'a> MakeBcbCounters<'a> {
497455
first_edge_counter_operand,
498456
Op::Add,
499457
some_sumup_edge_counter_operand.unwrap(),
500-
|| Some(format!("{bcb:?}")),
501458
);
502459
debug!(
503-
"{}{:?} gets a new counter (sum of predecessor counters): {}",
460+
"{}{:?} gets a new counter (sum of predecessor counters): {:?}",
504461
NESTED_INDENT.repeat(debug_indent_level),
505462
bcb,
506-
self.format_counter(&counter_kind)
463+
counter_kind
507464
);
508465
self.coverage_counters.set_bcb_counter(bcb, counter_kind)
509466
}
@@ -534,24 +491,23 @@ impl<'a> MakeBcbCounters<'a> {
534491
self.coverage_counters.bcb_edge_counters.get(&(from_bcb, to_bcb))
535492
{
536493
debug!(
537-
"{}Edge {:?}->{:?} already has a counter: {}",
494+
"{}Edge {:?}->{:?} already has a counter: {:?}",
538495
NESTED_INDENT.repeat(debug_indent_level),
539496
from_bcb,
540497
to_bcb,
541-
self.format_counter(counter_kind)
498+
counter_kind
542499
);
543500
return Ok(counter_kind.as_operand());
544501
}
545502

546503
// Make a new counter to count this edge.
547-
let counter_kind =
548-
self.coverage_counters.make_counter(|| Some(format!("{from_bcb:?}->{to_bcb:?}")));
504+
let counter_kind = self.coverage_counters.make_counter();
549505
debug!(
550-
"{}Edge {:?}->{:?} gets a new counter: {}",
506+
"{}Edge {:?}->{:?} gets a new counter: {:?}",
551507
NESTED_INDENT.repeat(debug_indent_level),
552508
from_bcb,
553509
to_bcb,
554-
self.format_counter(&counter_kind)
510+
counter_kind
555511
);
556512
self.coverage_counters.set_bcb_edge_counter(from_bcb, to_bcb, counter_kind)
557513
}
@@ -710,9 +666,4 @@ impl<'a> MakeBcbCounters<'a> {
710666
fn bcb_dominates(&self, dom: BasicCoverageBlock, node: BasicCoverageBlock) -> bool {
711667
self.basic_coverage_blocks.dominates(dom, node)
712668
}
713-
714-
#[inline]
715-
fn format_counter(&self, counter_kind: &BcbCounter) -> String {
716-
self.coverage_counters.debug_counters.format_counter(counter_kind)
717-
}
718669
}

0 commit comments

Comments
 (0)