[libcxx-commits] [libcxx] [libc++][chrono] Implements duration Rep constraints. (PR #80539)

Mark de Wever via libcxx-commits libcxx-commits at lists.llvm.org
Sat Feb 3 04:43:25 PST 2024


https://github.com/mordante created https://github.com/llvm/llvm-project/pull/80539

Applies LWG3050 to the constraints of operator*, operator/, and operator%. The changes to the constructor were done in
https://reviews.llvm.org/D118902, but that patch did not identify the related LWG-issue, and only adjusted the constructor to the wording in the Standard.

Implements:
- LWG 3050: Conversion specification problem in chrono::duration constructor

>From edea3d7951916f784e2552b386b6f6cad9606431 Mon Sep 17 00:00:00 2001
From: Mark de Wever <koraq at xs4all.nl>
Date: Sat, 3 Feb 2024 13:40:15 +0100
Subject: [PATCH] [libc++][chrono] Implements duration Rep constraints.

Applies LWG3050 to the constraints of operator*, operator/, and operator%.
The changes to the constructor were done in
https://reviews.llvm.org/D118902, but that patch did not identify the
related LWG-issue, and only adjusted the constructor to the wording in the
Standard.

Implements:
- LWG 3050: Conversion specification problem in chrono::duration constructor
---
 libcxx/docs/Status/Cxx20Issues.csv            |  2 +-
 libcxx/include/__chrono/duration.h            |  8 ++---
 libcxx/include/chrono                         |  2 +-
 libcxx/test/std/time/rep.h                    | 23 ++++++++++++++
 .../op_divide_duration.pass.cpp               | 15 ++++++++--
 .../op_mod_duration.pass.cpp                  | 15 ++++++++--
 .../op_times_rep.pass.cpp                     | 30 ++++++++++++++-----
 7 files changed, 77 insertions(+), 18 deletions(-)

diff --git a/libcxx/docs/Status/Cxx20Issues.csv b/libcxx/docs/Status/Cxx20Issues.csv
index 964c21df97e21..be1360adb55cd 100644
--- a/libcxx/docs/Status/Cxx20Issues.csv
+++ b/libcxx/docs/Status/Cxx20Issues.csv
@@ -192,7 +192,7 @@
 "`1203 <https://wg21.link/LWG1203>`__","More useful rvalue stream insertion","Prague","|Complete|","12.0"
 "`2859 <https://wg21.link/LWG2859>`__","Definition of *reachable* in [ptr.launder] misses pointer arithmetic from pointer-interconvertible object","Prague","",""
 "`3018 <https://wg21.link/LWG3018>`__","``shared_ptr``\  of function type","Prague","",""
-"`3050 <https://wg21.link/LWG3050>`__","Conversion specification problem in ``chrono::duration``\  constructor","Prague","","","|chrono|"
+"`3050 <https://wg21.link/LWG3050>`__","Conversion specification problem in ``chrono::duration``\  constructor","Prague","|Complete|","19.0","|chrono|"
 "`3141 <https://wg21.link/LWG3141>`__","``CopyConstructible``\  doesn't preserve source values","Prague","|Nothing to do|",""
 "`3150 <https://wg21.link/LWG3150>`__","``UniformRandomBitGenerator``\  should validate ``min``\  and ``max``\ ","Prague","|Complete|","13.0","|ranges|"
 "`3175 <https://wg21.link/LWG3175>`__","The ``CommonReference``\  requirement of concept ``SwappableWith``\  is not satisfied in the example","Prague","|Complete|","13.0"
diff --git a/libcxx/include/__chrono/duration.h b/libcxx/include/__chrono/duration.h
index 5693ee6440916..1e81420244b14 100644
--- a/libcxx/include/__chrono/duration.h
+++ b/libcxx/include/__chrono/duration.h
@@ -412,7 +412,7 @@ inline _LIBCPP_HIDE_FROM_ABI _LIBCPP_CONSTEXPR
 template <class _Rep1,
           class _Period,
           class _Rep2,
-          __enable_if_t<is_convertible<_Rep2, typename common_type<_Rep1, _Rep2>::type>::value, int> = 0>
+          __enable_if_t<is_convertible<const _Rep2&, typename common_type<_Rep1, _Rep2>::type>::value, int> = 0>
 inline _LIBCPP_HIDE_FROM_ABI _LIBCPP_CONSTEXPR duration<typename common_type<_Rep1, _Rep2>::type, _Period>
 operator*(const duration<_Rep1, _Period>& __d, const _Rep2& __s) {
   typedef typename common_type<_Rep1, _Rep2>::type _Cr;
@@ -423,7 +423,7 @@ operator*(const duration<_Rep1, _Period>& __d, const _Rep2& __s) {
 template <class _Rep1,
           class _Period,
           class _Rep2,
-          __enable_if_t<is_convertible<_Rep1, typename common_type<_Rep1, _Rep2>::type>::value, int> = 0>
+          __enable_if_t<is_convertible<const _Rep1&, typename common_type<_Rep1, _Rep2>::type>::value, int> = 0>
 inline _LIBCPP_HIDE_FROM_ABI _LIBCPP_CONSTEXPR duration<typename common_type<_Rep1, _Rep2>::type, _Period>
 operator*(const _Rep1& __s, const duration<_Rep2, _Period>& __d) {
   return __d * __s;
@@ -435,7 +435,7 @@ template <class _Rep1,
           class _Period,
           class _Rep2,
           __enable_if_t<!__is_duration<_Rep2>::value &&
-                            is_convertible<_Rep2, typename common_type<_Rep1, _Rep2>::type>::value,
+                            is_convertible<const _Rep2&, typename common_type<_Rep1, _Rep2>::type>::value,
                         int> = 0>
 inline _LIBCPP_HIDE_FROM_ABI _LIBCPP_CONSTEXPR duration<typename common_type<_Rep1, _Rep2>::type, _Period>
 operator/(const duration<_Rep1, _Period>& __d, const _Rep2& __s) {
@@ -457,7 +457,7 @@ template <class _Rep1,
           class _Period,
           class _Rep2,
           __enable_if_t<!__is_duration<_Rep2>::value &&
-                            is_convertible<_Rep2, typename common_type<_Rep1, _Rep2>::type>::value,
+                            is_convertible<const _Rep2&, typename common_type<_Rep1, _Rep2>::type>::value,
                         int> = 0>
 inline _LIBCPP_HIDE_FROM_ABI _LIBCPP_CONSTEXPR duration<typename common_type<_Rep1, _Rep2>::type, _Period>
 operator%(const duration<_Rep1, _Period>& __d, const _Rep2& __s) {
diff --git a/libcxx/include/chrono b/libcxx/include/chrono
index c80fa78a56ba1..f8407419c9544 100644
--- a/libcxx/include/chrono
+++ b/libcxx/include/chrono
@@ -58,7 +58,7 @@ public:
         constexpr explicit duration(const Rep2& r,
             typename enable_if
             <
-               is_convertible<Rep2, rep>::value &&
+               is_convertible<const Rep2&, rep>::value &&
                (treat_as_floating_point<rep>::value ||
                !treat_as_floating_point<rep>::value && !treat_as_floating_point<Rep2>::value)
             >::type* = 0);
diff --git a/libcxx/test/std/time/rep.h b/libcxx/test/std/time/rep.h
index 80a0e3c545718..6fcc8f6721db4 100644
--- a/libcxx/test/std/time/rep.h
+++ b/libcxx/test/std/time/rep.h
@@ -10,6 +10,7 @@
 #define REP_H
 
 #include "test_macros.h"
+#include <type_traits>
 
 class Rep
 {
@@ -29,6 +30,28 @@ class Rep
 
 struct NotARep {};
 
+#if TEST_STD_VER >= 11
+// Several duration operators take a Rep parameter. Before LWG3050 this
+// parameter was constrained to be convertible from a non-const object,
+// but the code always uses a const object. So the function was SFINEA's
+// away for this type. LWG3050 fixes the constrain to use a const
+// object.
+struct RepConstConvertibleLWG3050 {
+  operator long() = delete;
+  operator long() const { return 2; }
+};
+namespace std {
+template <>
+struct common_type<RepConstConvertibleLWG3050, int> {
+  using type = long;
+};
+template <>
+struct common_type<int, RepConstConvertibleLWG3050> {
+  using type = long;
+};
+} // namespace std
+#endif // TEST_STD_VER >= 11
+
 // std::chrono:::duration has only '*', '/' and '%' taking a "Rep" parameter
 
 // Multiplication is commutative, division is not.
diff --git a/libcxx/test/std/time/time.duration/time.duration.nonmember/op_divide_duration.pass.cpp b/libcxx/test/std/time/time.duration/time.duration.nonmember/op_divide_duration.pass.cpp
index d580f4edf2fe5..6cedd13a13cfa 100644
--- a/libcxx/test/std/time/time.duration/time.duration.nonmember/op_divide_duration.pass.cpp
+++ b/libcxx/test/std/time/time.duration/time.duration.nonmember/op_divide_duration.pass.cpp
@@ -21,6 +21,7 @@
 
 #include "test_macros.h"
 #include "truncate_fp.h"
+#include "../../rep.h"
 
 int main(int, char**)
 {
@@ -65,7 +66,17 @@ int main(int, char**)
     constexpr std::chrono::duration<double, std::ratio<3, 5> > s2(5);
     static_assert(s1 / s2 == 20./3, "");
     }
-#endif
+    {
+      std::chrono::duration<int> d(5);
+      RepConstConvertibleLWG3050 x;
+
+      {
+        auto r = d / x;
+        assert(r.count() == 2);
+        ASSERT_SAME_TYPE(std::chrono::duration<long>, decltype(r));
+      }
+    }
+#endif // TEST_STD_VER >= 11
 
-  return 0;
+    return 0;
 }
diff --git a/libcxx/test/std/time/time.duration/time.duration.nonmember/op_mod_duration.pass.cpp b/libcxx/test/std/time/time.duration/time.duration.nonmember/op_mod_duration.pass.cpp
index 8b8b50d940b84..df637e1c23e30 100644
--- a/libcxx/test/std/time/time.duration/time.duration.nonmember/op_mod_duration.pass.cpp
+++ b/libcxx/test/std/time/time.duration/time.duration.nonmember/op_mod_duration.pass.cpp
@@ -18,6 +18,7 @@
 #include <chrono>
 #include <cassert>
 #include <ratio>
+#include "../../rep.h"
 
 #include "test_macros.h"
 
@@ -60,7 +61,17 @@ int main(int, char**)
     constexpr std::chrono::duration<int, std::ratio<1, 15> > r = s1 % s2;
     static_assert(r.count() == 24, "");
     }
-#endif
+    {
+      std::chrono::duration<int> d(5);
+      RepConstConvertibleLWG3050 x;
+
+      {
+        auto r = d % x;
+        assert(r.count() == 1);
+        ASSERT_SAME_TYPE(std::chrono::duration<long>, decltype(r));
+      }
+    }
+#endif // TEST_STD_VER >= 11
 
-  return 0;
+    return 0;
 }
diff --git a/libcxx/test/std/time/time.duration/time.duration.nonmember/op_times_rep.pass.cpp b/libcxx/test/std/time/time.duration/time.duration.nonmember/op_times_rep.pass.cpp
index c331032312826..d7c8c2da3c325 100644
--- a/libcxx/test/std/time/time.duration/time.duration.nonmember/op_times_rep.pass.cpp
+++ b/libcxx/test/std/time/time.duration/time.duration.nonmember/op_times_rep.pass.cpp
@@ -26,28 +26,27 @@
 #include "test_macros.h"
 #include "../../rep.h"
 
-int main(int, char**)
-{
-    {
+int main(int, char**) {
+  {
     std::chrono::nanoseconds ns(3);
     ns = ns * 5;
     assert(ns.count() == 15);
     ns = 6 * ns;
     assert(ns.count() == 90);
-    }
+  }
 
 #if TEST_STD_VER >= 11
-    {
+  {
     constexpr std::chrono::nanoseconds ns(3);
     constexpr std::chrono::nanoseconds ns2 = ns * 5;
     static_assert(ns2.count() == 15, "");
     constexpr std::chrono::nanoseconds ns3 = 6 * ns;
     static_assert(ns3.count() == 18, "");
-    }
+  }
 #endif
 
 #if TEST_STD_VER >= 11
-    { // This is related to PR#41130
+  { // This is related to PR#41130
     typedef std::chrono::nanoseconds Duration;
     Duration d(5);
     NotARep n;
@@ -57,8 +56,23 @@ int main(int, char**)
     assert(d.count() == 5);
     d = n * d;
     assert(d.count() == 5);
+  }
+  {
+    std::chrono::duration<int> d(8);
+    RepConstConvertibleLWG3050 x;
+
+    {
+      auto r = d * x;
+      assert(r.count() == 16);
+      ASSERT_SAME_TYPE(std::chrono::duration<long>, decltype(r));
     }
-#endif
+    {
+      auto r = x * d;
+      assert(r.count() == 16);
+      ASSERT_SAME_TYPE(std::chrono::duration<long>, decltype(r));
+    }
+  }
+#endif // TEST_STD_VER >= 11
 
   return 0;
 }



More information about the libcxx-commits mailing list