From 31f523ff94247fd030473979782dda90923da1ef Mon Sep 17 00:00:00 2001
From: Rich Kadel <richkadel@google.com>
Date: Tue, 11 May 2021 01:13:52 -0700
Subject: [PATCH] Simplified body_span and filtered span code
Some code cleanup extracted from future (but unfinished) commit to fix
coverage in attr macro functions.
---
.../rustc_mir/src/transform/coverage/mod.rs | 64 ++++++++++-----
.../rustc_mir/src/transform/coverage/spans.rs | 78 +++++++++----------
.../rustc_mir/src/transform/coverage/tests.rs | 6 +-
3 files changed, 80 insertions(+), 68 deletions(-)
diff --git a/compiler/rustc_mir/src/transform/coverage/mod.rs b/compiler/rustc_mir/src/transform/coverage/mod.rs
index c15056575c8..dc21bfbb445 100644
--- a/compiler/rustc_mir/src/transform/coverage/mod.rs
+++ b/compiler/rustc_mir/src/transform/coverage/mod.rs
@@ -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());
}
}
@@ -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| {
@@ -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 {
@@ -160,12 +151,12 @@ 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;
+<<<<<<< HEAD
debug!(
"instrumenting {:?}, fn sig span: {}, body span: {}",
def_id,
@@ -173,6 +164,8 @@ impl<'a, 'tcx> Instrumentor<'a, 'tcx> {
source_map.span_to_diagnostic_string(body_span)
);
+=======
+>>>>>>> 476104d0f54 (Simplified body_span and filtered span code)
let mut graphviz_data = debug::GraphvizData::new();
let mut debug_used_expressions = debug::UsedExpressions::new();
@@ -561,6 +554,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()
diff --git a/compiler/rustc_mir/src/transform/coverage/spans.rs b/compiler/rustc_mir/src/transform/coverage/spans.rs
index 444b4e2ca19..f62171b3c53 100644
--- a/compiler/rustc_mir/src/transform/coverage/spans.rs
+++ b/compiler/rustc_mir/src/transform/coverage/spans.rs
@@ -530,17 +530,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()
}
@@ -795,13 +803,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`.
@@ -838,18 +842,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
@@ -873,7 +873,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
@@ -884,28 +884,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`):
+/// 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. 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).)
-///
-/// [^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 }
}
diff --git a/compiler/rustc_mir/src/transform/coverage/tests.rs b/compiler/rustc_mir/src/transform/coverage/tests.rs
index 9b84173c8a2..b04c2d542d4 100644
--- a/compiler/rustc_mir/src/transform/coverage/tests.rs
+++ b/compiler/rustc_mir/src/transform/coverage/tests.rs
@@ -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(),
));