[libcxx-commits] [libcxx] [libc++] Improve the test coverage for std::vector::emplace (PR #132440)

Peng Liu via libcxx-commits libcxx-commits at lists.llvm.org
Thu Mar 27 06:28:46 PDT 2025


================
@@ -7,150 +7,341 @@
 //===----------------------------------------------------------------------===//
 
 // UNSUPPORTED: c++03 && !stdlib=libc++
+// ADDITIONAL_COMPILE_FLAGS(has-fconstexpr-steps): -fconstexpr-steps=9000000
 
 // <vector>
 
 // template <class... Args> iterator emplace(const_iterator pos, Args&&... args);
 
-#include <vector>
 #include <cassert>
+#include <cstddef>
+#include <type_traits>
+#include <utility>
+#include <vector>
 
-#include "test_macros.h"
-#include "test_allocator.h"
-#include "min_allocator.h"
 #include "asan_testing.h"
+#include "common.h"
+#include "min_allocator.h"
+#include "test_allocator.h"
+#include "test_macros.h"
 
-class A {
-  int i_;
-  double d_;
+struct MoveOnly {
+  int value;
 
-  A(const A&);
-  A& operator=(const A&);
+  TEST_CONSTEXPR_CXX14 explicit MoveOnly(int i) : value(i) {}
 
-public:
-  TEST_CONSTEXPR_CXX14 A(int i, double d) : i_(i), d_(d) {}
+  MoveOnly(MoveOnly const&)            = delete;
+  MoveOnly& operator=(MoveOnly const&) = delete;
 
-  TEST_CONSTEXPR_CXX14 A(A&& a) : i_(a.i_), d_(a.d_) {
-    a.i_ = 0;
-    a.d_ = 0;
-  }
+  TEST_CONSTEXPR_CXX14 MoveOnly(MoveOnly&& other) TEST_NOEXCEPT : value(other.value) { other.value = -1; }
 
-  TEST_CONSTEXPR_CXX14 A& operator=(A&& a) {
-    i_   = a.i_;
-    d_   = a.d_;
-    a.i_ = 0;
-    a.d_ = 0;
+  TEST_CONSTEXPR_CXX14 MoveOnly& operator=(MoveOnly&& other) TEST_NOEXCEPT {
+    value       = other.value;
+    other.value = -1;
     return *this;
   }
 
-  TEST_CONSTEXPR_CXX14 int geti() const { return i_; }
-  TEST_CONSTEXPR_CXX14 double getd() const { return d_; }
+  TEST_CONSTEXPR_CXX14 friend bool operator==(MoveOnly const& lhs, MoveOnly const& rhs) {
+    return lhs.value == rhs.value;
+  }
 };
 
-TEST_CONSTEXPR_CXX20 bool tests() {
+template <class T>
+struct has_moved_from_sentinel : std::false_type {};
+
+template <>
+struct has_moved_from_sentinel<MoveOnly> : std::true_type {};
+
+template <template <class...> class Allocator, class T>
+TEST_CONSTEXPR_CXX20 void test() {
+  using Vector   = std::vector<T, Allocator<T> >;
+  using Iterator = typename Vector::iterator;
+
+  // Check the return type
   {
-    std::vector<A> c;
-    std::vector<A>::iterator i = c.emplace(c.cbegin(), 2, 3.5);
-    assert(i == c.begin());
-    assert(c.size() == 1);
-    assert(c.front().geti() == 2);
-    assert(c.front().getd() == 3.5);
-    assert(is_contiguous_container_asan_correct(c));
-    i = c.emplace(c.cend(), 3, 4.5);
-    assert(i == c.end() - 1);
-    assert(c.size() == 2);
-    assert(c.front().geti() == 2);
-    assert(c.front().getd() == 3.5);
-    assert(c.back().geti() == 3);
-    assert(c.back().getd() == 4.5);
-    assert(is_contiguous_container_asan_correct(c));
-    i = c.emplace(c.cbegin() + 1, 4, 6.5);
-    assert(i == c.begin() + 1);
-    assert(c.size() == 3);
-    assert(c.front().geti() == 2);
-    assert(c.front().getd() == 3.5);
-    assert(c[1].geti() == 4);
-    assert(c[1].getd() == 6.5);
-    assert(c.back().geti() == 3);
-    assert(c.back().getd() == 4.5);
-    assert(is_contiguous_container_asan_correct(c));
+    Vector v;
+    ASSERT_SAME_TYPE(decltype(v.emplace(v.cbegin(), 1)), Iterator);
   }
+
+  // Emplace at the end of a vector with increasing size
   {
-    std::vector<A, limited_allocator<A, 7> > c;
-    std::vector<A, limited_allocator<A, 7> >::iterator i = c.emplace(c.cbegin(), 2, 3.5);
-    assert(i == c.begin());
-    assert(c.size() == 1);
-    assert(c.front().geti() == 2);
-    assert(c.front().getd() == 3.5);
-    assert(is_contiguous_container_asan_correct(c));
-    i = c.emplace(c.cend(), 3, 4.5);
-    assert(i == c.end() - 1);
-    assert(c.size() == 2);
-    assert(c.front().geti() == 2);
-    assert(c.front().getd() == 3.5);
-    assert(c.back().geti() == 3);
-    assert(c.back().getd() == 4.5);
-    assert(is_contiguous_container_asan_correct(c));
-    i = c.emplace(c.cbegin() + 1, 4, 6.5);
-    assert(i == c.begin() + 1);
-    assert(c.size() == 3);
-    assert(c.front().geti() == 2);
-    assert(c.front().getd() == 3.5);
-    assert(c[1].geti() == 4);
-    assert(c[1].getd() == 6.5);
-    assert(c.back().geti() == 3);
-    assert(c.back().getd() == 4.5);
-    assert(is_contiguous_container_asan_correct(c));
+    Vector v;
+
+    // starts with size 0
+    {
+      Iterator it = v.emplace(v.cend(), 0);
+      assert(it == v.end() - 1);
+      assert(v.size() == 1);
+      assert(v[0] == T(0));
+      assert(is_contiguous_container_asan_correct(v));
+    }
+
+    // starts with size 1
+    {
+      Iterator it = v.emplace(v.cend(), 1);
+      assert(it == v.end() - 1);
+      assert(v.size() == 2);
+      assert(v[0] == T(0));
+      assert(v[1] == T(1));
+      assert(is_contiguous_container_asan_correct(v));
+    }
+
+    // starts with size 2
+    {
+      Iterator it = v.emplace(v.cend(), 2);
+      assert(it == v.end() - 1);
+      assert(v.size() == 3);
+      assert(v[0] == T(0));
+      assert(v[1] == T(1));
+      assert(v[2] == T(2));
+      assert(is_contiguous_container_asan_correct(v));
+    }
+
+    // starts with size n...
+    for (std::size_t n = 3; n != 100; ++n) {
+      Iterator it = v.emplace(v.cend(), n);
+      assert(it == v.end() - 1);
+      assert(v.size() == n + 1);
+      for (std::size_t i = 0; i != n + 1; ++i)
+        assert(v[i] == T(i));
+      assert(is_contiguous_container_asan_correct(v));
+    }
   }
+
+  // Emplace at the start of a vector with increasing size
   {
-    std::vector<A, min_allocator<A> > c;
-    std::vector<A, min_allocator<A> >::iterator i = c.emplace(c.cbegin(), 2, 3.5);
-    assert(i == c.begin());
-    assert(c.size() == 1);
-    assert(c.front().geti() == 2);
-    assert(c.front().getd() == 3.5);
-    i = c.emplace(c.cend(), 3, 4.5);
-    assert(i == c.end() - 1);
-    assert(c.size() == 2);
-    assert(c.front().geti() == 2);
-    assert(c.front().getd() == 3.5);
-    assert(c.back().geti() == 3);
-    assert(c.back().getd() == 4.5);
-    i = c.emplace(c.cbegin() + 1, 4, 6.5);
-    assert(i == c.begin() + 1);
-    assert(c.size() == 3);
-    assert(c.front().geti() == 2);
-    assert(c.front().getd() == 3.5);
-    assert(c[1].geti() == 4);
-    assert(c[1].getd() == 6.5);
-    assert(c.back().geti() == 3);
-    assert(c.back().getd() == 4.5);
+    Vector v;
+
+    // starts with size 0
+    {
+      Iterator it = v.emplace(v.cbegin(), 0);
+      assert(it == v.begin());
+      assert(v.size() == 1);
+      assert(v[0] == T(0));
+      assert(is_contiguous_container_asan_correct(v));
+    }
+
+    // starts with size 1
+    {
+      Iterator it = v.emplace(v.cbegin(), 1);
+      assert(it == v.begin());
+      assert(v.size() == 2);
+      assert(v[0] == T(1));
+      assert(v[1] == T(0));
+      assert(is_contiguous_container_asan_correct(v));
+    }
+
+    // starts with size 2
+    {
+      Iterator it = v.emplace(v.cbegin(), 2);
+      assert(it == v.begin());
+      assert(v.size() == 3);
+      assert(v[0] == T(2));
+      assert(v[1] == T(1));
+      assert(v[2] == T(0));
+      assert(is_contiguous_container_asan_correct(v));
+    }
+
+    // starts with size n...
+    for (std::size_t n = 3; n != 100; ++n) {
+      Iterator it = v.emplace(v.cbegin(), n);
+      assert(it == v.begin());
+      assert(v.size() == n + 1);
+      for (std::size_t i = 0; i != n + 1; ++i)
+        assert(v[i] == T(n - i));
+      assert(is_contiguous_container_asan_correct(v));
+    }
   }
+
+  // Emplace somewhere inside the vector
   {
-    std::vector<A, safe_allocator<A> > c;
-    std::vector<A, safe_allocator<A> >::iterator i = c.emplace(c.cbegin(), 2, 3.5);
-    assert(i == c.begin());
-    assert(c.size() == 1);
-    assert(c.front().geti() == 2);
-    assert(c.front().getd() == 3.5);
-    i = c.emplace(c.cend(), 3, 4.5);
-    assert(i == c.end() - 1);
-    assert(c.size() == 2);
-    assert(c.front().geti() == 2);
-    assert(c.front().getd() == 3.5);
-    assert(c.back().geti() == 3);
-    assert(c.back().getd() == 4.5);
-    i = c.emplace(c.cbegin() + 1, 4, 6.5);
-    assert(i == c.begin() + 1);
-    assert(c.size() == 3);
-    assert(c.front().geti() == 2);
-    assert(c.front().getd() == 3.5);
-    assert(c[1].geti() == 4);
-    assert(c[1].getd() == 6.5);
-    assert(c.back().geti() == 3);
-    assert(c.back().getd() == 4.5);
+    Vector v;
+    v.emplace_back(0);
+    v.emplace_back(1);
+    v.emplace_back(2);
+    // vector is {0, 1, 2}
+
+    {
+      Iterator it = v.emplace(v.cbegin() + 1, 3);
+      // vector is {0, 3, 1, 2}
+      assert(it == v.begin() + 1);
+      assert(v.size() == 4);
+      assert(v[0] == T(0));
+      assert(v[1] == T(3));
+      assert(v[2] == T(1));
+      assert(v[3] == T(2));
+      assert(is_contiguous_container_asan_correct(v));
+    }
+
+    {
+      Iterator it = v.emplace(v.cbegin() + 2, 4);
+      // vector is {0, 3, 4, 1, 2}
+      assert(it == v.begin() + 2);
+      assert(v.size() == 5);
+      assert(v[0] == T(0));
+      assert(v[1] == T(3));
+      assert(v[2] == T(4));
+      assert(v[3] == T(1));
+      assert(v[4] == T(2));
+      assert(is_contiguous_container_asan_correct(v));
+    }
   }
 
+  // Emplace after reserving
+  {
+    Vector v;
+    v.emplace_back(0);
+    v.emplace_back(1);
+    v.emplace_back(2);
+    // vector is {0, 1, 2}
+
+    v.reserve(1000);
+    Iterator it = v.emplace(v.cbegin() + 1, 3);
+    assert(it == v.begin() + 1);
+    assert(v.size() == 4);
+    assert(v[0] == T(0));
+    assert(v[1] == T(3));
+    assert(v[2] == T(1));
+    assert(v[3] == T(2));
+    assert(is_contiguous_container_asan_correct(v));
+  }
+
+  // Emplace with the same type that's stored in the vector (as opposed to just constructor arguments)
+  {
+    Vector v;
+    Iterator it = v.emplace(v.cbegin(), T(1));
+    assert(it == v.begin());
+    assert(v.size() == 1);
+    assert(v[0] == T(1));
+    assert(is_contiguous_container_asan_correct(v));
+  }
+
+  // Emplace from an element inside the vector itself. This is interesting for two reasons. First, if the
+  // vector must increase capacity, the implementation needs to make sure that it doesn't end up inserting
+  // from a dangling reference.
+  //
+  // Second, if the vector doesn't need to grow but its elements get shifted internally, the implementation
+  // must make sure that it doesn't end up inserting from an element whose position has changed.
+  {
+    // When capacity must increase
+    {
+      Vector v;
+      v.emplace_back(0);
+      v.emplace_back(1);
+
+      while (v.size() < v.capacity()) {
+        v.emplace_back(2);
+      }
+      assert(v.size() == v.capacity());
+      // vector is {0, 1, 2...}
+
+      std::size_t old_cap = v.capacity();
+      v.emplace(v.cbegin(), std::move(v[1]));
+      assert(v.capacity() > old_cap); // test the test
+
+      // vector is {1, 0, -1, 2...}
+      // Note that old v[1] has been set to -1 when it was moved-from
+      assert(v.size() >= 3);
+      assert(v[0] == T(1));
+      assert(v[1] == T(0));
+      if (has_moved_from_sentinel<T>::value)
+        assert(v[2] == T(-1));
+      assert(is_contiguous_container_asan_correct(v));
+    }
+
+    // When elements shift around
+    {
+      Vector v;
+      v.emplace_back(0);
+      v.emplace_back(1);
+      // vector is {0, 1}
+
+      v.reserve(3);
+      std::size_t old_cap = v.capacity();
+      v.emplace(v.cbegin(), std::move(v[1]));
+      assert(v.capacity() == old_cap); // test the test
+
+      // vector is {1, 0, -1}
+      // Note that old v[1] has been set to -1 when it was moved-from
+      assert(v.size() == 3);
+      assert(v[0] == T(1));
+      assert(v[1] == T(0));
+      if (has_moved_from_sentinel<T>::value)
+        assert(v[2] == T(-1));
+      assert(is_contiguous_container_asan_correct(v));
+    }
+  }
+
+  // Make sure that we don't reallocate when we have sufficient capacity
+  {
+    Vector v;
+    v.reserve(8);
+    assert(v.capacity() >= 8);
+
+    std::size_t old_capacity = v.capacity();
+    v.emplace_back(0);
+    v.emplace_back(1);
+    v.emplace_back(2);
+    v.emplace_back(3);
+    assert(v.capacity() == old_capacity);
+
+    v.emplace(v.cend(), 4);
+    assert(v.size() == 5);
+    assert(v.capacity() == old_capacity);
+    assert(v[0] == T(0));
+    assert(v[1] == T(1));
+    assert(v[2] == T(2));
+    assert(v[3] == T(3));
+    assert(v[4] == T(4));
+    assert(is_contiguous_container_asan_correct(v));
+  }
+
+  // Make sure that we correctly handle the case where an exception would be thrown if moving the element into place.
+  // This is a very specific test that aims to validate that the implementation doesn't create a temporary object e.g.
+  // on the stack and then moves it into its final location inside the newly allocated vector storage.
+  //
+  // If that were the case, and if the element happened to throw upon move construction or move assignment into its
+  // final location, we would have invalidated iterators despite the overall emplace being required to have the
+  // strong exception safety guarantee.
----------------
winner245 wrote:

This comment itself seems a bit confusing to me. IIUC, strong exception safety guarantee for insertion functions is waived if an exception is thrown by the move constructor when the throwing move constructor is used during insertion ([[vector.modifiers]/2](https://eel.is/c++draft/vector.modifiers#2)). For example, for a move-only element type with a throwing move constructor, the effects are unspecified after insertion.  

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


More information about the libcxx-commits mailing list