[clang] [clang][dataflow] Fix handling of cyclical data structures in HTMLLogger. (PR #66887)

[Sam McCall via cfe-commits]

================
@@ -88,10 +88,12 @@ class ModelDumper {
 
   void dump(Value &V) {
     JOS.attribute("value_id", llvm::to_string(&V));
-    if (!Visited.insert(&V).second)
-      return;
-
     JOS.attribute("kind", debugString(V.getKind()));
+    if (!Visited.insert(&V).second) {
+      JOS.attribute("[in_cycle]", " ");
+      return;
+    }
+    auto EraseVisited = llvm::make_scope_exit([&] { Visited.erase(&V); });
----------------
sam-mccall wrote:

(Less invasive ideas that might be useful either way: "undefined" is a bug which can be fixed. Duplicate nodes could/should also be made a different color)

I think showing a subtree twice is a pretty serious misrepresentation of the data, probably more so than pruning children from a duplicated node. There's no easy + perfect way to show a DAG in a tree-browser. It'd be possible to make this more explicit (e.g. have a "duplicate node" box contain a link to an anchor on the original node). But it's complexity, and if you *don't* care about the DAG structure then it's still not ideal.

> I assume your concern is that we could have data structures with lots and lots of repeated values, and this would bloat the JSON? Do we actually know that this is a problem though?

Yes, I believe I saw this. I don't remember the details though, and it might have involved the old BoolValue subclasses that bloated the tree.

https://github.com/llvm/llvm-project/pull/66887