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

Alex Lorenz via llvm-commits llvm-commits at lists.llvm.org
Thu Oct 13 07:15:51 PDT 2016


arphaman added a comment.

Thanks for working on this!



================
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});
----------------
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.


https://reviews.llvm.org/D25535





More information about the llvm-commits mailing list