[PATCH] D35925: [Coverage] Precise region termination with deferred regions

[Vedant Kumar via Phabricator via cfe-commits]

vsk added a comment.

In https://reviews.llvm.org/D35925#823978, @arphaman wrote:

> This is awesome!
>
> I noticed in the sample output that llvm-cov is now forced to print some new region markers because the terminator introduces a new region on the same line, e.g.
>
>   |// CHECK-LABEL: _Z10while_loopv:
>      88|      1|void while_loop() {
>      89|      1|  if (false)
>      90|      1|    return; // CHECK: [[@LINE]]:11 -> [[@LINE+2]]:3 = (#0 - #1)
>                     ^0    ^1  // Previously, llvm-cov didn't show region markers for this line
>      91|      1|
>
>
> Do you think this can be avoided? Should llvm-cov even try to avoid emitting the region markers? It seems to me that this situation affects just the command-line output of llvm-cov, and region highlighting in HTML won't be impacted by this.


Great question! The region highlighting for the html emitter matches exactly with the textual emitter's, so there is an impact. I think we can/should make two general improvements to llvm-cov to address the issue, which would be useful even without this patch. The first (as you brought up) is to stop emitting region markers for segments which start, but do not end, on one line. This fixes an existing annoyance with loops:

  for (...; ...; ...) -> { <- This singular curly brace gets a region marker because the body's region starts here. That's a little distracting.

It would also suppress highlighting for deferred regions, removing a visual distraction. The second improvement would be to to pick better line execution counts. In the sample output that you quoted, you see that the line with the return gets an execution count of 1. What's happening is that llvm-cov picks the maximum execution count from the regions on (or before) the line. The end result is a little weird, because we associate the line execution count with the first region on a line, and we know the "return" is never executed. I have a patch ready to fix this: it corrects a display issue in the existing llvm-cov test suite, and it helps even more with deferred regions enabled.


https://reviews.llvm.org/D35925