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

[Daniel Berlin via llvm-commits]

FWIW: TreeTraits would be very nice.

For example, the depth first iterator always stores a visited set, and for
trees like the dominator tree, this is a complete waste of time and memory.

It'd be nice to specialize it for trees.



On Wed, Aug 17, 2016 at 10:56 PM, Tim Shen via llvm-commits <
llvm-commits at lists.llvm.org> wrote:

> timshen added inline comments.
>
> ================
> Comment at: include/llvm/Analysis/RegionIterator.h:35
> @@ -34,3 @@
> -class RNSuccIterator : public std::iterator<std::forward_iterator_tag,
> -                                           NodeType, ptrdiff_t> {
> -  typedef std::iterator<std::forward_iterator_tag, NodeType, ptrdiff_t>
> super;
> ----------------
> dblaikie wrote:
> > If it has to be a pointer type anyway - would it be simpler to preserve
> that behavior of value_type being a pointer rather than a value? Or is
> there something else going on here as well that makes this change
> necessary/useful?
> Actually by looking at the code, I can tell that NodeType is always
> GraphTraits<BlockT*>::NodeType. After the migration I prefer to keep the
> consistency, that is the template parameter NodeRef is always
> GraphTraits<BlockT*>::NodeRef. The fact that NodeRef is required to be a
> pointer seems not worthy to be mentioned too frequently.
>
> The code also assumes that BlockT* is NodeRef/NodeType*. And the code also
> uses getParent(). I'm sad that people stick on GraphTraits just because it
> exists, when TreeTraits could be a better fit.
>
>
> https://reviews.llvm.org/D23625
>
>
>
> _______________________________________________
> llvm-commits mailing list
> llvm-commits at lists.llvm.org
> http://lists.llvm.org/cgi-bin/mailman/listinfo/llvm-commits
>
-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://lists.llvm.org/pipermail/llvm-commits/attachments/20160817/e15eb5a6/attachment.html>