[PATCH] libstdc++: Fix format call and test formatting with empty specs for durations.

[Tomasz Kaminski]

On Wed, Jun 4, 2025 at 12:43 PM Daniel Krügler <daniel.kruegler@gmail.com>
wrote:

> Am Mi., 4. Juni 2025 um 11:27 Uhr schrieb Tomasz Kamiński <
> tkaminsk@redhat.com>:
>
>> This patches fixes an obvious error, where the output iterator argument
>> was
>> missing for call to format_to, when duration with custom representation
>> types
>> are used.
>>
>> It's also adding the test for behavior of ostream operator and the
>> formatting
>> with empty chron-spec for the chrono types. Current coverage is:
>>  * duration and hh_mm_ss in this commit,
>>  * calendar types in r16-1016-g28a17985dd34b7.
>>
>> libstdc++-v3/ChangeLog:
>>
>>         * include/bits/chrono_io.h (__formatter_chrono:_M_s): Add missing
>>         __out argument to format_to call.
>>         * testsuite/std/time/format/empty_spec.cc: New test.
>> ---
>> Tested on x86_64-linux. OK for trunk?
>>
>>  libstdc++-v3/include/bits/chrono_io.h         |   3 +-
>>  .../testsuite/std/time/format/empty_spec.cc   | 271 ++++++++++++++++++
>>  2 files changed, 273 insertions(+), 1 deletion(-)
>>
>> diff --git a/libstdc++-v3/include/bits/chrono_io.h
>> b/libstdc++-v3/include/bits/chrono_io.h
>> index 346eb8b3c33..239f9c78009 100644
>> --- a/libstdc++-v3/include/bits/chrono_io.h
>> +++ b/libstdc++-v3/include/bits/chrono_io.h
>> @@ -1296,7 +1296,8 @@ namespace __format
>>                   else
>>                     {
>>                       auto __str = std::format(_S_empty_spec,
>> __ss.count());
>> -                     __out = std::format_to(_GLIBCXX_WIDEN("{:0>{}s}"),
>> +                     __out = std::format_to(std::move(__out),
>> +                                            _GLIBCXX_WIDEN("{:0>{}s}"),
>>                                              __str,
>>                                              __hms.fractional_width);
>>                     }
>> diff --git a/libstdc++-v3/testsuite/std/time/format/empty_spec.cc
>> b/libstdc++-v3/testsuite/std/time/format/empty_spec.cc
>> index 322faa1939d..46942dc30fc 100644
>> --- a/libstdc++-v3/testsuite/std/time/format/empty_spec.cc
>> +++ b/libstdc++-v3/testsuite/std/time/format/empty_spec.cc
>> @@ -1,7 +1,9 @@
>>  // { dg-do run { target c++20 } }
>> +// { dg-require-effective-target hosted }
>>  // { dg-timeout-factor 2 }
>>
>>  #include <chrono>
>> +#include <ranges>
>>  #include <sstream>
>>  #include <testsuite_hooks.h>
>>
>> @@ -49,6 +51,274 @@ void verify(const T& t, const _CharT* str)
>>    VERIFY( res == str );
>>  }
>>
>> +template<typename Ret = void>
>> +struct Rep
>> +{
>> +  using Return
>> +    = std::conditional_t<std::is_void_v<Ret>, Rep, Ret>;
>> +
>> +  Rep(long v = 0) : val(v) {}
>> +
>> +  operator long() const
>> +  { return val; }
>> +
>> +  Return
>> +  operator+() const
>> +  { return val; }
>> +
>> +  Rep
>> +  operator-() const
>> +  { return -val; }
>> +
>> +  friend Rep
>> +  operator+(Rep lhs, Rep rhs)
>> +  { return lhs.val + rhs.val; }
>> +
>> +  friend Rep
>> +  operator-(Rep lhs, Rep rhs)
>> +  { return lhs.val - rhs.val; }
>> +
>> +  friend Rep
>> +  operator*(Rep lhs, Rep rhs)
>> +  { return lhs.val * rhs.val; }
>> +
>> +  friend Rep
>> +  operator/(Rep lhs, Rep rhs)
>> +  { return lhs.val / rhs.val; }
>> +
>> +  friend auto operator<=>(Rep, Rep) = default;
>> +
>> +  template<typename _CharT>
>> +  friend std::basic_ostream<_CharT>&
>> +  operator<<(std::basic_ostream<_CharT>& os, const Rep& t)
>> +  { return os << t.val << WIDEN("[via <<]"); }
>> +
>> +  long val;
>> +};
>> +
>> +template<typename Ret, typename Other>
>> +  requires std::is_integral_v<Other>
>> +struct std::common_type<Rep<Ret>, Other>
>> +{
>> +  using type = Rep<Ret>;
>> +};
>> +
>> +template<typename Ret, typename Other>
>> +  requires std::is_integral_v<Other>
>> +struct std::common_type<Other, Rep<Ret>>
>> +  : std::common_type<Rep<Ret>, Other>
>> +{ };
>>
>
> Why is this second specialization needed? It seems to me
> that [meta.trans.other] p5 says that it is already provided by the library
> implementation of  std::common_type.
>
My reading of the paragraph is different. It says that if one is
specialized, we also need to specialize the other:
> Moreover, common_type_t<T1, T2> shall denote the same type, if any, as
does common_type_t<T2, T1>.
<https://eel.is/c++draft/meta.trans.other#5.sentence-4>
> No diagnostic is required for a violation of this Note's rules.
<https://eel.is/c++draft/meta.trans.other#5.sentence-5>
"shall" are requirements on the users.


>
> - Daniel
>
>
>
-------------- next part --------------
An HTML attachment was scrubbed...
URL: <https://gcc.gnu.org/pipermail/libstdc++/attachments/20250604/1021eda4/attachment-0001.htm>