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

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
>
-------------- next part --------------
An HTML attachment was scrubbed...
URL:
<http://lists.llvm.org/pipermail/llvm-dev/attachments/20190404/b831ec94/attachment.html>

There are a handful of places in LLVM that dosomething like  if
(dyn_cast_or_null<UndefValue>(P->hasConstantValue()))

~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/3451052b/attachment.html>

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/7ee2ec01/attachment-0001.html>