[libcxx-commits] [libcxx] [libc++] Fix no-op shrink_to_fit for vector<bool> (PR #120495)

[Louis Dionne via libcxx-commits]

================
@@ -20,21 +23,48 @@
 
 TEST_CONSTEXPR_CXX20 bool tests() {
   {
-    std::vector<bool> v(100);
+    using C = std::vector<bool>;
+    C v(100);
     v.push_back(1);
+    v.clear();
     v.shrink_to_fit();
-    assert(v.capacity() >= 101);
-    assert(v.size() >= 101);
+    assert(v.capacity() == 0);
+    assert(v.size() == 0);
   }
-#if TEST_STD_VER >= 11
   {
-    std::vector<bool, min_allocator<bool>> v(100);
+    using C = std::vector<bool, min_allocator<bool> >;
+    C v(100);
     v.push_back(1);
+    C::size_type before_cap = v.capacity();
     v.shrink_to_fit();
     assert(v.capacity() >= 101);
-    assert(v.size() >= 101);
+    assert(v.capacity() <= before_cap);
+    assert(v.size() == 101);
+  }
+  {
+    using C                = std::vector<bool>;
+    unsigned bits_per_word = static_cast<unsigned>(sizeof(C::__storage_type) * CHAR_BIT);
----------------
ldionne wrote:

The part of the test suite located under `libcxx/test/std` is intended to be portable and usable by other implementations (like MSVC STL or libstdc++). As such, we don't use things specific to libc++ like `C::__storage_type`. Or if we absolutely must, we either:

1. Put that part of the test under `libcxx/test/libcxx`, which contains tests that are not entirely portable, or
2. Guard the check with `#if defined(_LIBCPP_VERSION)`, or
3. Use something like `LIBCPP_ASSERT` from `test_macros.h`.

Which of those three we do depends on the amount of libc++ specific test code we're trying to add, maintenance burden, etc.

In this case, the very best would be to figure out the number of bits per word via some public-facing API if that's doable, but I'm not certain it is. Otherwise, I would probably go for `#if defined(_LIBCPP_VERSION)` around those two tests.

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