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

[Don Hinton via llvm-dev]

On Wed, Apr 10, 2019 at 11:37 AM David Greene <dag at cray.com> wrote:

> Don Hinton via llvm-dev <llvm-dev at lists.llvm.org> writes:
>
> > Used like this:  isa<T,1>(v) or isa<T, true>(v)
>
> I don't think I would know what that means when I see it in code.
>

Sorry, I was just trying to highlight a different way of doing it, and
lazily used a bool.  Here's something a little more realistic:

struct NulCheck : std::true_type {};

template <class X, class Check, class Y,
          typename std::enable_if<Check::value, X>::type * = nullptr>
LLVM_NODISCARD inline bool isa(const Y *Val) {
  return Val && isa<X>(Val);
}

isa<foo,NulCheck>(v)

Not sure if that's better than isa_and_nonnull or isa_nonnull, but it's
shorter than the first, and only slightly longer than the second.




>
>                        -David
>
> >  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
> >
> >  _______________________________________________
> >  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/20190411/e8b2dc1b/attachment.html>