[PATCH] D77341: [DomTree] Replace ChildrenGetter with GraphTraits over GraphDiff.

Jakub Kuderski via Phabricator via llvm-commits llvm-commits at lists.llvm.org
Sun Apr 5 19:45:49 PDT 2020


kuhar added a comment.

In D77341#1960854 <https://reviews.llvm.org/D77341#1960854>, @asbirlea wrote:

> Address comments.


Thanks for the changes and explanations. It think with a few more tweaks this will be a good refactoring step towards phasing out BUI.



================
Comment at: llvm/include/llvm/IR/CFGDiff.h:198
 
+namespace {
+template <typename Range>
----------------
What benefit does an anonymous namespace in a header have over a named one, e.g., `detail`/`impl`? Doesn't it make it more difficult to deduplicate symbols across different translation units? Not sure what the best practices are in C++ now in this area.


================
Comment at: llvm/include/llvm/Support/GenericDomTreeConstruction.h:325
+    auto &GD = BUI ? BUI->PreViewCFG : EmptyGD;
+    return !empty(children<GraphDiffNodePair>({&GD, N}));
   }
----------------
This pattern repeats a few times. Could it be pushed into a helper function to get something like this?

```
return !empty(children<GraphDiffNodePair>(GetGraphDiffNodePair(BUI, N)));
```



Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D77341





More information about the llvm-commits mailing list