[llvm-cov] Unify region marker placement between text/html modes

[vsk]

Summary

Make sure that the text and html emitters always emit the same set of
region markers, and avoid emitting redundant markers for line segments
which don't end on the line they start on.

Here is the html output from the deferred-region.cpp test:

region-markers.html25 KBDownload

This is related to D35925, and depends on D36014

[vsk]

+ Eli (heads-up, this is the last patch that's needed to make deferred regions look/behave nicely. If you're using ToT llvm-cov and start noticing strange region highlighting, where semi-colons at the ends of a line get weird highlights, this will fix it)

[efriedma]

By strange region highlighting, do you mean http://lab.llvm.org:8080/coverage/coverage-reports/all/coverage/Users/buildslave/jenkins/sharedspace/clang-stage2-coverage-R@2/llvm/lib/Target/ARM/ARMISelLowering.cpp.html#L1401 ?

[vsk]

In D36020#832656, @efriedma wrote:

By strange region highlighting, do you mean http://lab.llvm.org:8080/coverage/coverage-reports/all/coverage/Users/buildslave/jenkins/sharedspace/clang-stage2-coverage-R@2/llvm/lib/Target/ARM/ARMISelLowering.cpp.html#L1401 ?

Unfortunately, no, is a separate problem in llvm-cov exposed by cfe/r310010. Clang is producing reasonable mappings, but llvm-cov stops being able to find line segments after a certain point, and keeps printing out the line execution count from the last line segment it encountered. I reverted the clang change for now so I can look into what's happening.

llvm/trunk/test/tools/llvm-cov/deferred-region.cpp

-// RUN: llvm-cov show %S/Inputs/deferred-regions.covmapping -instr-profile %S/Inputs/deferred-regions.profdata -show-line-counts-or-regions -dump -path-equivalence=/Users/vk/src/llvm.org-coverage-braces/llvm/test/tools,%S/.. %s 2>&1 | FileCheck %s
+// RUN: llvm-cov show %S/Inputs/deferred-regions.covmapping -instr-profile %S/Inputs/deferred-regions.profdata -show-line-counts-or-regions -dump -path-equivalence=/Users/vk/src/llvm.org-coverage-braces/llvm/test/tools,%S/.. %s &> %t.out && FileCheck %s -input-file %t.out && FileCheck %s -input-file %t.out -check-prefix=MARKER
 
 void foo(int x) {
   if (x == 0) {
     return; // CHECK: [[@LINE]]|{{ +}}1|
   }
 
 } // CHECK: [[@LINE]]|{{ +}}2|
 
int main() {
   foo(0);
   foo(1);
   bar();
   for_loop();
   while_loop();
   gotos();
   return 0;
 }
+
+// MARKER: Marker at 4:7 = 2
+// MARKER-NEXT: Highlighted line 17, 5 -> 11
+// MARKER-NEXT: Marker at 17:5 = 0
+// MARKER-NEXT: Marker at 19:3 = 1
+// MARKER-NEXT: Marker at 19:19 = 2
+// MARKER-NEXT: Marker at 19:27 = 1
+// MARKER-NEXT: Marker at 21:7 = 1
+// MARKER-NEXT: Marker at 23:5 = 1
+// MARKER-NEXT: Marker at 23:9 = 1
+// MARKER-NEXT: Highlighted line 24, 7 -> 12
+// MARKER-NEXT: Marker at 24:7 = 0
+// MARKER-NEXT: Highlighted line 36, 5 -> 11
+// MARKER-NEXT: Marker at 36:5 = 0
+// MARKER-NEXT: Marker at 39:10 = 3
+// MARKER-NEXT: Marker at 41:7 = 1
+// MARKER-NEXT: Marker at 43:5 = 1
+// MARKER-NEXT: Marker at 43:12 = 1
+// MARKER-NEXT: Highlighted line 45, 14 -> ?
+// MARKER-NEXT: Marker at 45:9 = 1
+// MARKER-NEXT: Highlighted line 46, 1 -> ?
+// MARKER-NEXT: Highlighted line 47, 1 -> 7
+// MARKER-NEXT: Highlighted line 47, 7 -> 14
+// MARKER-NEXT: Highlighted line 47, 14 -> 21
+// MARKER-NEXT: Highlighted line 47, 21 -> 23
+// MARKER-NEXT: Highlighted line 47, 23 -> 25
+// MARKER-NEXT: Highlighted line 47, 25 -> ?
+// MARKER-NEXT: Marker at 47:7 = 0
+// MARKER-NEXT: Marker at 47:14 = 0
+// MARKER-NEXT: Marker at 47:23 = 0
+// MARKER-NEXT: Highlighted line 48, 1 -> 6
+// MARKER-NEXT: Highlighted line 51, 7 -> 20
+// MARKER-NEXT: Marker at 51:7 = 0
+// MARKER-NEXT: Marker at 53:5 = 1
+// MARKER-NEXT: Marker at 53:12 = 6
+// MARKER-NEXT: Highlighted line 55, 9 -> 14
+// MARKER-NEXT: Highlighted line 63, 5 -> 13
+// MARKER-NEXT: Marker at 63:5 = 0
+// MARKER-NEXT: Highlighted line 67, 1 -> ?
+// MARKER-NEXT: Highlighted line 68, 1 -> 8
+// MARKER-NEXT: Highlighted line 68, 8 -> ?
+// MARKER-NEXT: Highlighted line 69, 1 -> 2

llvm/trunk/test/tools/llvm-cov/showRegionMarkers.cpp

 // RUN: llvm-profdata merge %S/Inputs/regionMarkers.proftext -o %t.profdata
 
-int main() {                      // CHECK: Marker at [[@LINE]]:12 = 1.11M
+int main() {                      // CHECK-NOT: Marker at [[@LINE]]:12
   int x = 0;
 
-  if (x) {                        // CHECK: Marker at [[@LINE]]:10 = 0
+  if (x) {                        // CHECK-NOT: Marker at [[@LINE]]:10
     x = 0;
-  } else {                        // CHECK: Marker at [[@LINE]]:10 = 1.11M
+  } else {                        // CHECK-NOT: Marker at [[@LINE]]:10
     x = 1;
   }
                                   // CHECK: Marker at [[@LINE+2]]:19 = 112M
                                   // CHECK: Marker at [[@LINE+1]]:28 = 111M
-  for (int i = 0; i < 100; ++i) { // CHECK: Marker at [[@LINE]]:33 = 111M
+  for (int i = 0; i < 100; ++i) { // CHECK-NOT: Marker at [[@LINE]]:33
     x = 1;
   }
                                   // CHECK: Marker at [[@LINE+1]]:16 = 1.11M
   x = x < 10 ? x + 1 : x - 1;     // CHECK: Marker at [[@LINE]]:24 = 0
   x = x > 10 ?
-        x - 1:                    // CHECK: Marker at [[@LINE]]:9 = 0
-        x + 1;                    // CHECK: Marker at [[@LINE]]:9 = 1.11M
+        x - 1:                    // CHECK-NOT: Marker at [[@LINE]]:9
+        x + 1;                    // CHECK-NOT: Marker at [[@LINE]]:9
 
   return 0;
 }
 
 // RUN: llvm-cov show %S/Inputs/regionMarkers.covmapping -instr-profile %t.profdata -show-regions -dump -path-equivalence=/Users/bogner/code/llvm/test/tools,%S/.. %s 2>&1 | FileCheck %s
+// RUN: llvm-cov show %S/Inputs/regionMarkers.covmapping -instr-profile %t.profdata -show-regions -format=html -dump -path-equivalence=/Users/bogner/code/llvm/test/tools,%S/.. %s 2>&1 | FileCheck %s
 
 // RUN: llvm-cov export %S/Inputs/regionMarkers.covmapping -instr-profile %t.profdata 2>&1 | FileCheck %S/Inputs/regionMarkers.json

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

int CodeCoverageTool::show(int argc, const char **argv,
   auto Err = commandLineParser(argc, argv);
   if (Err)
     return Err;
 
   ViewOpts.ShowLineNumbers = true;
   ViewOpts.ShowLineStats = ShowLineExecutionCounts.getNumOccurrences() != 0 ||
                            !ShowRegions || ShowBestLineRegionsCounts;
   ViewOpts.ShowRegionMarkers = ShowRegions || ShowBestLineRegionsCounts;
-  ViewOpts.ShowLineStatsOrRegionMarkers = ShowBestLineRegionsCounts;
   ViewOpts.ShowExpandedRegions = ShowExpansions;
   ViewOpts.ShowFunctionInstantiations = ShowInstantiations;
   ViewOpts.ShowOutputDirectory = ShowOutputDirectory;
   ViewOpts.TabSize = TabSize;
   ViewOpts.ProjectTitle = ProjectTitle;
 
   if (ViewOpts.hasOutputDirectory()) {
     if (auto E = sys::fs::create_directories(ViewOpts.ShowOutputDirectory)) {

llvm/trunk/tools/llvm-cov/CoverageViewOptions.h

enum class OutputFormat {
     HTML
   };
 
   bool Debug;
   bool Colors;
   bool ShowLineNumbers;
   bool ShowLineStats;
   bool ShowRegionMarkers;
-  bool ShowLineStatsOrRegionMarkers;
   bool ShowExpandedRegions;
   bool ShowFunctionInstantiations;
   bool ShowFullFilenames;
   OutputFormat Format;
   std::string ShowOutputDirectory;
   std::vector<std::string> DemanglerOpts;
   uint32_t TabSize;
   std::string ProjectTitle;

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

protected:
 
   /// @}
 
   /// \brief Format a count using engineering notation with 3 significant
   /// digits.
   static std::string formatCount(uint64_t N);
 
   /// \brief Check if region marker output is expected for a line.
-  bool shouldRenderRegionMarkers(bool LineHasMultipleRegions) const;
+  bool shouldRenderRegionMarkers(CoverageSegmentArray Segments) const;
 
   /// \brief Check if there are any sub-views attached to this view.
   bool hasSubViews() const;
 
   SourceCoverageView(StringRef SourceName, const MemoryBuffer &File,
                      const CoverageViewOptions &Options,
                      coverage::CoverageData &&CoverageInfo)
       : SourceName(SourceName), File(File), Options(Options),

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

if (IntLen != 3) {
     Result.push_back('.');
     Result += Number.substr(IntLen, 3 - IntLen);
   }
   Result.push_back(" kMGTPEZY"[(Len - 1) / 3]);
   return Result;
 }
 
 bool SourceCoverageView::shouldRenderRegionMarkers(
-    bool LineHasMultipleRegions) const {
-  return getOptions().ShowRegionMarkers &&
-         (!getOptions().ShowLineStatsOrRegionMarkers || LineHasMultipleRegions);
+    CoverageSegmentArray Segments) const {
+  if (!getOptions().ShowRegionMarkers)
+    return false;
+
+  // Render the region markers if there's more than one count to show.
+  unsigned RegionCount = 0;
+  for (const auto *S : Segments)
+    if (S->IsRegionEntry)
+      if (++RegionCount > 1)
+        return true;
+  return false;
 }
 
 bool SourceCoverageView::hasSubViews() const {
   return !ExpansionSubViews.empty() || !InstantiationSubViews.empty();
 }
 
 std::unique_ptr<SourceCoverageView>
 SourceCoverageView::create(StringRef SourceName, const MemoryBuffer &File,
if (NextESV != EndESV && NextESV->getLine() == LI.line_number() &&
         getOptions().Colors)
       ExpansionColumn = NextESV->getStartCol();
 
     // Display the source code for the current line.
     renderLine(OS, {*LI, LI.line_number()}, WrappedSegment, LineSegments,
                ExpansionColumn, ViewDepth);
 
     // Show the region markers.
-    if (shouldRenderRegionMarkers(LineCount.hasMultipleRegions()))
+    if (shouldRenderRegionMarkers(LineSegments))
       renderRegionMarkers(OS, LineSegments, ViewDepth);
 
     // Show the expansions and instantiations for this line.
     bool RenderedSubView = false;
     for (; NextESV != EndESV && NextESV->getLine() == LI.line_number();
          ++NextESV) {
       renderViewDivider(OS, ViewDepth + 1);
 

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

for (const auto &Range : HighlightedRanges) {
         errs() << Range.second;
       errs() << "\n";
     }
   }
 
   // 4. Snippets[1:N+1] correspond to \p Segments[0:N]: use these to generate
   //    sub-line region count tooltips if needed.
 
-  bool HasMultipleRegions = [&] {
-    unsigned RegionCount = 0;
-    for (const auto *S : Segments)
-      if (S->HasCount && S->IsRegionEntry)
-        if (++RegionCount > 1)
-          return true;
-    return false;
-  }();
-
-  if (shouldRenderRegionMarkers(HasMultipleRegions)) {
-    for (unsigned I = 0, E = Segments.size(); I < E; ++I) {
+  if (shouldRenderRegionMarkers(Segments)) {
+    // Just consider the segments which start *and* end on this line.
+    for (unsigned I = 0, E = Segments.size() - 1; I < E; ++I) {
       const auto *CurSeg = Segments[I];
-      if (!CurSeg->IsRegionEntry || !CurSeg->HasCount)
+      if (!CurSeg->IsRegionEntry)
         continue;
 
       Snippets[I + 1] =
           tag("div", Snippets[I + 1] + tag("span", formatCount(CurSeg->Count),
                                            "tooltip-content"),
               "tooltip");
+
+      if (getOptions().Debug)
+        errs() << "Marker at " << CurSeg->Line << ":" << CurSeg->Col << " = "
+               << formatCount(CurSeg->Count) << "\n";
     }
   }
 
   OS << BeginCodeTD;
   OS << BeginPre;
   for (const auto &Snippet : Snippets)
     OS << Snippet;
   OS << EndPre;

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

void SourceCoverageViewText::renderLineNumberColumn(raw_ostream &OS,
   OS.indent(LineNumberColumnWidth - Str.size()) << Str << '|';
 }
 
 void SourceCoverageViewText::renderRegionMarkers(
     raw_ostream &OS, CoverageSegmentArray Segments, unsigned ViewDepth) {
   renderLinePrefix(OS, ViewDepth);
   OS.indent(getCombinedColumnWidth(getOptions()));
 
+  // Just consider the segments which start *and* end on this line.
+  if (Segments.size() > 1)
+    Segments = Segments.drop_back();
+
   unsigned PrevColumn = 1;
   for (const auto *S : Segments) {
     if (!S->IsRegionEntry)
       continue;
     // Skip to the new region.
     if (S->Col > PrevColumn)
       OS.indent(S->Col - PrevColumn);
     PrevColumn = S->Col + 1;
     std::string C = formatCount(S->Count);
     PrevColumn += C.size();
     OS << '^' << C;
-  }
-  OS << '\n';
 
     if (getOptions().Debug)
-    for (const auto *S : Segments)
       errs() << "Marker at " << S->Line << ":" << S->Col << " = "
-             << formatCount(S->Count) << (S->IsRegionEntry ? "\n" : " (pop)\n");
+            << formatCount(S->Count) << "\n";
+  }
+  OS << '\n';
 }
 
 void SourceCoverageViewText::renderExpansionSite(
     raw_ostream &OS, LineRef L, const coverage::CoverageSegment *WrappedSegment,
     CoverageSegmentArray Segments, unsigned ExpansionCol, unsigned ViewDepth) {
   renderLinePrefix(OS, ViewDepth);
   OS.indent(getCombinedColumnWidth(getOptions()) + (ViewDepth == 0 ? 0 : 1));
   renderLine(OS, L, WrappedSegment, Segments, ExpansionCol, ViewDepth);