On Fri, 21 Mar 2025 at 09:47, Tomasz Kaminski <tkami...@redhat.com> wrote:
>
>
>
> On Thu, Mar 20, 2025 at 5:29 PM Jonathan Wakely <jwak...@redhat.com> wrote:
>>
>> On Thu, 20 Mar 2025 at 16:16, Jonathan Wakely <jwak...@redhat.com> wrote:
>> >
>> > When using std::time_put to format a chrono value, we should imbue the
>> > formatting locale into the stream. This ensures that when
>> > std::time_put::do_put uses a ctype or __timepunct facet from the locale,
>> > it gets the correct facets.
>> >
>> > libstdc++-v3/ChangeLog:
>> >
>> > * include/bits/chrono_io.h (__formatter_chrono::_M_locale_fmt):
>> > Imbue locale into ostringstream.
>> > * testsuite/std/time/format/localized.cc: Check that correct
>> > locale is used for call to time_put::put.
>> > ---
>> >
>> > Now with a test that shows the bug independent of the PR117214 patches
>> > to use std::time_put for %c formats.
>> >
>> > Tested x86_64-linux.
>> >
>> > libstdc++-v3/include/bits/chrono_io.h | 1 +
>> > .../testsuite/std/time/format/localized.cc | 30 +++++++++++++++++++
>> > 2 files changed, 31 insertions(+)
>> >
>> > diff --git a/libstdc++-v3/include/bits/chrono_io.h
>> > b/libstdc++-v3/include/bits/chrono_io.h
>> > index c16b555df29..55ebd4ee061 100644
>> > --- a/libstdc++-v3/include/bits/chrono_io.h
>> > +++ b/libstdc++-v3/include/bits/chrono_io.h
>> > @@ -1697,6 +1697,7 @@ namespace __format
>> > char __fmt, char __mod) const
>> > {
>> > basic_ostringstream<_CharT> __os;
>> > + __os.imbue(__loc);
>> > const auto& __tp = use_facet<time_put<_CharT>>(__loc);
>> > __tp.put(__os, __os, _S_space, &__tm, __fmt, __mod);
>> > if (__os)
>> > diff --git a/libstdc++-v3/testsuite/std/time/format/localized.cc
>> > b/libstdc++-v3/testsuite/std/time/format/localized.cc
>> > index 393d0d200e4..437a4a011b2 100644
>> > --- a/libstdc++-v3/testsuite/std/time/format/localized.cc
>> > +++ b/libstdc++-v3/testsuite/std/time/format/localized.cc
>> > @@ -13,6 +13,7 @@
>> >
>> > #include <chrono>
>> > #include <format>
>> > +#include <locale>
>> > #include <stdio.h>
>> > #include <testsuite_hooks.h>
>> >
>> > @@ -81,6 +82,35 @@ test_en()
>> > }
>> > }
>> >
>> > +void
>> > +test_locale_imbued()
>> > +{
>> > + // Custom time_put facet which returns %b string for %Om
>> > + struct TimePut : std::time_put<char>
>> > + {
>> > + iter_type
>> > + do_put(iter_type out, std::ios_base& io, char_type fill, const tm* t,
>> > + char format, char modifier) const override
>> > + {
>> > + if (format == 'm' && modifier == 'O')
>> > + format = 'b';
>> > + return std::time_put<char>::do_put(out, io, fill, t, format, 0);
>> > + }
>> > + };
>> > +
>> > + auto m = std::chrono::March;
>> > +
>> > + std::locale fr(ISO_8859(1,fr_FR));
>> > + std::locale fr2(fr, new TimePut);
>> > + auto s1 = std::format(fr2, "{:L%Om}", m);
>> > + VERIFY( s1 == std::format(fr, "{:L}", m) );
>> > +
>> > + std::locale es(ISO_8859(1,es_ES));
>> > + std::locale es2(es, new TimePut);
>> > + auto s2 = std::format(es2, "{:L%Om}", m);
>> > + VERIFY( s2 == std::format(es, "{:L}", m) );
>> > +}
>> > +
>> > int main()
>> > {
>> > test_ru();
>>
>> It would help if I added the new test_locale_imbued() function to main
>> so that it actually runs.
>
> LGTM with that addition.
I also added some more comments in the new test, to explain what is
expected (so it's more clear why it didn't work before the commit).
>>
>>
>> Luckily it passes when I do that.
>>
