[PATCH] D156522: [LLVM] move verification of convergence control to a class template

[Matt Arsenault via Phabricator via llvm-commits]

arsenm added inline comments.


================
Comment at: llvm/include/llvm/ADT/GenericConvergenceVerifier.h:34
+  void initialize(raw_ostream &OS,
+                  std::function<void(const Twine &Message)> FailureCB,
+                  const FunctionT &F) {
----------------
can this be function_ref? I'd expect this to have the failure context as an explicit argument


================
Comment at: llvm/include/llvm/ADT/GenericConvergenceVerifier.h:70
+
+  void CheckFailed(const Twine &Message, ArrayRef<Printable> Values);
+};
----------------
Lowercase?


================
Comment at: llvm/lib/IR/Verifier.cpp:395
+    CV.initialize(
+        *OS, [&](const Twine &Message) { CheckFailed(Message); }, F);
 
----------------
Don't think you need the everything capture


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D156522