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

[Don Hinton via llvm-dev]

On Sun, May 5, 2019 at 5:46 AM Aaron Ballman via llvm-dev <
llvm-dev at lists.llvm.org> wrote:

> Personally, I find having a named predicate clearly expresses intent,
> especially for the few cases where we found an expression of the
> pattern: foo() && isa<Whatever>(foo()).
>

This was the original motivation.  The clang/llvm code base handles this
two step operation in 2 ways, either as Aaron described, or by using
`dyn_cast_or_null<Whatever>(foo())`.  So, here are the options as I see
them:

1) `foo() && isa<T>(foo())` :
  - pros:  correct and easy to understand
  - cons:  evaluates `foo()` twice -- this could be an issue if `foo()` is
expensive.

2) `dyn_cast_or_null<T>(foo())` :
  - pros:  correct and easy to understand
  - cons:  while only evaluating `foo()` a single time, it does an
unnecessary cast.
           has a long name

3) `isa_and_nonnull<T>(foo())`  or `isa_nonnull<T>(foo())` or something
else :
  - pros:  correct and easy to understand (perhaps I'm bias)
           evaluated `foo()` only once
           doesn't do an unnecessary cast.
  - cons:  requires addition of additional operator
           isn't absolutely needed
           has a long name -- but not as long as `dyn_cast_or_null<T>()`

Personally, I'm not overly invested in any of the above options, though I
would need to remove the new operator and modify the clang-tidy check to do
the right thing if we do decide to remove it.

However, I did want to mention that the `_or_null` suffix in
`cast_or_null<T>()` and `dyn_cast_or_null<T>()` has nothing to do with the
return type.  The suffix only means that that version of the operator
"accepts" null pointers.  So, it seems to me that `isa_or_null<T>()` would
have been the best name.  Anyone confused by the `_or_null` suffix should
have also had a problem with the `cast_or_null<T>()` and
`dyn_cast_or_null<T>()` operators for the same reason.  It's possible that
might be true, but I wasn't around and haven't researched it.

I'll keep watching this thread and will be happy to take action on whatever
you guys decide.

thanks..
don


> ~Aaron
>
> On Sun, May 5, 2019 at 1:52 AM Sean Silva via llvm-dev
> <llvm-dev at lists.llvm.org> wrote:
> >
> > +1 on not adding the new API
> >
> > On Sat, May 4, 2019, 11:51 AM David Blaikie via llvm-dev <
> llvm-dev at lists.llvm.org> wrote:
> >>
> >> +1, if we're voting. I don't think it adds to the readability of code
> >> for me personally.
> >>
> >> On Sat, May 4, 2019 at 11:47 AM Chandler Carruth via llvm-dev
> >> <llvm-dev at lists.llvm.org> wrote:
> >> >
> >> > On Mon, Apr 29, 2019, 02:37 David Chisnall via llvm-dev <
> llvm-dev at lists.llvm.org> wrote:
> >> >>
> >> >> On 22/04/2019 15:15, Don Hinton via llvm-dev wrote:
> >> >> > Although there were a few no votes, it looks like there's a
> consensus
> >> >> > for adding a `isa_and_nonnull` type operator.  While there were
> some who
> >> >> > preferred `isa_nonnull`, it wasn't overwhelming, and since
> >> >> > `isa_and_nonnull` is already committed, I'm going to leave it as
> >> >> > `isa_and_nonnull` for the time being.
> >> >>
> >> >> Maybe I missed something, but it looked to me as if the consensus was
> >> >> that `isa_and_some_words<T>(foo)` imposed a higher cognitive load on
> the
> >> >> reader than `foo && isa<T>(foo)`, as well as being more to type in
> most
> >> >> cases, so wasn't worth adding.
> >> >
> >> >
> >> > FWIW, I agree with this and Bogner: this doesn't seem like an
> improvement worth the cost.
> >> >
> >> >>
> >> >> David
> >> >> _______________________________________________
> >> >> 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
> _______________________________________________
> 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/20190505/e3bc6693/attachment.html>