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

Vedant Kumar via llvm-commits llvm-commits at lists.llvm.org
Fri Aug 26 11:40:51 PDT 2016


vsk added a comment.

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


================
Comment at: test/tools/llvm-cov/Inputs/native_separators.proftext:8
@@ +7,2 @@
+1
+
----------------
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).

================
Comment at: test/tools/llvm-cov/native_separators.c:10
@@ +9,3 @@
+// RUN: llvm-cov show %S/Inputs/native_separators.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/native_separators.covmapping -instr-profile=%t.profdata -o %t.dir
----------------
Please use the TEXT-INDEX check-prefix here.

================
Comment at: test/tools/llvm-cov/native_separators.c:20
@@ +19,3 @@
+// HTML: tools\llvm-cov\Inputs\native_separators.covmapping</pre>
+
+#include<stdio.h>
----------------
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).

================
Comment at: tools/llvm-cov/SourceCoverageViewHTML.cpp:312
@@ +311,3 @@
+    SmallString<128> LinkTextStr(sys::path::relative_path(SF));
+    sys::path::remove_dots(LinkTextStr, /*remove_dot_dots=*/true);
+    sys::path::native(LinkTextStr);
----------------
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.


https://reviews.llvm.org/D23922





More information about the llvm-commits mailing list