[libc++][test] Improve tests for assign in std::vector and vector<bool>

[winner245]

This PR enhances the test coverage for the assign function of std::vector by adding new tests for several important test cases that were previously missing, as shown in the following table:

test cases	forward_iterator	input_iterator
new_size > capacity()	Yes	Yes
size() < new_size <= capacity()	No	No
new_size <= size()	No	No

Similarly, no tests have previously covered assign(_InputIterator __first, _InputIterator __last) and assign(size_type __n, const value_type& __x) for vector<bool>.

With this patch applied, all missing tests will be covered.

[llvmbot]

@llvm/pr-subscribers-libcxx

Author: Peng Liu (winner245)

Changes

This PR enhances the test coverage for the assign function of std::vector by adding new tests for several important test cases that were previously missing, as shown in the following table:

test cases	forward_iterator	input_iterator
new_size > capacity()	Yes	Yes
size() < new_size <= capacity()	No	No
new_size <= size()	No	No

With this patch applied, all missing test cases will be covered.

---

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

1 Files Affected:

• (modified) libcxx/test/std/containers/sequences/vector/vector.cons/assign_iter_iter.pass.cpp (+100-7)

diff --git a/libcxx/test/std/containers/sequences/vector/vector.cons/assign_iter_iter.pass.cpp b/libcxx/test/std/containers/sequences/vector/vector.cons/assign_iter_iter.pass.cpp
index 9b52885b9bf8d2..584f641edd7c52 100644
--- a/libcxx/test/std/containers/sequences/vector/vector.cons/assign_iter_iter.pass.cpp
+++ b/libcxx/test/std/containers/sequences/vector/vector.cons/assign_iter_iter.pass.cpp
@@ -18,17 +18,16 @@
 #include "asan_testing.h"
 #include "test_iterators.h"
 #if TEST_STD_VER >= 11
-#include "emplace_constructible.h"
-#include "container_test_types.h"
+#  include "emplace_constructible.h"
+#  include "container_test_types.h"
 #endif
 
-
 TEST_CONSTEXPR_CXX20 bool test() {
 #if TEST_STD_VER >= 11
   int arr1[] = {42};
   int arr2[] = {1, 101, 42};
-  {
-    using T = EmplaceConstructibleMoveableAndAssignable<int>;
+  { // Test with new_size > capacity() == 0 for forward_iterator, resulting in reallocation during assign
+    using T  = EmplaceConstructibleMoveableAndAssignable<int>;
     using It = forward_iterator<int*>;
     {
       std::vector<T> v;
@@ -43,8 +42,8 @@ TEST_CONSTEXPR_CXX20 bool test() {
       assert(v[2].value == 42);
     }
   }
-  {
-    using T = EmplaceConstructibleMoveableAndAssignable<int>;
+  { // Test with new_size > capacity() == 0 for input_iterator, resulting in reallocation during assign
+    using T  = EmplaceConstructibleMoveableAndAssignable<int>;
     using It = cpp17_input_iterator<int*>;
     {
       std::vector<T> v;
@@ -63,6 +62,100 @@ TEST_CONSTEXPR_CXX20 bool test() {
       assert(v[2].value == 42);
     }
   }
+
+  { // Test with new_size < size() for forward_iterator, resulting in destruction at end during assign
+    using T  = EmplaceConstructibleMoveableAndAssignable<int>;
+    using It = forward_iterator<int*>;
+    {
+      std::vector<T> v;
+      v.reserve(5);
+      for (std::size_t i = 0; i < v.capacity(); ++i)
+        v.emplace_back(99);
+      v.assign(It(arr1), It(std::end(arr1)));
+      assert(v.size() == 1);
+      assert(v[0].value == 42);
+    }
+    {
+      std::vector<T> v;
+      v.reserve(5);
+      for (std::size_t i = 0; i < v.capacity(); ++i)
+        v.emplace_back(99);
+      v.assign(It(arr2), It(std::end(arr2)));
+      assert(v.size() == 3);
+      assert(v[0].value == 1);
+      assert(v[1].value == 101);
+      assert(v[2].value == 42);
+    }
+  }
+  { // Test with new_size < size() for input_iterator, resulting in destruction at end during assign
+    using T  = EmplaceConstructibleMoveableAndAssignable<int>;
+    using It = cpp17_input_iterator<int*>;
+    {
+      std::vector<T> v;
+      v.reserve(5);
+      for (std::size_t i = 0; i < v.capacity(); ++i)
+        v.emplace_back(99);
+      v.assign(It(arr1), It(std::end(arr1)));
+      assert(v.size() == 1);
+      assert(v[0].value == 42);
+    }
+    {
+      std::vector<T> v;
+      v.reserve(5);
+      for (std::size_t i = 0; i < v.capacity(); ++i)
+        v.emplace_back(99);
+      v.assign(It(arr2), It(std::end(arr2)));
+      assert(v.size() == 3);
+      assert(v[0].value == 1);
+      assert(v[1].value == 101);
+      assert(v[2].value == 42);
+    }
+  }
+
+  { // Test with size() < new_size < capacity() for forward_iterator, resulting in construction at end during assign
+    using T  = EmplaceConstructibleMoveableAndAssignable<int>;
+    using It = forward_iterator<int*>;
+    {
+      std::vector<T> v;
+      v.reserve(5);
+      v.assign(It(arr1), It(std::end(arr1)));
+      assert(v.size() == 1);
+      assert(v[0].value == 42);
+    }
+    {
+      std::vector<T> v;
+      v.reserve(5);
+      for (std::size_t i = 0; i < 2; ++i)
+        v.emplace_back(99);
+      v.assign(It(arr2), It(std::end(arr2)));
+      assert(v.size() == 3);
+      assert(v[0].value == 1);
+      assert(v[1].value == 101);
+      assert(v[2].value == 42);
+    }
+  }
+  { // Test with size() < new_size < capacity() for inputs_iterator, resulting in construction at end during assign
+    using T  = EmplaceConstructibleMoveableAndAssignable<int>;
+    using It = cpp17_input_iterator<int*>;
+    {
+      std::vector<T> v;
+      v.reserve(5);
+      v.assign(It(arr1), It(std::end(arr1)));
+      assert(v.size() == 1);
+      assert(v[0].value == 42);
+    }
+    {
+      std::vector<T> v;
+      v.reserve(5);
+      for (std::size_t i = 0; i < 2; ++i)
+        v.emplace_back(99);
+      v.assign(It(arr2), It(std::end(arr2)));
+      assert(v.size() == 3);
+      assert(v[0].value == 1);
+      assert(v[1].value == 101);
+      assert(v[2].value == 42);
+    }
+  }
 #endif
 
   // Test with a number of elements in the source range that is greater than capacity

[ldionne]

Great catch! I played around and we indeed didn't catch bugs in the code paths you described, which is quite bad. Thanks for adding coverage!

[ldionne]

Suggested change

	{ // Test with size() < new_size < capacity() for inputs_iterator, resulting in construction at end during assign
	{ // Test with size() < new_size < capacity() for input_iterator, resulting in construction at end during assign

[ldionne]

LGTM, thanks! Please rebase onto main (you'll get a small merge conflict in vector::assign tests because of a change I made, sorry about that).

[ldionne]

Sorry for the many small reviews, I didn't do a good job looking at this patch the first time around and I found a few more nitpicks. This is great though, they're all minor comments.

[ldionne]

Okay, now this should be good to go for real once CI is green. Sorry for the multiple notifications. Thanks a lot for the patch!

[winner245]

Thank you so much for your careful and thorough review! I truly appreciate the time and effort you've put into helping improve all my PRs. Your careful review has identified all my careless points and significantly improved my work. Please don't worry about the number of comments; they are indeed incredibly helpful! I'll go through all of them and make the necessary adjustments.

[ldionne]

LGTM with green CI. I merged main into your branch to get over a CI issue that was present in main.