This is an archive of the discontinued LLVM Phabricator instance.

[Coverage] Support loading multiple files into a CoverageMapping
ClosedPublic

Authored by vsk on Oct 12 2016, 3:41 PM.

Details

Summary

Add support for loading multiple coverage readers into a CoverageMapping
instance. This should make it easier to prepare coverage reports for
multiple binaries.

This should make it possible to substantially simplify D25086.

Diff Detail

Event Timeline

vsk updated this revision to Diff 74451.Oct 12 2016, 3:41 PM
vsk retitled this revision from to [Coverage] Support loading multiple files into a CoverageMapping.
vsk updated this object.
vsk added reviewers: MaggieYi, arphaman, ikudrin.
vsk added a subscriber: llvm-commits.
arphaman edited edge metadata.Oct 13 2016, 7:15 AM

Thanks for working on this!

unittests/ProfileData/CoverageMappingTest.cpp
294

I noticed that you have a bunch CoverageMappingTest renaming changes in this patch. Do you think it would be better to do the rename in a separate commit before this patch? So get rid of MaybeSparseCoverageMappingTest and make CoverageMappingTest a bool parameter test, and then in the second commit make CoverageMappingTest a std::pair<bool, bool> test? You don't have to split this patch, just the commits.

vsk added inline comments.Oct 13 2016, 10:36 AM
unittests/ProfileData/CoverageMappingTest.cpp
294

Makes sense -- I went ahead and committed the renaming changes in r284135.

vsk updated this revision to Diff 74544.Oct 13 2016, 10:37 AM
vsk edited edge metadata.
  • Split out renaming changes, rebase to master.
arphaman accepted this revision.Oct 13 2016, 10:43 AM
arphaman edited edge metadata.

LGTM

This revision is now accepted and ready to land.Oct 13 2016, 10:43 AM
This revision was automatically updated to reflect the committed changes.