[libcxx-commits] [libcxx] [libc++] Improve the test coverage for std::vector::emplace (PR #132440)
via libcxx-commits
libcxx-commits at lists.llvm.org
Fri Mar 21 11:12:40 PDT 2025
llvmbot wrote:
<!--LLVM PR SUMMARY COMMENT-->
@llvm/pr-subscribers-libcxx
Author: Louis Dionne (ldionne)
<details>
<summary>Changes</summary>
This patch refactors the test for std::vector::emplace back to cover new corner cases, and increase coverage for normal cases as well. This is in preparation for reworking the implementation of emplace.
---
Full diff: https://github.com/llvm/llvm-project/pull/132440.diff
3 Files Affected:
- (modified) libcxx/test/std/containers/sequences/vector/vector.modifiers/common.h (+29-1)
- (modified) libcxx/test/std/containers/sequences/vector/vector.modifiers/emplace.pass.cpp (+306-117)
- (removed) libcxx/test/std/containers/sequences/vector/vector.modifiers/emplace_extra.pass.cpp (-101)
``````````diff
diff --git a/libcxx/test/std/containers/sequences/vector/vector.modifiers/common.h b/libcxx/test/std/containers/sequences/vector/vector.modifiers/common.h
index 72cd47a50b2c0..92bea5997337a 100644
--- a/libcxx/test/std/containers/sequences/vector/vector.modifiers/common.h
+++ b/libcxx/test/std/containers/sequences/vector/vector.modifiers/common.h
@@ -38,7 +38,35 @@ struct Throws {
};
bool Throws::sThrows = false;
-#endif
+
+struct ThrowingMove {
+ TEST_CONSTEXPR ThrowingMove() : value(0), do_throw(false) {}
+ TEST_CONSTEXPR explicit ThrowingMove(int v) : value(v), do_throw(false) {}
+ TEST_CONSTEXPR explicit ThrowingMove(int v, bool do_throw) : value(v), do_throw(do_throw) {}
+
+ ThrowingMove(const ThrowingMove& rhs) = default;
+ ThrowingMove& operator=(const ThrowingMove&) = default;
+
+ TEST_CONSTEXPR_CXX14 ThrowingMove(ThrowingMove&& rhs) : value(rhs.value), do_throw(rhs.do_throw) {
+ if (do_throw)
+ throw 1;
+ }
+ TEST_CONSTEXPR_CXX14 ThrowingMove& operator=(ThrowingMove&& rhs) {
+ value = rhs.value;
+ do_throw = rhs.do_throw;
+ if (do_throw)
+ throw 1;
+ return *this;
+ }
+
+ TEST_CONSTEXPR_CXX14 friend bool operator==(ThrowingMove const& lhs, ThrowingMove const& rhs) {
+ return lhs.value == rhs.value;
+ }
+
+ int value;
+ bool do_throw;
+};
+#endif // TEST_HAS_NO_EXCEPTIONS
struct Tracker {
int copy_assignments = 0;
diff --git a/libcxx/test/std/containers/sequences/vector/vector.modifiers/emplace.pass.cpp b/libcxx/test/std/containers/sequences/vector/vector.modifiers/emplace.pass.cpp
index 667ce483806e7..3431f4336d1e2 100644
--- a/libcxx/test/std/containers/sequences/vector/vector.modifiers/emplace.pass.cpp
+++ b/libcxx/test/std/containers/sequences/vector/vector.modifiers/emplace.pass.cpp
@@ -7,150 +7,339 @@
//===----------------------------------------------------------------------===//
// 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 NonCopyable {
+ int i;
- A(const A&);
- A& operator=(const A&);
+ TEST_CONSTEXPR_CXX14 explicit NonCopyable(int i) : i(i) {}
-public:
- TEST_CONSTEXPR_CXX14 A(int i, double d) : i_(i), d_(d) {}
+ NonCopyable(NonCopyable const&) = delete;
+ NonCopyable& operator=(NonCopyable const&) = delete;
- TEST_CONSTEXPR_CXX14 A(A&& a) : i_(a.i_), d_(a.d_) {
- a.i_ = 0;
- a.d_ = 0;
- }
+ TEST_CONSTEXPR_CXX14 NonCopyable(NonCopyable&& other) : i(other.i) { other.i = -1; }
- TEST_CONSTEXPR_CXX14 A& operator=(A&& a) {
- i_ = a.i_;
- d_ = a.d_;
- a.i_ = 0;
- a.d_ = 0;
+ TEST_CONSTEXPR_CXX14 NonCopyable& operator=(NonCopyable&& other) {
+ i = other.i;
+ other.i = -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==(NonCopyable const& lhs, NonCopyable const& rhs) { return lhs.i == rhs.i; }
};
-TEST_CONSTEXPR_CXX20 bool tests() {
+template <class T>
+struct has_moved_from_sentinel : std::false_type {};
+
+template <>
+struct has_moved_from_sentinel<NonCopyable> : 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
+ {
+ Vector v;
+ ASSERT_SAME_TYPE(decltype(v.emplace(v.cbegin(), 1)), Iterator);
+ }
+
+ // Emplace at the end of a vector with increasing size
+ {
+ 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
+ {
+ 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> 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;
+ 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
{
- 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;
+ 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)
{
- 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;
+ 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.
{
- 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);
+ // 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.
+ //
+ // Instead, a conforming implementation needs to emplace the new element into its final location immediately, and
+ // only after this has been done, then start making non-reversible changes to the vector's underlying storage.
+#ifndef TEST_HAS_NO_EXCEPTIONS
+ {
+ std::vector<ThrowingMove, Allocator<ThrowingMove> > v;
+ v.emplace_back(0, /* do throw */ false);
+ v.emplace_back(1, /* do throw */ false);
+
+ while (v.size() < v.capacity()) {
+ v.emplace_back(2, /* do throw */ false);
+ }
+ assert(v.size() == v.capacity()); // the next emplace will be forced to invalidate iterators
+
+ v.emplace(v.cend(), 3, /* do throw */ true); // this shouldn't throw since we shouldn't move
+
+ assert(v.size() >= 3);
+ assert(v[0] == ThrowingMove(0));
+ assert(v[1] == ThrowingMove(1));
+ assert(v.back() == ThrowingMove(3));
+ assert(is_contiguous_container_asan_correct(v));
+ }
+#endif // TEST_HAS_NO_EXCEPTIONS
+}
+
+TEST_CONSTEXPR_CXX20 bool tests() {
+ test<std::allocator, int>();
+ test<min_allocator, int>();
+ test<safe_allocator, int>();
+
+ test<std::allocator, NonCopyable>();
+ test<min_allocator, NonCopyable>();
+ test<safe_allocator, NonCopyable>();
+
+ test<std::allocator, NonTriviallyRelocatable>();
+ test<min_allocator, NonTriviallyRelocatable>();
+ test<safe_allocator, NonTriviallyRelocatable>();
+
+ // test<limited_allocator<int, 7> >();
return true;
}
diff --git a/libcxx/test/std/containers/sequences/vector/vector.modifiers/emplace_extra.pass.cpp b/libcxx/test/std/containers/sequences/vector/vector.modifiers/emplace_extra.pass.cpp
deleted file mode 100644
index 43990b148cad6..0000000000000
--- a/libcxx/test/std/containers/sequences/vector/vector.modifiers/emplace_extra.pass.cpp
+++ /dev/null
@@ -1,101 +0,0 @@
-//===----------------------------------------------------------------------===//
-//
-// 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
-//
-//===----------------------------------------------------------------------===//
-
-// UNSUPPORTED: c++03
-
-// <vector>
-
-// template <class... Args> iterator emplace(const_iterator pos, Args&&... args);
-
-#include <vector>
-#include <cassert>
-
-#include "test_macros.h"
-#include "min_allocator.h"
-#include "asan_testing.h"
-
-TEST_CONSTEXPR_CXX20 bool tests() {
- {
- std::vector<int> v;
- v.reserve(3);
- assert(is_contiguous_container_asan_correct(v));
- v = {1, 2, 3};
- v.emplace(v.begin(), v.back());
- assert(v[0] == 3);
- assert(is_contiguous_container_asan_correct(v));
- }
- {
- std::vector<int> v;
- v.reserve(4);
- assert(is_contiguous_container_asan_correct(v));
- v = {1, 2, 3};
- v.emplace(v.begin(), v.back());
- assert(v[0] == 3);
- assert(is_contiguous_container_asan_correct(v));
- }
- {
- std::vector<int, min_allocator<int>> v;
- v.reserve(3);
- assert(is_contiguous_container_asan_correct(v));
- v = {1, 2, 3};
- v.emplace(v.begin(), v.back());
- assert(v[0] == 3);
- assert(is_contiguous_container_asan_correct(v));
- }
- {
- std::vector<int, min_allocator<int>> v;
- v.reserve(4);
- assert(is_contiguous_container_asan_correct(v));
- v = {1, 2, 3};
- v.emplace(v.begin(), v.back());
- assert(v[0] == 3);
- assert(is_contiguous_container_asan_correct(v));
- }
- {
- std::vector<int, safe_allocator<int>> v;
- v.reserve(3);
- assert(is_contiguous_container_asan_correct(v));
- v = {1, 2, 3};
- v.emplace(v.begin(), v.back());
- assert(v[0] == 3);
- assert(is_contiguous_container_asan_correct(v));
- }
- {
- std::vector<int, safe_allocator<int>> v;
- v.reserve(4);
- assert(is_contiguous_container_asan_correct(v));
- v = {1, 2, 3};
- v.emplace(v.begin(), v.back());
- assert(v[0] == 3);
- assert(is_contiguous_container_asan_correct(v));
- }
- {
- std::vector<int> v;
- v.reserve(8);
- std::size_t old_capacity = v.capacity();
- assert(old_capacity >= 8);
-
- v.resize(4); // keep the existing capacity
- assert(v.capacity() == old_capacity);
-
- v.emplace(v.cend(), 42);
- assert(v.size() == 5);
- assert(v.capacity() == old_capacity);
- assert(v[4] == 42);
- }
-
- return true;
-}
-
-int main(int, char**) {
- tests();
-#if TEST_STD_VER > 17
- static_assert(tests());
-#endif
- return 0;
-}
``````````
</details>
https://github.com/llvm/llvm-project/pull/132440
More information about the libcxx-commits
mailing list