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

[Louis Dionne via libcxx-commits]

================
@@ -536,30 +536,20 @@ vector<bool, _Allocator>::__recommend(size_type __new_size) const {
 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_;
+  iterator __old_end = end();
   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);
+  this->__begin_[(this->__size_ - 1) / __bits_per_word] = __storage_type(0);
----------------
ldionne wrote:

This was the source of my confusion:

```c++
vector<bool> v;
for (int i = 0; i != 65; ++i) {
  v.push_back(true);
}
assert(v[64] == true); // first bit of the second 64-bit word in the vector

v.__construct_at_end(1, false);
// My understanding is that after this operation, v[64] == false
```

And I think my reasoning does hold, except that it never applies because `__construct_at_end` is never called on a vector that already contains elements. In other words, it seems that we have an additional (undocumented) precondition to this function: `_LIBCPP_ASSERT_INTERNAL(empty(), "this function requires an empty vector");`.

I would suggest the following action items:
1. Try to validate that we do indeed have this precondition, which can be done by adding the above internal assertion and running the test suite with `libcxx-lit <build> -sv libcxx/test --param hardening_mode=debug`.
2. Also, make sure that we have test coverage for that case, something like inserting into a non-empty vector or assigning to that.

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