This is an archive of the discontinued LLVM Phabricator instance.

[llvm-cov] Add lcov tracefile export format.
ClosedPublic

Authored by allevato on Nov 8 2018, 8:57 AM.

Details

Summary

lcov tracefiles are used by various coverage reporting tools and build
systems (e.g., Bazel). It is a simple text-based format to parse and
more convenient to use than the JSON export format, which needs
additional processing to map regions/segments back to line numbers.

It's a little unfortunate that "text" format is now overloaded to refer
specifically to JSON for export, but I wanted to avoid making any
breaking changes to the UI of the llvm-cov tool at this time.

Patch by Tony Allevato (@allevato).

Diff Detail

Repository
rL LLVM

Event Timeline

allevato created this revision.Nov 8 2018, 8:57 AM
allevato updated this revision to Diff 173181.Nov 8 2018, 8:59 AM
  • Fix file comment.
vsk accepted this revision.Nov 8 2018, 11:02 AM

LGTM, thanks!

I suggest adding a note in docs/ReleaseNotes.rst about this.

This revision is now accepted and ready to land.Nov 8 2018, 11:02 AM
Dor1s accepted this revision.Nov 8 2018, 11:27 AM

Very nice!! LGTM!

tools/llvm-cov/CodeCoverage.cpp
1015 ↗(On Diff #173181)

Just wondering whether we should add json format as well and document it instead of the text. Maybe eventually we'll be able to remove text at all to avoid any confusion? @vsk , what do you think?

1037 ↗(On Diff #173181)

nit: maybe re-phrase like "export in HTML is not supported" ?

vsk added a comment.Nov 8 2018, 12:55 PM

Oh, I just realized this also needs an entry in CommandGuide (I think the file is called llvm-cov.rst).

vsk added inline comments.Nov 8 2018, 12:58 PM
tools/llvm-cov/CodeCoverage.cpp
1015 ↗(On Diff #173181)

This sounds like a good follow-up. In retrospect picking the name ‘text’ wasn’t a good idea, it’s a bit ambiguous.

allevato updated this revision to Diff 173237.Nov 8 2018, 3:34 PM
  • Clean up error message, update release notes and command guide.
allevato marked an inline comment as done.Nov 8 2018, 3:36 PM

PTAL.

In D54266#1291921, @vsk wrote:

I suggest adding a note in docs/ReleaseNotes.rst about this.

Done.

In D54266#1292039, @vsk wrote:

Oh, I just realized this also needs an entry in CommandGuide (I think the file is called llvm-cov.rst).

Done.

tools/llvm-cov/CodeCoverage.cpp
1015 ↗(On Diff #173181)

I'm happy to do this as a follow-up—deprecating an existing option feels better to do as a separate change.

vsk accepted this revision.Nov 8 2018, 3:38 PM

Thanks.

Dor1s accepted this revision.Nov 8 2018, 4:04 PM

Thanks again!

Thanks for the quick review!

Dor1s edited the summary of this revision. (Show Details)Nov 9 2018, 8:12 AM
This revision was automatically updated to reflect the committed changes.