vsk added inline comments.
================
Comment at: llvm/lib/ProfileData/Coverage/CoverageMapping.cpp:483
bool GapRegion = CR.value().Kind == CounterMappingRegion::GapRegion;
if (CR.index() + 1 == Regions.size() ||
----------------
zequanwu wrote:
> vsk wrote:
> > Why is this deletion necessary? Do we need to introduce 0-length regions
> > which alter the count? An example would help.
> Because a single empty line will be a 0 length region. I don't know why is
> this condition necessary before. Does zero-length region exists before this
> change?
>
> example:
> ```
> int main() {
>
> return 0;
> }
> ```
> Before, llvm-cov gives the following.
> ```
> Counter in file 0 1:12 -> 4:2, #0
> Counter in file 0 2:1 -> 2:1, 0
> Emitting segments for file: /tmp/a.c
> Combined regions:
> 1:12 -> 4:2 (count=1)
> 2:1 -> 2:1 (count=0)
> Segment at 1:12 (count = 1), RegionEntry
> Segment at 2:1 (count = 0), RegionEntry, Skipped
> Segment at 4:2 (count = 0), Skipped
> 1| 1|int main() {
> 2| |
> 3| | return 0;
> 4| |}
> ```
> After:
> ```
> Counter in file 0 1:12 -> 4:2, #0
> Counter in file 0 2:1 -> 2:1, 0
> Emitting segments for file: /tmp/a.c
> Combined regions:
> 1:12 -> 4:2 (count=1)
> 2:1 -> 2:1 (count=0)
> Segment at 1:12 (count = 1), RegionEntry
> Segment at 2:1 (count = 0), RegionEntry, Skipped
> Segment at 2:1 (count = 1)
> Segment at 4:2 (count = 0), Skipped
> 1| 1|int main() {
> 2| |
> 3| 1| return 0;
> 4| 1|}
> ```
It looks like we do occasionally see 0-length regions, possibly due to bugs in
the frontend
(http://lab.llvm.org:8080/coverage/coverage-reports/coverage/Users/buildslave/jenkins/workspace/coverage/llvm-project/llvm/lib/ProfileData/Coverage/CoverageMapping.cpp.html#L485).
I don't expect the reporting tools to be able to handle duplicate segments in a
robust way. Having duplicate segments was the source of "messed up" coverage
bugs in the past, due to the contradiction inherent in having two different
segments begin at the same source location.
Do you see some other way to represent empty lines? Perhaps, if we emit a
skipped region for an empty line, we can emit a follow-up segment that restores
the previously-active region starting on the next line? So in this case:
Segment at 1:12 (count = 1), RegionEntry
Segment at 2:1 (count = 0), RegionEntry, Skipped
Segment at 3:1 (count = 1)
Segment at 4:2 (count = 0), Skipped
Repository:
rG LLVM Github Monorepo
CHANGES SINCE LAST ACTION
https://reviews.llvm.org/D84988/new/
https://reviews.llvm.org/D84988
_______________________________________________
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
