Reland "[STLExtras] Add a template for detecting whether a type has an equality comparison operator"#176893
Conversation
|
@llvm/pr-subscribers-llvm-adt Author: Benjamin Stott (BStott6) ChangesIt seems there's a difference between Clang and MSVC's template handling, resulting in a difference in the behaviour of is_detected with private class members. This Godbolt link demonstrates the difference. This was causing one of the test cases I made, which was checking that it didn't pick up a private equality comparison operator, to fail when compiled using MSVC. Full diff: https://github.com/llvm/llvm-project/pull/176893.diff 2 Files Affected:
diff --git a/llvm/include/llvm/ADT/STLExtras.h b/llvm/include/llvm/ADT/STLExtras.h
index 23da931a63de1..d9c6b9ac5f1d7 100644
--- a/llvm/include/llvm/ADT/STLExtras.h
+++ b/llvm/include/llvm/ADT/STLExtras.h
@@ -2682,6 +2682,17 @@ template <typename T> using has_sizeof = decltype(sizeof(T));
template <typename T>
constexpr bool is_incomplete_v = !is_detected<detail::has_sizeof, T>::value;
+// Detect types with equality comparison operators.
+namespace detail {
+template <typename T, typename U>
+using has_equality_comparison =
+ decltype(std::declval<const T &>() == std::declval<const U &>());
+} // namespace detail
+
+/// Detects when type `const T` can be compared for equality with `const U`.
+template <typename T, typename U = T>
+constexpr bool has_equality_comparison_v =
+ is_detected<detail::has_equality_comparison, T, U>::value;
} // end namespace llvm
namespace std {
diff --git a/llvm/unittests/ADT/STLExtrasTest.cpp b/llvm/unittests/ADT/STLExtrasTest.cpp
index fe71945e4a794..af8d40575a22a 100644
--- a/llvm/unittests/ADT/STLExtrasTest.cpp
+++ b/llvm/unittests/ADT/STLExtrasTest.cpp
@@ -1777,8 +1777,35 @@ TEST(STLExtrasTest, ReverseConditionally) {
struct Foo;
struct Bar {};
-static_assert(is_incomplete_v<Foo>, "Foo is incomplete");
-static_assert(!is_incomplete_v<Bar>, "Bar is defined");
+TEST(STLExtrasTest, HasEqualityComparison) {
+ static_assert(is_incomplete_v<Foo>, "Foo is incomplete");
+ static_assert(!is_incomplete_v<Bar>, "Bar is defined");
+
+ struct NoEqualityComparison {};
+ static_assert(!has_equality_comparison_v<NoEqualityComparison>);
+
+ // Mutating equality comparison doesn't count.
+ struct MutatingEqualityComparison {
+ bool operator==(MutatingEqualityComparison &Other) { return false; }
+ };
+ static_assert(!has_equality_comparison_v<MutatingEqualityComparison>);
+
+ struct PublicEqualityComparison {
+ bool operator==(const PublicEqualityComparison &Other) const {
+ return false;
+ }
+ };
+ static_assert(has_equality_comparison_v<PublicEqualityComparison>);
+
+ struct StructA {};
+ struct StructB {
+ bool operator==(const StructA &Other) const { return false; }
+ };
+ static_assert(!has_equality_comparison_v<StructA, StructB>);
+ static_assert(has_equality_comparison_v<StructB, StructA>);
+
+ SUCCEED();
+}
TEST(STLExtrasTest, Search) {
// Test finding a subsequence in the middle.
|
…back out of that test function
|
Just a heads up: I'm getting a failure in one of your new tests: [llvm-project/llvm/unittests/ADT/STLExtrasTest.cpp:1804]:17: error: static assertion failed due to requirement '!has_equality_comparison_v<StructA, StructB>' |
|
Hmn, yes, and https://lab.llvm.org/buildbot/#/builders/108/builds/22080 is failing as well. Will revert. |
|
(Thanks for the heads up)! |
…pe has an equality comparison operator" (#176893)" This reverts commit 2984a28. Another buildbot is unhappy with another static assert: https://lab.llvm.org/buildbot/#/builders/108/builds/22080
|
I think it's because of the "rewritten candidates" rules added in C++20 https://en.cppreference.com/w/cpp/language/overload_resolution.html#Call_to_an_overloaded_operator.
I wasn't aware of this change when writing the test cases. |
IMO we should drop these excessive testcases, I don't think they provide much value anyway -- I'd expect the real-world usage to be much less ambiguous. |
…s an equality comparison operator" (#177415) Reland PR #176893 which was an attempt to reland PR #176429. Fix: There was a test case asserting that for types `StructA` `StructB` where `operator==(const StructB &, const StructA &)` is defined, `has_equality_comparison_v<StructA, StructB>` is false because the arguments are the wrong way around. However, in C++20, operator overload resolution was changed so that for reflexive comparison operators `==` and `!=` if a candidate exists with the arguments swapped then this will be used. This means the test case failed when compiled with C++20. That check was simply removed in this version.
…e has an equality comparison operator" (llvm#177415) Reland PR llvm#176893 which was an attempt to reland PR llvm#176429. Fix: There was a test case asserting that for types `StructA` `StructB` where `operator==(const StructB &, const StructA &)` is defined, `has_equality_comparison_v<StructA, StructB>` is false because the arguments are the wrong way around. However, in C++20, operator overload resolution was changed so that for reflexive comparison operators `==` and `!=` if a candidate exists with the arguments swapped then this will be used. This means the test case failed when compiled with C++20. That check was simply removed in this version.
…e has an equality comparison operator" (llvm#177415) Reland PR llvm#176893 which was an attempt to reland PR llvm#176429. Fix: There was a test case asserting that for types `StructA` `StructB` where `operator==(const StructB &, const StructA &)` is defined, `has_equality_comparison_v<StructA, StructB>` is false because the arguments are the wrong way around. However, in C++20, operator overload resolution was changed so that for reflexive comparison operators `==` and `!=` if a candidate exists with the arguments swapped then this will be used. This means the test case failed when compiled with C++20. That check was simply removed in this version.
This PR is an attempt to reland #176429.
It seems there's a difference between Clang and MSVC's template handling, resulting in a difference in the behaviour of is_detected with private class members. This Godbolt link demonstrates the difference. This was causing one of the test cases I made, which was checking that it didn't pick up a private equality comparison operator, to fail when compiled using MSVC.
In this PR I have just removed that test case.