[libcxx-commits] [libcxx] [libc++][chrono] Implements duration Rep constraints. (PR #80539)
Mark de Wever via libcxx-commits
libcxx-commits at lists.llvm.org
Sun Feb 4 09:33:32 PST 2024
https://github.com/mordante updated https://github.com/llvm/llvm-project/pull/80539
>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 1/2] [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;
}
>From d9fffc7a04097d3465c1c4ee4c8a1b3c62005c24 Mon Sep 17 00:00:00 2001
From: Mark de Wever <zar-rpg at xs4all.nl>
Date: Sun, 4 Feb 2024 18:33:26 +0100
Subject: [PATCH 2/2] Update libcxx/test/std/time/rep.h
Co-authored-by: h-vetinari <h.vetinari at gmx.com>
---
libcxx/test/std/time/rep.h | 4 ++--
1 file changed, 2 insertions(+), 2 deletions(-)
diff --git a/libcxx/test/std/time/rep.h b/libcxx/test/std/time/rep.h
index 6fcc8f6721db4..ddb5c0b249317 100644
--- a/libcxx/test/std/time/rep.h
+++ b/libcxx/test/std/time/rep.h
@@ -33,8 +33,8 @@ 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
+// 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;
More information about the libcxx-commits
mailing list