Mehdi AMINI via llvm-dev
2019-Apr-07 00:03 UTC
[llvm-dev] [RFC] Should we add isa_or_null<>?
I read `isa<T>(or_null(v))` as "v is a T or nullptr", which does not match the implementation semantics "v is a T and not null". On Sat, Apr 6, 2019 at 9:31 PM Zachary Turner <zturner at google.com> wrote:> Sorry, brain isn't fully working. I meant to call the function / type > `or_null` instead of `not_null` > > On Sat, Apr 6, 2019 at 11:16 AM Zachary Turner <zturner at google.com> wrote: > >> 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/20190407/105ceae2/attachment.html>
Zachary Turner via llvm-dev
2019-Apr-07 02:14 UTC
[llvm-dev] [RFC] Should we add isa_or_null<>?
In that case my original suggestion of isa<T>(not_null(v)) matches the semantics right? On Sat, Apr 6, 2019 at 5:03 PM Mehdi AMINI <joker.eph at gmail.com> wrote:> I read `isa<T>(or_null(v))` as "v is a T or nullptr", which does not > match the implementation semantics "v is a T and not null". > > On Sat, Apr 6, 2019 at 9:31 PM Zachary Turner <zturner at google.com> wrote: > >> Sorry, brain isn't fully working. I meant to call the function / type >> `or_null` instead of `not_null` >> >> On Sat, Apr 6, 2019 at 11:16 AM Zachary Turner <zturner at google.com> >> wrote: >> >>> 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/e94f002f/attachment.html>
I have to say `not_null(v)` reads more like an assertion than a predicate, in
which case `isa<T>(not_null(v))` reads like it has the exact same
semantics that `isa<T>(v)` has currently—asserts that `v` is not null.
I don't dispute that you can *make* it have the desired semantics, it just
won't *look* that way.
maybe `isaT_or_null<Foo>(v)` ? Still looks awkward but maybe less naively
misleading.
From: llvm-dev [mailto:llvm-dev-bounces at lists.llvm.org] On Behalf Of Zachary
Turner via llvm-dev
Sent: Saturday, April 06, 2019 10:15 PM
To: Mehdi AMINI
Cc: LLVM Development List; Aaron Ballman
Subject: Re: [llvm-dev] [RFC] Should we add isa_or_null<>?
In that case my original suggestion of isa<T>(not_null(v)) matches the
semantics right?
On Sat, Apr 6, 2019 at 5:03 PM Mehdi AMINI <joker.eph at
gmail.com<mailto:joker.eph at gmail.com>> wrote:
I read `isa<T>(or_null(v))` as "v is a T or nullptr", which
does not match the implementation semantics "v is a T and not null".
On Sat, Apr 6, 2019 at 9:31 PM Zachary Turner <zturner at
google.com<mailto:zturner at google.com>> wrote:
Sorry, brain isn't fully working. I meant to call the function / type
`or_null` instead of `not_null`
On Sat, Apr 6, 2019 at 11:16 AM Zachary Turner <zturner at
google.com<mailto:zturner at google.com>> wrote:
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<mailto: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<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 --
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<mailto: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<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/20190407/4dcb90c0/attachment.html>