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

I'd like to propose adding `isa_or_null<>` to replace the following
usage
pattern that's relatively common in conditionals:

  var && isa<T>(var)  =>>  isa_or_null<T>(var)

And in particular when `var` is a method call which might be expensive,
e.g.:

  X->foo() && isa<T>(X->foo())  =>> 
isa_or_null<T>(X->foo())

The implementation could be a simple wrapper around isa<>, and while the
IR
produced is only slightly more efficient, the elimination of an extra call
could be worthwhile.
-------------- next part --------------
An HTML attachment was scrubbed...
URL:
<http://lists.llvm.org/pipermail/llvm-dev/attachments/20190404/cf573877/attachment.html>

I don't think that's a correct replacement.

if (var && isa<T>(var)) {
  ...
}

is not the same as:

if (isa_or_null<T>(var)) {
  ...
}

at least according to what "isa_or_null" conveys to me.

not_null_and_isa<T> would seem a better fit, or maybe
exists_and_isa<T>.

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?

                   -David

Don Hinton via llvm-dev <llvm-dev at lists.llvm.org> writes:
> I'd like to propose adding `isa_or_null<>` to replace the
following usage pattern that's relatively common in conditionals:
>
>   var && isa<T>(var)  =>>  isa_or_null<T>(var)
>
> And in particular when `var` is a method call which might be expensive,
e.g.:
>
>   X->foo() && isa<T>(X->foo())  =>> 
isa_or_null<T>(X->foo())
>
> The implementation could be a simple wrapper around isa<>, and while
the IR produced is only slightly more efficient, the elimination of an extra
call could be
> worthwhile.
>
> _______________________________________________
> LLVM Developers mailing list
> llvm-dev at lists.llvm.org
> https://lists.llvm.org/cgi-bin/mailman/listinfo/llvm-dev

On Thu, Apr 4, 2019 at 10:30 AM David Greene <dag at cray.com> wrote:
> I don't think that's a correct replacement.
>
> if (var && isa<T>(var)) {
>   ...
> }
>
> is not the same as:
>
> 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.

>
> not_null_and_isa<T> would seem a better fit, or maybe
exists_and_isa<T>.
>
> 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.

>
>                    -David
>
> Don Hinton via llvm-dev <llvm-dev at lists.llvm.org> writes:
>
> > I'd like to propose adding `isa_or_null<>` to replace the
following
> usage pattern that's relatively common in conditionals:
> >
> >   var && isa<T>(var)  =>> 
isa_or_null<T>(var)
> >
> > And in particular when `var` is a method call which might be
expensive,
> e.g.:
> >
> >   X->foo() && isa<T>(X->foo())  =>> 
isa_or_null<T>(X->foo())
> >
> > The implementation could be a simple wrapper around isa<>, and
while the
> IR produced is only slightly more efficient, the elimination of an extra
> call could be
> > worthwhile.
> >
> > _______________________________________________
> > 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/245976fd/attachment.html>

> On Apr 4, 2019, at 5:37 AM, Don Hinton via llvm-dev <llvm-dev at
lists.llvm.org> wrote:
> 
> I'd like to propose adding `isa_or_null<>` to replace the
following usage pattern that's relatively common in conditionals:
> 
>   var && isa<T>(var)  =>>  isa_or_null<T>(var)
> 
> And in particular when `var` is a method call which might be expensive,
e.g.:
> 
>   X->foo() && isa<T>(X->foo())  =>> 
isa_or_null<T>(X->foo())
> 
> The implementation could be a simple wrapper around isa<>, and while
the IR produced is only slightly more efficient, the elimination of an extra
call could be worthwhile.
I’d love to see this, I agree with downstream comments though that this name
will be confusing.  isa_and_nonnull<>. ?

-Chris

On Thu, Apr 4, 2019 at 12:58 PM Chris Lattner <clattner at nondot.org>
wrote:
>
>
>
> > On Apr 4, 2019, at 5:37 AM, Don Hinton via llvm-dev <llvm-dev at
lists.llvm.org> wrote:
> >
> > I'd like to propose adding `isa_or_null<>` to replace the
following usage pattern that's relatively common in conditionals:
> >
> >   var && isa<T>(var)  =>> 
isa_or_null<T>(var)
> >
> > And in particular when `var` is a method call which might be
expensive, e.g.:
> >
> >   X->foo() && isa<T>(X->foo())  =>> 
isa_or_null<T>(X->foo())
> >
> > The implementation could be a simple wrapper around isa<>, and
while the IR produced is only slightly more efficient, the elimination of an
extra call could be worthwhile.
>
> I’d love to see this, I agree with downstream comments though that this
name will be confusing.  isa_and_nonnull<>. ?
tbh, I don't think the proposed name will be all that confusing --
we're used to _or_null() returning "the right thing" when given
null.
isa_and_nonnull<> is a bit of a weird name for me, but I could
probably live with it. We could spell it nonnull_and_isa<> to reflect
the order of the operations, but that sort of hides the important part
of the API (the "isa" bit).

~Aaron
>
> -Chris
>