Skip to content

Commit

Permalink
Simplified body_span and filtered span code
Browse files Browse the repository at this point in the history
Some code cleanup extracted from future (but unfinished) commit to fix
coverage in attr macro functions.
  • Loading branch information
richkadel committed May 11, 2021
1 parent 0c0d102 commit c61ec71
Show file tree
Hide file tree
Showing 3 changed files with 77 additions and 75 deletions.
68 changes: 40 additions & 28 deletions compiler/rustc_mir/src/transform/coverage/mod.rs
Original file line number Diff line number Diff line change
Expand Up @@ -95,7 +95,7 @@ impl<'tcx> MirPass<'tcx> for InstrumentCoverage {

trace!("InstrumentCoverage starting for {:?}", mir_source.def_id());
Instrumentor::new(&self.name(), tcx, mir_body).inject_counters();
trace!("InstrumentCoverage starting for {:?}", mir_source.def_id());
trace!("InstrumentCoverage done for {:?}", mir_source.def_id());
}
}

Expand All @@ -116,25 +116,7 @@ impl<'a, 'tcx> Instrumentor<'a, 'tcx> {
let def_id = mir_body.source.def_id();
let (some_fn_sig, hir_body) = fn_sig_and_body(tcx, def_id);

let mut body_span = hir_body.value.span;

if tcx.is_closure(def_id) {
// If the MIR function is a closure, and if the closure body span
// starts from a macro, but it's content is not in that macro, try
// to find a non-macro callsite, and instrument the spans there
// instead.
loop {
let expn_data = body_span.ctxt().outer_expn_data();
if expn_data.is_root() {
break;
}
if let ExpnKind::Macro(..) = expn_data.kind {
body_span = expn_data.call_site;
} else {
break;
}
}
}
let body_span = get_body_span(tcx, hir_body, mir_body);

let source_file = source_map.lookup_source_file(body_span.lo());
let fn_sig_span = match some_fn_sig.filter(|fn_sig| {
Expand All @@ -144,6 +126,15 @@ impl<'a, 'tcx> Instrumentor<'a, 'tcx> {
Some(fn_sig) => fn_sig.span.with_hi(body_span.lo()),
None => body_span.shrink_to_lo(),
};

debug!(
"instrumenting {}: {:?}, fn sig span: {:?}, body span: {:?}",
if tcx.is_closure(def_id) { "closure" } else { "function" },
def_id,
fn_sig_span,
body_span
);

let function_source_hash = hash_mir_source(tcx, hir_body);
let basic_coverage_blocks = CoverageGraph::from_mir(mir_body);
Self {
Expand All @@ -160,19 +151,11 @@ impl<'a, 'tcx> Instrumentor<'a, 'tcx> {

fn inject_counters(&'a mut self) {
let tcx = self.tcx;
let source_map = tcx.sess.source_map();
let mir_source = self.mir_body.source;
let def_id = mir_source.def_id();
let fn_sig_span = self.fn_sig_span;
let body_span = self.body_span;

debug!(
"instrumenting {:?}, fn sig span: {}, body span: {}",
def_id,
source_map.span_to_string(fn_sig_span),
source_map.span_to_string(body_span)
);

let mut graphviz_data = debug::GraphvizData::new();
let mut debug_used_expressions = debug::UsedExpressions::new();

Expand Down Expand Up @@ -561,6 +544,35 @@ fn fn_sig_and_body<'tcx>(
(hir::map::fn_sig(hir_node), tcx.hir().body(fn_body_id))
}

fn get_body_span<'tcx>(
tcx: TyCtxt<'tcx>,
hir_body: &rustc_hir::Body<'tcx>,
mir_body: &mut mir::Body<'tcx>,
) -> Span {
let mut body_span = hir_body.value.span;
let def_id = mir_body.source.def_id();

if tcx.is_closure(def_id) {
// If the MIR function is a closure, and if the closure body span
// starts from a macro, but it's content is not in that macro, try
// to find a non-macro callsite, and instrument the spans there
// instead.
loop {
let expn_data = body_span.ctxt().outer_expn_data();
if expn_data.is_root() {
break;
}
if let ExpnKind::Macro(..) = expn_data.kind {
body_span = expn_data.call_site;
} else {
break;
}
}
}

body_span
}

fn hash_mir_source<'tcx>(tcx: TyCtxt<'tcx>, hir_body: &'tcx rustc_hir::Body<'tcx>) -> u64 {
let mut hcx = tcx.create_no_span_stable_hashing_context();
hash(&mut hcx, &hir_body.value).to_smaller_hash()
Expand Down
78 changes: 35 additions & 43 deletions compiler/rustc_mir/src/transform/coverage/spans.rs
Original file line number Diff line number Diff line change
Expand Up @@ -527,17 +527,25 @@ impl<'a, 'tcx> CoverageSpans<'a, 'tcx> {
.iter()
.enumerate()
.filter_map(move |(index, statement)| {
filtered_statement_span(statement, self.body_span).map(
|(span, expn_span)| {
CoverageSpan::for_statement(
statement, span, expn_span, bcb, bb, index,
)
},
)
filtered_statement_span(statement).map(|span| {
CoverageSpan::for_statement(
statement,
function_source_span(span, self.body_span),
span,
bcb,
bb,
index,
)
})
})
.chain(filtered_terminator_span(data.terminator(), self.body_span).map(
|(span, expn_span)| CoverageSpan::for_terminator(span, expn_span, bcb, bb),
))
.chain(filtered_terminator_span(data.terminator()).map(|span| {
CoverageSpan::for_terminator(
function_source_span(span, self.body_span),
span,
bcb,
bb,
)
}))
})
.collect()
}
Expand Down Expand Up @@ -792,13 +800,9 @@ impl<'a, 'tcx> CoverageSpans<'a, 'tcx> {
}
}

/// See `function_source_span()` for a description of the two returned spans.
/// If the MIR `Statement` is not contributive to computing coverage spans,
/// returns `None`.
pub(super) fn filtered_statement_span(
statement: &'a Statement<'tcx>,
body_span: Span,
) -> Option<(Span, Span)> {
/// If the MIR `Statement` has a span contributive to computing coverage spans,
/// return it; otherwise return `None`.
pub(super) fn filtered_statement_span(statement: &'a Statement<'tcx>) -> Option<Span> {
match statement.kind {
// These statements have spans that are often outside the scope of the executed source code
// for their parent `BasicBlock`.
Expand Down Expand Up @@ -835,18 +839,14 @@ pub(super) fn filtered_statement_span(
| StatementKind::LlvmInlineAsm(_)
| StatementKind::Retag(_, _)
| StatementKind::AscribeUserType(_, _) => {
Some(function_source_span(statement.source_info.span, body_span))
Some(statement.source_info.span)
}
}
}

/// See `function_source_span()` for a description of the two returned spans.
/// If the MIR `Terminator` is not contributive to computing coverage spans,
/// returns `None`.
pub(super) fn filtered_terminator_span(
terminator: &'a Terminator<'tcx>,
body_span: Span,
) -> Option<(Span, Span)> {
/// If the MIR `Terminator` has a span contributive to computing coverage spans,
/// return it; otherwise return `None`.
pub(super) fn filtered_terminator_span(terminator: &'a Terminator<'tcx>) -> Option<Span> {
match terminator.kind {
// These terminators have spans that don't positively contribute to computing a reasonable
// span of actually executed source code. (For example, SwitchInt terminators extracted from
Expand All @@ -870,7 +870,7 @@ pub(super) fn filtered_terminator_span(
span = span.with_lo(constant.span.lo());
}
}
Some(function_source_span(span, body_span))
Some(span)
}

// Retain spans from all other terminators
Expand All @@ -881,28 +881,20 @@ pub(super) fn filtered_terminator_span(
| TerminatorKind::GeneratorDrop
| TerminatorKind::FalseUnwind { .. }
| TerminatorKind::InlineAsm { .. } => {
Some(function_source_span(terminator.source_info.span, body_span))
Some(terminator.source_info.span)
}
}
}

/// Returns two spans from the given span (the span associated with a
/// `Statement` or `Terminator`):
///
/// 1. An extrapolated span (pre-expansion[^1]) corresponding to a range within
/// the function's body source. This span is guaranteed to be contained
/// within, or equal to, the `body_span`. If the extrapolated span is not
/// contained within the `body_span`, the `body_span` is returned.
/// 2. The actual `span` value from the `Statement`, before expansion.
///
/// Only the first span is used when computing coverage code regions. The second
/// span is useful if additional expansion data is needed (such as to look up
/// the macro name for a composed span within that macro).)
/// Returns an extrapolated span (pre-expansion[^1]) corresponding to a range
/// within the function's body source. This span is guaranteed to be contained
/// within, or equal to, the `body_span`. If the extrapolated span is not
/// contained within the `body_span`, the `body_span` is returned.
///
/// [^1]Expansions result from Rust syntax including macros, syntactic
/// sugar, etc.).
/// [^1]Expansions result from Rust syntax including macros, syntactic sugar,
/// etc.).
#[inline]
fn function_source_span(span: Span, body_span: Span) -> (Span, Span) {
pub(super) fn function_source_span(span: Span, body_span: Span) -> Span {
let original_span = original_sp(span, body_span).with_ctxt(body_span.ctxt());
(if body_span.contains(original_span) { original_span } else { body_span }, span)
if body_span.contains(original_span) { original_span } else { body_span }
}
6 changes: 2 additions & 4 deletions compiler/rustc_mir/src/transform/coverage/tests.rs
Original file line number Diff line number Diff line change
Expand Up @@ -683,12 +683,10 @@ fn test_make_bcb_counters() {
let mut basic_coverage_blocks = graph::CoverageGraph::from_mir(&mir_body);
let mut coverage_spans = Vec::new();
for (bcb, data) in basic_coverage_blocks.iter_enumerated() {
if let Some((span, expn_span)) =
spans::filtered_terminator_span(data.terminator(&mir_body), body_span)
{
if let Some(span) = spans::filtered_terminator_span(data.terminator(&mir_body)) {
coverage_spans.push(spans::CoverageSpan::for_terminator(
spans::function_source_span(span, body_span),
span,
expn_span,
bcb,
data.last_bb(),
));
Expand Down

0 comments on commit c61ec71

Please sign in to comment.