On Wed, 28 Aug 2024 at 10:45, Jonathan Wakely <jwakely@redhat.com> wrote:
>
> On Wed, 28 Aug 2024 at 10:38, Jason Merrill <jason@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.