David Greene via llvm-dev
2019-Apr-10 18:37 UTC
[llvm-dev] [RFC] Should we add isa_or_null<>?
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. -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
Don Hinton via llvm-dev
2019-Apr-12 02:49 UTC
[llvm-dev] [RFC] Should we add isa_or_null<>?
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>
Don Hinton via llvm-dev
2019-Apr-22 14:15 UTC
[llvm-dev] [RFC] Should we add isa_or_null<>?
Hi All: Just wanted to wind this up and summarize the results. 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. Thanks for all the comments. don On Thu, Apr 11, 2019 at 7:49 PM Don Hinton <hintonda at gmail.com> wrote:> 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/20190422/766c9e32/attachment.html>