[libc++] Fix copy-paste damage in ranges::rotate_copy and its test#74544
[libc++] Fix copy-paste damage in ranges::rotate_copy and its test#74544StephanTLavavej merged 2 commits intollvm:mainfrom
ranges::rotate_copy and its test#74544Conversation
|
@llvm/pr-subscribers-libcxx Author: Stephan T. Lavavej (StephanTLavavej) ChangesFound while running libc++'s tests with MSVC's STL.
I fixed this by replacing the assertions with the test types that aren't quite forward iterators/ranges. Additionally, I noticed that the top-level Note: The fact that this test was passing for you is quite curious. From a quick glance, I don't see a problem with the libc++ product code constraints, so I'm not sure where the behavior difference is. Full diff: https://github.com/llvm/llvm-project/pull/74544.diff 1 Files Affected:
diff --git a/libcxx/test/std/algorithms/alg.modifying.operations/alg.rotate/ranges.rotate_copy.pass.cpp b/libcxx/test/std/algorithms/alg.modifying.operations/alg.rotate/ranges.rotate_copy.pass.cpp
index 1f18d787c2f4c..58b0f75d9b5f2 100644
--- a/libcxx/test/std/algorithms/alg.modifying.operations/alg.rotate/ranges.rotate_copy.pass.cpp
+++ b/libcxx/test/std/algorithms/alg.modifying.operations/alg.rotate/ranges.rotate_copy.pass.cpp
@@ -34,16 +34,16 @@ template <class Range, class Out = int*>
concept HasRotateCopyR = requires(Range range, Out out) { std::ranges::rotate_copy(range, nullptr, out); };
static_assert(HasRotateCopyIt<int*>);
-static_assert(!HasRotateCopyIt<BidirectionalIteratorNotDerivedFrom>);
-static_assert(!HasRotateCopyIt<BidirectionalIteratorNotDecrementable>);
+static_assert(!HasRotateCopyIt<ForwardIteratorNotDerivedFrom>);
+static_assert(!HasRotateCopyIt<ForwardIteratorNotIncrementable>);
static_assert(!HasRotateCopyIt<int*, SentinelForNotSemiregular>);
static_assert(!HasRotateCopyIt<int*, SentinelForNotWeaklyEqualityComparableWith>);
static_assert(!HasRotateCopyIt<int*, OutputIteratorNotIndirectlyWritable>);
static_assert(!HasRotateCopyIt<int*, OutputIteratorNotInputOrOutputIterator>);
static_assert(HasRotateCopyR<UncheckedRange<int*>>);
-static_assert(!HasRotateCopyR<BidirectionalRangeNotDerivedFrom>);
-static_assert(!HasRotateCopyR<BidirectionalRangeNotDecrementable>);
+static_assert(!HasRotateCopyR<ForwardRangeNotDerivedFrom>);
+static_assert(!HasRotateCopyR<ForwardRangeNotIncrementable>);
static_assert(!HasRotateCopyR<UncheckedRange<int*, SentinelForNotSemiregular>>);
static_assert(!HasRotateCopyR<UncheckedRange<int*>, OutputIteratorNotIndirectlyWritable>);
static_assert(!HasRotateCopyR<UncheckedRange<int*>, OutputIteratorNotInputOrOutputIterator>);
@@ -112,6 +112,7 @@ constexpr void test_out_iterators() {
}
constexpr bool test() {
+ test_out_iterators<forward_iterator<int*>>();
test_out_iterators<bidirectional_iterator<int*>>();
test_out_iterators<random_access_iterator<int*>>();
test_out_iterators<contiguous_iterator<int*>>();
|
Try restoring the original |
|
Good idea - which I didn't even need to try, as extending the test coverage with llvm-project/libcxx/include/__algorithm/ranges_rotate_copy.h Lines 35 to 56 in 06c5c27 Let's see if I can fix this despite not being tooled up to work on libc++ product code. |
In addition to taking `forward_iterator` and `forward_range` as depicted in the Standard, this drops the inclusion of `<__iterator/reverse_iterator.h>` as this algorithm doesn't need `std::__reverse_range`.
ranges.rotate_copy.pass.cppranges::rotate_copy and its test
philnik777
left a comment
There was a problem hiding this comment.
Thanks for the fix! LGTM with green CI.
|
Thanks, CI is green with only one spurious build failure on Apple. Merging now. |
Found while running libc++'s tests with MSVC's STL.
ranges::rotate_copytakesforward_iterators as this test's comment banner correctly depicts. However, this test had bogus assertions expecting thatranges::rotate_copywould be constrained away for not-quite-bidi iterators. @philnik777 confirmed that these were copy-paste relics from theranges::reverse_copytest.I fixed this by replacing the assertions with the test types that aren't quite forward iterators/ranges. Additionally, I noticed that the top-level
test()function was missing coverage with the weakest possibleforward_iterator<int*>.This revealed that the product code in
ranges_rotate_copy.hwas similarly damaged. In addition to fixing it by takingforward_iteratorandforward_rangeas depicted in the Standard, this drops the inclusion of<__iterator/reverse_iterator.h>as this algorithm doesn't needstd::__reverse_range.