[llvm-dev] [RFC] Should we add isa_or_null<>?

[Craig Topper via llvm-dev]

Agreed that the new isa_or_null style is better. Just wanted mention the
other style so we know we should migrate those to the new one.

~Craig


On Thu, Apr 4, 2019 at 4:37 PM Don Hinton <hintonda at gmail.com> wrote:

> On Thu, Apr 4, 2019 at 6:29 PM Craig Topper <craig.topper at gmail.com>
> wrote:
>
>> There are a handful of places in LLVM that dosomething like  if
>> (dyn_cast_or_null<UndefValue>(P->hasConstantValue()))
>>
>
> Yes, I've seen those, but while working on a new checker, I was advised
> that replacing `X && isa<Y>(X)` with `dyn_cast_or_null<Y>(X)` was
> suboptimal, and it was suggested something like a `isa_or_null` style
> operator would better express what was actually going on, i.e., we are
> expecting a bool, not a pointer.
>
>
>>
>> ~Craig
>>
>>
>> On Thu, Apr 4, 2019 at 4:16 PM Don Hinton via llvm-dev <
>> llvm-dev at lists.llvm.org> wrote:
>>
>>> I've added a patch, temporarily using the name Chris suggested.  Please
>>> let me know what you think.
>>>
>>> https://reviews.llvm.org/D60291
>>>
>>> thanks...
>>> don
>>>
>>> On Thu, Apr 4, 2019 at 2:55 PM David Greene <dag at cray.com> wrote:
>>>
>>>> Don Hinton <hintonda at gmail.com> writes:
>>>>
>>>> > >  if (isa_or_null<T>(var)) {
>>>> > >    ...
>>>> > >  }
>>>> > >
>>>> > >  at least according to what "isa_or_null" conveys to me.
>>>> >
>>>> > This is the same convention used by the existing "_or_null" varieties,
>>>> > i.e., "cast_or_null" and "dyn_cast_or_null".  They accept a null and
>>>> > propagate it.  In the "isa" case, it would accept a null and propagate
>>>> > it as false.
>>>>
>>>> isa<> is very different from *cast<>.  *cast<> gives you a pointer back,
>>>> which may be null.  isa<> is precondition check, so it "reads"
>>>> differently to me.  If I were to see:
>>>>
>>>> if (isa_or_null<T>(var)) {
>>>>   ...
>>>> }
>>>>
>>>> I would think, "Ok, the body is fine if var is null."
>>>>
>>>> Instead:
>>>>
>>>> if (exists_and_isa<T>(var)) {
>>>>   ...
>>>> }
>>>>
>>>> This tells me that the body expects a non-null value.
>>>>
>>>> > >  That said, I'm not sure sure we need a special API for this.  Are
>>>> > >  expensive calls used in the way you describe really common?
>>>> >
>>>> > I've only been looking at the ones involving method calls, but it's
>>>> > not too common.  Perhaps a dozen in clang/lib -- haven't run it
>>>> > against the rest of the code base.
>>>>
>>>> Thanks for checking.  I don't have a strong opinion about the need
>>>> either way, but I do care that the spelling is clear and intuitive.
>>>>
>>>>                            -David
>>>>
>>> _______________________________________________
>>> LLVM Developers mailing list
>>> llvm-dev at lists.llvm.org
>>> https://lists.llvm.org/cgi-bin/mailman/listinfo/llvm-dev
>>>
>>
-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://lists.llvm.org/pipermail/llvm-dev/attachments/20190404/ad2b74f6/attachment.html>