[libcxx-commits] [libcxx] [libc++] Simplify the implementation of reserve() and shrink_to_fit() (PR #113453)

[Nikolas Klauser via libcxx-commits]

================
@@ -3349,70 +3371,48 @@ inline _LIBCPP_CONSTEXPR_SINCE_CXX20 void basic_string<_CharT, _Traits, _Allocat
   if (__target_capacity == capacity())
     return;
 
-  __shrink_or_extend(__target_capacity);
-}
-
-template <class _CharT, class _Traits, class _Allocator>
-inline _LIBCPP_CONSTEXPR_SINCE_CXX20 void
-basic_string<_CharT, _Traits, _Allocator>::__shrink_or_extend(size_type __target_capacity) {
-  __annotate_delete();
-  size_type __cap = capacity();
-  size_type __sz  = size();
-
-  pointer __new_data, __p;
-  bool __was_long, __now_long;
   if (__fits_in_sso(__target_capacity)) {
-    __was_long = true;
-    __now_long = false;
-    __new_data = __get_short_pointer();
-    __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.
+    if (!__is_long())
+      return;
+    __annotation_guard __g(*this);
+    auto __ptr = __get_long_pointer();
+    auto __size = __get_long_size();
+    auto __cap = __get_long_cap();
+    traits_type::copy(std::__to_address(__get_short_pointer()), data(), __size + 1);
+    __alloc_traits::deallocate(__alloc_, __ptr, __cap);
+    __set_short_size(__size);
+    return;
+  }
+
+  // Shrink
+  // - called from shrink_to_fit should not throw.
+  // - called from reserve may throw but is not required to.
 #if _LIBCPP_HAS_EXCEPTIONS
-      try {
+  try {
 #endif // _LIBCPP_HAS_EXCEPTIONS
-        auto __allocation = std::__allocate_at_least(__alloc_, __target_capacity + 1);
-
-        // 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) {
-          __alloc_traits::deallocate(__alloc_, __allocation.ptr, __allocation.count);
-          __annotate_new(__sz); // Undoes the __annotate_delete()
-          return;
-        }
-        __new_data        = __allocation.ptr;
-        __target_capacity = __allocation.count - 1;
+    __annotation_guard __g(*this);
+    auto __size = size();
+    auto __allocation = std::__allocate_at_least(__alloc_, __target_capacity + 1);
----------------
philnik777 wrote:

I'm not sure how exactly you imagined this. We don't have lambdas, so we can't wrap the call easily. The only way I see would be to declare the pointer and count and assign to them, which seems to me like it'd have more of a readability impact than the larger try/catch.

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