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

[Mark de Wever via Phabricator via libcxx-commits]

Mordante created this revision.
Herald added a project: All.
Mordante updated this revision to Diff 532917.
Mordante added a comment.
Mordante updated this revision to Diff 532934.
Mordante updated this revision to Diff 533144.
Mordante added a subscriber: ldionne.
Mordante published this revision for review.
Herald added a project: libc++.
Herald added a subscriber: libcxx-commits.
Herald added a reviewer: libc++.

CI fixes.


Mordante added a comment.

Fixes C++03 compatibility of the test.


Mordante added a comment.

CI fixes.



================
Comment at: libcxx/include/__thread/thread.h:131
 
-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>
----------------
@ldionne This fails with localization disabled. To old `operator<<` would also fail but was not "removed" when localization was disabled. I can put to old code in the `#else` branch, but I wonder whether that is useful.

Note that we normally not comment out `operator<<`. Maybe we can discuss whether we should do that. I expect it "works" in our tests since they are disabled and these operators are templated on the `_CharT` of the `basic_ostream` thus not instantiated automatically.


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


Repository:
  rG LLVM Github Monorepo

https://reviews.llvm.org/D153336

Files:
  libcxx/include/__thread/thread.h
  libcxx/test/libcxx/transitive_includes/cxx03.csv
  libcxx/test/libcxx/transitive_includes/cxx11.csv
  libcxx/test/libcxx/transitive_includes/cxx14.csv
  libcxx/test/libcxx/transitive_includes/cxx17.csv
  libcxx/test/libcxx/transitive_includes/cxx20.csv
  libcxx/test/libcxx/transitive_includes/cxx23.csv
  libcxx/test/libcxx/transitive_includes/cxx26.csv
  libcxx/test/std/thread/thread.threads/thread.thread.class/thread.thread.id/stream.pass.cpp

-------------- next part --------------
A non-text attachment was scrubbed...
Name: D153336.533144.patch
Type: text/x-patch
Size: 9657 bytes
Desc: not available
URL: <http://lists.llvm.org/pipermail/libcxx-commits/attachments/20230621/ea101ef4/attachment-0001.bin>