vsk added a comment.

> ! In D83592#2156758 <https://reviews.llvm.org/D83592#2156758>, @zequanwu 
> wrote:
>  One way I could think is probably when we visit decl in 
> `CounterCoverageMappingBuilder`, check for if there is `SkippedRegion` in the 
> same line and then mark the decl range as `CodeRegion`. For the example, we 
> will have 3 more `CodeRegion` which covers the ranges of `int x = 0`, `int y 
> = 0` and `int z = 0`.

I don't think that will work well, because a decl can span multiple lines 
(which in turn can include comments).

The key issue seems to be that -- even having SourceRanges for all comments -- 
we don't know whether a line has non-whitespace, non-comment tokens. If it 
does, the line should have an execution count (and we don't even need to bother 
with emitting a skipped region covering the line). It sounds like we need a 
different hook in the preprocessor that reports SourceRanges for lines with no 
non-comment, non-whitespace tokens. Practically, I think this can be wired up 
just like a CommentHandler (maybe it could be called WhitespaceHandler?). One 
side-benefit of introducing a new hook like this is that coverage will handle 
whitespace-only lines the same way as comment-only lines.

There's probably more than one way to approach this. One thing to be aware of: 
llvm's LineCoverageStats class is the one responsible for determining whether a 
line is "mapped" (i.e. whether it has an execution count). It assumes that when 
a line begins with a skipped region, the subsequent portion of the line must 
also be skipped. If we only emit skipped regions for fully whitespace-like 
lines, that's fine. If not, depending on how C-style comments are handled, that 
may need updating.


Repository:
  rG LLVM Github Monorepo

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D83592/new/

https://reviews.llvm.org/D83592



_______________________________________________
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

Reply via email to