mikros/rust
1
0
Fork 0
Code Issues Pull Requests Packages Projects Releases Wiki Activity

Rollup merge of #115867 - Zalathar:debug, r=oli-obk

Browse Source 
coverage: Simplify internal representation of debug types

Most of these debug helper types store each of their fields as `Option<T>`, and then set them to `Some` when the relevant debug checks are enabled. This makes the struct fields awkward to read and results in some contortions when accessing the field values.

This PR addresses those problems by changing each of the helper types to have a single `state: Option<FooState>` field. Each individual method can then obtain the state up-front (or return early if it is absent), allowing the rest of the code to just access the state's contents directly.

---

There are some more improvements I'd like to make to the debug code, but for this PR I'm focusing on a straightforward mechanical change that should be fairly easy to review.

(I did thrown in a few trivial changes to imports and docs, along with one switch from `FxHashMap` to `FxHashSet`.)

---

Most of the changed lines are just indentation churn, so ignoring whitespace is recommended.
This commit is contained in:
Matthias Krüger 2023-09-16 15:18:23 +02:00 committed by GitHub
commit 77e0102d19
No known key found for this signature in database
GPG Key ID: 4AEE18F83AFDEB23
1 changed files with 142 additions and 147 deletions
Show Stats Download Patch File Download Diff File Expand all files Collapse all files

201
compiler/rustc_mir_transform/src/coverage/debug.rs
View File

@ -44,7 +44,7 @@
//! points, which can be enabled via environment variable: //! points, which can be enabled via environment variable:
//! //!
//! ```shell //! ```shell
//! RUSTC_LOG=rustc_mir_transform::transform::coverage=debug //! RUSTC_LOG=rustc_mir_transform::coverage=debug
//! ``` //! ```
//! //!
//! Other module paths with coverage-related debug logs may also be of interest, particularly for //! Other module paths with coverage-related debug logs may also be of interest, particularly for
@ -52,7 +52,7 @@
//! code generation pass). For example: //! code generation pass). For example:
//! //!
//! ```shell //! ```shell
//! RUSTC_LOG=rustc_mir_transform::transform::coverage,rustc_codegen_ssa::coverageinfo,rustc_codegen_llvm::coverageinfo=debug //! RUSTC_LOG=rustc_mir_transform::coverage,rustc_codegen_llvm::coverageinfo=debug
//! ``` //! ```
//! //!
//! Coverage Debug Options //! Coverage Debug Options
@ -108,24 +108,23 @@
//! recursively, generating labels with nested operations, enclosed in parentheses //! recursively, generating labels with nested operations, enclosed in parentheses
//! (for example: `bcb2 + (bcb0 - bcb1)`). //! (for example: `bcb2 + (bcb0 - bcb1)`).
use super::counters::{BcbCounter, CoverageCounters}; use std::iter;
use super::graph::{BasicCoverageBlock, BasicCoverageBlockData, CoverageGraph}; use std::ops::Deref;
use super::spans::CoverageSpan; use std::sync::OnceLock;
use itertools::Itertools; use itertools::Itertools;
use rustc_data_structures::fx::{FxHashMap, FxHashSet};
use rustc_middle::mir::coverage::*;
use rustc_middle::mir::create_dump_file; use rustc_middle::mir::create_dump_file;
use rustc_middle::mir::generic_graphviz::GraphvizWriter; use rustc_middle::mir::generic_graphviz::GraphvizWriter;
use rustc_middle::mir::spanview::{self, SpanViewable}; use rustc_middle::mir::spanview::{self, SpanViewable};
use rustc_data_structures::fx::FxHashMap;
use rustc_middle::mir::coverage::*;
use rustc_middle::mir::{self, BasicBlock}; use rustc_middle::mir::{self, BasicBlock};
use rustc_middle::ty::TyCtxt; use rustc_middle::ty::TyCtxt;
use rustc_span::Span; use rustc_span::Span;
use std::iter; use super::counters::{BcbCounter, CoverageCounters};
use std::ops::Deref; use super::graph::{BasicCoverageBlock, BasicCoverageBlockData, CoverageGraph};
use std::sync::OnceLock; use super::spans::CoverageSpan;
pub const NESTED_INDENT: &str = " "; pub const NESTED_INDENT: &str = " ";
@ -259,36 +258,42 @@ impl Default for ExpressionFormat {
/// `DebugCounters` supports a recursive rendering of `Expression` counters, so they can be /// `DebugCounters` supports a recursive rendering of `Expression` counters, so they can be
/// presented as nested expressions such as `(bcb3 - (bcb0 + bcb1))`. /// presented as nested expressions such as `(bcb3 - (bcb0 + bcb1))`.
pub(super) struct DebugCounters { pub(super) struct DebugCounters {
some_counters: Option<FxHashMap<Operand, DebugCounter>>, state: Option<DebugCountersState>,
}
#[derive(Default)]
struct DebugCountersState {
counters: FxHashMap<Operand, DebugCounter>,
} }
impl DebugCounters { impl DebugCounters {
pub fn new() -> Self { pub fn new() -> Self {
Self { some_counters: None } Self { state: None }
} }
pub fn enable(&mut self) { pub fn enable(&mut self) {
debug_assert!(!self.is_enabled()); debug_assert!(!self.is_enabled());
self.some_counters.replace(FxHashMap::default()); self.state = Some(DebugCountersState::default());
} }
pub fn is_enabled(&self) -> bool { pub fn is_enabled(&self) -> bool {
self.some_counters.is_some() self.state.is_some()
} }
pub fn add_counter(&mut self, counter_kind: &BcbCounter, some_block_label: Option<String>) { pub fn add_counter(&mut self, counter_kind: &BcbCounter, some_block_label: Option<String>) {
if let Some(counters) = &mut self.some_counters { let Some(state) = &mut self.state else { return };
let id = counter_kind.as_operand(); let id = counter_kind.as_operand();
counters state
.counters
.try_insert(id, DebugCounter::new(counter_kind.clone(), some_block_label)) .try_insert(id, DebugCounter::new(counter_kind.clone(), some_block_label))
.expect("attempt to add the same counter_kind to DebugCounters more than once"); .expect("attempt to add the same counter_kind to DebugCounters more than once");
} }
}
pub fn some_block_label(&self, operand: Operand) -> Option<&String> { pub fn some_block_label(&self, operand: Operand) -> Option<&String> {
self.some_counters.as_ref().and_then(|counters| { let Some(state) = &self.state else { return None };
counters.get(&operand).and_then(|debug_counter| debug_counter.some_block_label.as_ref())
}) state.counters.get(&operand)?.some_block_label.as_ref()
} }
pub fn format_counter(&self, counter_kind: &BcbCounter) -> String { pub fn format_counter(&self, counter_kind: &BcbCounter) -> String {
@ -308,7 +313,7 @@ impl DebugCounters {
if counter_format.operation { if counter_format.operation {
return format!( return format!(
"{}{} {} {}", "{}{} {} {}",
if counter_format.id || self.some_counters.is_none() { if counter_format.id || !self.is_enabled() {
format!("#{} = ", id.index()) format!("#{} = ", id.index())
} else { } else {
String::new() String::new()
@ -324,10 +329,9 @@ impl DebugCounters {
} }
let id = counter_kind.as_operand(); let id = counter_kind.as_operand();
if self.some_counters.is_some() && (counter_format.block || !counter_format.id) { if let Some(state) = &self.state && (counter_format.block || !counter_format.id) {
let counters = self.some_counters.as_ref().unwrap();
if let Some(DebugCounter { some_block_label: Some(block_label), .. }) = if let Some(DebugCounter { some_block_label: Some(block_label), .. }) =
counters.get(&id) state.counters.get(&id)
{ {
return if counter_format.id { return if counter_format.id {
format!("{}#{:?}", block_label, id) format!("{}#{:?}", block_label, id)
@ -343,8 +347,10 @@ impl DebugCounters {
if matches!(operand, Operand::Zero) { if matches!(operand, Operand::Zero) {
return String::from("0"); return String::from("0");
} }
if let Some(counters) = &self.some_counters { if let Some(state) = &self.state {
if let Some(DebugCounter { counter_kind, some_block_label }) = counters.get(&operand) { if let Some(DebugCounter { counter_kind, some_block_label }) =
state.counters.get(&operand)
{
if let BcbCounter::Expression { .. } = counter_kind { if let BcbCounter::Expression { .. } = counter_kind {
if let Some(label) = some_block_label && debug_options().counter_format.block { if let Some(label) = some_block_label && debug_options().counter_format.block {
return format!( return format!(
@ -378,30 +384,29 @@ impl DebugCounter {
/// If enabled, this data structure captures additional debugging information used when generating /// If enabled, this data structure captures additional debugging information used when generating
/// a Graphviz (.dot file) representation of the `CoverageGraph`, for debugging purposes. /// a Graphviz (.dot file) representation of the `CoverageGraph`, for debugging purposes.
pub(super) struct GraphvizData { pub(super) struct GraphvizData {
some_bcb_to_coverage_spans_with_counters: state: Option<GraphvizDataState>,
Option<FxHashMap<BasicCoverageBlock, Vec<(CoverageSpan, BcbCounter)>>>, }
some_bcb_to_dependency_counters: Option<FxHashMap<BasicCoverageBlock, Vec<BcbCounter>>>,
some_edge_to_counter: Option<FxHashMap<(BasicCoverageBlock, BasicBlock), BcbCounter>>, #[derive(Default)]
struct GraphvizDataState {
bcb_to_coverage_spans_with_counters:
FxHashMap<BasicCoverageBlock, Vec<(CoverageSpan, BcbCounter)>>,
bcb_to_dependency_counters: FxHashMap<BasicCoverageBlock, Vec<BcbCounter>>,
edge_to_counter: FxHashMap<(BasicCoverageBlock, BasicBlock), BcbCounter>,
} }
impl GraphvizData { impl GraphvizData {
pub fn new() -> Self { pub fn new() -> Self {
Self { Self { state: None }
some_bcb_to_coverage_spans_with_counters: None,
some_bcb_to_dependency_counters: None,
some_edge_to_counter: None,
}
} }
pub fn enable(&mut self) { pub fn enable(&mut self) {
debug_assert!(!self.is_enabled()); debug_assert!(!self.is_enabled());
self.some_bcb_to_coverage_spans_with_counters = Some(FxHashMap::default()); self.state = Some(GraphvizDataState::default());
self.some_bcb_to_dependency_counters = Some(FxHashMap::default());
self.some_edge_to_counter = Some(FxHashMap::default());
} }
pub fn is_enabled(&self) -> bool { pub fn is_enabled(&self) -> bool {
self.some_bcb_to_coverage_spans_with_counters.is_some() self.state.is_some()
} }
pub fn add_bcb_coverage_span_with_counter( pub fn add_bcb_coverage_span_with_counter(
@ -410,27 +415,22 @@ impl GraphvizData {
coverage_span: &CoverageSpan, coverage_span: &CoverageSpan,
counter_kind: &BcbCounter, counter_kind: &BcbCounter,
) { ) {
if let Some(bcb_to_coverage_spans_with_counters) = let Some(state) = &mut self.state else { return };
self.some_bcb_to_coverage_spans_with_counters.as_mut()
{ state
bcb_to_coverage_spans_with_counters .bcb_to_coverage_spans_with_counters
.entry(bcb) .entry(bcb)
.or_insert_with(Vec::new) .or_insert_with(Vec::new)
.push((coverage_span.clone(), counter_kind.clone())); .push((coverage_span.clone(), counter_kind.clone()));
} }
}
pub fn get_bcb_coverage_spans_with_counters( pub fn get_bcb_coverage_spans_with_counters(
&self, &self,
bcb: BasicCoverageBlock, bcb: BasicCoverageBlock,
) -> Option<&[(CoverageSpan, BcbCounter)]> { ) -> Option<&[(CoverageSpan, BcbCounter)]> {
if let Some(bcb_to_coverage_spans_with_counters) = let Some(state) = &self.state else { return None };
self.some_bcb_to_coverage_spans_with_counters.as_ref()
{ state.bcb_to_coverage_spans_with_counters.get(&bcb).map(Deref::deref)
bcb_to_coverage_spans_with_counters.get(&bcb).map(Deref::deref)
} else {
None
}
} }
pub fn add_bcb_dependency_counter( pub fn add_bcb_dependency_counter(
@ -438,20 +438,19 @@ impl GraphvizData {
bcb: BasicCoverageBlock, bcb: BasicCoverageBlock,
counter_kind: &BcbCounter, counter_kind: &BcbCounter,
) { ) {
if let Some(bcb_to_dependency_counters) = self.some_bcb_to_dependency_counters.as_mut() { let Some(state) = &mut self.state else { return };
bcb_to_dependency_counters
state
.bcb_to_dependency_counters
.entry(bcb) .entry(bcb)
.or_insert_with(Vec::new) .or_insert_with(Vec::new)
.push(counter_kind.clone()); .push(counter_kind.clone());
} }
}
pub fn get_bcb_dependency_counters(&self, bcb: BasicCoverageBlock) -> Option<&[BcbCounter]> { pub fn get_bcb_dependency_counters(&self, bcb: BasicCoverageBlock) -> Option<&[BcbCounter]> {
if let Some(bcb_to_dependency_counters) = self.some_bcb_to_dependency_counters.as_ref() { let Some(state) = &self.state else { return None };
bcb_to_dependency_counters.get(&bcb).map(Deref::deref)
} else { state.bcb_to_dependency_counters.get(&bcb).map(Deref::deref)
None
}
} }
pub fn set_edge_counter( pub fn set_edge_counter(
@ -460,23 +459,22 @@ impl GraphvizData {
to_bb: BasicBlock, to_bb: BasicBlock,
counter_kind: &BcbCounter, counter_kind: &BcbCounter,
) { ) {
if let Some(edge_to_counter) = self.some_edge_to_counter.as_mut() { let Some(state) = &mut self.state else { return };
edge_to_counter
state
.edge_to_counter
.try_insert((from_bcb, to_bb), counter_kind.clone()) .try_insert((from_bcb, to_bb), counter_kind.clone())
.expect("invalid attempt to insert more than one edge counter for the same edge"); .expect("invalid attempt to insert more than one edge counter for the same edge");
} }
}
pub fn get_edge_counter( pub fn get_edge_counter(
&self, &self,
from_bcb: BasicCoverageBlock, from_bcb: BasicCoverageBlock,
to_bb: BasicBlock, to_bb: BasicBlock,
) -> Option<&BcbCounter> { ) -> Option<&BcbCounter> {
if let Some(edge_to_counter) = self.some_edge_to_counter.as_ref() { let Some(state) = &self.state else { return None };
edge_to_counter.get(&(from_bcb, to_bb))
} else { state.edge_to_counter.get(&(from_bcb, to_bb))
None
}
} }
} }
@ -485,41 +483,42 @@ impl GraphvizData {
/// _not_ used are retained in the `unused_expressions` Vec, to be included in debug output (logs /// _not_ used are retained in the `unused_expressions` Vec, to be included in debug output (logs
/// and/or a `CoverageGraph` graphviz output). /// and/or a `CoverageGraph` graphviz output).
pub(super) struct UsedExpressions { pub(super) struct UsedExpressions {
some_used_expression_operands: Option<FxHashMap<Operand, Vec<ExpressionId>>>, state: Option<UsedExpressionsState>,
some_unused_expressions: }
Option<Vec<(BcbCounter, Option<BasicCoverageBlock>, BasicCoverageBlock)>>,
#[derive(Default)]
struct UsedExpressionsState {
used_expression_operands: FxHashSet<Operand>,
unused_expressions: Vec<(BcbCounter, Option<BasicCoverageBlock>, BasicCoverageBlock)>,
} }
impl UsedExpressions { impl UsedExpressions {
pub fn new() -> Self { pub fn new() -> Self {
Self { some_used_expression_operands: None, some_unused_expressions: None } Self { state: None }
} }
pub fn enable(&mut self) { pub fn enable(&mut self) {
debug_assert!(!self.is_enabled()); debug_assert!(!self.is_enabled());
self.some_used_expression_operands = Some(FxHashMap::default()); self.state = Some(UsedExpressionsState::default())
self.some_unused_expressions = Some(Vec::new());
} }
pub fn is_enabled(&self) -> bool { pub fn is_enabled(&self) -> bool {
self.some_used_expression_operands.is_some() self.state.is_some()
} }
pub fn add_expression_operands(&mut self, expression: &BcbCounter) { pub fn add_expression_operands(&mut self, expression: &BcbCounter) {
if let Some(used_expression_operands) = self.some_used_expression_operands.as_mut() { let Some(state) = &mut self.state else { return };
if let BcbCounter::Expression { id, lhs, rhs, .. } = *expression {
used_expression_operands.entry(lhs).or_insert_with(Vec::new).push(id); if let BcbCounter::Expression { lhs, rhs, .. } = *expression {
used_expression_operands.entry(rhs).or_insert_with(Vec::new).push(id); state.used_expression_operands.insert(lhs);
} state.used_expression_operands.insert(rhs);
} }
} }
pub fn expression_is_used(&self, expression: &BcbCounter) -> bool { pub fn expression_is_used(&self, expression: &BcbCounter) -> bool {
if let Some(used_expression_operands) = self.some_used_expression_operands.as_ref() { let Some(state) = &self.state else { return false };
used_expression_operands.contains_key(&expression.as_operand())
} else { state.used_expression_operands.contains(&expression.as_operand())
false
}
} }
pub fn add_unused_expression_if_not_found( pub fn add_unused_expression_if_not_found(
@ -528,14 +527,10 @@ impl UsedExpressions {
edge_from_bcb: Option<BasicCoverageBlock>, edge_from_bcb: Option<BasicCoverageBlock>,
target_bcb: BasicCoverageBlock, target_bcb: BasicCoverageBlock,
) { ) {
if let Some(used_expression_operands) = self.some_used_expression_operands.as_ref() { let Some(state) = &mut self.state else { return };
if !used_expression_operands.contains_key(&expression.as_operand()) {
self.some_unused_expressions.as_mut().unwrap().push(( if !state.used_expression_operands.contains(&expression.as_operand()) {
expression.clone(), state.unused_expressions.push((expression.clone(), edge_from_bcb, target_bcb));
edge_from_bcb,
target_bcb,
));
}
} }
} }
@ -544,11 +539,9 @@ impl UsedExpressions {
pub fn get_unused_expressions( pub fn get_unused_expressions(
&self, &self,
) -> Vec<(BcbCounter, Option<BasicCoverageBlock>, BasicCoverageBlock)> { ) -> Vec<(BcbCounter, Option<BasicCoverageBlock>, BasicCoverageBlock)> {
if let Some(unused_expressions) = self.some_unused_expressions.as_ref() { let Some(state) = &self.state else { return Vec::new() };
unused_expressions.clone()
} else { state.unused_expressions.clone()
Vec::new()
}
} }
/// If enabled, validate that every BCB or edge counter not directly associated with a coverage /// If enabled, validate that every BCB or edge counter not directly associated with a coverage
@ -562,7 +555,10 @@ impl UsedExpressions {
BcbCounter, BcbCounter,
)], )],
) { ) {
if self.is_enabled() { if !self.is_enabled() {
return;
}
let mut not_validated = bcb_counters_without_direct_coverage_spans let mut not_validated = bcb_counters_without_direct_coverage_spans
.iter() .iter()
.map(|(_, _, counter_kind)| counter_kind) .map(|(_, _, counter_kind)| counter_kind)
@ -580,11 +576,11 @@ impl UsedExpressions {
} }
} }
} }
}
pub fn alert_on_unused_expressions(&self, debug_counters: &DebugCounters) { pub fn alert_on_unused_expressions(&self, debug_counters: &DebugCounters) {
if let Some(unused_expressions) = self.some_unused_expressions.as_ref() { let Some(state) = &self.state else { return };
for (counter_kind, edge_from_bcb, target_bcb) in unused_expressions {
for (counter_kind, edge_from_bcb, target_bcb) in &state.unused_expressions {
let unused_counter_message = if let Some(from_bcb) = edge_from_bcb.as_ref() { let unused_counter_message = if let Some(from_bcb) = edge_from_bcb.as_ref() {
format!( format!(
"non-coverage edge counter found without a dependent expression, in \ "non-coverage edge counter found without a dependent expression, in \
@ -610,7 +606,6 @@ impl UsedExpressions {
} }
} }
} }
}
/// Generates the MIR pass `CoverageSpan`-specific spanview dump file. /// Generates the MIR pass `CoverageSpan`-specific spanview dump file.
pub(super) fn dump_coverage_spanview<'tcx>( pub(super) fn dump_coverage_spanview<'tcx>(