[libc++][chrono] Fix streaming for unsigned durations.

[mordante]

This fixes formatting for durations using the unsigned types unsigned short, unsigned, unsigned long, and unsigned long long. It does not allow the unsigned char type. Since the formatting function uses ostream::operator<< this type probably does not do what it should do.

Note that based on the standard all arithmetic types are allowed, including bool, char, wchar_t. These types are not implemented either. Allowing them seems like a defect in the Standard.

No effort is done to support user-defined types; the wording in the Standard is unclear regarding the constrains for these types.

LWG 4118 discusses this issue further.

Fixes #96820

[llvmbot]

@llvm/pr-subscribers-libcxx

Author: Mark de Wever (mordante)

Changes

This fixes formatting for durations using the unsigned types unsigned short, unsigned, unsigned long, and unsigned long long. It does not allow the unsigned char type. Since the formatting function uses ostream::operator<< this type probably does not what it should do.

Note that based on the standard all artithemetic types are allowed, including bool, char, wchar_t. These types are not implemented either. Allowing them seems like a defect in the Standard.

No effort is done to support user-defined types; the wording in the Standard is unclear regarding the constrains for these types.

Fixes #96820

---

Full diff: https://github.com/llvm/llvm-project/pull/97889.diff

3 Files Affected:

• (modified) libcxx/include/__chrono/formatter.h (+9-3)
• (modified) libcxx/test/std/time/time.duration/time.duration.nonmember/ostream.pass.cpp (+25)
• (modified) libcxx/test/std/time/time.syn/formatter.duration.pass.cpp (+12)

diff --git a/libcxx/include/__chrono/formatter.h b/libcxx/include/__chrono/formatter.h
index e9b81c3de8a700..22694eb3bd083c 100644
--- a/libcxx/include/__chrono/formatter.h
+++ b/libcxx/include/__chrono/formatter.h
@@ -46,6 +46,7 @@
 #include <__memory/addressof.h>
 #include <cmath>
 #include <ctime>
+#include <limits>
 #include <sstream>
 #include <string_view>
 
@@ -589,9 +590,14 @@ __format_chrono(const _Tp& __value,
     __sstr << __value;
   else {
     if constexpr (chrono::__is_duration<_Tp>::value) {
-      if (__value < __value.zero())
-        __sstr << _CharT('-');
-      __formatter::__format_chrono_using_chrono_specs(__sstr, chrono::abs(__value), __chrono_specs);
+      if constexpr (numeric_limits<typename _Tp::rep>::is_signed) {
+        if (__value < __value.zero()) {
+          __sstr << _CharT('-');
+          __formatter::__format_chrono_using_chrono_specs(__sstr, -__value, __chrono_specs);
+        } else
+          __formatter::__format_chrono_using_chrono_specs(__sstr, __value, __chrono_specs);
+      } else
+        __formatter::__format_chrono_using_chrono_specs(__sstr, __value, __chrono_specs);
       // TODO FMT When keeping the precision it will truncate the string.
       // Note that the behaviour what the precision does isn't specified.
       __specs.__precision_ = -1;
diff --git a/libcxx/test/std/time/time.duration/time.duration.nonmember/ostream.pass.cpp b/libcxx/test/std/time/time.duration/time.duration.nonmember/ostream.pass.cpp
index e5d11ab4672bdf..aecb96b58719e2 100644
--- a/libcxx/test/std/time/time.duration/time.duration.nonmember/ostream.pass.cpp
+++ b/libcxx/test/std/time/time.duration/time.duration.nonmember/ostream.pass.cpp
@@ -216,10 +216,35 @@ static void test_units() {
   assert(stream_ja_JP_locale<CharT>(std::chrono::duration<int, std::ratio<11, 9>>(0)) == SV("0[11/9]s"));
 }
 
+template <class CharT>
+static void test_unsigned_types() {
+  // Reported in https://github.com/llvm/llvm-project/issues/96820
+  using namespace std::literals::chrono_literals;
+
+  // C locale
+  assert(stream_c_locale<CharT>(std::chrono::duration<unsigned short, std::atto>(0)) == SV("0as"));
+  assert(stream_c_locale<CharT>(std::chrono::duration<unsigned, std::femto>(0)) == SV("0fs"));
+  assert(stream_c_locale<CharT>(std::chrono::duration<unsigned long, std::pico>(0)) == SV("0ps"));
+  assert(stream_c_locale<CharT>(std::chrono::duration<unsigned long long, std::nano>(0)) == SV("0ns"));
+
+  // fr_FR locale
+  assert(stream_fr_FR_locale<CharT>(std::chrono::duration<unsigned short, std::atto>(0)) == SV("0as"));
+  assert(stream_fr_FR_locale<CharT>(std::chrono::duration<unsigned, std::femto>(0)) == SV("0fs"));
+  assert(stream_fr_FR_locale<CharT>(std::chrono::duration<unsigned long, std::pico>(0)) == SV("0ps"));
+  assert(stream_fr_FR_locale<CharT>(std::chrono::duration<unsigned long long, std::nano>(0)) == SV("0ns"));
+
+  // ja_JP locale
+  assert(stream_ja_JP_locale<CharT>(std::chrono::duration<unsigned short, std::atto>(0)) == SV("0as"));
+  assert(stream_ja_JP_locale<CharT>(std::chrono::duration<unsigned, std::femto>(0)) == SV("0fs"));
+  assert(stream_ja_JP_locale<CharT>(std::chrono::duration<unsigned long, std::pico>(0)) == SV("0ps"));
+  assert(stream_ja_JP_locale<CharT>(std::chrono::duration<unsigned long long, std::nano>(0)) == SV("0ns"));
+}
+
 template <class CharT>
 static void test() {
   test_values<CharT>();
   test_units<CharT>();
+  test_unsigned_types<CharT>();
 }
 
 int main(int, char**) {
diff --git a/libcxx/test/std/time/time.syn/formatter.duration.pass.cpp b/libcxx/test/std/time/time.syn/formatter.duration.pass.cpp
index dbf373a19ba806..b503d8ecc2817c 100644
--- a/libcxx/test/std/time/time.syn/formatter.duration.pass.cpp
+++ b/libcxx/test/std/time/time.syn/formatter.duration.pass.cpp
@@ -1157,12 +1157,24 @@ static void test_pr62082() {
   check(SV("00.111111"), SV("{:%S}"), std::chrono::duration<long double, std::ratio<1, 9>>{1});
 }
 
+template <class CharT>
+static void test_unsigned_duration() {
+  // Reported in https://github.com/llvm/llvm-project/issues/96820
+  using namespace std::literals::chrono_literals;
+
+  check(SV("1as"), SV("{}"), std::chrono::duration<unsigned short, std::atto>(500));
+  check(SV("1fs"), SV("{}"), std::chrono::duration<unsigned, std::femto>(1));
+  check(SV("1ps"), SV("{}"), std::chrono::duration<unsigned long, std::pico>(1));
+  check(SV("1ns"), SV("{}"), std::chrono::duration<unsigned long long, std::nano>(1));
+}
+
 template <class CharT>
 static void test() {
   using namespace std::literals::chrono_literals;
 
   test_no_chrono_specs<CharT>();
   test_valid_values<CharT>();
+  test_unsigned_duration<CharT>();
   test_pr62082<CharT>();
 
   check_invalid_types<CharT>(

[ldionne]

I would use std::is_signed_v<typename _Tp::rep> instead. It allows you to include only <__type_traits/is_signed.h>.

[mordante]

Actually there is a difference between the two. Users may specialize numeric_limits, but specializing is_signed is not allowed. Durations can use a user defined arithmetic type. So using numeric_limits seems to be the better choice. I've added comment in the code.

[github-actions]

✅ With the latest revision this PR passed the C/C++ code formatter.