https://gcc.gnu.org/g:256060380c30e2d8ced63c2f04a0856d46b77343
commit r15-8004-g256060380c30e2d8ced63c2f04a0856d46b77343
Author: Jonathan Wakely <jwak...@redhat.com>
Date: Sat Mar 8 12:07:40 2025 +0000
libstdc++: Optimize basic_format_parse_context::check_dynamic_spec
This change makes the check_dynamic_spec precondition checks slightly
faster to compile, and avoids those checks entirely for the common cases
of calling check_dynamic_spec_integral or check_dynamic_spec_string.
Instead of checking for unique types by keeping 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.
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::__once): New
variable template.
(basic_format_parse_context::__valid_types_for_check_dynamic_spec):
New function template for checking argument types.
(basic_format_parse_context::__check_dynamic_spec): New function
template to implement the common check_dynamic_spec logic.
(basic_format_parse_context::check_dynamic_spec_integral): Call
__check_dynamic_spec instead of check_dynamic_spec.
(basic_format_parse_context::check_dynamic_spec_string):
Likewise. Use _CharT instead of char_type consistently.
(basic_format_parse_context::check_dynamic_spec): Use
__valid_types_for_check_dynamic_spec for precondition checks and
call __check_dynamic_spec for checking the arg id.
* testsuite/std/format/parse_ctx_neg.cc: Adjust expected errors.
Reviewed-by: Tomasz Kamiński <tkami...@redhat.com>
Diff:
---
libstdc++-v3/include/std/format | 110 +++++++++++----------
libstdc++-v3/testsuite/std/format/parse_ctx_neg.cc | 6 +-
2 files changed, 62 insertions(+), 54 deletions(-)
diff --git a/libstdc++-v3/include/std/format b/libstdc++-v3/include/std/format
index 6cfc84cd01b7..bc26599b9f97 100644
--- a/libstdc++-v3/include/std/format
+++ b/libstdc++-v3/include/std/format
@@ -294,55 +294,73 @@ namespace __format
#if __cpp_lib_format >= 202305L
template<typename... _Ts>
constexpr void
- check_dynamic_spec(size_t __id) noexcept;
+ check_dynamic_spec(size_t __id) noexcept
+ {
+ 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);
+ }
+ }
constexpr void
check_dynamic_spec_integral(size_t __id) noexcept
{
- check_dynamic_spec<int, unsigned, long long, unsigned long long>(__id);
+ if consteval {
+ __check_dynamic_spec<int, unsigned, long long,
+ unsigned long long>(__id);
+ }
}
constexpr void
check_dynamic_spec_string(size_t __id) noexcept
{
- check_dynamic_spec<const char_type*, basic_string_view<_CharT>>(__id);
+ if consteval {
+ __check_dynamic_spec<const _CharT*, basic_string_view<_CharT>>(__id);
+ }
}
private:
- // Check the Mandates: condition for check_dynamic_spec<Ts...>(n)
+ // True if _Tp occurs exactly once in _Ts.
+ template<typename _Tp, typename... _Ts>
+ static constexpr bool __once = (is_same_v<_Tp, _Ts> + ...) == 1;
+
template<typename... _Ts>
- static consteval bool
- __check_dynamic_spec_types()
+ consteval bool
+ __valid_types_for_check_dynamic_spec()
{
- if constexpr (sizeof...(_Ts))
+ // _GLIBCXX_RESOLVE_LIB_DEFECTS
+ // 4142. check_dynamic_spec should require at least one type
+ if constexpr (sizeof...(_Ts) == 0)
+ return false;
+ else
{
- 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...>;
+ return __sum == sizeof...(_Ts);
}
- return true;
}
+ template<typename... _Ts>
+ consteval void
+ __check_dynamic_spec(size_t __id) noexcept;
+
// This must not be constexpr.
static void __invalid_dynamic_spec(const char*);
@@ -4316,29 +4334,21 @@ namespace __format
} // namespace __format
/// @endcond
-#if __cpp_lib_format >= 202305L
+#if __cpp_lib_format >= 202305L // >= C++26
+ /// @cond undocumented
+ // Common implementation of check_dynamic_spec{,_string,_integral}
template<typename _CharT>
- template<typename... _Ts>
- constexpr void
- basic_format_parse_context<_CharT>::check_dynamic_spec(size_t __id)
noexcept
- {
- // _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 consteval {
+ 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_context = __format::_Scanner<_CharT>::_Parse_context;
- auto __arg = static_cast<_Parse_context*>(this)->_M_types[__id];
+ 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>()...
};
@@ -4348,7 +4358,7 @@ namespace __format
}
__invalid_dynamic_spec("arg(id) type does not match");
}
- }
+ /// @endcond
#endif
template<typename _CharT, typename... _Args>
diff --git a/libstdc++-v3/testsuite/std/format/parse_ctx_neg.cc
b/libstdc++-v3/testsuite/std/format/parse_ctx_neg.cc
index d83fd8c7a7b0..57c21a738610 100644
--- a/libstdc++-v3/testsuite/std/format/parse_ctx_neg.cc
+++ b/libstdc++-v3/testsuite/std/format/parse_ctx_neg.cc
@@ -38,7 +38,5 @@ test_invalid()
wpc.check_dynamic_spec<char32_t>(0); // { dg-error "here" }
}
-// Each failure above will point to a call to this non-constexpr function:
-// { dg-error "__invalid_dynamic_spec" "" { target *-*-* } 0 }
-// Except the check_dynamic_spec<>(0) one for LWG 4142 which matches this:
-// { dg-error "static assertion failed" "" { target *-*-* } 0 }
+// Each failure above will trigger this static_assert:
+// { dg-error "allowed types" "" { target *-*-* } 0 }
