[libcxx-commits] [PATCH] D131362: [libc++] Implement `operator<=> for `thread::id`

[Mark de Wever via Phabricator via libcxx-commits]

Mordante added inline comments.


================
Comment at: libcxx/include/__threading_support:614
+    static _LIBCPP_INLINE_VISIBILITY
+        bool __eq_impl(__thread_id __x, __thread_id __y) _NOEXCEPT
+        { // don't pass id==0 to underlying routines
----------------
Note when implementing this according to P1614 we don't need to expose this function.


================
Comment at: libcxx/include/__threading_support:651
+#else // _LIBCPP_STD_VER <= 17
+    friend _LIBCPP_INLINE_VISIBILITY
+        strong_ordering operator<=>(__thread_id __x, __thread_id __y) noexcept
----------------
I read your reasoning about why you did it, but I think we should fix it. There's already a bug report regarding this issue.
https://github.com/llvm/llvm-project/issues/56187


================
Comment at: libcxx/include/__threading_support:651
+#else // _LIBCPP_STD_VER <= 17
+    friend _LIBCPP_INLINE_VISIBILITY
+        strong_ordering operator<=>(__thread_id __x, __thread_id __y) noexcept
----------------
Mordante wrote:
> I read your reasoning about why you did it, but I think we should fix it. There's already a bug report regarding this issue.
> https://github.com/llvm/llvm-project/issues/56187
For new code please use `_LIBCPP_HIDE_FROM_ABI` instead, both do the same but this name is the new name.


================
Comment at: libcxx/include/__threading_support:654
+        {
+           if (__eq_impl(__x, __y)) return strong_ordering::equal;
+           if (__lt_impl(__x, __y)) return strong_ordering::less;
----------------
Please use clang-format on the new code.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D131362