Skip to content

Conversation

@philnik777
Copy link
Contributor

__clear_and_shrink is only used in a single place and does more work than actually required.

@philnik777 philnik777 requested a review from a team as a code owner February 6, 2025 11:27
@llvmbot llvmbot added the libc++ libc++ C++ Standard Library. Not GNU libstdc++. Not libc++abi. label Feb 6, 2025
@llvmbot
Copy link
Member

llvmbot commented Feb 6, 2025

@llvm/pr-subscribers-libcxx

Author: Nikolas Klauser (philnik777)

Changes

__clear_and_shrink is only used in a single place and does more work than actually required.


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

2 Files Affected:

  • (modified) libcxx/include/string (+5-15)
  • (removed) libcxx/test/libcxx/strings/basic.string/string.modifiers/clear_and_shrink.pass.cpp (-44)
diff --git a/libcxx/include/string b/libcxx/include/string
index b7f2d1226946392..11af160c0f8ff47 100644
--- a/libcxx/include/string
+++ b/libcxx/include/string
@@ -1879,8 +1879,6 @@ public:
 
   _LIBCPP_HIDE_FROM_ABI _LIBCPP_CONSTEXPR_SINCE_CXX20 bool __invariants() const;
 
-  _LIBCPP_HIDE_FROM_ABI _LIBCPP_CONSTEXPR_SINCE_CXX20 void __clear_and_shrink() _NOEXCEPT;
-
 private:
   _LIBCPP_HIDE_FROM_ABI _LIBCPP_CONSTEXPR_SINCE_CXX20 _LIBCPP_STRING_INTERNAL_MEMORY_ACCESS bool
   __is_long() const _NOEXCEPT {
@@ -2189,7 +2187,11 @@ private:
       __alloc_ = __str.__alloc_;
     else {
       if (!__str.__is_long()) {
-        __clear_and_shrink();
+        if (__is_long()) {
+          __annotate_delete();
+          __alloc_traits::deallocate(__alloc_, __get_long_pointer(), capacity() + 1);
+          __rep_ = __rep();
+        }
         __alloc_ = __str.__alloc_;
       } else {
         __annotate_delete();
@@ -3820,18 +3822,6 @@ inline _LIBCPP_CONSTEXPR_SINCE_CXX20 bool basic_string<_CharT, _Traits, _Allocat
   return true;
 }
 
-// __clear_and_shrink
-
-template <class _CharT, class _Traits, class _Allocator>
-inline _LIBCPP_CONSTEXPR_SINCE_CXX20 void basic_string<_CharT, _Traits, _Allocator>::__clear_and_shrink() _NOEXCEPT {
-  clear();
-  if (__is_long()) {
-    __annotate_delete();
-    __alloc_traits::deallocate(__alloc_, __get_long_pointer(), capacity() + 1);
-    __rep_ = __rep();
-  }
-}
-
 // operator==
 
 template <class _CharT, class _Traits, class _Allocator>
diff --git a/libcxx/test/libcxx/strings/basic.string/string.modifiers/clear_and_shrink.pass.cpp b/libcxx/test/libcxx/strings/basic.string/string.modifiers/clear_and_shrink.pass.cpp
deleted file mode 100644
index 8f4c9102fde87d7..000000000000000
--- a/libcxx/test/libcxx/strings/basic.string/string.modifiers/clear_and_shrink.pass.cpp
+++ /dev/null
@@ -1,44 +0,0 @@
-//===----------------------------------------------------------------------===//
-//
-// Part of the LLVM Project, under the Apache License v2.0 with LLVM Exceptions.
-// See https://llvm.org/LICENSE.txt for license information.
-// SPDX-License-Identifier: Apache-2.0 WITH LLVM-exception
-//
-//===----------------------------------------------------------------------===//
-
-// <string>
-
-// Call __clear_and_shrink() and ensure string invariants hold
-
-#include <string>
-#include <cassert>
-
-#include "test_macros.h"
-
-TEST_CONSTEXPR_CXX20 bool test() {
-  std::string l = "Long string so that allocation definitely, for sure, absolutely happens. Probably.";
-  std::string s = "short";
-
-  assert(l.__invariants());
-  assert(s.__invariants());
-
-  s.__clear_and_shrink();
-  assert(s.__invariants());
-  assert(s.size() == 0);
-
-  std::string::size_type cap = l.capacity();
-  l.__clear_and_shrink();
-  assert(l.__invariants());
-  assert(l.size() == 0);
-  assert(l.capacity() < cap);
-
-  return true;
-}
-
-int main(int, char**) {
-  test();
-#if TEST_STD_VER > 17
-  static_assert(test());
-#endif
-  return 0;
-}

@ldionne ldionne added the pending-ci Merging the PR is only pending completion of CI label Feb 6, 2025
@philnik777
Copy link
Contributor Author

The buildkite failure is the Andrdoid CI being broken again and the mingw failure is due to some server being unavailable. Merging.

@philnik777 philnik777 merged commit ee3bcca into llvm:main Feb 6, 2025
75 of 80 checks passed
@philnik777 philnik777 deleted the remove_clear_and_shrink branch February 6, 2025 13:07
Icohedron pushed a commit to Icohedron/llvm-project that referenced this pull request Feb 11, 2025
`__clear_and_shrink` is only used in a single place and does more work
than actually required.
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. pending-ci Merging the PR is only pending completion of CI

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants