[PATCH] D124904: [DomPrinter] migrate -dot-dom to the new pass manager

[Michael Kruse via Phabricator via llvm-commits]

Meinersbur added a reviewer: aeubanks.
Meinersbur added inline comments.


================
Comment at: llvm/include/llvm/Analysis/DomPrinter.h:1
-//===-- DomPrinter.h - Dom printer external interface ------------*- C++ -*-===//
+//===-- DomPrinterWrapperPass.h - Dom printer external interface -*-C++ -*-===//
 //
----------------
This is not the name of the file


================
Comment at: llvm/include/llvm/Analysis/DomPrinter.h:36-39
+    if (isSimple())
+      return DOTGraphTraits<DOTFuncInfo *>::getSimpleNodeLabel(BB, nullptr);
+    else
+      return DOTGraphTraits<DOTFuncInfo *>::getCompleteNodeLabel(BB, nullptr);
----------------
[style] [[ https://llvm.org/docs/CodingStandards.html#don-t-use-else-after-a-return | Don’t use else after a return ]]


================
Comment at: llvm/include/llvm/Analysis/PostDominators.h:123-126
+    if (getEntryNode(N))
+      return df_begin(getEntryNode(N));
+    else
+      return df_end(getEntryNode(N));
----------------
[style] Don’t use else after a return

Can `DT.getRootNode()` actually return nullptr? AFAIK it always creates a virtual root node.


================
Comment at: llvm/include/llvm/IR/Dominators.h:267
+template <>
+struct GraphTraits<DominatorTree> : public GraphTraits<DomTreeNode *> {
+  static NodeRef getEntryNode(const DominatorTree &DT) {
----------------
Consider defining the traits of the pointer type `GraphTraits<DominatorTree*>` for consistency with other GraphTraits, including `GraphTraits<DomTreeNode *>`. that it derives from. Users of `GraphTraits` may assume it is a pointer.


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

https://reviews.llvm.org/D124904