release/21.x: [libc++][ranges] Fix ranges::join_view segmented iterator trait (#158347)
#161008
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
…vm#158347) The outer iterator needs to move to the next segment when calling __compose. Without this change, `find_segment_if` would never reach the end of the join_view which caused erroneous result when calling `ranges::find` on a join_view of bidirectional ranges. Other specializations using the segmented iterator trait were likely to be affected as well. Fixes llvm#158279 Fixes llvm#93180 (cherry picked from commit d1b5607)
|
@frederick-vs-ja What do you think about merging this PR to the release branch? |
llvmbot
added
the
libc++
|
@llvm/pr-subscribers-libcxx Author: None (llvmbot) ChangesBackport d1b5607 Requested by: @frederick-vs-ja Full diff: https://github.com/llvm/llvm-project/pull/161008.diff 2 Files Affected:
diff --git a/libcxx/include/__ranges/join_view.h b/libcxx/include/__ranges/join_view.h
index 327b349f476a7..364f056d8d2cf 100644
--- a/libcxx/include/__ranges/join_view.h
+++ b/libcxx/include/__ranges/join_view.h
@@ -410,8 +410,13 @@ struct __segmented_iterator_traits<_JoinViewIterator> {
static constexpr _LIBCPP_HIDE_FROM_ABI _JoinViewIterator
__compose(__segment_iterator __seg_iter, __local_iterator __local_iter) {
- return _JoinViewIterator(
- std::move(__seg_iter).__get_data(), std::move(__seg_iter).__get_iter(), std::move(__local_iter));
+ auto&& __parent = std::move(__seg_iter).__get_data();
+ auto&& __outer = std::move(__seg_iter).__get_iter();
+ if (__local_iter == ranges::end(*__outer)) {
+ ++__outer;
+ return _JoinViewIterator(*__parent, __outer);
+ }
+ return _JoinViewIterator(__parent, __outer, std::move(__local_iter));
}
};
diff --git a/libcxx/test/std/algorithms/alg.nonmodifying/alg.find/ranges.find.pass.cpp b/libcxx/test/std/algorithms/alg.nonmodifying/alg.find/ranges.find.pass.cpp
index d7e6be9928a2d..5f730f0f5bba8 100644
--- a/libcxx/test/std/algorithms/alg.nonmodifying/alg.find/ranges.find.pass.cpp
+++ b/libcxx/test/std/algorithms/alg.nonmodifying/alg.find/ranges.find.pass.cpp
@@ -272,57 +272,100 @@ class Comparable {
friend bool operator==(const Comparable& lhs, long long rhs) { return comparable_data[lhs.index_] == rhs; }
};
-void test_deque() {
- { // empty deque
- std::deque<int> data;
- assert(std::ranges::find(data, 4) == data.end());
- assert(std::ranges::find(data.begin(), data.end(), 4) == data.end());
- }
-
- { // single element - match
- std::deque<int> data = {4};
- assert(std::ranges::find(data, 4) == data.begin());
- assert(std::ranges::find(data.begin(), data.end(), 4) == data.begin());
- }
-
- { // single element - no match
- std::deque<int> data = {3};
- assert(std::ranges::find(data, 4) == data.end());
- assert(std::ranges::find(data.begin(), data.end(), 4) == data.end());
- }
-
- // many elements
- for (auto size : {2, 3, 1023, 1024, 1025, 2047, 2048, 2049}) {
- { // last element match
+void test_segmented_iterator_types() {
+ // Test the optimized find algorithm for types that implement the segment iterator trait
+ // deque
+ {
+ { // empty deque
std::deque<int> data;
- data.resize(size);
- std::fill(data.begin(), data.end(), 3);
- data[size - 1] = 4;
- assert(std::ranges::find(data, 4) == data.end() - 1);
- assert(std::ranges::find(data.begin(), data.end(), 4) == data.end() - 1);
+ assert(std::ranges::find(data, 4) == data.end());
+ assert(std::ranges::find(data.begin(), data.end(), 4) == data.end());
}
- { // second-last element match
- std::deque<int> data;
- data.resize(size);
- std::fill(data.begin(), data.end(), 3);
- data[size - 2] = 4;
- assert(std::ranges::find(data, 4) == data.end() - 2);
- assert(std::ranges::find(data.begin(), data.end(), 4) == data.end() - 2);
+ { // single element - match
+ std::deque<int> data = {4};
+ assert(std::ranges::find(data, 4) == data.begin());
+ assert(std::ranges::find(data.begin(), data.end(), 4) == data.begin());
}
- { // no match
- std::deque<int> data;
- data.resize(size);
- std::fill(data.begin(), data.end(), 3);
+ { // single element - no match
+ std::deque<int> data = {3};
assert(std::ranges::find(data, 4) == data.end());
assert(std::ranges::find(data.begin(), data.end(), 4) == data.end());
}
+
+ // many elements
+ for (auto size : {2, 3, 1023, 1024, 1025, 2047, 2048, 2049}) {
+ { // last element match
+ std::deque<int> data;
+ data.resize(size);
+ std::fill(data.begin(), data.end(), 3);
+ data[size - 1] = 4;
+ assert(std::ranges::find(data, 4) == data.end() - 1);
+ assert(std::ranges::find(data.begin(), data.end(), 4) == data.end() - 1);
+ }
+
+ { // second-last element match
+ std::deque<int> data;
+ data.resize(size);
+ std::fill(data.begin(), data.end(), 3);
+ data[size - 2] = 4;
+ assert(std::ranges::find(data, 4) == data.end() - 2);
+ assert(std::ranges::find(data.begin(), data.end(), 4) == data.end() - 2);
+ }
+
+ { // no match
+ std::deque<int> data;
+ data.resize(size);
+ std::fill(data.begin(), data.end(), 3);
+ assert(std::ranges::find(data, 4) == data.end());
+ assert(std::ranges::find(data.begin(), data.end(), 4) == data.end());
+ }
+ }
+ }
+ // join_view ranges adaptor
+ {
+ { // single element - match
+ int data[1][1] = {{4}};
+ auto joined = std::views::join(data);
+ assert(std::ranges::find(joined, 4) == std::ranges::begin(joined));
+ }
+ { // single element - no match
+ // (reproducer for https://llvm.org/PR158279, where the iterator would never reach the end sentinel)
+ int data[1][1] = {{3}};
+ auto joined = std::views::join(data);
+ assert(std::ranges::find(joined, 4) == std::ranges::end(joined));
+ }
+ { // several sub-arrays of size 1 - match
+ int data[3][1] = {{0}, {4}, {0}};
+ auto joined = std::views::join(data);
+ assert(std::ranges::find(joined, 4) == std::next(std::ranges::begin(joined)));
+ }
+ { // several sub-arrays of size 2 - match in second element of an array
+ int data[3][2] = {{0, 0}, {0, 4}, {0, 0}};
+ auto joined = std::views::join(data);
+ assert(std::ranges::find(joined, 4) == std::ranges::next(std::ranges::begin(joined), 3));
+ }
+ { // vector of empty vectors
+ std::vector<std::vector<int>> data = {{}, {}};
+ auto joined = std::views::join(data);
+ assert(std::ranges::find(joined, 4) == std::ranges::end(joined));
+ }
+ { // vector of variably sized vectors - match
+ std::vector<std::vector<int>> data = {{}, {}, {3, 4}, {}, {}};
+ auto joined = std::views::join(data);
+ assert(std::ranges::find(joined, 4) == std::ranges::next(std::ranges::begin(joined)));
+ }
+ { // vector of variably sized vectors - no match
+ std::vector<std::vector<int>> data = {{}, {}, {3, 5}, {}, {}};
+ auto joined = std::views::join(data);
+ assert(std::ranges::find(joined, 4) == std::ranges::end(joined));
+ }
}
}
int main(int, char**) {
- test_deque();
+ test_segmented_iterator_types();
test();
static_assert(test());
|
|
|
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.
Backport d1b5607
Requested by: @frederick-vs-ja