Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Unbox and unwrap the contents of StatementKind::Coverage #122937

Merged
merged 1 commit into from
Mar 24, 2024
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
4 changes: 2 additions & 2 deletions compiler/rustc_codegen_gcc/src/coverageinfo.rs
Original file line number Diff line number Diff line change
@@ -1,11 +1,11 @@
use rustc_codegen_ssa::traits::CoverageInfoBuilderMethods;
use rustc_middle::mir::Coverage;
use rustc_middle::mir::coverage::CoverageKind;
use rustc_middle::ty::Instance;

use crate::builder::Builder;

impl<'a, 'gcc, 'tcx> CoverageInfoBuilderMethods<'tcx> for Builder<'a, 'gcc, 'tcx> {
fn add_coverage(&mut self, _instance: Instance<'tcx>, _coverage: &Coverage) {
fn add_coverage(&mut self, _instance: Instance<'tcx>, _kind: &CoverageKind) {
// TODO(antoyo)
}
}
4 changes: 1 addition & 3 deletions compiler/rustc_codegen_llvm/src/coverageinfo/mod.rs
Original file line number Diff line number Diff line change
Expand Up @@ -14,7 +14,6 @@ use rustc_data_structures::fx::{FxHashMap, FxIndexMap};
use rustc_llvm::RustString;
use rustc_middle::bug;
use rustc_middle::mir::coverage::CoverageKind;
use rustc_middle::mir::Coverage;
use rustc_middle::ty::layout::HasTyCtxt;
use rustc_middle::ty::Instance;

Expand Down Expand Up @@ -75,7 +74,7 @@ impl<'ll, 'tcx> CodegenCx<'ll, 'tcx> {

impl<'tcx> CoverageInfoBuilderMethods<'tcx> for Builder<'_, '_, 'tcx> {
#[instrument(level = "debug", skip(self))]
fn add_coverage(&mut self, instance: Instance<'tcx>, coverage: &Coverage) {
fn add_coverage(&mut self, instance: Instance<'tcx>, kind: &CoverageKind) {
// Our caller should have already taken care of inlining subtleties,
// so we can assume that counter/expression IDs in this coverage
// statement are meaningful for the given instance.
Expand All @@ -98,7 +97,6 @@ impl<'tcx> CoverageInfoBuilderMethods<'tcx> for Builder<'_, '_, 'tcx> {
.entry(instance)
.or_insert_with(|| FunctionCoverageCollector::new(instance, function_coverage_info));

let Coverage { kind } = coverage;
match *kind {
CoverageKind::SpanMarker | CoverageKind::BlockMarker { .. } => unreachable!(
"marker statement {kind:?} should have been removed by CleanupPostBorrowck"
Expand Down
6 changes: 3 additions & 3 deletions compiler/rustc_codegen_ssa/src/mir/coverageinfo.rs
Original file line number Diff line number Diff line change
@@ -1,12 +1,12 @@
use crate::traits::*;

use rustc_middle::mir::Coverage;
use rustc_middle::mir::coverage::CoverageKind;
use rustc_middle::mir::SourceScope;

use super::FunctionCx;

impl<'a, 'tcx, Bx: BuilderMethods<'a, 'tcx>> FunctionCx<'a, 'tcx, Bx> {
pub fn codegen_coverage(&self, bx: &mut Bx, coverage: &Coverage, scope: SourceScope) {
pub fn codegen_coverage(&self, bx: &mut Bx, kind: &CoverageKind, scope: SourceScope) {
// Determine the instance that coverage data was originally generated for.
let instance = if let Some(inlined) = scope.inlined_instance(&self.mir.source_scopes) {
self.monomorphize(inlined)
Expand All @@ -15,6 +15,6 @@ impl<'a, 'tcx, Bx: BuilderMethods<'a, 'tcx>> FunctionCx<'a, 'tcx, Bx> {
};

// Handle the coverage info in a backend-specific way.
bx.add_coverage(instance, coverage);
bx.add_coverage(instance, kind);
}
}
4 changes: 2 additions & 2 deletions compiler/rustc_codegen_ssa/src/mir/statement.rs
Original file line number Diff line number Diff line change
Expand Up @@ -64,8 +64,8 @@ impl<'a, 'tcx, Bx: BuilderMethods<'a, 'tcx>> FunctionCx<'a, 'tcx, Bx> {
cg_indirect_place.storage_dead(bx);
}
}
mir::StatementKind::Coverage(box ref coverage) => {
self.codegen_coverage(bx, coverage, statement.source_info.scope);
mir::StatementKind::Coverage(ref kind) => {
self.codegen_coverage(bx, kind, statement.source_info.scope);
}
mir::StatementKind::Intrinsic(box NonDivergingIntrinsic::Assume(ref op)) => {
if !matches!(bx.tcx().sess.opts.optimize, OptLevel::No | OptLevel::Less) {
Expand Down
4 changes: 2 additions & 2 deletions compiler/rustc_codegen_ssa/src/traits/coverageinfo.rs
Original file line number Diff line number Diff line change
@@ -1,11 +1,11 @@
use super::BackendTypes;
use rustc_middle::mir::Coverage;
use rustc_middle::mir::coverage::CoverageKind;
use rustc_middle::ty::Instance;

pub trait CoverageInfoBuilderMethods<'tcx>: BackendTypes {
/// Handle the MIR coverage info in a backend-specific way.
///
/// This can potentially be a no-op in backends that don't support
/// coverage instrumentation.
fn add_coverage(&mut self, instance: Instance<'tcx>, coverage: &Coverage);
fn add_coverage(&mut self, instance: Instance<'tcx>, kind: &CoverageKind);
}
3 changes: 1 addition & 2 deletions compiler/rustc_const_eval/src/transform/validate.rs
Original file line number Diff line number Diff line change
Expand Up @@ -346,8 +346,7 @@ impl<'a, 'tcx> Visitor<'tcx> for CfgChecker<'a, 'tcx> {
self.fail(location, format!("explicit `{kind:?}` is forbidden"));
}
}
StatementKind::Coverage(coverage) => {
let kind = &coverage.kind;
StatementKind::Coverage(kind) => {
if self.mir_phase >= MirPhase::Analysis(AnalysisPhase::PostCleanup)
&& let CoverageKind::BlockMarker { .. } | CoverageKind::SpanMarker { .. } = kind
{
Expand Down
4 changes: 2 additions & 2 deletions compiler/rustc_middle/src/mir/pretty.rs
Original file line number Diff line number Diff line change
Expand Up @@ -13,7 +13,7 @@ use rustc_middle::mir::interpret::{
Provenance,
};
use rustc_middle::mir::visit::Visitor;
use rustc_middle::mir::{self, *};
use rustc_middle::mir::*;
use rustc_target::abi::Size;

const INDENT: &str = " ";
Expand Down Expand Up @@ -711,7 +711,7 @@ impl Debug for Statement<'_> {
AscribeUserType(box (ref place, ref c_ty), ref variance) => {
write!(fmt, "AscribeUserType({place:?}, {variance:?}, {c_ty:?})")
}
Coverage(box mir::Coverage { ref kind }) => write!(fmt, "Coverage::{kind:?}"),
Coverage(ref kind) => write!(fmt, "Coverage::{kind:?}"),
Intrinsic(box ref intrinsic) => write!(fmt, "{intrinsic}"),
ConstEvalCounter => write!(fmt, "ConstEvalCounter"),
Nop => write!(fmt, "nop"),
Expand Down
9 changes: 2 additions & 7 deletions compiler/rustc_middle/src/mir/syntax.rs
Original file line number Diff line number Diff line change
Expand Up @@ -373,7 +373,7 @@ pub enum StatementKind<'tcx> {
///
/// Interpreters and codegen backends that don't support coverage instrumentation
/// can usually treat this as a no-op.
Coverage(Box<Coverage>),
Coverage(CoverageKind),

/// Denotes a call to an intrinsic that does not require an unwind path and always returns.
/// This avoids adding a new block and a terminator for simple intrinsics.
Expand Down Expand Up @@ -517,12 +517,6 @@ pub enum FakeReadCause {
ForIndex,
}

#[derive(Clone, Debug, PartialEq, TyEncodable, TyDecodable, Hash, HashStable)]
#[derive(TypeFoldable, TypeVisitable)]
pub struct Coverage {
pub kind: CoverageKind,
}

#[derive(Clone, Debug, PartialEq, TyEncodable, TyDecodable, Hash, HashStable)]
#[derive(TypeFoldable, TypeVisitable)]
pub struct CopyNonOverlapping<'tcx> {
Expand Down Expand Up @@ -1465,5 +1459,6 @@ mod size_asserts {
static_assert_size!(Place<'_>, 16);
static_assert_size!(PlaceElem<'_>, 24);
static_assert_size!(Rvalue<'_>, 40);
static_assert_size!(StatementKind<'_>, 16);
// tidy-alphabetical-end
}
6 changes: 3 additions & 3 deletions compiler/rustc_middle/src/mir/visit.rs
Original file line number Diff line number Diff line change
Expand Up @@ -156,10 +156,10 @@ macro_rules! make_mir_visitor {

fn visit_coverage(
&mut self,
coverage: & $($mutability)? Coverage,
kind: & $($mutability)? coverage::CoverageKind,
location: Location,
) {
self.super_coverage(coverage, location);
self.super_coverage(kind, location);
}

fn visit_retag(
Expand Down Expand Up @@ -803,7 +803,7 @@ macro_rules! make_mir_visitor {
}

fn super_coverage(&mut self,
_coverage: & $($mutability)? Coverage,
_kind: & $($mutability)? coverage::CoverageKind,
_location: Location) {
}

Expand Down
4 changes: 1 addition & 3 deletions compiler/rustc_mir_build/src/build/cfg.rs
Original file line number Diff line number Diff line change
Expand Up @@ -107,9 +107,7 @@ impl<'tcx> CFG<'tcx> {
/// This results in more accurate coverage reports for certain kinds of
/// syntax (e.g. `continue` or `if !`) that would otherwise not appear in MIR.
pub(crate) fn push_coverage_span_marker(&mut self, block: BasicBlock, source_info: SourceInfo) {
let kind = StatementKind::Coverage(Box::new(Coverage {
kind: coverage::CoverageKind::SpanMarker,
}));
let kind = StatementKind::Coverage(coverage::CoverageKind::SpanMarker);
let stmt = Statement { source_info, kind };
self.push(block, stmt);
}
Expand Down
4 changes: 1 addition & 3 deletions compiler/rustc_mir_build/src/build/coverageinfo.rs
Original file line number Diff line number Diff line change
Expand Up @@ -127,9 +127,7 @@ impl Builder<'_, '_> {

let marker_statement = mir::Statement {
source_info,
kind: mir::StatementKind::Coverage(Box::new(mir::Coverage {
kind: CoverageKind::BlockMarker { id },
})),
kind: mir::StatementKind::Coverage(CoverageKind::BlockMarker { id }),
};
self.cfg.push(block, marker_statement);

Expand Down
9 changes: 4 additions & 5 deletions compiler/rustc_mir_transform/src/cleanup_post_borrowck.rs
Original file line number Diff line number Diff line change
Expand Up @@ -18,7 +18,7 @@

use crate::MirPass;
use rustc_middle::mir::coverage::CoverageKind;
use rustc_middle::mir::{Body, BorrowKind, Coverage, Rvalue, StatementKind, TerminatorKind};
use rustc_middle::mir::{Body, BorrowKind, Rvalue, StatementKind, TerminatorKind};
use rustc_middle::ty::TyCtxt;

pub struct CleanupPostBorrowck;
Expand All @@ -30,12 +30,11 @@ impl<'tcx> MirPass<'tcx> for CleanupPostBorrowck {
match statement.kind {
StatementKind::AscribeUserType(..)
| StatementKind::Assign(box (_, Rvalue::Ref(_, BorrowKind::Fake, _)))
| StatementKind::Coverage(box Coverage {
| StatementKind::Coverage(
// These kinds of coverage statements are markers inserted during
// MIR building, and are not needed after InstrumentCoverage.
kind: CoverageKind::BlockMarker { .. } | CoverageKind::SpanMarker { .. },
..
})
CoverageKind::BlockMarker { .. } | CoverageKind::SpanMarker { .. },
)
| StatementKind::FakeRead(..) => statement.make_nop(),
_ => (),
}
Expand Down
7 changes: 2 additions & 5 deletions compiler/rustc_mir_transform/src/coverage/mod.rs
Original file line number Diff line number Diff line change
Expand Up @@ -15,7 +15,7 @@ use crate::MirPass;

use rustc_middle::mir::coverage::*;
use rustc_middle::mir::{
self, BasicBlock, BasicBlockData, Coverage, SourceInfo, Statement, StatementKind, Terminator,
self, BasicBlock, BasicBlockData, SourceInfo, Statement, StatementKind, Terminator,
TerminatorKind,
};
use rustc_middle::ty::TyCtxt;
Expand Down Expand Up @@ -230,10 +230,7 @@ fn inject_statement(mir_body: &mut mir::Body<'_>, counter_kind: CoverageKind, bb
debug!(" injecting statement {counter_kind:?} for {bb:?}");
let data = &mut mir_body[bb];
let source_info = data.terminator().source_info;
let statement = Statement {
source_info,
kind: StatementKind::Coverage(Box::new(Coverage { kind: counter_kind })),
};
let statement = Statement { source_info, kind: StatementKind::Coverage(counter_kind) };
data.statements.insert(0, statement);
}

Expand Down
10 changes: 4 additions & 6 deletions compiler/rustc_mir_transform/src/coverage/query.rs
Original file line number Diff line number Diff line change
@@ -1,7 +1,7 @@
use rustc_data_structures::captures::Captures;
use rustc_middle::middle::codegen_fn_attrs::CodegenFnAttrFlags;
use rustc_middle::mir::coverage::{CounterId, CoverageKind};
use rustc_middle::mir::{Body, Coverage, CoverageIdsInfo, Statement, StatementKind};
use rustc_middle::mir::{Body, CoverageIdsInfo, Statement, StatementKind};
use rustc_middle::query::TyCtxtAt;
use rustc_middle::ty::{self, TyCtxt};
use rustc_middle::util::Providers;
Expand Down Expand Up @@ -54,7 +54,7 @@ fn coverage_ids_info<'tcx>(
let mir_body = tcx.instance_mir(instance_def);

let max_counter_id = all_coverage_in_mir_body(mir_body)
.filter_map(|coverage| match coverage.kind {
.filter_map(|kind| match *kind {
CoverageKind::CounterIncrement { id } => Some(id),
_ => None,
})
Expand All @@ -66,12 +66,10 @@ fn coverage_ids_info<'tcx>(

fn all_coverage_in_mir_body<'a, 'tcx>(
body: &'a Body<'tcx>,
) -> impl Iterator<Item = &'a Coverage> + Captures<'tcx> {
) -> impl Iterator<Item = &'a CoverageKind> + Captures<'tcx> {
body.basic_blocks.iter().flat_map(|bb_data| &bb_data.statements).filter_map(|statement| {
match statement.kind {
StatementKind::Coverage(box ref coverage) if !is_inlined(body, statement) => {
Some(coverage)
}
StatementKind::Coverage(ref kind) if !is_inlined(body, statement) => Some(kind),
_ => None,
}
})
Expand Down
34 changes: 14 additions & 20 deletions compiler/rustc_mir_transform/src/coverage/spans/from_mir.rs
Original file line number Diff line number Diff line change
Expand Up @@ -187,9 +187,7 @@ fn filtered_statement_span(statement: &Statement<'_>) -> Option<Span> {
// for their parent `BasicBlock`.
StatementKind::StorageLive(_)
| StatementKind::StorageDead(_)
// Ignore `ConstEvalCounter`s
| StatementKind::ConstEvalCounter
// Ignore `Nop`s
Comment on lines -190 to -192
Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I noticed that these comments were preventing rustfmt from formatting this match, presumably due to rust-lang/rustfmt#4119.

Since these particular comments are low-value anyway, I worked around the issue by just deleting them.

| StatementKind::Nop => None,

// FIXME(#78546): MIR InstrumentCoverage - Can the source_info.span for `FakeRead`
Expand All @@ -211,30 +209,28 @@ fn filtered_statement_span(statement: &Statement<'_>) -> Option<Span> {
StatementKind::FakeRead(box (FakeReadCause::ForGuardBinding, _)) => None,

// Retain spans from most other statements.
StatementKind::FakeRead(box (_, _)) // Not including `ForGuardBinding`
StatementKind::FakeRead(_)
| StatementKind::Intrinsic(..)
| StatementKind::Coverage(box mir::Coverage {
| StatementKind::Coverage(
// The purpose of `SpanMarker` is to be matched and accepted here.
kind: CoverageKind::SpanMarker
})
CoverageKind::SpanMarker,
)
| StatementKind::Assign(_)
| StatementKind::SetDiscriminant { .. }
| StatementKind::Deinit(..)
| StatementKind::Retag(_, _)
| StatementKind::PlaceMention(..)
| StatementKind::AscribeUserType(_, _) => {
Some(statement.source_info.span)
}
| StatementKind::AscribeUserType(_, _) => Some(statement.source_info.span),

StatementKind::Coverage(box mir::Coverage {
// Block markers are used for branch coverage, so ignore them here.
kind: CoverageKind::BlockMarker {..}
}) => None,
// Block markers are used for branch coverage, so ignore them here.
StatementKind::Coverage(CoverageKind::BlockMarker { .. }) => None,

StatementKind::Coverage(box mir::Coverage {
// These coverage statements should not exist prior to coverage instrumentation.
kind: CoverageKind::CounterIncrement { .. } | CoverageKind::ExpressionUsed { .. }
}) => bug!("Unexpected coverage statement found during coverage instrumentation: {statement:?}"),
// These coverage statements should not exist prior to coverage instrumentation.
StatementKind::Coverage(
CoverageKind::CounterIncrement { .. } | CoverageKind::ExpressionUsed { .. },
) => bug!(
"Unexpected coverage statement found during coverage instrumentation: {statement:?}"
),
}
}

Expand Down Expand Up @@ -382,9 +378,7 @@ pub(super) fn extract_branch_mappings(
// Fill out the mapping from block marker IDs to their enclosing blocks.
for (bb, data) in mir_body.basic_blocks.iter_enumerated() {
for statement in &data.statements {
if let StatementKind::Coverage(coverage) = &statement.kind
&& let CoverageKind::BlockMarker { id } = coverage.kind
{
if let StatementKind::Coverage(CoverageKind::BlockMarker { id }) = statement.kind {
block_markers[id] = Some(bb);
}
}
Expand Down
Loading