[libc++] P2167R3: Improved Proposed Wording for LWG 2114 #109102
Merged
+191
−32
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
llvmbot
added
the
libc++
|
@llvm/pr-subscribers-libcxx Author: A. Jiang (frederick-vs-ja) ChangesOnly the [cmp.alg] part (for I strongly feel that P2167R3 should be a DR despite that it is not a DR officially: CPOs -> C++20; remain parts -> C++98/11 (except that Note that P2167R3 damaged the resolution of LWG3465: the type of Drive-by change:
Closes #105241. Full diff: https://github.com/llvm/llvm-project/pull/109102.diff 7 Files Affected:
diff --git a/libcxx/docs/Status/Cxx23Papers.csv b/libcxx/docs/Status/Cxx23Papers.csv
index 2c8a91d8401b53..21c4bfe7e4a244 100644
--- a/libcxx/docs/Status/Cxx23Papers.csv
+++ b/libcxx/docs/Status/Cxx23Papers.csv
@@ -96,7 +96,7 @@
"`P1202R5 <https://wg21.link/P1202R5>`__","Asymmetric Fences","2022-11 (Kona)","","",""
"`P1264R2 <https://wg21.link/P1264R2>`__","Revising the wording of ``stream`` input operations","2022-11 (Kona)","|Complete|","9.0",""
"`P1478R8 <https://wg21.link/P1478R8>`__","``Byte-wise`` ``atomic`` ``memcpy``","2022-11 (Kona)","","",""
-"`P2167R3 <https://wg21.link/P2167R3>`__","Improved Proposed Wording for LWG 2114","2022-11 (Kona)","","",""
+"`P2167R3 <https://wg21.link/P2167R3>`__","Improved Proposed Wording for LWG 2114","2022-11 (Kona)","|Complete|","20.0","The `[cmp.alg] <https://eel.is/c++draft/cmp.alg>`__ part is implemented as a DR against C++20. MSVC STL does the same. Other parts are Nothing To Do."
"`P2396R1 <https://wg21.link/P2396R1>`__","Concurrency TS 2 fixes ","2022-11 (Kona)","","",""
"`P2505R5 <https://wg21.link/P2505R5>`__","Monadic Functions for ``std::expected``","2022-11 (Kona)","|Complete|","17.0",""
"`P2539R4 <https://wg21.link/P2539R4>`__","Should the output of ``std::print`` to a terminal be synchronized with the underlying stream?","2022-11 (Kona)","|Complete|","18.0",""
diff --git a/libcxx/include/__compare/compare_partial_order_fallback.h b/libcxx/include/__compare/compare_partial_order_fallback.h
index e0efa3ccb88db7..fbbd3c3d4ba259 100644
--- a/libcxx/include/__compare/compare_partial_order_fallback.h
+++ b/libcxx/include/__compare/compare_partial_order_fallback.h
@@ -11,6 +11,7 @@
#include <__compare/ordering.h>
#include <__compare/partial_order.h>
+#include <__concepts/boolean_testable.h>
#include <__config>
#include <__type_traits/decay.h>
#include <__type_traits/is_same.h>
@@ -37,18 +38,15 @@ struct __fn {
}
template <class _Tp, class _Up>
- requires is_same_v<decay_t<_Tp>, decay_t<_Up>>
- _LIBCPP_HIDE_FROM_ABI static constexpr auto __go(_Tp&& __t, _Up&& __u, __priority_tag<0>) noexcept(noexcept(
- std::forward<_Tp>(__t) == std::forward<_Up>(__u) ? partial_ordering::equivalent
- : std::forward<_Tp>(__t) < std::forward<_Up>(__u) ? partial_ordering::less
- : std::forward<_Up>(__u) < std::forward<_Tp>(__t)
- ? partial_ordering::greater
- : partial_ordering::unordered))
- -> decltype(std::forward<_Tp>(__t) == std::forward<_Up>(__u) ? partial_ordering::equivalent
- : std::forward<_Tp>(__t) < std::forward<_Up>(__u) ? partial_ordering::less
- : std::forward<_Up>(__u) < std::forward<_Tp>(__t)
- ? partial_ordering::greater
- : partial_ordering::unordered) {
+ requires is_same_v<decay_t<_Tp>, decay_t<_Up>> &&
+ __boolean_testable<decltype(std::declval<_Tp>() == std::declval<_Up>())> &&
+ __boolean_testable<decltype(std::declval<_Tp>() < std::declval<_Up>())> &&
+ __boolean_testable<decltype(std::declval<_Up>() < std::declval<_Tp>())>
+ _LIBCPP_HIDE_FROM_ABI static constexpr partial_ordering __go(_Tp&& __t, _Up&& __u, __priority_tag<0>) noexcept(
+ noexcept(std::forward<_Tp>(__t) == std::forward<_Up>(__u) ? partial_ordering::equivalent
+ : std::forward<_Tp>(__t) < std::forward<_Up>(__u) ? partial_ordering::less
+ : std::forward<_Up>(__u) < std::forward<_Tp>(__t) ? partial_ordering::greater
+ : partial_ordering::unordered)) {
return std::forward<_Tp>(__t) == std::forward<_Up>(__u) ? partial_ordering::equivalent
: std::forward<_Tp>(__t) < std::forward<_Up>(__u) ? partial_ordering::less
: std::forward<_Up>(__u) < std::forward<_Tp>(__t)
diff --git a/libcxx/include/__compare/compare_strong_order_fallback.h b/libcxx/include/__compare/compare_strong_order_fallback.h
index a94d517ed30fce..4a4155cf9aacc1 100644
--- a/libcxx/include/__compare/compare_strong_order_fallback.h
+++ b/libcxx/include/__compare/compare_strong_order_fallback.h
@@ -11,6 +11,7 @@
#include <__compare/ordering.h>
#include <__compare/strong_order.h>
+#include <__concepts/boolean_testable.h>
#include <__config>
#include <__type_traits/decay.h>
#include <__type_traits/is_same.h>
@@ -37,16 +38,13 @@ struct __fn {
}
template <class _Tp, class _Up>
- requires is_same_v<decay_t<_Tp>, decay_t<_Up>>
- _LIBCPP_HIDE_FROM_ABI static constexpr auto __go(_Tp&& __t, _Up&& __u, __priority_tag<0>) noexcept(noexcept(
- std::forward<_Tp>(__t) == std::forward<_Up>(__u) ? strong_ordering::equal
- : std::forward<_Tp>(__t) < std::forward<_Up>(__u)
- ? strong_ordering::less
- : strong_ordering::greater))
- -> decltype(std::forward<_Tp>(__t) == std::forward<_Up>(__u) ? strong_ordering::equal
- : std::forward<_Tp>(__t) < std::forward<_Up>(__u)
- ? strong_ordering::less
- : strong_ordering::greater) {
+ requires is_same_v<decay_t<_Tp>, decay_t<_Up>> &&
+ __boolean_testable<decltype(std::declval<_Tp>() == std::declval<_Up>())> &&
+ __boolean_testable<decltype(std::declval<_Tp>() < std::declval<_Up>())>
+ _LIBCPP_HIDE_FROM_ABI static constexpr strong_ordering __go(_Tp&& __t, _Up&& __u, __priority_tag<0>) noexcept(
+ noexcept(std::forward<_Tp>(__t) == std::forward<_Up>(__u) ? strong_ordering::equal
+ : std::forward<_Tp>(__t) < std::forward<_Up>(__u) ? strong_ordering::less
+ : strong_ordering::greater)) {
return std::forward<_Tp>(__t) == std::forward<_Up>(__u) ? strong_ordering::equal
: std::forward<_Tp>(__t) < std::forward<_Up>(__u)
? strong_ordering::less
diff --git a/libcxx/include/__compare/compare_weak_order_fallback.h b/libcxx/include/__compare/compare_weak_order_fallback.h
index 062b7b582cd7eb..48c6dab7ca3769 100644
--- a/libcxx/include/__compare/compare_weak_order_fallback.h
+++ b/libcxx/include/__compare/compare_weak_order_fallback.h
@@ -11,6 +11,7 @@
#include <__compare/ordering.h>
#include <__compare/weak_order.h>
+#include <__concepts/boolean_testable.h>
#include <__config>
#include <__type_traits/decay.h>
#include <__type_traits/is_same.h>
@@ -37,16 +38,14 @@ struct __fn {
}
template <class _Tp, class _Up>
- requires is_same_v<decay_t<_Tp>, decay_t<_Up>>
- _LIBCPP_HIDE_FROM_ABI static constexpr auto __go(_Tp&& __t, _Up&& __u, __priority_tag<0>) noexcept(noexcept(
+ requires is_same_v<decay_t<_Tp>, decay_t<_Up>> &&
+ __boolean_testable<decltype(std::declval<_Tp>() == std::declval<_Up>())> &&
+ __boolean_testable<decltype(std::declval<_Tp>() < std::declval<_Up>())>
+ _LIBCPP_HIDE_FROM_ABI static constexpr weak_ordering __go(_Tp&& __t, _Up&& __u, __priority_tag<0>) noexcept(noexcept(
std::forward<_Tp>(__t) == std::forward<_Up>(__u) ? weak_ordering::equivalent
: std::forward<_Tp>(__t) < std::forward<_Up>(__u)
? weak_ordering::less
- : weak_ordering::greater))
- -> decltype(std::forward<_Tp>(__t) == std::forward<_Up>(__u) ? weak_ordering::equivalent
- : std::forward<_Tp>(__t) < std::forward<_Up>(__u)
- ? weak_ordering::less
- : weak_ordering::greater) {
+ : weak_ordering::greater)) {
return std::forward<_Tp>(__t) == std::forward<_Up>(__u) ? weak_ordering::equivalent
: std::forward<_Tp>(__t) < std::forward<_Up>(__u)
? weak_ordering::less
diff --git a/libcxx/test/std/language.support/cmp/cmp.alg/compare_partial_order_fallback.pass.cpp b/libcxx/test/std/language.support/cmp/cmp.alg/compare_partial_order_fallback.pass.cpp
index a057a34136e0f5..f3fdd2ecb04ced 100644
--- a/libcxx/test/std/language.support/cmp/cmp.alg/compare_partial_order_fallback.pass.cpp
+++ b/libcxx/test/std/language.support/cmp/cmp.alg/compare_partial_order_fallback.pass.cpp
@@ -267,6 +267,44 @@ namespace N2 {
friend bool operator<(const VC2&, VC2&);
friend bool operator<(VC2&, const VC2&);
};
+
+ enum class comparison_result_kind : bool {
+ convertible_bool,
+ boolean_testable,
+ };
+
+ template <comparison_result_kind K>
+ struct comparison_result {
+ bool value;
+
+ constexpr operator bool() const noexcept { return value; }
+
+ constexpr auto operator!() const noexcept {
+ if constexpr (K == comparison_result_kind::boolean_testable) {
+ return comparison_result{!value};
+ }
+ }
+ };
+
+ template <comparison_result_kind EqKind, comparison_result_kind LeKind>
+ struct boolean_tested_type {
+ friend constexpr comparison_result<EqKind> operator==(boolean_tested_type, boolean_tested_type) noexcept {
+ return comparison_result<EqKind>{true};
+ }
+
+ friend constexpr comparison_result<LeKind> operator<(boolean_tested_type, boolean_tested_type) noexcept {
+ return comparison_result<LeKind>{false};
+ }
+ };
+
+ using test_only_convertible =
+ boolean_tested_type<comparison_result_kind::convertible_bool, comparison_result_kind::convertible_bool>;
+ using test_eq_boolean_testable =
+ boolean_tested_type<comparison_result_kind::boolean_testable, comparison_result_kind::convertible_bool>;
+ using test_le_boolean_testable =
+ boolean_tested_type<comparison_result_kind::convertible_bool, comparison_result_kind::boolean_testable>;
+ using test_boolean_testable =
+ boolean_tested_type<comparison_result_kind::boolean_testable, comparison_result_kind::boolean_testable>;
}
constexpr bool test_2()
@@ -306,6 +344,21 @@ constexpr bool test_2()
assert( has_partial_order(cvc, vc));
assert(!has_partial_order(vc, cvc));
}
+ {
+ // P2167R3 as modified by the intent of LWG3465:
+ // All of decltype(e == f), decltype(e < f), and decltype(f < e) need to be well-formed and boolean-testable.
+ N2::test_only_convertible tc;
+ N2::test_eq_boolean_testable teq;
+ N2::test_le_boolean_testable tle;
+ N2::test_boolean_testable tbt;
+
+ assert(!has_partial_order(tc, tc));
+ assert(!has_partial_order(teq, teq));
+ assert(!has_partial_order(tle, tle));
+ assert(has_partial_order(tbt, tbt));
+
+ assert(std::compare_partial_order_fallback(tbt, tbt) == std::partial_ordering::equivalent);
+ }
return true;
}
diff --git a/libcxx/test/std/language.support/cmp/cmp.alg/compare_strong_order_fallback.pass.cpp b/libcxx/test/std/language.support/cmp/cmp.alg/compare_strong_order_fallback.pass.cpp
index 2ad05cb1ec6dd3..e1e7bc37b1d799 100644
--- a/libcxx/test/std/language.support/cmp/cmp.alg/compare_strong_order_fallback.pass.cpp
+++ b/libcxx/test/std/language.support/cmp/cmp.alg/compare_strong_order_fallback.pass.cpp
@@ -473,6 +473,44 @@ namespace N2 {
friend bool operator<(const VC2&, VC2&);
friend bool operator<(VC2&, const VC2&);
};
+
+ enum class comparison_result_kind : bool {
+ convertible_bool,
+ boolean_testable,
+ };
+
+ template <comparison_result_kind K>
+ struct comparison_result {
+ bool value;
+
+ constexpr operator bool() const noexcept { return value; }
+
+ constexpr auto operator!() const noexcept {
+ if constexpr (K == comparison_result_kind::boolean_testable) {
+ return comparison_result{!value};
+ }
+ }
+ };
+
+ template <comparison_result_kind EqKind, comparison_result_kind LeKind>
+ struct boolean_tested_type {
+ friend constexpr comparison_result<EqKind> operator==(boolean_tested_type, boolean_tested_type) noexcept {
+ return comparison_result<EqKind>{true};
+ }
+
+ friend constexpr comparison_result<LeKind> operator<(boolean_tested_type, boolean_tested_type) noexcept {
+ return comparison_result<LeKind>{false};
+ }
+ };
+
+ using test_only_convertible =
+ boolean_tested_type<comparison_result_kind::convertible_bool, comparison_result_kind::convertible_bool>;
+ using test_eq_boolean_testable =
+ boolean_tested_type<comparison_result_kind::boolean_testable, comparison_result_kind::convertible_bool>;
+ using test_le_boolean_testable =
+ boolean_tested_type<comparison_result_kind::convertible_bool, comparison_result_kind::boolean_testable>;
+ using test_boolean_testable =
+ boolean_tested_type<comparison_result_kind::boolean_testable, comparison_result_kind::boolean_testable>;
}
constexpr bool test_2()
@@ -506,6 +544,20 @@ constexpr bool test_2()
assert( has_strong_order(cvc, vc));
assert(!has_strong_order(vc, cvc));
}
+ {
+ // P2167R3: Both decltype(e == f) and decltype(e < f) need to be well-formed and boolean-testable.
+ N2::test_only_convertible tc;
+ N2::test_eq_boolean_testable teq;
+ N2::test_le_boolean_testable tle;
+ N2::test_boolean_testable tbt;
+
+ assert(!has_strong_order(tc, tc));
+ assert(!has_strong_order(teq, teq));
+ assert(!has_strong_order(tle, tle));
+ assert(has_strong_order(tbt, tbt));
+
+ assert(std::compare_strong_order_fallback(tbt, tbt) == std::strong_ordering::equal);
+ }
return true;
}
@@ -515,13 +567,17 @@ int main(int, char**)
test_1_2();
test_1_3<float>();
test_1_3<double>();
- // test_1_3<long double>(); // UNIMPLEMENTED
+#ifdef TEST_LONG_DOUBLE_IS_DOUBLE
+ test_1_3<long double>(); // UNIMPLEMENTED when long double is a distinct type
+#endif
test_1_4();
test_2();
static_assert(test_1_3<float>());
static_assert(test_1_3<double>());
- // static_assert(test_1_3<long double>()); // UNIMPLEMENTED
+#ifdef TEST_LONG_DOUBLE_IS_DOUBLE
+ static_assert(test_1_3<long double>()); // UNIMPLEMENTED when long double is a distinct type
+#endif
static_assert(test_1_4());
static_assert(test_2());
diff --git a/libcxx/test/std/language.support/cmp/cmp.alg/compare_weak_order_fallback.pass.cpp b/libcxx/test/std/language.support/cmp/cmp.alg/compare_weak_order_fallback.pass.cpp
index 65674b4006fc89..07b5fcd0a6d178 100644
--- a/libcxx/test/std/language.support/cmp/cmp.alg/compare_weak_order_fallback.pass.cpp
+++ b/libcxx/test/std/language.support/cmp/cmp.alg/compare_weak_order_fallback.pass.cpp
@@ -520,6 +520,44 @@ namespace N2 {
friend bool operator<(const VC2&, VC2&);
friend bool operator<(VC2&, const VC2&);
};
+
+ enum class comparison_result_kind : bool {
+ convertible_bool,
+ boolean_testable,
+ };
+
+ template <comparison_result_kind K>
+ struct comparison_result {
+ bool value;
+
+ constexpr operator bool() const noexcept { return value; }
+
+ constexpr auto operator!() const noexcept {
+ if constexpr (K == comparison_result_kind::boolean_testable) {
+ return comparison_result{!value};
+ }
+ }
+ };
+
+ template <comparison_result_kind EqKind, comparison_result_kind LeKind>
+ struct boolean_tested_type {
+ friend constexpr comparison_result<EqKind> operator==(boolean_tested_type, boolean_tested_type) noexcept {
+ return comparison_result<EqKind>{true};
+ }
+
+ friend constexpr comparison_result<LeKind> operator<(boolean_tested_type, boolean_tested_type) noexcept {
+ return comparison_result<LeKind>{false};
+ }
+ };
+
+ using test_only_convertible =
+ boolean_tested_type<comparison_result_kind::convertible_bool, comparison_result_kind::convertible_bool>;
+ using test_eq_boolean_testable =
+ boolean_tested_type<comparison_result_kind::boolean_testable, comparison_result_kind::convertible_bool>;
+ using test_le_boolean_testable =
+ boolean_tested_type<comparison_result_kind::convertible_bool, comparison_result_kind::boolean_testable>;
+ using test_boolean_testable =
+ boolean_tested_type<comparison_result_kind::boolean_testable, comparison_result_kind::boolean_testable>;
}
constexpr bool test_2()
@@ -553,6 +591,20 @@ constexpr bool test_2()
assert( has_weak_order(cvc, vc));
assert(!has_weak_order(vc, cvc));
}
+ {
+ // P2167R3: Both decltype(e == f) and decltype(e < f) need to be well-formed and boolean-testable.
+ N2::test_only_convertible tc;
+ N2::test_eq_boolean_testable teq;
+ N2::test_le_boolean_testable tle;
+ N2::test_boolean_testable tbt;
+
+ assert(!has_weak_order(tc, tc));
+ assert(!has_weak_order(teq, teq));
+ assert(!has_weak_order(tle, tle));
+ assert(has_weak_order(tbt, tbt));
+
+ assert(std::compare_weak_order_fallback(tbt, tbt) == std::weak_ordering::equivalent);
+ }
return true;
}
|
frederick-vs-ja
force-pushed
the
p2167r3
branch
2 times, most recently
from
dac24da to
8e12487
Compare
ldionne
reviewed
Sep 19, 2024
There was a problem hiding this comment.
Thanks for the patch! I have a few questions but I think I'm fine with everything in the patch, once the questions have been answered.
I strongly feel that P2167R3 should be a DR despite that it is not a DR officially: CPOs -> C++20; remain parts -> C++98/11 (except that boolean-testable should be transformed into the original BooleanTestable requirements in the old resolution of LWG2114).
I didn't see any changes related to CPOs in the paper (which I only skimmed). Did I miss something?
Note that P2167R3 damaged the resolution of LWG3465: the type of F < E was left underconstrained. I've tried to submit an LWG issue for this.
I don't think I see test changes reflecting that. Did we have proper coverage for LWG3465?
The paper changed
Yes, the test coverage for LWG3465 is here - although it didn't cover convertibility to I think it's OK to just keep the existing test cases above. Perhaps the test coverage should be extended... |
frederick-vs-ja
force-pushed
the
p2167r3
branch
from
8e12487 to
3220ecd
Compare
Only the [cmp.alg] part (for `comparison_meow_fallback` CPOs) in the paper required changes. Other parts merely fixed preconditions of some standard library functions. I strongly feel that P2167R3 should be a DR despite that it is not a DR officially: CPOs -> C++20; remain parts -> C++98/11 (except that _`boolean-testable`_ should be transformed into the original BooleanTestable requirements in the old resolution of LWG2114). Note that P2167R3 damaged the resolution of LWG3465: the type of `F < E` was left underconstrained. I've tried to submit an LWG issue for this. Drive-by change: - enable some test coverages in `compare_strong_order_fallback.pass.cpp` when `TEST_LONG_DOUBLE_IS_DOUBLE`
frederick-vs-ja
force-pushed
the
p2167r3
branch
from
3220ecd to
fb097c4
Compare
ldionne
approved these changes
Oct 1, 2024
Sterling-Augustine
pushed a commit
to Sterling-Augustine/llvm-project
that referenced
this pull request
Oct 3, 2024
Only the [cmp.alg] part (for `comparison_meow_fallback` CPOs) in the paper required changes. Other parts merely fixed preconditions of some standard library functions. I strongly feel that P2167R3 should be a DR despite that it is not a DR officially: CPOs -> C++20; remain parts -> C++98/11 (except that _`boolean-testable`_ should be transformed into the original _BooleanTestable_ requirements in the old resolution of LWG2114). Note that P2167R3 damaged the resolution of LWG3465: the type of `F < E` was left underconstrained. I've tried to submit an LWG issue for this, which is now LWG4157. Drive-by change: - enable some test coverages in `compare_strong_order_fallback.pass.cpp` when `TEST_LONG_DOUBLE_IS_DOUBLE`, following up llvm#106742 Closes llvm#105241.
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.
Only the [cmp.alg] part (for
comparison_meow_fallbackCPOs) in the paper required changes. Other parts merely fixed preconditions of some standard library functions.I strongly feel that P2167R3 should be a DR despite that it is not a DR officially: CPOs -> C++20; remain parts -> C++98/11 (except that
boolean-testableshould be transformed into the original BooleanTestable requirements in the old resolution of LWG2114).Note that P2167R3 damaged the resolution of LWG3465: the type of
F < Ewas left underconstrained. I've tried to submit an LWG issue for this, which is now LWG4157.Drive-by change:
compare_strong_order_fallback.pass.cppwhenTEST_LONG_DOUBLE_IS_DOUBLE, following up [libcxx][test] Use long double test macro in strong_order.pass.cpp #106742Closes #105241.