[libcxx-commits] [libcxx] 5d95656 - Revert "Revert "[libc++] [P0879] constexpr std::nth_element, and rewrite its tests.""
Arthur O'Dwyer via libcxx-commits
libcxx-commits at lists.llvm.org
Fri Feb 5 09:03:05 PST 2021
Author: Arthur O'Dwyer
Date: 2021-02-05T12:02:43-05:00
New Revision: 5d9565634c97a075940041a6756a2583fa1f0bc9
URL: https://github.com/llvm/llvm-project/commit/5d9565634c97a075940041a6756a2583fa1f0bc9
DIFF: https://github.com/llvm/llvm-project/commit/5d9565634c97a075940041a6756a2583fa1f0bc9.diff
LOG: Revert "Revert "[libc++] [P0879] constexpr std::nth_element, and rewrite its tests.""
This reverts commit b6ffece32035a90d181101f356bd9c04ea1d3122.
The bug is now fixed (it was a stupid cut-and-paste kind of error),
and the regression test added. The new patch is also simpler than the old one!
Differential Revision: https://reviews.llvm.org/D96084
Added:
Modified:
libcxx/include/algorithm
libcxx/test/std/algorithms/alg.sorting/alg.nth.element/nth_element.pass.cpp
libcxx/test/std/algorithms/alg.sorting/alg.nth.element/nth_element_comp.pass.cpp
Removed:
################################################################################
diff --git a/libcxx/include/algorithm b/libcxx/include/algorithm
index a6fceaa91292..04126a1f4836 100644
--- a/libcxx/include/algorithm
+++ b/libcxx/include/algorithm
@@ -385,11 +385,11 @@ template <class InputIterator, class RandomAccessIterator, class Compare>
RandomAccessIterator result_first, RandomAccessIterator result_last, Compare comp);
template <class RandomAccessIterator>
- void
+ constexpr void // constexpr in C++20
nth_element(RandomAccessIterator first, RandomAccessIterator nth, RandomAccessIterator last);
template <class RandomAccessIterator, class Compare>
- void
+ constexpr void // constexpr in C++20
nth_element(RandomAccessIterator first, RandomAccessIterator nth, RandomAccessIterator last, Compare comp);
template <class ForwardIterator, class T>
@@ -3807,7 +3807,7 @@ is_sorted(_ForwardIterator __first, _ForwardIterator __last)
// stable, 2-3 compares, 0-2 swaps
template <class _Compare, class _ForwardIterator>
-unsigned
+_LIBCPP_CONSTEXPR_AFTER_CXX11 unsigned
__sort3(_ForwardIterator __x, _ForwardIterator __y, _ForwardIterator __z, _Compare __c)
{
unsigned __r = 0;
@@ -3901,7 +3901,7 @@ __sort5(_ForwardIterator __x1, _ForwardIterator __x2, _ForwardIterator __x3,
// Assumes size > 0
template <class _Compare, class _BidirectionalIterator>
-void
+_LIBCPP_CONSTEXPR_AFTER_CXX11 void
__selection_sort(_BidirectionalIterator __first, _BidirectionalIterator __last, _Compare __comp)
{
_BidirectionalIterator __lm1 = __last;
@@ -5218,8 +5218,24 @@ partial_sort_copy(_InputIterator __first, _InputIterator __last,
// nth_element
+template<class _Compare, class _RandomAccessIterator>
+_LIBCPP_CONSTEXPR_AFTER_CXX11 bool
+__nth_element_find_guard(_RandomAccessIterator& __i, _RandomAccessIterator& __j,
+ _RandomAccessIterator __m, _Compare __comp)
+{
+ // manually guard downward moving __j against __i
+ while (true) {
+ if (__i == --__j) {
+ return false;
+ }
+ if (__comp(*__j, *__m)) {
+ return true; // found guard for downward moving __j, now use unguarded partition
+ }
+ }
+}
+
template <class _Compare, class _RandomAccessIterator>
-void
+_LIBCPP_CONSTEXPR_AFTER_CXX11 void
__nth_element(_RandomAccessIterator __first, _RandomAccessIterator __nth, _RandomAccessIterator __last, _Compare __comp)
{
// _Compare is known to be a reference type
@@ -5227,7 +5243,6 @@ __nth_element(_RandomAccessIterator __first, _RandomAccessIterator __nth, _Rando
const
diff erence_type __limit = 7;
while (true)
{
- __restart:
if (__nth == __last)
return;
diff erence_type __len = __last - __first;
@@ -5267,61 +5282,51 @@ __nth_element(_RandomAccessIterator __first, _RandomAccessIterator __nth, _Rando
if (!__comp(*__i, *__m)) // if *__first == *__m
{
// *__first == *__m, *__first doesn't go in first part
- // manually guard downward moving __j against __i
- while (true)
- {
- if (__i == --__j)
- {
- // *__first == *__m, *__m <= all other elements
- // Partition instead into [__first, __i) == *__first and *__first < [__i, __last)
- ++__i; // __first + 1
- __j = __last;
- if (!__comp(*__first, *--__j)) // we need a guard if *__first == *(__last-1)
- {
- while (true)
- {
- if (__i == __j)
- return; // [__first, __last) all equivalent elements
- if (__comp(*__first, *__i))
- {
- swap(*__i, *__j);
- ++__n_swaps;
- ++__i;
- break;
- }
- ++__i;
- }
- }
- // [__first, __i) == *__first and *__first < [__j, __last) and __j == __last - 1
- if (__i == __j)
- return;
- while (true)
- {
- while (!__comp(*__first, *__i))
+ if (_VSTD::__nth_element_find_guard<_Compare>(__i, __j, __m, __comp)) {
+ swap(*__i, *__j);
+ ++__n_swaps;
+ } else {
+ // *__first == *__m, *__m <= all other elements
+ // Partition instead into [__first, __i) == *__first and *__first < [__i, __last)
+ ++__i; // __first + 1
+ __j = __last;
+ if (!__comp(*__first, *--__j)) { // we need a guard if *__first == *(__last-1)
+ while (true) {
+ if (__i == __j) {
+ return; // [__first, __last) all equivalent elements
+ } else if (__comp(*__first, *__i)) {
+ swap(*__i, *__j);
+ ++__n_swaps;
++__i;
- while (__comp(*__first, *--__j))
- ;
- if (__i >= __j)
break;
- swap(*__i, *__j);
- ++__n_swaps;
+ }
++__i;
}
- // [__first, __i) == *__first and *__first < [__i, __last)
- // The first part is sorted,
- if (__nth < __i)
- return;
- // __nth_element the second part
- // _VSTD::__nth_element<_Compare>(__i, __nth, __last, __comp);
- __first = __i;
- goto __restart;
}
- if (__comp(*__j, *__m))
- {
+ // [__first, __i) == *__first and *__first < [__j, __last) and __j == __last - 1
+ if (__i == __j) {
+ return;
+ }
+ while (true) {
+ while (!__comp(*__first, *__i))
+ ++__i;
+ while (__comp(*__first, *--__j))
+ ;
+ if (__i >= __j)
+ break;
swap(*__i, *__j);
++__n_swaps;
- break; // found guard for downward moving __j, now use unguarded partition
+ ++__i;
}
+ // [__first, __i) == *__first and *__first < [__i, __last)
+ // The first part is sorted,
+ if (__nth < __i) {
+ return;
+ }
+ // __nth_element the second part
+ // _VSTD::__nth_element<_Compare>(__i, __nth, __last, __comp);
+ __first = __i;
+ continue;
}
}
++__i;
@@ -5365,32 +5370,35 @@ __nth_element(_RandomAccessIterator __first, _RandomAccessIterator __nth, _Rando
{
// Check for [__first, __i) already sorted
__j = __m = __first;
- while (++__j != __i)
- {
- if (__comp(*__j, *__m))
+ while (true) {
+ if (++__j == __i) {
+ // [__first, __i) sorted
+ return;
+ }
+ if (__comp(*__j, *__m)) {
// not yet sorted, so sort
- goto not_sorted;
+ break;
+ }
__m = __j;
}
- // [__first, __i) sorted
- return;
}
else
{
// Check for [__i, __last) already sorted
__j = __m = __i;
- while (++__j != __last)
- {
- if (__comp(*__j, *__m))
+ while (true) {
+ if (++__j == __last) {
+ // [__i, __last) sorted
+ return;
+ }
+ if (__comp(*__j, *__m)) {
// not yet sorted, so sort
- goto not_sorted;
+ break;
+ }
__m = __j;
}
- // [__i, __last) sorted
- return;
}
}
-not_sorted:
// __nth_element on range containing __nth
if (__nth < __i)
{
@@ -5406,7 +5414,7 @@ not_sorted:
}
template <class _RandomAccessIterator, class _Compare>
-inline _LIBCPP_INLINE_VISIBILITY
+inline _LIBCPP_INLINE_VISIBILITY _LIBCPP_CONSTEXPR_AFTER_CXX17
void
nth_element(_RandomAccessIterator __first, _RandomAccessIterator __nth, _RandomAccessIterator __last, _Compare __comp)
{
@@ -5415,7 +5423,7 @@ nth_element(_RandomAccessIterator __first, _RandomAccessIterator __nth, _RandomA
}
template <class _RandomAccessIterator>
-inline _LIBCPP_INLINE_VISIBILITY
+inline _LIBCPP_INLINE_VISIBILITY _LIBCPP_CONSTEXPR_AFTER_CXX17
void
nth_element(_RandomAccessIterator __first, _RandomAccessIterator __nth, _RandomAccessIterator __last)
{
diff --git a/libcxx/test/std/algorithms/alg.sorting/alg.nth.element/nth_element.pass.cpp b/libcxx/test/std/algorithms/alg.sorting/alg.nth.element/nth_element.pass.cpp
index 18c2d1392938..37472a12231f 100644
--- a/libcxx/test/std/algorithms/alg.sorting/alg.nth.element/nth_element.pass.cpp
+++ b/libcxx/test/std/algorithms/alg.sorting/alg.nth.element/nth_element.pass.cpp
@@ -9,61 +9,78 @@
// <algorithm>
// template<RandomAccessIterator Iter>
-// requires ShuffleIterator<Iter>
-// && LessThanComparable<Iter::value_type>
-// void
+// requires ShuffleIterator<Iter> && LessThanComparable<Iter::value_type>
+// constexpr void // constexpr in C++20
// nth_element(Iter first, Iter nth, Iter last);
#include <algorithm>
-#include <random>
#include <cassert>
#include "test_macros.h"
+#include "test_iterators.h"
+#include "MoveOnly.h"
-std::mt19937 randomness;
-
-void
-test_one(int N, int M)
+template<class T, class Iter>
+TEST_CONSTEXPR_CXX20 bool test()
{
- assert(N != 0);
- assert(M < N);
- int* array = new int[N];
- for (int i = 0; i < N; ++i)
- array[i] = i;
- std::shuffle(array, array+N, randomness);
- std::nth_element(array, array+M, array+N);
- assert(array[M] == M);
- std::nth_element(array, array+N, array+N); // begin, end, end
- delete [] array;
-}
+ int orig[15] = {3,1,4,1,5, 9,2,6,5,3, 5,8,9,7,9};
+ T work[15] = {3,1,4,1,5, 9,2,6,5,3, 5,8,9,7,9};
+ for (int n = 0; n < 15; ++n) {
+ for (int m = 0; m < n; ++m) {
+ std::nth_element(Iter(work), Iter(work+m), Iter(work+n));
+ assert(std::is_permutation(work, work+n, orig));
+ // No element to m's left is greater than m.
+ for (int i = 0; i < m; ++i) {
+ assert(!(work[i] > work[m]));
+ }
+ // No element to m's right is less than m.
+ for (int i = m; i < n; ++i) {
+ assert(!(work[i] < work[m]));
+ }
+ std::copy(orig, orig+15, work);
+ }
+ }
-void
-test(int N)
-{
- test_one(N, 0);
- test_one(N, 1);
- test_one(N, 2);
- test_one(N, 3);
- test_one(N, N/2-1);
- test_one(N, N/2);
- test_one(N, N/2+1);
- test_one(N, N-3);
- test_one(N, N-2);
- test_one(N, N-1);
+ {
+ T input[] = {3,1,4,1,5,9,2};
+ std::nth_element(Iter(input), Iter(input+4), Iter(input+7));
+ assert(input[4] == 4);
+ assert(input[5] + input[6] == 5 + 9);
+ }
+
+ {
+ T input[] = {0, 1, 2, 3, 4, 5, 7, 6};
+ std::nth_element(Iter(input), Iter(input + 6), Iter(input + 8));
+ assert(input[6] == 6);
+ assert(input[7] == 7);
+ }
+
+ {
+ T input[] = {1, 0, 2, 3, 4, 5, 6, 7};
+ std::nth_element(Iter(input), Iter(input + 1), Iter(input + 8));
+ assert(input[0] == 0);
+ assert(input[1] == 1);
+ }
+
+ return true;
}
int main(int, char**)
{
- int d = 0;
- std::nth_element(&d, &d, &d);
- assert(d == 0);
- test(256);
- test(257);
- test(499);
- test(500);
- test(997);
- test(1000);
- test(1009);
+ test<int, random_access_iterator<int*> >();
+ test<int, int*>();
+
+#if TEST_STD_VER >= 11
+ test<MoveOnly, random_access_iterator<MoveOnly*>>();
+ test<MoveOnly, MoveOnly*>();
+#endif
+
+#if TEST_STD_VER >= 20
+ static_assert(test<int, random_access_iterator<int*>>());
+ static_assert(test<int, int*>());
+ static_assert(test<MoveOnly, random_access_iterator<MoveOnly*>>());
+ static_assert(test<MoveOnly, MoveOnly*>());
+#endif
- return 0;
+ return 0;
}
diff --git a/libcxx/test/std/algorithms/alg.sorting/alg.nth.element/nth_element_comp.pass.cpp b/libcxx/test/std/algorithms/alg.sorting/alg.nth.element/nth_element_comp.pass.cpp
index 980b2b9881b8..f45ba114c0fc 100644
--- a/libcxx/test/std/algorithms/alg.sorting/alg.nth.element/nth_element_comp.pass.cpp
+++ b/libcxx/test/std/algorithms/alg.sorting/alg.nth.element/nth_element_comp.pass.cpp
@@ -9,82 +9,79 @@
// <algorithm>
// template<RandomAccessIterator Iter, StrictWeakOrder<auto, Iter::value_type> Compare>
-// requires ShuffleIterator<Iter>
-// && CopyConstructible<Compare>
-// void
+// requires ShuffleIterator<Iter> && CopyConstructible<Compare>
+// constexpr void // constexpr in C++20
// nth_element(Iter first, Iter nth, Iter last, Compare comp);
#include <algorithm>
-#include <functional>
-#include <vector>
-#include <random>
#include <cassert>
-#include <cstddef>
-#include <memory>
+#include <functional>
#include "test_macros.h"
+#include "test_iterators.h"
+#include "MoveOnly.h"
-struct indirect_less
+template<class T, class Iter>
+TEST_CONSTEXPR_CXX20 bool test()
{
- template <class P>
- bool operator()(const P& x, const P& y)
- {return *x < *y;}
-};
+ int orig[15] = {3,1,4,1,5, 9,2,6,5,3, 5,8,9,7,9};
+ T work[15] = {3,1,4,1,5, 9,2,6,5,3, 5,8,9,7,9};
+ for (int n = 0; n < 15; ++n) {
+ for (int m = 0; m < n; ++m) {
+ std::nth_element(Iter(work), Iter(work+m), Iter(work+n), std::greater<T>());
+ assert(std::is_permutation(work, work+n, orig));
+ // No element to m's left is less than m.
+ for (int i = 0; i < m; ++i) {
+ assert(!(work[i] < work[m]));
+ }
+ // No element to m's right is greater than m.
+ for (int i = m; i < n; ++i) {
+ assert(!(work[i] > work[m]));
+ }
+ std::copy(orig, orig+15, work);
+ }
+ }
-std::mt19937 randomness;
+ {
+ T input[] = {3,1,4,1,5,9,2};
+ std::nth_element(Iter(input), Iter(input+4), Iter(input+7), std::greater<T>());
+ assert(input[4] == 2);
+ assert(input[5] + input[6] == 1 + 1);
+ }
-void
-test_one(int N, int M)
-{
- assert(N != 0);
- assert(M < N);
- int* array = new int[N];
- for (int i = 0; i < N; ++i)
- array[i] = i;
- std::shuffle(array, array+N, randomness);
- std::nth_element(array, array+M, array+N, std::greater<int>());
- assert(array[M] == N-M-1);
- std::nth_element(array, array+N, array+N, std::greater<int>()); // begin, end, end
- delete [] array;
-}
+ {
+ T input[] = {0, 1, 2, 3, 4, 5, 7, 6};
+ std::nth_element(Iter(input), Iter(input + 6), Iter(input + 8), std::greater<T>());
+ assert(input[6] == 1);
+ assert(input[7] == 0);
+ }
-void
-test(int N)
-{
- test_one(N, 0);
- test_one(N, 1);
- test_one(N, 2);
- test_one(N, 3);
- test_one(N, N/2-1);
- test_one(N, N/2);
- test_one(N, N/2+1);
- test_one(N, N-3);
- test_one(N, N-2);
- test_one(N, N-1);
+ {
+ T input[] = {1, 0, 2, 3, 4, 5, 6, 7};
+ std::nth_element(Iter(input), Iter(input + 1), Iter(input + 8), std::greater<T>());
+ assert(input[0] == 7);
+ assert(input[1] == 6);
+ }
+
+ return true;
}
int main(int, char**)
{
- int d = 0;
- std::nth_element(&d, &d, &d);
- assert(d == 0);
- test(256);
- test(257);
- test(499);
- test(500);
- test(997);
- test(1000);
- test(1009);
+ test<int, random_access_iterator<int*> >();
+ test<int, int*>();
#if TEST_STD_VER >= 11
- {
- std::vector<std::unique_ptr<int> > v(1000);
- for (int i = 0; static_cast<std::size_t>(i) < v.size(); ++i)
- v[i].reset(new int(i));
- std::nth_element(v.begin(), v.begin() + v.size()/2, v.end(), indirect_less());
- assert(static_cast<std::size_t>(*v[v.size()/2]) == v.size()/2);
- }
+ test<MoveOnly, random_access_iterator<MoveOnly*>>();
+ test<MoveOnly, MoveOnly*>();
+#endif
+
+#if TEST_STD_VER >= 20
+ static_assert(test<int, random_access_iterator<int*>>());
+ static_assert(test<int, int*>());
+ static_assert(test<MoveOnly, random_access_iterator<MoveOnly*>>());
+ static_assert(test<MoveOnly, MoveOnly*>());
#endif
- return 0;
+ return 0;
}
More information about the libcxx-commits
mailing list