libstdc++: Optimize basic_format_parse_context::__check_dynamic_spec_types
Checks
Commit Message
This change makes the consteval function slightly faster to compile.
Instead of keeping the counts in an array and looping over that array,
we can just keep a sum of how many valid types are present, and check
that it equals the total number of types in the pack. We can also avoid
doing the check entirely for the common cases where the call comes from
check_dynamic_spec_integer or check_dynamic_spec_string, because those
always use valid lists of types.
The diagnostic is slightly worse now, because there's only a single
"invalid template argument types" string that appears in the output,
where previously we had either "non-unique template argument type" or
"disallowed template argument type" depending on the failure mode.
Given that most users will never use this function directly, and
probably won't use invalid types anyway, the inferior diagnostic seems
acceptable.
libstdc++-v3/ChangeLog:
* include/std/format (basic_format_parse_context::__check_types):
New variable template and partial specializations.
(basic_format_parse_context::__check_dynamic_spec_types):
Simplify for faster compilation.
(basic_format_parse_context::check_dynamic_spec): Only use
__check_dynamic_spec_types when __check_types is true. Use it as
the condition for a constexpr if statement instead of as the
initializer for a constexpr variable.
(basic_format_parse_context::check_dynamic_spec_string): Use
_CharT instead of char_type consistently.
---
Tested x86_64-linux.
libstdc++-v3/include/std/format | 81 +++++++++++++++++++--------------
1 file changed, 47 insertions(+), 34 deletions(-)
Comments
On Tue, Mar 11, 2025 at 11:46 PM Jonathan Wakely <jwakely@redhat.com> wrote:
> This change makes the consteval function slightly faster to compile.
> Instead of keeping the counts in an array and looping over that array,
> we can just keep a sum of how many valid types are present, and check
> that it equals the total number of types in the pack. We can also avoid
> doing the check entirely for the common cases where the call comes from
> check_dynamic_spec_integer or check_dynamic_spec_string, because those
> always use valid lists of types.
>
> The diagnostic is slightly worse now, because there's only a single
> "invalid template argument types" string that appears in the output,
> where previously we had either "non-unique template argument type" or
> "disallowed template argument type" depending on the failure mode.
>
> Given that most users will never use this function directly, and
> probably won't use invalid types anyway, the inferior diagnostic seems
> acceptable.
>
> libstdc++-v3/ChangeLog:
>
> * include/std/format (basic_format_parse_context::__check_types):
> New variable template and partial specializations.
> (basic_format_parse_context::__check_dynamic_spec_types):
> Simplify for faster compilation.
> (basic_format_parse_context::check_dynamic_spec): Only use
> __check_dynamic_spec_types when __check_types is true. Use it as
> the condition for a constexpr if statement instead of as the
> initializer for a constexpr variable.
> (basic_format_parse_context::check_dynamic_spec_string): Use
> _CharT instead of char_type consistently.
> ---
>
> Tested x86_64-linux.
>
> libstdc++-v3/include/std/format | 81 +++++++++++++++++++--------------
> 1 file changed, 47 insertions(+), 34 deletions(-)
>
> diff --git a/libstdc++-v3/include/std/format
> b/libstdc++-v3/include/std/format
> index 0d6cc7f6bef..6269cbb80e6 100644
> --- a/libstdc++-v3/include/std/format
> +++ b/libstdc++-v3/include/std/format
> @@ -305,41 +305,51 @@ namespace __format
> constexpr void
> check_dynamic_spec_string(size_t __id) noexcept
> {
> - check_dynamic_spec<const char_type*,
> basic_string_view<_CharT>>(__id);
> + check_dynamic_spec<const _CharT*, basic_string_view<_CharT>>(__id);
> }
>
> private:
> - // Check the Mandates: condition for check_dynamic_spec<Ts...>(n)
> + template<typename _Tp, typename... _Ts>
> + static constexpr bool __once = (is_same_v<_Tp, _Ts> + ...) == 1;
> +
> + // True if we need to call __check_dynamic_spec_types for the pack
> Ts
> + template<typename... _Ts>
> + static constexpr bool __check_types = sizeof...(_Ts) > 0;
> + // The pack used by check_dynamic_spec_integral is valid, don't
> check it.
> + // FIXME: simplify these when PR c++/85282 is supported.
> + template<typename _Tp>
> + static constexpr bool
> + __check_types<_Tp, unsigned, long long, unsigned long long>
> + = ! is_same_v<_Tp, int>;
>
These seem to produce wrong answer (false) if the user called
check_dynamic_spec<basic_string_view<_CharT>, unsigned, long long, unsigned
long long>,
which is a valid set of types.
> + // The pack used by check_dynamic_spec_string is valid, don't check
> it.
> + template<typename _Tp>
> + static constexpr bool
> + __check_types<_Tp, basic_string_view<_CharT>>
> + = ! is_same_v<const _CharT*, _Tp>;
>
As above check_dynamic_spec<int, basic_string_view<_CharT>>.
I think much batter solution would be to have __check_dynamic_spec<Ts...>
private function,
that would not perform the check at all and will be called without checking
from check_dynamic_spec_integrral, e.t.c.
And in check_dynamic_spec, we would do __check_dynamic_spec_types<_Ts...>()
and sizeof..(Ts) > 0, before
calling __check_dynamic_spec.
+
> + // Check the Mandates: conditions for check_dynamic_spec<Ts...>(n)
> template<typename... _Ts>
> static consteval bool
> __check_dynamic_spec_types()
> {
> - if constexpr (sizeof...(_Ts))
> - {
> - int __counts[] = {
> - (is_same_v<bool, _Ts> + ...),
> - (is_same_v<_CharT, _Ts> + ...),
> - (is_same_v<int, _Ts> + ...),
> - (is_same_v<unsigned, _Ts> + ...),
> - (is_same_v<long long, _Ts> + ...),
> - (is_same_v<unsigned long long, _Ts> + ...),
> - (is_same_v<float, _Ts> + ...),
> - (is_same_v<double, _Ts> + ...),
> - (is_same_v<long double, _Ts> + ...),
> - (is_same_v<const _CharT*, _Ts> + ...),
> - (is_same_v<basic_string_view<_CharT>, _Ts> + ...),
> - (is_same_v<const void*, _Ts> + ...)
> - };
> - int __sum = 0;
> - for (int __c : __counts)
> - {
> - __sum += __c;
> - if (__c > 1)
> - __invalid_dynamic_spec("non-unique template argument
> type");
> - }
> - if (__sum != sizeof...(_Ts))
> - __invalid_dynamic_spec("disallowed template argument
> type");
> - }
> + // The types in Ts... are unique. Each type in Ts... is one of
> + // bool, char_type, int, unsigned int, long long int,
> + // unsigned long long int, float, double, long double,
> + // const char_type*, basic_string_view<char_type>, or const
> void*.
> + unsigned __sum = __once<bool, _Ts...>
> + + __once<char_type, _Ts...>
> + + __once<int, _Ts...>
> + + __once<unsigned int, _Ts...>
> + + __once<long long int, _Ts...>
> + + __once<unsigned long long int, _Ts...>
> + + __once<float, _Ts...>
> + + __once<double, _Ts...>
> + + __once<long double, _Ts...>
> + + __once<const char_type*, _Ts...>
> + + __once<basic_string_view<char_type>, _Ts...>
> + + __once<const void*, _Ts...>;
> + if (__sum != sizeof...(_Ts))
> + __invalid_dynamic_spec("invalid template argument types");
> return true;
> }
>
> @@ -4341,12 +4351,15 @@ namespace __format
> // _GLIBCXX_RESOLVE_LIB_DEFECTS
> // 4142. check_dynamic_spec should require at least one type
> static_assert(sizeof...(_Ts) >= 1);
> - // This call enforces the Mandates: condition that _Ts contains
> valid
> - // types and each type appears at most once. It could be a
> static_assert
> - // but this way failures give better diagnostics, due to calling the
> - // non-constexpr __invalid_dynamic_spec function.
> - [[maybe_unused]]
> - constexpr bool __ok = __check_dynamic_spec_types<_Ts...>();
> +
> + if constexpr (__check_types<_Ts...>)
> + if constexpr (__check_dynamic_spec_types<_Ts...>())
> + {
> + // The call above enforces the Mandates: condition that _Ts
> + // contains valid types and each type appears at most once.
> + // Checking it in `if constexpr` means that failures give
> better
> + // diagnostics than if we used it as a static_assert condition.
> + }
>
> if consteval {
> if (__id >= _M_num_args)
> --
> 2.48.1
>
>
On Wed, 12 Mar 2025 at 07:59, Tomasz Kaminski <tkaminsk@redhat.com> wrote:
>
>
>
> On Tue, Mar 11, 2025 at 11:46 PM Jonathan Wakely <jwakely@redhat.com> wrote:
>>
>> This change makes the consteval function slightly faster to compile.
>> Instead of keeping the counts in an array and looping over that array,
>> we can just keep a sum of how many valid types are present, and check
>> that it equals the total number of types in the pack. We can also avoid
>> doing the check entirely for the common cases where the call comes from
>> check_dynamic_spec_integer or check_dynamic_spec_string, because those
>> always use valid lists of types.
>>
>> The diagnostic is slightly worse now, because there's only a single
>> "invalid template argument types" string that appears in the output,
>> where previously we had either "non-unique template argument type" or
>> "disallowed template argument type" depending on the failure mode.
>>
>> Given that most users will never use this function directly, and
>> probably won't use invalid types anyway, the inferior diagnostic seems
>> acceptable.
>>
>> libstdc++-v3/ChangeLog:
>>
>> * include/std/format (basic_format_parse_context::__check_types):
>> New variable template and partial specializations.
>> (basic_format_parse_context::__check_dynamic_spec_types):
>> Simplify for faster compilation.
>> (basic_format_parse_context::check_dynamic_spec): Only use
>> __check_dynamic_spec_types when __check_types is true. Use it as
>> the condition for a constexpr if statement instead of as the
>> initializer for a constexpr variable.
>> (basic_format_parse_context::check_dynamic_spec_string): Use
>> _CharT instead of char_type consistently.
>> ---
>>
>> Tested x86_64-linux.
>>
>> libstdc++-v3/include/std/format | 81 +++++++++++++++++++--------------
>> 1 file changed, 47 insertions(+), 34 deletions(-)
>>
>> diff --git a/libstdc++-v3/include/std/format b/libstdc++-v3/include/std/format
>> index 0d6cc7f6bef..6269cbb80e6 100644
>> --- a/libstdc++-v3/include/std/format
>> +++ b/libstdc++-v3/include/std/format
>> @@ -305,41 +305,51 @@ namespace __format
>> constexpr void
>> check_dynamic_spec_string(size_t __id) noexcept
>> {
>> - check_dynamic_spec<const char_type*, basic_string_view<_CharT>>(__id);
>> + check_dynamic_spec<const _CharT*, basic_string_view<_CharT>>(__id);
>> }
>>
>> private:
>> - // Check the Mandates: condition for check_dynamic_spec<Ts...>(n)
>> + template<typename _Tp, typename... _Ts>
>> + static constexpr bool __once = (is_same_v<_Tp, _Ts> + ...) == 1;
>> +
>> + // True if we need to call __check_dynamic_spec_types for the pack Ts
>> + template<typename... _Ts>
>> + static constexpr bool __check_types = sizeof...(_Ts) > 0;
>> + // The pack used by check_dynamic_spec_integral is valid, don't check it.
>> + // FIXME: simplify these when PR c++/85282 is supported.
>> + template<typename _Tp>
>> + static constexpr bool
>> + __check_types<_Tp, unsigned, long long, unsigned long long>
>> + = ! is_same_v<_Tp, int>;
>
> These seem to produce wrong answer (false) if the user called check_dynamic_spec<basic_string_view<_CharT>, unsigned, long long, unsigned long long>,
> which is a valid set of types.
The variable template __check_types means "do we need to check the
types", so false is the correct answer here. We do not need to check
the types for this list of types, because we know it's a valid set of
types.
The variable template is used to decide whether to call
__check_dynamic_spec_types or to skip calling it.
>>
>> + // The pack used by check_dynamic_spec_string is valid, don't check it.
>> + template<typename _Tp>
>> + static constexpr bool
>> + __check_types<_Tp, basic_string_view<_CharT>>
>> + = ! is_same_v<const _CharT*, _Tp>;
>
> As above check_dynamic_spec<int, basic_string_view<_CharT>>.
> I think much batter solution would be to have __check_dynamic_spec<Ts...> private function,
> that would not perform the check at all and will be called without checking from check_dynamic_spec_integrral, e.t.c.
> And in check_dynamic_spec, we would do __check_dynamic_spec_types<_Ts...>() and sizeof..(Ts) > 0, before
> calling __check_dynamic_spec.
Yes I considered that, and that would also work.
>> +
>> + // Check the Mandates: conditions for check_dynamic_spec<Ts...>(n)
>> template<typename... _Ts>
>> static consteval bool
>> __check_dynamic_spec_types()
>> {
>> - if constexpr (sizeof...(_Ts))
>> - {
>> - int __counts[] = {
>> - (is_same_v<bool, _Ts> + ...),
>> - (is_same_v<_CharT, _Ts> + ...),
>> - (is_same_v<int, _Ts> + ...),
>> - (is_same_v<unsigned, _Ts> + ...),
>> - (is_same_v<long long, _Ts> + ...),
>> - (is_same_v<unsigned long long, _Ts> + ...),
>> - (is_same_v<float, _Ts> + ...),
>> - (is_same_v<double, _Ts> + ...),
>> - (is_same_v<long double, _Ts> + ...),
>> - (is_same_v<const _CharT*, _Ts> + ...),
>> - (is_same_v<basic_string_view<_CharT>, _Ts> + ...),
>> - (is_same_v<const void*, _Ts> + ...)
>> - };
>> - int __sum = 0;
>> - for (int __c : __counts)
>> - {
>> - __sum += __c;
>> - if (__c > 1)
>> - __invalid_dynamic_spec("non-unique template argument type");
>> - }
>> - if (__sum != sizeof...(_Ts))
>> - __invalid_dynamic_spec("disallowed template argument type");
>> - }
>> + // The types in Ts... are unique. Each type in Ts... is one of
>> + // bool, char_type, int, unsigned int, long long int,
>> + // unsigned long long int, float, double, long double,
>> + // const char_type*, basic_string_view<char_type>, or const void*.
>> + unsigned __sum = __once<bool, _Ts...>
>> + + __once<char_type, _Ts...>
>> + + __once<int, _Ts...>
>> + + __once<unsigned int, _Ts...>
>> + + __once<long long int, _Ts...>
>> + + __once<unsigned long long int, _Ts...>
>> + + __once<float, _Ts...>
>> + + __once<double, _Ts...>
>> + + __once<long double, _Ts...>
>> + + __once<const char_type*, _Ts...>
>> + + __once<basic_string_view<char_type>, _Ts...>
>> + + __once<const void*, _Ts...>;
>> + if (__sum != sizeof...(_Ts))
>> + __invalid_dynamic_spec("invalid template argument types");
>> return true;
>> }
>>
>> @@ -4341,12 +4351,15 @@ namespace __format
>> // _GLIBCXX_RESOLVE_LIB_DEFECTS
>> // 4142. check_dynamic_spec should require at least one type
>> static_assert(sizeof...(_Ts) >= 1);
>> - // This call enforces the Mandates: condition that _Ts contains valid
>> - // types and each type appears at most once. It could be a static_assert
>> - // but this way failures give better diagnostics, due to calling the
>> - // non-constexpr __invalid_dynamic_spec function.
>> - [[maybe_unused]]
>> - constexpr bool __ok = __check_dynamic_spec_types<_Ts...>();
>> +
>> + if constexpr (__check_types<_Ts...>)
>> + if constexpr (__check_dynamic_spec_types<_Ts...>())
>> + {
>> + // The call above enforces the Mandates: condition that _Ts
>> + // contains valid types and each type appears at most once.
>> + // Checking it in `if constexpr` means that failures give better
>> + // diagnostics than if we used it as a static_assert condition.
>> + }
>>
>> if consteval {
>> if (__id >= _M_num_args)
>> --
>> 2.48.1
>>
On Wed, Mar 12, 2025 at 9:23 AM Jonathan Wakely <jwakely@redhat.com> wrote:
> On Wed, 12 Mar 2025 at 07:59, Tomasz Kaminski <tkaminsk@redhat.com> wrote:
> >
> >
> >
> > On Tue, Mar 11, 2025 at 11:46 PM Jonathan Wakely <jwakely@redhat.com>
> wrote:
> >>
> >> This change makes the consteval function slightly faster to compile.
> >> Instead of keeping the counts in an array and looping over that array,
> >> we can just keep a sum of how many valid types are present, and check
> >> that it equals the total number of types in the pack. We can also avoid
> >> doing the check entirely for the common cases where the call comes from
> >> check_dynamic_spec_integer or check_dynamic_spec_string, because those
> >> always use valid lists of types.
> >>
> >> The diagnostic is slightly worse now, because there's only a single
> >> "invalid template argument types" string that appears in the output,
> >> where previously we had either "non-unique template argument type" or
> >> "disallowed template argument type" depending on the failure mode.
> >>
> >> Given that most users will never use this function directly, and
> >> probably won't use invalid types anyway, the inferior diagnostic seems
> >> acceptable.
> >>
> >> libstdc++-v3/ChangeLog:
> >>
> >> * include/std/format
> (basic_format_parse_context::__check_types):
> >> New variable template and partial specializations.
> >> (basic_format_parse_context::__check_dynamic_spec_types):
> >> Simplify for faster compilation.
> >> (basic_format_parse_context::check_dynamic_spec): Only use
> >> __check_dynamic_spec_types when __check_types is true. Use it as
> >> the condition for a constexpr if statement instead of as the
> >> initializer for a constexpr variable.
> >> (basic_format_parse_context::check_dynamic_spec_string): Use
> >> _CharT instead of char_type consistently.
> >> ---
> >>
> >> Tested x86_64-linux.
> >>
> >> libstdc++-v3/include/std/format | 81 +++++++++++++++++++--------------
> >> 1 file changed, 47 insertions(+), 34 deletions(-)
> >>
> >> diff --git a/libstdc++-v3/include/std/format
> b/libstdc++-v3/include/std/format
> >> index 0d6cc7f6bef..6269cbb80e6 100644
> >> --- a/libstdc++-v3/include/std/format
> >> +++ b/libstdc++-v3/include/std/format
> >> @@ -305,41 +305,51 @@ namespace __format
> >> constexpr void
> >> check_dynamic_spec_string(size_t __id) noexcept
> >> {
> >> - check_dynamic_spec<const char_type*,
> basic_string_view<_CharT>>(__id);
> >> + check_dynamic_spec<const _CharT*,
> basic_string_view<_CharT>>(__id);
> >> }
> >>
> >> private:
> >> - // Check the Mandates: condition for check_dynamic_spec<Ts...>(n)
> >> + template<typename _Tp, typename... _Ts>
> >> + static constexpr bool __once = (is_same_v<_Tp, _Ts> + ...) == 1;
> >> +
> >> + // True if we need to call __check_dynamic_spec_types for the
> pack Ts
> >> + template<typename... _Ts>
> >> + static constexpr bool __check_types = sizeof...(_Ts) > 0;
> >> + // The pack used by check_dynamic_spec_integral is valid, don't
> check it.
> >> + // FIXME: simplify these when PR c++/85282 is supported.
> >> + template<typename _Tp>
> >> + static constexpr bool
> >> + __check_types<_Tp, unsigned, long long, unsigned long long>
> >> + = ! is_same_v<_Tp, int>;
> >
> > These seem to produce wrong answer (false) if the user called
> check_dynamic_spec<basic_string_view<_CharT>, unsigned, long long, unsigned
> long long>,
> > which is a valid set of types.
>
> The variable template __check_types means "do we need to check the
> types", so false is the correct answer here. We do not need to check
> the types for this list of types, because we know it's a valid set of
> types.
>
> The variable template is used to decide whether to call
> __check_dynamic_spec_types or to skip calling it.
>
Any particular reason to use this as a solution, as compared to
alternatives with implementation defined function?
I find the latter much more understandable, and while this allows us to
avoid checking if user specified types in exactly same order,
passing <basic_string_view, char> to check_dynamic_spec will still perform
checking.
>
> >>
> >> + // The pack used by check_dynamic_spec_string is valid, don't
> check it.
> >> + template<typename _Tp>
> >> + static constexpr bool
> >> + __check_types<_Tp, basic_string_view<_CharT>>
> >> + = ! is_same_v<const _CharT*, _Tp>;
> >
> > As above check_dynamic_spec<int, basic_string_view<_CharT>>.
> > I think much batter solution would be to have
> __check_dynamic_spec<Ts...> private function,
> > that would not perform the check at all and will be called without
> checking from check_dynamic_spec_integrral, e.t.c.
> > And in check_dynamic_spec, we would do
> __check_dynamic_spec_types<_Ts...>() and sizeof..(Ts) > 0, before
> > calling __check_dynamic_spec.
>
> Yes I considered that, and that would also work.
>
> >> +
> >> + // Check the Mandates: conditions for
> check_dynamic_spec<Ts...>(n)
> >> template<typename... _Ts>
> >> static consteval bool
> >> __check_dynamic_spec_types()
> >> {
> >> - if constexpr (sizeof...(_Ts))
> >> - {
> >> - int __counts[] = {
> >> - (is_same_v<bool, _Ts> + ...),
> >> - (is_same_v<_CharT, _Ts> + ...),
> >> - (is_same_v<int, _Ts> + ...),
> >> - (is_same_v<unsigned, _Ts> + ...),
> >> - (is_same_v<long long, _Ts> + ...),
> >> - (is_same_v<unsigned long long, _Ts> + ...),
> >> - (is_same_v<float, _Ts> + ...),
> >> - (is_same_v<double, _Ts> + ...),
> >> - (is_same_v<long double, _Ts> + ...),
> >> - (is_same_v<const _CharT*, _Ts> + ...),
> >> - (is_same_v<basic_string_view<_CharT>, _Ts> + ...),
> >> - (is_same_v<const void*, _Ts> + ...)
> >> - };
> >> - int __sum = 0;
> >> - for (int __c : __counts)
> >> - {
> >> - __sum += __c;
> >> - if (__c > 1)
> >> - __invalid_dynamic_spec("non-unique template
> argument type");
> >> - }
> >> - if (__sum != sizeof...(_Ts))
> >> - __invalid_dynamic_spec("disallowed template argument
> type");
> >> - }
> >> + // The types in Ts... are unique. Each type in Ts... is one of
> >> + // bool, char_type, int, unsigned int, long long int,
> >> + // unsigned long long int, float, double, long double,
> >> + // const char_type*, basic_string_view<char_type>, or const
> void*.
> >> + unsigned __sum = __once<bool, _Ts...>
> >> + + __once<char_type, _Ts...>
> >> + + __once<int, _Ts...>
> >> + + __once<unsigned int, _Ts...>
> >> + + __once<long long int, _Ts...>
> >> + + __once<unsigned long long int, _Ts...>
> >> + + __once<float, _Ts...>
> >> + + __once<double, _Ts...>
> >> + + __once<long double, _Ts...>
> >> + + __once<const char_type*, _Ts...>
> >> + + __once<basic_string_view<char_type>, _Ts...>
> >> + + __once<const void*, _Ts...>;
> >> + if (__sum != sizeof...(_Ts))
> >> + __invalid_dynamic_spec("invalid template argument types");
> >> return true;
> >> }
> >>
> >> @@ -4341,12 +4351,15 @@ namespace __format
> >> // _GLIBCXX_RESOLVE_LIB_DEFECTS
> >> // 4142. check_dynamic_spec should require at least one type
> >> static_assert(sizeof...(_Ts) >= 1);
> >> - // This call enforces the Mandates: condition that _Ts contains
> valid
> >> - // types and each type appears at most once. It could be a
> static_assert
> >> - // but this way failures give better diagnostics, due to calling
> the
> >> - // non-constexpr __invalid_dynamic_spec function.
> >> - [[maybe_unused]]
> >> - constexpr bool __ok = __check_dynamic_spec_types<_Ts...>();
> >> +
> >> + if constexpr (__check_types<_Ts...>)
> >> + if constexpr (__check_dynamic_spec_types<_Ts...>())
> >> + {
> >> + // The call above enforces the Mandates: condition that _Ts
> >> + // contains valid types and each type appears at most once.
> >> + // Checking it in `if constexpr` means that failures give
> better
> >> + // diagnostics than if we used it as a static_assert
> condition.
> >> + }
> >>
> >> if consteval {
> >> if (__id >= _M_num_args)
> >> --
> >> 2.48.1
> >>
>
>
@@ -305,41 +305,51 @@ namespace __format
constexpr void
check_dynamic_spec_string(size_t __id) noexcept
{
- check_dynamic_spec<const char_type*, basic_string_view<_CharT>>(__id);
+ check_dynamic_spec<const _CharT*, basic_string_view<_CharT>>(__id);
}
private:
- // Check the Mandates: condition for check_dynamic_spec<Ts...>(n)
+ template<typename _Tp, typename... _Ts>
+ static constexpr bool __once = (is_same_v<_Tp, _Ts> + ...) == 1;
+
+ // True if we need to call __check_dynamic_spec_types for the pack Ts
+ template<typename... _Ts>
+ static constexpr bool __check_types = sizeof...(_Ts) > 0;
+ // The pack used by check_dynamic_spec_integral is valid, don't check it.
+ // FIXME: simplify these when PR c++/85282 is supported.
+ template<typename _Tp>
+ static constexpr bool
+ __check_types<_Tp, unsigned, long long, unsigned long long>
+ = ! is_same_v<_Tp, int>;
+ // The pack used by check_dynamic_spec_string is valid, don't check it.
+ template<typename _Tp>
+ static constexpr bool
+ __check_types<_Tp, basic_string_view<_CharT>>
+ = ! is_same_v<const _CharT*, _Tp>;
+
+ // Check the Mandates: conditions for check_dynamic_spec<Ts...>(n)
template<typename... _Ts>
static consteval bool
__check_dynamic_spec_types()
{
- if constexpr (sizeof...(_Ts))
- {
- int __counts[] = {
- (is_same_v<bool, _Ts> + ...),
- (is_same_v<_CharT, _Ts> + ...),
- (is_same_v<int, _Ts> + ...),
- (is_same_v<unsigned, _Ts> + ...),
- (is_same_v<long long, _Ts> + ...),
- (is_same_v<unsigned long long, _Ts> + ...),
- (is_same_v<float, _Ts> + ...),
- (is_same_v<double, _Ts> + ...),
- (is_same_v<long double, _Ts> + ...),
- (is_same_v<const _CharT*, _Ts> + ...),
- (is_same_v<basic_string_view<_CharT>, _Ts> + ...),
- (is_same_v<const void*, _Ts> + ...)
- };
- int __sum = 0;
- for (int __c : __counts)
- {
- __sum += __c;
- if (__c > 1)
- __invalid_dynamic_spec("non-unique template argument type");
- }
- if (__sum != sizeof...(_Ts))
- __invalid_dynamic_spec("disallowed template argument type");
- }
+ // The types in Ts... are unique. Each type in Ts... is one of
+ // bool, char_type, int, unsigned int, long long int,
+ // unsigned long long int, float, double, long double,
+ // const char_type*, basic_string_view<char_type>, or const void*.
+ unsigned __sum = __once<bool, _Ts...>
+ + __once<char_type, _Ts...>
+ + __once<int, _Ts...>
+ + __once<unsigned int, _Ts...>
+ + __once<long long int, _Ts...>
+ + __once<unsigned long long int, _Ts...>
+ + __once<float, _Ts...>
+ + __once<double, _Ts...>
+ + __once<long double, _Ts...>
+ + __once<const char_type*, _Ts...>
+ + __once<basic_string_view<char_type>, _Ts...>
+ + __once<const void*, _Ts...>;
+ if (__sum != sizeof...(_Ts))
+ __invalid_dynamic_spec("invalid template argument types");
return true;
}
@@ -4341,12 +4351,15 @@ namespace __format
// _GLIBCXX_RESOLVE_LIB_DEFECTS
// 4142. check_dynamic_spec should require at least one type
static_assert(sizeof...(_Ts) >= 1);
- // This call enforces the Mandates: condition that _Ts contains valid
- // types and each type appears at most once. It could be a static_assert
- // but this way failures give better diagnostics, due to calling the
- // non-constexpr __invalid_dynamic_spec function.
- [[maybe_unused]]
- constexpr bool __ok = __check_dynamic_spec_types<_Ts...>();
+
+ if constexpr (__check_types<_Ts...>)
+ if constexpr (__check_dynamic_spec_types<_Ts...>())
+ {
+ // The call above enforces the Mandates: condition that _Ts
+ // contains valid types and each type appears at most once.
+ // Checking it in `if constexpr` means that failures give better
+ // diagnostics than if we used it as a static_assert condition.
+ }
if consteval {
if (__id >= _M_num_args)