* [PATCH] libstdc++: Implement std::formatter<std::thread::id> without <sstream> [PR115099]
@ 2024-05-17 13:42 Jonathan Wakely
2024-05-22 9:15 ` Jonathan Wakely
0 siblings, 1 reply; 2+ messages in thread
From: Jonathan Wakely @ 2024-05-17 13:42 UTC (permalink / raw)
To: libstdc++, gcc-patches
Does anybody see any issue with the drive-by fixes to constraint
std::formatter<thread::id> to only work for pointers and integers (since
we don't know how to format pthread_t if it's an arbitrary struct, for
example), and to cast pointers to const void* for output (because if
pthread_t is char* then writing it to a stream would be bad! and we
don't want to allow users to overload operator<< for pointers to opaque
structs, for example). I don't think this will change anything in
practice for common targets, where pthread_t is either an integer or
void*.
Tested x86_64-linux.
-- >8 --
The std::thread::id formatter uses std::basic_ostringstream without
including <sstream>, which went unnoticed because the test for it uses
a stringstream to check the output is correct.
The fix implemented here is to stop using basic_ostringstream for
formatting thread::id and just use std::format instead.
As a drive-by fix, the formatter specialization is constrained to
require that the thread::id::native_handle_type can be formatted, to
avoid making the formatter ill-formed if the pthread_t type is not a
pointer or integer. Since non-void pointers can't be formatted, ensure
that we convert pointers to const void* for formatting. Make a similar
change to the existing operator<< overload so that in the unlikely case
that pthread_t is a typedef for char* we don't treat it as a
null-terminated string when inserting into a stream.
libstdc++-v3/ChangeLog:
PR libstdc++/115099
* include/bits/std_thread.h: Declare formatter as friend of
thread::id.
* include/std/thread (operator<<): Convert non-void pointers to
void pointers for output.
(formatter): Add constraint that thread::native_handle_type is a
pointer or integer.
(formatter::format): Reimplement without basic_ostringstream.
* testsuite/30_threads/thread/id/output.cc: Check output
compiles before <sstream> has been included.
---
libstdc++-v3/include/bits/std_thread.h | 11 ++++-
libstdc++-v3/include/std/thread | 43 ++++++++++++++-----
.../testsuite/30_threads/thread/id/output.cc | 21 ++++++++-
3 files changed, 63 insertions(+), 12 deletions(-)
diff --git a/libstdc++-v3/include/bits/std_thread.h b/libstdc++-v3/include/bits/std_thread.h
index 2d7df12650d..5817bfb29dd 100644
--- a/libstdc++-v3/include/bits/std_thread.h
+++ b/libstdc++-v3/include/bits/std_thread.h
@@ -53,6 +53,10 @@ namespace std _GLIBCXX_VISIBILITY(default)
{
_GLIBCXX_BEGIN_NAMESPACE_VERSION
+#if __glibcxx_formatters
+ template<typename, typename> class formatter;
+#endif
+
/** @addtogroup threads
* @{
*/
@@ -117,13 +121,18 @@ _GLIBCXX_BEGIN_NAMESPACE_VERSION
template<class _CharT, class _Traits>
friend basic_ostream<_CharT, _Traits>&
operator<<(basic_ostream<_CharT, _Traits>& __out, id __id);
+
+#if __glibcxx_formatters
+ friend formatter<id, char>;
+ friend formatter<id, wchar_t>;
+#endif
};
private:
id _M_id;
// _GLIBCXX_RESOLVE_LIB_DEFECTS
- // 2097. packaged_task constructors should be constrained
+ // 2097. packaged_task constructors should be constrained
// 3039. Unnecessary decay in thread and packaged_task
template<typename _Tp>
using __not_same = __not_<is_same<__remove_cvref_t<_Tp>, thread>>;
diff --git a/libstdc++-v3/include/std/thread b/libstdc++-v3/include/std/thread
index 09ca3116e7f..e994d683bff 100644
--- a/libstdc++-v3/include/std/thread
+++ b/libstdc++-v3/include/std/thread
@@ -42,10 +42,6 @@
# include <stop_token> // std::stop_source, std::stop_token, std::nostopstate
#endif
-#if __cplusplus > 202002L
-# include <format>
-#endif
-
#include <bits/std_thread.h> // std::thread, get_id, yield
#include <bits/this_thread_sleep.h> // std::this_thread::sleep_for, sleep_until
@@ -53,6 +49,10 @@
#define __glibcxx_want_formatters
#include <bits/version.h>
+#if __cpp_lib_formatters
+# include <format>
+#endif
+
namespace std _GLIBCXX_VISIBILITY(default)
{
_GLIBCXX_BEGIN_NAMESPACE_VERSION
@@ -104,10 +104,16 @@ _GLIBCXX_BEGIN_NAMESPACE_VERSION
inline basic_ostream<_CharT, _Traits>&
operator<<(basic_ostream<_CharT, _Traits>& __out, thread::id __id)
{
+ // Convert non-void pointers to const void* for formatted output.
+ using __output_type
+ = __conditional_t<is_pointer<thread::native_handle_type>::value,
+ const void*,
+ thread::native_handle_type>;
+
if (__id == thread::id())
return __out << "thread::id of a non-executing thread";
else
- return __out << __id._M_thread;
+ return __out << static_cast<__output_type>(__id._M_thread);
}
/// @}
@@ -287,8 +293,9 @@ _GLIBCXX_BEGIN_NAMESPACE_VERSION
#endif // __cpp_lib_jthread
#ifdef __cpp_lib_formatters // C++ >= 23
-
template<typename _CharT>
+ requires is_pointer_v<thread::native_handle_type>
+ || is_integral_v<thread::native_handle_type>
class formatter<thread::id, _CharT>
{
public:
@@ -331,10 +338,26 @@ _GLIBCXX_BEGIN_NAMESPACE_VERSION
typename basic_format_context<_Out, _CharT>::iterator
format(thread::id __id, basic_format_context<_Out, _CharT>& __fc) const
{
- std::basic_ostringstream<_CharT> __os;
- __os << __id;
- auto __str = __os.view();
- return __format::__write_padded_as_spec(__str, __str.size(),
+ basic_string_view<_CharT> __sv;
+ if constexpr (is_same_v<_CharT, char>)
+ __sv = "{}thread::id of a non-executing thread";
+ else
+ __sv = L"{}thread::id of a non-executing thread";
+ basic_string<_CharT> __str;
+ if (__id == thread::id())
+ __sv.remove_prefix(2);
+ else
+ {
+ using _FmtStr = __format::_Runtime_format_string<_CharT>;
+ // Convert non-void pointers to const void* for formatted output.
+ using __output_type
+ = __conditional_t<is_pointer_v<thread::native_handle_type>,
+ const void*,
+ thread::native_handle_type>;
+ auto __o = static_cast<__output_type>(__id._M_thread);
+ __sv = __str = std::format(_FmtStr(__sv.substr(0, 2)), __o);
+ }
+ return __format::__write_padded_as_spec(__sv, __sv.size(),
__fc, _M_spec,
__format::_Align_right);
}
diff --git a/libstdc++-v3/testsuite/30_threads/thread/id/output.cc b/libstdc++-v3/testsuite/30_threads/thread/id/output.cc
index 3c167202b02..94a6ff0e2a1 100644
--- a/libstdc++-v3/testsuite/30_threads/thread/id/output.cc
+++ b/libstdc++-v3/testsuite/30_threads/thread/id/output.cc
@@ -3,8 +3,27 @@
// { dg-require-gthreads "" }
#include <thread>
-#include <sstream>
+
+#include <ostream>
#include <format>
+
+void
+test_no_includes(std::ostream& out)
+{
+ std::thread::id i{};
+ // Check stream insertion works without including <sstream>:
+ out << i;
+#if __cpp_lib_formatters >= 202302
+ // PR libstdc++/115099 - compilation error: format thread::id
+ // Verify we can use std::thread::id with std::format without <sstream>:
+ (void) std::format("{}", i);
+#ifdef _GLIBCXX_USE_WCHAR_T
+ (void) std::format(L"{}", i);
+#endif
+#endif
+}
+
+#include <sstream>
#include <testsuite_hooks.h>
void
--
2.45.0
^ permalink raw reply [flat|nested] 2+ messages in thread
* Re: [PATCH] libstdc++: Implement std::formatter<std::thread::id> without <sstream> [PR115099]
2024-05-17 13:42 [PATCH] libstdc++: Implement std::formatter<std::thread::id> without <sstream> [PR115099] Jonathan Wakely
@ 2024-05-22 9:15 ` Jonathan Wakely
0 siblings, 0 replies; 2+ messages in thread
From: Jonathan Wakely @ 2024-05-22 9:15 UTC (permalink / raw)
To: libstdc++, gcc-patches
Pushed to trunk. Backport to gcc-14 to follow.
On Fri, 17 May 2024 at 14:45, Jonathan Wakely <jwakely@redhat.com> wrote:
>
> Does anybody see any issue with the drive-by fixes to constraint
> std::formatter<thread::id> to only work for pointers and integers (since
> we don't know how to format pthread_t if it's an arbitrary struct, for
> example), and to cast pointers to const void* for output (because if
> pthread_t is char* then writing it to a stream would be bad! and we
> don't want to allow users to overload operator<< for pointers to opaque
> structs, for example). I don't think this will change anything in
> practice for common targets, where pthread_t is either an integer or
> void*.
>
> Tested x86_64-linux.
>
> -- >8 --
>
> The std::thread::id formatter uses std::basic_ostringstream without
> including <sstream>, which went unnoticed because the test for it uses
> a stringstream to check the output is correct.
>
> The fix implemented here is to stop using basic_ostringstream for
> formatting thread::id and just use std::format instead.
>
> As a drive-by fix, the formatter specialization is constrained to
> require that the thread::id::native_handle_type can be formatted, to
> avoid making the formatter ill-formed if the pthread_t type is not a
> pointer or integer. Since non-void pointers can't be formatted, ensure
> that we convert pointers to const void* for formatting. Make a similar
> change to the existing operator<< overload so that in the unlikely case
> that pthread_t is a typedef for char* we don't treat it as a
> null-terminated string when inserting into a stream.
>
> libstdc++-v3/ChangeLog:
>
> PR libstdc++/115099
> * include/bits/std_thread.h: Declare formatter as friend of
> thread::id.
> * include/std/thread (operator<<): Convert non-void pointers to
> void pointers for output.
> (formatter): Add constraint that thread::native_handle_type is a
> pointer or integer.
> (formatter::format): Reimplement without basic_ostringstream.
> * testsuite/30_threads/thread/id/output.cc: Check output
> compiles before <sstream> has been included.
> ---
> libstdc++-v3/include/bits/std_thread.h | 11 ++++-
> libstdc++-v3/include/std/thread | 43 ++++++++++++++-----
> .../testsuite/30_threads/thread/id/output.cc | 21 ++++++++-
> 3 files changed, 63 insertions(+), 12 deletions(-)
>
> diff --git a/libstdc++-v3/include/bits/std_thread.h b/libstdc++-v3/include/bits/std_thread.h
> index 2d7df12650d..5817bfb29dd 100644
> --- a/libstdc++-v3/include/bits/std_thread.h
> +++ b/libstdc++-v3/include/bits/std_thread.h
> @@ -53,6 +53,10 @@ namespace std _GLIBCXX_VISIBILITY(default)
> {
> _GLIBCXX_BEGIN_NAMESPACE_VERSION
>
> +#if __glibcxx_formatters
> + template<typename, typename> class formatter;
> +#endif
> +
> /** @addtogroup threads
> * @{
> */
> @@ -117,13 +121,18 @@ _GLIBCXX_BEGIN_NAMESPACE_VERSION
> template<class _CharT, class _Traits>
> friend basic_ostream<_CharT, _Traits>&
> operator<<(basic_ostream<_CharT, _Traits>& __out, id __id);
> +
> +#if __glibcxx_formatters
> + friend formatter<id, char>;
> + friend formatter<id, wchar_t>;
> +#endif
> };
>
> private:
> id _M_id;
>
> // _GLIBCXX_RESOLVE_LIB_DEFECTS
> - // 2097. packaged_task constructors should be constrained
> + // 2097. packaged_task constructors should be constrained
> // 3039. Unnecessary decay in thread and packaged_task
> template<typename _Tp>
> using __not_same = __not_<is_same<__remove_cvref_t<_Tp>, thread>>;
> diff --git a/libstdc++-v3/include/std/thread b/libstdc++-v3/include/std/thread
> index 09ca3116e7f..e994d683bff 100644
> --- a/libstdc++-v3/include/std/thread
> +++ b/libstdc++-v3/include/std/thread
> @@ -42,10 +42,6 @@
> # include <stop_token> // std::stop_source, std::stop_token, std::nostopstate
> #endif
>
> -#if __cplusplus > 202002L
> -# include <format>
> -#endif
> -
> #include <bits/std_thread.h> // std::thread, get_id, yield
> #include <bits/this_thread_sleep.h> // std::this_thread::sleep_for, sleep_until
>
> @@ -53,6 +49,10 @@
> #define __glibcxx_want_formatters
> #include <bits/version.h>
>
> +#if __cpp_lib_formatters
> +# include <format>
> +#endif
> +
> namespace std _GLIBCXX_VISIBILITY(default)
> {
> _GLIBCXX_BEGIN_NAMESPACE_VERSION
> @@ -104,10 +104,16 @@ _GLIBCXX_BEGIN_NAMESPACE_VERSION
> inline basic_ostream<_CharT, _Traits>&
> operator<<(basic_ostream<_CharT, _Traits>& __out, thread::id __id)
> {
> + // Convert non-void pointers to const void* for formatted output.
> + using __output_type
> + = __conditional_t<is_pointer<thread::native_handle_type>::value,
> + const void*,
> + thread::native_handle_type>;
> +
> if (__id == thread::id())
> return __out << "thread::id of a non-executing thread";
> else
> - return __out << __id._M_thread;
> + return __out << static_cast<__output_type>(__id._M_thread);
> }
> /// @}
>
> @@ -287,8 +293,9 @@ _GLIBCXX_BEGIN_NAMESPACE_VERSION
> #endif // __cpp_lib_jthread
>
> #ifdef __cpp_lib_formatters // C++ >= 23
> -
> template<typename _CharT>
> + requires is_pointer_v<thread::native_handle_type>
> + || is_integral_v<thread::native_handle_type>
> class formatter<thread::id, _CharT>
> {
> public:
> @@ -331,10 +338,26 @@ _GLIBCXX_BEGIN_NAMESPACE_VERSION
> typename basic_format_context<_Out, _CharT>::iterator
> format(thread::id __id, basic_format_context<_Out, _CharT>& __fc) const
> {
> - std::basic_ostringstream<_CharT> __os;
> - __os << __id;
> - auto __str = __os.view();
> - return __format::__write_padded_as_spec(__str, __str.size(),
> + basic_string_view<_CharT> __sv;
> + if constexpr (is_same_v<_CharT, char>)
> + __sv = "{}thread::id of a non-executing thread";
> + else
> + __sv = L"{}thread::id of a non-executing thread";
> + basic_string<_CharT> __str;
> + if (__id == thread::id())
> + __sv.remove_prefix(2);
> + else
> + {
> + using _FmtStr = __format::_Runtime_format_string<_CharT>;
> + // Convert non-void pointers to const void* for formatted output.
> + using __output_type
> + = __conditional_t<is_pointer_v<thread::native_handle_type>,
> + const void*,
> + thread::native_handle_type>;
> + auto __o = static_cast<__output_type>(__id._M_thread);
> + __sv = __str = std::format(_FmtStr(__sv.substr(0, 2)), __o);
> + }
> + return __format::__write_padded_as_spec(__sv, __sv.size(),
> __fc, _M_spec,
> __format::_Align_right);
> }
> diff --git a/libstdc++-v3/testsuite/30_threads/thread/id/output.cc b/libstdc++-v3/testsuite/30_threads/thread/id/output.cc
> index 3c167202b02..94a6ff0e2a1 100644
> --- a/libstdc++-v3/testsuite/30_threads/thread/id/output.cc
> +++ b/libstdc++-v3/testsuite/30_threads/thread/id/output.cc
> @@ -3,8 +3,27 @@
> // { dg-require-gthreads "" }
>
> #include <thread>
> -#include <sstream>
> +
> +#include <ostream>
> #include <format>
> +
> +void
> +test_no_includes(std::ostream& out)
> +{
> + std::thread::id i{};
> + // Check stream insertion works without including <sstream>:
> + out << i;
> +#if __cpp_lib_formatters >= 202302
> + // PR libstdc++/115099 - compilation error: format thread::id
> + // Verify we can use std::thread::id with std::format without <sstream>:
> + (void) std::format("{}", i);
> +#ifdef _GLIBCXX_USE_WCHAR_T
> + (void) std::format(L"{}", i);
> +#endif
> +#endif
> +}
> +
> +#include <sstream>
> #include <testsuite_hooks.h>
>
> void
> --
> 2.45.0
>
^ permalink raw reply [flat|nested] 2+ messages in thread
end of thread, other threads:[~2024-05-22 9:15 UTC | newest]
Thread overview: 2+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2024-05-17 13:42 [PATCH] libstdc++: Implement std::formatter<std::thread::id> without <sstream> [PR115099] Jonathan Wakely
2024-05-22 9:15 ` Jonathan Wakely
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for read-only IMAP folder(s) and NNTP newsgroup(s).