Skip to content

[libc++] Add missed constexpr to erase(_if) in <string> #129666

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
merged 3 commits into from
Mar 5, 2025

Conversation

frederick-vs-ja
Copy link
Contributor

std::erase(_if) for basic_string were made constexpr in C++20 by cplusplus/draft@2c1ab97 as follow-up changes of P0980R1.

This patch implements the missed changes that were not tracked in a specific paper.

@frederick-vs-ja frederick-vs-ja requested a review from a team as a code owner March 4, 2025 08:00
@llvmbot llvmbot added the libc++ libc++ C++ Standard Library. Not GNU libstdc++. Not libc++abi. label Mar 4, 2025
@llvmbot
Copy link
Member

llvmbot commented Mar 4, 2025

@llvm/pr-subscribers-libcxx

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

Changes

std::erase(_if) for basic_string were made constexpr in C++20 by cplusplus/draft@2c1ab9775cc53e848a1efff4f9976455538994d4 as follow-up changes of P0980R1.

This patch implements the missed changes that were not tracked in a specific paper.


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

4 Files Affected:

  • (modified) libcxx/docs/ReleaseNotes/21.rst (+2)
  • (modified) libcxx/include/string (+4-4)
  • (modified) libcxx/test/std/strings/strings.erasure/erase.pass.cpp (+11-4)
  • (modified) libcxx/test/std/strings/strings.erasure/erase_if.pass.cpp (+11-4)
diff --git a/libcxx/docs/ReleaseNotes/21.rst b/libcxx/docs/ReleaseNotes/21.rst
index e7cfa625a132c..663514ce36681 100644
--- a/libcxx/docs/ReleaseNotes/21.rst
+++ b/libcxx/docs/ReleaseNotes/21.rst
@@ -52,6 +52,8 @@ Improvements and New Features
 
 - Updated formatting library to Unicode 16.0.0.
 
+- ``std::erase`` and ``std::erase_if`` overloads for ``std::basic_string`` are made ``constexpr`` in C++20 and later.
+
 Deprecations and Removals
 -------------------------
 
diff --git a/libcxx/include/string b/libcxx/include/string
index 419e3eee6746e..8cbd859e70e73 100644
--- a/libcxx/include/string
+++ b/libcxx/include/string
@@ -516,10 +516,10 @@ basic_istream<charT, traits>&
 getline(basic_istream<charT, traits>& is, basic_string<charT, traits, Allocator>& str);
 
 template<class charT, class traits, class Allocator, class U>
-typename basic_string<charT, traits, Allocator>::size_type
+constexpr typename basic_string<charT, traits, Allocator>::size_type
 erase(basic_string<charT, traits, Allocator>& c, const U& value);    // C++20
 template<class charT, class traits, class Allocator, class Predicate>
-typename basic_string<charT, traits, Allocator>::size_type
+constexpr typename basic_string<charT, traits, Allocator>::size_type
 erase_if(basic_string<charT, traits, Allocator>& c, Predicate pred); // C++20
 
 typedef basic_string<char>    string;
@@ -4022,7 +4022,7 @@ getline(basic_istream<_CharT, _Traits>&& __is, basic_string<_CharT, _Traits, _Al
 
 #  if _LIBCPP_STD_VER >= 20
 template <class _CharT, class _Traits, class _Allocator, class _Up>
-inline _LIBCPP_HIDE_FROM_ABI typename basic_string<_CharT, _Traits, _Allocator>::size_type
+_LIBCPP_HIDE_FROM_ABI inline constexpr typename basic_string<_CharT, _Traits, _Allocator>::size_type
 erase(basic_string<_CharT, _Traits, _Allocator>& __str, const _Up& __v) {
   auto __old_size = __str.size();
   __str.erase(std::remove(__str.begin(), __str.end(), __v), __str.end());
@@ -4030,7 +4030,7 @@ erase(basic_string<_CharT, _Traits, _Allocator>& __str, const _Up& __v) {
 }
 
 template <class _CharT, class _Traits, class _Allocator, class _Predicate>
-inline _LIBCPP_HIDE_FROM_ABI typename basic_string<_CharT, _Traits, _Allocator>::size_type
+_LIBCPP_HIDE_FROM_ABI inline constexpr typename basic_string<_CharT, _Traits, _Allocator>::size_type
 erase_if(basic_string<_CharT, _Traits, _Allocator>& __str, _Predicate __pred) {
   auto __old_size = __str.size();
   __str.erase(std::remove_if(__str.begin(), __str.end(), __pred), __str.end());
diff --git a/libcxx/test/std/strings/strings.erasure/erase.pass.cpp b/libcxx/test/std/strings/strings.erasure/erase.pass.cpp
index 35139dd6ad407..a5a542b40c52a 100644
--- a/libcxx/test/std/strings/strings.erasure/erase.pass.cpp
+++ b/libcxx/test/std/strings/strings.erasure/erase.pass.cpp
@@ -11,7 +11,7 @@
 // <string>
 
 // template <class charT, class traits, class Allocator, class U>
-//   typename basic_string<charT, traits, Allocator>::size_type
+//   constexpr typename basic_string<charT, traits, Allocator>::size_type
 //   erase(basic_string<charT, traits, Allocator>& c, const U& value);
 
 #include <string>
@@ -22,7 +22,7 @@
 #include "min_allocator.h"
 
 template <class S, class U>
-void test0(S s, U val, S expected, std::size_t expected_erased_count) {
+constexpr void test0(S s, U val, S expected, std::size_t expected_erased_count) {
   ASSERT_SAME_TYPE(typename S::size_type, decltype(std::erase(s, val)));
   assert(expected_erased_count == std::erase(s, val));
   LIBCPP_ASSERT(s.__invariants());
@@ -30,7 +30,7 @@ void test0(S s, U val, S expected, std::size_t expected_erased_count) {
 }
 
 template <class S>
-void test() {
+constexpr void test() {
   test0(S(""), 'a', S(""), 0);
 
   test0(S("a"), 'a', S(""), 1);
@@ -64,10 +64,17 @@ void test() {
   test0(S("aba"), opt('c'), S("aba"), 0);
 }
 
-int main(int, char**) {
+constexpr bool test() {
   test<std::string>();
   test<std::basic_string<char, std::char_traits<char>, min_allocator<char>>>();
   test<std::basic_string<char, std::char_traits<char>, test_allocator<char>>>();
 
+  return true;
+}
+
+int main(int, char**) {
+  test();
+  static_assert(test());
+
   return 0;
 }
diff --git a/libcxx/test/std/strings/strings.erasure/erase_if.pass.cpp b/libcxx/test/std/strings/strings.erasure/erase_if.pass.cpp
index 5bedd394ee578..0f1c3fcf675eb 100644
--- a/libcxx/test/std/strings/strings.erasure/erase_if.pass.cpp
+++ b/libcxx/test/std/strings/strings.erasure/erase_if.pass.cpp
@@ -11,7 +11,7 @@
 // <string>
 
 // template <class charT, class traits, class Allocator, class Predicate>
-//   typename basic_string<charT, traits, Allocator>::size_type
+//   constexpr typename basic_string<charT, traits, Allocator>::size_type
 //   erase_if(basic_string<charT, traits, Allocator>& c, Predicate pred);
 
 #include <string>
@@ -21,7 +21,7 @@
 #include "min_allocator.h"
 
 template <class S, class Pred>
-void test0(S s, Pred p, S expected, std::size_t expected_erased_count) {
+constexpr void test0(S s, Pred p, S expected, std::size_t expected_erased_count) {
   ASSERT_SAME_TYPE(typename S::size_type, decltype(std::erase_if(s, p)));
   assert(expected_erased_count == std::erase_if(s, p));
   LIBCPP_ASSERT(s.__invariants());
@@ -29,7 +29,7 @@ void test0(S s, Pred p, S expected, std::size_t expected_erased_count) {
 }
 
 template <typename S>
-void test() {
+constexpr void test() {
   auto isA   = [](auto ch) { return ch == 'a'; };
   auto isB   = [](auto ch) { return ch == 'b'; };
   auto isC   = [](auto ch) { return ch == 'c'; };
@@ -66,10 +66,17 @@ void test() {
   test0(S("aba"), True, S(""), 3);
 }
 
-int main(int, char**) {
+constexpr bool test() {
   test<std::string>();
   test<std::basic_string<char, std::char_traits<char>, min_allocator<char>>>();
   test<std::basic_string<char, std::char_traits<char>, test_allocator<char>>>();
 
+  return true;
+}
+
+int main(int, char**) {
+  test();
+  static_assert(test());
+
   return 0;
 }

`std::erase(_if)` for `basic_string` were made `constexpr` in C++20 by
cplusplus/draft@2c1ab97 as follow-up
changes of P0980R1.

This patch implements the missed changes that were not tracked in a
specific paper.
@frederick-vs-ja frederick-vs-ja force-pushed the constexpr-string-erase branch from bf96e93 to 073d679 Compare March 4, 2025 09:14
@frederick-vs-ja frederick-vs-ja merged commit e739ce2 into llvm:main Mar 5, 2025
83 checks passed
@llvm-ci
Copy link
Collaborator

llvm-ci commented Mar 5, 2025

LLVM Buildbot has detected a new failure on builder sanitizer-x86_64-linux-android running on sanitizer-buildbot-android while building libcxx at step 2 "annotate".

Full details are available at: https://lab.llvm.org/buildbot/#/builders/186/builds/7039

Here is the relevant piece of the build log for the reference
Step 2 (annotate) failure: 'python ../sanitizer_buildbot/sanitizers/zorg/buildbot/builders/sanitizers/buildbot_selector.py' (failure)
...
[       OK ] AddressSanitizer.AtoiAndFriendsOOBTest (2223 ms)
[ RUN      ] AddressSanitizer.HasFeatureAddressSanitizerTest
[       OK ] AddressSanitizer.HasFeatureAddressSanitizerTest (0 ms)
[ RUN      ] AddressSanitizer.CallocReturnsZeroMem
[       OK ] AddressSanitizer.CallocReturnsZeroMem (14 ms)
[ DISABLED ] AddressSanitizer.DISABLED_TSDTest
[ RUN      ] AddressSanitizer.IgnoreTest
[       OK ] AddressSanitizer.IgnoreTest (0 ms)
[ RUN      ] AddressSanitizer.SignalTest
[       OK ] AddressSanitizer.SignalTest (217 ms)
[ RUN      ] AddressSanitizer.ReallocTest
[       OK ] AddressSanitizer.ReallocTest (40 ms)
[ RUN      ] AddressSanitizer.WrongFreeTest
[       OK ] AddressSanitizer.WrongFreeTest (133 ms)
[ RUN      ] AddressSanitizer.LongJmpTest
[       OK ] AddressSanitizer.LongJmpTest (0 ms)
[ RUN      ] AddressSanitizer.ThreadStackReuseTest
[       OK ] AddressSanitizer.ThreadStackReuseTest (2 ms)
[ DISABLED ] AddressSanitizer.DISABLED_MemIntrinsicUnalignedAccessTest
[ DISABLED ] AddressSanitizer.DISABLED_LargeFunctionSymbolizeTest
[ DISABLED ] AddressSanitizer.DISABLED_MallocFreeUnwindAndSymbolizeTest
[ RUN      ] AddressSanitizer.UseThenFreeThenUseTest
[       OK ] AddressSanitizer.UseThenFreeThenUseTest (125 ms)
[ RUN      ] AddressSanitizer.FileNameInGlobalReportTest
[       OK ] AddressSanitizer.FileNameInGlobalReportTest (111 ms)
[ DISABLED ] AddressSanitizer.DISABLED_StressStackReuseAndExceptionsTest
[ RUN      ] AddressSanitizer.MlockTest
[       OK ] AddressSanitizer.MlockTest (0 ms)
[ DISABLED ] AddressSanitizer.DISABLED_DemoThreadedTest
[ DISABLED ] AddressSanitizer.DISABLED_DemoStackTest
[ DISABLED ] AddressSanitizer.DISABLED_DemoThreadStackTest
[ DISABLED ] AddressSanitizer.DISABLED_DemoUAFLowIn
[ DISABLED ] AddressSanitizer.DISABLED_DemoUAFLowLeft
[ DISABLED ] AddressSanitizer.DISABLED_DemoUAFLowRight
[ DISABLED ] AddressSanitizer.DISABLED_DemoUAFHigh
[ DISABLED ] AddressSanitizer.DISABLED_DemoOOM
[ DISABLED ] AddressSanitizer.DISABLED_DemoDoubleFreeTest
[ DISABLED ] AddressSanitizer.DISABLED_DemoNullDerefTest
[ DISABLED ] AddressSanitizer.DISABLED_DemoFunctionStaticTest
[ DISABLED ] AddressSanitizer.DISABLED_DemoTooMuchMemoryTest
[ RUN      ] AddressSanitizer.LongDoubleNegativeTest
[       OK ] AddressSanitizer.LongDoubleNegativeTest (0 ms)
[----------] 19 tests from AddressSanitizer (28050 ms total)

[----------] Global test environment tear-down
[==========] 22 tests from 2 test suites ran. (28054 ms total)
[  PASSED  ] 22 tests.

  YOU HAVE 1 DISABLED TEST

Step 34 (run instrumented asan tests [aarch64/bluejay-userdebug/TQ3A.230805.001]) failure: run instrumented asan tests [aarch64/bluejay-userdebug/TQ3A.230805.001] (failure)
...
[ RUN      ] AddressSanitizer.HasFeatureAddressSanitizerTest
[       OK ] AddressSanitizer.HasFeatureAddressSanitizerTest (0 ms)
[ RUN      ] AddressSanitizer.CallocReturnsZeroMem
[       OK ] AddressSanitizer.CallocReturnsZeroMem (14 ms)
[ DISABLED ] AddressSanitizer.DISABLED_TSDTest
[ RUN      ] AddressSanitizer.IgnoreTest
[       OK ] AddressSanitizer.IgnoreTest (0 ms)
[ RUN      ] AddressSanitizer.SignalTest
[       OK ] AddressSanitizer.SignalTest (217 ms)
[ RUN      ] AddressSanitizer.ReallocTest
[       OK ] AddressSanitizer.ReallocTest (40 ms)
[ RUN      ] AddressSanitizer.WrongFreeTest
[       OK ] AddressSanitizer.WrongFreeTest (133 ms)
[ RUN      ] AddressSanitizer.LongJmpTest
[       OK ] AddressSanitizer.LongJmpTest (0 ms)
[ RUN      ] AddressSanitizer.ThreadStackReuseTest
[       OK ] AddressSanitizer.ThreadStackReuseTest (2 ms)
[ DISABLED ] AddressSanitizer.DISABLED_MemIntrinsicUnalignedAccessTest
[ DISABLED ] AddressSanitizer.DISABLED_LargeFunctionSymbolizeTest
[ DISABLED ] AddressSanitizer.DISABLED_MallocFreeUnwindAndSymbolizeTest
[ RUN      ] AddressSanitizer.UseThenFreeThenUseTest
[       OK ] AddressSanitizer.UseThenFreeThenUseTest (125 ms)
[ RUN      ] AddressSanitizer.FileNameInGlobalReportTest
[       OK ] AddressSanitizer.FileNameInGlobalReportTest (111 ms)
[ DISABLED ] AddressSanitizer.DISABLED_StressStackReuseAndExceptionsTest
[ RUN      ] AddressSanitizer.MlockTest
[       OK ] AddressSanitizer.MlockTest (0 ms)
[ DISABLED ] AddressSanitizer.DISABLED_DemoThreadedTest
[ DISABLED ] AddressSanitizer.DISABLED_DemoStackTest
[ DISABLED ] AddressSanitizer.DISABLED_DemoThreadStackTest
[ DISABLED ] AddressSanitizer.DISABLED_DemoUAFLowIn
[ DISABLED ] AddressSanitizer.DISABLED_DemoUAFLowLeft
[ DISABLED ] AddressSanitizer.DISABLED_DemoUAFLowRight
[ DISABLED ] AddressSanitizer.DISABLED_DemoUAFHigh
[ DISABLED ] AddressSanitizer.DISABLED_DemoOOM
[ DISABLED ] AddressSanitizer.DISABLED_DemoDoubleFreeTest
[ DISABLED ] AddressSanitizer.DISABLED_DemoNullDerefTest
[ DISABLED ] AddressSanitizer.DISABLED_DemoFunctionStaticTest
[ DISABLED ] AddressSanitizer.DISABLED_DemoTooMuchMemoryTest
[ RUN      ] AddressSanitizer.LongDoubleNegativeTest
[       OK ] AddressSanitizer.LongDoubleNegativeTest (0 ms)
[----------] 19 tests from AddressSanitizer (28050 ms total)

[----------] Global test environment tear-down
[==========] 22 tests from 2 test suites ran. (28054 ms total)
[  PASSED  ] 22 tests.

  YOU HAVE 1 DISABLED TEST
program finished with exit code 0
elapsedTime=2293.832926

@frederick-vs-ja frederick-vs-ja deleted the constexpr-string-erase branch March 5, 2025 01:43
jph-13 pushed a commit to jph-13/llvm-project that referenced this pull request Mar 21, 2025
…29666)

`std::erase(_if)` for `basic_string` were made `constexpr` in C++20 by
cplusplus/draft@2c1ab97 as follow-up
changes of P0980R1.

This patch implements the missed changes that were not tracked in a
specific paper.
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.

4 participants