<div style="font-family: arial, helvetica, sans-serif; font-size: 10pt"><div dir="ltr"><div style>Just a couple of quick clarifications...</div><br><div class="gmail_extra"><div class="gmail_quote">On Mon, Dec 10, 2012 at 1:28 AM, Duncan Sands <span dir="ltr"><<a href="mailto:baldrick@free.fr" target="_blank">baldrick@free.fr</a>></span> wrote:<br>
<blockquote class="gmail_quote" style="margin:0 0 0 .8ex;border-left:1px #ccc solid;padding-left:1ex"><div><div class="h5"><blockquote class="gmail_quote" style="margin:0 0 0 .8ex;border-left:1px #ccc solid;padding-left:1ex">
+    /// \brief Did we abort the visit early?<br>
+    bool isAborted() const { return AbortedInfo.getInt(); }<br>
</blockquote>
<br></div></div>
Can't this just return whether the "aborted pointer" is not null?</blockquote><div><br></div><div style>We might want to abort the visit without having a particular instruction to blame for that decision. The visitor tries to be generic and support either instruction-based aborting (and tracking that instruction), or just a simple signal "no more". The pair seemed the cleanest way to express that.</div>
<div style><br></div><div style>Certainly, I can add asserts as if there *is* a pointer, then the bool must be true.</div><div><br></div><blockquote class="gmail_quote" style="margin:0 0 0 .8ex;border-left:1px #ccc solid;padding-left:1ex">
<div class="im"><br>
<blockquote class="gmail_quote" style="margin:0 0 0 .8ex;border-left:1px #ccc solid;padding-left:1ex">
+<br>
+    /// \brief Is the pointer escaped at some point?<br>
</blockquote>
<br></div>
Is the pointer escaped -> Did the pointer escape<br>
Can't this just be a test that the "escaped pointer" is not null?</blockquote><div><br></div><div style>Same pattern as above.</div><div> </div><blockquote class="gmail_quote" style="margin:0 0 0 .8ex;border-left:1px #ccc solid;padding-left:1ex">
<div class="im"><br>
<blockquote class="gmail_quote" style="margin:0 0 0 .8ex;border-left:1px #ccc solid;padding-left:1ex">
+    /// \brief Get the instruction causing the visit to abort.<br>
+    /// \returns a pointer to the instruction causing the abort if one is<br>
+    /// available; otherwise returns null.<br>
+    Instruction *getAbortingInst() const { return AbortedInfo.getPointer(); }<br>
</blockquote>
<br></div>
Otherwise you could have an assertion here that if the pointer is non-null then<br>
isAborted is true, to catch mistakes.</blockquote><div><br></div><div style>And yes, will add asserts.</div><div style><br></div><blockquote class="gmail_quote" style="margin:0 0 0 .8ex;border-left:1px #ccc solid;padding-left:1ex">
<div class="im"><blockquote class="gmail_quote" style="margin:0 0 0 .8ex;border-left:1px #ccc solid;padding-left:1ex">+<br>
+    /// \brief Mark the visit as aborted. Intended for use in a void return.<br>
</blockquote>
<br></div>
Intended for use in a void return?  What does this mean?  What happens if I is<br>
used for an instruction that is not a void return?</blockquote><div><br></div><div style>The 'set' methods are intended to work cleanly with code like:</div><div style><br></div><div style>void visitMyInstruction(MyInstruction &I) {</div>
<div style>  // .. some code ..</div><div style><br></div><div style>  if (detectErrorCondition())</div><div style>    return PI.setAborted(&I);</div><div style><br></div><div style>  // .. rest of my code ..</div><div style>
}</div><div style><br></div><div style>Thus, they are designed to be called within a void-type return statement by the implementor of the visitor. Does that clarify? It has nothing to do with the return of the LLVM IR under analysis.</div>
<div style><br></div><div style>I'd love to have a better name for this. I can add an example to the comment maybe?</div><div style><br></div><blockquote class="gmail_quote" style="margin:0 0 0 .8ex;border-left:1px #ccc solid;padding-left:1ex">
<div class="im"><blockquote class="gmail_quote" style="margin:0 0 0 .8ex;border-left:1px #ccc solid;padding-left:1ex">+/// offsets.<br>
+///<br>
+/// All visit methods on the underlying InstVisitor return a boolean. This<br>
+/// return short-circuits the visit, stopping it immediately.<br>
</blockquote>
<br></div>
Short-circuits when the returned value is true, or when it is false?</blockquote><div><br></div><div style>This comment is completely stale, will update.</div></div></div></div></div>