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

[via libcxx-commits]

llvmbot wrote:


<!--LLVM PR SUMMARY COMMENT-->

@llvm/pr-subscribers-libcxx

Author: Peng Liu (winner245)

<details>
<summary>Changes</summary>

Following #<!-- -->119502, I realized that `vector<bool>::__construct_at_end` performs unnecessary work. This PR proposes removing the redundant initialization of storage bits in both overloads of `__construct_at_end`. This function serves as a useful helper for various constructors, `assign`, and `reserve` methods of the class. The specific code segment in question is:

```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);
}


```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 set every bit in the last storage word of `__storage_type` to 0 when the new size crosses the boundary of the current last storage word. However, setting the raw bits to 0 before an immediate assignment is completely unnecessary, as explained below:

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

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

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.


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


1 Files Affected:

- (modified) libcxx/include/__vector/vector_bool.h (+14-35) 


``````````diff
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)

``````````

</details>


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