[libc++][chrono] Implements duration Rep constraints. #80539
Merged
Conversation
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
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
llvmbot
added
the
libc++
|
@llvm/pr-subscribers-libcxx Author: Mark de Wever (mordante) ChangesApplies LWG3050 to the constraints of operator*, operator/, and operator%. The changes to the constructor were done in Implements:
Full diff: https://github.com/llvm/llvm-project/pull/80539.diff 7 Files Affected:
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;
}
|
h-vetinari
reviewed
Feb 4, 2024
Co-authored-by: h-vetinari <[email protected]>
|
While I'm flattered, I don't think a typo fix is worth a |
|
That is indeed automatically added when accepting a suggestion. |
|
I know, but at least it's possible to edit the commit message of the squash merge. ;) |
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
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: