[libcxx-commits] [libcxx] [libc++] Simplify vector<bool>::__construct_at_end implementation (PR #119632)

Peng Liu via libcxx-commits libcxx-commits at lists.llvm.org
Wed Dec 11 15:29:39 PST 2024


https://github.com/winner245 created https://github.com/llvm/llvm-project/pull/119632

This PR proposes the removal of the unnecessary initialization in both overloads of `__construct_at_end` in `vector<bool>`, which serves as a helper function for various constructors, `assign`, and `reserve` methods of the class. The specific code segment in question is as follows:

```cpp
if (__old_size == 0 || ((__old_size - 1) / __bits_per_word) != ((this->__size_ - 1) / __bits_per_word)) {
  if (this->__size_ <= __bits_per_word)
    this->__begin_[0] = __storage_type(0);
  else
    this->__begin_[(this->__size_ - 1) / __bits_per_word] = __storage_type(0);
}
```

**Rationale for Removal**: 
The purpose of the above code is to initialize every bit in the last word to 0 when the new size crosses the boundary of the current last word. However, this initialization is unnecessary for the following reasons:


- ***Redundancy***: The initialization of the last word to zero is unnecessary, as it is immediately overwritten by the subsequent call to `std::fill_n`. Even without this initialization, the uninitialized bits in the range `[0, __size_)` will be correctly set by the `std::fill_n` call, ensuring that the bits of interest are properly initialized.

- ***Safety against out-of-bounds access***: Leaving the bits in the last word uninitialized does not introduce any risk, as they remain out of bounds for `vector<bool>`. The `std::fill_n` function itself operates within the valid range of the vector, so any uninitialized values in the last word will not affect the integrity of the operation.

The existing comprehensive tests for the constructors, `assign`, and `reserve` functions continue to pass successfully after the removal of the above code, thereby validating the changes in this PR.


>From ba2304685167f2ce482b1fa410175eb9a2f88a77 Mon Sep 17 00:00:00 2001
From: Peng Liu <winner245 at hotmail.com>
Date: Wed, 11 Dec 2024 18:21:46 -0500
Subject: [PATCH] Simplify vector<bool>::__construct_at_end

---
 libcxx/include/__vector/vector_bool.h | 49 ++++++++-------------------
 1 file changed, 14 insertions(+), 35 deletions(-)

diff --git a/libcxx/include/__vector/vector_bool.h b/libcxx/include/__vector/vector_bool.h
index 36eb7f350ac406..bc3411c4d97791 100644
--- a/libcxx/include/__vector/vector_bool.h
+++ b/libcxx/include/__vector/vector_bool.h
@@ -438,10 +438,22 @@ class _LIBCPP_TEMPLATE_VIS vector<bool, _Allocator> {
     return (__new_size + (__bits_per_word - 1)) & ~((size_type)__bits_per_word - 1);
   }
   _LIBCPP_HIDE_FROM_ABI _LIBCPP_CONSTEXPR_SINCE_CXX20 size_type __recommend(size_type __new_size) const;
-  _LIBCPP_HIDE_FROM_ABI _LIBCPP_CONSTEXPR_SINCE_CXX20 void __construct_at_end(size_type __n, bool __x);
+
+  //  Default constructs __n objects starting at __end_
+  //  Precondition:  __n > 0
+  //  Precondition:  size() + __n <= capacity()
+  //  Postcondition:  size() == size() + __n
+  _LIBCPP_HIDE_FROM_ABI _LIBCPP_CONSTEXPR_SINCE_CXX20 void __construct_at_end(size_type __n, bool __x) {
+    std::fill_n(end(), __n, __x);
+    this->__size_ += __n;
+  }
   template <class _InputIterator, class _Sentinel>
   _LIBCPP_HIDE_FROM_ABI _LIBCPP_CONSTEXPR_SINCE_CXX20 void
-  __construct_at_end(_InputIterator __first, _Sentinel __last, size_type __n);
+  __construct_at_end(_InputIterator __first, _Sentinel __last, size_type __n) {
+    std::__copy(__first, __last, end());
+    this->__size_ += __n;
+  }
+
   _LIBCPP_HIDE_FROM_ABI _LIBCPP_CONSTEXPR_SINCE_CXX20 void __append(size_type __n, const_reference __x);
   _LIBCPP_HIDE_FROM_ABI _LIBCPP_CONSTEXPR_SINCE_CXX20 reference __make_ref(size_type __pos) _NOEXCEPT {
     return reference(__begin_ + __pos / __bits_per_word, __storage_type(1) << __pos % __bits_per_word);
@@ -530,39 +542,6 @@ vector<bool, _Allocator>::__recommend(size_type __new_size) const {
   return std::max(2 * __cap, __align_it(__new_size));
 }
 
-//  Default constructs __n objects starting at __end_
-//  Precondition:  __n > 0
-//  Precondition:  size() + __n <= capacity()
-//  Postcondition:  size() == size() + __n
-template <class _Allocator>
-inline _LIBCPP_HIDE_FROM_ABI _LIBCPP_CONSTEXPR_SINCE_CXX20 void
-vector<bool, _Allocator>::__construct_at_end(size_type __n, bool __x) {
-  size_type __old_size = this->__size_;
-  this->__size_ += __n;
-  if (__old_size == 0 || ((__old_size - 1) / __bits_per_word) != ((this->__size_ - 1) / __bits_per_word)) {
-    if (this->__size_ <= __bits_per_word)
-      this->__begin_[0] = __storage_type(0);
-    else
-      this->__begin_[(this->__size_ - 1) / __bits_per_word] = __storage_type(0);
-  }
-  std::fill_n(__make_iter(__old_size), __n, __x);
-}
-
-template <class _Allocator>
-template <class _InputIterator, class _Sentinel>
-_LIBCPP_CONSTEXPR_SINCE_CXX20 void
-vector<bool, _Allocator>::__construct_at_end(_InputIterator __first, _Sentinel __last, size_type __n) {
-  size_type __old_size = this->__size_;
-  this->__size_ += __n;
-  if (__old_size == 0 || ((__old_size - 1) / __bits_per_word) != ((this->__size_ - 1) / __bits_per_word)) {
-    if (this->__size_ <= __bits_per_word)
-      this->__begin_[0] = __storage_type(0);
-    else
-      this->__begin_[(this->__size_ - 1) / __bits_per_word] = __storage_type(0);
-  }
-  std::__copy(__first, __last, __make_iter(__old_size));
-}
-
 template <class _Allocator>
 inline _LIBCPP_HIDE_FROM_ABI _LIBCPP_CONSTEXPR_SINCE_CXX20 vector<bool, _Allocator>::vector()
     _NOEXCEPT_(is_nothrow_default_constructible<allocator_type>::value)



More information about the libcxx-commits mailing list