[libcxx-commits] [libcxx] 6c97ad4 - [libc++][chrono] Fix streaming for unsigned durations. (#97889)

[via libcxx-commits]

Author: Mark de Wever
Date: 2024-07-10T20:13:05+02:00
New Revision: 6c97ad4e6838a1172d957df83ec60af646ccacc0

URL: https://github.com/llvm/llvm-project/commit/6c97ad4e6838a1172d957df83ec60af646ccacc0
DIFF: https://github.com/llvm/llvm-project/commit/6c97ad4e6838a1172d957df83ec60af646ccacc0.diff

LOG: [libc++][chrono] Fix streaming for unsigned durations. (#97889)

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](https://cplusplus.github.io/LWG/issue4118) discusses this
issue further.

Fixes https://github.com/llvm/llvm-project/issues/96820

Added: 
    

Modified: 
    libcxx/include/__chrono/formatter.h
    libcxx/test/std/time/time.duration/time.duration.nonmember/ostream.pass.cpp
    libcxx/test/std/time/time.syn/formatter.duration.pass.cpp

Removed: 
    


################################################################################
diff  --git a/libcxx/include/__chrono/formatter.h b/libcxx/include/__chrono/formatter.h
index e9b81c3de8a70..9a77316385abd 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,16 @@ __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);
+      // A duration can be a user defined arithmetic type. Users may specialize
+      // numeric_limits, but they may not specialize is_signed.
+      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 e5d11ab4672bd..aecb96b58719e 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 dbf373a19ba80..c1d7c21d864d0 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>(1));
+  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>(