<div dir="ltr"><div dir="ltr"><br></div><br><div class="gmail_quote"><div dir="ltr" class="gmail_attr">On Sun, May 5, 2019 at 5:46 AM Aaron Ballman via llvm-dev <<a href="mailto:llvm-dev@lists.llvm.org">llvm-dev@lists.llvm.org</a>> wrote:<br></div><blockquote class="gmail_quote" style="margin:0px 0px 0px 0.8ex;border-left:1px solid rgb(204,204,204);padding-left:1ex">Personally, I find having a named predicate clearly expresses intent,<br>
especially for the few cases where we found an expression of the<br>
pattern: foo() && isa<Whatever>(foo()).<br></blockquote><div><br></div><div>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:</div><div><br></div><div><font face="monospace, monospace">1) `foo() && isa<T>(foo())` : </font></div><div><font face="monospace, monospace">  - pros:  correct and easy to understand</font></div><div><font face="monospace, monospace">  - cons:  evaluates `foo()` twice -- this could be an issue if `foo()` is expensive.</font></div><div><font face="monospace, monospace"><br></font></div><div><font face="monospace, monospace">2) `dyn_cast_or_null<T>(foo())` :</font></div><div><font face="monospace, monospace">  - pros:  correct and easy to understand</font></div><div><font face="monospace, monospace">  - cons:  while only evaluating `foo()` a single time, it does an unnecessary cast.</font></div><div><font face="monospace, monospace">           has a long name</font></div><div><font face="monospace, monospace"><br></font></div><div><font face="monospace, monospace">3) `isa_and_nonnull<T>(foo())`  or `isa_nonnull<T>(foo())` or something else :</font></div><div><font face="monospace, monospace">  - pros:  correct and easy to understand (perhaps I'm bias)</font></div><div><font face="monospace, monospace">           evaluated `foo()` only once</font></div><div><font face="monospace, monospace">           doesn't do an unnecessary cast.</font></div><div><font face="monospace, monospace">  - cons:  requires addition of additional operator</font></div><div><font face="monospace, monospace">           isn't absolutely needed</font></div><div><font face="monospace, monospace">           has a long name -- but not as long as `dyn_cast_or_null<T>()`</font></div><div><font face="monospace, monospace"><br></font></div><div><font face="arial, helvetica, sans-serif">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.  </font></div><div><font face="arial, helvetica, sans-serif"><br></font></div><div><font face="arial, helvetica, sans-serif">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.</font></div><div> </div><div>I'll keep watching this thread and will be happy to take action on whatever you guys decide.</div><div><br></div><div>thanks..</div><div>don</div><div><br></div><blockquote class="gmail_quote" style="margin:0px 0px 0px 0.8ex;border-left:1px solid rgb(204,204,204);padding-left:1ex">
<br>
~Aaron<br>
<br>
On Sun, May 5, 2019 at 1:52 AM Sean Silva via llvm-dev<br>
<<a href="mailto:llvm-dev@lists.llvm.org" target="_blank">llvm-dev@lists.llvm.org</a>> wrote:<br>
><br>
> +1 on not adding the new API<br>
><br>
> On Sat, May 4, 2019, 11:51 AM David Blaikie via llvm-dev <<a href="mailto:llvm-dev@lists.llvm.org" target="_blank">llvm-dev@lists.llvm.org</a>> wrote:<br>
>><br>
>> +1, if we're voting. I don't think it adds to the readability of code<br>
>> for me personally.<br>
>><br>
>> On Sat, May 4, 2019 at 11:47 AM Chandler Carruth via llvm-dev<br>
>> <<a href="mailto:llvm-dev@lists.llvm.org" target="_blank">llvm-dev@lists.llvm.org</a>> wrote:<br>
>> ><br>
>> > On Mon, Apr 29, 2019, 02:37 David Chisnall via llvm-dev <<a href="mailto:llvm-dev@lists.llvm.org" target="_blank">llvm-dev@lists.llvm.org</a>> wrote:<br>
>> >><br>
>> >> On 22/04/2019 15:15, Don Hinton via llvm-dev wrote:<br>
>> >> > Although there were a few no votes, it looks like there's a consensus<br>
>> >> > for adding a `isa_and_nonnull` type operator.  While there were some who<br>
>> >> > preferred `isa_nonnull`, it wasn't overwhelming, and since<br>
>> >> > `isa_and_nonnull` is already committed, I'm going to leave it as<br>
>> >> > `isa_and_nonnull` for the time being.<br>
>> >><br>
>> >> Maybe I missed something, but it looked to me as if the consensus was<br>
>> >> that `isa_and_some_words<T>(foo)` imposed a higher cognitive load on the<br>
>> >> reader than `foo && isa<T>(foo)`, as well as being more to type in most<br>
>> >> cases, so wasn't worth adding.<br>
>> ><br>
>> ><br>
>> > FWIW, I agree with this and Bogner: this doesn't seem like an improvement worth the cost.<br>
>> ><br>
>> >><br>
>> >> David<br>
>> >> _______________________________________________<br>
>> >> LLVM Developers mailing list<br>
>> >> <a href="mailto:llvm-dev@lists.llvm.org" target="_blank">llvm-dev@lists.llvm.org</a><br>
>> >> <a href="https://lists.llvm.org/cgi-bin/mailman/listinfo/llvm-dev" rel="noreferrer" target="_blank">https://lists.llvm.org/cgi-bin/mailman/listinfo/llvm-dev</a><br>
>> ><br>
>> > _______________________________________________<br>
>> > LLVM Developers mailing list<br>
>> > <a href="mailto:llvm-dev@lists.llvm.org" target="_blank">llvm-dev@lists.llvm.org</a><br>
>> > <a href="https://lists.llvm.org/cgi-bin/mailman/listinfo/llvm-dev" rel="noreferrer" target="_blank">https://lists.llvm.org/cgi-bin/mailman/listinfo/llvm-dev</a><br>
>> _______________________________________________<br>
>> LLVM Developers mailing list<br>
>> <a href="mailto:llvm-dev@lists.llvm.org" target="_blank">llvm-dev@lists.llvm.org</a><br>
>> <a href="https://lists.llvm.org/cgi-bin/mailman/listinfo/llvm-dev" rel="noreferrer" target="_blank">https://lists.llvm.org/cgi-bin/mailman/listinfo/llvm-dev</a><br>
><br>
> _______________________________________________<br>
> LLVM Developers mailing list<br>
> <a href="mailto:llvm-dev@lists.llvm.org" target="_blank">llvm-dev@lists.llvm.org</a><br>
> <a href="https://lists.llvm.org/cgi-bin/mailman/listinfo/llvm-dev" rel="noreferrer" target="_blank">https://lists.llvm.org/cgi-bin/mailman/listinfo/llvm-dev</a><br>
_______________________________________________<br>
LLVM Developers mailing list<br>
<a href="mailto:llvm-dev@lists.llvm.org" target="_blank">llvm-dev@lists.llvm.org</a><br>
<a href="https://lists.llvm.org/cgi-bin/mailman/listinfo/llvm-dev" rel="noreferrer" target="_blank">https://lists.llvm.org/cgi-bin/mailman/listinfo/llvm-dev</a><br>
</blockquote></div></div>