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

[Louis Dionne via libcxx-commits]

================
@@ -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) {
----------------
ldionne wrote:

Adding this here really assumes that we're only getting into this code path in `shrink_to_fit`. Otherwise, we may be discarding the larger block of memory we just allocated without really needing to. Would it make sense to instead rewrite `shrink_to_fit` to avoid using this `__shrink_or_extend` method which seems kinda convoluted?

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