[aa_inplace_vector] Fix some bugs in (unusual) allocator propagation

Quuxplusone · Aug 27, 2024 · 2406ba8 · 2406ba8
1 parent 8f1aa6f
commit 2406ba8
Show file tree

Hide file tree

Showing 3 changed files with 111 additions and 8 deletions.
diff --git a/include/sg14/aa_inplace_vector.h b/include/sg14/aa_inplace_vector.h
@@ -385,15 +385,15 @@ struct SG14_INPLACE_VECTOR_TRIVIALLY_RELOCATABLE_IF((sg14::aaipv::be_trivially_r
           std::is_trivially_destructible_v<T> &&
           sg14::aaipv::has_trivial_construct<Alloc, T, const T&>::value &&
           sg14::aaipv::has_trivial_destroy<Alloc, T>::value) || (N == 0)) &&
-        (std::allocator_traits<Alloc>::propagate_on_container_copy_assignment::value || std::allocator_traits<Alloc>::is_always_equal::value);
+        std::allocator_traits<Alloc>::is_always_equal::value;
 
     static constexpr bool MoveAssignIsDefaultable =
         ((std::is_trivially_move_constructible_v<T> &&
           std::is_trivially_move_assignable_v<T> &&
           std::is_trivially_destructible_v<T> &&
           sg14::aaipv::has_trivial_construct<Alloc, T, T&&>::value &&
           sg14::aaipv::has_trivial_destroy<Alloc, T>::value) || (N == 0)) &&
-        (std::allocator_traits<Alloc>::propagate_on_container_move_assignment::value || std::allocator_traits<Alloc>::is_always_equal::value);
+        std::allocator_traits<Alloc>::is_always_equal::value;
 
     static constexpr bool DtorIsDefaultable =
         ((std::is_trivially_destructible_v<T> &&
@@ -546,11 +546,11 @@ struct SG14_INPLACE_VECTOR_TRIVIALLY_RELOCATABLE_IF((sg14::aaipv::be_trivially_r
 #if defined(__cpp_lib_trivially_relocatable)
             size_t n = a.size_;
             a.set_size_(b.size_);
-            sg14::aaipv::uninitialized_relocate_a(get_allocator_(), a.data_ + b.size_, a.data_ + n, b.data_ + b.size_);
+            sg14::aaipv::uninitialized_relocate_a(a.get_allocator_(), a.data_ + b.size_, a.data_ + n, b.data_ + b.size_);
             b.set_size_(n);
 #else
-            sg14::aaipv::uninitialized_move_a(get_allocator_(), a.data_ + b.size_, a.data_ + a.size_, b.data_ + b.size_);
-            sg14::aaipv::destroy_a(get_allocator_(), a.data_ + b.size_, a.data_ + a.size_);
+            sg14::aaipv::uninitialized_move_a(a.get_allocator_(), a.data_ + b.size_, a.data_ + a.size_, b.data_ + b.size_);
+            sg14::aaipv::destroy_a(a.get_allocator_(), a.data_ + b.size_, a.data_ + a.size_);
             a.swap_sizes_(b);
 #endif
         }

diff --git a/test/aa_inplace_vector_smallsize_test.cpp b/test/aa_inplace_vector_smallsize_test.cpp
@@ -14,6 +14,7 @@ struct SmallSizeAllocator {
     // not-an-allocator from the Standard's point of view. SG14 is happy
     // with it, though.
 };
+static_assert(std::allocator_traits<SmallSizeAllocator<int, int>>::is_always_equal::value);
 
 namespace smallsize {
   template<class T, size_t N> using inplace_vector = sg14::inplace_vector<T, N, SmallSizeAllocator<T, unsigned char>>;

diff --git a/test/aa_inplace_vector_test.cpp b/test/aa_inplace_vector_test.cpp
@@ -3,9 +3,16 @@
 #include <exception>
 #include <gtest/gtest.h>
 #include <set>
-#include <sg14/aa_inplace_vector.h>
 #include <utility>
 
+struct AssertFail : std::runtime_error {
+    using std::runtime_error::runtime_error;
+};
+
+#define SG14_INPLACE_VECTOR_ASSERT_PRECONDITION(x, msg) if (!(x)) throw AssertFail(msg);
+
+#include <sg14/aa_inplace_vector.h>
+
 template<class T>
 struct CountingAlloc {
     using value_type = T;
@@ -50,6 +57,27 @@ struct NonNoexceptAlloc {
     int i_;
 };
 
+template<class T>
+struct PropagatingAllocator {
+    using value_type = T;
+    using propagate_on_container_copy_assignment = std::true_type;
+    using propagate_on_container_move_assignment = std::true_type;
+    using propagate_on_container_swap = std::true_type;
+    explicit PropagatingAllocator(int i) : i_(i) {}
+    int i_ = 0;
+    friend bool operator==(const PropagatingAllocator&, const PropagatingAllocator&) = default;
+
+    template<class U, class... Args>
+    void construct(U *p, Args&&... args) {
+        ::new (p) U(std::forward<Args>(args)...);
+    }
+
+    template<class U>
+    void destroy(U *p) {
+        p->~U();
+    }
+};
+
 TEST(aa_inplace_vector, AllocExtendedCopyCtor)
 {
     using A = CountingAlloc<int>;
@@ -86,8 +114,8 @@ TEST(aa_inplace_vector, IgnoreNoexceptnessOfAllocator)
         using T = sg14::inplace_vector<int, 10, NonNoexceptAlloc<int, true>>;
         static_assert(std::is_nothrow_copy_constructible_v<T>);
         static_assert(std::is_nothrow_move_constructible_v<T>);
-        static_assert(std::is_nothrow_copy_assignable_v<T>);
-        static_assert(std::is_nothrow_move_assignable_v<T>);
+        static_assert(!std::is_nothrow_copy_assignable_v<T>); // because Lakos rule
+        static_assert(!std::is_nothrow_move_assignable_v<T>); // because Lakos rule
         static_assert(std::is_nothrow_destructible_v<T>);
     }
     {
@@ -100,4 +128,78 @@ TEST(aa_inplace_vector, IgnoreNoexceptnessOfAllocator)
     }
 }
 
+TEST(aa_inplace_vector, SwapAllocators)
+{
+    using A = PropagatingAllocator<int>;
+    static_assert(std::allocator_traits<A>::propagate_on_container_swap::value);
+    {
+        using T = sg14::inplace_vector<int, 4, A>;
+        T a = T(A(1));
+        T b = T(A(1));
+        a.swap(b);
+        swap(a, b);
+        T c = T(A(2));
+        try { a.swap(c); EXPECT_TRUE(false); } catch (const AssertFail& ex) { EXPECT_STREQ(ex.what(), "swap tried to swap unequal allocators; this is UB"); }
+        try { swap(a, c); EXPECT_TRUE(false); } catch (const AssertFail& ex) { EXPECT_STREQ(ex.what(), "swap tried to swap unequal allocators; this is UB"); }
+    }
+    {
+        using T = sg14::inplace_vector<int, 0, A>;
+        T a = T(A(1));
+        T b = T(A(1));
+        a.swap(b);
+        swap(a, b);
+        T c = T(A(2));
+        try { a.swap(c); EXPECT_TRUE(false); } catch (const AssertFail& ex) { EXPECT_STREQ(ex.what(), "swap tried to swap unequal allocators; this is UB"); }
+        try { swap(a, c); EXPECT_TRUE(false); } catch (const AssertFail& ex) { EXPECT_STREQ(ex.what(), "swap tried to swap unequal allocators; this is UB"); }
+    }
+}
+
+TEST(aa_inplace_vector, CopyAssignAllocators)
+{
+    using A = PropagatingAllocator<int>;
+    static_assert(std::allocator_traits<A>::propagate_on_container_copy_assignment::value);
+    {
+        using T = sg14::inplace_vector<int, 4, A>;
+        static_assert(!std::is_trivially_copy_assignable_v<T>);
+        T a = T(A(1));
+        T b = T(A(1));
+        a = b;
+        T c = T(A(2));
+        try { a = c; EXPECT_TRUE(false); } catch (const AssertFail& ex) { EXPECT_STREQ(ex.what(), "operator= tried to propagate an unequal allocator; this is UB"); }
+    }
+    {
+        using T = sg14::inplace_vector<int, 0, A>;
+        static_assert(!std::is_trivially_copy_assignable_v<T>);
+        T a = T(A(1));
+        T b = T(A(1));
+        a = b;
+        T c = T(A(2));
+        try { a = c; EXPECT_TRUE(false); } catch (const AssertFail& ex) { EXPECT_STREQ(ex.what(), "operator= tried to propagate an unequal allocator; this is UB"); }
+    }
+}
+
+TEST(aa_inplace_vector, MoveAssignAllocators)
+{
+    using A = PropagatingAllocator<int>;
+    static_assert(std::allocator_traits<A>::propagate_on_container_move_assignment::value);
+    {
+        using T = sg14::inplace_vector<int, 4, A>;
+        static_assert(!std::is_trivially_move_assignable_v<T>);
+        T a = T(A(1));
+        T b = T(A(1));
+        a = std::move(b);
+        T c = T(A(2));
+        try { a = std::move(c); EXPECT_TRUE(false); } catch (const AssertFail& ex) { EXPECT_STREQ(ex.what(), "operator= tried to propagate an unequal allocator; this is UB"); }
+    }
+    {
+        using T = sg14::inplace_vector<int, 0, A>;
+        static_assert(!std::is_trivially_move_assignable_v<T>);
+        T a = T(A(1));
+        T b = T(A(1));
+        a = std::move(b);
+        T c = T(A(2));
+        try { a = std::move(c); EXPECT_TRUE(false); } catch (const AssertFail& ex) { EXPECT_STREQ(ex.what(), "operator= tried to propagate an unequal allocator; this is UB"); }
+    }
+}
+
 #endif // __cplusplus >= 202002L