[libc++][NFC] Centralize test for support of == and != in ranges #78481
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: Will Hawkins (hawkinsw) ChangesPreviously, tests for whether comparison using == was supported by iterators derived from ranges adaptors was spread throughout the testing codebase. This PR centralizes the implementation of those tests. Full diff: https://github.com/llvm/llvm-project/pull/78481.diff 10 Files Affected:
diff --git a/libcxx/test/std/ranges/range.adaptors/range.elements/iterator/compare.pass.cpp b/libcxx/test/std/ranges/range.adaptors/range.elements/iterator/compare.pass.cpp
index 16df3e40bd77a0..8d946d4f5d6e8b 100644
--- a/libcxx/test/std/ranges/range.adaptors/range.elements/iterator/compare.pass.cpp
+++ b/libcxx/test/std/ranges/range.adaptors/range.elements/iterator/compare.pass.cpp
@@ -27,6 +27,7 @@
#include <tuple>
#include "test_iterators.h"
+#include "test_range.h"
constexpr void compareOperatorTest(const auto& iter1, const auto& iter2) {
assert(!(iter1 < iter1));
@@ -139,8 +140,7 @@ constexpr bool test() {
auto it = ev.begin();
using ElemIter = decltype(it);
- static_assert(!std::invocable<std::equal_to<>, ElemIter, ElemIter>);
- static_assert(!std::invocable<std::not_equal_to<>, ElemIter, ElemIter>);
+ static_assert(!weakly_equality_comparable_with<ElemIter>);
inequalityOperatorsDoNotExistTest(it, it);
}
diff --git a/libcxx/test/std/ranges/range.adaptors/range.elements/sentinel/equality.pass.cpp b/libcxx/test/std/ranges/range.adaptors/range.elements/sentinel/equality.pass.cpp
index df95e07c97d972..e2ae7c6539c527 100644
--- a/libcxx/test/std/ranges/range.adaptors/range.elements/sentinel/equality.pass.cpp
+++ b/libcxx/test/std/ranges/range.adaptors/range.elements/sentinel/equality.pass.cpp
@@ -17,6 +17,7 @@
#include <ranges>
#include "../types.h"
+#include "test_range.h"
template <bool Const>
struct Iter {
@@ -63,36 +64,32 @@ struct Range : TupleBufferView {
using R = Range<Sent>;
using CrossComparableR = Range<CrossComparableSent>;
-// Test Constraint
-template <class I, class S>
-concept HasEqual = requires(const I i, const S s) { i == s; };
-
using std::ranges::elements_view;
using std::ranges::iterator_t;
using std::ranges::sentinel_t;
-static_assert(HasEqual<iterator_t<elements_view<R, 0>>, //
+static_assert(weakly_equality_comparable_with<iterator_t<elements_view<R, 0>>, //
sentinel_t<elements_view<R, 0>>>);
-static_assert(!HasEqual<iterator_t<const elements_view<R, 0>>, //
+static_assert(!weakly_equality_comparable_with<iterator_t<const elements_view<R, 0>>, //
sentinel_t<elements_view<R, 0>>>);
-static_assert(!HasEqual<iterator_t<elements_view<R, 0>>, //
+static_assert(!weakly_equality_comparable_with<iterator_t<elements_view<R, 0>>, //
sentinel_t<const elements_view<R, 0>>>);
-static_assert(HasEqual<iterator_t<const elements_view<R, 0>>, //
+static_assert(weakly_equality_comparable_with<iterator_t<const elements_view<R, 0>>, //
sentinel_t<const elements_view<R, 0>>>);
-static_assert(HasEqual<iterator_t<elements_view<R, 0>>, //
+static_assert(weakly_equality_comparable_with<iterator_t<elements_view<R, 0>>, //
sentinel_t<elements_view<R, 0>>>);
-static_assert(HasEqual<iterator_t<const elements_view<CrossComparableR, 0>>, //
+static_assert(weakly_equality_comparable_with<iterator_t<const elements_view<CrossComparableR, 0>>, //
sentinel_t<elements_view<CrossComparableR, 0>>>);
-static_assert(HasEqual<iterator_t<elements_view<CrossComparableR, 0>>, //
+static_assert(weakly_equality_comparable_with<iterator_t<elements_view<CrossComparableR, 0>>, //
sentinel_t<const elements_view<CrossComparableR, 0>>>);
-static_assert(HasEqual<iterator_t<const elements_view<CrossComparableR, 0>>, //
+static_assert(weakly_equality_comparable_with<iterator_t<const elements_view<CrossComparableR, 0>>, //
sentinel_t<const elements_view<CrossComparableR, 0>>>);
template <class R, bool ConstIter, bool ConstSent>
diff --git a/libcxx/test/std/ranges/range.adaptors/range.filter/iterator/compare.pass.cpp b/libcxx/test/std/ranges/range.adaptors/range.filter/iterator/compare.pass.cpp
index 78881e8ac6df19..9dec5d81ddc2ed 100644
--- a/libcxx/test/std/ranges/range.adaptors/range.filter/iterator/compare.pass.cpp
+++ b/libcxx/test/std/ranges/range.adaptors/range.filter/iterator/compare.pass.cpp
@@ -17,12 +17,13 @@
#include <cassert>
#include <concepts>
#include <utility>
+
#include "test_iterators.h"
#include "test_macros.h"
+#include "test_range.h"
+
#include "../types.h"
-template <class T>
-concept has_equal = requires (T const& x, T const& y) { { x == y }; };
template <class Iterator>
constexpr void test() {
@@ -76,7 +77,7 @@ constexpr bool tests() {
using Sentinel = sentinel_wrapper<Iterator>;
using FilterView = std::ranges::filter_view<minimal_view<Iterator, Sentinel>, AlwaysTrue>;
using FilterIterator = std::ranges::iterator_t<FilterView>;
- static_assert(!has_equal<FilterIterator>);
+ static_assert(!weakly_equality_comparable_with<FilterIterator>);
}
return true;
diff --git a/libcxx/test/std/ranges/range.adaptors/range.join/range.join.sentinel/eq.pass.cpp b/libcxx/test/std/ranges/range.adaptors/range.join/range.join.sentinel/eq.pass.cpp
index bc7d4bec94d3e6..823f112cbc956c 100644
--- a/libcxx/test/std/ranges/range.adaptors/range.join/range.join.sentinel/eq.pass.cpp
+++ b/libcxx/test/std/ranges/range.adaptors/range.join/range.join.sentinel/eq.pass.cpp
@@ -19,9 +19,7 @@
#include <type_traits>
#include "../types.h"
-
-template <class Iter, class Sent>
-concept EqualityComparable = std::invocable<std::equal_to<>, const Iter&, const Sent&> ;
+#include "test_range.h"
using Iterator = random_access_iterator<BufferView<int*>*>;
using ConstIterator = random_access_iterator<const BufferView<int*>*>;
@@ -53,9 +51,9 @@ struct ConstComparableView : BufferView<BufferView<int*>*> {
constexpr auto end() const { return ConstComparableSentinel<true>(ConstIterator(data_ + size_)); }
};
-static_assert(EqualityComparable<std::ranges::iterator_t<ConstComparableView>,
+static_assert(weakly_equality_comparable_with<std::ranges::iterator_t<ConstComparableView>,
std::ranges::sentinel_t<const ConstComparableView>>);
-static_assert(EqualityComparable<std::ranges::iterator_t<const ConstComparableView>,
+static_assert(weakly_equality_comparable_with<std::ranges::iterator_t<const ConstComparableView>,
std::ranges::sentinel_t<ConstComparableView>>);
constexpr bool test() {
diff --git a/libcxx/test/std/ranges/range.adaptors/range.lazy.split/range.lazy.split.inner/equal.pass.cpp b/libcxx/test/std/ranges/range.adaptors/range.lazy.split/range.lazy.split.inner/equal.pass.cpp
index 5a83a05ead9198..ae0fd164ae2dc0 100644
--- a/libcxx/test/std/ranges/range.adaptors/range.lazy.split/range.lazy.split.inner/equal.pass.cpp
+++ b/libcxx/test/std/ranges/range.adaptors/range.lazy.split/range.lazy.split.inner/equal.pass.cpp
@@ -17,13 +17,10 @@
#include <concepts>
#include <string_view>
+
#include "../types.h"
-template <class Iter>
-concept CanCallEquals = requires(const Iter& i) {
- i == i;
- i != i;
-};
+#include "test_range.h"
constexpr bool test() {
// When `View` is a forward range, `inner-iterator` supports both overloads of `operator==`.
@@ -56,7 +53,7 @@ constexpr bool test() {
auto b = val.begin();
std::same_as<std::default_sentinel_t> decltype(auto) e = val.end();
- static_assert(!CanCallEquals<decltype(b)>);
+ static_assert(!weakly_equality_comparable_with<decltype(b)>);
assert(!(b == std::default_sentinel));
assert(b != std::default_sentinel);
diff --git a/libcxx/test/std/ranges/range.adaptors/range.lazy.split/range.lazy.split.outer/equal.pass.cpp b/libcxx/test/std/ranges/range.adaptors/range.lazy.split/range.lazy.split.outer/equal.pass.cpp
index 49cac708947325..89193f566d528f 100644
--- a/libcxx/test/std/ranges/range.adaptors/range.lazy.split/range.lazy.split.outer/equal.pass.cpp
+++ b/libcxx/test/std/ranges/range.adaptors/range.lazy.split/range.lazy.split.outer/equal.pass.cpp
@@ -17,13 +17,10 @@
#include <concepts>
#include <string_view>
+
#include "../types.h"
-template <class Iter>
-concept CanCallEquals = requires(const Iter& i) {
- i == i;
- i != i;
-};
+#include "test_range.h"
constexpr bool test() {
// Forward range supports both overloads of `operator==`.
@@ -69,7 +66,7 @@ constexpr bool test() {
auto b = v.begin();
std::same_as<std::default_sentinel_t> decltype(auto) e = v.end();
- static_assert(!CanCallEquals<decltype(b)>);
+ static_assert(!weakly_equality_comparable_with<decltype(b)>);
assert(!(b == std::default_sentinel));
assert(b != std::default_sentinel);
diff --git a/libcxx/test/std/ranges/range.adaptors/range.take.while/sentinel/equality.pass.cpp b/libcxx/test/std/ranges/range.adaptors/range.take.while/sentinel/equality.pass.cpp
index db3e5764421af7..2533b5128cc9e6 100644
--- a/libcxx/test/std/ranges/range.adaptors/range.take.while/sentinel/equality.pass.cpp
+++ b/libcxx/test/std/ranges/range.adaptors/range.take.while/sentinel/equality.pass.cpp
@@ -70,36 +70,32 @@ struct LessThan3 {
constexpr bool operator()(int i) const { return i < 3; }
};
-// Test Constraint
-template <class I, class S>
-concept HasEqual = requires(const I i, const S s) { i == s; };
-
using std::ranges::iterator_t;
using std::ranges::sentinel_t;
using std::ranges::take_while_view;
-static_assert(HasEqual<iterator_t<take_while_view<R, LessThan3>>, //
+static_assert(weakly_equality_comparable_with<iterator_t<take_while_view<R, LessThan3>>, //
sentinel_t<take_while_view<R, LessThan3>>>);
-static_assert(!HasEqual<iterator_t<const take_while_view<R, LessThan3>>, //
+static_assert(!weakly_equality_comparable_with<iterator_t<const take_while_view<R, LessThan3>>, //
sentinel_t<take_while_view<R, LessThan3>>>);
-static_assert(!HasEqual<iterator_t<take_while_view<R, LessThan3>>, //
+static_assert(!weakly_equality_comparable_with<iterator_t<take_while_view<R, LessThan3>>, //
sentinel_t<const take_while_view<R, LessThan3>>>);
-static_assert(HasEqual<iterator_t<const take_while_view<R, LessThan3>>, //
+static_assert(weakly_equality_comparable_with<iterator_t<const take_while_view<R, LessThan3>>, //
sentinel_t<const take_while_view<R, LessThan3>>>);
-static_assert(HasEqual<iterator_t<take_while_view<R, LessThan3>>, //
+static_assert(weakly_equality_comparable_with<iterator_t<take_while_view<R, LessThan3>>, //
sentinel_t<take_while_view<R, LessThan3>>>);
-static_assert(HasEqual<iterator_t<const take_while_view<CrossComparableR, LessThan3>>, //
+static_assert(weakly_equality_comparable_with<iterator_t<const take_while_view<CrossComparableR, LessThan3>>, //
sentinel_t<take_while_view<CrossComparableR, LessThan3>>>);
-static_assert(HasEqual<iterator_t<take_while_view<CrossComparableR, LessThan3>>, //
+static_assert(weakly_equality_comparable_with<iterator_t<take_while_view<CrossComparableR, LessThan3>>, //
sentinel_t<const take_while_view<CrossComparableR, LessThan3>>>);
-static_assert(HasEqual<iterator_t<const take_while_view<CrossComparableR, LessThan3>>, //
+static_assert(weakly_equality_comparable_with<iterator_t<const take_while_view<CrossComparableR, LessThan3>>, //
sentinel_t<const take_while_view<CrossComparableR, LessThan3>>>);
template <class R, bool ConstIter, bool ConstSent>
diff --git a/libcxx/test/std/ranges/range.adaptors/range.zip/iterator/compare.pass.cpp b/libcxx/test/std/ranges/range.adaptors/range.zip/iterator/compare.pass.cpp
index fcbff722c39b3c..cf345300649836 100644
--- a/libcxx/test/std/ranges/range.adaptors/range.zip/iterator/compare.pass.cpp
+++ b/libcxx/test/std/ranges/range.adaptors/range.zip/iterator/compare.pass.cpp
@@ -26,6 +26,8 @@
#include <compare>
#include "test_iterators.h"
+#include "test_range.h"
+
#include "../types.h"
// This is for testing that zip iterator never calls underlying iterator's >, >=, <=, !=.
@@ -240,7 +242,7 @@ constexpr bool test() {
std::ranges::zip_view r(IterNoEqualView{buffer});
auto it = r.begin();
using Iter = decltype(it);
- static_assert(!std::invocable<std::equal_to<>, Iter, Iter>);
+ static_assert(!weakly_equality_comparable_with<Iter>);
inequalityOperatorsDoNotExistTest(it, it);
}
return true;
diff --git a/libcxx/test/std/ranges/range.adaptors/range.zip/sentinel/eq.pass.cpp b/libcxx/test/std/ranges/range.adaptors/range.zip/sentinel/eq.pass.cpp
index 5db73721108141..04542c3ae4d14d 100644
--- a/libcxx/test/std/ranges/range.adaptors/range.zip/sentinel/eq.pass.cpp
+++ b/libcxx/test/std/ranges/range.adaptors/range.zip/sentinel/eq.pass.cpp
@@ -18,6 +18,7 @@
#include <tuple>
#include "../types.h"
+#include "test_range.h"
using Iterator = random_access_iterator<int*>;
using ConstIterator = random_access_iterator<const int*>;
@@ -54,11 +55,6 @@ struct ConstIncompatibleView : std::ranges::view_base {
sentinel_wrapper<forward_iterator<const int*>> end() const;
};
-// clang-format off
-template <class Iter, class Sent>
-concept EqualComparable = std::invocable<std::equal_to<>, const Iter&, const Sent&>;
-// clang-format on
-
constexpr bool test() {
int buffer1[4] = {1, 2, 3, 4};
int buffer2[5] = {1, 2, 3, 4, 5};
@@ -95,10 +91,10 @@ constexpr bool test() {
using ConstSentinel = std::ranges::sentinel_t<const decltype(v)>;
static_assert(!std::is_same_v<Sentinel, ConstSentinel>);
- static_assert(EqualComparable<Iter, Sentinel>);
- static_assert(!EqualComparable<ConstIter, Sentinel>);
- static_assert(EqualComparable<Iter, ConstSentinel>);
- static_assert(EqualComparable<ConstIter, ConstSentinel>);
+ static_assert(weakly_equality_comparable_with<Iter, Sentinel>);
+ static_assert(!weakly_equality_comparable_with<ConstIter, Sentinel>);
+ static_assert(weakly_equality_comparable_with<Iter, ConstSentinel>);
+ static_assert(weakly_equality_comparable_with<ConstIter, ConstSentinel>);
}
{
@@ -120,10 +116,10 @@ constexpr bool test() {
using ConstSentinel = std::ranges::sentinel_t<const decltype(v)>;
static_assert(!std::is_same_v<Sentinel, ConstSentinel>);
- static_assert(EqualComparable<Iter, Sentinel>);
- static_assert(EqualComparable<ConstIter, Sentinel>);
- static_assert(EqualComparable<Iter, ConstSentinel>);
- static_assert(EqualComparable<ConstIter, ConstSentinel>);
+ static_assert(weakly_equality_comparable_with<Iter, Sentinel>);
+ static_assert(weakly_equality_comparable_with<ConstIter, Sentinel>);
+ static_assert(weakly_equality_comparable_with<Iter, ConstSentinel>);
+ static_assert(weakly_equality_comparable_with<ConstIter, ConstSentinel>);
}
{
@@ -139,10 +135,10 @@ constexpr bool test() {
using ConstSentinel = std::ranges::sentinel_t<const decltype(v)>;
static_assert(!std::is_same_v<Sentinel, ConstSentinel>);
- static_assert(EqualComparable<Iter, Sentinel>);
- static_assert(!EqualComparable<ConstIter, Sentinel>);
- static_assert(!EqualComparable<Iter, ConstSentinel>);
- static_assert(EqualComparable<ConstIter, ConstSentinel>);
+ static_assert(weakly_equality_comparable_with<Iter, Sentinel>);
+ static_assert(!weakly_equality_comparable_with<ConstIter, Sentinel>);
+ static_assert(!weakly_equality_comparable_with<Iter, ConstSentinel>);
+ static_assert(weakly_equality_comparable_with<ConstIter, ConstSentinel>);
}
return true;
}
diff --git a/libcxx/test/support/test_range.h b/libcxx/test/support/test_range.h
index 6061f710a26363..287ba982ea2a9b 100644
--- a/libcxx/test/support/test_range.h
+++ b/libcxx/test/support/test_range.h
@@ -10,9 +10,11 @@
#define LIBCXX_TEST_SUPPORT_TEST_RANGE_H
#include <concepts>
+#include <functional>
#include <iterator>
#include <ranges>
+#include "__concepts/invocable.h"
#include "test_iterators.h"
#if TEST_STD_VER < 17
@@ -89,4 +91,12 @@ concept simple_view =
std::same_as<std::ranges::iterator_t<Range>, std::ranges::iterator_t<const Range>> &&
std::same_as<std::ranges::sentinel_t<Range>, std::ranges::sentinel_t<const Range>>;
+template <class T, class U = T>
+concept weakly_equality_comparable_with = requires(const T& t, const U& u) {
+ { t == u } -> std::same_as<bool>;
+ { t != u } -> std::same_as<bool>;
+ { u == t } -> std::same_as<bool>;
+ { u != t } -> std::same_as<bool>;
+};
+
#endif // LIBCXX_TEST_SUPPORT_TEST_RANGE_H
|
|
I noticed that these tests were implemented differently in different places throughout the range adapter test code. I hope that this is something that is useful! |
|
✅ With the latest revision this PR passed the C/C++ code formatter. |
|
Hello @cjdb! Just let me know if there is anything I can do to make this better! |
|
Hope that this is helpful! Let me know if there is anything I can do to improve it! cc @var-const @cjdb |
|
I hope that this refactoring is useful! |
Please let me know if there is anything I can do to make this more useful. |
ldionne
reviewed
Mar 8, 2024
libcxx/test/support/test_range.h
Outdated
| @@ -89,4 +91,12 @@ concept simple_view = | |||
| std::same_as<std::ranges::iterator_t<Range>, std::ranges::iterator_t<const Range>> && | |||
| std::same_as<std::ranges::sentinel_t<Range>, std::ranges::sentinel_t<const Range>>; | |||
|
|
|||
| template <class T, class U = T> | |||
There was a problem hiding this comment.
I would remove the default template argument since weakly_equality_comparable_with<Iterator> is a bit confusing. I think it increases clarity if we force writing weakly_equality_comparable_with<Iterator, Iterator>.
There was a problem hiding this comment.
I believe that I addressed this with ee61f15.
libcxx/test/support/test_range.h
Outdated
| #include <iterator> | ||
| #include <ranges> | ||
|
|
||
| #include "__concepts/invocable.h" |
There was a problem hiding this comment.
We can't include private libc++ headers in the test suite since the test suite must work with other implementations too. Also I think this header is not necessary anyway.
There was a problem hiding this comment.
I believe that I addressed this with ee61f15.
var-const
requested changes
Mar 12, 2024
libcxx/test/support/test_range.h
Outdated
| @@ -89,4 +91,12 @@ concept simple_view = | |||
| std::same_as<std::ranges::iterator_t<Range>, std::ranges::iterator_t<const Range>> && | |||
| std::same_as<std::ranges::sentinel_t<Range>, std::ranges::sentinel_t<const Range>>; | |||
|
|
|||
| template <class T, class U = T> | |||
There was a problem hiding this comment.
Optional: perhaps add a comment like // [concept.equalitycomparable] to indicate that this concept comes from the Standard (where it's exposition-only so we can't use it directly)?
There was a problem hiding this comment.
I believe that I addressed this with ee61f15.
|
|
||
| static_assert(HasEqual<iterator_t<elements_view<R, 0>>, // | ||
| sentinel_t<elements_view<R, 0>>>); | ||
| static_assert(weakly_equality_comparable_with<iterator_t<elements_view<R, 0>>, // |
There was a problem hiding this comment.
Preexisting issue: unless I'm missing something, this repeats line 74 (71 after refactoring), R should probably be CrossComparableR.
There was a problem hiding this comment.
I concur. It looks like there is a pattern of four (4) tests for each of R and CrossComparableR (i.e.,
| left operand | right operand |
|---|---|
| type | type |
| const type | type |
| type | const type |
| const type | const type |
and there was a simple copy/paste error here. I have addressed it with the update!
Good eyes!
libcxx/test/std/ranges/range.adaptors/range.take.while/sentinel/equality.pass.cpp
Outdated
Show resolved
Hide resolved
hawkinsw
force-pushed
the
centralize_equal_comparison_test_range_adaptors
branch
from
72ad377 to
ee61f15
Compare
|
Hello! Just let me know if there is anything remaining that I can do to make this better! |
var-const
approved these changes
Mar 27, 2024
hawkinsw
force-pushed
the
centralize_equal_comparison_test_range_adaptors
branch
from
d5027f8 to
fad4d86
Compare
|
Because I had to rebase and fix a merge conflict, I just wanted to have a second pair of eyes on the updated PR before I squash/merge. Sorry for the additional bandwidth, @var-const ! |
var-const
approved these changes
Mar 31, 2024
hawkinsw
force-pushed
the
centralize_equal_comparison_test_range_adaptors
branch
from
fad4d86 to
d072b60
Compare
Previously, tests for whether comparison using == was supported by iterators derived from ranges adaptors was spread throughout the testing codebase. This PR centralizes the implementation of those tests.
Fix formatting issues.
Remove extraneous change from lit python.
Addressing final feedback and consolidating one final spot.
hawkinsw
force-pushed
the
centralize_equal_comparison_test_range_adaptors
branch
from
d072b60 to
95112df
Compare
|
If everyone is okay with this (especially @var-const ), I am more than willing to Squash and Merge. |
var-const
approved these changes
Apr 18, 2024
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.
Previously, tests for whether comparison using == was supported by iterators derived from ranges adaptors was spread throughout the testing codebase. This PR centralizes the implementation of those tests.