[PATCH] D30406: [Analyzer] Add support for displaying cross-file diagnostic paths in HTML output

2017-08-03 Thread Devin Coughlin via Phabricator via cfe-commits
This revision was automatically updated to reflect the committed changes.
Closed by commit rL309968: [Analyzer] Add support for displaying cross-file 
diagnostic paths in HTML output (authored by dcoughlin).

Changed prior to commit:
  https://reviews.llvm.org/D30406?vs=102838&id=109601#toc

Repository:
  rL LLVM

https://reviews.llvm.org/D30406

Files:
  cfe/trunk/include/clang/StaticAnalyzer/Core/Analyses.def
  cfe/trunk/lib/Rewrite/HTMLRewrite.cpp
  cfe/trunk/lib/StaticAnalyzer/Core/HTMLDiagnostics.cpp
  cfe/trunk/test/Analysis/diagnostics/diag-cross-file-boundaries.c
  cfe/trunk/test/Analysis/diagnostics/diag-cross-file-boundaries.h
  cfe/trunk/test/Analysis/html-diag-singlefile.c
  cfe/trunk/test/Analysis/html-diag-singlefile.h
  cfe/trunk/test/Analysis/html-diags-analyze-headers.c
  cfe/trunk/test/Analysis/html-diags-analyze-headers.h
  cfe/trunk/test/Analysis/html-diags-multifile.c
  cfe/trunk/test/Analysis/html-diags.c
  cfe/trunk/test/Coverage/html-diagnostics.c
  cfe/trunk/test/Coverage/html-multifile-diagnostics.c
  cfe/trunk/test/Coverage/html-multifile-diagnostics.h
  cfe/trunk/www/analyzer/open_projects.html

Index: cfe/trunk/lib/StaticAnalyzer/Core/HTMLDiagnostics.cpp
===
--- cfe/trunk/lib/StaticAnalyzer/Core/HTMLDiagnostics.cpp
+++ cfe/trunk/lib/StaticAnalyzer/Core/HTMLDiagnostics.cpp
@@ -44,8 +44,12 @@
   bool createdDir, noDir;
   const Preprocessor &PP;
   AnalyzerOptions &AnalyzerOpts;
+  const bool SupportsCrossFileDiagnostics;
 public:
-  HTMLDiagnostics(AnalyzerOptions &AnalyzerOpts, const std::string& prefix, const Preprocessor &pp);
+  HTMLDiagnostics(AnalyzerOptions &AnalyzerOpts,
+  const std::string& prefix,
+  const Preprocessor &pp,
+  bool supportsMultipleFiles);
 
   ~HTMLDiagnostics() override { FlushDiagnostics(nullptr); }
 
@@ -56,6 +60,10 @@
 return "HTMLDiagnostics";
   }
 
+  bool supportsCrossFileDiagnostics() const override {
+return SupportsCrossFileDiagnostics;
+  }
+
   unsigned ProcessMacroPiece(raw_ostream &os,
  const PathDiagnosticMacroPiece& P,
  unsigned num);
@@ -69,21 +77,47 @@
 
   void ReportDiag(const PathDiagnostic& D,
   FilesMade *filesMade);
+
+  // Generate the full HTML report
+  std::string GenerateHTML(const PathDiagnostic& D, Rewriter &R,
+   const SourceManager& SMgr, const PathPieces& path,
+   const char *declName);
+
+  // Add HTML header/footers to file specified by FID
+  void FinalizeHTML(const PathDiagnostic& D, Rewriter &R,
+const SourceManager& SMgr, const PathPieces& path,
+FileID FID, const FileEntry *Entry, const char *declName);
+
+  // Rewrite the file specified by FID with HTML formatting.
+  void RewriteFile(Rewriter &R, const SourceManager& SMgr,
+   const PathPieces& path, FileID FID);
 };
 
 } // end anonymous namespace
 
 HTMLDiagnostics::HTMLDiagnostics(AnalyzerOptions &AnalyzerOpts,
  const std::string& prefix,
- const Preprocessor &pp)
-: Directory(prefix), createdDir(false), noDir(false), PP(pp), AnalyzerOpts(AnalyzerOpts) {
-}
+ const Preprocessor &pp,
+ bool supportsMultipleFiles)
+: Directory(prefix),
+  createdDir(false),
+  noDir(false),
+  PP(pp),
+  AnalyzerOpts(AnalyzerOpts),
+  SupportsCrossFileDiagnostics(supportsMultipleFiles) {}
 
 void ento::createHTMLDiagnosticConsumer(AnalyzerOptions &AnalyzerOpts,
 PathDiagnosticConsumers &C,
 const std::string& prefix,
 const Preprocessor &PP) {
-  C.push_back(new HTMLDiagnostics(AnalyzerOpts, prefix, PP));
+  C.push_back(new HTMLDiagnostics(AnalyzerOpts, prefix, PP, true));
+}
+
+void ento::createHTMLSingleFileDiagnosticConsumer(AnalyzerOptions &AnalyzerOpts,
+  PathDiagnosticConsumers &C,
+  const std::string& prefix,
+  const Preprocessor &PP) {
+  C.push_back(new HTMLDiagnostics(AnalyzerOpts, prefix, PP, false));
 }
 
 //===--===//
@@ -121,24 +155,24 @@
   // First flatten out the entire path to make it easier to use.
   PathPieces path = D.path.flatten(/*ShouldFlattenMacros=*/false);
 
-  // The path as already been prechecked that all parts of the path are
-  // from the same file and that it is non-empty.
-  const SourceManager &SMgr = path.front()->getLocation().getManager();
+  // The path as already been prechecked that the path is non-empty.
   assert(!path.empty());
-  FileID FID =
-pa

[PATCH] D30406: [Analyzer] Add support for displaying cross-file diagnostic paths in HTML output

2017-06-15 Thread Vlad Tsyrklevich via Phabricator via cfe-commits
vlad.tsyrklevich updated this revision to Diff 102765.
vlad.tsyrklevich added a comment.
Herald added a subscriber: xazax.hun.

Updates to cleanly rebase on a recent clang master. Ran all tests with ASan to 
ensure I avoided a broken build post-merge as with  
https://reviews.llvm.org/D30909


https://reviews.llvm.org/D30406

Files:
  include/clang/StaticAnalyzer/Core/Analyses.def
  lib/Rewrite/HTMLRewrite.cpp
  lib/StaticAnalyzer/Core/HTMLDiagnostics.cpp
  test/Analysis/diagnostics/diag-cross-file-boundaries.c
  test/Analysis/diagnostics/diag-cross-file-boundaries.h
  test/Analysis/html-diag-singlefile.c
  test/Analysis/html-diag-singlefile.h
  test/Analysis/html-diags-analyze-headers.c
  test/Analysis/html-diags-analyze-headers.h
  test/Analysis/html-diags-multifile.c
  test/Analysis/html-diags.c
  test/Coverage/html-diagnostics.c
  test/Coverage/html-multifile-diagnostics.c
  test/Coverage/html-multifile-diagnostics.h
  www/analyzer/open_projects.html

Index: www/analyzer/open_projects.html
===
--- www/analyzer/open_projects.html
+++ www/analyzer/open_projects.html
@@ -107,13 +107,6 @@
 
   Bug Reporting 
   
-Add support for displaying cross-file diagnostic paths in HTML output
-(used by scan-build).
-Currently scan-build output does not display reports that span 
-multiple files. The main problem is that we do not have a good format to
-display such paths in HTML output. (Difficulty: Medium) 
-
-
 Refactor path diagnostic generation in http://clang.llvm.org/doxygen/BugReporter_8cpp_source.html";>BugReporter.cpp.
 It would be great to have more code reuse between "Minimal" and 
 "Extensive" PathDiagnostic generation algorithms. One idea is to create an 
Index: test/Coverage/html-multifile-diagnostics.h
===
--- /dev/null
+++ test/Coverage/html-multifile-diagnostics.h
@@ -0,0 +1,3 @@
+void f1(int *ptr) {
+  *ptr = 0;
+}
Index: test/Coverage/html-multifile-diagnostics.c
===
--- /dev/null
+++ test/Coverage/html-multifile-diagnostics.c
@@ -0,0 +1,21 @@
+// RUN: rm -rf %t
+// RUN: %clang_cc1 -analyze -analyzer-output=html -analyzer-checker=core -o %t %s
+// RUN: find %t -name "*.html" -exec cat "{}" ";" | FileCheck %s
+
+// REQUIRES: staticanalyzer
+
+// CHECK: Annotated Source Code
+
+// Make sure it's generated as multi-file HTML output
+// CHECK: {{.*}}html-multifile-diagnostics.c
+// CHECK: {{.*}}html-multifile-diagnostics.h
+
+// Without tweaking expr, the expr would hit to the line below
+// emitted to the output as comment.
+// CHECK: {{[D]ereference of null pointer}}
+
+#include "html-multifile-diagnostics.h"
+
+void f0() {
+  f1((int*)0);
+}
Index: test/Coverage/html-diagnostics.c
===
--- test/Coverage/html-diagnostics.c
+++ test/Coverage/html-diagnostics.c
@@ -1,11 +1,18 @@
 // RUN: rm -rf %t
 // RUN: %clang_cc1 -analyze -analyzer-output=html -analyzer-checker=core -o %t %s
 // RUN: find %t -name "*.html" -exec cat "{}" ";" | FileCheck %s
+//
+// RUN: rm -rf %t
+// RUN: %clang_cc1 -analyze -analyzer-output=html-single-file -analyzer-checker=core -o %t %s
+// RUN: find %t -name "*.html" -exec cat "{}" ";" | FileCheck %s
 
 // REQUIRES: staticanalyzer
 
 // CHECK: Annotated Source Code
 
+// Make sure it's not generated as a multi-file HTML output
+// CHECK-NOT: {{.*}}
+
 // Without tweaking expr, the expr would hit to the line below
 // emitted to the output as comment.
 // CHECK: {{[D]ereference of null pointer}}
Index: test/Analysis/html-diags.c
===
--- test/Analysis/html-diags.c
+++ test/Analysis/html-diags.c
@@ -3,6 +3,12 @@
 // RUN: %clang_analyze_cc1 -analyzer-output=html -analyzer-checker=core -o %T/dir %s
 // RUN: ls %T/dir | grep report
 
+// D30406: Test new html-single-file output
+// RUN: rm -fR %T/dir
+// RUN: mkdir %T/dir
+// RUN: %clang_cc1 -analyze -analyzer-output=html-single-file -analyzer-checker=core -o %T/dir %s
+// RUN: ls %T/dir | grep report
+
 // PR16547: Test relative paths
 // RUN: cd %T/dir
 // RUN: %clang_analyze_cc1 -analyzer-output=html -analyzer-checker=core -o testrelative %s
Index: test/Analysis/html-diags-multifile.c
===
--- test/Analysis/html-diags-multifile.c
+++ test/Analysis/html-diags-multifile.c
@@ -1,10 +1,9 @@
 // RUN: mkdir -p %t.dir
 // RUN: %clang_analyze_cc1 -analyzer-output=html -analyzer-checker=core -o %t.dir %s
-// RUN: ls %t.dir | not grep report
+// RUN: ls %t.dir | grep report
 // RUN: rm -fR %t.dir
 
-// This tests that we do not currently emit HTML diagnostics for reports that
-// cross file boundaries.
+// This tests that we emit HTML diagnostics for reports that cross file boundaries.
 
 #include "html-diags-multifile.h"
 
Ind

[PATCH] D30406: [Analyzer] Add support for displaying cross-file diagnostic paths in HTML output

2017-06-16 Thread Vlad Tsyrklevich via Phabricator via cfe-commits
vlad.tsyrklevich updated this revision to Diff 102838.
vlad.tsyrklevich marked an inline comment as done.
vlad.tsyrklevich added a comment.

After reviewing this patch again last night I:

1. Updated some "%clang_cc1 -analyze" calls with  "%clang_analyze_cc1" due to 
2cfd901321423a96edd8513afc7c7c2bb0d18b2e
2. Isolated HTML generation into a separate function to simplify 
`HTMLDiagnostics::ReportDiag` even further


https://reviews.llvm.org/D30406

Files:
  include/clang/StaticAnalyzer/Core/Analyses.def
  lib/Rewrite/HTMLRewrite.cpp
  lib/StaticAnalyzer/Core/HTMLDiagnostics.cpp
  test/Analysis/diagnostics/diag-cross-file-boundaries.c
  test/Analysis/diagnostics/diag-cross-file-boundaries.h
  test/Analysis/html-diag-singlefile.c
  test/Analysis/html-diag-singlefile.h
  test/Analysis/html-diags-analyze-headers.c
  test/Analysis/html-diags-analyze-headers.h
  test/Analysis/html-diags-multifile.c
  test/Analysis/html-diags.c
  test/Coverage/html-diagnostics.c
  test/Coverage/html-multifile-diagnostics.c
  test/Coverage/html-multifile-diagnostics.h
  www/analyzer/open_projects.html

Index: www/analyzer/open_projects.html
===
--- www/analyzer/open_projects.html
+++ www/analyzer/open_projects.html
@@ -107,13 +107,6 @@
 
   Bug Reporting 
   
-Add support for displaying cross-file diagnostic paths in HTML output
-(used by scan-build).
-Currently scan-build output does not display reports that span 
-multiple files. The main problem is that we do not have a good format to
-display such paths in HTML output. (Difficulty: Medium) 
-
-
 Refactor path diagnostic generation in http://clang.llvm.org/doxygen/BugReporter_8cpp_source.html";>BugReporter.cpp.
 It would be great to have more code reuse between "Minimal" and 
 "Extensive" PathDiagnostic generation algorithms. One idea is to create an 
Index: test/Coverage/html-multifile-diagnostics.h
===
--- /dev/null
+++ test/Coverage/html-multifile-diagnostics.h
@@ -0,0 +1,3 @@
+void f1(int *ptr) {
+  *ptr = 0;
+}
Index: test/Coverage/html-multifile-diagnostics.c
===
--- /dev/null
+++ test/Coverage/html-multifile-diagnostics.c
@@ -0,0 +1,21 @@
+// RUN: rm -rf %t
+// RUN: %clang_cc1 -analyze -analyzer-output=html -analyzer-checker=core -o %t %s
+// RUN: find %t -name "*.html" -exec cat "{}" ";" | FileCheck %s
+
+// REQUIRES: staticanalyzer
+
+// CHECK: Annotated Source Code
+
+// Make sure it's generated as multi-file HTML output
+// CHECK: {{.*}}html-multifile-diagnostics.c
+// CHECK: {{.*}}html-multifile-diagnostics.h
+
+// Without tweaking expr, the expr would hit to the line below
+// emitted to the output as comment.
+// CHECK: {{[D]ereference of null pointer}}
+
+#include "html-multifile-diagnostics.h"
+
+void f0() {
+  f1((int*)0);
+}
Index: test/Coverage/html-diagnostics.c
===
--- test/Coverage/html-diagnostics.c
+++ test/Coverage/html-diagnostics.c
@@ -1,11 +1,18 @@
 // RUN: rm -rf %t
 // RUN: %clang_cc1 -analyze -analyzer-output=html -analyzer-checker=core -o %t %s
 // RUN: find %t -name "*.html" -exec cat "{}" ";" | FileCheck %s
+//
+// RUN: rm -rf %t
+// RUN: %clang_cc1 -analyze -analyzer-output=html-single-file -analyzer-checker=core -o %t %s
+// RUN: find %t -name "*.html" -exec cat "{}" ";" | FileCheck %s
 
 // REQUIRES: staticanalyzer
 
 // CHECK: Annotated Source Code
 
+// Make sure it's not generated as a multi-file HTML output
+// CHECK-NOT: {{.*}}
+
 // Without tweaking expr, the expr would hit to the line below
 // emitted to the output as comment.
 // CHECK: {{[D]ereference of null pointer}}
Index: test/Analysis/html-diags.c
===
--- test/Analysis/html-diags.c
+++ test/Analysis/html-diags.c
@@ -3,6 +3,12 @@
 // RUN: %clang_analyze_cc1 -analyzer-output=html -analyzer-checker=core -o %T/dir %s
 // RUN: ls %T/dir | grep report
 
+// D30406: Test new html-single-file output
+// RUN: rm -fR %T/dir
+// RUN: mkdir %T/dir
+// RUN: %clang_analyze_cc1 -analyzer-output=html-single-file -analyzer-checker=core -o %T/dir %s
+// RUN: ls %T/dir | grep report
+
 // PR16547: Test relative paths
 // RUN: cd %T/dir
 // RUN: %clang_analyze_cc1 -analyzer-output=html -analyzer-checker=core -o testrelative %s
Index: test/Analysis/html-diags-multifile.c
===
--- test/Analysis/html-diags-multifile.c
+++ test/Analysis/html-diags-multifile.c
@@ -1,10 +1,9 @@
 // RUN: mkdir -p %t.dir
 // RUN: %clang_analyze_cc1 -analyzer-output=html -analyzer-checker=core -o %t.dir %s
-// RUN: ls %t.dir | not grep report
+// RUN: ls %t.dir | grep report
 // RUN: rm -fR %t.dir
 
-// This tests that we do not currently emit HTML diagnostics for reports that
-// cross file boundaries.
+//

[PATCH] D30406: [Analyzer] Add support for displaying cross-file diagnostic paths in HTML output

2017-06-16 Thread Anna Zaks via Phabricator via cfe-commits
zaks.anna accepted this revision.
zaks.anna added a comment.
This revision is now accepted and ready to land.

LGTM. Thank you!

(Not sure if @NoQ wants to do a final review.)

Do you have commit access or should we commit on your behalf?


https://reviews.llvm.org/D30406



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


[PATCH] D30406: [Analyzer] Add support for displaying cross-file diagnostic paths in HTML output

2017-06-16 Thread Vlad Tsyrklevich via Phabricator via cfe-commits
vlad.tsyrklevich added a comment.

Please commit on my behalf.


https://reviews.llvm.org/D30406



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


[PATCH] D30406: [Analyzer] Add support for displaying cross-file diagnostic paths in HTML output

2017-02-27 Thread Vlad Tsyrklevich via Phabricator via cfe-commits
vlad.tsyrklevich created this revision.

While looking at checker output with https://reviews.llvm.org/D30289 applied I 
realized I was missing results. After some digging I found that it's because 
the Linux kernel makes heavy use of inlined functions included in header files, 
and the resulting diagnostic path now included cross-file diagnostics which are 
unsupported. This change adds support for cross-file diagnostic paths, 
resulting in output that looks like 
https://rawgit.com/vlad902/805a58327e12636fef119cdeb21ad639/raw/48f4183152a15e3e0f6fddfdc4edeb75c5a62295/report.html
 If the diagnostic path is not cross-file, there is no change in the output.


https://reviews.llvm.org/D30406

Files:
  lib/Rewrite/HTMLRewrite.cpp
  lib/StaticAnalyzer/Core/HTMLDiagnostics.cpp
  test/Analysis/diagnostics/diag-cross-file-boundaries.c
  test/Analysis/diagnostics/diag-cross-file-boundaries.h
  test/Analysis/html-diags-multifile.c
  test/Coverage/html-diagnostics.c
  test/Coverage/html-multifile-diagnostics.c
  test/Coverage/html-multifile-diagnostics.h
  www/analyzer/open_projects.html

Index: www/analyzer/open_projects.html
===
--- www/analyzer/open_projects.html
+++ www/analyzer/open_projects.html
@@ -107,13 +107,6 @@
 
   Bug Reporting 
   
-Add support for displaying cross-file diagnostic paths in HTML output
-(used by scan-build).
-Currently scan-build output does not display reports that span 
-multiple files. The main problem is that we do not have a good format to
-display such paths in HTML output. (Difficulty: Medium) 
-
-
 Refactor path diagnostic generation in http://clang.llvm.org/doxygen/BugReporter_8cpp_source.html";>BugReporter.cpp.
 It would be great to have more code reuse between "Minimal" and 
 "Extensive" PathDiagnostic generation algorithms. One idea is to create an 
Index: test/Coverage/html-multifile-diagnostics.h
===
--- /dev/null
+++ test/Coverage/html-multifile-diagnostics.h
@@ -0,0 +1,3 @@
+void f1(int *ptr) {
+  *ptr = 0;
+}
Index: test/Coverage/html-multifile-diagnostics.c
===
--- /dev/null
+++ test/Coverage/html-multifile-diagnostics.c
@@ -0,0 +1,21 @@
+// RUN: rm -rf %t
+// RUN: %clang_cc1 -analyze -analyzer-output=html -analyzer-checker=core -o %t %s
+// RUN: find %t -name "*.html" -exec cat "{}" ";" | FileCheck %s
+
+// REQUIRES: staticanalyzer
+
+// CHECK: Annotated Source Code
+
+// Make sure it's generated as multi-file HTML output
+// CHECK: {{.*}}html-multifile-diagnostics.c
+// CHECK: {{.*}}html-multifile-diagnostics.h
+
+// Without tweaking expr, the expr would hit to the line below
+// emitted to the output as comment.
+// CHECK: {{[D]ereference of null pointer}}
+
+#include "html-multifile-diagnostics.h"
+
+void f0() {
+  f1((int*)0);
+}
Index: test/Coverage/html-diagnostics.c
===
--- test/Coverage/html-diagnostics.c
+++ test/Coverage/html-diagnostics.c
@@ -6,6 +6,9 @@
 
 // CHECK: Annotated Source Code
 
+// Make sure it's not generated as a multi-file HTML output
+// CHECK-NOT: {{.*}}
+
 // Without tweaking expr, the expr would hit to the line below
 // emitted to the output as comment.
 // CHECK: {{[D]ereference of null pointer}}
Index: test/Analysis/html-diags-multifile.c
===
--- test/Analysis/html-diags-multifile.c
+++ test/Analysis/html-diags-multifile.c
@@ -1,10 +1,9 @@
 // RUN: mkdir -p %t.dir
 // RUN: %clang_cc1 -analyze -analyzer-output=html -analyzer-checker=core -o %t.dir %s
-// RUN: ls %t.dir | not grep report
+// RUN: ls %t.dir | grep report
 // RUN: rm -fR %t.dir
 
-// This tests that we do not currently emit HTML diagnostics for reports that
-// cross file boundaries.
+// This tests that we emit HTML diagnostics for reports that cross file boundaries.
 
 #include "html-diags-multifile.h"
 
Index: test/Analysis/diagnostics/diag-cross-file-boundaries.h
===
--- test/Analysis/diagnostics/diag-cross-file-boundaries.h
+++ /dev/null
@@ -1,4 +0,0 @@
-static void f() {
-  int *p = 0;
-  *p = 1;   // expected-warning{{Dereference of null pointer}}
-}
Index: test/Analysis/diagnostics/diag-cross-file-boundaries.c
===
--- test/Analysis/diagnostics/diag-cross-file-boundaries.c
+++ /dev/null
@@ -1,12 +0,0 @@
-// RUN: %clang_cc1 -analyze -analyzer-checker=core -verify %s
-// RUN: %clang_cc1 -analyze -analyzer-checker=core -analyzer-output=html -o PR12421.html %s 2>&1 | FileCheck %s
-
-// Test for PR12421
-#include "diag-cross-file-boundaries.h"
-
-int main(){
-  f();
-  return 0;
-}
-
-// CHECK: warning: Path diagnostic report is not generated.
Index: lib/StaticAnalyzer/Core/HTMLDiagnostics

[PATCH] D30406: [Analyzer] Add support for displaying cross-file diagnostic paths in HTML output

2017-02-27 Thread Vlad Tsyrklevich via Phabricator via cfe-commits
vlad.tsyrklevich added a comment.

Here's an example 3 file report from the Linux source: 
https://cdn.rawgit.com/vlad902/73bd32181151f2f0b07fa501a0f0b627/raw/0106f09cdb0d045b757de3d71ddc58072d33/report2.html

It's not as immediately clear this is a multi-file output. Ideas to improve 
this could include anchor tags in the header for every file included, or 
augmenting the line count to something like '$FILE:$LINE' instead. I'm open to 
suggestions on this front.


https://reviews.llvm.org/D30406



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


[PATCH] D30406: [Analyzer] Add support for displaying cross-file diagnostic paths in HTML output

2017-02-28 Thread Artem Dergachev via Phabricator via cfe-commits
NoQ added subscribers: a.sidorin, NoQ.
NoQ added a comment.

I think this is great. We've been hearing a lot of complaints on the mailing 
lists recently about that problem.

Did you check that scan-build properly de-duplicates cross-file reports that 
originate from different clang runs but point to the same header? With your 
approach i think it should work out of the box, but i'd rather be sure.

> It's not as immediately clear this is a multi-file output.

I'm still to have a closer look at the actual code, sorry for the delays.

I'm not having problems with that, i think. If you want to fix that, maybe a 
list of files at the header, with links to the beginning of each file, would be 
enough. Or maybe modify arrows between diagnostic pieces to highlight cases 
when they cross file boundaries: `<- 5. Calling foo() -> (into file foo.c)`.


https://reviews.llvm.org/D30406



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


[PATCH] D30406: [Analyzer] Add support for displaying cross-file diagnostic paths in HTML output

2017-03-01 Thread Anna Zaks via Phabricator via cfe-commits
zaks.anna added a comment.

No multi-file support is a long outstanding limitation of scan-build html 
output. Great to see the patch!! Thank you for working on it!

> It's not as immediately clear this is a multi-file output.

In addition to Artem's suggestions, you might want to insert  multiple lines of 
padding to make the distinction on the border more clear. I think it would help 
especially when scrolling a large report like in the link for the Linux source.

Also, could you put this behind an option or introduce a new format like 
-analyzer-output=plist-multi-file but for html? Just in case someone is relying 
on a single file output format, we'd want to have an option to give them to 
turn it on.


https://reviews.llvm.org/D30406



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


[PATCH] D30406: [Analyzer] Add support for displaying cross-file diagnostic paths in HTML output

2017-03-02 Thread Vlad Tsyrklevich via Phabricator via cfe-commits
vlad.tsyrklevich updated this revision to Diff 90417.
vlad.tsyrklevich edited the summary of this revision.
vlad.tsyrklevich added a comment.

Updated the formatting to make the file split more obvious (padding/line break 
height) and added simple navigation across files, example here: 
https://rawgit.com/vlad902/4aaa4e5e7a777b7098337370791352d7/raw/fc88cef667935e1eeae492b15590c18516e645dd/report.html

@NoQ: I had not thought about deduplication. I looked at scan-build and it 
looks like it currently de-dups based on MD5 matches of the HTML files. The 
simple case of multi-file reports with the same MD5 (e.g. because of repeated 
compilations of a single file during a build) being deduplicated still works 
correctly.

I also looked at what would happen if two separate C files include a common 
header file that only generates a report with -analyzer-opt-analyze-headers 
specified (e.g. if the path originates in the header file and doesn’t traverse 
the main C file at all.) This generated zero reports! My logic assumed the main 
file would always be included in the path and threw away 
-analyzer-opt-analyze-headers reports incorrectly. I was able to fix the logic 
such that these reports are correctly generated and also properly deduplicated 
by structuring the reports such that the main C file source is not included if 
it’s not traversed in the path.

@zaks.anna: I’ve added the single file output option but I would like to keep 
the multi-file option default—I suspect there are very few users parsing the 
HTML manually when plist is available and I’d like to ensure people don’t miss 
any results or have to dig into the source to understand they needed something 
other than the default output setting. What do you think?


https://reviews.llvm.org/D30406

Files:
  include/clang/StaticAnalyzer/Core/Analyses.def
  lib/Rewrite/HTMLRewrite.cpp
  lib/StaticAnalyzer/Core/HTMLDiagnostics.cpp
  test/Analysis/diagnostics/diag-cross-file-boundaries.c
  test/Analysis/diagnostics/diag-cross-file-boundaries.h
  test/Analysis/html-diag-singlefile.c
  test/Analysis/html-diag-singlefile.h
  test/Analysis/html-diags-analyze-headers.c
  test/Analysis/html-diags-analyze-headers.h
  test/Analysis/html-diags-multifile.c
  test/Analysis/html-diags.c
  test/Coverage/html-diagnostics.c
  test/Coverage/html-multifile-diagnostics.c
  test/Coverage/html-multifile-diagnostics.h
  www/analyzer/open_projects.html

Index: www/analyzer/open_projects.html
===
--- www/analyzer/open_projects.html
+++ www/analyzer/open_projects.html
@@ -107,13 +107,6 @@
 
   Bug Reporting 
   
-Add support for displaying cross-file diagnostic paths in HTML output
-(used by scan-build).
-Currently scan-build output does not display reports that span 
-multiple files. The main problem is that we do not have a good format to
-display such paths in HTML output. (Difficulty: Medium) 
-
-
 Refactor path diagnostic generation in http://clang.llvm.org/doxygen/BugReporter_8cpp_source.html";>BugReporter.cpp.
 It would be great to have more code reuse between "Minimal" and 
 "Extensive" PathDiagnostic generation algorithms. One idea is to create an 
Index: test/Coverage/html-multifile-diagnostics.h
===
--- /dev/null
+++ test/Coverage/html-multifile-diagnostics.h
@@ -0,0 +1,3 @@
+void f1(int *ptr) {
+  *ptr = 0;
+}
Index: test/Coverage/html-multifile-diagnostics.c
===
--- /dev/null
+++ test/Coverage/html-multifile-diagnostics.c
@@ -0,0 +1,21 @@
+// RUN: rm -rf %t
+// RUN: %clang_cc1 -analyze -analyzer-output=html -analyzer-checker=core -o %t %s
+// RUN: find %t -name "*.html" -exec cat "{}" ";" | FileCheck %s
+
+// REQUIRES: staticanalyzer
+
+// CHECK: Annotated Source Code
+
+// Make sure it's generated as multi-file HTML output
+// CHECK: {{.*}}html-multifile-diagnostics.c
+// CHECK: {{.*}}html-multifile-diagnostics.h
+
+// Without tweaking expr, the expr would hit to the line below
+// emitted to the output as comment.
+// CHECK: {{[D]ereference of null pointer}}
+
+#include "html-multifile-diagnostics.h"
+
+void f0() {
+  f1((int*)0);
+}
Index: test/Coverage/html-diagnostics.c
===
--- test/Coverage/html-diagnostics.c
+++ test/Coverage/html-diagnostics.c
@@ -1,11 +1,18 @@
 // RUN: rm -rf %t
 // RUN: %clang_cc1 -analyze -analyzer-output=html -analyzer-checker=core -o %t %s
 // RUN: find %t -name "*.html" -exec cat "{}" ";" | FileCheck %s
+//
+// RUN: rm -rf %t
+// RUN: %clang_cc1 -analyze -analyzer-output=html-single-file -analyzer-checker=core -o %t %s
+// RUN: find %t -name "*.html" -exec cat "{}" ";" | FileCheck %s
 
 // REQUIRES: staticanalyzer
 
 // CHECK: Annotated Source Code
 
+// Make sure it's not generated as a multi-file HTML output
+// CHECK-NOT: {{.*}}
+
 // Without

[PATCH] D30406: [Analyzer] Add support for displaying cross-file diagnostic paths in HTML output

2017-03-02 Thread Vlad Tsyrklevich via Phabricator via cfe-commits
vlad.tsyrklevich added inline comments.



Comment at: lib/StaticAnalyzer/Core/HTMLDiagnostics.cpp:211
+
+  if (I + 1 != E) {
+os << "getHashValue()

Is there a cleaner way to do these two comparisons?


https://reviews.llvm.org/D30406



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


[PATCH] D30406: [Analyzer] Add support for displaying cross-file diagnostic paths in HTML output

2017-03-02 Thread Anna Zaks via Phabricator via cfe-commits
zaks.anna added a comment.

>   I’ve added the single file output option but I would like to keep the 
> multi-file option default

This sounds good to me! I agree that this is a very useful addition.


https://reviews.llvm.org/D30406



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