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

sammccall added a comment. Sorry about the delay. I hope this is fixed by 28114722baabb468732a9cc24784abafd6c47792 . I wasn't able to reproduce the problem locally, so I'm not sure. add_llvm_library says DEPENDS is the same

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

chapuni added a comment. It comes from handling of objlib. 1. Use add_clang_library's DEPENDS It adds deps on both libs. 2. Don't create a custom target. Just add to sources. add_library(HTMLLogger.inc) Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION

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

mstorsjo added a comment. FYI, I'm seeing occasional spurious build breakage from this change. The issue is that even if we've got `add_dependencies(clangAnalysisFlowSensitive clangAnalysisFlowSensitiveResources)`, it looks like `clangAnalysisFlowSensitiveResources` only is considered a

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

sammccall added inline comments. Comment at: clang/utils/bundle_resources.py:6 +# Part of the LLVM Project, under the Apache License v2.0 with LLVM Exceptions. +# See https://llvm.org/LICENSE.txt for license information. +# SPDX-License-Identifier: Apache-2.0 WITH LLVM-exception

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

thakis added inline comments. Comment at: clang/utils/bundle_resources.py:6 +# Part of the LLVM Project, under the Apache License v2.0 with LLVM Exceptions. +# See https://llvm.org/LICENSE.txt for license information. +# SPDX-License-Identifier: Apache-2.0 WITH LLVM-exception

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

sammccall added a comment. In D146591#4278074 , @xazax.hun wrote: > I am a bit overloaded at the moment, feel free to commit. I can still add > comments later that could be addressed in a follow up. Thanks! Happy to address as they come up.

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

This revision was landed with ongoing or failed builds. This revision was automatically updated to reflect the committed changes. Closed by commit rGa443b3d18ef4: [dataflow] add HTML logger: browse code/cfg/analysis timeline/state (authored by sammccall). Changed prior to commit:

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

xazax.hun accepted this revision. xazax.hun added a comment. I am a bit overloaded at the moment, feel free to commit. I can still add comments later that could be addressed in a follow up. Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D146591/new/

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

sammccall added a comment. @xazax.hun do you still want to look at this again? I'm itching a little to get some use out of it :-) Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D146591/new/ https://reviews.llvm.org/D146591

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

sammccall updated this revision to Diff 513544. sammccall added a comment. oops, after testing this time Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D146591/new/ https://reviews.llvm.org/D146591 Files:

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

sammccall updated this revision to Diff 513543. sammccall added a comment. Extract reinflate() function Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D146591/new/ https://reviews.llvm.org/D146591 Files:

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

gribozavr2 accepted this revision. gribozavr2 added inline comments. This revision is now accepted and ready to land. Comment at: clang/lib/Analysis/FlowSensitive/HTMLLogger.js:47-50 +for (tmpl of root.getElementsByTagName('template')) { + // Clear previously rendered

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

sammccall added inline comments. Comment at: clang/lib/Analysis/FlowSensitive/HTMLLogger.cpp:77 + +void escape(char C, llvm::raw_ostream ) { + switch (C) { xazax.hun wrote: > We already sort of have a way to escape HTML here: >

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

sammccall updated this revision to Diff 512377. sammccall marked an inline comment as done. sammccall added a comment. use llvm::printHTMLEscaped Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D146591/new/ https://reviews.llvm.org/D146591 Files:

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

xazax.hun added inline comments. Comment at: clang/lib/Analysis/FlowSensitive/HTMLLogger.cpp:77 + +void escape(char C, llvm::raw_ostream ) { + switch (C) { We already sort of have a way to escape HTML here:

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

sammccall added a comment. doh, forgot to send Comment at: clang/lib/Analysis/FlowSensitive/HTMLLogger.cpp:128 +*OS << "" << HTMLLogger_css << "\n"; +*OS << "" << HTMLLogger_js << "\n"; + gribozavr2 wrote: > Now that we have an HTML template file, you

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

sammccall updated this revision to Diff 511126. sammccall marked 8 inline comments as done. sammccall added a comment. address comments (sorry about long turnaround!) Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D146591/new/

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

gribozavr2 added inline comments. Comment at: clang/lib/Analysis/FlowSensitive/HTMLLogger.html:19 +Function + + sammccall wrote: > gribozavr2 wrote: > > Could you add the file name and line number of the start location of the > > function? > > > > It might be

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

gribozavr2 added inline comments. Comment at: clang/lib/Analysis/FlowSensitive/HTMLLogger.cpp:34 +// the data into tags embedded in the HTML. (see inflate() in JS). +// +// SELECTION Comment at:

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

sammccall added inline comments. Comment at: clang/lib/Analysis/FlowSensitive/HTMLLogger.html:19 +Function + + gribozavr2 wrote: > Could you add the file name and line number of the start location of the > function? > > It might be helpful not only for the

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

sammccall updated this revision to Diff 508518. sammccall added a comment. Only show the filename, not the full path, to avoid overflowing the box Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D146591/new/ https://reviews.llvm.org/D146591 Files:

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

sammccall updated this revision to Diff 508517. sammccall marked 4 inline comments as done. sammccall edited the summary of this revision. sammccall added a comment. Address Dmitri's comments Update demo link Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION

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

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 flagLogger()

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

sammccall updated this revision to Diff 508273. sammccall added a comment. revert accidental edit Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D146591/new/ https://reviews.llvm.org/D146591 Files:

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

sammccall updated this revision to Diff 508272. sammccall added a comment. Move data into JSON object, render into DOM with templates. This is cleaner in a few ways: less ugly C++ string manipulation, data is in principle reusable, HTML layout is extracted and easier to inspect and edit.

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

sammccall planned changes to this revision. sammccall added a comment. Going to have a go at passing data via JSON Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D146591/new/ https://reviews.llvm.org/D146591

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

sammccall updated this revision to Diff 507718. sammccall marked 2 inline comments as done. sammccall added a comment. Address all comments except moving data from HTML => JSON Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D146591/new/

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

sammccall marked 15 inline comments as done. sammccall added inline comments. Comment at: clang/lib/Analysis/FlowSensitive/CMakeLists.txt:22 +add_custom_command(OUTPUT HTMLLogger.inc + COMMAND "${Python3_EXECUTABLE}" bundle_resources.py +

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

gribozavr2 added inline comments. Comment at: clang/include/clang/Analysis/FlowSensitive/Logger.h:34 static std::unique_ptr textual(llvm::raw_ostream &); + // A logger that builds an HTML UI to inspect the analysis results. + // One file is written under the specified dir

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

xazax.hun added a comment. In D146591#4213614 , @sammccall wrote: > (e.g. does the MLIR dataflow framework have the same idea about how the > analysis timeline relates to the CFG and elements within it? And if we want > to show the clang AST or the

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

sammccall added a comment. In D146591#4213319 , @xazax.hun wrote: > While I am OK with this approach, I wonder if it would be too much work to > split the HTML generation and emitting data up. E.g., the logger could emit > YAML or JSON that could be

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

xazax.hun added a comment. While I am OK with this approach, I wonder if it would be too much work to split the HTML generation and emitting data up. E.g., the logger could emit YAML or JSON that could be parsed by a python script to create the HTML (or the HTML itself could use JS to parse it

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

sammccall updated this revision to Diff 507339. sammccall added a comment. Use a process-shared counter for HTML output filenames to avoid clobbering. Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D146591/new/ https://reviews.llvm.org/D146591

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

sammccall added a comment. Added a bunch of reviewers for feedback on how this should work. Happy to have detailed comments on the JS/HTML generator implementation too of course, but it may inevitably be a bit of a mess (ugly, weird languages, undertested). I've added things like this to a few

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

sammccall created this revision. Herald added subscribers: martong, xazax.hun. Herald added a reviewer: NoQ. Herald added a project: All. sammccall requested review of this revision. Herald added a project: clang. Herald added a subscriber: cfe-commits. With -dataflow-log=/dir we will write