[PATCH] D151283: [llvm-cov] Support directory layout in coverage reports
Petr Hosek via Phabricator via llvm-commits
llvm-commits at lists.llvm.org
Tue Aug 22 16:03:08 PDT 2023
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
More information about the llvm-commits
mailing list