[PATCH] D16133: [Coverage] : introduce class hierarchy (funcRecordReader) to support multiple versions of coverage data

[Vedant Kumar via llvm-commits]

vsk added a comment.

Thanks for the explanation, this lgtm.


================
Comment at: lib/ProfileData/CoverageMappingReader.cpp:309-341
@@ -308,12 +308,35 @@
 
-template <typename T, support::endianness Endian>
-static std::error_code readCoverageMappingData(
-    InstrProfSymtab &ProfileNames, StringRef Data,
-    std::vector<BinaryCoverageReader::ProfileMappingRecord> &Records,
-    std::vector<StringRef> &Filenames) {
-  using namespace support;
-  llvm::DenseSet<T> UniqueFunctionMappingData;
-
-  // Read the records in the coverage data section.
-  for (const char *Buf = Data.data(), *End = Buf + Data.size(); Buf < End;) {
+struct CovMapFuncRecordReader {
+  virtual std::error_code readFunctionRecords(const char *&Buf,
+                                              const char *End) = 0;
+  template <class IntPtrT, support::endianness Endian>
+  static std::unique_ptr<CovMapFuncRecordReader>
+  get(coverage::CoverageMappingVersion Version, InstrProfSymtab &P,
+      std::vector<BinaryCoverageReader::ProfileMappingRecord> &R,
+      std::vector<StringRef> &F);
+};
+
+template <coverage::CoverageMappingVersion CovMapVersion, class IntPtrT,
+          support::endianness Endian>
+class VersionedCovMapFuncRecordReader : public CovMapFuncRecordReader {
+  typedef typename coverage::CovMapTraits<
+      CovMapVersion, IntPtrT>::CovMapFuncRecordType FuncRecordType;
+  typedef typename coverage::CovMapTraits<CovMapVersion, IntPtrT>::NameRefType
+      NameRefType;
+
+  llvm::DenseSet<NameRefType> UniqueFunctionMappingData;
+  InstrProfSymtab &ProfileNames;
+  std::vector<StringRef> &Filenames;
+  std::vector<BinaryCoverageReader::ProfileMappingRecord> &Records;
+
+public:
+  VersionedCovMapFuncRecordReader(
+      InstrProfSymtab &P,
+      std::vector<BinaryCoverageReader::ProfileMappingRecord> &R,
+      std::vector<StringRef> &F)
+      : ProfileNames(P), Filenames(F), Records(R) {}
+
+  std::error_code readFunctionRecords(const char *&Buf,
+                                      const char *End) override {
+    using namespace support;
     if (Buf + sizeof(CovMapHeader) > End)
----------------
Got it.

================
Comment at: lib/ProfileData/CoverageMappingReader.cpp:433
@@ +432,3 @@
+  // Read the records in the coverage data section.
+  auto CovHeader =
+      reinterpret_cast<const coverage::CovMapHeader *>(Data.data());
----------------
I see, I misread this as a parser for a single module's worth of coverage mapping information.


http://reviews.llvm.org/D16133