Fix ranges_rotate.pass.cpp complexity checks #158144
Conversation
The complexity is "at most N swaps" _for each invocation of `rotate`_, but the tests currently assert that the total number of swaps for N calls is at most N. The standard allows that to be N squared, so the test is either requiring more than the standard (and the comment in the test) promises, or somebody just forgot to reset the counter on each iteration. If the intent really is to verify the libc++ guarantee, not the standard one, then shouldn't `expected` be set to N/2 i.e. 5, since that's what libc++ actually does? Either way, to be portable to other implementations, swaps should be reset on each loop. If you want to guard that with `#ifndef _LIBCPP_VERSION` it would not change the test for libc++.
llvmbot
added
the
libc++
|
@llvm/pr-subscribers-libcxx Author: Jonathan Wakely (jwakely) ChangesThe complexity is "at most N swaps" for each invocation of If the intent really is to verify the libc++ guarantee, not the standard one, then shouldn't Either way, to be portable to other implementations, swaps should be reset on each loop. If you want to guard that with Full diff: https://github.com/llvm/llvm-project/pull/158144.diff 1 Files Affected:
diff --git a/libcxx/test/std/algorithms/alg.modifying.operations/alg.rotate/ranges_rotate.pass.cpp b/libcxx/test/std/algorithms/alg.modifying.operations/alg.rotate/ranges_rotate.pass.cpp
index 5f594400e8321..574e96dea46a0 100644
--- a/libcxx/test/std/algorithms/alg.modifying.operations/alg.rotate/ranges_rotate.pass.cpp
+++ b/libcxx/test/std/algorithms/alg.modifying.operations/alg.rotate/ranges_rotate.pass.cpp
@@ -173,6 +173,7 @@ constexpr bool test() {
auto end = adl::Iterator::TrackSwaps(in.data() + in.size(), swaps);
for (std::size_t mid = 0; mid != input.size(); ++mid) {
+ swaps = 0;
std::ranges::rotate(begin, begin + mid, end);
assert(swaps <= expected);
}
@@ -186,6 +187,7 @@ constexpr bool test() {
auto range = std::ranges::subrange(begin, end);
for (std::size_t mid = 0; mid != input.size(); ++mid) {
+ swaps = 0;
std::ranges::rotate(range, begin + mid);
assert(swaps <= expected);
}
|
|
Thanks! one more local change I can drop when using these for libstdc++ testing :) |
The complexity is "at most N swaps" for each invocation of
rotate, but the tests currently assert that the total number of swaps for N calls is at most N. The standard allows that to be N squared, so the test is either requiring more than the standard (and the comment in the test) promises, or somebody just forgot to reset the counter on each iteration.If the intent really is to verify the libc++ guarantee, not the standard one, then shouldn't
expectedbe set to N/2 i.e. 5, since that's what libc++ actually does?Either way, to be portable to other implementations, swaps should be reset on each loop. If you want to guard that with
#ifndef _LIBCPP_VERSIONit would not change the test for libc++.