[PATCH] D151283: [WIP][llvm-cov] Support a Hierarchical Structure for `show --format=html` (WORK IN PROGRESS)

Sun Jun 4 23:03:25 PDT 2023

AtomicGu created this revision.
Herald added a project: All.
AtomicGu updated this revision to Diff 524988.
AtomicGu added a comment.
AtomicGu edited the summary of this revision.
AtomicGu edited the summary of this revision.
AtomicGu added reviewers: phosek, gulfem, davidxl.
AtomicGu retitled this revision from "[llvm-cov] Change the HTML Report to Hierarchical Structure (WORK IN PROGRESS)" to "[llvm-cov] Support a Hierarchical Structure for HTML Report (WORK IN PROGRESS)".
AtomicGu retitled this revision from "[llvm-cov] Support a Hierarchical Structure for HTML Report (WORK IN PROGRESS)" to "[llvm-cov] Support a Hierarchical Structure for `show --format=html` (WORK IN PROGRESS)".
AtomicGu retitled this revision from "[llvm-cov] Support a Hierarchical Structure for `show --format=html` (WORK IN PROGRESS)" to "[WIP][llvm-cov] Support a Hierarchical Structure for `show --format=html` (WORK IN PROGRESS)".
AtomicGu published this revision for review.
Herald added a project: LLVM.
Herald added a subscriber: llvm-commits.

use clang-format

gulfem added a comment.

It can be useful to add an option for hierarchical directory structure like other coverage viewing options (https://github.com/llvm/llvm-project/blob/main/llvm/tools/llvm-cov/CoverageViewOptions.h#L30) to be able to turn it on/off:

================
Comment at: llvm/tools/llvm-cov/CoverageReport.h:47
-  static void
-  prepareSingleFileReport(const StringRef Filename,
-                          const coverage::CoverageMapping *Coverage,
----------------
Can you please make sure that you only format the lines that you change, and not format the file whole file? This makes it easier for the reviewers to see the actual changes instead of formatting changes?  AFAICT, most of the changes in `CoverageReport.h` and `CoverageReport.cpp` are formatting changes.

================
Comment at: llvm/tools/llvm-cov/CoverageReport.h:47
-  static void
-  prepareSingleFileReport(const StringRef Filename,
-                          const coverage::CoverageMapping *Coverage,
----------------
gulfem wrote:
> Can you please make sure that you only format the lines that you change, and not format the file whole file? This makes it easier for the reviewers to see the actual changes instead of formatting changes?  AFAICT, most of the changes in `CoverageReport.h` and `CoverageReport.cpp` are formatting changes.
> Can you please make sure that you only format the lines that you change, and not format the file whole file? This makes it easier for the reviewers to see the actual changes instead of formatting changes?  AFAICT, most of the changes in `CoverageReport.h` and `CoverageReport.cpp` are formatting changes.

================
Comment at: llvm/tools/llvm-cov/CoverageReport.h:47
-  static void
-  prepareSingleFileReport(const StringRef Filename,
-                          const coverage::CoverageMapping *Coverage,
----------------
AtomicGu wrote:
> gulfem wrote:
> > Can you please make sure that you only format the lines that you change, and not format the file whole file? This makes it easier for the reviewers to see the actual changes instead of formatting changes?  AFAICT, most of the changes in `CoverageReport.h` and `CoverageReport.cpp` are formatting changes.
> > Can you please make sure that you only format the lines that you change, and not format the file whole file? This makes it easier for the reviewers to see the actual changes instead of formatting changes?  AFAICT, most of the changes in `CoverageReport.h` and `CoverageReport.cpp` are formatting changes.
> 
> 
Sorry, this is done by clang-format automatically. I will figure this out in the next commit.

================
Comment at: llvm/tools/llvm-cov/SourceCoverageViewHTML.cpp:503
+    if (DirLCP.size() == 0) {
+      Files.push_back(DirLCP.str());
+      continue; // In this case, only one file in that directory.
----------------
When the directory contains a single file, it is added to `Files`, but all the `FileReports` for `Files` are previously generated at line 494. So, there won't be any report generated for a single file in a directory. 

================
Comment at: llvm/tools/llvm-cov/SourceCoverageViewHTML.cpp:503
+    if (DirLCP.size() == 0) {
+      Files.push_back(DirLCP.str());
+      continue; // In this case, only one file in that directory.
----------------
gulfem wrote:
> When the directory contains a single file, it is added to `Files`, but all the `FileReports` for `Files` are previously generated at line 494. So, there won't be any report generated for a single file in a directory. 
> 
Thank you! You are right, this is a mistake. I was too careless. I will solve this in the next commit.

> This is a GSoC23 project (discourse link <https://discourse.llvm.org/t/coverage-support-a-hierarchical-directory-structure-in-generated-coverage-html-reports/68239/7>).
> This is a work in progess, and may changes a lot in future revisions.

Clang supports source-based coverage <https://clang.llvm.org/docs/SourceBasedCodeCoverage.html> that shows which lines of code are covered by the executed tests. It uses llvm-profdata <https://llvm.org/docs/CommandGuide/llvm-profdata.html> and llvm-cov <https://llvm.org/docs/CommandGuide/llvm-cov.html> tools to generate coverage reports. llvm-cov currently generates a single top-level index HTML file. For example, a single top-level directory code coverage report <https://lab.llvm.org/coverage/coverage-reports/index.html> for LLVM repo is published on a coverage bot. Top-level indexing causes rendering scalability issues in large projects, such as Fuchsia <https://fuchsia.dev>. The goal of this project is to generate a hierarchical directory structure in generated coverage html reports to match the directory structure and solve scalability issues. Chromium uses its own post-processing tools <https://source.chromium.org/chromium/chromium/tools/build/+/main:recipes/recipe_modules/code_coverage/resources/generate_coverage_metadata.py> to show a per-directory hierarchical structure for coverage results <https://analysis.chromium.org/coverage/p/chromium>. Similarly, Lcov <https://llvm.org/reports/coverage/index.html>, which is a graphical front-end Gcov <https://gcc.gnu.org/onlinedocs/gcc/Gcov.html>, provides a one-level directory structure to display coverage results.

Known issues with current prototype:

1. It changes the behavior of the orignial '--format=html' option instead of add a new one.
2. The path and hyperlink generating code is very messy and has much unessasary string copying.
3. It doesn't use multi-threading to generate output.

Repository:
  rG LLVM Github Monorepo

https://reviews.llvm.org/D151283

Files:
  llvm/tools/llvm-cov/CoverageReport.cpp
  llvm/tools/llvm-cov/CoverageReport.h
  llvm/tools/llvm-cov/SourceCoverageView.h
  llvm/tools/llvm-cov/SourceCoverageViewHTML.cpp
  llvm/tools/llvm-cov/SourceCoverageViewHTML.h

-------------- next part --------------
A non-text attachment was scrubbed...
Name: D151283.524988.patch
Type: text/x-patch
Size: 15527 bytes
Desc: not available
URL: <http://lists.llvm.org/pipermail/llvm-commits/attachments/20230605/f1d55dfa/attachment.bin>