[libcxx-commits] [libcxx] 1c1edd1 - [libc++] Fixes thread::id's operator<<.

Mark de Wever via libcxx-commits libcxx-commits at lists.llvm.org
Sat Jul 15 02:52:25 PDT 2023


Author: Mark de Wever
Date: 2023-07-15T11:52:19+02:00
New Revision: 1c1edd1b5d00daae2efdfbc5754a9fae77597dd4

URL: https://github.com/llvm/llvm-project/commit/1c1edd1b5d00daae2efdfbc5754a9fae77597dd4
DIFF: https://github.com/llvm/llvm-project/commit/1c1edd1b5d00daae2efdfbc5754a9fae77597dd4.diff

LOG: [libc++] Fixes thread::id's operator<<.

The output of

  template<class charT, class traits>
    basic_ostream<charT, traits>&
      operator<<(basic_ostream<charT, traits>& out, thread::id id);

is affected by the state of out. The wording states

[thread.thread.id]/2
  The text representation for the character type charT of an object of
  type thread::id is an unspecified sequence of charT such that, for two
  objects of type thread::id x and y, if x == y is true, the thread::id
  objects have the same text representation, and if x != y is true, the
  thread::id objects have distinct text representations.

[thread.thread.id]/9
  template<class charT, class traits>
    basic_ostream<charT, traits>&
      operator<< (basic_ostream<charT, traits>& out, thread::id id);

  Effects: Inserts the text representation for charT of id into out.

This wording changed in C++23 due to adding a formatter specialization for
thread::id. However the requirement was the same in older versions of C++.

This issue is that thread::id is an integral or pointer and affected by the
formatting manipulators for them. Thus the text representation can differ if
x == y which violates the requirements.

The fix has to hard-code some formatting style for the text
representation. It uses the Standard specified default values

Table 124: basic_ios::init() effects [tab:basic.ios.cons] flags()
  flags() skipws | dec

Fixes PR: https://llvm.org/PR62073

Reviewed By: #libc, ldionne

Differential Revision: https://reviews.llvm.org/D153336

Added: 
    

Modified: 
    libcxx/include/__thread/thread.h
    libcxx/test/std/thread/thread.threads/thread.thread.class/thread.thread.id/stream.pass.cpp

Removed: 
    


################################################################################
diff  --git a/libcxx/include/__thread/thread.h b/libcxx/include/__thread/thread.h
index 3d52d87f702689..dec40c7dc1f04c 100644
--- a/libcxx/include/__thread/thread.h
+++ b/libcxx/include/__thread/thread.h
@@ -24,6 +24,10 @@
 #include <iosfwd>
 #include <tuple>
 
+#ifndef _LIBCPP_HAS_NO_LOCALIZATION
+#  include <locale>
+#endif
+
 #if !defined(_LIBCPP_HAS_NO_PRAGMA_SYSTEM_HEADER)
 #  pragma GCC system_header
 #endif
@@ -126,11 +130,32 @@ struct _LIBCPP_TEMPLATE_VIS hash<__thread_id>
     }
 };
 
-template<class _CharT, class _Traits>
-_LIBCPP_INLINE_VISIBILITY
-basic_ostream<_CharT, _Traits>&
-operator<<(basic_ostream<_CharT, _Traits>& __os, __thread_id __id)
-{return __os << __id.__id_;}
+#ifndef _LIBCPP_HAS_NO_LOCALIZATION
+template <class _CharT, class _Traits>
+_LIBCPP_INLINE_VISIBILITY basic_ostream<_CharT, _Traits>&
+operator<<(basic_ostream<_CharT, _Traits>& __os, __thread_id __id) {
+    // [thread.thread.id]/9
+    //   Effects: Inserts the text representation for charT of id into out.
+    //
+    // [thread.thread.id]/2
+    //   The text representation for the character type charT of an
+    //   object of type thread::id is an unspecified sequence of charT
+    //   such that, for two objects of type thread::id x and y, if
+    //   x == y is true, the thread::id objects have the same text
+    //   representation, and if x != y is true, the thread::id objects
+    //   have distinct text representations.
+    //
+    // Since various flags in the output stream can affect how the
+    // thread id is represented (e.g. numpunct or showbase), we
+    // use a temporary stream instead and just output the thread
+    // id representation as a string.
+
+    basic_ostringstream<_CharT, _Traits> __sstr;
+    __sstr.imbue(locale::classic());
+    __sstr << __id.__id_;
+    return __os << __sstr.str();
+}
+#endif // _LIBCPP_HAS_NO_LOCALIZATION
 
 class _LIBCPP_EXPORTED_FROM_ABI thread
 {

diff  --git a/libcxx/test/std/thread/thread.threads/thread.thread.class/thread.thread.id/stream.pass.cpp b/libcxx/test/std/thread/thread.threads/thread.thread.class/thread.thread.id/stream.pass.cpp
index b89a1bb55628f4..97e797f9a8fbb9 100644
--- a/libcxx/test/std/thread/thread.threads/thread.thread.class/thread.thread.id/stream.pass.cpp
+++ b/libcxx/test/std/thread/thread.threads/thread.thread.class/thread.thread.id/stream.pass.cpp
@@ -10,6 +10,8 @@
 // UNSUPPORTED: no-localization
 // UNSUPPORTED: GCC-ALWAYS_INLINE-FIXME
 
+// REQUIRES: locale.fr_FR.UTF-8
+
 // TODO FMT This test should not require std::to_chars(floating-point)
 // XFAIL: availability-fp_to_chars-missing
 
@@ -21,16 +23,18 @@
 // basic_ostream<charT, traits>&
 // operator<<(basic_ostream<charT, traits>& out, thread::id id);
 
-#include <thread>
+#include <cassert>
 #include <format>
+#include <locale>
 #include <sstream>
-#include <cassert>
+#include <thread>
 
 #include "make_string.h"
+#include "platform_support.h" // locale name macros
 #include "test_macros.h"
 
 template <class CharT>
-static void test() {
+static void basic() {
   std::thread::id id0 = std::this_thread::get_id();
   std::basic_ostringstream<CharT> os;
   os << id0;
@@ -49,6 +53,91 @@ static void test() {
 #endif
 }
 
+template <class CharT>
+static std::basic_string<CharT> format(std::ios_base::fmtflags flags) {
+  std::basic_stringstream<CharT> sstr;
+  sstr.flags(flags);
+  sstr << std::this_thread::get_id();
+  return sstr.str();
+}
+
+template <class CharT>
+static void stream_state() {
+  std::basic_stringstream<CharT> sstr;
+  sstr << std::this_thread::get_id();
+  std::basic_string<CharT> expected = sstr.str();
+
+  // Unaffected by fill, width, and align.
+
+  assert(expected == format<CharT>(std::ios_base::dec | std::ios_base::skipws)); // default flags
+
+  assert(expected == format<CharT>(std::ios_base::oct));
+  assert(expected == format<CharT>(std::ios_base::hex));
+
+  assert(expected == format<CharT>(std::ios_base::scientific));
+  assert(expected == format<CharT>(std::ios_base::fixed));
+
+  assert(expected == format<CharT>(std::ios_base::boolalpha));
+  assert(expected == format<CharT>(std::ios_base::showbase));
+  assert(expected == format<CharT>(std::ios_base::showpoint));
+  assert(expected == format<CharT>(std::ios_base::showpos));
+  assert(expected == format<CharT>(std::ios_base::skipws));  // added for completeness
+  assert(expected == format<CharT>(std::ios_base::unitbuf)); // added for completeness
+  assert(expected == format<CharT>(std::ios_base::uppercase));
+
+  // Test fill, width, and align.
+
+  sstr.str(std::basic_string<CharT>());
+  sstr.fill(CharT('#'));
+  sstr.width(expected.size() + 10); // Make sure fill and align affect the output.
+  sstr.flags(std::ios_base::dec | std::ios_base::skipws | std::ios_base::right);
+  sstr << std::this_thread::get_id();
+  expected = sstr.str();
+
+  sstr.str(std::basic_string<CharT>());
+  sstr.fill(CharT('*'));
+  sstr.width(expected.size());
+  sstr.flags(std::ios_base::dec | std::ios_base::skipws | std::ios_base::right);
+  sstr << std::this_thread::get_id();
+  assert(expected != sstr.str());
+
+  sstr.str(std::basic_string<CharT>());
+  sstr.fill(CharT('#'));
+  sstr.width(expected.size() - 1);
+  sstr.flags(std::ios_base::dec | std::ios_base::skipws | std::ios_base::right);
+  sstr << std::this_thread::get_id();
+  assert(expected != sstr.str());
+
+  sstr.str(std::basic_string<CharT>());
+  sstr.fill(CharT('#'));
+  sstr.width(expected.size());
+  sstr.flags(std::ios_base::dec | std::ios_base::skipws | std::ios_base::left);
+  sstr << std::this_thread::get_id();
+  assert(expected != sstr.str());
+
+  sstr.str(std::basic_string<CharT>());
+  sstr.fill(CharT('#'));
+  sstr.width(expected.size());
+  sstr.flags(std::ios_base::dec | std::ios_base::skipws | std::ios_base::internal);
+  sstr << std::this_thread::get_id();
+  assert(expected == sstr.str()); // internal does *not* affect strings
+
+  // Test the locale's numpunct.
+
+  std::locale::global(std::locale(LOCALE_fr_FR_UTF_8));
+  sstr.str(std::basic_string<CharT>());
+  sstr.fill(CharT('#'));
+  sstr.width(expected.size());
+  sstr << std::this_thread::get_id();
+  assert(expected == sstr.str());
+}
+
+template <class CharT>
+static void test() {
+  basic<CharT>();
+  stream_state<CharT>();
+}
+
 int main(int, char**) {
   test<char>();
 #ifndef TEST_HAS_NO_WIDE_CHARACTERS


        


More information about the libcxx-commits mailing list