On Wed, Mar 4, 2026 at 3:54 PM Jonathan Wakely <[email protected]> wrote:
> On Tue, 24 Feb 2026 at 10:47 +0100, Tomasz Kamiński wrote: > >This patch changes the type of _M_handle member of __format::_Arg_value > >from __format::_HandleBase union member > basic_format_arg<_Context>::handle. > >This allows handle to be stored (using placement new) inside _Arg_value at > >compile time, as type _M_handle member now matches stored object. > > > >In addition to above, to make handle usable at compile time, we adjust > >the _M_func signature to match the stored function, avoiding the need > >for reinterpret cast. > > > >To avoid cycling dependency, where basic_format_arg<_Context> requires > >instantiating _Arg_value<_Context> for its _M_val member, that in turn > >requires basic_format_arg<_Context>::handle, we introduce > __format::_Handle, > >and change basic_format_arg<_Context>::handle to alias for it. To make > >this unobservable we also define _Handle<_Context>::handle alias, to > >support uses of injected-class name, > > > >Finally, the _Handle(_Tp&) constructor is now constrained with to not > accept > >_Handle itself, as otherwise it would be used instead of copy-constructor > >when constructing from _Handle&. > > > >As _Arg_value is already templated on _Context, this change should not > lead > >to additional template instantiations. > > > >libstdc++-v3/ChangeLog: > > > > * include/std/format (__format::_Handle): Define, extracted > > with modification from basic_format_arg::handle. > > (_Arg_value::_Handle_base): Remove. > > (_Arg_value::_M_handle): Change type to _Handle<_Context>. > > (_Arg_value::_M_get, _Arg_value::_M_set): Check for handle > > type directly, and return result unmodified. > > (basic_format_arg::__formattable): Remove. > > (basic_format_arg::handle): Replace with alias to _Handle. > > > >Signed-off-by: Tomasz Kamiński <[email protected]> > >--- > >I do not see there is any drawback of doing the above, and this > >technically would be ABI break, but the actual binary representation > >does not change. So I think it would be coode to merge it to GCC-16. > > > >Testing on x86_64-linux. All *format* test already passed. > >OK for trunk when all test passes. > > > > libstdc++-v3/include/std/format | 122 +++++++++++++++----------------- > > 1 file changed, 57 insertions(+), 65 deletions(-) > > > >diff --git a/libstdc++-v3/include/std/format > b/libstdc++-v3/include/std/format > >index 2e4463c6596..66bf3c246e5 100644 > >--- a/libstdc++-v3/include/std/format > >+++ b/libstdc++-v3/include/std/format > >@@ -4109,15 +4109,60 @@ namespace __format > > using enum _Arg_t; > > > > template<typename _Context> > >- struct _Arg_value > >+ class _Handle > > { > > using _CharT = typename _Context::char_type; > >+ using _Func = void(*)(basic_format_parse_context<_CharT>&, > >+ _Context&, const void*); > > > >- struct _HandleBase > >- { > >- const void* _M_ptr; > >- void (*_M_func)(); > > We could have just fixed the type of this member, right? > > I stored it as void(*)() and used reinterpret_cast, but I think it > could always have been this type instead: > void(*)(basic_format_parse_context<_CharT>&, _Context&, const void*) > It would help with reintepret_cast, but we also have a limitation for placement new, that new(ptr) handle(....) works only if ptr points to object of type handle https://eel.is/c++draft/expr.const#9.19.2, so the union member needs to be exaclty handle and not it base class. > > But I think it does still makes sense to get rid of the _HandleBase > type and just deal with one type (parameterized on Context) instead of > a base and a derived type. > > >- }; > >+ // Format as const if possible, to reduce instantiations. > >+ template<typename _Tp> > >+ using __maybe_const_t > >+ = __conditional_t<__formattable_with<const _Tp, _Context>, > >+ const _Tp, _Tp>; > >+ > >+ template<typename _Tq> > >+ static void > >+ _S_format(basic_format_parse_context<_CharT>& __parse_ctx, > >+ _Context& __format_ctx, const void* __ptr) > >+ { > >+ using _Td = remove_const_t<_Tq>; > >+ typename _Context::template formatter_type<_Td> __f; > >+ __parse_ctx.advance_to(__f.parse(__parse_ctx)); > >+ _Tq& __val = *const_cast<_Tq*>(static_cast<const _Td*>(__ptr)); > >+ __format_ctx.advance_to(__f.format(__val, __format_ctx)); > >+ } > >+ > >+ template<typename _Tp> > >+ requires (!is_same_v<remove_cv_t<_Tp>, _Handle>) > >+ explicit > >+ _Handle(_Tp& __val) noexcept > >+ : _M_ptr(__builtin_addressof(__val)) > >+ , _M_func(&_S_format<__maybe_const_t<_Tp>>) > >+ { } > >+ > >+ friend class basic_format_arg<_Context>; > >+ > >+ public: > >+ using handle = _Handle; // emulate injected class name > > We could define _Arg_value<Context>::handle instead of a separate > _Handle<Context>, right? Then its injected class name would be > "handle" anyway. > > i.e. still remove _Arg_value<C>::_HandleBase, but move > basic_format_arg<C>::handle to be defined as a nested class in > _Arg_value<C> instead. Would that be a little simpler? > Yes, I think that will work. I will adjust accordingly. > > >+ > >+ _Handle(const _Handle&) = default; > >+ _Handle& operator=(const _Handle&) = default; > >+ > >+ [[__gnu__::__always_inline__]] > >+ void > >+ format(basic_format_parse_context<_CharT>& __pc, _Context& __fc) > const > >+ { _M_func(__pc, __fc, this->_M_ptr); } > >+ > >+ private: > >+ const void* _M_ptr; > >+ _Func _M_func; > >+ }; > >+ > >+ template<typename _Context> > >+ struct _Arg_value > >+ { > >+ using _CharT = typename _Context::char_type; > > > > union > > { > >@@ -4142,7 +4187,7 @@ namespace __format > > const _CharT* _M_str; > > basic_string_view<_CharT> _M_sv; > > const void* _M_ptr; > >- _HandleBase _M_handle; > >+ _Handle<_Context> _M_handle; > > #ifdef __SIZEOF_INT128__ > > __int128 _M_i128; > > unsigned __int128 _M_u128; > >@@ -4232,8 +4277,8 @@ namespace __format > > else if constexpr (is_same_v<_Tp, _Float64>) > > return __u._M_f64; > > #endif > >- else if constexpr (derived_from<_Tp, _HandleBase>) > >- return static_cast<_Tp&>(__u._M_handle); > >+ else if constexpr (is_same_v<_Tp, _Handle<_Context>>) > >+ return __u._M_handle; > > // Otherwise, ill-formed. > > } > > > >@@ -4254,7 +4299,7 @@ namespace __format > > void > > _M_set(_Tp __v) noexcept > > { > >- if constexpr (derived_from<_Tp, _HandleBase>) > >+ if constexpr (is_same_v<_Tp, _Handle<_Context>>) > > std::construct_at(&_M_handle, __v); > > else > > _S_get<_Tp>(*this) = __v; > >@@ -4279,58 +4324,8 @@ namespace __format > > { > > using _CharT = typename _Context::char_type; > > > >- template<typename _Tp> > >- static constexpr bool __formattable > >- = __format::__formattable_with<_Tp, _Context>; > >- > > public: > >- class handle : public __format::_Arg_value<_Context>::_HandleBase > >- { > >- using _Base = typename __format::_Arg_value<_Context>::_HandleBase; > >- > >- // Format as const if possible, to reduce instantiations. > >- template<typename _Tp> > >- using __maybe_const_t > >- = __conditional_t<__formattable<const _Tp>, const _Tp, _Tp>; > >- > >- template<typename _Tq> > >- static void > >- _S_format(basic_format_parse_context<_CharT>& __parse_ctx, > >- _Context& __format_ctx, const void* __ptr) > >- { > >- using _Td = remove_const_t<_Tq>; > >- typename _Context::template formatter_type<_Td> __f; > >- __parse_ctx.advance_to(__f.parse(__parse_ctx)); > >- _Tq& __val = *const_cast<_Tq*>(static_cast<const _Td*>(__ptr)); > >- __format_ctx.advance_to(__f.format(__val, __format_ctx)); > >- } > >- > >- template<typename _Tp> > >- explicit > >- handle(_Tp& __val) noexcept > >- { > >- this->_M_ptr = __builtin_addressof(__val); > >- auto __func = _S_format<__maybe_const_t<_Tp>>; > >- this->_M_func = reinterpret_cast<void(*)()>(__func); > >- } > >- > >- friend class basic_format_arg<_Context>; > >- > >- public: > >- handle(const handle&) = default; > >- handle& operator=(const handle&) = default; > >- > >- [[__gnu__::__always_inline__]] > >- void > >- format(basic_format_parse_context<_CharT>& __pc, _Context& __fc) > const > >- { > >- using _Func = void(*)(basic_format_parse_context<_CharT>&, > >- _Context&, const void*); > >- auto __f = reinterpret_cast<_Func>(this->_M_func); > >- __f(__pc, __fc, this->_M_ptr); > >- } > >- }; > >- > >+ using handle = __format::_Handle<_Context>; > > Blank line between this using-declaration and the constructor please: > > > [[__gnu__::__always_inline__]] > > basic_format_arg() noexcept : _M_type(__format::_Arg_none) { } > > > >@@ -4623,10 +4618,7 @@ namespace __format > > case _Arg_ptr: > > return std::forward<_Visitor>(__vis)(_M_val._M_ptr); > > case _Arg_handle: > >- { > >- auto& __h = static_cast<handle&>(_M_val._M_handle); > >- return std::forward<_Visitor>(__vis)(__h); > >- } > >+ return std::forward<_Visitor>(__vis)(_M_val._M_handle); > > #ifdef __SIZEOF_INT128__ > > case _Arg_i128: > > return std::forward<_Visitor>(__vis)(_M_val._M_i128); > >-- > >2.53.0 > > > > > >
