[libcxx-commits] [PATCH] D110515: [libc++] [compare] Named comparison functions, is_eq etc.

[Arthur O'Dwyer via Phabricator via libcxx-commits]

Quuxplusone added inline comments.


================
Comment at: libcxx/include/__compare/is_eq.h:23-28
+constexpr bool is_eq(partial_ordering __c) noexcept { return __c == 0; }
+constexpr bool is_neq(partial_ordering __c) noexcept { return __c != 0; }
+constexpr bool is_lt(partial_ordering __c) noexcept { return __c < 0; }
+constexpr bool is_lteq(partial_ordering __c) noexcept { return __c <= 0; }
+constexpr bool is_gt(partial_ordering __c) noexcept { return __c > 0; }
+constexpr bool is_gteq(partial_ordering __c) noexcept { return __c >= 0; }
----------------
ldionne wrote:
> Quuxplusone wrote:
> > (1) Not marked `_LIBCPP_HIDE_FROM_ABI`, because I can't imagine their ABI will ever change, but I don't really understand the meaning of the marking, and maybe it's better to mark them; @ldionne opinion?
> > 
> > (2) TIL (again) that although `constexpr` on a global variable implies `static`, `constexpr` on a function implies `inline`. So it's correct that these header-defined functions aren't explicitly marked `inline`; they still have the right behavior.
> Yes, we want `_LIBCPP_HIDE_FROM_ABI` here, like we do everywhere else. I agree in this case we could probably skip it since they are probably never going to change, but we might as well be consistent.
> 
> I wasn't aware of the `constexpr` implies `inline` bit. I find that surprising and weird. If it were me, I wouldn't rely on that corner case and I'd mark them as `inline` explicitly, since I expect most readers of the code will go "wut?" if we don't.
> If it were me, I wouldn't rely on that corner case and I'd mark them as inline explicitly, since I expect most readers of the code will go "wut?" if we don't.

Works for me! Marked.


Repository:
  rG LLVM Github Monorepo

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D110515/new/

https://reviews.llvm.org/D110515