[PATCH] D151283: [llvm-cov] Support directory layout in coverage reports

[Petr Hosek via Phabricator via llvm-commits]

phosek added inline comments.


================
Comment at: llvm/docs/CommandGuide/llvm-cov.rst:254-256
+ Generate index files in each level of the directory to show the coverage.
+ Defaults to false and a single index file will be generated in the output
+ directory.
----------------
I think this would be a better summary of what this flag does.


================
Comment at: llvm/tools/llvm-cov/CoverageReport.cpp:559
+    const ArrayRef<StringRef> &Files, FileCoverageSummary *Totals) {
+  assert(!Files.empty() && "Only works when Files is not empty");
+
----------------
Rather than using an `assert` which would be compiled out when assertions are disabled, I'd consider returning an `Error` in this case.


================
Comment at: llvm/tools/llvm-cov/CoverageReport.h:105
+  // For calling CoverageReport::prepareSingleFileReport asynchronously
+  // in prepareSubDirectoryReports. It's not disigned to be changed by
+  // operator().
----------------



================
Comment at: llvm/tools/llvm-cov/CoverageReport.h:130
+  /// relative path of the only file in that directory.
+  virtual Error operator()(SubFileReports &&SubFiles, SubDirReports &&SubDirs,
+                           FileCoverageSummary &&SubTotals) = 0;
----------------
Is there a reason for using an operator here rather than a regular method?


Repository:
  rG LLVM Github Monorepo

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D151283/new/

https://reviews.llvm.org/D151283