[libcxx-commits] [libcxx] f1c341c - [libc++] Refactor __split_buffer to eliminate code duplication (#114138)
via libcxx-commits
libcxx-commits at lists.llvm.org
Mon Nov 4 04:54:30 PST 2024
Author: Peng Liu
Date: 2024-11-04T13:54:26+01:00
New Revision: f1c341c36f1a8582163217196abf7401f81a4d2b
URL: https://github.com/llvm/llvm-project/commit/f1c341c36f1a8582163217196abf7401f81a4d2b
DIFF: https://github.com/llvm/llvm-project/commit/f1c341c36f1a8582163217196abf7401f81a4d2b.diff
LOG: [libc++] Refactor __split_buffer to eliminate code duplication (#114138)
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.
Added:
Modified:
libcxx/include/__split_buffer
libcxx/include/__vector/vector.h
libcxx/include/deque
Removed:
################################################################################
diff --git a/libcxx/include/__split_buffer b/libcxx/include/__split_buffer
index 6fc3d9255946ce..102df63e814ae4 100644
--- a/libcxx/include/__split_buffer
+++ b/libcxx/include/__split_buffer
@@ -147,11 +147,9 @@ public:
_LIBCPP_CONSTEXPR_SINCE_CXX20 _LIBCPP_HIDE_FROM_ABI void reserve(size_type __n);
_LIBCPP_CONSTEXPR_SINCE_CXX20 _LIBCPP_HIDE_FROM_ABI void shrink_to_fit() _NOEXCEPT;
- _LIBCPP_CONSTEXPR_SINCE_CXX20 _LIBCPP_HIDE_FROM_ABI void push_front(const_reference __x);
- _LIBCPP_CONSTEXPR_SINCE_CXX20 _LIBCPP_HIDE_FROM_ABI void push_back(const_reference __x);
- _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);
@@ -455,29 +453,8 @@ _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()) {
-
diff erence_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_;
-}
-
-template <class _Tp, class _Allocator>
-_LIBCPP_CONSTEXPR_SINCE_CXX20 void __split_buffer<_Tp, _Allocator>::push_front(value_type&& __x) {
+template <class... _Args>
+_LIBCPP_CONSTEXPR_SINCE_CXX20 void __split_buffer<_Tp, _Allocator>::emplace_front(_Args&&... __args) {
if (__begin_ == __first_) {
if (__end_ < __end_cap()) {
diff erence_type __d = __end_cap() - __end_;
@@ -494,55 +471,10 @@ _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_) {
-
diff erence_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_;
-}
-
-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_) {
-
diff erence_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_;
-}
-
template <class _Tp, class _Allocator>
template <class... _Args>
_LIBCPP_CONSTEXPR_SINCE_CXX20 void __split_buffer<_Tp, _Allocator>::emplace_back(_Args&&... __args) {
diff --git a/libcxx/include/__vector/vector.h b/libcxx/include/__vector/vector.h
index 844e5d6a210568..7d95e5deef5f3b 100644
--- a/libcxx/include/__vector/vector.h
+++ b/libcxx/include/__vector/vector.h
@@ -1263,7 +1263,7 @@ vector<_Tp, _Allocator>::insert(const_iterator __position, const_reference __x)
} else {
allocator_type& __a = this->__alloc();
__split_buffer<value_type, allocator_type&> __v(__recommend(size() + 1), __p - this->__begin_, __a);
- __v.push_back(__x);
+ __v.emplace_back(__x);
__p = __swap_out_circular_buffer(__v, __p);
}
return __make_iter(__p);
@@ -1283,7 +1283,7 @@ vector<_Tp, _Allocator>::insert(const_iterator __position, value_type&& __x) {
} else {
allocator_type& __a = this->__alloc();
__split_buffer<value_type, allocator_type&> __v(__recommend(size() + 1), __p - this->__begin_, __a);
- __v.push_back(std::move(__x));
+ __v.emplace_back(std::move(__x));
__p = __swap_out_circular_buffer(__v, __p);
}
return __make_iter(__p);
diff --git a/libcxx/include/deque b/libcxx/include/deque
index 11219d1a99244e..1a83465b39ef81 100644
--- a/libcxx/include/deque
+++ b/libcxx/include/deque
@@ -2041,20 +2041,20 @@ void deque<_Tp, _Allocator>::__add_front_capacity() {
__start_ += __block_size;
pointer __pt = __map_.back();
__map_.pop_back();
- __map_.push_front(__pt);
+ __map_.emplace_front(__pt);
}
// Else if __map_.size() < __map_.capacity() then we need to allocate 1 buffer
else if (__map_.size() < __map_.capacity()) { // we can put the new buffer into the map, but don't shift things around
// until all buffers are allocated. If we throw, we don't need to fix
// anything up (any added buffers are undetectible)
if (__map_.__front_spare() > 0)
- __map_.push_front(__alloc_traits::allocate(__a, __block_size));
+ __map_.emplace_front(__alloc_traits::allocate(__a, __block_size));
else {
- __map_.push_back(__alloc_traits::allocate(__a, __block_size));
+ __map_.emplace_back(__alloc_traits::allocate(__a, __block_size));
// Done allocating, reorder capacity
pointer __pt = __map_.back();
__map_.pop_back();
- __map_.push_front(__pt);
+ __map_.emplace_front(__pt);
}
__start_ = __map_.size() == 1 ? __block_size / 2 : __start_ + __block_size;
}
@@ -2065,11 +2065,11 @@ void deque<_Tp, _Allocator>::__add_front_capacity() {
typedef __allocator_destructor<_Allocator> _Dp;
unique_ptr<pointer, _Dp> __hold(__alloc_traits::allocate(__a, __block_size), _Dp(__a, __block_size));
- __buf.push_back(__hold.get());
+ __buf.emplace_back(__hold.get());
__hold.release();
for (__map_pointer __i = __map_.begin(); __i != __map_.end(); ++__i)
- __buf.push_back(*__i);
+ __buf.emplace_back(*__i);
std::swap(__map_.__first_, __buf.__first_);
std::swap(__map_.__begin_, __buf.__begin_);
std::swap(__map_.__end_, __buf.__end_);
@@ -2095,7 +2095,7 @@ void deque<_Tp, _Allocator>::__add_front_capacity(size_type __n) {
for (; __back_capacity > 0; --__back_capacity) {
pointer __pt = __map_.back();
__map_.pop_back();
- __map_.push_front(__pt);
+ __map_.emplace_front(__pt);
}
}
// Else if __nb <= __map_.capacity() - __map_.size() then we need to allocate __nb buffers
@@ -2106,17 +2106,17 @@ void deque<_Tp, _Allocator>::__add_front_capacity(size_type __n) {
for (; __nb > 0; --__nb, __start_ += __block_size - (__map_.size() == 1)) {
if (__map_.__front_spare() == 0)
break;
- __map_.push_front(__alloc_traits::allocate(__a, __block_size));
+ __map_.emplace_front(__alloc_traits::allocate(__a, __block_size));
__annotate_whole_block(0, __asan_poison);
}
for (; __nb > 0; --__nb, ++__back_capacity)
- __map_.push_back(__alloc_traits::allocate(__a, __block_size));
+ __map_.emplace_back(__alloc_traits::allocate(__a, __block_size));
// Done allocating, reorder capacity
__start_ += __back_capacity * __block_size;
for (; __back_capacity > 0; --__back_capacity) {
pointer __pt = __map_.back();
__map_.pop_back();
- __map_.push_front(__pt);
+ __map_.emplace_front(__pt);
__annotate_whole_block(0, __asan_poison);
}
}
@@ -2129,7 +2129,7 @@ void deque<_Tp, _Allocator>::__add_front_capacity(size_type __n) {
try {
#endif // _LIBCPP_HAS_EXCEPTIONS
for (; __nb > 0; --__nb) {
- __buf.push_back(__alloc_traits::allocate(__a, __block_size));
+ __buf.emplace_back(__alloc_traits::allocate(__a, __block_size));
// ASan: this is empty container, we have to poison whole block
__annotate_poison_block(std::__to_address(__buf.back()), std::__to_address(__buf.back() + __block_size));
}
@@ -2142,11 +2142,11 @@ void deque<_Tp, _Allocator>::__add_front_capacity(size_type __n) {
}
#endif // _LIBCPP_HAS_EXCEPTIONS
for (; __back_capacity > 0; --__back_capacity) {
- __buf.push_back(__map_.back());
+ __buf.emplace_back(__map_.back());
__map_.pop_back();
}
for (__map_pointer __i = __map_.begin(); __i != __map_.end(); ++__i)
- __buf.push_back(*__i);
+ __buf.emplace_back(*__i);
std::swap(__map_.__first_, __buf.__first_);
std::swap(__map_.__begin_, __buf.__begin_);
std::swap(__map_.__end_, __buf.__end_);
@@ -2164,20 +2164,20 @@ void deque<_Tp, _Allocator>::__add_back_capacity() {
__start_ -= __block_size;
pointer __pt = __map_.front();
__map_.pop_front();
- __map_.push_back(__pt);
+ __map_.emplace_back(__pt);
}
// Else if __nb <= __map_.capacity() - __map_.size() then we need to allocate __nb buffers
else if (__map_.size() < __map_.capacity()) { // we can put the new buffer into the map, but don't shift things around
// until it is allocated. If we throw, we don't need to fix
// anything up (any added buffers are undetectible)
if (__map_.__back_spare() != 0)
- __map_.push_back(__alloc_traits::allocate(__a, __block_size));
+ __map_.emplace_back(__alloc_traits::allocate(__a, __block_size));
else {
- __map_.push_front(__alloc_traits::allocate(__a, __block_size));
+ __map_.emplace_front(__alloc_traits::allocate(__a, __block_size));
// Done allocating, reorder capacity
pointer __pt = __map_.front();
__map_.pop_front();
- __map_.push_back(__pt);
+ __map_.emplace_back(__pt);
}
__annotate_whole_block(__map_.size() - 1, __asan_poison);
}
@@ -2188,11 +2188,11 @@ void deque<_Tp, _Allocator>::__add_back_capacity() {
typedef __allocator_destructor<_Allocator> _Dp;
unique_ptr<pointer, _Dp> __hold(__alloc_traits::allocate(__a, __block_size), _Dp(__a, __block_size));
- __buf.push_back(__hold.get());
+ __buf.emplace_back(__hold.get());
__hold.release();
for (__map_pointer __i = __map_.end(); __i != __map_.begin();)
- __buf.push_front(*--__i);
+ __buf.emplace_front(*--__i);
std::swap(__map_.__first_, __buf.__first_);
std::swap(__map_.__begin_, __buf.__begin_);
std::swap(__map_.__end_, __buf.__end_);
@@ -2217,7 +2217,7 @@ void deque<_Tp, _Allocator>::__add_back_capacity(size_type __n) {
for (; __front_capacity > 0; --__front_capacity) {
pointer __pt = __map_.front();
__map_.pop_front();
- __map_.push_back(__pt);
+ __map_.emplace_back(__pt);
}
}
// Else if __nb <= __map_.capacity() - __map_.size() then we need to allocate __nb buffers
@@ -2228,11 +2228,11 @@ void deque<_Tp, _Allocator>::__add_back_capacity(size_type __n) {
for (; __nb > 0; --__nb) {
if (__map_.__back_spare() == 0)
break;
- __map_.push_back(__alloc_traits::allocate(__a, __block_size));
+ __map_.emplace_back(__alloc_traits::allocate(__a, __block_size));
__annotate_whole_block(__map_.size() - 1, __asan_poison);
}
for (; __nb > 0; --__nb, ++__front_capacity, __start_ += __block_size - (__map_.size() == 1)) {
- __map_.push_front(__alloc_traits::allocate(__a, __block_size));
+ __map_.emplace_front(__alloc_traits::allocate(__a, __block_size));
__annotate_whole_block(0, __asan_poison);
}
// Done allocating, reorder capacity
@@ -2240,7 +2240,7 @@ void deque<_Tp, _Allocator>::__add_back_capacity(size_type __n) {
for (; __front_capacity > 0; --__front_capacity) {
pointer __pt = __map_.front();
__map_.pop_front();
- __map_.push_back(__pt);
+ __map_.emplace_back(__pt);
}
}
// Else need to allocate __nb buffers, *and* we need to reallocate __map_.
@@ -2254,7 +2254,7 @@ void deque<_Tp, _Allocator>::__add_back_capacity(size_type __n) {
try {
#endif // _LIBCPP_HAS_EXCEPTIONS
for (; __nb > 0; --__nb) {
- __buf.push_back(__alloc_traits::allocate(__a, __block_size));
+ __buf.emplace_back(__alloc_traits::allocate(__a, __block_size));
// ASan: this is an empty container, we have to poison the whole block
__annotate_poison_block(std::__to_address(__buf.back()), std::__to_address(__buf.back() + __block_size));
}
@@ -2267,11 +2267,11 @@ void deque<_Tp, _Allocator>::__add_back_capacity(size_type __n) {
}
#endif // _LIBCPP_HAS_EXCEPTIONS
for (; __front_capacity > 0; --__front_capacity) {
- __buf.push_back(__map_.front());
+ __buf.emplace_back(__map_.front());
__map_.pop_front();
}
for (__map_pointer __i = __map_.end(); __i != __map_.begin();)
- __buf.push_front(*--__i);
+ __buf.emplace_front(*--__i);
std::swap(__map_.__first_, __buf.__first_);
std::swap(__map_.__begin_, __buf.__begin_);
std::swap(__map_.__end_, __buf.__end_);
More information about the libcxx-commits
mailing list