Skip to content

Conversation

llvmbot
Copy link
Member

@llvmbot llvmbot commented Jul 23, 2024

Backport c2e4386

Requested by: @ldionne

@llvmbot llvmbot requested a review from a team as a code owner July 23, 2024 16:08
@llvmbot llvmbot added this to the LLVM 19.X Release milestone Jul 23, 2024
@llvmbot
Copy link
Member Author

llvmbot commented Jul 23, 2024

@ldionne What do you think about merging this PR to the release branch?

@llvmbot llvmbot requested a review from ldionne July 23, 2024 16:08
@llvmbot llvmbot added the libc++ libc++ C++ Standard Library. Not GNU libstdc++. Not libc++abi. label Jul 23, 2024
@llvmbot
Copy link
Member Author

llvmbot commented Jul 23, 2024

@llvm/pr-subscribers-libcxx

Author: None (llvmbot)

Changes

Backport c2e4386

Requested by: @ldionne


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

1 Files Affected:

  • (modified) libcxx/test/std/containers/sequences/vector.bool/shrink_to_fit.pass.cpp (+44-1)
diff --git a/libcxx/test/std/containers/sequences/vector.bool/shrink_to_fit.pass.cpp b/libcxx/test/std/containers/sequences/vector.bool/shrink_to_fit.pass.cpp
index b39245cab7bf4..f8bcee31964bb 100644
--- a/libcxx/test/std/containers/sequences/vector.bool/shrink_to_fit.pass.cpp
+++ b/libcxx/test/std/containers/sequences/vector.bool/shrink_to_fit.pass.cpp
@@ -39,11 +39,54 @@ TEST_CONSTEXPR_CXX20 bool tests()
     return true;
 }
 
+#if TEST_STD_VER >= 23
+template <typename T>
+struct increasing_allocator {
+  using value_type         = T;
+  std::size_t min_elements = 1000;
+  increasing_allocator()   = default;
+
+  template <typename U>
+  constexpr increasing_allocator(const increasing_allocator<U>& other) noexcept : min_elements(other.min_elements) {}
+
+  constexpr std::allocation_result<T*> allocate_at_least(std::size_t n) {
+    if (n < min_elements)
+      n = min_elements;
+    min_elements += 1000;
+    return std::allocator<T>{}.allocate_at_least(n);
+  }
+  constexpr T* allocate(std::size_t n) { return allocate_at_least(n).ptr; }
+  constexpr void deallocate(T* p, std::size_t n) noexcept { std::allocator<T>{}.deallocate(p, n); }
+};
+
+template <typename T, typename U>
+bool operator==(increasing_allocator<T>, increasing_allocator<U>) {
+  return true;
+}
+
+// https://github.com/llvm/llvm-project/issues/95161
+constexpr bool test_increasing_allocator() {
+  std::vector<bool, increasing_allocator<bool>> v;
+  v.push_back(1);
+  std::size_t capacity = v.capacity();
+  v.shrink_to_fit();
+  assert(v.capacity() <= capacity);
+  assert(v.size() == 1);
+
+  return true;
+}
+#endif // TEST_STD_VER >= 23
+
 int main(int, char**)
 {
-    tests();
+  tests();
 #if TEST_STD_VER > 17
     static_assert(tests());
 #endif
+#if TEST_STD_VER >= 23
+    test_increasing_allocator();
+    static_assert(test_increasing_allocator());
+#endif // TEST_STD_VER >= 23
+
     return 0;
 }

`vector<bool>`'s shrink_to_fit implementation is using the
"swap-to-free-container-resources-trick" which only shrinks when the
input vector is empty. Since the request to shrink_to_fit is
non-binding, this is a valid implementation. It is not a high-quality
implementation. Since `vector<bool>` is not a very popular container the
implementation has not been changed and only a test to validate the
non-growing property has been added.

This was discovered while investigating llvm#95161.

(cherry picked from commit c2e4386)
@tru tru merged commit a930d1f into llvm:release/19.x Jul 24, 2024
Copy link

@ldionne (or anyone else). If you would like to add a note about this fix in the release notes (completely optional). Please reply to this comment with a one or two sentence description of the fix. When you are done, please add the release:note label to this PR.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

libc++ libc++ C++ Standard Library. Not GNU libstdc++. Not libc++abi.

Projects

Development

Successfully merging this pull request may close these issues.

3 participants