Skip to content

Commit

Permalink
Rollup merge of #129686 - Zalathar:source-region, r=compiler-errors
Browse files Browse the repository at this point in the history
coverage: Rename `CodeRegion` to `SourceRegion`

LLVM uses the word "code" to refer to a particular kind of coverage mapping. This unrelated usage of the word is confusing, and makes it harder to introduce types whose names correspond to the LLVM classification of coverage kinds.

No functional changes.
  • Loading branch information
matthiaskrgr authored Aug 28, 2024
2 parents 27d7fb0 + 46e1b5b commit 4854fa7
Show file tree
Hide file tree
Showing 7 changed files with 36 additions and 43 deletions.
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

0 comments on commit 4854fa7

Please sign in to comment.