Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Jump to bottom

Coverage instruments closure bodies in macros (not the macro body) #84897

Merged
merged 1 commit into from
May 7, 2021
Merged

Coverage instruments closure bodies in macros (not the macro body) #84897

merged 1 commit into from
May 7, 2021

Commits on May 6, 2021

  1. Coverage instruments closure bodies in macros (not the macro body) 

    Fixes: rust-lang#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 rust-lang#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.
    @richkadel
    richkadel committed May 6, 2021
    Configuration menu
    Copy the full SHA
    cb70221 View commit details
    Browse the repository at this point in the history