Skip to content

[libc++] Implement part of P2562R1: constexpr std::stable_partition #128868

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Merged
merged 4 commits into from
Mar 4, 2025
Merged

[libc++] Implement part of P2562R1: constexpr std::stable_partition #128868

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
4 commits
Select commit Hold shift + click to select a range
e70a02f
[libc++] Implement constexpr `std::stable_partition`
frederick-vs-ja Mar 1, 2025
81986a6
clang-format some test files
frederick-vs-ja Mar 1, 2025
b9b392a
Merge branch 'main' into constexpr-std-stable_partition
frederick-vs-ja Mar 3, 2025
cc7878b
Address @mordante's review comments
frederick-vs-ja Mar 3, 2025
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
21 changes: 11 additions & 10 deletions libcxx/include/__algorithm/stable_partition.h
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -16,6 +16,7 @@
#include <__iterator/advance.h>
#include <__iterator/distance.h>
#include <__iterator/iterator_traits.h>
#include <__memory/construct_at.h>
#include <__memory/destruct_n.h>
#include <__memory/unique_ptr.h>
#include <__memory/unique_temporary_buffer.h>
Expand All @@ -33,7 +34,7 @@ _LIBCPP_PUSH_MACROS
_LIBCPP_BEGIN_NAMESPACE_STD

template <class _AlgPolicy, class _Predicate, class _ForwardIterator, class _Distance, class _Pair>
_LIBCPP_HIDE_FROM_ABI _ForwardIterator __stable_partition_impl(
_LIBCPP_HIDE_FROM_ABI _LIBCPP_CONSTEXPR_SINCE_CXX26 _ForwardIterator __stable_partition_impl(
_ForwardIterator __first,
_ForwardIterator __last,
_Predicate __pred,
Expand Down Expand Up @@ -61,7 +62,7 @@ _LIBCPP_HIDE_FROM_ABI _ForwardIterator __stable_partition_impl(
// Move the falses into the temporary buffer, and the trues to the front of the line
// Update __first to always point to the end of the trues
value_type* __t = __p.first;
::new ((void*)__t) value_type(_Ops::__iter_move(__first));
std::__construct_at(__t, _Ops::__iter_move(__first));
__d.template __incr<value_type>();
++__t;
_ForwardIterator __i = __first;
Expand All @@ -70,7 +71,7 @@ _LIBCPP_HIDE_FROM_ABI _ForwardIterator __stable_partition_impl(
*__first = _Ops::__iter_move(__i);
++__first;
} else {
::new ((void*)__t) value_type(_Ops::__iter_move(__i));
std::__construct_at(__t, _Ops::__iter_move(__i));
__d.template __incr<value_type>();
++__t;
}
Expand Down Expand Up @@ -116,7 +117,7 @@ _LIBCPP_HIDE_FROM_ABI _ForwardIterator __stable_partition_impl(
}

template <class _AlgPolicy, class _Predicate, class _ForwardIterator>
_LIBCPP_HIDE_FROM_ABI _ForwardIterator
_LIBCPP_HIDE_FROM_ABI _LIBCPP_CONSTEXPR_SINCE_CXX26 _ForwardIterator
__stable_partition_impl(_ForwardIterator __first, _ForwardIterator __last, _Predicate __pred, forward_iterator_tag) {
typedef typename iterator_traits<_ForwardIterator>::difference_type difference_type;
typedef typename iterator_traits<_ForwardIterator>::value_type value_type;
Expand Down Expand Up @@ -145,7 +146,7 @@ __stable_partition_impl(_ForwardIterator __first, _ForwardIterator __last, _Pred
}

template <class _AlgPolicy, class _Predicate, class _BidirectionalIterator, class _Distance, class _Pair>
_BidirectionalIterator __stable_partition_impl(
_LIBCPP_CONSTEXPR_SINCE_CXX26 _BidirectionalIterator __stable_partition_impl(
_BidirectionalIterator __first,
_BidirectionalIterator __last,
_Predicate __pred,
Expand Down Expand Up @@ -179,7 +180,7 @@ _BidirectionalIterator __stable_partition_impl(
// Move the falses into the temporary buffer, and the trues to the front of the line
// Update __first to always point to the end of the trues
value_type* __t = __p.first;
::new ((void*)__t) value_type(_Ops::__iter_move(__first));
std::__construct_at(__t, _Ops::__iter_move(__first));
__d.template __incr<value_type>();
++__t;
_BidirectionalIterator __i = __first;
Expand All @@ -188,7 +189,7 @@ _BidirectionalIterator __stable_partition_impl(
*__first = _Ops::__iter_move(__i);
++__first;
} else {
::new ((void*)__t) value_type(_Ops::__iter_move(__i));
std::__construct_at(__t, _Ops::__iter_move(__i));
__d.template __incr<value_type>();
++__t;
}
Expand Down Expand Up @@ -247,7 +248,7 @@ _BidirectionalIterator __stable_partition_impl(
}

template <class _AlgPolicy, class _Predicate, class _BidirectionalIterator>
_LIBCPP_HIDE_FROM_ABI _BidirectionalIterator __stable_partition_impl(
_LIBCPP_HIDE_FROM_ABI _LIBCPP_CONSTEXPR_SINCE_CXX26 _BidirectionalIterator __stable_partition_impl(
_BidirectionalIterator __first, _BidirectionalIterator __last, _Predicate __pred, bidirectional_iterator_tag) {
typedef typename iterator_traits<_BidirectionalIterator>::difference_type difference_type;
typedef typename iterator_traits<_BidirectionalIterator>::value_type value_type;
Expand Down Expand Up @@ -283,14 +284,14 @@ _LIBCPP_HIDE_FROM_ABI _BidirectionalIterator __stable_partition_impl(
}

template <class _AlgPolicy, class _Predicate, class _ForwardIterator, class _IterCategory>
_LIBCPP_HIDE_FROM_ABI _ForwardIterator __stable_partition(
_LIBCPP_HIDE_FROM_ABI _LIBCPP_CONSTEXPR_SINCE_CXX26 _ForwardIterator __stable_partition(
_ForwardIterator __first, _ForwardIterator __last, _Predicate&& __pred, _IterCategory __iter_category) {
return std::__stable_partition_impl<_AlgPolicy, __remove_cvref_t<_Predicate>&>(
std::move(__first), std::move(__last), __pred, __iter_category);
}

template <class _ForwardIterator, class _Predicate>
inline _LIBCPP_HIDE_FROM_ABI _ForwardIterator
_LIBCPP_HIDE_FROM_ABI inline _LIBCPP_CONSTEXPR_SINCE_CXX26 _ForwardIterator
stable_partition(_ForwardIterator __first, _ForwardIterator __last, _Predicate __pred) {
using _IterCategory = typename iterator_traits<_ForwardIterator>::iterator_category;
return std::__stable_partition<_ClassicAlgPolicy, _Predicate&>(
Expand Down
2 changes: 1 addition & 1 deletion libcxx/include/algorithm
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -1498,7 +1498,7 @@ template <class InputIterator, class OutputIterator1,
Predicate pred);

template <class ForwardIterator, class Predicate>
ForwardIterator
constexpr ForwardIterator // constexpr in C++26
stable_partition(ForwardIterator first, ForwardIterator last, Predicate pred);

template<class ForwardIterator, class Predicate>
Expand Down
68 changes: 35 additions & 33 deletions libcxx/test/std/algorithms/alg.modifying.operations/alg.partitions/stable_partition.pass.cpp
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -11,7 +11,7 @@
// template<BidirectionalIterator Iter, Predicate<auto, Iter::value_type> Pred>
// requires ShuffleIterator<Iter>
// && CopyConstructible<Pred>
// Iter
// constexpr Iter // constexpr since C++26
// stable_partition(Iter first, Iter last, Pred pred);

#include <algorithm>
Expand All @@ -23,21 +23,16 @@
#include "test_iterators.h"
#include "test_macros.h"

struct is_odd
{
bool operator()(const int& i) const {return i & 1;}
struct is_odd {
TEST_CONSTEXPR_CXX26 bool operator()(const int& i) const { return i & 1; }
};

struct odd_first
{
bool operator()(const std::pair<int,int>& p) const
{return p.first & 1;}
struct odd_first {
TEST_CONSTEXPR_CXX26 bool operator()(const std::pair<int, int>& p) const { return p.first & 1; }
};

template <class Iter>
void
test()
{
TEST_CONSTEXPR_CXX26 void test() {
{ // check mixed
typedef std::pair<int,int> P;
P array[] =
Expand Down Expand Up @@ -282,10 +277,10 @@ test()
assert(array[9] == P(0, 2));
}
#if TEST_STD_VER >= 11 && !defined(TEST_HAS_NO_EXCEPTIONS)
// TODO: Re-enable this test once we get recursive inlining fixed.
// TODO: Re-enable this test for GCC once we get recursive inlining fixed.
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Do you know whether there is a GCC bug report? If there is it would be great to add a link here.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

It seems that this is not a GCC bug. When compiling with GCC, our _LIBCPP_HIDE_FROM_ABI contains __attribute__((__always_inline__)), but many of our implementation details are recursive, and thus possibly trigger errors.

https://reviews.llvm.org/D154161 is related.

// For now it trips up GCC due to the use of always_inline.
# if 0
{ // check that the algorithm still works when no memory is available
# if !defined(TEST_COMPILER_GCC)
if (!TEST_IS_CONSTANT_EVALUATED) { // check that the algorithm still works when no memory is available
std::vector<int> vec(150, 3);
vec[5] = 6;
getGlobalMemCounter()->throw_after = 0;
Expand All @@ -300,38 +295,45 @@ test()
assert(std::is_partitioned(vec.begin(), vec.end(), [](int i) { return i < 5; }));
getGlobalMemCounter()->reset();
}
# endif
#endif // TEST_STD_VER >= 11 && !defined(TEST_HAS_NO_EXCEPTIONS)
# endif // !defined(TEST_COMPILER_GCC)
#endif // TEST_STD_VER >= 11 && !defined(TEST_HAS_NO_EXCEPTIONS)
}

#if TEST_STD_VER >= 11

struct is_null
{
template <class P>
bool operator()(const P& p) {return p == 0;}
struct is_null {
template <class P>
TEST_CONSTEXPR_CXX26 bool operator()(const P& p) {
return p == 0;
}
};

template <class Iter>
void
test1()
{
const unsigned size = 5;
std::unique_ptr<int> array[size];
Iter r = std::stable_partition(Iter(array), Iter(array+size), is_null());
assert(r == Iter(array+size));
TEST_CONSTEXPR_CXX26 void test1() {
const unsigned size = 5;
std::unique_ptr<int> array[size];
Iter r = std::stable_partition(Iter(array), Iter(array + size), is_null());
assert(r == Iter(array + size));
}

#endif // TEST_STD_VER >= 11

int main(int, char**)
{
test<bidirectional_iterator<std::pair<int,int>*> >();
test<random_access_iterator<std::pair<int,int>*> >();
test<std::pair<int,int>*>();
TEST_CONSTEXPR_CXX26 bool test() {
test<bidirectional_iterator<std::pair<int, int>*> >();
test<random_access_iterator<std::pair<int, int>*> >();
test<std::pair<int, int>*>();

#if TEST_STD_VER >= 11
test1<bidirectional_iterator<std::unique_ptr<int>*> >();
test1<bidirectional_iterator<std::unique_ptr<int>*> >();
#endif

return true;
}

int main(int, char**) {
test();
#if TEST_STD_VER >= 26
static_assert(test());
#endif

return 0;
Expand Down
14 changes: 11 additions & 3 deletions libcxx/test/std/algorithms/robust_against_proxy_iterators_lifetime_bugs.pass.cpp
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -735,12 +735,20 @@ TEST_CONSTEXPR_CXX20 bool test() {
test(simple_in, [&](I b, I e) { (void) std::shuffle(b, e, rand_gen()); });
// TODO: unique
test(simple_in, [&](I b, I e) { (void) std::partition(b, e, is_neg); });
#if TEST_STD_VER < 26
if (!TEST_IS_CONSTANT_EVALUATED)
test(simple_in, [&](I b, I e) { (void) std::stable_partition(b, e, is_neg); });
#endif
{
test(simple_in, [&](I b, I e) { (void)std::stable_partition(b, e, is_neg); });
}
if (!TEST_IS_CONSTANT_EVALUATED)
test(sort_test_in, [&](I b, I e) { (void) std::sort(b, e); });
test(sort_test_in, [&](I b, I e) { (void)std::sort(b, e); });
#if TEST_STD_VER < 26
if (!TEST_IS_CONSTANT_EVALUATED)
test(sort_test_in, [&](I b, I e) { (void) std::stable_sort(b, e); });
#endif
{
test(sort_test_in, [&](I b, I e) { (void)std::stable_sort(b, e); });
}
// TODO: partial_sort
// TODO: nth_element
// TODO: inplace_merge
Expand Down
11 changes: 8 additions & 3 deletions libcxx/test/std/algorithms/robust_re_difference_type.compile.pass.cpp
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -240,9 +240,14 @@ TEST_CONSTEXPR_CXX20 bool all_the_algorithms()
(void)std::sort(first, last, std::less<void*>());
(void)std::sort_heap(first, last);
(void)std::sort_heap(first, last, std::less<void*>());
if (!TEST_IS_CONSTANT_EVALUATED) (void)std::stable_partition(first, last, UnaryTrue());
if (!TEST_IS_CONSTANT_EVALUATED) (void)std::stable_sort(first, last);
if (!TEST_IS_CONSTANT_EVALUATED) (void)std::stable_sort(first, last, std::less<void*>());
#if TEST_STD_VER < 26
if (!TEST_IS_CONSTANT_EVALUATED)
#endif
{
(void)std::stable_partition(first, last, UnaryTrue());
(void)std::stable_sort(first, last);
(void)std::stable_sort(first, last, std::less<void*>());
Comment on lines +248 to +249
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Why did you change stable_sort, and why is the CI happy?

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This part was drive-by. I changed the lines because they were adjacent to the line for stable_partition. Previously, constexpr stable_sort was implemented by #110320, so the CI is happy.

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I saw an open stable_sort PR, but I overlooked that is the ranges version. My bad.

}
(void)std::swap_ranges(first, last, first2);
(void)std::transform(first, last, first2, UnaryTransform());
(void)std::transform(first, mid, mid, first2, BinaryTransform());
Expand Down