[libc++] Simplify vector<bool>::flip() and add new tests

[winner245]

This PR simplifies the internal bitwise logic of the flip() function for vector<bool>, and creates new tests to validate the changes.

[llvmbot]

@llvm/pr-subscribers-libcxx

Author: Peng Liu (winner245)

Changes

This PR simplifies the internal bitwise logic of the flip() function for vector<bool>, and creates new tests to validate the changes.

---

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

2 Files Affected:

• (modified) libcxx/include/__vector/vector_bool.h (+4-8)
• (added) libcxx/test/std/containers/sequences/vector.bool/flip.pass.cpp (+100)

diff --git a/libcxx/include/__vector/vector_bool.h b/libcxx/include/__vector/vector_bool.h
index 36eb7f350ac406..1021465a9325b9 100644
--- a/libcxx/include/__vector/vector_bool.h
+++ b/libcxx/include/__vector/vector_bool.h
@@ -1049,18 +1049,14 @@ _LIBCPP_CONSTEXPR_SINCE_CXX20 void vector<bool, _Allocator>::resize(size_type __
 
 template <class _Allocator>
 _LIBCPP_CONSTEXPR_SINCE_CXX20 void vector<bool, _Allocator>::flip() _NOEXCEPT {
-  // do middle whole words
+  // Process the whole words in the front
   size_type __n         = __size_;
   __storage_pointer __p = __begin_;
   for (; __n >= __bits_per_word; ++__p, __n -= __bits_per_word)
     *__p = ~*__p;
-  // do last partial word
-  if (__n > 0) {
-    __storage_type __m = ~__storage_type(0) >> (__bits_per_word - __n);
-    __storage_type __b = *__p & __m;
-    *__p &= ~__m;
-    *__p |= ~__b & __m;
-  }
+  // Process the last partial word, if it exists
+  if (__n > 0)
+    *__p ^= ~__storage_type(0) >> (__bits_per_word - __n);
 }
 
 template <class _Allocator>
diff --git a/libcxx/test/std/containers/sequences/vector.bool/flip.pass.cpp b/libcxx/test/std/containers/sequences/vector.bool/flip.pass.cpp
new file mode 100644
index 00000000000000..5be52bffc5ffb5
--- /dev/null
+++ b/libcxx/test/std/containers/sequences/vector.bool/flip.pass.cpp
@@ -0,0 +1,100 @@
+//===----------------------------------------------------------------------===//
+//
+// Part of the LLVM Project, under the Apache License v2.0 with LLVM Exceptions.
+// See https://llvm.org/LICENSE.txt for license information.
+// SPDX-License-Identifier: Apache-2.0 WITH LLVM-exception
+//
+//===----------------------------------------------------------------------===//
+
+// <vector>
+
+// flip()
+
+#include <cassert>
+#include <vector>
+
+#include "min_allocator.h"
+#include "test_allocator.h"
+#include "test_macros.h"
+
+TEST_CONSTEXPR_CXX20 bool tests() {
+  //
+  // Testing flip() function with small vectors and various allocators
+  //
+  {
+    std::vector<bool> v;
+    v.push_back(true);
+    v.push_back(false);
+    v.push_back(true);
+    v.flip();
+    assert(!v[0]);
+    assert(v[1]);
+    assert(!v[2]);
+  }
+  {
+    std::vector<bool, min_allocator<bool> > v;
+    v.push_back(true);
+    v.push_back(false);
+    v.push_back(true);
+    v.flip();
+    assert(!v[0]);
+    assert(v[1]);
+    assert(!v[2]);
+  }
+  {
+    std::vector<bool, test_allocator<bool> > v(test_allocator<bool>(5));
+    v.push_back(true);
+    v.push_back(false);
+    v.push_back(true);
+    v.flip();
+    assert(!v[0]);
+    assert(v[1]);
+    assert(!v[2]);
+  }
+
+  //
+  // Testing flip() function with larger vectors
+  //
+  {
+    std::vector<bool> v(1000);
+    for (std::size_t i = 0; i < v.size(); ++i)
+      v[i] = i & 1;
+    std::vector<bool> original = v;
+    v.flip();
+    for (size_t i = 0; i < v.size(); ++i) {
+      assert(v[i] == !original[i]);
+    }
+  }
+  {
+    std::vector<bool, min_allocator<bool> > v(1000, false, min_allocator<bool>());
+    for (std::size_t i = 0; i < v.size(); ++i)
+      v[i] = i & 1;
+    std::vector<bool, min_allocator<bool> > original = v;
+    v.flip();
+    for (size_t i = 0; i < v.size(); ++i)
+      assert(v[i] == !original[i]);
+    v.flip();
+    assert(v == original);
+  }
+  {
+    std::vector<bool, test_allocator<bool> > v(1000, false, test_allocator<bool>(5));
+    for (std::size_t i = 0; i < v.size(); ++i)
+      v[i] = i & 1;
+    std::vector<bool, test_allocator<bool> > original = v;
+    v.flip();
+    for (size_t i = 0; i < v.size(); ++i)
+      assert(v[i] == !original[i]);
+    v.flip();
+    assert(v == original);
+  }
+
+  return true;
+}
+
+int main(int, char**) {
+  tests();
+#if TEST_STD_VER > 17
+  static_assert(tests());
+#endif
+  return 0;
+}

[philnik777]

Why can't we just flip the whole word?

[winner245]

We are indeed flipping each whole word in the front, and the last partial word.

[philnik777]

Yeah, but why? Is it an invariant that bits past the end are zeroed? If not, we can just flip them as well unconditionally and not care about flipping only relevant bits. It's not like we gain anything by not flipping irrelevant bits.

[winner245]

Thank you for your clarification. I agree that unconditionally flipping the unused bits in the last word would simplify the code further. My current implementation keeps those bits intact—they are neither zeroed nor changed in any way, which seems 100% safe to me. However, your suggestion makes sense as well, considering those unused bits shouldn't be used by others in any way. I'll proceed with updating the implementation to flip the entire word and ensure it passes all CI checks.

[philnik777]

LGTM with nits addressed.

[ldionne]

I'm deferring to @philnik777 's review. I double-checked that all of his comments had been addressed. Thanks!