[libcxx-commits] [libcxx] [libc++] Refactor __split_buffer to eliminate code duplication (PR #114138)

[via libcxx-commits]

llvmbot wrote:


<!--LLVM PR SUMMARY COMMENT-->

@llvm/pr-subscribers-libcxx

Author: Peng Liu (winner245)

<details>
<summary>Changes</summary>

This PR refactors the `__split_buffer` class to eliminate code duplication in the `push_back` and `push_front` member functions.

**Motivation:**  
The lvalue and rvalue reference overloads of `push_back` share identical logic, which coincides with that of `emplace_back`. Similarly, the overloads of `push_front` also share identical logic but lack an `emplace_front` member function, leading to an inconsistency. These identical internal logics lead to significant code duplication, making future maintenance more difficult.

**Summary of Refactor:**  
This PR reduces code redundancy by:
1. Modifying both overloads of `push_back` to call `emplace_back`.
2. Introducing a new `emplace_front` member function that encapsulates the logic of `push_front`, allowing both overloads of `push_front` to call it (The addition of `emplace_front` also avoids the inconsistency regarding the absence of `emplace_front`).

The refactoring results in reduced code duplication, improved maintainability, and enhanced readability.

---
Full diff: https://github.com/llvm/llvm-project/pull/114138.diff


1 Files Affected:

- (modified) libcxx/include/__split_buffer (+12-55) 


``````````diff
diff --git a/libcxx/include/__split_buffer b/libcxx/include/__split_buffer
index c4817601039f3b..99fe4059a0e15d 100644
--- a/libcxx/include/__split_buffer
+++ b/libcxx/include/__split_buffer
@@ -152,6 +152,8 @@ public:
   _LIBCPP_CONSTEXPR_SINCE_CXX20 _LIBCPP_HIDE_FROM_ABI void push_front(value_type&& __x);
   _LIBCPP_CONSTEXPR_SINCE_CXX20 _LIBCPP_HIDE_FROM_ABI void push_back(value_type&& __x);
 
+	template <class... _Args>
+  _LIBCPP_CONSTEXPR_SINCE_CXX20 _LIBCPP_HIDE_FROM_ABI void emplace_front(_Args&&... __args);
   template <class... _Args>
   _LIBCPP_CONSTEXPR_SINCE_CXX20 _LIBCPP_HIDE_FROM_ABI void emplace_back(_Args&&... __args);
 
@@ -456,28 +458,17 @@ _LIBCPP_CONSTEXPR_SINCE_CXX20 void __split_buffer<_Tp, _Allocator>::shrink_to_fi
 
 template <class _Tp, class _Allocator>
 _LIBCPP_CONSTEXPR_SINCE_CXX20 void __split_buffer<_Tp, _Allocator>::push_front(const_reference __x) {
-  if (__begin_ == __first_) {
-    if (__end_ < __end_cap()) {
-      difference_type __d = __end_cap() - __end_;
-      __d                 = (__d + 1) / 2;
-      __begin_            = std::move_backward(__begin_, __end_, __end_ + __d);
-      __end_ += __d;
-    } else {
-      size_type __c = std::max<size_type>(2 * static_cast<size_t>(__end_cap() - __first_), 1);
-      __split_buffer<value_type, __alloc_rr&> __t(__c, (__c + 3) / 4, __alloc());
-      __t.__construct_at_end(move_iterator<pointer>(__begin_), move_iterator<pointer>(__end_));
-      std::swap(__first_, __t.__first_);
-      std::swap(__begin_, __t.__begin_);
-      std::swap(__end_, __t.__end_);
-      std::swap(__end_cap(), __t.__end_cap());
-    }
-  }
-  __alloc_traits::construct(__alloc(), std::__to_address(__begin_ - 1), __x);
-  --__begin_;
+  emplace_front(__x);
 }
 
 template <class _Tp, class _Allocator>
 _LIBCPP_CONSTEXPR_SINCE_CXX20 void __split_buffer<_Tp, _Allocator>::push_front(value_type&& __x) {
+  emplace_front(std::move(__x));
+}
+
+template <class _Tp, class _Allocator>
+template <class... _Args>
+_LIBCPP_CONSTEXPR_SINCE_CXX20 void __split_buffer<_Tp, _Allocator>::emplace_front(_Args&&... __args) {
   if (__begin_ == __first_) {
     if (__end_ < __end_cap()) {
       difference_type __d = __end_cap() - __end_;
@@ -494,53 +485,19 @@ _LIBCPP_CONSTEXPR_SINCE_CXX20 void __split_buffer<_Tp, _Allocator>::push_front(v
       std::swap(__end_cap(), __t.__end_cap());
     }
   }
-  __alloc_traits::construct(__alloc(), std::__to_address(__begin_ - 1), std::move(__x));
+  __alloc_traits::construct(__alloc(), std::__to_address(__begin_ - 1), std::forward<_Args>(__args)...);
   --__begin_;
 }
 
 template <class _Tp, class _Allocator>
 _LIBCPP_CONSTEXPR_SINCE_CXX20 inline _LIBCPP_HIDE_FROM_ABI void
 __split_buffer<_Tp, _Allocator>::push_back(const_reference __x) {
-  if (__end_ == __end_cap()) {
-    if (__begin_ > __first_) {
-      difference_type __d = __begin_ - __first_;
-      __d                 = (__d + 1) / 2;
-      __end_              = std::move(__begin_, __end_, __begin_ - __d);
-      __begin_ -= __d;
-    } else {
-      size_type __c = std::max<size_type>(2 * static_cast<size_t>(__end_cap() - __first_), 1);
-      __split_buffer<value_type, __alloc_rr&> __t(__c, __c / 4, __alloc());
-      __t.__construct_at_end(move_iterator<pointer>(__begin_), move_iterator<pointer>(__end_));
-      std::swap(__first_, __t.__first_);
-      std::swap(__begin_, __t.__begin_);
-      std::swap(__end_, __t.__end_);
-      std::swap(__end_cap(), __t.__end_cap());
-    }
-  }
-  __alloc_traits::construct(__alloc(), std::__to_address(__end_), __x);
-  ++__end_;
+  emplace_back(__x);
 }
 
 template <class _Tp, class _Allocator>
 _LIBCPP_CONSTEXPR_SINCE_CXX20 void __split_buffer<_Tp, _Allocator>::push_back(value_type&& __x) {
-  if (__end_ == __end_cap()) {
-    if (__begin_ > __first_) {
-      difference_type __d = __begin_ - __first_;
-      __d                 = (__d + 1) / 2;
-      __end_              = std::move(__begin_, __end_, __begin_ - __d);
-      __begin_ -= __d;
-    } else {
-      size_type __c = std::max<size_type>(2 * static_cast<size_t>(__end_cap() - __first_), 1);
-      __split_buffer<value_type, __alloc_rr&> __t(__c, __c / 4, __alloc());
-      __t.__construct_at_end(move_iterator<pointer>(__begin_), move_iterator<pointer>(__end_));
-      std::swap(__first_, __t.__first_);
-      std::swap(__begin_, __t.__begin_);
-      std::swap(__end_, __t.__end_);
-      std::swap(__end_cap(), __t.__end_cap());
-    }
-  }
-  __alloc_traits::construct(__alloc(), std::__to_address(__end_), std::move(__x));
-  ++__end_;
+  emplace_back(std::move(__x));
 }
 
 template <class _Tp, class _Allocator>

``````````

</details>


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