about summary refs log tree commit diff
path: root/compiler/rustc_mir/src/transform/coverage/mod.rs
diff options
context:
space:
mode:
authorRich Kadel <richkadel@google.com>2021-05-03 23:21:24 -0700
committerRich Kadel <richkadel@google.com>2021-05-06 11:15:39 -0700
commitcb70221857d7a44bf4625f1a2d5af189f6a12495 (patch)
tree37ae4510ee4376f0e311724a6b90a0047428f838 /compiler/rustc_mir/src/transform/coverage/mod.rs
parent603a42ec5458c547b51173cfa48c23ad37b03c3f (diff)
downloadrust-cb70221857d7a44bf4625f1a2d5af189f6a12495.tar.gz
rust-cb70221857d7a44bf4625f1a2d5af189f6a12495.zip
Coverage instruments closure bodies in macros (not the macro body)
Fixes: #84884

This solution might be considered a compromise, but I think it is the
better choice.

The results in the `closure.rs` test correctly resolve all test cases
broken as described in #84884.

One test pattern (in both `closure_macro.rs` and
`closure_macro_async.rs`) was also affected, and removes coverage
statistics for the lines inside the closure, because the closure
includes a macro. (The coverage remains at the callsite of the macro, so
we lose some detail, but there isn't a perfect choice with macros.

Often macro implementations are split across the macro and the callsite,
and there doesn't appear to be a single "right choice" for which body
should be covered. For the current implementation, we can't do both.

The callsite is most likely to be the preferred site for coverage.

I applied this fix to all `MacroKinds`, not just `Bang`.

I'm trying to resolve an issue of lost coverage in a
`MacroKind::Attr`-based, function-scoped macro. Instead of only
searching for a body_span that is "not a function-like macro" (that is,
MacroKind::Bang), I'm expanding this to all `MacroKind`s. Maybe I should
expand this to `ExpnKind::Desugaring` and `ExpnKind::AstPass` (or
subsets, depending on their sub-kinds) as well, but I'm not sure that's
a good idea.

I'd like to add a test of the `Attr` macro on functions, but I need time
to figure out how to constract a good, simple example without external
crate dependencies. For the moment, all tests still work as expected (no
change), this new commit shouldn't have a negative affect, and more
importantly, I believe it will have a positive effect. I will try to
confirm this.
Diffstat (limited to 'compiler/rustc_mir/src/transform/coverage/mod.rs')
-rw-r--r--compiler/rustc_mir/src/transform/coverage/mod.rs27
1 files changed, 24 insertions, 3 deletions
diff --git a/compiler/rustc_mir/src/transform/coverage/mod.rs b/compiler/rustc_mir/src/transform/coverage/mod.rs
index eaeb44289cf..c1e8f620b30 100644
--- a/compiler/rustc_mir/src/transform/coverage/mod.rs
+++ b/compiler/rustc_mir/src/transform/coverage/mod.rs
@@ -32,7 +32,7 @@ use rustc_middle::mir::{
 use rustc_middle::ty::TyCtxt;
 use rustc_span::def_id::DefId;
 use rustc_span::source_map::SourceMap;
-use rustc_span::{CharPos, Pos, SourceFile, Span, Symbol};
+use rustc_span::{CharPos, ExpnKind, Pos, SourceFile, Span, Symbol};
 
 /// A simple error message wrapper for `coverage::Error`s.
 #[derive(Debug)]
@@ -113,8 +113,29 @@ struct Instrumentor<'a, 'tcx> {
 impl<'a, 'tcx> Instrumentor<'a, 'tcx> {
     fn new(pass_name: &'a str, tcx: TyCtxt<'tcx>, mir_body: &'a mut mir::Body<'tcx>) -> Self {
         let source_map = tcx.sess.source_map();
-        let (some_fn_sig, hir_body) = fn_sig_and_body(tcx, mir_body.source.def_id());
-        let body_span = hir_body.value.span;
+        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 source_file = source_map.lookup_source_file(body_span.lo());
         let fn_sig_span = match some_fn_sig.filter(|fn_sig| {
             fn_sig.span.ctxt() == body_span.ctxt()