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

[Don Hinton via llvm-dev]

Sorry, meant Hubert.

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

> I'm leaning toward Herbert's suggestion:  `nonnull_and_isa`.
>
> What do you think?
>
> On Thu, Apr 4, 2019 at 7:17 PM Don Hinton <hintonda at gmail.com> wrote:
>
>> On Thu, Apr 4, 2019 at 7:10 PM Craig Topper <craig.topper at gmail.com>
>> wrote:
>>
>>> 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.
>>>
>>
>> I have a checker under review that could be enhanced to do that -- though
>> it currently replaces `X->foo() && isa<Y>(X->foo())` with
>> `dyn_cast_or_null<Y>(X->foo())`.
>>
>> Please see: https://reviews.llvm.org/D59802
>>
>> thanks...
>> don
>>
>>
>>>
>>> ~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/b09f8fec/attachment.html>