[libc++][chrono] Implements duration Rep constraints. (#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 --------- Co-authored-by: h-vetinari <[email protected]>
intel · Feb 10, 2024 · 7d9540e · 7d9540e
1 parent 33c6b77
commit 7d9540e
Show file tree

Hide file tree

Showing 7 changed files with 77 additions and 18 deletions.
diff --git 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
@@ -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
@@ -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
@@ -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 SFINAE'd
+// away for this type. LWG3050 fixes the constraint 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
@@ -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
@@ -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
@@ -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;
 }