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

Sun Apr 5 23:03:03 PDT 2020

dblaikie added inline comments.

================
Comment at: llvm/include/llvm/IR/CFGDiff.h:198

+namespace {
+template <typename Range>
----------------
kuhar wrote:
> kuhar wrote:
> > 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.
> Found an article on this matter: https://wiki.sei.cmu.edu/confluence/display/cplusplus/DCL59-CPP.+Do+not+define+an+unnamed+namespace+in+a+header+file
Oh, yeah, sorry I didn't spot that - yeah, anonymous namespaces in header files are a no-go. This should be a `detail` (`detail` outnumbers `impl` 361 to 41 in the LLVM project) namespace within the llvm namespace as suggested.

Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D77341