[libcxx-commits] [libcxx] [libc++][string] Fixes shrink_to_fit. (PR #97961)

Mark de Wever via libcxx-commits libcxx-commits at lists.llvm.org
Sat Jul 20 08:43:10 PDT 2024


================
@@ -3265,23 +3265,38 @@ basic_string<_CharT, _Traits, _Allocator>::__shrink_or_extend(size_type __target
     __p        = __get_long_pointer();
   } else {
     if (__target_capacity > __cap) {
+      // Extend
+	  // - called from reserve should propagate the exception thrown.
       auto __allocation = std::__allocate_at_least(__alloc(), __target_capacity + 1);
       __new_data        = __allocation.ptr;
       __target_capacity = __allocation.count - 1;
     } else {
+      // Shrink
+      // - called from shrink_to_fit should not throw.
+      // - called from reserve may throw but is not required to.
 #ifndef _LIBCPP_HAS_NO_EXCEPTIONS
       try {
 #endif // _LIBCPP_HAS_NO_EXCEPTIONS
         auto __allocation = std::__allocate_at_least(__alloc(), __target_capacity + 1);
+
+#ifdef _LIBCPP_HAS_NO_EXCEPTIONS
+        if (__allocation.ptr == nullptr)
+          return;
+#endif // _LIBCPP_HAS_NO_EXCEPTIONS
+
+        // The Standard mandates shrink_to_fit() does not increase the capacity.
+        // With equal capacity keep the existing buffer. This avoids extra work
+        // due to swapping the elements.
+        if (__allocation.count - 1 > __target_capacity) {
----------------
mordante wrote:

It would require quite a bit of copy pasting. Both `resize` and `shrink_to_fit` need to handle the SSO buffer part. IMO it's no issue to keep this in one function. I expect the growing buffer is very unlikely to happen with typical allocators. I also expect the shrinking `reserve` and `shrink_to_fit` are not very popular functions unless you know it will safe a considerable amount of memory.

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


More information about the libcxx-commits mailing list