<div dir="ltr">Given that there exists only one algorithm that uses CallGraphTraits (this one) it is hard for me to argue that every algorithm will require has_function_def and so I'll remove this. <br><div class="gmail_extra"><br><div class="gmail_quote">On Mon, Jan 22, 2018 at 5:20 PM, Daniel Berlin via Phabricator <span dir="ltr"><<a href="mailto:reviews@reviews.llvm.org" target="_blank">reviews@reviews.llvm.org</a>></span> wrote:<br><blockquote class="gmail_quote" style="margin:0 0 0 .8ex;border-left:1px #ccc solid;padding-left:1ex">dberlin added a comment.<br>
<br>
Is it a thing that is even different for each callgraph implementation? (If not, why is it a trait?)<br>
Even if it is, if you look, you'll see we don't try to encode random computed Node property differences in GraphTraits. </blockquote><div> </div><blockquote class="gmail_quote" style="margin:0 0 0 .8ex;border-left:1px #ccc solid;padding-left:1ex">Only the things that *every* graph algorithm needs (IE it caters to anything, not everything).  </blockquote><div><br></div><div>I don't understand what you've written inside the parenthesis, but I take issue with the assertion  that "only things that every graph algorithm needs [are encoded in traits]". As an example, DepthFirstIterator only needs the get_entry_node, child_begin and child_end. Similarly, once can think of a graph algorithm like connected components for which get_entry_node makes no sense. </div><div><br></div><div> </div><blockquote class="gmail_quote" style="margin:0 0 0 .8ex;border-left:1px #ccc solid;padding-left:1ex">  Random node property differences are handled through other trait classes or filtering.  This does not, at a glance, seem like a thing that literally every algorithm that is going to use a Callgraph needs. It looks like one that some may need.  You even prove this yourself.<br>
<br>
If you want to do something different here for CallGraphTraits, i'd like to understand what advantage it has.  At least in this patch as currently written, there is no advantage to has_function_def existing as a CallGraphTrait.<br>
It would work just existing fine outside of CallGraphTraits, and even as a templated function  local to a single file.<br>
<div class="HOEnZb"><div class="h5"><br>
<br>
Repository:<br>
  rL LLVM<br>
<br>
<a href="https://reviews.llvm.org/D42311" rel="noreferrer" target="_blank">https://reviews.llvm.org/<wbr>D42311</a><br>
<br>
<br>
<br>
</div></div></blockquote></div><br></div></div>