[libcxx-commits] [libcxx] [libc++] Consolidate ordering/hashing/formatting of std::thread::id. (PR #195763)

[Petr Hosek via libcxx-commits]

================
@@ -35,24 +39,70 @@ template <>
 struct hash<__thread_id>;
 
 class __thread_id {
-  // FIXME: pthread_t is a pointer on Darwin but a long on Linux.
-  // NULL is the no-thread value on Darwin.  Someone needs to check
-  // on other platforms.  We assume 0 works everywhere for now.
   __libcpp_thread_id __id_;
 
-  static _LIBCPP_HIDE_FROM_ABI bool
-  __lt_impl(__thread_id __x, __thread_id __y) _NOEXCEPT { // id==0 is always less than any other thread_id
+  // Even though __libcpp_thread_id is provided by underlying threading implementation
+  // (e.g. C11, pthreads, or Windows) its type may still be unspecified. E.g. for pthreads
+  // implementation __libcpp_thread_id is an alias for pthread_t, which is left unspecified
+  // in POSIX. Typically it's either an integral type (glibc) or a pointer (Apple systems),
+  // but it can also be an opaque type on some systems / libc implementations.
+  //
+  // Note that in order to satisfy standard requirements on std::thread::id, we need:
+  // * strong total order
+  // * formatter support
+  // * std::hash implementation
+  // Currently, we can implement all of the above only on pointer and integral types.
+  using _Tp = __libcpp_thread_id;
+  static_assert(is_pointer<_Tp>::value || is_integral<_Tp>::value,
+                "unsupported thread::id type, please file a bug report");
+
+  // Strong total order implementation.
+  // Here we provide a best-effort implementation of strong total order, comparing
+  // integral types as-is and routing pointers through uintptr_t for a well-defined comparison.
+  template <typename Thr, typename enable_if<is_pointer<typename Thr::_Tp>::value, int>::type = 0>
+  static _LIBCPP_HIDE_FROM_ABI bool __eq_impl(Thr __x, Thr __y) _NOEXCEPT {
+    return reinterpret_cast<uintptr_t>(__x.__id_) == reinterpret_cast<uintptr_t>(__y.__id_);
+  }
+  template <typename Thr, typename enable_if<is_integral<typename Thr::_Tp>::value, int>::type = 0>
+  static _LIBCPP_HIDE_FROM_ABI bool __eq_impl(Thr __x, Thr __y) _NOEXCEPT {
+    return __x.__id_ == __y.__id_;
+  }
+
+  template <typename Thr, typename enable_if<is_pointer<typename Thr::_Tp>::value, int>::type = 0>
+  static _LIBCPP_HIDE_FROM_ABI bool __lt_impl(Thr __x, Thr __y) _NOEXCEPT {
+    return reinterpret_cast<uintptr_t>(__x.__id_) < reinterpret_cast<uintptr_t>(__y.__id_);
+  }
+  template <typename Thr, typename enable_if<is_integral<typename Thr::_Tp>::value, int>::type = 0>
+  static _LIBCPP_HIDE_FROM_ABI bool __lt_impl(Thr __x, Thr __y) _NOEXCEPT {
+    // For integral thread IDs, assume 0 is always less than any other thread_id.
     if (__x.__id_ == 0)
       return __y.__id_ != 0;
     if (__y.__id_ == 0)
       return false;
-    return __libcpp_thread_id_less(__x.__id_, __y.__id_);
+    return __x.__id_ < __y.__id_;
   }
 
+  // Hashing implementation.
+  // Simply use the underlying pointer or integral types as-is.
+  using _HashTp = _Tp;
+  _LIBCPP_HIDE_FROM_ABI _HashTp __hash_value() const { return __id_; }
+
+  // Formatter implementation.
+  // Note the output should match what the stream operator does. Since
+  // the ostream operator has been shipped years before the formatter
+  // was added to the Standard, our logic mimics what the stream
+  // operator does (i.e. prints thread-id as integer, but uses a hexadecimal
+  // format if it's represented by a pointer)
+  using _FormatterTp = conditional<is_integral<_Tp>::value, _Tp, uintptr_t>::type;
+
+  static _LIBCPP_CONSTEXPR const bool __PRINT_AS_HEX = is_pointer<_Tp>::value;
+
+  _LIBCPP_HIDE_FROM_ABI _FormatterTp __get_formatter_value() const { return reinterpret_cast<_FormatterTp>(__id_); }
+
 public:
-  _LIBCPP_HIDE_FROM_ABI __thread_id() _NOEXCEPT : __id_(0) {}
+  _LIBCPP_HIDE_FROM_ABI __thread_id() _NOEXCEPT : __id_(__libcpp_thread_id()) {}
 
-  _LIBCPP_HIDE_FROM_ABI void __reset() { __id_ = 0; }
+  _LIBCPP_HIDE_FROM_ABI void __reset() { __id_ = __libcpp_thread_id(); }
----------------
petrhosek wrote:

We already have the `_LIBCPP_NULL_THREAD` macro to handle the initialization case which should be used here.
```suggestion
  _LIBCPP_HIDE_FROM_ABI __thread_id() _NOEXCEPT : __id_(_LIBCPP_NULL_THREAD) {}

  _LIBCPP_HIDE_FROM_ABI void __reset() { __id_ = _LIBCPP_NULL_THREAD; }
```

https://github.com/llvm/llvm-project/pull/195763