[PATCH] D25535: [Coverage] Support loading multiple files into a CoverageMapping

Vedant Kumar via llvm-commits llvm-commits at lists.llvm.org
Thu Oct 13 10:36:38 PDT 2016


vsk added inline comments.


================
Comment at: unittests/ProfileData/CoverageMappingTest.cpp:285
 
-TEST_P(MaybeSparseCoverageMappingTest, load_coverage_for_more_than_two_files) {
+TEST_P(CoverageMappingTest, load_coverage_for_more_than_two_files) {
   InstrProfRecord Record("func", 0x1234, {0});
----------------
arphaman wrote:
> 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.
Makes sense -- I went ahead and committed the renaming changes in r284135.


https://reviews.llvm.org/D25535





More information about the llvm-commits mailing list