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

+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 05/04/2019 09:13, via llvm-dev wrote:
> I think "isa_nonnull" would read fine too. To me, the extra
"and" makes
> the ordering more of an issue.
At the risk of bikeshedding:

To me, isa_nonnull sounds as if the caller is guaranteeing that the 
argument is nonnull.  I don't think I've seen it in LLVM, but elsewhere 
I've come across a convention of adding nonnull variants of functions 
that skip null checks and pass the non-null restriction to the caller.

I wonder if the better solution is to rename isa to isa_nonnull and 
introduce a new isa that can take a null argument.  If these have the 
correct nullability annotations then anyone building with clang should 
get a warning if they use the wrong one...

David

Hi David:

On Fri, Apr 5, 2019 at 7:50 AM David Chisnall via llvm-dev <
llvm-dev at lists.llvm.org> wrote:
> On 05/04/2019 09:13, via llvm-dev wrote:
> > I think "isa_nonnull" would read fine too. To me, the extra
"and" makes
> > the ordering more of an issue.
>
> At the risk of bikeshedding:
>
> To me, isa_nonnull sounds as if the caller is guaranteeing that the
> argument is nonnull.  I don't think I've seen it in LLVM, but
elsewhere
> I've come across a convention of adding nonnull variants of functions
> that skip null checks and pass the non-null restriction to the caller.
>
> I wonder if the better solution is to rename isa to isa_nonnull and
> introduce a new isa that can take a null argument.  If these have the
> correct nullability annotations then anyone building with clang should
> get a warning if they use the wrong one...
>
I like your logic, but not sure it's feasible to detect which one you
should use in all cases, e.g., if the user already knows `p` can't be null,
they just use `isa`.  However, if it could be null, they use `p &&
isa<X>(p)`, `cast_or_null` or `dyn_cast_or_null`.  So, we could switch all
the current uses of `isa` to the null variety when we make the change, but
wouldn't we have to trust the user from them on.

Also, wouldn't this be a pretty big change across llvm and all subprojects,
and be an issue for downstream projects.

thanks...
don
>
> David
> _______________________________________________
> 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/8daa3d4f/attachment.html>