[PATCH] D23625: [Analysis] Change several Analysis pieces to use NodeRef. NFC.

[Tim Shen via llvm-commits]

timshen added inline comments.

================
Comment at: include/llvm/Analysis/BlockFrequencyInfoImpl.h:1274
@@ -1274,3 +1273,3 @@
            I != E; ++I) {
-        NodeType &N = *I;
+        NodeRef N = &*I;
         MaxFrequency =
----------------
dblaikie wrote:
> I'm confused by this '&'.
> 
> If GraphTraits is meant to be node handle agnostic, why is it producing a dereferenced node handle type here? (such that the address needs to be taken again)
This is more or less embarrassing:

Some implementors decide to let *I return NodeType& (e.g. I as Function::iterator), while others decide to let *I return NodeType*.

GraphWriter has the same problem. The solution of GraphWriter (even before the NodeType -> NodeRef change) is to pass *I into Foo, where Foo is overloaded on NodeType&, NodeType* and NodeType **.

================
Comment at: include/llvm/Analysis/RegionIterator.h:53
@@ -52,3 +52,3 @@
   // Use two bit to represent the mode iterator.
-  PointerIntPair<NodeType*, 2, ItMode> Node;
+  PointerIntPair<NodeRef, 2, ItMode> Node;
 
----------------
dblaikie wrote:
> This would still be restricted to pointers - so it doesn't actually generalize over the new node handles you are introducing.
> 
> Does this need deeper changes?
As discussed offline, put a static_assert here.


https://reviews.llvm.org/D23625