[PATCH] D29767: GraphTraits: Add range versions of graph traits functions (graph_nodes, graph_children, inverse_graph_nodes, inverse_graph_children).

[David Blaikie via llvm-commits]

On Thu, Feb 9, 2017 at 8:14 AM Daniel Berlin via Phabricator <
reviews at reviews.llvm.org> wrote:

> dberlin created this revision.
> Herald added a reviewer: tstellarAMD.
> Herald added subscribers: nhaehnle, arsenm.
>
> Convert all obvious node_begin/node_end and child_begin/child_end
> pairs to range based for.
>
> Sending for review in case someone has a good idea how to make
> graph_children able to be inferred. It looks like it would require
> changing GraphTraits to be two argument or something. I presume
> inference does not happen because it would have to check every
> GraphTraits in the world to see if the noderef types matched.
>

Yep, that. One could use an extra trait for the node type to map it back to
the graph trait to facilitate this - but that'd also mean each node type
would have to be unique to the graph type (which is probably true - but one
could imagine different graphs over the same nodes? Maybe... maybe not)

Did you consider changing the trait so they exposed range accessors
directly, instead of adding utility functions?
"graph_children(graph)" is a bit of an awkward construct. Alternatively
maybe drop the graph_ prefix - ADL/overload resolution/etc will make it
correct anyway, so just "children(graph)", etc?

(re: your other comment, I'm not sure I quite followed:

"Note that this use of const auto & is required to make clang build, as the
iterator type it for CFG blocks ends up returning blocks directly otherwise
here." - not sure I quite follow. Best guess/reading is that the CFG's
iterators' value type is heavyweight, but most other GraphTraits types have
lightweight nodes that are cheap to copy? (like pointers, etc)


>
> Note: This change was 3-staged with clang as well, which uses
> Dominators/etc from LLVM.
>
>
> https://reviews.llvm.org/D29767
>
> Files:
>   include/llvm/ADT/GraphTraits.h
>   include/llvm/Support/GenericDomTree.h
>   include/llvm/Support/GenericDomTreeConstruction.h
>   lib/Analysis/IteratedDominanceFrontier.cpp
>   lib/Target/AMDGPU/AMDILCFGStructurizer.cpp
>   lib/Target/Hexagon/HexagonBitSimplify.cpp
>   lib/Target/Hexagon/HexagonCommonGEP.cpp
>   lib/Target/Hexagon/HexagonGenExtract.cpp
>   lib/Target/Hexagon/HexagonGenInsert.cpp
>
>
-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://lists.llvm.org/pipermail/llvm-commits/attachments/20170209/bc6066ba/attachment.html>