[libcxx-commits] [libcxx] bfdc562 - [libc++] Fix copy-paste damage in `ranges::rotate_copy` and its test (#74544)

[via libcxx-commits]

Author: Stephan T. Lavavej
Date: 2023-12-06T02:29:09-08:00
New Revision: bfdc562d0c6ff18ae3a8d618a2cb05c22a04898b

URL: https://github.com/llvm/llvm-project/commit/bfdc562d0c6ff18ae3a8d618a2cb05c22a04898b
DIFF: https://github.com/llvm/llvm-project/commit/bfdc562d0c6ff18ae3a8d618a2cb05c22a04898b.diff

LOG: [libc++] Fix copy-paste damage in `ranges::rotate_copy` and its test (#74544)

Found while running libc++'s tests with MSVC's STL.

`ranges::rotate_copy` takes `forward_iterator`s as this test's comment
banner correctly depicts. However, this test had bogus assertions
expecting that `ranges::rotate_copy` would be constrained away for
not-quite-**bidi** iterators. @philnik777 confirmed that these were
copy-paste relics from the `ranges::reverse_copy` test.

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
possible `forward_iterator<int*>`.

This revealed that the product code in `ranges_rotate_copy.h` was
similarly damaged. In addition to fixing it by 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`.

Added: 
    

Modified: 
    libcxx/include/__algorithm/ranges_rotate_copy.h
    libcxx/test/std/algorithms/alg.modifying.operations/alg.rotate/ranges.rotate_copy.pass.cpp

Removed: 
    


################################################################################
diff  --git a/libcxx/include/__algorithm/ranges_rotate_copy.h b/libcxx/include/__algorithm/ranges_rotate_copy.h
index 301672707f72b..ab76c0944c477 100644
--- a/libcxx/include/__algorithm/ranges_rotate_copy.h
+++ b/libcxx/include/__algorithm/ranges_rotate_copy.h
@@ -13,7 +13,6 @@
 #include <__algorithm/ranges_copy.h>
 #include <__config>
 #include <__iterator/concepts.h>
-#include <__iterator/reverse_iterator.h>
 #include <__ranges/access.h>
 #include <__ranges/concepts.h>
 #include <__ranges/dangling.h>
@@ -34,7 +33,7 @@ using rotate_copy_result = in_out_result<_InIter, _OutIter>;
 
 namespace __rotate_copy {
 struct __fn {
-  template <bidirectional_iterator _InIter, sentinel_for<_InIter> _Sent, weakly_incrementable _OutIter>
+  template <forward_iterator _InIter, sentinel_for<_InIter> _Sent, weakly_incrementable _OutIter>
     requires indirectly_copyable<_InIter, _OutIter>
   _LIBCPP_HIDE_FROM_ABI constexpr rotate_copy_result<_InIter, _OutIter>
   operator()(_InIter __first, _InIter __middle, _Sent __last, _OutIter __result) const {
@@ -43,7 +42,7 @@ struct __fn {
     return {std::move(__res1.in), std::move(__res2.out)};
   }
 
-  template <bidirectional_range _Range, weakly_incrementable _OutIter>
+  template <forward_range _Range, weakly_incrementable _OutIter>
     requires indirectly_copyable<iterator_t<_Range>, _OutIter>
   _LIBCPP_HIDE_FROM_ABI constexpr rotate_copy_result<borrowed_iterator_t<_Range>, _OutIter>
   operator()(_Range&& __range, iterator_t<_Range> __middle, _OutIter __result) const {

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*>>();