[cfe-dev] RFC: Attribute to suppress coverage mapping for functions

[Vedant Kumar via cfe-dev]

> I imagine that it will be fairly easy to disable coverage
> for regions which contains just one statement that references a declaration with
> a “nocoverage” attribute.

The way I'd imagine this working is with a new VisitCallExpr. If we grab the
callee decl and figure out that it's marked nocoverage, we could (1) terminate
the current region before the location of the call, (2) skip past the CallExpr,
and (3) and push a new region with a counter that references the old region.

I can see how this would be useful. If "foo" is marked "nocoverage", and "bar"
just calls "foo", then maybe the body of "bar" doesn't really need coverage
reporting. However, I'm not convinced this makes the reports much more useful,
and we'd have to spit out extra CounterMappingRegions to achieve this.

> Do you think it will be difficult to handle cases where
> regions have normal statements that are covered mixed with statements which reference
> declarations with a “nocoverage” attribute? Moreover, how will you handle the case of a 
> sub-expression referencing a “nocoverage” declaration?

I think the scheme I hand-waved through above could work. I'm not sure whether
or not we'd also need to add the source range corresponding to the call marked
"nocoverage" to the skipped regions list. 

The question is whether we really want to go through the trouble. If "foo" is
marked nocoverage, and we e.g use its return value in some expresssion, do we
really want to suppress coverage information for that code region? That sounds
like it could get sketchy.

> By the way, do you think that we should have something similar for macros? We obviously
> wouldn’t be able to use the attribute, but it seems like it would be useful to suppress coverage
> for certain macros.

I haven't seen any requests for this. I hadn't really thought about macros.

If that's something we want, it could be a good argument for implementing
coverage suppression with pragmas. It'd be interesting to here feedback about
this.

vedant

> On Sep 26, 2016, at 4:27 PM, Alex L <arphaman at gmail.com> wrote:
> 
> 
> 
> On 26 September 2016 at 15:43, Vedant Kumar via cfe-dev <cfe-dev at lists.llvm.org> wrote:
> Hi,
> 
> I'd like to add a new attribute which can be used to suppress code coverage
> reporting on the function level. This is meant to work with clang's
> frontend-based coverage implementation [1]. Here are some potential users of
> the new attribute:
> 
>   * llvm_unreachable_internal.
> 
>   * The various dump() functions for llvm Values, AST nodes, etc.
> 
>   * Methods in a base class which just call llvm_unreachable.
> 
> These functions are usually not covered. It's not practical to write "death
> tests" for all of them, and it's also unhelpful to report missing coverage for
> them.
> 
> I'd like to be able to write this in C:
> 
>   void foo() __attribute__((nocoverage)) { llvm_unreachable("boom!"); }
> 
> And this in C++11:
> 
>   void foo() [[clang::nocoverage]] { llvm_unreachable("boom!"); }
> 
> Here are some alternatives and why I think they're not as good:
> 
>   * Define a preprocessor macro when -fcoverage-mapping is enabled.
> 
>     Conditionally compiling code based on whether code coverage is enabled
>     sounds scary. We shouldn't make it easy (or possible?) to change the
>     meaning of a program by enabling coverage.
> 
>   * Pass a function blacklist to llvm-cov.
> 
>     The blacklist would have to live separately from the source code, and may
>     get out of sync. We also would go through the trouble of emitting coverage
>     mappings for functions even though they aren't needed.
> 
>   * Add a pair of pragmas to arbitrarily stop/resume coverage mapping.
> 
>     We'd need some extra diagnostics to catch abuse of the pragmas. It also
>     requires more typing in the common case (disabling coverage at the function
>     level).
> 
>   * Look at the function CFG. If all paths through the function can be shown to
>     reach __builtin_unreachable(), don't create coverage mappings for the
>     function.
> 
>     I'm worried this might be complicated and inflexible. It wouldn't let us
>     mark dump() functions as uncovered.
> 
> Wdyt?
> 
> 
> 
> This definitely sounds like a good idea. I imagine that it will be fairly easy to disable coverage
> for regions which contains just one statement that references a declaration with
> a “nocoverage” attribute. Do you think it will be difficult to handle cases where
> regions have normal statements that are covered mixed with statements which reference
> declarations with a “nocoverage” attribute? Moreover, how will you handle the case of a 
> sub-expression referencing a “nocoverage” declaration?
> 
> By the way, do you think that we should have something similar for macros? We obviously
> wouldn’t be able to use the attribute, but it seems like it would be useful to suppress coverage
> for certain macros.
> 
> Alex
>  
> vedant
> 
> [1] http://clang.llvm.org/docs/SourceBasedCodeCoverage.html
> _______________________________________________
> cfe-dev mailing list
> cfe-dev at lists.llvm.org
> http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-dev
>