On Mon, Sep 14, 2026 at 10:46 AM Jonathan Wakely <[email protected]> wrote:
> On Mon, 14 Sept 2026 at 08:10, Tomasz Kamiński <[email protected]> > wrote: > > > > From: Jonathan Wakely <[email protected]> > > > > Clang cannot compile format_parse_context::check_dynamic_spec due to a > > longstanding bug[1] with out-of-line function templates called during > > constant evaluation. This patch refactors check_dynamic_spec and related > > functions, so that the definitions are inline in the class body. > > > > To make this work we move the definition of __format::_Arg_t earlier, > > and redefine _Scanner<C>::_Parse_context earlier by making it a nested > > class within basic_format_parse_context<C>, called _Scan_parse_context. > > The __check_dynamic_spec function is changed to take a span<_Arg_t> > > instead of being instantiated with a template parameter pack of arg > > types. The callers that have fixed arg types can pass a span that refers > > to an array of the fixed integer or string enumerators, reducing the > > template instantiations needed for those common cases. The generic form > > (which is rarely needed in user code) uses the __to_arg_t_enum to > > convert arbitrary types to _Arg_t enums, and then passes a span > > referring to that. > > > > As well as simplifying the code a little, this allows checking dynamic > > spec to work with Clang because it no longer needs to instantiate an > > out-of-line function template. > > > > [1]: https://github.com/llvm/llvm-project/issues/73232 > > > > libstdc++-v3/ChangeLog: > > > > PR libstdc++/127220 > > * include/std/format (__format::_Arg_t): Move definition > > earlier. > > (basic_format_parse_context::check_dynamic_spec): Construct > > array of _Arg_t from template parameter list. > > (basic_format_parse_context::check_dynamic_spec_integral): > > Construct array of _Arg_t based on supported types. > > (basic_format_parse_context::check_dynamic_spec_string): > > Likewise. > > (basic_format_parse_context::__check_dynamic_spec): Replace > > template parameter list with span<const _Arg_t> function > > parameter and simplify accordingly. Define in class body. > > (basic_format_parse_context::_Scan_parse_context): New struct > > the replaces _Scanner::_Parse_context. > > (__format::_Scanner::_M_pc): Change type to _Scan_parse_context. > > > > Reviewed-by: Tomasz Kamiński <[email protected]> > > (cherry picked from commit 3a26070979bcd3f8d2594a9c08b02addd9369cde) > > --- > > I have changed _Arg_t to enum class in r16-760-g67584acb5906eb, and > > this patch moves the defintion from 15 up, instead of effectivelly > > backporting that change. > > > > I have checked that using enum is supported since clang 12 from 2021, > > so it is safe to user here. > > And 'if consteval' since Clang 14. > if consteval was already used on 15 branch, I was introducing using enum, at least to this file. > > > > Tested on x86_64-linux. *format* tested in all standard modes. > > Ok for 15? > > OK thanks. > > > > > libstdc++-v3/include/std/format | 127 +++++++++++++++++--------------- > > 1 file changed, 68 insertions(+), 59 deletions(-) > > > > diff --git a/libstdc++-v3/include/std/format > b/libstdc++-v3/include/std/format > > index 3373eadea2d..589be4ec506 100644 > > --- a/libstdc++-v3/include/std/format > > +++ b/libstdc++-v3/include/std/format > > @@ -266,6 +266,25 @@ namespace __format > > > > template<typename _CharT> class _Scanner; > > > > + enum _Arg_t : unsigned char { > > + _Arg_none, _Arg_bool, _Arg_c, _Arg_i, _Arg_u, _Arg_ll, _Arg_ull, > > + _Arg_flt, _Arg_dbl, _Arg_ldbl, _Arg_str, _Arg_sv, _Arg_ptr, > _Arg_handle, > > + _Arg_i128, _Arg_u128, > > + _Arg_bf16, _Arg_f16, _Arg_f32, _Arg_f64, // These are unused. > > +#ifdef _GLIBCXX_LONG_DOUBLE_ALT128_COMPAT > > + _Arg_next_value_, > > + _Arg_f128 = _Arg_ldbl, > > + _Arg_ibm128 = _Arg_next_value_, > > +#else > > + _Arg_f128, > > +#endif > > + _Arg_max_ > > + }; > > + > > + template<typename _CharT, typename _Tp> > > + consteval _Arg_t > > + __to_arg_t_enum() noexcept; > > + > > } // namespace __format > > /// @endcond > > > > @@ -326,7 +345,7 @@ namespace __format > > __format::__invalid_arg_id_in_format_string(); > > } > > > > -#if __cpp_lib_format >= 202305L > > +#if __cpp_lib_format >= 202305L // >= C++26 > > template<typename... _Ts> > > constexpr void > > check_dynamic_spec(size_t __id) noexcept > > @@ -334,17 +353,22 @@ namespace __format > > static_assert(__valid_types_for_check_dynamic_spec<_Ts...>(), > > "template arguments for > check_dynamic_spec<Ts...>(id) " > > "must be unique and must be one of the allowed > types"); > > - if consteval { > > - __check_dynamic_spec<_Ts...>(__id); > > - } > > + if constexpr (sizeof...(_Ts)) > > + if consteval { > > + const __format::_Arg_t __t[] = { > > + __format::__to_arg_t_enum<_CharT, _Ts>()... > > + }; > > + __check_dynamic_spec(__id, __t); > > + } > > } > > > > constexpr void > > check_dynamic_spec_integral(size_t __id) noexcept > > { > > if consteval { > > - __check_dynamic_spec<int, unsigned, long long, > > - unsigned long long>(__id); > > + using enum __format::_Arg_t; > > + const __format::_Arg_t __t[] = { _Arg_i, _Arg_u, _Arg_ll, > _Arg_ull }; > > + __check_dynamic_spec(__id, __t); > > } > > } > > > > @@ -352,7 +376,9 @@ namespace __format > > check_dynamic_spec_string(size_t __id) noexcept > > { > > if consteval { > > - __check_dynamic_spec<const _CharT*, > basic_string_view<_CharT>>(__id); > > + using enum __format::_Arg_t; > > + const __format::_Arg_t __t[] = { _Arg_str, _Arg_sv }; > > + __check_dynamic_spec(__id, __t); > > } > > } > > > > @@ -392,14 +418,31 @@ namespace __format > > } > > } > > > > - template<typename... _Ts> > > - consteval void > > - __check_dynamic_spec(size_t __id) noexcept; > > + // Common implementation of check_dynamic_spec{,_string,_integral} > > + consteval void > > + __check_dynamic_spec(size_t __id, > > + span<const __format::_Arg_t> __types) noexcept > > + { > > + if (__id >= _M_num_args) > > + __format::__invalid_arg_id_in_format_string(); > > + > > + // This static_cast is safe because all parse contexts created > by the > > + // library are _Scan_parse_context (or a type derived from that). > > + // User code can only create basic_format_parse_context objects > with > > + // _M_num_args == 0 and those will be rejected by the condition > above. > > + if (auto* __args = > static_cast<_Scan_parse_context*>(this)->_M_types) > > + { > > + for (auto __t : __types) > > + if (__args[__id] == __t) > > + return; > > + > > + __invalid_dynamic_spec("arg(id) type does not match"); > > + } > > + // else this is a formatting scanner, do not do any type checks. > > + } > > > > // This must not be constexpr. > > static void __invalid_dynamic_spec(const char*); > > - > > - friend __format::_Scanner<_CharT>; > > #endif > > > > // This constructor should only be used by the implementation. > > @@ -416,6 +459,18 @@ namespace __format > > _Indexing _M_indexing = _Unknown; > > size_t _M_next_arg_id = 0; > > size_t _M_num_args = 0; > > + > > + // Derived parse context used by Scanner when checking format > strings. > > + struct _Scan_parse_context; > > + friend __format::_Scanner<_CharT>; > > + }; > > + > > + template<typename _CharT> > > + struct basic_format_parse_context<_CharT>::_Scan_parse_context > > + : basic_format_parse_context<_CharT> > > + { > > + using > basic_format_parse_context<_CharT>::basic_format_parse_context; > > + const __format::_Arg_t* _M_types = nullptr; > > }; > > > > /// @cond undocumented > > @@ -3564,21 +3619,6 @@ namespace __format > > } > > }; > > > > - enum _Arg_t : unsigned char { > > - _Arg_none, _Arg_bool, _Arg_c, _Arg_i, _Arg_u, _Arg_ll, _Arg_ull, > > - _Arg_flt, _Arg_dbl, _Arg_ldbl, _Arg_str, _Arg_sv, _Arg_ptr, > _Arg_handle, > > - _Arg_i128, _Arg_u128, > > - _Arg_bf16, _Arg_f16, _Arg_f32, _Arg_f64, // These are unused. > > -#ifdef _GLIBCXX_LONG_DOUBLE_ALT128_COMPAT > > - _Arg_next_value_, > > - _Arg_f128 = _Arg_ldbl, > > - _Arg_ibm128 = _Arg_next_value_, > > -#else > > - _Arg_f128, > > -#endif > > - _Arg_max_ > > - }; > > - > > template<typename _Context> > > struct _Arg_value > > { > > @@ -4444,11 +4484,7 @@ namespace __format > > { > > using iterator = typename > basic_format_parse_context<_CharT>::iterator; > > > > - struct _Parse_context : basic_format_parse_context<_CharT> > > - { > > - using > basic_format_parse_context<_CharT>::basic_format_parse_context; > > - const _Arg_t* _M_types = nullptr; > > - } _M_pc; > > + typename basic_format_parse_context<_CharT>::_Scan_parse_context > _M_pc; > > > > constexpr explicit > > _Scanner(basic_string_view<_CharT> __str, size_t __nargs = > (size_t)-1) > > @@ -4769,33 +4805,6 @@ namespace __format > > } // namespace __format > > /// @endcond > > > > -#if __cpp_lib_format >= 202305L // >= C++26 > > - /// @cond undocumented > > - // Common implementation of check_dynamic_spec{,_string,_integral} > > - template<typename _CharT> > > - template<typename... _Ts> > > - consteval void > > - basic_format_parse_context<_CharT>:: > > - __check_dynamic_spec(size_t __id) noexcept > > - { > > - if (__id >= _M_num_args) > > - __format::__invalid_arg_id_in_format_string(); > > - if constexpr (sizeof...(_Ts) != 0) > > - { > > - using _Parse_ctx = > __format::_Scanner<_CharT>::_Parse_context; > > - auto __arg = static_cast<_Parse_ctx*>(this)->_M_types[__id]; > > - __format::_Arg_t __types[] = { > > - __format::__to_arg_t_enum<_CharT, _Ts>()... > > - }; > > - for (auto __t : __types) > > - if (__arg == __t) > > - return; > > - } > > - __invalid_dynamic_spec("arg(id) type does not match"); > > - } > > - /// @endcond > > -#endif > > - > > template<typename _CharT, typename... _Args> > > template<typename _Tp> > > requires convertible_to<const _Tp&, basic_string_view<_CharT>> > > -- > > 2.55.0 > > > >
