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

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
>

On Thu, Apr 4, 2019 at 11:15 PM Aaron Ballman via llvm-dev <
llvm-dev at lists.llvm.org> wrote:
> 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).
>
I think "isa_nonnull" would read fine too. To me, the extra
"and" makes the
ordering more of an issue.

-- HT

>
> ~Aaron
>
> >
> > -Chris
> >
> _______________________________________________
> 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/20190405/fb1baab0/attachment.html>

+1 for "isa_nonnull"
--paulr

From: llvm-dev [mailto:llvm-dev-bounces at lists.llvm.org] On Behalf Of Hubert
Tong via llvm-dev
Sent: Friday, April 05, 2019 12:10 AM
To: Aaron Ballman
Cc: LLVM Development List
Subject: Re: [llvm-dev] [RFC] Should we add isa_or_null<>?

On Thu, Apr 4, 2019 at 11:15 PM Aaron Ballman via llvm-dev <llvm-dev at
lists.llvm.org<mailto:llvm-dev at lists.llvm.org>> wrote:
On Thu, Apr 4, 2019 at 12:58 PM Chris Lattner <clattner at
nondot.org<mailto:clattner at nondot.org>>
wrote:
>
>
>
> > On Apr 4, 2019, at 5:37 AM, Don Hinton via llvm-dev <llvm-dev at
lists.llvm.org<mailto: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).
I think "isa_nonnull" would read fine too. To me, the extra
"and" makes the ordering more of an issue.

-- HT


~Aaron
>
> -Chris
>
_______________________________________________
LLVM Developers mailing list
llvm-dev at lists.llvm.org<mailto: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/20190405/4a8df2e5/attachment.html>

On Fri, Apr 5, 2019 at 5:15 AM Aaron Ballman via llvm-dev <
llvm-dev at lists.llvm.org> wrote:
> 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 --
>
I am with David on this, this sounds like misleading naming to me, I would
expect true on null value when reading : if (isa_or_null<T>(var))

we're used to _or_null() returning "the right thing" when given
null.
>
I think we're used to have "the right thing" because the name
matches the
semantic: the "_or_null()" suffix matches the semantics a conversion
operator that returns nullptr on failure.
It does not translate with isa<> IMO.


> 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).
>
isa_nonnulll works fine for me, isa_and_nonnull is a bit verbose but seems
OK as well.

For nonnull_and_isa<T>(val) ; it starts to look strangely close to the
pattern !val && isa<T>(val) ; and I'm not sure it is really
such a
readability improvement anymore?

-- 
Mehdi


>
> ~Aaron
>
> >
> > -Chris
> >
> _______________________________________________
> 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/20190406/3027753a/attachment.html>

What about a type not_null_impl<T> and we could write:

then you could just write bool x = isa<T>(not_null(val));

We provide a function not_null<T> that returns a not_null_impl<T>:

template<typename T>
not_null_impl<T> not_null(T *t) { return not_null_impl<T>{t}; }

and a specialization of isa that takes a not_null_impl<T>

template<typename T, typename U>
isa<T, not_null_impl<U>>(const not_null_impl<U> &u) {
  return u ? isa<T>(*u) : false;
}

On Sat, Apr 6, 2019 at 9:45 AM Mehdi AMINI via llvm-dev <
llvm-dev at lists.llvm.org> wrote:
>
>
> On Fri, Apr 5, 2019 at 5:15 AM Aaron Ballman via llvm-dev <
> llvm-dev at lists.llvm.org> wrote:
>
>> 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 --
>>
>
> I am with David on this, this sounds like misleading naming to me, I would
> expect true on null value when reading : if (isa_or_null<T>(var))
>
> we're used to _or_null() returning "the right thing" when
given null.
>>
>
> I think we're used to have "the right thing" because the name
matches the
> semantic: the "_or_null()" suffix matches the semantics a
conversion
> operator that returns nullptr on failure.
> It does not translate with isa<> IMO.
>
>
>
>> 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).
>>
>
> isa_nonnulll works fine for me, isa_and_nonnull is a bit verbose but seems
> OK as well.
>
> For nonnull_and_isa<T>(val) ; it starts to look strangely close to
the
> pattern !val && isa<T>(val) ; and I'm not sure it is
really such a
> readability improvement anymore?
>
> --
> Mehdi
>
>
>
>>
>> ~Aaron
>>
>> >
>> > -Chris
>> >
>> _______________________________________________
>> LLVM Developers mailing list
>> llvm-dev at lists.llvm.org
>> https://lists.llvm.org/cgi-bin/mailman/listinfo/llvm-dev
>>
> _______________________________________________
> 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/20190406/ad5183d0/attachment.html>