[PATCH] D20286: [Coverage] Fix an issue where improper coverage mapping data could be loaded for an inline function.

[Vedant Kumar via llvm-commits]

vsk added a comment.

I just have nitpicks. Still looks good :).


================
Comment at: lib/ProfileData/Coverage/CoverageMappingReader.cpp:297
@@ -296,1 +296,3 @@
 
+Error RawCoverageMappingDummyChecker::isDummy(bool &Result) {
+  // A dummy coverage mapping data consists of just one region with zero count.
----------------
nit, could you use `Expected<bool>` here? That should let you simplify this a bit.

================
Comment at: lib/ProfileData/Coverage/CoverageMappingReader.cpp:347
@@ +346,3 @@
+// Check if the mapping data is a dummy, i.e. is emitted for an unused function.
+static Error isCoverageMappingDummy(uint64_t Hash, StringRef Mapping,
+                                    bool &Result) {
----------------
Same `Expected<bool>` nit.

================
Comment at: lib/ProfileData/Coverage/CoverageMappingReader.cpp:408
@@ +407,3 @@
+    }
+    // Update the existent record if it's a dummy and the new record is real.
+    size_t OldRecordIndex = InsertResult.first->second;
----------------
nit, existing


http://reviews.llvm.org/D20286