This is an archive of the discontinued LLVM Phabricator instance.

[Coverage] Emit a gap region to cover switch bodies
ClosedPublic

Authored by vsk on Nov 21 2019, 2:42 PM.

Details

Summary

Emit a gap region beginning where the switch body begins. This sets line
execution counts in the areas between non-overlapping cases to 0.

This does not resolve an outstanding issue with case statement regions
that do not end when a region is terminated. But it should address
llvm.org/PR44011.

Diff Detail

Event Timeline

vsk created this revision.Nov 21 2019, 2:42 PM
vsk edited the summary of this revision. (Show Details)

Could you write a description somewhere of what the overall region tree should look like for a switch?

clang/test/CoverageMapping/switch.cpp
32

I'm not sure I understand the effect here. Will we show that nop() never executes, or will we not show any coverage information for it?

vsk planned changes to this revision.Nov 21 2019, 5:48 PM
vsk marked an inline comment as done.

I'll write something up in the coverage mapping docs. Briefly, though, with this change there should be a gap region that covers the entire switch body (starting from the '{' in 'switch {', and terminating where the last case ends).

clang/test/CoverageMapping/switch.cpp
32

The report will show an execution count of 0 for that call to nop(). In general a gap region's count is selected as the line count when it's the only region present in a line.

vsk updated this revision to Diff 230714.Nov 22 2019, 1:06 PM
  • Add some documentation about clang internals.
hans added a comment.Nov 24 2019, 11:54 PM

Thanks!
I'm not that familiar with the coverage counters, but this seems good as far as I can tell.
Maybe add the test case from PR44011? I didn't see any test cases with that kind of gaps between cases.

vsk updated this revision to Diff 231724.Dec 2 2019, 9:01 AM

Add test case from PR44011.

hans accepted this revision.Dec 3 2019, 12:45 AM

Looks fine to me.

clang/docs/SourceBasedCodeCoverage.rst
330 ↗(On Diff #231724)

I thought the point is that it doesn't appear uncovered in coverage reports? I.e. it will have a zero count, but that's expected.

This revision is now accepted and ready to land.Dec 3 2019, 12:45 AM
vsk marked an inline comment as done.Dec 3 2019, 12:12 PM
vsk added inline comments.
clang/docs/SourceBasedCodeCoverage.rst
330 ↗(On Diff #231724)

Yes, I'll make it clear that a zero count is expected here.

This revision was automatically updated to reflect the committed changes.
Herald added a project: Restricted Project. · View Herald TranscriptDec 3 2019, 12:43 PM