<html><head><meta http-equiv="Content-Type" content="text/html charset=windows-1252"></head><body style="word-wrap: break-word; -webkit-nbsp-mode: space; -webkit-line-break: after-white-space;"><br><div><div>On Mar 28, 2013, at 5:50 PM, Jordan Rose <<a href="mailto:jordan_rose@apple.com">jordan_rose@apple.com</a>> wrote:</div><br class="Apple-interchange-newline"><blockquote type="cite"><div style="letter-spacing: normal; orphans: auto; text-align: start; text-indent: 0px; text-transform: none; white-space: normal; widows: auto; word-spacing: 0px; -webkit-text-stroke-width: 0px;">I'm sad to have another one-shot visitor, rather than just doing the work up front in trackNullOrUndef.</div></blockquote><div><br></div><div>Is there a policy on when we should walk up the path up front vs when we should do it in the visitors? Currently, we seem to be mixing both, sacrificing the cleanness of the code.. I thought that we had decided to go with visitors from now on. We could redesign that code/try to improve performance more if we think this approach is generally too slow.</div><div><br></div><blockquote type="cite"><div style="letter-spacing: normal; orphans: auto; text-align: start; text-indent: 0px; text-transform: none; white-space: normal; widows: auto; word-spacing: 0px; -webkit-text-stroke-width: 0px;"> Now that suppression can take a significant portion of analysis time, we should probably not add new visitors as freely as we once would have.<br><br><blockquote type="cite">+  // The message send could be null if the receiver is null.<br>+  if (const Expr *Receiver = NilReceiverBRVisitor::getReceiver(S)) {<br>+    report.addVisitor(new NilReceiverBRVisitor(Receiver,<br>+                                               EnableNullFPSuppression));<br>+  }<br>+<br></blockquote><br>if (...) {<br><span class="Apple-tab-span" style="white-space: pre;">  </span>trackNullOrUndefValue(N, Receiver, report);<br>}<br><br></div></blockquote><br>The receiver might not be nil. Logically, we should only register to track it if it is indeed nil.  We cannot know if it is nil or not until we walk up the path to find the node where it has a value. Using the existing visitor to do this seems to be the cleanest approach. That was the motivation for doing it this way.</div><div><br><blockquote type="cite"><div style="letter-spacing: normal; orphans: auto; text-align: start; text-indent: 0px; text-transform: none; white-space: normal; widows: auto; word-spacing: 0px; -webkit-text-stroke-width: 0px;">This seems much more efficient—no restarting of path generation due to the new visitor.</div></blockquote></div><div><br></div><br></body></html>