[analyzer] Highlight arrows for currently selected event

[vsavchenko]

Summary

In some cases, when the execution path of the diagnostic
goes back and forth, arrows can overlap and create a mess.
Dimming arrows that are not relevant at the moment, solves this issue.
They are still visible, but don't draw too much attention.

[vsavchenko]

Here is the HTML file for the test.

report-399795.html27 KBDownload

[ASDenysPetrov]

HTML sample looks fine! But there is the same problem as in D92639. IE doesn't draw arrows.

clang/lib/StaticAnalyzer/Core/HTMLDiagnostics.cpp

166

1. Whether size() is not an aim?
2. It can fail in case of empty container.

[vsavchenko]

clang/lib/StaticAnalyzer/Core/HTMLDiagnostics.cpp

166

size() doesn't mean the same semantically. This structure is at least of size 1. Please, consider the following code on how this data structure is constructed and used.

[vsavchenko]

Rebase

[ASDenysPetrov]

I've made some debugging with IE. Let's keep the diagnostics IE friendly. We might not know how many users work with IE in real.

clang/lib/StaticAnalyzer/Core/HTMLDiagnostics.cpp

1275–1283

IE doesn't support classList for elements.

1318–1320

Let's make this more sofisticated, as it only works for English keyboard layout with Caps off.

1335–1338

IE doesn't support Array.from/fill functions.

1357

[ASDenysPetrov]

P.S. Sorry. My prev comments get dimmed, since I've been playing with history commits, but they still are relevant.

[vsavchenko]

Maybe I'm missing something, but do we really need to care about IE? The last version was released in 2013, and even Microsoft itself stops supporting IE. Why should we care? Is there anyone who uses old deprecated browser that is not maintained? classList thing was here for almost 4 years and no one seemed to care. Am I missing something here?

[vsavchenko]

I want to say that I really appreciate the effort you put into finding all the workarounds for IE, but it makes adding new features here incredibly painful because IE doesn't seem to support anything. And the majority of developers (on Linux and on MacOS) have literally no way to test it. What we gain from supporting IE for non-existing users, we lose in the ability to actually improve this code!

[ASDenysPetrov]

In D92928#2890448, @vsavchenko wrote:

IE doesn't seem to support anything. And the majority of developers (on Linux and on MacOS) have literally no way to test it. What we gain from supporting IE for non-existing users, we lose in the ability to actually improve this code!

Oh, I absolutely agree with every of your statements. I'm not really excited to test and adjust this stuff for IE. But I occasionally get requests from users which use IE and, get ready, the internal browser of Visual Studio :D which exploits IE core. So I kindly ask you to keep the JS IE friendly at least for a while. We don't really make changes too often here.

[ASDenysPetrov]

Or, we can find another symbiotic way. You can make changes the way without painfull part of thinking about IE. And I will prepare the next patch adjusting it. Thus, revisions would be smaller. That's would be easier for you to test all the things before the load. I will take a charge for IE part on my own and prepare a new revision.

[ASDenysPetrov]

Let's load this patch and I will prepare the adjustment for IE in pursuit.

[vsavchenko]

In D92928#2913552, @ASDenysPetrov wrote:

Or, we can find another symbiotic way. You can make changes the way without painfull part of thinking about IE. And I will prepare the next patch adjusting it. Thus, revisions would be smaller. That's would be easier for you to test all the things before the load. I will take a charge for IE part on my own and prepare a new revision.

That actually can work! Thanks!

Let's load this patch and I will prepare the adjustment for IE in pursuit.

Ah, OK. Sure, let's do it!

clang/lib/Rewrite/HTMLRewrite.cpp

 .PathIndex { border-radius:8px }
 .PathIndexEvent { background-color:#bfba87 }
 .PathIndexControl { background-color:#8c8c8c }
 .PathIndexPopUp { background-color: #879abc; }
 .PathNav a { text-decoration:none; font-size: larger }
 .CodeInsertionHint { font-weight: bold; background-color: #10dd10 }
 .CodeRemovalHint { background-color:#de1010 }
 .CodeRemovalHint { border-bottom:1px solid #6F9DBE }
-.selected{ background-color:orange !important; }
+.msg.selected{ background-color:orange !important; }
 
 table.simpletable {
   padding: 5px;
   font-size:12pt;
   margin:20px;
   border-collapse: collapse; border-spacing: 0px;
 }
 td.rowname {

clang/lib/StaticAnalyzer/Core/HTMLDiagnostics.cpp

 #include "clang/Basic/SourceManager.h"
 #include "clang/Lex/Lexer.h"
 #include "clang/Lex/Preprocessor.h"
 #include "clang/Lex/Token.h"
 #include "clang/Rewrite/Core/HTMLRewrite.h"
 #include "clang/Rewrite/Core/Rewriter.h"
 #include "clang/StaticAnalyzer/Core/PathDiagnosticConsumers.h"
 #include "llvm/ADT/ArrayRef.h"
+#include "llvm/ADT/STLExtras.h"
 #include "llvm/ADT/Sequence.h"
 #include "llvm/ADT/SmallString.h"
 #include "llvm/ADT/StringRef.h"
 #include "llvm/ADT/iterator_range.h"
 #include "llvm/Support/Casting.h"
 #include "llvm/Support/Errc.h"
 #include "llvm/Support/ErrorHandling.h"
 #include "llvm/Support/FileSystem.h"

 using namespace ento;
 
 //===----------------------------------------------------------------------===//
 // Boilerplate.
 //===----------------------------------------------------------------------===//
 
 namespace {
 
+class ArrowMap;
+
 class HTMLDiagnostics : public PathDiagnosticConsumer {
   PathDiagnosticConsumerOptions DiagOpts;
   std::string Directory;
   bool createdDir = false;
   bool noDir = false;
   const Preprocessor &PP;
   const bool SupportsCrossFileDiagnostics;
 
public:
   // Rewrite the file specified by FID with HTML formatting.
   void RewriteFile(Rewriter &R, const PathPieces& path, FileID FID);
 
   PathGenerationScheme getGenerationScheme() const override {
     return Everything;
   }
 
 private:
-  void addArrowSVGs(Rewriter &R, FileID BugFileID, unsigned NumberOfArrows);
+  void addArrowSVGs(Rewriter &R, FileID BugFileID,
+                    const ArrowMap &ArrowIndices);
 
   /// \return Javascript for displaying shortcuts help;
   StringRef showHelpJavascript();
 
   /// \return Javascript for navigating the HTML report using j/k keys.
   StringRef generateKeyboardNavigationJavascript();
 
   /// \return Javascript for drawing control-flow arrows.

 
 unsigned getPathSizeWithoutArrows(const PathPieces &Path) {
   unsigned TotalPieces = Path.size();
   unsigned TotalArrowPieces = llvm::count_if(
       Path, [](const PathDiagnosticPieceRef &P) { return isArrowPiece(*P); });
   return TotalPieces - TotalArrowPieces;
 }
 
+class ArrowMap : public std::vector<unsigned> {
+  using Base = std::vector<unsigned>;
+
+public:
+  ArrowMap(unsigned Size) : Base(Size, 0) {}
+  unsigned getTotalNumberOfArrows() const { return at(0); }
+};
+
+llvm::raw_ostream &operator<<(llvm::raw_ostream &OS, const ArrowMap &Indices) {
+  OS << "[ ";
+  llvm::interleave(Indices, OS, ",");
+  return OS << " ]";
+}
+
 } // namespace
 
 void ento::createHTMLDiagnosticConsumer(
     PathDiagnosticConsumerOptions DiagOpts, PathDiagnosticConsumers &C,
     const std::string &OutputDir, const Preprocessor &PP,
     const cross_tu::CrossTranslationUnitContext &CTU) {
 
   // FIXME: HTML is currently our default output type, but if the output
unsigned PopUpPieceCount =
       });
 
   unsigned TotalRegularPieces = TotalPieces - TotalNotePieces - PopUpPieceCount;
   unsigned NumRegularPieces = TotalRegularPieces;
   unsigned NumNotePieces = TotalNotePieces;
   unsigned NumberOfArrows = 0;
   // Stores the count of the regular piece indices.
   std::map<int, int> IndexMap;
+  ArrowMap ArrowIndices(TotalRegularPieces + 1);
 
   // Stores the different ranges where we have reported something.
   std::vector<SourceRange> PopUpRanges;
   for (auto I = path.rbegin(), E = path.rend(); I != E; ++I) {
     const auto &Piece = *I->get();
 
     if (isa<PathDiagnosticPopUpPiece>(Piece)) {
       ++IndexMap[NumRegularPieces];
     } else if (isa<PathDiagnosticNotePiece>(Piece)) {
       // This adds diagnostic bubbles, but not navigation.
       // Navigation through note pieces would be added later,
       // as a separate pass through the piece list.
       HandlePiece(R, FID, Piece, PopUpRanges, NumNotePieces, TotalNotePieces);
       --NumNotePieces;
 
     } else if (isArrowPiece(Piece)) {
       NumberOfArrows = ProcessControlFlowPiece(
           R, FID, cast<PathDiagnosticControlFlowPiece>(Piece), NumberOfArrows);
+      ArrowIndices[NumRegularPieces] = NumberOfArrows;
 
     } else {
       HandlePiece(R, FID, Piece, PopUpRanges, NumRegularPieces,
                   TotalRegularPieces);
       --NumRegularPieces;
+      ArrowIndices[NumRegularPieces] = ArrowIndices[NumRegularPieces + 1];
     }
   }
+  ArrowIndices[0] = NumberOfArrows;
+
+  // At this point ArrowIndices represent the following data structure:
+  //   [a_0, a_1, ..., a_N]
+  // where N is the number of events in the path.
+  //
+  // Then for every event with index i \in [0, N - 1], we can say that
+  // arrows with indices \in [a_(i+1), a_i) correspond to that event.
+  // We can say that because arrows with these indices appeared in the
+  // path in between the i-th and the (i+1)-th events.
+  assert(ArrowIndices.back() == 0 &&
+         "No arrows should be after the last event");
+  // This assertion also guarantees that all indices in are <= NumberOfArrows.
+  assert(llvm::is_sorted(ArrowIndices, std::greater<unsigned>()) &&
+         "Incorrect arrow indices map");
 
   // Secondary indexing if we are having multiple pop-ups between two notes.
   // (e.g. [(13) 'a' is 'true'];  [(13.1) 'b' is 'false'];  [(13.2) 'c' is...)
   NumRegularPieces = TotalRegularPieces;
   for (auto I = path.rbegin(), E = path.rend(); I != E; ++I) {
     const auto &Piece = *I->get();
 
     if (const auto *PopUpP = dyn_cast<PathDiagnosticPopUpPiece>(&Piece)) {
void HTMLDiagnostics::RewriteFile(Rewriter &R, const PathPieces &path,
 
   // Add the <table> start tag of pop-up pieces based on the stored ranges.
   HandlePopUpPieceStartTag(R, PopUpRanges);
 
   // Add line numbers, header, footer, etc.
   html::EscapeText(R, FID);
   html::AddLineNumbers(R, FID);
 
-  addArrowSVGs(R, FID, NumberOfArrows);
+  addArrowSVGs(R, FID, ArrowIndices);
 
   // If we have a preprocessor, relex the file and syntax highlight.
   // We might not have a preprocessor if we come from a deserialized AST file,
   // for example.
   html::SyntaxHighlight(R, FID, PP);
   html::HighlightMacros(R, FID, PP);
 }
 
if (const auto *EP = dyn_cast<PathDiagnosticEventPiece>(subPiece.get())) {
          << "</td></tr></table></div>\n";
     }
   }
 
   return num;
 }
 
 void HTMLDiagnostics::addArrowSVGs(Rewriter &R, FileID BugFileID,
-                                   unsigned NumberOfArrows) {
+                                   const ArrowMap &ArrowIndices) {
   std::string S;
   llvm::raw_string_ostream OS(S);
 
   OS << R"<<<(
 <style type="text/css">
   svg {
       position:absolute;
       top:0;
       left:0;
       height:100%;
       width:100%;
       pointer-events: none;
       overflow: visible
   }
+  .arrow {
+      stroke-opacity: 0.2;
+      stroke-width: 1;
+      marker-end: url(#arrowhead);
+  }
+
+  .arrow.selected {
+      stroke-opacity: 0.6;
+      stroke-width: 2;
+      marker-end: url(#arrowheadSelected);
+  }
+
+  .arrowhead {
+      orient: auto;
+      stroke: none;
+      opacity: 0.6;
+      fill: blue;
+  }
 </style>
 <svg xmlns="http://www.w3.org/2000/svg">
   <defs>
-    <marker id="arrowhead" viewBox="0 0 10 10" refX="3" refY="5"
-            markerWidth="4" markerHeight="4" orient="auto" stroke="none" opacity="0.6" fill="blue">
+    <marker id="arrowheadSelected" class="arrowhead" opacity="0.6"
+            viewBox="0 0 10 10" refX="3" refY="5"
+            markerWidth="4" markerHeight="4">
+      <path d="M 0 0 L 10 5 L 0 10 z" />
+    </marker>
+    <marker id="arrowhead" class="arrowhead" opacity="0.2"
+            viewBox="0 0 10 10" refX="3" refY="5"
+            markerWidth="4" markerHeight="4">
       <path d="M 0 0 L 10 5 L 0 10 z" />
     </marker>
   </defs>
-  <g id="arrows" fill="none" stroke="blue"
-     visibility="hidden" stroke-width="2"
-     stroke-opacity="0.6" marker-end="url(#arrowhead)">
+  <g id="arrows" fill="none" stroke="blue" visibility="hidden">
 )<<<";
 
-  for (unsigned Index : llvm::seq(0u, NumberOfArrows)) {
-    OS << "    <path id=\"arrow" << Index << "\"/>\n";
+  for (unsigned Index : llvm::seq(0u, ArrowIndices.getTotalNumberOfArrows())) {
+    OS << "    <path class=\"arrow\" id=\"arrow" << Index << "\"/>\n";
   }
 
   OS << R"<<<(
   </g>
 </svg>
-)<<<";
+<script type='text/javascript'>
+const arrowIndices = )<<<";
+
+  OS << ArrowIndices << "\n</script>\n";
 
   R.InsertTextBefore(R.getSourceMgr().getLocForStartOfFile(BugFileID),
                      OS.str());
 }
 
 std::string getSpanBeginForControl(const char *ClassName, unsigned Index) {
   std::string Result;
   llvm::raw_string_ostream OS(Result);
querySelectorAllArray(".PathNav > a").forEach(
             currentValue.onclick = function() {
                 scrollTo(document.querySelector(hrefValue));
                 return false;
             };
         });
 });
 
 var findNum = function() {
-    var s = document.querySelector(".selected");
+    var s = document.querySelector(".msg.selected");
     if (!s || s.id == "EndPath") {
         return 0;
     }
     var out = parseInt(digitMatcher.exec(s.id)[0]);
     return out;
 };
 
 var scrollTo = function(el) {
     querySelectorAllArray(".selected").forEach(function(s) {
         s.classList.remove("selected");
     });
     el.classList.add("selected");
     window.scrollBy(0, el.getBoundingClientRect().top -
         (window.innerHeight / 2));
+    highlightArrowsForSelectedEvent();
 }
 
 var move = function(num, up, numItems) {
   if (num == 1 && up || num == numItems - 1 && !up) {
     return 0;
   } else if (num == 0 && up) {
     return numItems - 1;
   } else if (num == 0 && !up) {
     return 1 % numItems;
var navigateTo = function(up) {
   // Scroll element into center.
   scrollTo(newEl);
 };
 
 window.addEventListener("keydown", function (event) {
   if (event.defaultPrevented) {
     return;
   }
   if (event.key == "j") {
     navigateTo(/*up=*/false);
   } else if (event.key == "k") {
     navigateTo(/*up=*/true);
   } else {
     return;
   }
   event.preventDefault();
 }, true);
 </script>
   )<<<";
 }
 
 StringRef HTMLDiagnostics::generateArrowDrawingJavascript() {
   return R"<<<(
 <script type='text/javascript'>
+// Return range of numbers from a range [lower, upper).
+function range(lower, upper) {
+  const size = upper - lower;
+  return Array.from(new Array(size), (x, i) => i + lower);
+}
+
+var getRelatedArrowIndices = function(pathId) {
+  // HTML numeration of events is a bit different than it is in the path.
+  // Everything is rotated one step to the right, so the last element
+  // (error diagnostic) has index 0.
+  if (pathId == 0) {
+    // arrowIndices has at least 2 elements
+    pathId = arrowIndices.length - 1;
+  }
+
+  return range(arrowIndices[pathId], arrowIndices[pathId - 1]);
+}
+
+var highlightArrowsForSelectedEvent = function() {
+  const selectedNum = findNum();
+  const arrowIndicesToHighlight = getRelatedArrowIndices(selectedNum);
+  arrowIndicesToHighlight.forEach((index) => {
+    var arrow = document.querySelector("#arrow" + index);
+    arrow.classList.add("selected");
+  });
+}
+
 var getAbsoluteBoundingRect = function(element) {
   const relative = element.getBoundingClientRect();
   return {
     left: relative.left + window.scrollX,
     right: relative.right + window.scrollX,
     top: relative.top + window.scrollY,
     bottom: relative.bottom + window.scrollY,
     height: relative.height,
document.addEventListener("DOMContentLoaded", function() {
   // Hiding irrelevant lines also should cause arrow rerender.
   document
     .querySelector('input[name="showCounterexample"]')
     .addEventListener("change", drawArrows);
   document
     .querySelector('input[name="showArrows"]')
     .addEventListener("change", toggleArrows);
   drawArrows();
+  // Default highlighting for the last event.
+  highlightArrowsForSelectedEvent();
 });
 </script>
   )<<<";
 }

clang/test/Analysis/html_diagnostics/control-arrows.cpp

 int foobar(bool cond, int *x) {
   if (cond)
     x = 0;
   return dereference(x);
 }
 
 // CHECK:          <svg
 // CHECK:            <g
-// CHECK-COUNT-9:      <path id="arrow{{[0-9]+}}"/>
-// CHECK-NOT:          <path id="arrow{{[0-9]+}}"/>
+// CHECK-COUNT-9:      <path class="arrow" id="arrow{{[0-9]+}}"/>
+// CHECK-NOT:          <path class="arrow" id="arrow{{[0-9]+}}"/>
 // CHECK:            </g>
 // CHECK-NEXT:     </svg>
+// CHECK-NEXT:     <script type='text/javascript'>
+// CHECK-NEXT:     const arrowIndices = [ 9,8,6,5,3,2,0 ]
+// CHECK-NEXT:     </script>
 //
 // Except for arrows we still want to have grey bubbles with control notes.
 // CHECK:          <div id="Path2" class="msg msgControl"
 // CHECK-SAME:       <div class="PathIndex PathIndexControl">2</div>
 // CHECK-SAME:       <td>Taking true branch</td>