-Z instrument-coverage shows missing coverage for unbraced closures invoking only a macro

[richkadel]

I expect something that looks like the result, for a similar (non-macro) closure:

  131|      1|    let _short_unused_closure = | _unused_arg: u8 | countdown += 1;
                                                                  ^0

But instead we get results like the following:

  135|      1|    let short_used_not_covered_closure_macro = | used_arg: u8 | println!("not called");
  136|      1|    let _short_unused_closure = | _unused_arg: u8 | println!("not called");
...
  170|      1|    let short_used_not_covered_closure_line_break_no_block_embedded_branch =
  171|       |        | _unused_arg: u8 |
  172|       |            println!(
  173|       |                "not called: {}",
  174|       |                if is_true { "check" } else { "me" }
  175|       |            )
  176|       |    ;
  177|      1|    if is_false {
  178|      0|        short_used_not_covered_closure_macro(0);
  179|      0|        short_used_not_covered_closure_line_break_no_block_embedded_branch(0);
  180|      1|    }

This issue is demonstrated in the coverage results from the test: https://github.com/rust-lang/rust/blob/031c756d16290f3f8ed25a64126ff82cca17123d/src/test/run-make/coverage/closure.rs#L136

I have a new PR in progress to show more examples, and those examples expose the fact that the coverage for println!(...) is actually being added to the macro's source itself.

Note that there were 5 println!(...) macro calls with missing coverage in the corresponding test case, and the coverage report shows 5 Unexecuted instantiations. So all are taken into account.

Nevertheless, this is a function-like macro, and it should behave like a function call, and show the coverage at the calling site.

/usr/local/google/home/richkadel/rust/library/std/src/macros.rs:
   94|       |macro_rules! println {
   95|       |    () => ($crate::print!("\n"));
   96|      0|    ($($arg:tt)*) => ({
   97|      0|        $crate::io::_print($crate::format_args_nl!($($arg)*));
  ------------------
  | Unexecuted instantiation: closure::main::{closure#13}
  ------------------
   98|      0|    })
  ------------------
  | Unexecuted instantiation: closure::main::{closure#5}
  ------------------
  | Unexecuted instantiation: closure::main::{closure#12}
  ------------------
  | Unexecuted instantiation: closure::main::{closure#6}
  ------------------
  | Unexecuted instantiation: closure::main::{closure#11}
  ------------------
   99|       |}

[richkadel]

I suspect the problem is related to how the expansion is handled for the macro body. When defined without braces, it is (likely) "de-sugared" (adding braces perhaps?) in a way that confuses the macro expansion handling in transform/coverage/spans.rs.

[richkadel]

@rustbot +A-code-coverage

fyi: @tmandry @wesleywiser

[richkadel]

I looked into this (reviewing debug logs) and the issue it not exactly a macro problem, I think.

The closure tests I used all call println!(), and that macro is defined as follows:

macro_rules! println {
    () => ($crate::print!("\n"));
    ($($arg:tt)*) => ({
        $crate::io::_print($crate::format_args_nl!($($arg)*));
    })
}

Note the expansion starts with {. That creates a closure that looks like it starts:

| arg | { ...
        ^ where this brace is from the macro

Since the opening brace comes from the macro, the closure MIR body source starts within the macro.

Generating the coverage relative to the source that contains the first character of the function body is generally reasonable, and even in this case it might be hard to argue that it's wrong.

But it's probably not right either. The issue is, where should coverage be generated? If the macro is more complex, instrumenting the macro is good. If the caller (which could be another macro, by the way) is more complex, instrumenting the caller source is often better.

For example, intuitively we want the coverage results of the two macros below to look similar. Putting the coverage results in the source for the println!() macro loses a lot of coverage span detail!

  171|      1|    let short_used_not_covered_closure_line_break_no_block_embedded_branch =
  172|      1|        | _unused_arg: u8 |
  173|       |            println!(
  174|       |                "not called: {}",
  175|       |                if is_true { "check" } else { "me" }
  176|       |            )
  177|       |    ;
  178|       |
  179|      1|    let short_used_not_covered_closure_line_break_block_embedded_branch =
  180|      1|        | _unused_arg: u8 |
  181|      0|        {
  182|      0|            println!(
  183|      0|                "not called: {}",
  184|      0|                if is_true { "check" } else { "me" }
  185|       |            )
  186|      0|        }
  187|       |    ;

I'm not exactly sure what to do.

I'm thinking of treating closures (at least closures that start with a macro) as a special case (at least, when coverage is enabled), and find some way of handling the body span as if it included its own block braces.