[libcxx-commits] [libcxx] [libc++][NFC] Simplify string a bit (PR #127135)

Peng Liu via libcxx-commits libcxx-commits at lists.llvm.org
Mon Feb 24 17:45:02 PST 2025


https://github.com/winner245 updated https://github.com/llvm/llvm-project/pull/127135

>From 596189ec32196d03b94d485c07244f3a5fd8f47e Mon Sep 17 00:00:00 2001
From: Peng Liu <winner245 at hotmail.com>
Date: Thu, 13 Feb 2025 17:03:18 -0500
Subject: [PATCH 1/2] Simplify string a bit

---
 libcxx/include/string | 87 ++++++++++++++++++-------------------------
 1 file changed, 36 insertions(+), 51 deletions(-)

diff --git a/libcxx/include/string b/libcxx/include/string
index 7b86bbe5bffa7..01d8c40edb95b 100644
--- a/libcxx/include/string
+++ b/libcxx/include/string
@@ -235,9 +235,9 @@ public:
     template <class T>
         basic_string& insert(size_type pos1, const T& t);                                       // constexpr since C++20
     basic_string& insert(size_type pos1, const basic_string& str,
-                         size_type pos2, size_type n);                                          // constexpr since C++20
+                         size_type pos2, size_type n2=npos);                                    // constexpr since C++20
     template <class T>
-        basic_string& insert(size_type pos1, const T& t, size_type pos2, size_type n);          // C++17, constexpr since C++20
+        basic_string& insert(size_type pos1, const T& t, size_type pos2, size_type n=npos);     // C++17, constexpr since C++20
     basic_string& insert(size_type pos, const value_type* s, size_type n=npos);                 // C++14, constexpr since C++20
     basic_string& insert(size_type pos, const value_type* s);                                   // constexpr since C++20
     basic_string& insert(size_type pos, size_type n, value_type c);                             // constexpr since C++20
@@ -260,7 +260,7 @@ public:
                           size_type pos2, size_type n2=npos);                                   // C++14, constexpr since C++20
     template <class T>
         basic_string& replace(size_type pos1, size_type n1, const T& t,
-                              size_type pos2, size_type n);                                     // C++17, constexpr since C++20
+                              size_type pos2, size_type n2=npos);                               // C++17, constexpr since C++20
     basic_string& replace(size_type pos, size_type n1, const value_type* s, size_type n2);      // constexpr since C++20
     basic_string& replace(size_type pos, size_type n1, const value_type* s);                    // constexpr since C++20
     basic_string& replace(size_type pos, size_type n1, size_type n2, value_type c);             // constexpr since C++20
@@ -2303,7 +2303,7 @@ private:
       if (!__str.__is_long()) {
         if (__is_long()) {
           __annotate_delete();
-          __alloc_traits::deallocate(__alloc_, __get_long_pointer(), capacity() + 1);
+          __alloc_traits::deallocate(__alloc_, __get_long_pointer(), __get_long_cap());
           __rep_ = __rep();
         }
         __alloc_ = __str.__alloc_;
@@ -2318,7 +2318,7 @@ private:
         __alloc_ = std::move(__a);
         __set_long_pointer(__allocation.ptr);
         __set_long_cap(__allocation.count);
-        __set_long_size(__str.size());
+        __set_long_size(__str.__get_long_size());
       }
     }
   }
@@ -2437,8 +2437,8 @@ template <class _CharT,
           class _Traits,
           class _Allocator = allocator<_CharT>,
           class            = enable_if_t<__is_allocator<_Allocator>::value> >
-explicit basic_string(basic_string_view<_CharT, _Traits>,
-                      const _Allocator& = _Allocator()) -> basic_string<_CharT, _Traits, _Allocator>;
+explicit basic_string(basic_string_view<_CharT, _Traits>, const _Allocator& = _Allocator())
+    -> basic_string<_CharT, _Traits, _Allocator>;
 
 template <class _CharT,
           class _Traits,
@@ -2701,20 +2701,19 @@ template <class _CharT, class _Traits, class _Allocator>
 template <bool __is_short>
 _LIBCPP_CONSTEXPR_SINCE_CXX20 _LIBCPP_NOINLINE basic_string<_CharT, _Traits, _Allocator>&
 basic_string<_CharT, _Traits, _Allocator>::__assign_no_alias(const value_type* __s, size_type __n) {
-  size_type __cap = __is_short ? static_cast<size_type>(__min_cap) : __get_long_cap();
-  if (__n < __cap) {
-    size_type __old_size = __is_short ? __get_short_size() : __get_long_size();
+  size_type __cap      = capacity();
+  size_type __old_size = size();
+  if (__n <= __cap) {
     if (__n > __old_size)
       __annotate_increase(__n - __old_size);
-    pointer __p = __is_short ? __get_short_pointer() : __get_long_pointer();
-    __is_short ? __set_short_size(__n) : __set_long_size(__n);
+    pointer __p =
+        __is_short ? (__set_short_size(__n), __get_short_pointer()) : (__set_long_size(__n), __get_long_pointer());
     traits_type::copy(std::__to_address(__p), __s, __n);
     traits_type::assign(__p[__n], value_type());
     if (__old_size > __n)
       __annotate_shrink(__old_size);
   } else {
-    size_type __sz = __is_short ? __get_short_size() : __get_long_size();
-    __grow_by_and_replace(__cap - 1, __n - __cap + 1, __sz, 0, __sz, __n, __s);
+    __grow_by_and_replace(__cap, __n - __cap, __old_size, 0, __old_size, __n, __s);
   }
   return *this;
 }
@@ -2722,17 +2721,16 @@ basic_string<_CharT, _Traits, _Allocator>::__assign_no_alias(const value_type* _
 template <class _CharT, class _Traits, class _Allocator>
 _LIBCPP_CONSTEXPR_SINCE_CXX20 _LIBCPP_NOINLINE basic_string<_CharT, _Traits, _Allocator>&
 basic_string<_CharT, _Traits, _Allocator>::__assign_external(const value_type* __s, size_type __n) {
-  size_type __cap = capacity();
+  size_type __cap      = capacity();
+  size_type __old_size = size();
   if (__cap >= __n) {
-    size_type __old_size = size();
     if (__n > __old_size)
       __annotate_increase(__n - __old_size);
     value_type* __p = std::__to_address(__get_pointer());
     traits_type::move(__p, __s, __n);
     return __null_terminate_at(__p, __n);
   } else {
-    size_type __sz = size();
-    __grow_by_and_replace(__cap, __n - __cap, __sz, 0, __sz, __n, __s);
+    __grow_by_and_replace(__cap, __n - __cap, __old_size, 0, __old_size, __n, __s);
     return *this;
   }
 }
@@ -2750,8 +2748,7 @@ basic_string<_CharT, _Traits, _Allocator>::assign(size_type __n, value_type __c)
   size_type __cap      = capacity();
   size_type __old_size = size();
   if (__cap < __n) {
-    size_type __sz = size();
-    __grow_by_without_replace(__cap, __n - __cap, __sz, 0, __sz);
+    __grow_by_without_replace(__cap, __n - __cap, __old_size, 0, __old_size);
     __annotate_increase(__n);
   } else if (__n > __old_size)
     __annotate_increase(__n - __old_size);
@@ -2763,17 +2760,10 @@ basic_string<_CharT, _Traits, _Allocator>::assign(size_type __n, value_type __c)
 template <class _CharT, class _Traits, class _Allocator>
 _LIBCPP_CONSTEXPR_SINCE_CXX20 basic_string<_CharT, _Traits, _Allocator>&
 basic_string<_CharT, _Traits, _Allocator>::operator=(value_type __c) {
-  pointer __p;
   size_type __old_size = size();
   if (__old_size == 0)
     __annotate_increase(1);
-  if (__is_long()) {
-    __p = __get_long_pointer();
-    __set_long_size(1);
-  } else {
-    __p = __get_short_pointer();
-    __set_short_size(1);
-  }
+  pointer __p = __is_long() ? (__set_long_size(1), __get_long_pointer()) : (__set_short_size(1), __get_short_pointer());
   traits_type::assign(*__p, __c);
   traits_type::assign(*++__p, value_type());
   if (__old_size > 1)
@@ -2789,8 +2779,8 @@ basic_string<_CharT, _Traits, _Allocator>::operator=(const basic_string& __str)
     if (!__is_long()) {
       if (!__str.__is_long()) {
         size_type __old_size = __get_short_size();
-        if (__get_short_size() < __str.__get_short_size())
-          __annotate_increase(__str.__get_short_size() - __get_short_size());
+        if (__old_size < __str.__get_short_size())
+          __annotate_increase(__str.__get_short_size() - __old_size);
         __rep_ = __str.__rep_;
         if (__old_size > __get_short_size())
           __annotate_shrink(__old_size);
@@ -2957,10 +2947,11 @@ template <class _CharT, class _Traits, class _Allocator>
 _LIBCPP_CONSTEXPR_SINCE_CXX20 basic_string<_CharT, _Traits, _Allocator>&
 basic_string<_CharT, _Traits, _Allocator>::assign(const value_type* __s) {
   _LIBCPP_ASSERT_NON_NULL(__s != nullptr, "string::assign received nullptr");
-  return __builtin_constant_p(*__s)
-           ? (__fits_in_sso(traits_type::length(__s)) ? __assign_short(__s, traits_type::length(__s))
-                                                      : __assign_external(__s, traits_type::length(__s)))
-           : __assign_external(__s);
+  if (__builtin_constant_p(*__s)) {
+    const size_type __n = traits_type::length(__s);
+    return __fits_in_sso(__n) ? __assign_short(__s, __n) : __assign_external(__s, __n);
+  }
+  return __assign_external(__s);
 }
 // append
 
@@ -3032,18 +3023,11 @@ _LIBCPP_CONSTEXPR_SINCE_CXX20 void basic_string<_CharT, _Traits, _Allocator>::pu
   }
   if (__sz == __cap) {
     __grow_by_without_replace(__cap, 1, __sz, __sz, 0);
-    __annotate_increase(1);
     __is_short = false; // the string is always long after __grow_by
-  } else
-    __annotate_increase(1);
-  pointer __p = __get_pointer();
-  if (__is_short) {
-    __p = __get_short_pointer() + __sz;
-    __set_short_size(__sz + 1);
-  } else {
-    __p = __get_long_pointer() + __sz;
-    __set_long_size(__sz + 1);
   }
+  __annotate_increase(1);
+  pointer __p = __is_short ? (__set_short_size(__sz + 1), __get_short_pointer() + __sz)
+                           : (__set_long_size(__sz + 1), __get_long_pointer() + __sz);
   traits_type::assign(*__p, __c);
   traits_type::assign(*++__p, value_type());
 }
@@ -3420,11 +3404,13 @@ inline _LIBCPP_CONSTEXPR_SINCE_CXX20 void basic_string<_CharT, _Traits, _Allocat
 
 template <class _CharT, class _Traits, class _Allocator>
 inline _LIBCPP_CONSTEXPR_SINCE_CXX20 void basic_string<_CharT, _Traits, _Allocator>::clear() _NOEXCEPT {
-  size_type __old_size = size();
+  size_type __old_size;
   if (__is_long()) {
+    __old_size = __get_long_size();
     traits_type::assign(*__get_long_pointer(), value_type());
     __set_long_size(0);
   } else {
+    __old_size = __get_short_size();
     traits_type::assign(*__get_short_pointer(), value_type());
     __set_short_size(0);
   }
@@ -3482,11 +3468,12 @@ inline _LIBCPP_CONSTEXPR_SINCE_CXX20 void basic_string<_CharT, _Traits, _Allocat
   _LIBCPP_ASSERT_INTERNAL(__is_long(), "Trying to shrink small string");
 
   // We're a long string and we're shrinking into the small buffer.
+  auto __ptr  = __get_long_pointer();
+  auto __size = __get_long_size();
+  auto __cap  = __get_long_cap();
+
   if (__fits_in_sso(__target_capacity)) {
     __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()), std::__to_address(__ptr), __size + 1);
     __set_short_size(__size);
     __alloc_traits::deallocate(__alloc_, __ptr, __cap);
@@ -3497,7 +3484,6 @@ inline _LIBCPP_CONSTEXPR_SINCE_CXX20 void basic_string<_CharT, _Traits, _Allocat
   try {
 #  endif // _LIBCPP_HAS_EXCEPTIONS
     __annotation_guard __g(*this);
-    auto __size       = size();
     auto __allocation = std::__allocate_at_least(__alloc_, __target_capacity + 1);
 
     // The Standard mandates shrink_to_fit() does not increase the capacity.
@@ -3509,9 +3495,8 @@ inline _LIBCPP_CONSTEXPR_SINCE_CXX20 void basic_string<_CharT, _Traits, _Allocat
     }
 
     __begin_lifetime(__allocation.ptr, __allocation.count);
-    auto __ptr = __get_long_pointer();
     traits_type::copy(std::__to_address(__allocation.ptr), std::__to_address(__ptr), __size + 1);
-    __alloc_traits::deallocate(__alloc_, __ptr, __get_long_cap());
+    __alloc_traits::deallocate(__alloc_, __ptr, __cap);
     __set_long_cap(__allocation.count);
     __set_long_pointer(__allocation.ptr);
 #  if _LIBCPP_HAS_EXCEPTIONS

>From d897bacb33457862dd8103c30f497dddf2ae6eaf Mon Sep 17 00:00:00 2001
From: Peng Liu <winner245 at hotmail.com>
Date: Thu, 20 Feb 2025 11:05:35 -0500
Subject: [PATCH 2/2] Address philnik777's suggestions

---
 libcxx/include/string | 77 ++++++++++++++++++++++++++++---------------
 1 file changed, 50 insertions(+), 27 deletions(-)

diff --git a/libcxx/include/string b/libcxx/include/string
index 01d8c40edb95b..99e85b9f71f67 100644
--- a/libcxx/include/string
+++ b/libcxx/include/string
@@ -2360,8 +2360,14 @@ private:
     size_type __old_size = size();
     if (__n > __old_size)
       __annotate_increase(__n - __old_size);
-    pointer __p =
-        __is_long() ? (__set_long_size(__n), __get_long_pointer()) : (__set_short_size(__n), __get_short_pointer());
+    pointer __p;
+    if (__is_long()) {
+      __set_long_size(__n);
+      __p = __get_long_pointer();
+    } else {
+      __set_short_size(__n);
+      __p = __get_short_pointer();
+    }
     traits_type::move(std::__to_address(__p), __s, __n);
     traits_type::assign(__p[__n], value_type());
     if (__old_size > __n)
@@ -2701,19 +2707,25 @@ template <class _CharT, class _Traits, class _Allocator>
 template <bool __is_short>
 _LIBCPP_CONSTEXPR_SINCE_CXX20 _LIBCPP_NOINLINE basic_string<_CharT, _Traits, _Allocator>&
 basic_string<_CharT, _Traits, _Allocator>::__assign_no_alias(const value_type* __s, size_type __n) {
-  size_type __cap      = capacity();
-  size_type __old_size = size();
-  if (__n <= __cap) {
-    if (__n > __old_size)
-      __annotate_increase(__n - __old_size);
-    pointer __p =
-        __is_short ? (__set_short_size(__n), __get_short_pointer()) : (__set_long_size(__n), __get_long_pointer());
+  const auto __cap = __is_short ? static_cast<size_type>(__min_cap) : __get_long_cap();
+  const auto __sz  = __is_short ? __get_short_size() : __get_long_size();
+  if (__n < __cap) {
+    if (__n > __sz)
+      __annotate_increase(__n - __sz);
+    pointer __p;
+    if (__is_short) {
+      __p = __get_short_pointer();
+      __set_short_size(__n);
+    } else {
+      __p = __get_long_pointer();
+      __set_long_size(__n);
+    }
     traits_type::copy(std::__to_address(__p), __s, __n);
     traits_type::assign(__p[__n], value_type());
-    if (__old_size > __n)
-      __annotate_shrink(__old_size);
+    if (__sz > __n)
+      __annotate_shrink(__sz);
   } else {
-    __grow_by_and_replace(__cap, __n - __cap, __old_size, 0, __old_size, __n, __s);
+    __grow_by_and_replace(__cap - 1, __n - __cap + 1, __sz, 0, __sz, __n, __s);
   }
   return *this;
 }
@@ -2721,16 +2733,16 @@ basic_string<_CharT, _Traits, _Allocator>::__assign_no_alias(const value_type* _
 template <class _CharT, class _Traits, class _Allocator>
 _LIBCPP_CONSTEXPR_SINCE_CXX20 _LIBCPP_NOINLINE basic_string<_CharT, _Traits, _Allocator>&
 basic_string<_CharT, _Traits, _Allocator>::__assign_external(const value_type* __s, size_type __n) {
-  size_type __cap      = capacity();
-  size_type __old_size = size();
+  const auto __cap = capacity();
+  const auto __sz  = size();
   if (__cap >= __n) {
-    if (__n > __old_size)
-      __annotate_increase(__n - __old_size);
+    if (__n > __sz)
+      __annotate_increase(__n - __sz);
     value_type* __p = std::__to_address(__get_pointer());
     traits_type::move(__p, __s, __n);
     return __null_terminate_at(__p, __n);
   } else {
-    __grow_by_and_replace(__cap, __n - __cap, __old_size, 0, __old_size, __n, __s);
+    __grow_by_and_replace(__cap, __n - __cap, __sz, 0, __sz, __n, __s);
     return *this;
   }
 }
@@ -2763,7 +2775,14 @@ basic_string<_CharT, _Traits, _Allocator>::operator=(value_type __c) {
   size_type __old_size = size();
   if (__old_size == 0)
     __annotate_increase(1);
-  pointer __p = __is_long() ? (__set_long_size(1), __get_long_pointer()) : (__set_short_size(1), __get_short_pointer());
+  pointer __p;
+  if (__is_long()) {
+    __set_long_size(1);
+    __p = __get_long_pointer();
+  } else {
+    __set_short_size(1);
+    __p = __get_short_pointer();
+  }
   traits_type::assign(*__p, __c);
   traits_type::assign(*++__p, value_type());
   if (__old_size > 1)
@@ -2947,10 +2966,8 @@ template <class _CharT, class _Traits, class _Allocator>
 _LIBCPP_CONSTEXPR_SINCE_CXX20 basic_string<_CharT, _Traits, _Allocator>&
 basic_string<_CharT, _Traits, _Allocator>::assign(const value_type* __s) {
   _LIBCPP_ASSERT_NON_NULL(__s != nullptr, "string::assign received nullptr");
-  if (__builtin_constant_p(*__s)) {
-    const size_type __n = traits_type::length(__s);
-    return __fits_in_sso(__n) ? __assign_short(__s, __n) : __assign_external(__s, __n);
-  }
+  if (auto __len = traits_type::length(__s); __builtin_constant_p(__len) && __fits_in_sso(__len))
+    return __assign_short(__s, __len);
   return __assign_external(__s);
 }
 // append
@@ -3026,8 +3043,14 @@ _LIBCPP_CONSTEXPR_SINCE_CXX20 void basic_string<_CharT, _Traits, _Allocator>::pu
     __is_short = false; // the string is always long after __grow_by
   }
   __annotate_increase(1);
-  pointer __p = __is_short ? (__set_short_size(__sz + 1), __get_short_pointer() + __sz)
-                           : (__set_long_size(__sz + 1), __get_long_pointer() + __sz);
+  pointer __p;
+  if (__is_short) {
+    __p = __get_short_pointer() + __sz;
+    __set_short_size(__sz + 1);
+  } else {
+    __p = __get_long_pointer() + __sz;
+    __set_long_size(__sz + 1);
+  }
   traits_type::assign(*__p, __c);
   traits_type::assign(*++__p, value_type());
 }
@@ -3468,9 +3491,9 @@ inline _LIBCPP_CONSTEXPR_SINCE_CXX20 void basic_string<_CharT, _Traits, _Allocat
   _LIBCPP_ASSERT_INTERNAL(__is_long(), "Trying to shrink small string");
 
   // We're a long string and we're shrinking into the small buffer.
-  auto __ptr  = __get_long_pointer();
-  auto __size = __get_long_size();
-  auto __cap  = __get_long_cap();
+  const auto __ptr  = __get_long_pointer();
+  const auto __size = __get_long_size();
+  const auto __cap  = __get_long_cap();
 
   if (__fits_in_sso(__target_capacity)) {
     __annotation_guard __g(*this);



More information about the libcxx-commits mailing list