zequanwu 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() ||
----------------
vsk wrote:
> 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
I think we should have the following, because the wrapped segment's count will
be used in next line (e.g. line 3).
```
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
```
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
