[llvm-dev] [RFC] Should we add isa_or_null<>?
Bruno Ricci via llvm-dev
llvm-dev at lists.llvm.org
Sun Apr 7 07:40:44 PDT 2019
On 07/04/2019 14:13, via llvm-dev wrote:
> 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.
>
... or we keep using "v && isa<T>(v)" (which is not even longer!), which is unambiguous for everyone.
In the rare special case where the pattern "someExpensiveCall() && isa<T>(someExpensiveCall())" is used,
just assign the result of "someExpensiveCall()" to a variable.
Bruno
>
>
>
>
> *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
>
>
> _______________________________________________
> LLVM Developers mailing list
> llvm-dev at lists.llvm.org
> https://lists.llvm.org/cgi-bin/mailman/listinfo/llvm-dev
>
More information about the llvm-dev
mailing list