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.

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

Conversation

frederick-vs-ja
Copy link
Contributor

@frederick-vs-ja frederick-vs-ja commented Feb 26, 2025

Drive-by changes:

  • Enables no-memory case for Clang.
  • Enables robust_re_difference_type.compile.pass.cpp and robust_against_proxy_iterators_lifetime_bugs.pass.cpp test coverage for std::stable_sort in constant evaluation since C++26. The changes were missing in the PR making std::stable_sort constexpr.

Fixes #119396.

@frederick-vs-ja frederick-vs-ja requested a review from a team as a code owner February 26, 2025 12:27
@llvmbot llvmbot added the libc++ libc++ C++ Standard Library. Not GNU libstdc++. Not libc++abi. label Feb 26, 2025
@llvmbot
Copy link
Member

llvmbot commented Feb 26, 2025

@llvm/pr-subscribers-libcxx

Author: A. Jiang (frederick-vs-ja)

Changes

Drive-by changes:

  • Adds one missed _LIBCPP_HIDE_FROM_ABI.
  • Enables no-memory case for Clang.

Fixes #119396.


Full diff: https://github.com/llvm/llvm-project/pull/128868.diff

5 Files Affected:

  • (modified) libcxx/include/__algorithm/stable_partition.h (+11-10)
  • (modified) libcxx/include/algorithm (+1-1)
  • (modified) libcxx/test/std/algorithms/alg.modifying.operations/alg.partitions/stable_partition.pass.cpp (+71-165)
  • (modified) libcxx/test/std/algorithms/robust_against_proxy_iterators_lifetime_bugs.pass.cpp (+12-4)
  • (modified) libcxx/test/std/algorithms/robust_re_difference_type.compile.pass.cpp (+8-3)
diff --git a/libcxx/include/__algorithm/stable_partition.h b/libcxx/include/__algorithm/stable_partition.h
index 2ba7239a3a039..8f4fe80a15aa8 100644
--- a/libcxx/include/__algorithm/stable_partition.h
+++ b/libcxx/include/__algorithm/stable_partition.h
@@ -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>
@@ -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,
@@ -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;
@@ -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;
       }
@@ -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;
@@ -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_HIDE_FROM_ABI _LIBCPP_CONSTEXPR_SINCE_CXX26 _BidirectionalIterator __stable_partition_impl(
     _BidirectionalIterator __first,
     _BidirectionalIterator __last,
     _Predicate __pred,
@@ -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;
@@ -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;
       }
@@ -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;
@@ -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&>(
diff --git a/libcxx/include/algorithm b/libcxx/include/algorithm
index 7b4cb8e496196..0c7cea11d1a91 100644
--- a/libcxx/include/algorithm
+++ b/libcxx/include/algorithm
@@ -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>
diff --git a/libcxx/test/std/algorithms/alg.modifying.operations/alg.partitions/stable_partition.pass.cpp b/libcxx/test/std/algorithms/alg.modifying.operations/alg.partitions/stable_partition.pass.cpp
index 44027543aaf16..7bc521ff154a9 100644
--- a/libcxx/test/std/algorithms/alg.modifying.operations/alg.partitions/stable_partition.pass.cpp
+++ b/libcxx/test/std/algorithms/alg.modifying.operations/alg.partitions/stable_partition.pass.cpp
@@ -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>
@@ -23,38 +23,21 @@
 #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[] =
-    {
-        P(0, 1),
-        P(0, 2),
-        P(1, 1),
-        P(1, 2),
-        P(2, 1),
-        P(2, 2),
-        P(3, 1),
-        P(3, 2),
-        P(4, 1),
-        P(4, 2)
-    };
-    const unsigned size = sizeof(array)/sizeof(array[0]);
-    Iter r = std::stable_partition(Iter(array), Iter(array+size), odd_first());
+    typedef std::pair<int, int> P;
+    P array[]           = {P(0, 1), P(0, 2), P(1, 1), P(1, 2), P(2, 1), P(2, 2), P(3, 1), P(3, 2), P(4, 1), P(4, 2)};
+    const unsigned size = sizeof(array) / sizeof(array[0]);
+    Iter r              = std::stable_partition(Iter(array), Iter(array + size), odd_first());
     assert(base(r) == array + 4);
     assert(array[0] == P(1, 1));
     assert(array[1] == P(1, 2));
@@ -68,22 +51,10 @@ test()
     assert(array[9] == P(4, 2));
   }
   {
-    typedef std::pair<int,int> P;
-    P array[] =
-    {
-        P(0, 1),
-        P(0, 2),
-        P(1, 1),
-        P(1, 2),
-        P(2, 1),
-        P(2, 2),
-        P(3, 1),
-        P(3, 2),
-        P(4, 1),
-        P(4, 2)
-    };
-    const unsigned size = sizeof(array)/sizeof(array[0]);
-    Iter r = std::stable_partition(Iter(array), Iter(array+size), odd_first());
+    typedef std::pair<int, int> P;
+    P array[]           = {P(0, 1), P(0, 2), P(1, 1), P(1, 2), P(2, 1), P(2, 2), P(3, 1), P(3, 2), P(4, 1), P(4, 2)};
+    const unsigned size = sizeof(array) / sizeof(array[0]);
+    Iter r              = std::stable_partition(Iter(array), Iter(array + size), odd_first());
     assert(base(r) == array + 4);
     assert(array[0] == P(1, 1));
     assert(array[1] == P(1, 2));
@@ -99,31 +70,19 @@ test()
     r = std::stable_partition(Iter(array), Iter(array), odd_first());
     assert(base(r) == array);
     // check one true
-    r = std::stable_partition(Iter(array), Iter(array+1), odd_first());
-    assert(base(r) == array+1);
+    r = std::stable_partition(Iter(array), Iter(array + 1), odd_first());
+    assert(base(r) == array + 1);
     assert(array[0] == P(1, 1));
     // check one false
-    r = std::stable_partition(Iter(array+4), Iter(array+5), odd_first());
-    assert(base(r) == array+4);
+    r = std::stable_partition(Iter(array + 4), Iter(array + 5), odd_first());
+    assert(base(r) == array + 4);
     assert(array[4] == P(0, 1));
   }
   { // check all false
-    typedef std::pair<int,int> P;
-    P array[] =
-    {
-        P(0, 1),
-        P(0, 2),
-        P(2, 1),
-        P(2, 2),
-        P(4, 1),
-        P(4, 2),
-        P(6, 1),
-        P(6, 2),
-        P(8, 1),
-        P(8, 2)
-    };
-    const unsigned size = sizeof(array)/sizeof(array[0]);
-    Iter r = std::stable_partition(Iter(array), Iter(array+size), odd_first());
+    typedef std::pair<int, int> P;
+    P array[]           = {P(0, 1), P(0, 2), P(2, 1), P(2, 2), P(4, 1), P(4, 2), P(6, 1), P(6, 2), P(8, 1), P(8, 2)};
+    const unsigned size = sizeof(array) / sizeof(array[0]);
+    Iter r              = std::stable_partition(Iter(array), Iter(array + size), odd_first());
     assert(base(r) == array);
     assert(array[0] == P(0, 1));
     assert(array[1] == P(0, 2));
@@ -137,22 +96,10 @@ test()
     assert(array[9] == P(8, 2));
   }
   { // check all true
-    typedef std::pair<int,int> P;
-    P array[] =
-    {
-        P(1, 1),
-        P(1, 2),
-        P(3, 1),
-        P(3, 2),
-        P(5, 1),
-        P(5, 2),
-        P(7, 1),
-        P(7, 2),
-        P(9, 1),
-        P(9, 2)
-    };
-    const unsigned size = sizeof(array)/sizeof(array[0]);
-    Iter r = std::stable_partition(Iter(array), Iter(array+size), odd_first());
+    typedef std::pair<int, int> P;
+    P array[]           = {P(1, 1), P(1, 2), P(3, 1), P(3, 2), P(5, 1), P(5, 2), P(7, 1), P(7, 2), P(9, 1), P(9, 2)};
+    const unsigned size = sizeof(array) / sizeof(array[0]);
+    Iter r              = std::stable_partition(Iter(array), Iter(array + size), odd_first());
     assert(base(r) == array + size);
     assert(array[0] == P(1, 1));
     assert(array[1] == P(1, 2));
@@ -166,22 +113,10 @@ test()
     assert(array[9] == P(9, 2));
   }
   { // check all false but first true
-    typedef std::pair<int,int> P;
-    P array[] =
-    {
-        P(1, 1),
-        P(0, 2),
-        P(2, 1),
-        P(2, 2),
-        P(4, 1),
-        P(4, 2),
-        P(6, 1),
-        P(6, 2),
-        P(8, 1),
-        P(8, 2)
-    };
-    const unsigned size = sizeof(array)/sizeof(array[0]);
-    Iter r = std::stable_partition(Iter(array), Iter(array+size), odd_first());
+    typedef std::pair<int, int> P;
+    P array[]           = {P(1, 1), P(0, 2), P(2, 1), P(2, 2), P(4, 1), P(4, 2), P(6, 1), P(6, 2), P(8, 1), P(8, 2)};
+    const unsigned size = sizeof(array) / sizeof(array[0]);
+    Iter r              = std::stable_partition(Iter(array), Iter(array + size), odd_first());
     assert(base(r) == array + 1);
     assert(array[0] == P(1, 1));
     assert(array[1] == P(0, 2));
@@ -195,22 +130,10 @@ test()
     assert(array[9] == P(8, 2));
   }
   { // check all false but last true
-    typedef std::pair<int,int> P;
-    P array[] =
-    {
-        P(0, 1),
-        P(0, 2),
-        P(2, 1),
-        P(2, 2),
-        P(4, 1),
-        P(4, 2),
-        P(6, 1),
-        P(6, 2),
-        P(8, 1),
-        P(1, 2)
-    };
-    const unsigned size = sizeof(array)/sizeof(array[0]);
-    Iter r = std::stable_partition(Iter(array), Iter(array+size), odd_first());
+    typedef std::pair<int, int> P;
+    P array[]           = {P(0, 1), P(0, 2), P(2, 1), P(2, 2), P(4, 1), P(4, 2), P(6, 1), P(6, 2), P(8, 1), P(1, 2)};
+    const unsigned size = sizeof(array) / sizeof(array[0]);
+    Iter r              = std::stable_partition(Iter(array), Iter(array + size), odd_first());
     assert(base(r) == array + 1);
     assert(array[0] == P(1, 2));
     assert(array[1] == P(0, 1));
@@ -224,23 +147,11 @@ test()
     assert(array[9] == P(8, 1));
   }
   { // check all true but first false
-    typedef std::pair<int,int> P;
-    P array[] =
-    {
-        P(0, 1),
-        P(1, 2),
-        P(3, 1),
-        P(3, 2),
-        P(5, 1),
-        P(5, 2),
-        P(7, 1),
-        P(7, 2),
-        P(9, 1),
-        P(9, 2)
-    };
-    const unsigned size = sizeof(array)/sizeof(array[0]);
-    Iter r = std::stable_partition(Iter(array), Iter(array+size), odd_first());
-    assert(base(r) == array + size-1);
+    typedef std::pair<int, int> P;
+    P array[]           = {P(0, 1), P(1, 2), P(3, 1), P(3, 2), P(5, 1), P(5, 2), P(7, 1), P(7, 2), P(9, 1), P(9, 2)};
+    const unsigned size = sizeof(array) / sizeof(array[0]);
+    Iter r              = std::stable_partition(Iter(array), Iter(array + size), odd_first());
+    assert(base(r) == array + size - 1);
     assert(array[0] == P(1, 2));
     assert(array[1] == P(3, 1));
     assert(array[2] == P(3, 2));
@@ -253,23 +164,11 @@ test()
     assert(array[9] == P(0, 1));
   }
   { // check all true but last false
-    typedef std::pair<int,int> P;
-    P array[] =
-    {
-        P(1, 1),
-        P(1, 2),
-        P(3, 1),
-        P(3, 2),
-        P(5, 1),
-        P(5, 2),
-        P(7, 1),
-        P(7, 2),
-        P(9, 1),
-        P(0, 2)
-    };
-    const unsigned size = sizeof(array)/sizeof(array[0]);
-    Iter r = std::stable_partition(Iter(array), Iter(array+size), odd_first());
-    assert(base(r) == array + size-1);
+    typedef std::pair<int, int> P;
+    P array[]           = {P(1, 1), P(1, 2), P(3, 1), P(3, 2), P(5, 1), P(5, 2), P(7, 1), P(7, 2), P(9, 1), P(0, 2)};
+    const unsigned size = sizeof(array) / sizeof(array[0]);
+    Iter r              = std::stable_partition(Iter(array), Iter(array + size), odd_first());
+    assert(base(r) == array + size - 1);
     assert(array[0] == P(1, 1));
     assert(array[1] == P(1, 2));
     assert(array[2] == P(3, 1));
@@ -282,10 +181,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.
   // 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;
@@ -306,32 +205,39 @@ test()
 
 #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;
diff --git a/libcxx/test/std/algorithms/robust_against_proxy_iterators_lifetime_bugs.pass.cpp b/libcxx/test/std/algorithms/robust_against_proxy_iterators_lifetime_bugs.pass.cpp
index c89d6f5a229e8..614c6e2510ca6 100644
--- a/libcxx/test/std/algorithms/robust_against_proxy_iterators_lifetime_bugs.pass.cpp
+++ b/libcxx/test/std/algorithms/robust_against_proxy_iterators_lifetime_bugs.pass.cpp
@@ -734,13 +734,21 @@ TEST_CONSTEXPR_CXX20 bool test() {
   if (!TEST_IS_CONSTANT_EVALUATED)
     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); });
+  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
diff --git a/libcxx/test/std/algorithms/robust_re_difference_type.compile.pass.cpp b/libcxx/test/std/algorithms/robust_re_difference_type.compile.pass.cpp
index 7d6be48430179..187113b0bdeae 100644
--- a/libcxx/test/std/algorithms/robust_re_difference_type.compile.pass.cpp
+++ b/libcxx/test/std/algorithms/robust_re_difference_type.compile.pass.cpp
@@ -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*>());
+    }
     (void)std::swap_ranges(first, last, first2);
     (void)std::transform(first, last, first2, UnaryTransform());
     (void)std::transform(first, mid, mid, first2, BinaryTransform());

@frederick-vs-ja frederick-vs-ja force-pushed the constexpr-std-stable_partition branch 2 times, most recently from bdacffe to 309cf3e Compare March 1, 2025 11:18
Copy link
Member

@mordante mordante left a comment

Choose a reason for hiding this comment

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

Thanks for working on this! I like to have another look after the formatting part has been removed from the patch.

@mordante mordante self-assigned this Mar 1, 2025
@frederick-vs-ja frederick-vs-ja force-pushed the constexpr-std-stable_partition branch from 309cf3e to 93d9167 Compare March 1, 2025 17:52
@frederick-vs-ja frederick-vs-ja force-pushed the constexpr-std-stable_partition branch from 93d9167 to 81986a6 Compare March 2, 2025 11:44
Copy link
Member

@mordante mordante left a comment

Choose a reason for hiding this comment

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

This mostly looks good, I've a few questions.

@@ -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.

Comment on lines +248 to +249
(void)std::stable_sort(first, last);
(void)std::stable_sort(first, last, std::less<void*>());
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.

Copy link
Member

@mordante mordante left a comment

Choose a reason for hiding this comment

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

LGTM!

Comment on lines +248 to +249
(void)std::stable_sort(first, last);
(void)std::stable_sort(first, last, std::less<void*>());
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.

@frederick-vs-ja frederick-vs-ja merged commit bf9bf29 into llvm:main Mar 4, 2025
86 checks passed
@frederick-vs-ja frederick-vs-ja deleted the constexpr-std-stable_partition branch March 4, 2025 01:42
jph-13 pushed a commit to jph-13/llvm-project that referenced this pull request Mar 21, 2025
…llvm#128868)

Drive-by changes:
- Enables no-memory case for Clang.
- Enables `robust_re_difference_type.compile.pass.cpp` and
`robust_against_proxy_iterators_lifetime_bugs.pass.cpp` test coverage
for `std::stable_sort` in constant evaluation since C++26. The changes
were missing in the PR making `std::stable_sort` `constexpr`.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
libc++ libc++ C++ Standard Library. Not GNU libstdc++. Not libc++abi.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

[libc++] P2562R1: stable_partition
3 participants