[PATCH] D22569: [LLVM-COV] Add the coverage of lines in the summary report.

[Vedant Kumar via llvm-commits]

vsk added a comment.

Nice! This looks really close.

Apart from the nits I mentioned, please update the test in `test/tools/llvm-cov/prevent_false_instantiations.h`. It currently checks for:

`NAN-NOT: 0{{[ \t]+}}nan%{{[ \t]+}}0{{[ \t]+}}nan%`

But with the extra columns you added, it should be relaxed to:

`NAN-NOT: {{[ \t]+}}nan%`


================
Comment at: test/tools/llvm-cov/report.cpp:4
@@ -3,3 +3,3 @@
 
-// CHECK:      Filename   Regions  Miss   Cover  Functions  Executed
+// CHECK:      Filename   Regions  Miss   Cover  Functions  Miss   Executed   Lines    Miss   Cover
 // CHECK-NEXT: ---
----------------
Could you rename the three "Miss" columns to {"Missed Regions", "Missed Functions", "Missed Lines"}? I think that would make the report less confusing.

================
Comment at: tools/llvm-cov/CoverageReport.cpp:91
@@ -90,2 +90,3 @@
 
-static size_t FileReportColumns[] = {25, 10, 8, 8, 10, 10};
+// Add the column widths for line coverage.
+static size_t FileReportColumns[] = {25, 10, 8, 8, 10, 8, 10, 10, 8, 8};
----------------
The comment here doesn't exactly match the source. I'd prefer something more general, e.g: "Specify the default column widths."


https://reviews.llvm.org/D22569