[llvm-commits] [llvm] r169728 - in /llvm/trunk: include/llvm/Analysis/PtrUseVisitor.h lib/Analysis/CMakeLists.txt lib/Analysis/PtrUseVisitor.cpp lib/Transforms/Scalar/SROA.cpp

[Chandler Carruth]

Just a couple of quick clarifications...

On Mon, Dec 10, 2012 at 1:28 AM, Duncan Sands <baldrick at free.fr> wrote:

> +    /// \brief Did we abort the visit early?
>> +    bool isAborted() const { return AbortedInfo.getInt(); }
>>
>
> Can't this just return whether the "aborted pointer" is not null?


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.

Certainly, I can add asserts as if there *is* a pointer, then the bool must
be true.


>  +
>> +    /// \brief Is the pointer escaped at some point?
>>
>
> Is the pointer escaped -> Did the pointer escape
> Can't this just be a test that the "escaped pointer" is not null?


Same pattern as above.


>
>  +    /// \brief Get the instruction causing the visit to abort.
>> +    /// \returns a pointer to the instruction causing the abort if one is
>> +    /// available; otherwise returns null.
>> +    Instruction *getAbortingInst() const { return
>> AbortedInfo.getPointer(); }
>>
>
> Otherwise you could have an assertion here that if the pointer is non-null
> then
> isAborted is true, to catch mistakes.


And yes, will add asserts.

+
>> +    /// \brief Mark the visit as aborted. Intended for use in a void
>> return.
>>
>
> Intended for use in a void return?  What does this mean?  What happens if
> I is
> used for an instruction that is not a void return?


The 'set' methods are intended to work cleanly with code like:

void visitMyInstruction(MyInstruction &I) {
  // .. some code ..

  if (detectErrorCondition())
    return PI.setAborted(&I);

  // .. rest of my code ..
}

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.

I'd love to have a better name for this. I can add an example to the
comment maybe?

+/// offsets.
>> +///
>> +/// All visit methods on the underlying InstVisitor return a boolean.
>> This
>> +/// return short-circuits the visit, stopping it immediately.
>>
>
> Short-circuits when the returned value is true, or when it is false?


This comment is completely stale, will update.
-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://lists.llvm.org/pipermail/llvm-commits/attachments/20121210/16e3a609/attachment.html>