[libcxx-commits] [libcxx] [libc++] Fix shrink_to_fit to swap buffer only when capacity is strictly smaller (PR #127321)

[Nikolas Klauser via libcxx-commits]

================
@@ -61,28 +61,52 @@ TEST_CONSTEXPR_CXX20 bool test() {
   test_string<std::basic_string<char, std::char_traits<char>, safe_allocator<char>>>();
 #endif
 
-  return true;
-}
-
 #if TEST_STD_VER >= 23
-// https://github.com/llvm/llvm-project/issues/95161
-void test_increasing_allocator() {
-  std::basic_string<char, std::char_traits<char>, increasing_allocator<char>> s{
-      "String does not fit in the internal buffer"};
-  std::size_t capacity = s.capacity();
-  std::size_t size     = s.size();
-  s.shrink_to_fit();
-  assert(s.capacity() <= capacity);
-  assert(s.size() == size);
-  LIBCPP_ASSERT(is_string_asan_correct(s));
+  { // Make sure shrink_to_fit never increases capacity
+    // See: https://github.com/llvm/llvm-project/issues/95161
+    std::basic_string<char, std::char_traits<char>, increasing_allocator<char>> s{
+        "String does not fit in the internal buffer"};
+    std::size_t capacity = s.capacity();
+    std::size_t size     = s.size();
+    s.shrink_to_fit();
+    assert(s.capacity() <= capacity);
+    assert(s.size() == size);
+    LIBCPP_ASSERT(is_string_asan_correct(s));
+  }
+#endif
+
+  {   // Ensure that the libc++ implementation of shrink_to_fit does NOT swap buffer with equal allocation sizes
+    { // Test with custom allocator with a minimum allocation size
+      std::basic_string<char, std::char_traits<char>, min_size_allocator<128, char> > s(
+          "A long string exceeding SSO limit but within min alloc size");
+      std::size_t capacity = s.capacity();
+      std::size_t size     = s.size();
+      auto data            = s.data();
+      s.shrink_to_fit();
+      assert(s.capacity() <= capacity);
+      assert(s.size() == size);
+      LIBCPP_ASSERT(is_string_asan_correct(s));
+      LIBCPP_ASSERT(s.capacity() == capacity && s.data() == data);
+    }
+    { // Test with custom allocator with a minimum power of two allocation size
+      std::basic_string<char, std::char_traits<char>, pow2_allocator<char> > s(
+          "This is a long string that exceeds the SSO limit");
+      std::size_t capacity = s.capacity();
+      std::size_t size     = s.size();
+      auto data            = s.data();
+      s.shrink_to_fit();
+      assert(s.capacity() <= capacity);
+      assert(s.size() == size);
+      LIBCPP_ASSERT(is_string_asan_correct(s));
+      LIBCPP_ASSERT(s.capacity() == capacity && s.data() == data);
+    }
+  }
----------------
philnik777 wrote:

Let's move these tests to `libcxx/test/libcxx/strings/basic.string/string.capacity/shrink_to_fit.pass.cpp` and replace the `LIBCPP_ASSERT` with normal `assert`s. I don't think we're really adding coverage for anything that's not libc++.

https://github.com/llvm/llvm-project/pull/127321