https://gcc.gnu.org/bugzilla/show_bug.cgi?id=127220

Steve Sudit <stevensudit at gmail dot com> changed:

           What    |Removed                     |Added
----------------------------------------------------------------------------
  Attachment #65512|0                           |1
        is obsolete|                            |

--- Comment #8 from Steve Sudit <stevensudit at gmail dot com> ---
Created attachment 65527
  --> https://gcc.gnu.org/bugzilla/attachment.cgi?id=65527&action=edit
alternative: non-template wrappers, no layout change

You have a point: the patch I proposed is indeed an ABI break and that's
too high a price to pay to address a problem whose root cause lies
outside this project. Even worse, it would have moved _M_types into
basic_format_parse_context, under a C++26-only guard at that, so C++23
and C++26 translation units would disagree about the layout of the same
type.

I concede that this was not a good solution and withdraw that patch.
Still, while I accept your decision, I also take it as a challenge to
do better. There is a way to get the same result without touching any
layout, so I'm attaching it in case it is closer to acceptable.

The problem only exists for templates: clang instantiates
__check_dynamic_spec at the first non-dependent reference and does not
retry before the end of the translation unit. A non-template function
is never instantiated, so it can be declared before the class and
defined after _Scanner.

The new patch declares four consteval non-template functions,
__format::__check_dynamic_spec_{integral,string} for char and wchar_t,
friends them, has check_dynamic_spec_integral and _string call them,
and defines each as a one-line forwarder to the existing
__check_dynamic_spec right after its out-of-class definition.

Other character types keep calling the template directly (an if
constexpr selects the path), since nothing in the header instantiates
it for them before its definition. Nothing moves, no member changes,
and the generic check_dynamic_spec<Ts...> is untouched.

It is larger than I would like (+77/-3), mostly because the wrappers
have to be non-templates, so char and wchar_t each get their own, and
the two call sites need the if constexpr. Also, the shared template is
private, so each wrapper needs a friend declaration. Making
__check_dynamic_spec public would cut about 14 lines, if that is
preferable.

Verified against trunk r17-3939 (with the PR 127219 change applied):
clang 23.1 and trunk g++ at -std=c++20, 23 and 26 accept the attached
format_dynamic_width.cc and the test from PR 127219 with no errors. A
wrong dynamic argument type ({:{}} with a string as the width) is still
rejected at compile time by both. A runtime test of dynamic width and
precision prints the expected output with both compilers linked against
the built libstdc++. Runtime calls to check_dynamic_spec_integral and
_string on a char8_t or char16_t parse context still compile with both,
as they do today. The -Wall -Wextra warning set is unchanged. I have not
run the libstdc++ testsuite on it. As with the first patch, it was
prepared with LLM assistance and reviewed by me.

This is the best I could come up with, so if it's not good enough then
I'll accept that and move on. Whether the extra indirection is worth
carrying for a clang bug is a maintainability judgment that is yours to
make. If it looks acceptable, I will send it to the lists with a
ChangeLog; if not, just mark it WONTFIX.

Either way, thanks for your patience with me. I realize this is not your
rodeo and not your goat, but these interdependencies are how the
ecosystem works.

Reply via email to