[libc++] Improve the tests for vector::erase

libcxx/test/benchmarks/ContainerBenchmarks.h

@@ -11,6 +11,8 @@
 #define BENCHMARK_CONTAINER_BENCHMARKS_H
 
 #include <cassert>
+#include <iterator>
+#include <utility>
 
 #include "Utilities.h"
 #include "benchmark/benchmark.h"
@@ -149,6 +151,34 @@ void BM_EmplaceDuplicate(benchmark::State& st, Container c, GenInputs gen) {
   }
 }
 
+template <class Container, class GenInputs>
+void BM_erase_iter_in_middle(benchmark::State& st, Container, GenInputs gen) {
+  auto in = gen(st.range(0));
+  Container c(in.begin(), in.end());
+  assert(c.size() > 2);
+  for (auto _ : st) {
+    auto mid    = std::next(c.begin(), c.size() / 2);
+    auto tmp    = *mid;
+    auto result = c.erase(mid); // erase an element in the middle
+    benchmark::DoNotOptimize(result);
+    c.push_back(std::move(tmp)); // and then push it back at the end to avoid needing a new container
+  }
+}
+
+template <class Container, class GenInputs>
+void BM_erase_iter_at_start(benchmark::State& st, Container, GenInputs gen) {
+  auto in = gen(st.range(0));
+  Container c(in.begin(), in.end());
+  assert(c.size() > 2);
+  for (auto _ : st) {
+    auto it     = c.begin();
+    auto tmp    = *it;
+    auto result = c.erase(it); // erase the first element
+    benchmark::DoNotOptimize(result);
+    c.push_back(std::move(tmp)); // and then push it back at the end to avoid needing a new container
+  }
+}
+
 template <class Container, class GenInputs>
 void BM_Find(benchmark::State& st, Container c, GenInputs gen) {
   auto in = gen(st.range(0));

libcxx/test/benchmarks/deque.bench.cpp

@@ -9,6 +9,7 @@
 // UNSUPPORTED: c++03, c++11, c++14, c++17, c++20
 
 #include <deque>
+#include <string>
 
 #include "benchmark/benchmark.h"
 
@@ -41,4 +42,14 @@ BENCHMARK_CAPTURE(BM_ConstructFromRange, deque_size_t, std::deque<size_t>{}, get
 BENCHMARK_CAPTURE(BM_ConstructFromRange, deque_string, std::deque<std::string>{}, getRandomStringInputs)
     ->Arg(TestNumInputs);
 
+BENCHMARK_CAPTURE(BM_erase_iter_in_middle, deque_int, std::deque<int>{}, getRandomIntegerInputs<int>)
+    ->Range(TestNumInputs, TestNumInputs * 10);
+BENCHMARK_CAPTURE(BM_erase_iter_in_middle, deque_string, std::deque<std::string>{}, getRandomStringInputs)
+    ->Range(TestNumInputs, TestNumInputs * 10);
+
+BENCHMARK_CAPTURE(BM_erase_iter_at_start, deque_int, std::deque<int>{}, getRandomIntegerInputs<int>)
+    ->Range(TestNumInputs, TestNumInputs * 10);
+BENCHMARK_CAPTURE(BM_erase_iter_at_start, deque_string, std::deque<std::string>{}, getRandomStringInputs)
+    ->Range(TestNumInputs, TestNumInputs * 10);
+
 BENCHMARK_MAIN();

libcxx/test/benchmarks/vector_operations.bench.cpp

@@ -54,6 +54,16 @@ BENCHMARK_CAPTURE(BM_ConstructFromRange, vector_string, std::vector<std::string>
 
 BENCHMARK_CAPTURE(BM_Pushback_no_grow, vector_int, std::vector<int>{})->Arg(TestNumInputs);
 
+BENCHMARK_CAPTURE(BM_erase_iter_in_middle, vector_int, std::vector<int>{}, getRandomIntegerInputs<int>)
+    ->Range(TestNumInputs, TestNumInputs * 10);
+BENCHMARK_CAPTURE(BM_erase_iter_in_middle, vector_string, std::vector<std::string>{}, getRandomStringInputs)
+    ->Range(TestNumInputs, TestNumInputs * 10);
+
+BENCHMARK_CAPTURE(BM_erase_iter_at_start, vector_int, std::vector<int>{}, getRandomIntegerInputs<int>)
+    ->Range(TestNumInputs, TestNumInputs * 10);
+BENCHMARK_CAPTURE(BM_erase_iter_at_start, vector_string, std::vector<std::string>{}, getRandomStringInputs)
+    ->Range(TestNumInputs, TestNumInputs * 10);
+
 template <class T>
 void bm_grow(benchmark::State& state) {
   for (auto _ : state) {

libcxx/test/std/containers/sequences/vector/vector.modifiers/common.h

@@ -0,0 +1,86 @@
+//===----------------------------------------------------------------------===//
+//
+// 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
+//
+//===----------------------------------------------------------------------===//
+
+#ifndef TEST_STD_CONTAINERS_SEQUENCES_VECTOR_VECTOR_MODIFIERS_COMMON_H
+#define TEST_STD_CONTAINERS_SEQUENCES_VECTOR_VECTOR_MODIFIERS_COMMON_H
+
+#include "test_macros.h"
+
+#include <type_traits> // for __libcpp_is_trivially_relocatable
+
+#ifndef TEST_HAS_NO_EXCEPTIONS
+struct Throws {
+  Throws() : v_(0) {}
+  Throws(int v) : v_(v) {}
+  Throws(const Throws& rhs) : v_(rhs.v_) {
+    if (sThrows)
+      throw 1;
+  }
+  Throws(Throws&& rhs) : v_(rhs.v_) {
+    if (sThrows)
+      throw 1;
+  }
+  Throws& operator=(const Throws& rhs) {
+    v_ = rhs.v_;
+    return *this;
+  }
+  Throws& operator=(Throws&& rhs) {
+    v_ = rhs.v_;
+    return *this;
+  }
+  int v_;
+  static bool sThrows;
+};
+
+bool Throws::sThrows = false;
+#endif
+
+struct Tracker {
+  int copy_assignments = 0;
+  int move_assignments = 0;
+};
+
+struct TrackedAssignment {
+  Tracker* tracker_;
+  TEST_CONSTEXPR_CXX14 explicit TrackedAssignment(Tracker* tracker) : tracker_(tracker) {}
+
+  TrackedAssignment(TrackedAssignment const&) = default;
+  TrackedAssignment(TrackedAssignment&&)      = default;
+
+  TEST_CONSTEXPR_CXX14 TrackedAssignment& operator=(TrackedAssignment const&) {
+    tracker_->copy_assignments++;
+    return *this;
+  }
+  TEST_CONSTEXPR_CXX14 TrackedAssignment& operator=(TrackedAssignment&&) {
+    tracker_->move_assignments++;
+    return *this;
+  }
+};
+
+struct NonTriviallyRelocatable {
+  int value_;
+  TEST_CONSTEXPR NonTriviallyRelocatable() : value_(0) {}
+  TEST_CONSTEXPR explicit NonTriviallyRelocatable(int v) : value_(v) {}
+  TEST_CONSTEXPR NonTriviallyRelocatable(NonTriviallyRelocatable const& other) : value_(other.value_) {}
+  TEST_CONSTEXPR NonTriviallyRelocatable(NonTriviallyRelocatable&& other) : value_(other.value_) {}
+  TEST_CONSTEXPR_CXX14 NonTriviallyRelocatable& operator=(NonTriviallyRelocatable const& other) {
+    value_ = other.value_;
+    return *this;
+  }
+  TEST_CONSTEXPR_CXX14 NonTriviallyRelocatable& operator=(NonTriviallyRelocatable&& other) {
+    value_ = other.value_;
+    return *this;
+  }
+
+  TEST_CONSTEXPR_CXX14 friend bool operator==(NonTriviallyRelocatable const& a, NonTriviallyRelocatable const& b) {
+    return a.value_ == b.value_;
+  }
+};
+LIBCPP_STATIC_ASSERT(!std::__libcpp_is_trivially_relocatable<NonTriviallyRelocatable>::value, "");
+
+#endif // TEST_STD_CONTAINERS_SEQUENCES_VECTOR_VECTOR_MODIFIERS_COMMON_H

libcxx/test/std/containers/sequences/vector/vector.modifiers/erase_iter.pass.cpp

@@ -11,135 +11,79 @@
 // iterator erase(const_iterator position);
 
 #include <vector>
-#include <iterator>
 #include <cassert>
+#include <memory>
 
 #include "asan_testing.h"
+#include "common.h"
 #include "min_allocator.h"
 #include "MoveOnly.h"
 #include "test_macros.h"
 
-#ifndef TEST_HAS_NO_EXCEPTIONS
-struct Throws {
-  Throws() : v_(0) {}
-  Throws(int v) : v_(v) {}
-  Throws(const Throws& rhs) : v_(rhs.v_) {
-    if (sThrows)
-      throw 1;
-  }
-  Throws(Throws&& rhs) : v_(rhs.v_) {
-    if (sThrows)
-      throw 1;
-  }
-  Throws& operator=(const Throws& rhs) {
-    v_ = rhs.v_;
-    return *this;
-  }
-  Throws& operator=(Throws&& rhs) {
-    v_ = rhs.v_;
-    return *this;
-  }
-  int v_;
-  static bool sThrows;
-};
-
-bool Throws::sThrows = false;
-#endif
-
-TEST_CONSTEXPR_CXX20 bool tests() {
-  {
-    int a1[] = {1, 2, 3, 4, 5};
-    std::vector<int> l1(a1, a1 + 5);
-    l1.erase(l1.begin());
-    assert(is_contiguous_container_asan_correct(l1));
-    assert(l1 == std::vector<int>(a1 + 1, a1 + 5));
-  }
+template <template <class> class Allocator, class T>
+TEST_CONSTEXPR_CXX20 void tests() {
   {
-    int a1[] = {1, 2, 3, 4, 5};
-    int e1[] = {1, 3, 4, 5};
-    std::vector<int> l1(a1, a1 + 5);
-    l1.erase(l1.begin() + 1);
-    assert(is_contiguous_container_asan_correct(l1));
-    assert(l1 == std::vector<int>(e1, e1 + 4));
-  }
-  {
-    int a1[] = {1, 2, 3};
-    std::vector<int> l1(a1, a1 + 3);
-    std::vector<int>::const_iterator i = l1.begin();
-    assert(is_contiguous_container_asan_correct(l1));
-    ++i;
-    std::vector<int>::iterator j = l1.erase(i);
-    assert(l1.size() == 2);
-    assert(std::distance(l1.begin(), l1.end()) == 2);
-    assert(*j == 3);
-    assert(*l1.begin() == 1);
-    assert(*std::next(l1.begin()) == 3);
-    assert(is_contiguous_container_asan_correct(l1));
-    j = l1.erase(j);
-    assert(j == l1.end());
-    assert(l1.size() == 1);
-    assert(std::distance(l1.begin(), l1.end()) == 1);
-    assert(*l1.begin() == 1);
-    assert(is_contiguous_container_asan_correct(l1));
-    j = l1.erase(l1.begin());
-    assert(j == l1.end());
-    assert(l1.size() == 0);
-    assert(std::distance(l1.begin(), l1.end()) == 0);
-    assert(is_contiguous_container_asan_correct(l1));
-  }
+    T arr[]             = {T(1), T(2), T(3), T(4), T(5)};
+    using Vector        = std::vector<T, Allocator<T> >;
+    using Iterator      = typename Vector::iterator;
 
-  // Make sure vector::erase works with move-only types
-  // When non-trivial
-  {
-    std::vector<MoveOnly> v;
-    v.emplace_back(1);
-    v.emplace_back(2);
-    v.emplace_back(3);
-    v.erase(v.begin());
-    assert(v.size() == 2);
-    assert(v[0] == MoveOnly(2));
-    assert(v[1] == MoveOnly(3));
-  }
-  // When trivial
-  {
-    std::vector<TrivialMoveOnly> v;
-    v.emplace_back(1);
-    v.emplace_back(2);
-    v.emplace_back(3);
-    v.erase(v.begin());
-    assert(v.size() == 2);
-    assert(v[0] == TrivialMoveOnly(2));
-    assert(v[1] == TrivialMoveOnly(3));
+    {
+      Vector v(arr, arr + 5);
+      Iterator it = v.erase(v.cbegin());
+      assert(v == Vector(arr + 1, arr + 5));
+      assert(it == v.begin());
+      assert(is_contiguous_container_asan_correct(v));
+    }
+    {
+      T expected[] = {T(1), T(3), T(4), T(5)};
+      Vector v(arr, arr + 5);
+      Iterator it = v.erase(v.cbegin() + 1);
+      assert(v == Vector(expected, expected + 4));
+      assert(it == v.begin() + 1);
+      assert(is_contiguous_container_asan_correct(v));
+    }
+    {
+      T expected[] = {T(1), T(2), T(3), T(4)};
+      Vector v(arr, arr + 5);
+      Iterator it = v.erase(v.cbegin() + 4);
+      assert(v == Vector(expected, expected + 4));
+      assert(it == v.end());
+      assert(is_contiguous_container_asan_correct(v));
+    }
   }
 
-#if TEST_STD_VER >= 11
+  // Make sure vector::erase works with move-only types
   {
-    int a1[] = {1, 2, 3};
-    std::vector<int, min_allocator<int>> l1(a1, a1 + 3);
-    std::vector<int, min_allocator<int>>::const_iterator i = l1.begin();
-    assert(is_contiguous_container_asan_correct(l1));
-    ++i;
-    std::vector<int, min_allocator<int>>::iterator j = l1.erase(i);
-    assert(l1.size() == 2);
-    assert(std::distance(l1.begin(), l1.end()) == 2);
-    assert(*j == 3);
-    assert(*l1.begin() == 1);
-    assert(*std::next(l1.begin()) == 3);
-    assert(is_contiguous_container_asan_correct(l1));
-    j = l1.erase(j);
-    assert(j == l1.end());
-    assert(l1.size() == 1);
-    assert(std::distance(l1.begin(), l1.end()) == 1);
-    assert(*l1.begin() == 1);
-    assert(is_contiguous_container_asan_correct(l1));
-    j = l1.erase(l1.begin());
-    assert(j == l1.end());
-    assert(l1.size() == 0);
-    assert(std::distance(l1.begin(), l1.end()) == 0);
-    assert(is_contiguous_container_asan_correct(l1));
+    // When non-trivial
+    {
+      std::vector<MoveOnly, Allocator<MoveOnly> > v;
+      v.emplace_back(1);
+      v.emplace_back(2);
+      v.emplace_back(3);
+      v.erase(v.begin());
+      assert(v.size() == 2);
+      assert(v[0] == MoveOnly(2));
+      assert(v[1] == MoveOnly(3));
+    }
+    // When trivial
+    {
+      std::vector<TrivialMoveOnly, Allocator<TrivialMoveOnly> > v;
+      v.emplace_back(1);
+      v.emplace_back(2);
+      v.emplace_back(3);
+      v.erase(v.begin());
+      assert(v.size() == 2);
+      assert(v[0] == TrivialMoveOnly(2));
+      assert(v[1] == TrivialMoveOnly(3));
+    }
   }
-#endif
+}
 
+TEST_CONSTEXPR_CXX20 bool tests() {
+  tests<std::allocator, int>();
+  tests<std::allocator, NonTriviallyRelocatable>();
+  tests<min_allocator, int>();
+  tests<min_allocator, NonTriviallyRelocatable>();
   return true;
 }
 
@@ -163,5 +107,31 @@ int main(int, char**) {
   }
 #endif
 
+  // Make sure we satisfy the complexity requirement in terms of the number of times the assignment
+  // operator is called.
+  //
+  // There is currently ambiguity as to whether this is truly mandated by the Standard, so we only
+  // test it for libc++.
+#ifdef _LIBCPP_VERSION
+  {
+    Tracker tracker;
+    std::vector<TrackedAssignment> v;
+
+    // Set up the vector with 5 elements.
+    for (int i = 0; i != 5; ++i) {
+      v.emplace_back(&tracker);
+    }
+    assert(tracker.copy_assignments == 0);
+    assert(tracker.move_assignments == 0);
+
+    // Erase element [1] from it. Elements [2] [3] [4] should be shifted, so we should
+    // see 3 move assignments (and nothing else).
+    v.erase(v.begin() + 1);
+    assert(v.size() == 4);
+    assert(tracker.copy_assignments == 0);
+    assert(tracker.move_assignments == 3);
+  }
+#endif
+
   return 0;
 }