On Wed, 28 Aug 2024 at 10:45, Jonathan Wakely <jwak...@redhat.com> wrote:
>
> On Wed, 28 Aug 2024 at 10:38, Jason Merrill <ja...@redhat.com> wrote:
> >
> > Tested x86_64-pc-linux-gnu.  OK for trunk, or were these supposed to be 
> > used?
>
> I think the _RefT typedef was used in an earlier version of the code
> (possibly a much earlier version that was never even pushed). It can
> be removed.

It should have been removed in r14-6199-g45630fbcf7875b.


>
> I think the __ok variable exists to force constant evaluation of
> __check_dynamic_spec_types, although I think that could be done this
> way instead:
>
> if (is_constant_evaluated())
>   __check_dynamic_spec_types();

That's not right because it needs to be called for non-constexpr
evaluations too.

I tried simply making it return void and then calling it like this:

diff --git a/libstdc++-v3/include/std/format b/libstdc++-v3/include/std/format
index 3280dadfb90..450d65dc80d 100644
--- a/libstdc++-v3/include/std/format
+++ b/libstdc++-v3/include/std/format
@@ -305,7 +305,7 @@ namespace __format
    private:
      // Check the Mandates: condition for check_dynamic_spec<Ts...>(n)
      template<typename... _Ts>
-       static consteval bool
+       static consteval void
       __check_dynamic_spec_types()
       {
         if constexpr (sizeof...(_Ts))
@@ -334,7 +334,6 @@ namespace __format
             if (__sum != sizeof...(_Ts))
               __invalid_dynamic_spec("disallowed template argument type");
           }
-         return true;
       }

      // This must not be constexpr.
@@ -4323,7 +4322,7 @@ namespace __format
    constexpr void
    basic_format_parse_context<_CharT>::check_dynamic_spec(size_t __id) noexcept
    {
-      constexpr bool __ok = __check_dynamic_spec_types<_Ts...>();
+      __check_dynamic_spec_types<_Ts...>();

      if consteval {
       if (__id >= _M_num_args)

But now the std/format/parse_ctx_neg.cc test fails. I think the code
actually works as intended, but the quality of the diagnostic suffers
due to the change above. The errors no longer point to the line where
the immediately-escalating expression is originally called from, and
so the { dg-error "here" } lines in the test no longer match.

So I think keeping the constexpr local variable in the function is
preferable, so marking it [[maybe_unused]] is the right fix.

Reply via email to