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

coverage: Rename CodeRegion to SourceRegion #129686

Merged
merged 3 commits into from
Aug 28, 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
7 changes: 4 additions & 3 deletions compiler/rustc_codegen_llvm/src/coverageinfo/ffi.rs
Original file line number Diff line number Diff line change
@@ -1,5 +1,5 @@
use rustc_middle::mir::coverage::{
CodeRegion, ConditionInfo, CounterId, CovTerm, DecisionInfo, ExpressionId, MappingKind,
ConditionInfo, CounterId, CovTerm, DecisionInfo, ExpressionId, MappingKind, SourceRegion,
};

/// Must match the layout of `LLVMRustCounterKind`.
Expand Down Expand Up @@ -236,9 +236,10 @@ impl CounterMappingRegion {
pub(crate) fn from_mapping(
mapping_kind: &MappingKind,
local_file_id: u32,
code_region: &CodeRegion,
source_region: &SourceRegion,
) -> Self {
let &CodeRegion { file_name: _, start_line, start_col, end_line, end_col } = code_region;
let &SourceRegion { file_name: _, start_line, start_col, end_line, end_col } =
source_region;
match *mapping_kind {
MappingKind::Code(term) => Self::code_region(
Counter::from_term(term),
Expand Down
12 changes: 6 additions & 6 deletions compiler/rustc_codegen_llvm/src/coverageinfo/map_data.rs
Original file line number Diff line number Diff line change
Expand Up @@ -2,8 +2,8 @@ use rustc_data_structures::captures::Captures;
use rustc_data_structures::fx::FxIndexSet;
use rustc_index::bit_set::BitSet;
use rustc_middle::mir::coverage::{
CodeRegion, CounterId, CovTerm, Expression, ExpressionId, FunctionCoverageInfo, Mapping,
MappingKind, Op,
CounterId, CovTerm, Expression, ExpressionId, FunctionCoverageInfo, Mapping, MappingKind, Op,
SourceRegion,
};
use rustc_middle::ty::Instance;
use rustc_span::Symbol;
Expand Down Expand Up @@ -201,7 +201,7 @@ impl<'tcx> FunctionCoverage<'tcx> {

/// Returns an iterator over all filenames used by this function's mappings.
pub(crate) fn all_file_names(&self) -> impl Iterator<Item = Symbol> + Captures<'_> {
self.function_coverage_info.mappings.iter().map(|mapping| mapping.code_region.file_name)
self.function_coverage_info.mappings.iter().map(|mapping| mapping.source_region.file_name)
}

/// Convert this function's coverage expression data into a form that can be
Expand Down Expand Up @@ -230,12 +230,12 @@ impl<'tcx> FunctionCoverage<'tcx> {
/// that will be used by `mapgen` when preparing for FFI.
pub(crate) fn counter_regions(
&self,
) -> impl Iterator<Item = (MappingKind, &CodeRegion)> + ExactSizeIterator {
) -> impl Iterator<Item = (MappingKind, &SourceRegion)> + ExactSizeIterator {
self.function_coverage_info.mappings.iter().map(move |mapping| {
let Mapping { kind, code_region } = mapping;
let Mapping { kind, source_region } = mapping;
let kind =
kind.map_terms(|term| if self.is_zero_term(term) { CovTerm::Zero } else { term });
(kind, code_region)
(kind, source_region)
})
}

Expand Down
1 change: 0 additions & 1 deletion compiler/rustc_middle/src/arena.rs
Original file line number Diff line number Diff line change
Expand Up @@ -35,7 +35,6 @@ macro_rules! arena_types {
)>,
[] crate_for_resolver: rustc_data_structures::steal::Steal<(rustc_ast::Crate, rustc_ast::AttrVec)>,
[] resolutions: rustc_middle::ty::ResolverGlobalCtxt,
[decode] code_region: rustc_middle::mir::coverage::CodeRegion,
[] const_allocs: rustc_middle::mir::interpret::Allocation,
[] region_scope_tree: rustc_middle::middle::region::ScopeTree,
// Required for the incremental on-disk cache
Expand Down
6 changes: 3 additions & 3 deletions compiler/rustc_middle/src/mir/coverage.rs
Original file line number Diff line number Diff line change
Expand Up @@ -163,15 +163,15 @@ impl Debug for CoverageKind {

#[derive(Clone, TyEncodable, TyDecodable, Hash, HashStable, PartialEq, Eq, PartialOrd, Ord)]
#[derive(TypeFoldable, TypeVisitable)]
pub struct CodeRegion {
pub struct SourceRegion {
pub file_name: Symbol,
pub start_line: u32,
pub start_col: u32,
pub end_line: u32,
pub end_col: u32,
}

impl Debug for CodeRegion {
impl Debug for SourceRegion {
fn fmt(&self, fmt: &mut Formatter<'_>) -> fmt::Result {
write!(
fmt,
Expand Down Expand Up @@ -242,7 +242,7 @@ impl MappingKind {
#[derive(TyEncodable, TyDecodable, Hash, HashStable, TypeFoldable, TypeVisitable)]
pub struct Mapping {
pub kind: MappingKind,
pub code_region: CodeRegion,
pub source_region: SourceRegion,
}

/// Stores per-function coverage information attached to a `mir::Body`,
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 @@ -547,8 +547,8 @@ fn write_function_coverage_info(
for (id, expression) in expressions.iter_enumerated() {
writeln!(w, "{INDENT}coverage {id:?} => {expression:?};")?;
}
for coverage::Mapping { kind, code_region } in mappings {
writeln!(w, "{INDENT}coverage {kind:?} => {code_region:?};")?;
for coverage::Mapping { kind, source_region } in mappings {
writeln!(w, "{INDENT}coverage {kind:?} => {source_region:?};")?;
}
writeln!(w)?;

Expand Down
1 change: 0 additions & 1 deletion compiler/rustc_middle/src/ty/codec.rs
Original file line number Diff line number Diff line change
Expand Up @@ -462,7 +462,6 @@ impl_decodable_via_ref! {
&'tcx traits::ImplSource<'tcx, ()>,
&'tcx mir::Body<'tcx>,
&'tcx mir::BorrowCheckResult<'tcx>,
&'tcx mir::coverage::CodeRegion,
&'tcx ty::List<ty::BoundVariableKind>,
&'tcx ty::ListWithCachedTypeInfo<ty::Clause<'tcx>>,
&'tcx ty::List<FieldIdx>,
Expand Down
48 changes: 21 additions & 27 deletions compiler/rustc_mir_transform/src/coverage/mod.rs
Original file line number Diff line number Diff line change
Expand Up @@ -13,7 +13,7 @@ use rustc_hir::intravisit::{walk_expr, Visitor};
use rustc_middle::hir::map::Map;
use rustc_middle::hir::nested_filter;
use rustc_middle::mir::coverage::{
CodeRegion, CoverageKind, DecisionInfo, FunctionCoverageInfo, Mapping, MappingKind,
CoverageKind, DecisionInfo, FunctionCoverageInfo, Mapping, MappingKind, SourceRegion,
};
use rustc_middle::mir::{
self, BasicBlock, BasicBlockData, SourceInfo, Statement, StatementKind, Terminator,
Expand Down Expand Up @@ -159,7 +159,7 @@ fn create_mappings<'tcx>(
.expect("all BCBs with spans were given counters")
.as_term()
};
let region_for_span = |span: Span| make_code_region(source_map, file_name, span, body_span);
let region_for_span = |span: Span| make_source_region(source_map, file_name, span, body_span);

// Fully destructure the mappings struct to make sure we don't miss any kinds.
let ExtractedMappings {
Expand All @@ -175,9 +175,9 @@ fn create_mappings<'tcx>(
mappings.extend(code_mappings.iter().filter_map(
// Ordinary code mappings are the simplest kind.
|&mappings::CodeMapping { span, bcb }| {
let code_region = region_for_span(span)?;
let source_region = region_for_span(span)?;
let kind = MappingKind::Code(term_for_bcb(bcb));
Some(Mapping { kind, code_region })
Some(Mapping { kind, source_region })
},
));

Expand All @@ -186,29 +186,29 @@ fn create_mappings<'tcx>(
let true_term = term_for_bcb(true_bcb);
let false_term = term_for_bcb(false_bcb);
let kind = MappingKind::Branch { true_term, false_term };
let code_region = region_for_span(span)?;
Some(Mapping { kind, code_region })
let source_region = region_for_span(span)?;
Some(Mapping { kind, source_region })
},
));

mappings.extend(mcdc_branches.iter().filter_map(
|&mappings::MCDCBranch { span, true_bcb, false_bcb, condition_info, decision_depth: _ }| {
let code_region = region_for_span(span)?;
let source_region = region_for_span(span)?;
let true_term = term_for_bcb(true_bcb);
let false_term = term_for_bcb(false_bcb);
let kind = match condition_info {
Some(mcdc_params) => MappingKind::MCDCBranch { true_term, false_term, mcdc_params },
None => MappingKind::Branch { true_term, false_term },
};
Some(Mapping { kind, code_region })
Some(Mapping { kind, source_region })
},
));

mappings.extend(mcdc_decisions.iter().filter_map(
|&mappings::MCDCDecision { span, bitmap_idx, num_conditions, .. }| {
let code_region = region_for_span(span)?;
let source_region = region_for_span(span)?;
let kind = MappingKind::MCDCDecision(DecisionInfo { bitmap_idx, num_conditions });
Some(Mapping { kind, code_region })
Some(Mapping { kind, source_region })
},
));

Expand Down Expand Up @@ -362,19 +362,13 @@ fn inject_statement(mir_body: &mut mir::Body<'_>, counter_kind: CoverageKind, bb
/// but it's hard to rule out entirely (especially in the presence of complex macros
/// or other expansions), and if it does happen then skipping a span or function is
/// better than an ICE or `llvm-cov` failure that the user might have no way to avoid.
fn make_code_region(
#[instrument(level = "debug", skip(source_map))]
fn make_source_region(
source_map: &SourceMap,
file_name: Symbol,
span: Span,
body_span: Span,
) -> Option<CodeRegion> {
debug!(
"Called make_code_region(file_name={}, span={}, body_span={})",
file_name,
source_map.span_to_diagnostic_string(span),
source_map.span_to_diagnostic_string(body_span)
);

) -> Option<SourceRegion> {
let lo = span.lo();
let hi = span.hi();

Expand Down Expand Up @@ -424,7 +418,7 @@ fn make_code_region(
start_line = source_map.doctest_offset_line(&file.name, start_line);
end_line = source_map.doctest_offset_line(&file.name, end_line);

check_code_region(CodeRegion {
check_source_region(SourceRegion {
file_name,
start_line: start_line as u32,
start_col: start_col as u32,
Expand All @@ -433,12 +427,12 @@ fn make_code_region(
})
}

/// If `llvm-cov` sees a code region that is improperly ordered (end < start),
/// If `llvm-cov` sees a source region that is improperly ordered (end < start),
/// it will immediately exit with a fatal error. To prevent that from happening,
/// discard regions that are improperly ordered, or might be interpreted in a
/// way that makes them improperly ordered.
fn check_code_region(code_region: CodeRegion) -> Option<CodeRegion> {
let CodeRegion { file_name: _, start_line, start_col, end_line, end_col } = code_region;
fn check_source_region(source_region: SourceRegion) -> Option<SourceRegion> {
let SourceRegion { file_name: _, start_line, start_col, end_line, end_col } = source_region;

// Line/column coordinates are supposed to be 1-based. If we ever emit
// coordinates of 0, `llvm-cov` might misinterpret them.
Expand All @@ -451,17 +445,17 @@ fn check_code_region(code_region: CodeRegion) -> Option<CodeRegion> {
let is_ordered = (start_line, start_col) <= (end_line, end_col);

if all_nonzero && end_col_has_high_bit_unset && is_ordered {
Some(code_region)
Some(source_region)
} else {
debug!(
?code_region,
?source_region,
?all_nonzero,
?end_col_has_high_bit_unset,
?is_ordered,
"Skipping code region that would be misinterpreted or rejected by LLVM"
"Skipping source region that would be misinterpreted or rejected by LLVM"
);
// If this happens in a debug build, ICE to make it easier to notice.
debug_assert!(false, "Improper code region: {code_region:?}");
debug_assert!(false, "Improper source region: {source_region:?}");
None
}
}
Expand Down
Loading