[llvm-cov] Use the native path in the coverage report.

[MaggieYi]

Summary

This patch is spin-off from https://reviews.llvm.org/D23345.

The coverage reports contain the source or binary file paths. On Windows, the file path might contain the separators of both '/' and '\'.
This patch uses the native path in the coverage reports. For example, on Windows, all '/' are converted to '\'.

[vsk]

This looks like it's in good shape. Just a few comments:

test/tools/llvm-cov/Inputs/native_separators.proftext

8 ↗

(On Diff #69375)

Do you mind using just "int main() {}" (without the #include), and re-using doube_dots.proftext? I'm asking because we already have 10 *.proftext files in test/tools/llvm-cov/Inputs, and only 7 of them are unique (md5 test/tools/llvm-cov/Inputs/*.proftext | cut -d'=' -f2 | sort -u | uniq | wc -l).

test/tools/llvm-cov/native_separators.c

10 ↗

(On Diff #69375)

Please use the TEXT-INDEX check-prefix here.

20 ↗

(On Diff #69375)

This is missing a test for the text source view (i.e, FileCheck -check-prefixes=TEXT -input-file=%t.dir/coverage/tmp/native_separators.c.txt).

tools/llvm-cov/SourceCoverageViewHTML.cpp

312 ↗

(On Diff #69375)

I don't think remove_dot_dots should be true. If I have a relative path which looks like: "../x/../y", I wouldn't want that presented as "/x/y" (or even "x/y"). There should be a test for this.

[MaggieYi]

Thanks Vedant, the patch has been updated following your comments.

[MaggieYi]

test/tools/llvm-cov/native_separators.c

21 ↗

(On Diff #69517)

Currently, the source and binary files are only shown in the HTML report, not in the txt report. Should I put the source and binary files in the txt report as well?

tools/llvm-cov/SourceCoverageViewHTML.cpp

312 ↗

(On Diff #69517)

I have added an index html test in the double_dots.c test.

I also considered the case you mentioned here, it could happen in the following case:
Assuming:

1. The “test.cpp”, “test.elf” and “test.profdata” are in the folder: “i:\codecoverage\test1”;
2. There are two folders “test1” and “test2” in the folder i:\codecoverage.

Running the following commands:

$ cd i:\codecoverage\test1\
$ llvm-cov show –format=html –o output test.elf –instr-profile=test.profdata -filename-equivalence ..\test2\..\test1\test.cpp

Checking the index.html file:

The “codecoverage\test1\test.cpp” is generated by llvm-cov.

I have checked the index.html fie, which is correct. I don’t know how to pass “..\test2\..\test1\test.cpp” to the command line in the llvm testing system. Could you please let me know how to do it?

[vsk]

test/tools/llvm-cov/native_separators.c

21 ↗

(On Diff #69517)

Ideally I'd like to preserve feature-parity between the html/text views. It would be nice to extend the text view in a follow-up patch, but don't worry about it for now.

tools/llvm-cov/SourceCoverageViewHTML.cpp

312 ↗

(On Diff #69517)

Good catch. You should be able to write "../test2/../test1/test.cpp" in a RUN line. IIRC, llvm-lit has logic to parse Unix filenames and re-write them on Windows hosts.

[MaggieYi]

Thanks, the test has been added.

[vsk]

Lgtm, thanks.

llvm/trunk/test/tools/llvm-cov/double_dots.c

 // To create the covmapping for this file, copy this file to /tmp/dots/,
 // cd into /tmp/dots, and pass "../dots/double_dots.c" to the compiler. Use
 // llvm-cov convert-for-testing to extract the covmapping.
 
 // RUN: llvm-profdata merge %S/Inputs/double_dots.proftext -o %t.profdata
 // RUN: llvm-cov show %S/Inputs/double_dots.covmapping -instr-profile=%t.profdata -o %t.dir
 // RUN: FileCheck -input-file=%t.dir/index.txt %s
+// RUN: llvm-cov show -format=html %S/Inputs/double_dots.covmapping -instr-profile=%t.profdata -o %t.dir
+// RUN: FileCheck -input-file=%t.dir/index.html %s
 
 // CHECK-NOT: coverage{{.*}}dots{{.*}}..{{.*}}dots
 
 int main() {}
 
 // Re-purpose this file to test that we use relative paths when creating
 // report indices:
 
 // RUN: FileCheck -check-prefix=REL-INDEX -input-file %t.dir/index.txt %s
 // REL-INDEX-NOT: %t.dir
 
 // Check that we get the right error when writing to an invalid path:
 
 // RUN: not llvm-cov show %S/Inputs/double_dots.covmapping -instr-profile=%t.profdata -o /dev/null 2>&1 | FileCheck %s -check-prefix=ERROR-MESSAGE
 // ERROR-MESSAGE: error: {{.*}}: Could not create index file!

llvm/trunk/test/tools/llvm-cov/native_separators.c

+// To create the covmapping for this file on Linux, copy this file to /tmp
+// cd into /tmp. Use llvm-cov convert-for-testing to extract the covmapping.
+// This test is Windows-only. It checks that all paths, which are generated
+// in the index and source coverage reports, are native path. For example,
+// on Windows all '/' are converted to '\'.
+// REQUIRES: system-windows
+
+// RUN: llvm-profdata merge %S/Inputs/double_dots.proftext -o %t.profdata
+// RUN: llvm-cov show %S/Inputs/native_separators.covmapping -instr-profile=%t.profdata -o %t.dir
+// RUN: FileCheck -check-prefixes=TEXT-INDEX -input-file=%t.dir/index.txt %s
+// RUN: llvm-cov show -format=html %S/Inputs/native_separators.covmapping -instr-profile=%t.profdata -filename-equivalence ../llvm-config/../llvm-cov/native_separators.c -o %t.dir
+// RUN: FileCheck -check-prefixes=HTML-INDEX -input-file=%t.dir/index.html %s
+// RUN: llvm-cov show -format=html %S/Inputs/native_separators.covmapping -instr-profile=%t.profdata -filename-equivalence %s -o %t.dir
+// RUN: FileCheck -check-prefixes=HTML -input-file=%t.dir/coverage/tmp/native_separators.c.html %s
+
+// TEXT-INDEX: \tmp\native_separators.c
+// HTML-INDEX: >tmp\native_separators.c</a>
+// HTML: <pre>Source: \tmp\native_separators.c</pre>
+// HTML: tools\llvm-cov\Inputs\native_separators.covmapping</pre>
+
+int main() {}

llvm/trunk/tools/llvm-cov/CodeCoverage.cpp

auto commandLineParser = [&, this](int argc, const char **argv) -> int {
     CompareFilenamesOnly = FilenameEquivalence;
 
     ViewOpts.Format = Format;
     SmallString<128> ObjectFilePath(this->ObjectFilename);
     if (std::error_code EC = sys::fs::make_absolute(ObjectFilePath)) {
       error(EC.message(), this->ObjectFilename);
       return 1;
     }
+    sys::path::native(ObjectFilePath);
     ViewOpts.ObjectFilename = ObjectFilePath.c_str();
     switch (ViewOpts.Format) {
     case CoverageViewOptions::OutputFormat::Text:
       ViewOpts.Colors = UseColor == cl::BOU_UNSET
                             ? sys::Process::StandardOutHasColors()
                             : UseColor == cl::BOU_TRUE;
       break;
     case CoverageViewOptions::OutputFormat::HTML:

llvm/trunk/tools/llvm-cov/SourceCoverageView.cpp

if (!InToplevel)
     sys::path::append(FullPath, getCoverageDir());
 
   SmallString<256> ParentPath = sys::path::parent_path(Path);
   sys::path::remove_dots(ParentPath, /*remove_dot_dots=*/true);
   sys::path::append(FullPath, sys::path::relative_path(ParentPath));
 
   auto PathFilename = (sys::path::filename(Path) + "." + Extension).str();
   sys::path::append(FullPath, PathFilename);
+  sys::path::native(FullPath);
 
   return FullPath.str();
 }
 
 Expected<CoveragePrinter::OwnedStream>
 CoveragePrinter::createOutputStream(StringRef Path, StringRef Extension,
                                     bool InToplevel) {
   if (!Opts.hasOutputDirectory())

llvm/trunk/tools/llvm-cov/SourceCoverageViewHTML.cpp

Error CoveragePrinterHTML::createIndexFile(ArrayRef<StringRef> SourceFiles) {
   if (Opts.hasCreatedTime())
     OSRef << BeginCreatedTimeDiv
           << tag("span", escape(Opts.CreatedTimeStr, Opts))
           << EndCreatedTimeDiv;
   OSRef << LineBreak;
   OSRef << BeginCenteredDiv << BeginTable;
   OSRef << BeginSourceNameDiv << "Index" << EndSourceNameDiv;
   for (StringRef SF : SourceFiles) {
-    std::string LinkText = escape(sys::path::relative_path(SF), Opts);
+    SmallString<128> LinkTextStr(sys::path::relative_path(SF));
+    sys::path::remove_dots(LinkTextStr, /*remove_dot_dots=*/true);
+    sys::path::native(LinkTextStr);
+    std::string LinkText = escape(sys::path::relative_path(LinkTextStr), Opts);
     std::string LinkTarget =
         escape(getOutputPath(SF, "html", /*InToplevel=*/false), Opts);
     OSRef << tag("tr", tag("td", tag("pre", a(LinkTarget, LinkText), "code")));
   }
   OSRef << EndTable << EndCenteredDiv;
   emitEpilog(OSRef);
 
   // Emit the default stylesheet.
void SourceCoverageViewHTML::renderViewFooter(raw_ostream &OS) {
   OS << EndTable << EndCenteredDiv;
 }
 
 void SourceCoverageViewHTML::renderSourceName(raw_ostream &OS, bool WholeFile) {
   OS << BeginSourceNameDiv;
   // Render the source name for the view.
   std::string SourceFile = isFunctionView() ? "Function: " : "Source: ";
   SourceFile += getSourceName().str();
-  OS << tag("pre", escape(SourceFile, getOptions()));
+  SmallString<128> SourceText(SourceFile);
+  sys::path::remove_dots(SourceText, /*remove_dot_dots=*/true);
+  sys::path::native(SourceText);
+  OS << tag("pre", escape(SourceText, getOptions()));
   // Render the object file name for the view.
   if (WholeFile)
     OS << tag("pre",
               escape("Binary: " + getOptions().ObjectFilename, getOptions()));
   OS << EndSourceNameDiv;
 }
 
 void SourceCoverageViewHTML::renderLinePrefix(raw_ostream &OS, unsigned) {