[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:

This breaks the existing use of `Visited`, which is to ensure we don't recursively dump a value's structure multiple times, even in acyclic cases. (e.g. a `pair<Struct*, Struct*>` where the two pointers are the same).

 if you want to detect cycles specifically, then we should use a different set to track the values currently on the stack, with Visited still used to track everything that we've seen.

But I'm not sure the distinction is worth the code: the idea "we've seen this node before, and won't print its details again" applies whether the reason is a cycle or just multiple paths to a node, and they both benefit from some explicit hint.

So I'd probably rather keep the existing meaning of "Visited" and replacing "in_cycle" with "already dumped" or so. WDYT?

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