This is an archive of the discontinued LLVM Phabricator instance.

[CMake] Add targets for generating coverage reports
ClosedPublic

Authored by beanz on Aug 31 2021, 1:10 PM.

Details

Summary

This is a pretty small bit of CMake goop to generate code coverage
reports. I always forget the right script invocation and end up
fumbling around too much.

Wouldn't it be great to have targets that "Just Work"?

Well, I thought so.

At present this only really works correctly for LLVM, but I'll extend
it in subsequent patches to work for subprojects.

Diff Detail

Event Timeline

beanz created this revision.Aug 31 2021, 1:10 PM
beanz requested review of this revision.Aug 31 2021, 1:10 PM
Herald added a project: Restricted Project. · View Herald TranscriptAug 31 2021, 1:10 PM
compnerd added inline comments.Sep 1 2021, 2:42 PM
llvm/cmake/modules/CoverageReport.cmake
3

LLVM tends to use if(...) as the style, please uniformly use that across the file.

8

Do you really expect users to provide an alternate implementation of this?

beanz added inline comments.Sep 2 2021, 9:40 AM
llvm/cmake/modules/CoverageReport.cmake
8

Probably not. I was thinking about some changes, but I suspect it is probably easier to modify the existing tool than make a replacement. I can make this a hard-coded value.

beanz updated this revision to Diff 370309.Sep 2 2021, 9:45 AM

Updates based on feedback from @compnerd

phosek accepted this revision.Sep 2 2021, 10:11 AM

LGTM

llvm/cmake/modules/CoverageReport.cmake
18

Have you considered using SEND_ERROR? I'm worried that people might miss the warning and then become surprised when they don't get coverage so I'd slightly prefer error over warning.

llvm/docs/CMake.rst
322
327
This revision is now accepted and ready to land.Sep 2 2021, 10:11 AM
beanz added inline comments.Sep 2 2021, 11:02 AM
llvm/cmake/modules/CoverageReport.cmake
18

LLVM_BUILD_INSTRUMENTED_COVERAGE works with gcc and gcov, but the python script in our repo doesn't. So I wanted to only generate these targets if you have llvm-cov and llvm-profdata, but I don't want it to be fatal if you don't.

I felt like a warning would get a little more attention than a note or generic message if something went wrong, but not fail the build configuration.

Does that make sense?

beanz updated this revision to Diff 370331.Sep 2 2021, 11:02 AM

I can't spell... Thanks @phosek!

phosek added inline comments.Sep 2 2021, 4:16 PM
llvm/cmake/modules/CoverageReport.cmake
18

That makes sense, I wasn't aware that gcc and gcov is also supported.

This revision was automatically updated to reflect the committed changes.