<html><head><meta http-equiv="Content-Type" content="text/html charset=us-ascii"></head><body style="word-wrap: break-word; -webkit-nbsp-mode: space; -webkit-line-break: after-white-space;" class=""><br class=""><div><blockquote type="cite" class=""><div class="">On Nov 21, 2014, at 3:35 PM, Daniel DeFreez <<a href="mailto:dcdefreez@ucdavis.edu" class="">dcdefreez@ucdavis.edu</a>> wrote:</div><br class="Apple-interchange-newline"><div class=""><div dir="ltr" class=""><div class="gmail_extra"><div class="gmail_quote"><blockquote class="gmail_quote" style="margin:0px 0px 0px 0.8ex;border-left:1px solid rgb(204,204,204);padding-left:1ex"><div style="word-wrap:break-word" class=""><div class=""><span class=""><blockquote type="cite" class=""><div class=""><div dir="ltr" class=""><div class=""><div class="gmail_extra"><div class="gmail_quote"><blockquote class="gmail_quote" style="margin:0px 0px 0px 0.8ex;border-left:1px solid rgb(204,204,204);padding-left:1ex"><div style="word-wrap:break-word" class=""><div class=""> I believe with your patch, the node will get added, but the definition used will not have the body attached to it, so the analyzer will not process the body. </div></div></blockquote><div class=""><br class=""></div><div class="">Hmm, doesn't the analyzer itself use getBody() to access the body, thereby walking the list of redeclarations? It seems like it is still processing the body of the function, but maybe I don't know what to look for.</div></div></div></div></div></div></blockquote></span><div class="">This is a good point. We might not be gaining anything by replacing the decl with the one that has a body since getBody is guaranteed to search the redeclarations regardless.</div><div class=""><br class=""></div><div class="">How about just removing both calls to isThisDeclarationADefinition() (from includeInGraph) and replacing the call to getBody with hasBody (to avoid the unnecessary AST de-serialization of the body)? That should work and would simplify the function greatly. What do you think?</div></div></div></blockquote><div class=""><br class=""></div><div class="">I think replacing getBody() with hasBody() is a good idea regardless of the approach taken to fix the missing edges. And avoiding node replacement would make it cleaner.<br class=""></div><div class=""> <br class=""><br class=""></div><blockquote class="gmail_quote" style="margin:0px 0px 0px 0.8ex;border-left:1px solid rgb(204,204,204);padding-left:1ex"><div style="word-wrap:break-word" class=""><div class=""><div class="">I am not sure why some of the checks in your first patch were necessary (like this one).</div><span class=""><div class="">+ // Skip definitions with previous declarations</div><div class="">+ if (FD->getPreviousDecl())<br class="">+ return false;<br class="">+<br class=""></div></span><br class=""></div></div></blockquote><div class=""> </div></div></div><div class="gmail_extra">The reason I put that in was to avoid inserting redundant nodes. Just removing isThisDeclarationADefinition() yields:<br class=""><br class=""></div></div></div></blockquote><div><br class=""></div>Oh, I see! getOrInsertNode() would not insert an existing declaration, but it does not check for re-declarations.</div><div><br class=""></div><div>But would your approach work for this set?</div><div>void a();</div><div><div>void a() { }</div>void b() { a(); } // Here, a call to "a()" might not be added since "a()" has a previous declaration.<br class=""></div><div><br class=""></div><div>Sounds like we would want to store canonical declarations, which brings us back to the suggestion of using "hasBody(Definition)" and only store the definitions that have bodies. Changing the signature of "includeInGraph" so that it mutates the Decl is fine; though, you might want to rename it as well.</div><div><br class=""></div><div>Anna.</div><div><br class=""></div><div><blockquote type="cite" class=""><div class=""><div dir="ltr" class=""><div class="gmail_extra">void a();<br class="">void b() { a(); }<br class="">void a() { }<br class=""><br class=""> --- Call graph Dump --- <br class=""> Function: < root > calls: a b a <br class=""> Function: a calls: <br class=""> Function: b calls: a <br class=""> Function: a calls: <br class=""><br class=""></div><div class="gmail_extra">Since call expressions seem to reference the first declaration, I followed suit. I still need to verify this is always the case.<br class=""></div><div class="gmail_extra"><br class=""></div><div class="gmail_extra">The other check for templates is unnecessary.<br class=""></div></div>
</div></blockquote></div><br class=""></body></html>