[PATCH] D92639: [analyzer] Add control flow arrows to the analyzer's HTML reports

[Valeriy Savchenko via Phabricator via cfe-commits]

This revision was landed with ongoing or failed builds.
This revision was automatically updated to reflect the committed changes.
Closed by commit rG97bcafa28deb: [analyzer] Add control flow arrows to the 
analyzers HTML reports (authored by vsavchenko).

Repository:
  rG LLVM Github Monorepo

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D92639/new/

https://reviews.llvm.org/D92639

Files:
  clang/include/clang/Analysis/PathDiagnostic.h
  clang/lib/Rewrite/HTMLRewrite.cpp
  clang/lib/StaticAnalyzer/Core/BugReporter.cpp
  clang/lib/StaticAnalyzer/Core/HTMLDiagnostics.cpp
  clang/test/Analysis/html_diagnostics/control-arrows.cpp

Index: clang/test/Analysis/html_diagnostics/control-arrows.cpp
===
--- /dev/null
+++ clang/test/Analysis/html_diagnostics/control-arrows.cpp
@@ -0,0 +1,27 @@
+// RUN: rm -fR %t
+// RUN: mkdir %t
+// RUN: %clang_analyze_cc1 -analyzer-checker=core \
+// RUN:-analyzer-output=html -o %t -verify %s
+// RUN: cat %t/report-*.html | FileCheck %s
+
+int dereference(int *x) {
+  return *x; // expected-warning{{Dereference of null pointer (loaded from variable 'x')}}
+}
+
+int foobar(bool cond, int *x) {
+  if (cond)
+x = 0;
+  return dereference(x);
+}
+
+// CHECK:  
+// CHECK-NOT:  
+// CHECK:
+// CHECK-NEXT: 
+//
+// Except for arrows we still want to have grey bubbles with control notes.
+// CHECK:  2
+// CHECK-SAME:   Taking true branch
Index: clang/lib/StaticAnalyzer/Core/HTMLDiagnostics.cpp
===
--- clang/lib/StaticAnalyzer/Core/HTMLDiagnostics.cpp
+++ clang/lib/StaticAnalyzer/Core/HTMLDiagnostics.cpp
@@ -27,6 +27,7 @@
 #include "clang/Rewrite/Core/Rewriter.h"
 #include "clang/StaticAnalyzer/Core/PathDiagnosticConsumers.h"
 #include "llvm/ADT/ArrayRef.h"
+#include "llvm/ADT/Sequence.h"
 #include "llvm/ADT/SmallString.h"
 #include "llvm/ADT/StringRef.h"
 #include "llvm/ADT/iterator_range.h"
@@ -77,60 +78,78 @@
   void FlushDiagnosticsImpl(std::vector ,
 FilesMade *filesMade) override;
 
-  StringRef getName() const override {
-return "HTMLDiagnostics";
-  }
+  StringRef getName() const override { return "HTMLDiagnostics"; }
 
   bool supportsCrossFileDiagnostics() const override {
 return SupportsCrossFileDiagnostics;
   }
 
-  unsigned ProcessMacroPiece(raw_ostream ,
- const PathDiagnosticMacroPiece& P,
+  unsigned ProcessMacroPiece(raw_ostream , const PathDiagnosticMacroPiece ,
  unsigned num);
 
+  unsigned ProcessControlFlowPiece(Rewriter , FileID BugFileID,
+   const PathDiagnosticControlFlowPiece ,
+   unsigned Number);
+
   void HandlePiece(Rewriter , FileID BugFileID, const PathDiagnosticPiece ,
const std::vector , unsigned num,
unsigned max);
 
-  void HighlightRange(Rewriter& R, FileID BugFileID, SourceRange Range,
+  void HighlightRange(Rewriter , FileID BugFileID, SourceRange Range,
   const char *HighlightStart = "",
   const char *HighlightEnd = "");
 
-  void ReportDiag(const PathDiagnostic& D,
-  FilesMade *filesMade);
+  void ReportDiag(const PathDiagnostic , FilesMade *filesMade);
 
   // Generate the full HTML report
-  std::string GenerateHTML(const PathDiagnostic& D, Rewriter ,
-   const SourceManager& SMgr, const PathPieces& path,
+  std::string GenerateHTML(const PathDiagnostic , Rewriter ,
+   const SourceManager , const PathPieces ,
const char *declName);
 
   // Add HTML header/footers to file specified by FID
-  void FinalizeHTML(const PathDiagnostic& D, Rewriter ,
-const SourceManager& SMgr, const PathPieces& path,
+  void FinalizeHTML(const PathDiagnostic , Rewriter ,
+const SourceManager , const PathPieces ,
 FileID FID, const FileEntry *Entry, const char *declName);
 
   // Rewrite the file specified by FID with HTML formatting.
-  void RewriteFile(Rewriter , const PathPieces& path, FileID FID);
+  void RewriteFile(Rewriter , const PathPieces , FileID FID);
 
+  PathGenerationScheme getGenerationScheme() const override {
+return Everything;
+  }
 
 private:
+  void addArrowSVGs(Rewriter , FileID BugFileID, unsigned NumberOfArrows);
+
   /// \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.
+  StringRef generateArrowDrawingJavascript();
+
   /// \return JavaScript for an option to only show relevant lines.
-  std::string showRelevantLinesJavascript(
-

[PATCH] D92639: [analyzer] Add control flow arrows to the analyzer's HTML reports

[Valeriy Savchenko via Phabricator via cfe-commits]

vsavchenko added a comment.

In D92639#2786648 , @ASDenysPetrov 
wrote:

> OK, then. Let's land it!

Can you please take a look at D92928  as well?


Repository:
  rG LLVM Github Monorepo

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D92639/new/

https://reviews.llvm.org/D92639

___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D92639: [analyzer] Add control flow arrows to the analyzer's HTML reports

[Denys Petrov via Phabricator via cfe-commits]

ASDenysPetrov accepted this revision.
ASDenysPetrov added a comment.
This revision is now accepted and ready to land.

OK, then. Let's land it!


Repository:
  rG LLVM Github Monorepo

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D92639/new/

https://reviews.llvm.org/D92639

___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D92639: [analyzer] Add control flow arrows to the analyzer's HTML reports

[Valeriy Savchenko via Phabricator via cfe-commits]

vsavchenko added a comment.

In D92639#2785057 , @ASDenysPetrov 
wrote:

> @vsavchenko How about this?

Yep, thanks for reminding!


Repository:
  rG LLVM Github Monorepo

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D92639/new/

https://reviews.llvm.org/D92639

___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D92639: [analyzer] Add control flow arrows to the analyzer's HTML reports

[Valeriy Savchenko via Phabricator via cfe-commits]

vsavchenko updated this revision to Diff 348479.
vsavchenko added a comment.

Fix IE issue and rebase


Repository:
  rG LLVM Github Monorepo

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D92639/new/

https://reviews.llvm.org/D92639

Files:
  clang/include/clang/Analysis/PathDiagnostic.h
  clang/lib/Rewrite/HTMLRewrite.cpp
  clang/lib/StaticAnalyzer/Core/BugReporter.cpp
  clang/lib/StaticAnalyzer/Core/HTMLDiagnostics.cpp
  clang/test/Analysis/html_diagnostics/control-arrows.cpp

Index: clang/test/Analysis/html_diagnostics/control-arrows.cpp
===
--- /dev/null
+++ clang/test/Analysis/html_diagnostics/control-arrows.cpp
@@ -0,0 +1,27 @@
+// RUN: rm -fR %t
+// RUN: mkdir %t
+// RUN: %clang_analyze_cc1 -analyzer-checker=core \
+// RUN:-analyzer-output=html -o %t -verify %s
+// RUN: cat %t/report-*.html | FileCheck %s
+
+int dereference(int *x) {
+  return *x; // expected-warning{{Dereference of null pointer (loaded from variable 'x')}}
+}
+
+int foobar(bool cond, int *x) {
+  if (cond)
+x = 0;
+  return dereference(x);
+}
+
+// CHECK:  
+// CHECK-NOT:  
+// CHECK:
+// CHECK-NEXT: 
+//
+// Except for arrows we still want to have grey bubbles with control notes.
+// CHECK:  2
+// CHECK-SAME:   Taking true branch
Index: clang/lib/StaticAnalyzer/Core/HTMLDiagnostics.cpp
===
--- clang/lib/StaticAnalyzer/Core/HTMLDiagnostics.cpp
+++ clang/lib/StaticAnalyzer/Core/HTMLDiagnostics.cpp
@@ -27,6 +27,7 @@
 #include "clang/Rewrite/Core/Rewriter.h"
 #include "clang/StaticAnalyzer/Core/PathDiagnosticConsumers.h"
 #include "llvm/ADT/ArrayRef.h"
+#include "llvm/ADT/Sequence.h"
 #include "llvm/ADT/SmallString.h"
 #include "llvm/ADT/StringRef.h"
 #include "llvm/ADT/iterator_range.h"
@@ -77,60 +78,78 @@
   void FlushDiagnosticsImpl(std::vector ,
 FilesMade *filesMade) override;
 
-  StringRef getName() const override {
-return "HTMLDiagnostics";
-  }
+  StringRef getName() const override { return "HTMLDiagnostics"; }
 
   bool supportsCrossFileDiagnostics() const override {
 return SupportsCrossFileDiagnostics;
   }
 
-  unsigned ProcessMacroPiece(raw_ostream ,
- const PathDiagnosticMacroPiece& P,
+  unsigned ProcessMacroPiece(raw_ostream , const PathDiagnosticMacroPiece ,
  unsigned num);
 
+  unsigned ProcessControlFlowPiece(Rewriter , FileID BugFileID,
+   const PathDiagnosticControlFlowPiece ,
+   unsigned Number);
+
   void HandlePiece(Rewriter , FileID BugFileID, const PathDiagnosticPiece ,
const std::vector , unsigned num,
unsigned max);
 
-  void HighlightRange(Rewriter& R, FileID BugFileID, SourceRange Range,
+  void HighlightRange(Rewriter , FileID BugFileID, SourceRange Range,
   const char *HighlightStart = "",
   const char *HighlightEnd = "");
 
-  void ReportDiag(const PathDiagnostic& D,
-  FilesMade *filesMade);
+  void ReportDiag(const PathDiagnostic , FilesMade *filesMade);
 
   // Generate the full HTML report
-  std::string GenerateHTML(const PathDiagnostic& D, Rewriter ,
-   const SourceManager& SMgr, const PathPieces& path,
+  std::string GenerateHTML(const PathDiagnostic , Rewriter ,
+   const SourceManager , const PathPieces ,
const char *declName);
 
   // Add HTML header/footers to file specified by FID
-  void FinalizeHTML(const PathDiagnostic& D, Rewriter ,
-const SourceManager& SMgr, const PathPieces& path,
+  void FinalizeHTML(const PathDiagnostic , Rewriter ,
+const SourceManager , const PathPieces ,
 FileID FID, const FileEntry *Entry, const char *declName);
 
   // Rewrite the file specified by FID with HTML formatting.
-  void RewriteFile(Rewriter , const PathPieces& path, FileID FID);
+  void RewriteFile(Rewriter , const PathPieces , FileID FID);
 
+  PathGenerationScheme getGenerationScheme() const override {
+return Everything;
+  }
 
 private:
+  void addArrowSVGs(Rewriter , FileID BugFileID, unsigned NumberOfArrows);
+
   /// \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.
+  StringRef generateArrowDrawingJavascript();
+
   /// \return JavaScript for an option to only show relevant lines.
-  std::string showRelevantLinesJavascript(
-const PathDiagnostic , const PathPieces );
+  std::string showRelevantLinesJavascript(const PathDiagnostic ,
+  

[PATCH] D92639: [analyzer] Add control flow arrows to the analyzer's HTML reports

[Denys Petrov via Phabricator via cfe-commits]

ASDenysPetrov added a comment.
Herald added a subscriber: manas.

@vsavchenko How about this?


Repository:
  rG LLVM Github Monorepo

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D92639/new/

https://reviews.llvm.org/D92639

___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D92639: [analyzer] Add control flow arrows to the analyzer's HTML reports

[Denys Petrov via Phabricator via cfe-commits]

ASDenysPetrov added a comment.

In D92639#2717134 , @vsavchenko wrote:

> In D92639#2716973 , @ASDenysPetrov 
> wrote:
>
>> @vsavchenko 
>> I make some tests and fixes. Please, consider.
>
> OMG, that's so awesome! Thank you so much!

Never mind :-) Apply the changes and I will approve the patch.


Repository:
  rG LLVM Github Monorepo

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D92639/new/

https://reviews.llvm.org/D92639

___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D92639: [analyzer] Add control flow arrows to the analyzer's HTML reports

[Valeriy Savchenko via Phabricator via cfe-commits]

vsavchenko added a comment.

In D92639#2716973 , @ASDenysPetrov 
wrote:

> @vsavchenko 
> I make some tests and fixes. Please, consider.

OMG, that's so awesome! Thank you so much!


Repository:
  rG LLVM Github Monorepo

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D92639/new/

https://reviews.llvm.org/D92639

___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D92639: [analyzer] Add control flow arrows to the analyzer's HTML reports

[Denys Petrov via Phabricator via cfe-commits]

ASDenysPetrov added a comment.

@vsavchenko 
I make some tests and fixes. Please, consider.




Comment at: clang/lib/StaticAnalyzer/Core/HTMLDiagnostics.cpp:458
   }
   if (event.key == "S") {
 var checked = document.getElementsByName("showCounterexample")[0].checked;

vsavchenko wrote:
> ASDenysPetrov wrote:
> > Seems like this shortcut works only with the capital **S** aka //shift+s//
> > Should we support any **s** state?
> As far as I understand, that matches the original intention because the help 
> message mentions exactly "Shift+S"
> {F15355624}
I've checked again.
When CapsLock is ON, `shift+s` has no effect, but single `s` has. The same vice 
versa.
The real issue is that we check for **capital s** instead of **shift+s**.
This change will handle it correctly. I've checked it on IE and Chrome. It 
works for me.



Comment at: clang/lib/StaticAnalyzer/Core/HTMLDiagnostics.cpp:1275-1278
+left: relative.left + window.scrollX,
+right: relative.right + window.scrollX,
+top: relative.top + window.scrollY,
+bottom: relative.bottom + window.scrollY,

This is the root cause of why I wasn't able to see arrows in IE11.
I found and checked a fix. It works on IE and Chrome. Thanks to 
https://github.com/imgix/drift/issues/33#issue-165784939



Repository:
  rG LLVM Github Monorepo

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D92639/new/

https://reviews.llvm.org/D92639

___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D92639: [analyzer] Add control flow arrows to the analyzer's HTML reports

[Valeriy Savchenko via Phabricator via cfe-commits]

vsavchenko added a comment.
Herald added a subscriber: nullptr.cpp.

In D92639#2505163 , @ASDenysPetrov 
wrote:

> @vsavchenko
>
>> F14534708: report-6ea17d.html 
>
> I checked it in IE. It doesn't draw arrows. Investigate this, please.

Oh, I checked it (using a bunch of weird websites to host html and then to see 
it in IE).  It doesn't show it and I have no idea why.  I don't have easy 
access to IE and I couldn't fix it unfortunately :-(
If you know any good way I can run it on Mac, I'd be happy to test it.




Comment at: clang/lib/StaticAnalyzer/Core/HTMLDiagnostics.cpp:147
+bool isArrowPiece(const PathDiagnosticPiece ) {
+  return isa(P) && P.getString().empty();
+}

ASDenysPetrov wrote:
> Are you sure that **non-arrow** piece **always** has **non-empty** string 
> representation? Can this give us a false positive result?
I haven't seen empty grey bubbles.  If there are pieces like this, it'd be a 
bug in event generation.



Comment at: clang/lib/StaticAnalyzer/Core/HTMLDiagnostics.cpp:458
   }
   if (event.key == "S") {
 var checked = document.getElementsByName("showCounterexample")[0].checked;

ASDenysPetrov wrote:
> Seems like this shortcut works only with the capital **S** aka //shift+s//
> Should we support any **s** state?
As far as I understand, that matches the original intention because the help 
message mentions exactly "Shift+S"
{F15355624}


Repository:
  rG LLVM Github Monorepo

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D92639/new/

https://reviews.llvm.org/D92639

___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D92639: [analyzer] Add control flow arrows to the analyzer's HTML reports

[Denys Petrov via Phabricator via cfe-commits]

ASDenysPetrov added a comment.

@vsavchenko

> F14534708: report-6ea17d.html 

I checked it in IE. It doesn't draw arrows. Investigate this, please.




Comment at: clang/lib/StaticAnalyzer/Core/HTMLDiagnostics.cpp:147
+bool isArrowPiece(const PathDiagnosticPiece ) {
+  return isa(P) && P.getString().empty();
+}

Are you sure that **non-arrow** piece **always** has **non-empty** string 
representation? Can this give us a false positive result?



Comment at: clang/lib/StaticAnalyzer/Core/HTMLDiagnostics.cpp:458
   }
   if (event.key == "S") {
 var checked = document.getElementsByName("showCounterexample")[0].checked;

Seems like this shortcut works only with the capital **S** aka //shift+s//
Should we support any **s** state?


Repository:
  rG LLVM Github Monorepo

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D92639/new/

https://reviews.llvm.org/D92639

___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D92639: [analyzer] Add control flow arrows to the analyzer's HTML reports

[Valeriy Savchenko via Phabricator via cfe-commits]

vsavchenko updated this revision to Diff 310481.
vsavchenko added a comment.

Replace let with const


Repository:
  rG LLVM Github Monorepo

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D92639/new/

https://reviews.llvm.org/D92639

Files:
  clang/include/clang/Analysis/PathDiagnostic.h
  clang/lib/Rewrite/HTMLRewrite.cpp
  clang/lib/StaticAnalyzer/Core/BugReporter.cpp
  clang/lib/StaticAnalyzer/Core/HTMLDiagnostics.cpp
  clang/test/Analysis/html_diagnostics/control-arrows.cpp

Index: clang/test/Analysis/html_diagnostics/control-arrows.cpp
===
--- /dev/null
+++ clang/test/Analysis/html_diagnostics/control-arrows.cpp
@@ -0,0 +1,27 @@
+// RUN: rm -fR %t
+// RUN: mkdir %t
+// RUN: %clang_analyze_cc1 -analyzer-checker=core \
+// RUN:-analyzer-output=html -o %t -verify %s
+// RUN: cat %t/report-*.html | FileCheck %s
+
+int dereference(int *x) {
+  return *x; // expected-warning{{Dereference of null pointer (loaded from variable 'x')}}
+}
+
+int foobar(bool cond, int *x) {
+  if (cond)
+x = 0;
+  return dereference(x);
+}
+
+// CHECK:  
+// CHECK-NOT:  
+// CHECK:
+// CHECK-NEXT: 
+//
+// Except for arrows we still want to have grey bubbles with control notes.
+// CHECK:  2
+// CHECK-SAME:   Taking true branch
Index: clang/lib/StaticAnalyzer/Core/HTMLDiagnostics.cpp
===
--- clang/lib/StaticAnalyzer/Core/HTMLDiagnostics.cpp
+++ clang/lib/StaticAnalyzer/Core/HTMLDiagnostics.cpp
@@ -10,11 +10,11 @@
 //
 //===--===//
 
-#include "clang/Analysis/IssueHash.h"
-#include "clang/Analysis/PathDiagnostic.h"
 #include "clang/AST/Decl.h"
 #include "clang/AST/DeclBase.h"
 #include "clang/AST/Stmt.h"
+#include "clang/Analysis/IssueHash.h"
+#include "clang/Analysis/PathDiagnostic.h"
 #include "clang/Basic/FileManager.h"
 #include "clang/Basic/LLVM.h"
 #include "clang/Basic/SourceLocation.h"
@@ -26,6 +26,7 @@
 #include "clang/Rewrite/Core/Rewriter.h"
 #include "clang/StaticAnalyzer/Core/PathDiagnosticConsumers.h"
 #include "llvm/ADT/ArrayRef.h"
+#include "llvm/ADT/Sequence.h"
 #include "llvm/ADT/SmallString.h"
 #include "llvm/ADT/StringRef.h"
 #include "llvm/ADT/iterator_range.h"
@@ -88,6 +89,10 @@
  const PathDiagnosticMacroPiece& P,
  unsigned num);
 
+  unsigned ProcessControlFlowPiece(Rewriter , FileID BugFileID,
+   const PathDiagnosticControlFlowPiece ,
+   unsigned Number);
+
   void HandlePiece(Rewriter , FileID BugFileID, const PathDiagnosticPiece ,
const std::vector , unsigned num,
unsigned max);
@@ -112,14 +117,22 @@
   // Rewrite the file specified by FID with HTML formatting.
   void RewriteFile(Rewriter , const PathPieces& path, FileID FID);
 
+  PathGenerationScheme getGenerationScheme() const override {
+return Everything;
+  }
 
 private:
+  void addArrowSVGs(Rewriter , FileID BugFileID, unsigned NumberOfArrows);
+
   /// \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.
+  StringRef generateArrowDrawingJavascript();
+
   /// \return JavaScript for an option to only show relevant lines.
   std::string showRelevantLinesJavascript(
 const PathDiagnostic , const PathPieces );
@@ -130,6 +143,17 @@
 llvm::raw_string_ostream );
 };
 
+bool isArrowPiece(const PathDiagnosticPiece ) {
+  return isa(P) && P.getString().empty();
+}
+
+unsigned getPathSizeWithoutArrows(const PathPieces ) {
+  unsigned TotalPieces = Path.size();
+  unsigned TotalArrowPieces = llvm::count_if(
+  Path, [](const PathDiagnosticPieceRef ) { return isArrowPiece(*P); });
+  return TotalPieces - TotalArrowPieces;
+}
+
 } // namespace
 
 void ento::createHTMLDiagnosticConsumer(
@@ -434,7 +458,7 @@
   if (event.key == "S") {
 var checked = document.getElementsByName("showCounterexample")[0].checked;
 filterCounterexample(!checked);
-document.getElementsByName("showCounterexample")[0].checked = !checked;
+document.getElementsByName("showCounterexample")[0].click();
   } else {
 return;
   }
@@ -454,6 +478,11 @@
 
Show only relevant lines
 
+
+
+   Show control flow arrows
+
 
 )<<<";
 
@@ -482,6 +511,9 @@
   R.InsertTextBefore(SMgr.getLocForStartOfFile(FID),
  generateKeyboardNavigationJavascript());
 
+  R.InsertTextBefore(SMgr.getLocForStartOfFile(FID),
+ generateArrowDrawingJavascript());
+
   // Checkbox and javascript for filtering the output to the counterexample.
   

[PATCH] D92639: [analyzer] Add control flow arrows to the analyzer's HTML reports

[Valeriy Savchenko via Phabricator via cfe-commits]

vsavchenko updated this revision to Diff 309497.
vsavchenko added a comment.

Fix incorrect comment


Repository:
  rG LLVM Github Monorepo

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D92639/new/

https://reviews.llvm.org/D92639

Files:
  clang/include/clang/Analysis/PathDiagnostic.h
  clang/lib/Rewrite/HTMLRewrite.cpp
  clang/lib/StaticAnalyzer/Core/BugReporter.cpp
  clang/lib/StaticAnalyzer/Core/HTMLDiagnostics.cpp
  clang/test/Analysis/html_diagnostics/control-arrows.cpp

Index: clang/test/Analysis/html_diagnostics/control-arrows.cpp
===
--- /dev/null
+++ clang/test/Analysis/html_diagnostics/control-arrows.cpp
@@ -0,0 +1,27 @@
+// RUN: rm -fR %t
+// RUN: mkdir %t
+// RUN: %clang_analyze_cc1 -analyzer-checker=core \
+// RUN:-analyzer-output=html -o %t -verify %s
+// RUN: cat %t/report-*.html | FileCheck %s
+
+int dereference(int *x) {
+  return *x; // expected-warning{{Dereference of null pointer (loaded from variable 'x')}}
+}
+
+int foobar(bool cond, int *x) {
+  if (cond)
+x = 0;
+  return dereference(x);
+}
+
+// CHECK:  
+// CHECK-NOT:  
+// CHECK:
+// CHECK-NEXT: 
+//
+// Except for arrows we still want to have grey bubbles with control notes.
+// CHECK:  2
+// CHECK-SAME:   Taking true branch
Index: clang/lib/StaticAnalyzer/Core/HTMLDiagnostics.cpp
===
--- clang/lib/StaticAnalyzer/Core/HTMLDiagnostics.cpp
+++ clang/lib/StaticAnalyzer/Core/HTMLDiagnostics.cpp
@@ -10,11 +10,11 @@
 //
 //===--===//
 
-#include "clang/Analysis/IssueHash.h"
-#include "clang/Analysis/PathDiagnostic.h"
 #include "clang/AST/Decl.h"
 #include "clang/AST/DeclBase.h"
 #include "clang/AST/Stmt.h"
+#include "clang/Analysis/IssueHash.h"
+#include "clang/Analysis/PathDiagnostic.h"
 #include "clang/Basic/FileManager.h"
 #include "clang/Basic/LLVM.h"
 #include "clang/Basic/SourceLocation.h"
@@ -26,6 +26,7 @@
 #include "clang/Rewrite/Core/Rewriter.h"
 #include "clang/StaticAnalyzer/Core/PathDiagnosticConsumers.h"
 #include "llvm/ADT/ArrayRef.h"
+#include "llvm/ADT/Sequence.h"
 #include "llvm/ADT/SmallString.h"
 #include "llvm/ADT/StringRef.h"
 #include "llvm/ADT/iterator_range.h"
@@ -88,6 +89,10 @@
  const PathDiagnosticMacroPiece& P,
  unsigned num);
 
+  unsigned ProcessControlFlowPiece(Rewriter , FileID BugFileID,
+   const PathDiagnosticControlFlowPiece ,
+   unsigned Number);
+
   void HandlePiece(Rewriter , FileID BugFileID, const PathDiagnosticPiece ,
const std::vector , unsigned num,
unsigned max);
@@ -112,14 +117,22 @@
   // Rewrite the file specified by FID with HTML formatting.
   void RewriteFile(Rewriter , const PathPieces& path, FileID FID);
 
+  PathGenerationScheme getGenerationScheme() const override {
+return Everything;
+  }
 
 private:
+  void addArrowSVGs(Rewriter , FileID BugFileID, unsigned NumberOfArrows);
+
   /// \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.
+  StringRef generateArrowDrawingJavascript();
+
   /// \return JavaScript for an option to only show relevant lines.
   std::string showRelevantLinesJavascript(
 const PathDiagnostic , const PathPieces );
@@ -130,6 +143,17 @@
 llvm::raw_string_ostream );
 };
 
+bool isArrowPiece(const PathDiagnosticPiece ) {
+  return isa(P) && P.getString().empty();
+}
+
+unsigned getPathSizeWithoutArrows(const PathPieces ) {
+  unsigned TotalPieces = Path.size();
+  unsigned TotalArrowPieces = llvm::count_if(
+  Path, [](const PathDiagnosticPieceRef ) { return isArrowPiece(*P); });
+  return TotalPieces - TotalArrowPieces;
+}
+
 } // namespace
 
 void ento::createHTMLDiagnosticConsumer(
@@ -434,7 +458,7 @@
   if (event.key == "S") {
 var checked = document.getElementsByName("showCounterexample")[0].checked;
 filterCounterexample(!checked);
-document.getElementsByName("showCounterexample")[0].checked = !checked;
+document.getElementsByName("showCounterexample")[0].click();
   } else {
 return;
   }
@@ -454,6 +478,11 @@
 
Show only relevant lines
 
+
+
+   Show control flow arrows
+
 
 )<<<";
 
@@ -482,6 +511,9 @@
   R.InsertTextBefore(SMgr.getLocForStartOfFile(FID),
  generateKeyboardNavigationJavascript());
 
+  R.InsertTextBefore(SMgr.getLocForStartOfFile(FID),
+ generateArrowDrawingJavascript());
+
   // Checkbox and javascript for filtering the output to the counterexample.
   

[PATCH] D92639: [analyzer] Add control flow arrows to the analyzer's HTML reports

[Valeriy Savchenko via Phabricator via cfe-commits]

vsavchenko added a comment.

In D92639#2433303 , @OikawaKirie wrote:

> It is really a good idea!

Thanks 

> The operations that would not leave an event in the report are now clearly 
> printed.
>
> But there are three arrows that confuse me in the example report: the 
> assignment `x = 0` (x -> 0 -> x), the function call `dereference(x)` (x -> 
> dereference), and the return statement `return *x` (int -> *x). I know the 
> arrow is based on the evaluation order of the engine. But from the view of a 
> user, I think these arrows are confusing to some extent.
>
> For the first two, I think it would be better to point just the statement 
> (maybe a `CFGElement`) without inner arrows (x -> 0 -> x and x -> 
> dereference), or point to the location of the operator itself rather than the 
> BeginLoc (e.g. x -> 0 -> =). For the third one, an arrow from the function 
> name to the first `CFGElement` looks good to me. And an arrow from the 
> returned expr to the return type or to a special mark (e.g. ⬅️) can also be 
> added, together with function calls (an arrow from the callstmt to a special 
> mark, e.g. ➡️).

Sorry, for the confusion.  I did not come up with the idea of arrows and 
neither did I chose which tokens are connected by arrows.  It is an existing 
feature, and it existed for quite a long time.  The analyzer can produce 
`plist` files that contain all these locations, `plist` files are further used 
by Xcode to draw arrows.  You can see a very old example on our website: 
https://clang-analyzer.llvm.org
Essentially I took this existing information and used it in the HTML report 
generation as well.  The biggest chunk of this commit is the algorithm for 
drawing SVG curves.

> By the way, what do you think about adding arrows for data flows of specific 
> symbolic values in the future?

I'm open for many ideas in the ways how we can improve our HTML reports!  I 
thought of a popup when you hover over an arrow showing values/constraints for 
symbols actively involved in the report.


Repository:
  rG LLVM Github Monorepo

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D92639/new/

https://reviews.llvm.org/D92639

___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D92639: [analyzer] Add control flow arrows to the analyzer's HTML reports

[Ella Ma via Phabricator via cfe-commits]

OikawaKirie added a comment.

It is really a good idea!

The operations that would not leave an event in the report are now clearly 
printed.

But there are three arrows that confuse me in the example report: the 
assignment `x = 0` (x -> 0 -> x), the function call `dereference(x)` (x -> 
dereference), and the return statement `return *x` (int -> *x). I know the 
arrow is based on the evaluation order of the engine. But from the view of a 
user, I think these arrows are confusing to some extent.

For the first two, I think it would be better to point just the statement 
(maybe a `CFGElement`) without inner arrows (x -> 0 -> x and x -> dereference), 
or point to the location of the operator itself rather than the BeginLoc (e.g. 
x -> 0 -> =). For the third one, an arrow from the function name to the first 
`CFGElement` looks good to me. And an arrow from the returned expr to the 
return type or to a special mark (e.g. ⬅️) can also be added, together with 
function calls (an arrow from the callstmt to a special mark, e.g. ➡️).

By the way, what do you think about adding arrows for data flows of specific 
symbolic values in the future?


Repository:
  rG LLVM Github Monorepo

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D92639/new/

https://reviews.llvm.org/D92639

___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D92639: [analyzer] Add control flow arrows to the analyzer's HTML reports

[Valeriy Savchenko via Phabricator via cfe-commits]

vsavchenko added a comment.

Here is how it looks for that test file:
F14534708: report-6ea17d.html 


Repository:
  rG LLVM Github Monorepo

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D92639/new/

https://reviews.llvm.org/D92639

___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D92639: [analyzer] Add control flow arrows to the analyzer's HTML reports

[Valeriy Savchenko via Phabricator via cfe-commits]

vsavchenko created this revision.
vsavchenko added reviewers: NoQ, xazax.hun, Szelethus, steakhal, martong, 
ASDenysPetrov.
Herald added subscribers: Charusso, dkrupp, donat.nagy, mikhail.ramalho, 
a.sidorin, rnkovacs, szepet, baloghadamsoftware.
vsavchenko requested review of this revision.
Herald added a project: clang.
Herald added a subscriber: cfe-commits.

This commit adds a very first version of this feature.
It is off by default and has to be turned on by checking the
corresponding box.  For this reason, HTML reports still keep
control notes (aka grey bubbles).

Further on, we plan on attaching arrows to events and having all arrows
not related to a currently selected event barely visible.  This will
help with reports where control flow goes back and forth (eg in loops).
Right now, it can get pretty crammed with all the arrows.


Repository:
  rG LLVM Github Monorepo

https://reviews.llvm.org/D92639

Files:
  clang/include/clang/Analysis/PathDiagnostic.h
  clang/lib/Rewrite/HTMLRewrite.cpp
  clang/lib/StaticAnalyzer/Core/BugReporter.cpp
  clang/lib/StaticAnalyzer/Core/HTMLDiagnostics.cpp
  clang/test/Analysis/html_diagnostics/control-arrows.cpp

Index: clang/test/Analysis/html_diagnostics/control-arrows.cpp
===
--- /dev/null
+++ clang/test/Analysis/html_diagnostics/control-arrows.cpp
@@ -0,0 +1,27 @@
+// RUN: rm -fR %t
+// RUN: mkdir %t
+// RUN: %clang_analyze_cc1 -analyzer-checker=core \
+// RUN:-analyzer-output=html -o %t -verify %s
+// RUN: cat %t/report-*.html | FileCheck %s
+
+int dereference(int *x) {
+  return *x; // expected-warning{{Dereference of null pointer (loaded from variable 'x')}}
+}
+
+int foobar(bool cond, int *x) {
+  if (cond)
+x = 0;
+  return dereference(x);
+}
+
+// CHECK:  
+// CHECK-NOT:  
+// CHECK:
+// CHECK-NEXT: 
+//
+// Except for arrows we still want to have grey bubbles with control notes.
+// CHECK:  2
+// CHECK-SAME:   Taking true branch
Index: clang/lib/StaticAnalyzer/Core/HTMLDiagnostics.cpp
===
--- clang/lib/StaticAnalyzer/Core/HTMLDiagnostics.cpp
+++ clang/lib/StaticAnalyzer/Core/HTMLDiagnostics.cpp
@@ -10,11 +10,11 @@
 //
 //===--===//
 
-#include "clang/Analysis/IssueHash.h"
-#include "clang/Analysis/PathDiagnostic.h"
 #include "clang/AST/Decl.h"
 #include "clang/AST/DeclBase.h"
 #include "clang/AST/Stmt.h"
+#include "clang/Analysis/IssueHash.h"
+#include "clang/Analysis/PathDiagnostic.h"
 #include "clang/Basic/FileManager.h"
 #include "clang/Basic/LLVM.h"
 #include "clang/Basic/SourceLocation.h"
@@ -26,6 +26,7 @@
 #include "clang/Rewrite/Core/Rewriter.h"
 #include "clang/StaticAnalyzer/Core/PathDiagnosticConsumers.h"
 #include "llvm/ADT/ArrayRef.h"
+#include "llvm/ADT/Sequence.h"
 #include "llvm/ADT/SmallString.h"
 #include "llvm/ADT/StringRef.h"
 #include "llvm/ADT/iterator_range.h"
@@ -88,6 +89,10 @@
  const PathDiagnosticMacroPiece& P,
  unsigned num);
 
+  unsigned ProcessControlFlowPiece(Rewriter , FileID BugFileID,
+   const PathDiagnosticControlFlowPiece ,
+   unsigned Number);
+
   void HandlePiece(Rewriter , FileID BugFileID, const PathDiagnosticPiece ,
const std::vector , unsigned num,
unsigned max);
@@ -112,14 +117,22 @@
   // Rewrite the file specified by FID with HTML formatting.
   void RewriteFile(Rewriter , const PathPieces& path, FileID FID);
 
+  PathGenerationScheme getGenerationScheme() const override {
+return Everything;
+  }
 
 private:
+  void addArrowSVGs(Rewriter , FileID BugFileID, unsigned NumberOfArrows);
+
   /// \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.
+  StringRef generateArrowDrawingJavascript();
+
   /// \return JavaScript for an option to only show relevant lines.
   std::string showRelevantLinesJavascript(
 const PathDiagnostic , const PathPieces );
@@ -130,6 +143,17 @@
 llvm::raw_string_ostream );
 };
 
+bool isArrowPiece(const PathDiagnosticPiece ) {
+  return isa(P) && P.getString().empty();
+}
+
+unsigned getPathSizeWithoutArrows(const PathPieces ) {
+  unsigned TotalPieces = Path.size();
+  unsigned TotalArrowPieces = llvm::count_if(
+  Path, [](const PathDiagnosticPieceRef ) { return isArrowPiece(*P); });
+  return TotalPieces - TotalArrowPieces;
+}
+
 } // namespace
 
 void ento::createHTMLDiagnosticConsumer(
@@ -434,7 +458,7 @@
   if (event.key == "S") {
 var checked = document.getElementsByName("showCounterexample")[0].checked;