[PATCH] D146591: [dataflow] add HTML logger: browse code/cfg/analysis timeline/state

[Dmitri Gribenko via Phabricator via cfe-commits]

gribozavr2 added a comment.

Could you upload an updated sample HTML file? It is easier to review the HTML generation and javascript code when an example is available.



================
Comment at: clang/lib/Analysis/FlowSensitive/DataflowAnalysisContext.cpp:388
 
+std::unique_ptr<Logger> flagLogger() {
+  if (DataflowLog.empty())
----------------
The name is too short for me. Also it is ambiguous whether "flag" is a noun or a verb.

How about `makeLoggerFromCommandLine`?


================
Comment at: clang/lib/Analysis/FlowSensitive/DataflowAnalysisContext.cpp:388
 
+std::unique_ptr<Logger> flagLogger() {
+  if (DataflowLog.empty())
----------------
gribozavr2 wrote:
> The name is too short for me. Also it is ambiguous whether "flag" is a noun or a verb.
> 
> How about `makeLoggerFromCommandLine`?
Since it is a file-local helper, we can make it static.


================
Comment at: clang/lib/Analysis/FlowSensitive/DataflowAnalysisContext.cpp:395
+    llvm::errs() << "Failed to create log dir: " << EC.message() << "\n";
+  // Separate analyses will create loggers writing to the same directory.
+  // Share a counter so they don't all overwrite each other's 0.html.
----------------



================
Comment at: clang/lib/Analysis/FlowSensitive/HTMLLogger.html:19
+<header>Function</header>
+<div id="code"></div>
+<div id="cfg"></div>
----------------
Could you add the file name and line number of the start location of the function?

It might be helpful not only for the reader (to confirm that they are debugging the right thing), but also when finding the right html file using grep.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D146591