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 <tkaminsk@redhat.com>

diff --git a/libstdc++-v3/include/std/format b/libstdc++-v3/include/std/format
index 6cfc84cd01b73300834c13cc8b4d137d1f06aeea..bc26599b9f97b1a7d1f2cd792c142595ada9da8a 100644 (file)
--- 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 d83fd8c7a7b068aeb7da9916648b8ee4222631dc..57c21a73861095ab45e2951bd3ad2e2a0059131f 100644 (file)
--- 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 }