[PATCH] D84567: [CFGDiff] Refactor Succ/Pred maps.

[David Blaikie via Phabricator via llvm-commits]

dblaikie added inline comments.


================
Comment at: llvm/include/llvm/Support/CFGDiff.h:81
+        OS << DIText[IsInsert] << " edges: \n";
+        for (auto Child : Pair.second.DI[0]) {
+          OS << "(";
----------------
Should this `0` be `IsInsert`?


================
Comment at: llvm/include/llvm/Support/CFGDiff.h:122-130
+    if (SuccList.empty() && SuccDIList.DI[1 - IsInsert].empty())
+      Succ.erase(U.getFrom());
 
-    auto &PredList = Edges[IsInsert].Pred[U.getTo()];
+    auto &PredDIList = Pred[U.getTo()];
+    auto &PredList = PredDIList.DI[IsInsert];
     assert(PredList.back() == U.getFrom());
     PredList.pop_back();
----------------
`1 - IsInsert` might be more clearly written as `!IsInsert` or `IsInsert == false`? (I'd tend towards the `!` notation myself, but I understand the `== false` notation might be a bit more legible/clear/harder-to-miss)


================
Comment at: llvm/include/llvm/Support/CFGDiff.h:148-150
+      Res.erase(std::remove(Res.begin(), Res.end(), Child), Res.end());
+    // Remove nullptr children for clang.
+    Res.erase(std::remove(Res.begin(), Res.end(), nullptr), Res.end());
----------------
I think we've probably got a utility for erase+remove in STLExtras.h?


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D84567