Don Hinton via llvm-dev
2019-Apr-04 23:16 UTC
[llvm-dev] [RFC] Should we add isa_or_null<>?
I've added a patch, temporarily using the name Chris suggested. Please let me know what you think. https://reviews.llvm.org/D60291 thanks... don On Thu, Apr 4, 2019 at 2:55 PM David Greene <dag at cray.com> wrote:> Don Hinton <hintonda at gmail.com> writes: > > > > if (isa_or_null<T>(var)) { > > > ... > > > } > > > > > > at least according to what "isa_or_null" conveys to me. > > > > This is the same convention used by the existing "_or_null" varieties, > > i.e., "cast_or_null" and "dyn_cast_or_null". They accept a null and > > propagate it. In the "isa" case, it would accept a null and propagate > > it as false. > > isa<> is very different from *cast<>. *cast<> gives you a pointer back, > which may be null. isa<> is precondition check, so it "reads" > differently to me. If I were to see: > > if (isa_or_null<T>(var)) { > ... > } > > I would think, "Ok, the body is fine if var is null." > > Instead: > > if (exists_and_isa<T>(var)) { > ... > } > > This tells me that the body expects a non-null value. > > > > That said, I'm not sure sure we need a special API for this. Are > > > expensive calls used in the way you describe really common? > > > > I've only been looking at the ones involving method calls, but it's > > not too common. Perhaps a dozen in clang/lib -- haven't run it > > against the rest of the code base. > > Thanks for checking. I don't have a strong opinion about the need > either way, but I do care that the spelling is clear and intuitive. > > -David >-------------- next part -------------- An HTML attachment was scrubbed... URL: <http://lists.llvm.org/pipermail/llvm-dev/attachments/20190404/b831ec94/attachment.html>
Craig Topper via llvm-dev
2019-Apr-04 23:29 UTC
[llvm-dev] [RFC] Should we add isa_or_null<>?
There are a handful of places in LLVM that dosomething like if (dyn_cast_or_null<UndefValue>(P->hasConstantValue())) ~Craig On Thu, Apr 4, 2019 at 4:16 PM Don Hinton via llvm-dev < llvm-dev at lists.llvm.org> wrote:> I've added a patch, temporarily using the name Chris suggested. Please > let me know what you think. > > https://reviews.llvm.org/D60291 > > thanks... > don > > On Thu, Apr 4, 2019 at 2:55 PM David Greene <dag at cray.com> wrote: > >> Don Hinton <hintonda at gmail.com> writes: >> >> > > if (isa_or_null<T>(var)) { >> > > ... >> > > } >> > > >> > > at least according to what "isa_or_null" conveys to me. >> > >> > This is the same convention used by the existing "_or_null" varieties, >> > i.e., "cast_or_null" and "dyn_cast_or_null". They accept a null and >> > propagate it. In the "isa" case, it would accept a null and propagate >> > it as false. >> >> isa<> is very different from *cast<>. *cast<> gives you a pointer back, >> which may be null. isa<> is precondition check, so it "reads" >> differently to me. If I were to see: >> >> if (isa_or_null<T>(var)) { >> ... >> } >> >> I would think, "Ok, the body is fine if var is null." >> >> Instead: >> >> if (exists_and_isa<T>(var)) { >> ... >> } >> >> This tells me that the body expects a non-null value. >> >> > > That said, I'm not sure sure we need a special API for this. Are >> > > expensive calls used in the way you describe really common? >> > >> > I've only been looking at the ones involving method calls, but it's >> > not too common. Perhaps a dozen in clang/lib -- haven't run it >> > against the rest of the code base. >> >> Thanks for checking. I don't have a strong opinion about the need >> either way, but I do care that the spelling is clear and intuitive. >> >> -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/20190404/3451052b/attachment.html>
Don Hinton via llvm-dev
2019-Apr-04 23:37 UTC
[llvm-dev] [RFC] Should we add isa_or_null<>?
On Thu, Apr 4, 2019 at 6:29 PM Craig Topper <craig.topper at gmail.com> wrote:> There are a handful of places in LLVM that dosomething like if > (dyn_cast_or_null<UndefValue>(P->hasConstantValue())) >Yes, I've seen those, but while working on a new checker, I was advised that replacing `X && isa<Y>(X)` with `dyn_cast_or_null<Y>(X)` was suboptimal, and it was suggested something like a `isa_or_null` style operator would better express what was actually going on, i.e., we are expecting a bool, not a pointer.> > ~Craig > > > On Thu, Apr 4, 2019 at 4:16 PM Don Hinton via llvm-dev < > llvm-dev at lists.llvm.org> wrote: > >> I've added a patch, temporarily using the name Chris suggested. Please >> let me know what you think. >> >> https://reviews.llvm.org/D60291 >> >> thanks... >> don >> >> On Thu, Apr 4, 2019 at 2:55 PM David Greene <dag at cray.com> wrote: >> >>> Don Hinton <hintonda at gmail.com> writes: >>> >>> > > if (isa_or_null<T>(var)) { >>> > > ... >>> > > } >>> > > >>> > > at least according to what "isa_or_null" conveys to me. >>> > >>> > This is the same convention used by the existing "_or_null" varieties, >>> > i.e., "cast_or_null" and "dyn_cast_or_null". They accept a null and >>> > propagate it. In the "isa" case, it would accept a null and propagate >>> > it as false. >>> >>> isa<> is very different from *cast<>. *cast<> gives you a pointer back, >>> which may be null. isa<> is precondition check, so it "reads" >>> differently to me. If I were to see: >>> >>> if (isa_or_null<T>(var)) { >>> ... >>> } >>> >>> I would think, "Ok, the body is fine if var is null." >>> >>> Instead: >>> >>> if (exists_and_isa<T>(var)) { >>> ... >>> } >>> >>> This tells me that the body expects a non-null value. >>> >>> > > That said, I'm not sure sure we need a special API for this. Are >>> > > expensive calls used in the way you describe really common? >>> > >>> > I've only been looking at the ones involving method calls, but it's >>> > not too common. Perhaps a dozen in clang/lib -- haven't run it >>> > against the rest of the code base. >>> >>> Thanks for checking. I don't have a strong opinion about the need >>> either way, but I do care that the spelling is clear and intuitive. >>> >>> -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/20190404/7ee2ec01/attachment-0001.html>