gribozavr2 added inline comments.
================
Comment at: clang/lib/Analysis/FlowSensitive/HTMLLogger.cpp:34
+// the data into <template> tags embedded in the HTML. (see inflate() in JS).
+//
+// SELECTION
----------------
================
Comment at: clang/lib/Analysis/FlowSensitive/HTMLLogger.cpp:128
+ *OS << "<style>" << HTMLLogger_css << "</style>\n";
+ *OS << "<script>" << HTMLLogger_js << "</script>\n";
+
----------------
Now that we have an HTML template file, you could inline css and js into the
file - if you like.
================
Comment at: clang/lib/Analysis/FlowSensitive/HTMLLogger.html:1
+<!doctype html>
+<html>
----------------
Please add a copyright statement.
================
Comment at: clang/lib/Analysis/FlowSensitive/HTMLLogger.js:11
+//
+// For example, if we the selection is {bb=BB4, elt=BB4.6 iter=BB4:2}:
+// - show the "block" and "element" sections
----------------
================
Comment at: clang/lib/Analysis/FlowSensitive/HTMLLogger.js:11
+//
+// For example, if we the selection is {bb=BB4, elt=BB4.6 iter=BB4:2}:
+// - show the "block" and "element" sections
----------------
gribozavr2 wrote:
>
"if we the selection is" - parse error :)
================
Comment at: clang/lib/Analysis/FlowSensitive/HTMLLogger.js:45
+ }
+ root.hidden = false;
+ if (changed) {
----------------
Should `hidden = false` be under `if (changed)`?
Or can it be the case that nothing changed, but we still need to show the
element?
================
Comment at: clang/lib/Analysis/FlowSensitive/HTMLLogger.js:47-50
+ for (tmpl of root.getElementsByTagName('template')) {
+ // Clear previously rendered template contents.
+ while (tmpl.nextSibling && tmpl.nextSibling.inflated)
+ tmpl.parentNode.removeChild(tmpl.nextSibling);
----------------
Would it make sense to move this removal logic into `inflate` itself?
(Of course the recursive part of `inflate` would stay as a separate function,
`inflateImpl` or something.)
================
Comment at: clang/lib/Analysis/FlowSensitive/HTMLLogger.js:124
+// hover: a function from target to the class name to highlight
+// bb: a function from target to the basic-block name to select (BB4)A
+// elt: a function from target to the CFG element name to select (BB4.5)
----------------
What's "A" in "(BB4)A"?
================
Comment at: clang/lib/Analysis/FlowSensitive/HTMLLogger.js:184
+}
+function classSelector(cls) {
+ if (cls == null) return null;
----------------
Could you add a doc comment that describes what cls is?
Specifically, I see that in the array case we assume that the dot is already
present, and in the single-value case we prepend the dot - is that intentional?
Repository:
rG LLVM Github Monorepo
CHANGES SINCE LAST ACTION
https://reviews.llvm.org/D146591/new/
https://reviews.llvm.org/D146591
_______________________________________________
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
