[libcxx-commits] [libcxx] [libc++] Avoid discarding const-ness when copy-constructing vectors (PR #69988)

Louis Dionne via libcxx-commits libcxx-commits at lists.llvm.org
Mon Oct 23 17:20:13 PDT 2023


https://github.com/ldionne created https://github.com/llvm/llvm-project/pull/69988

Copy-constructing a std::vector<T> currently discards the const-ness of the elements of the source vector. So for example, if the elements in the vector have both `T(T&)` and `T(T const&)` constructors, the non-const constructor is going to be preferred. This patch fixes that and makes sure that constness is respected by the copy constructor and copy assignment operators.

rdar://107652763

>From 6472669ecf8e3ad5f37aeab9734437aec5b79286 Mon Sep 17 00:00:00 2001
From: Louis Dionne <ldionne.2 at gmail.com>
Date: Mon, 23 Oct 2023 16:47:37 -0700
Subject: [PATCH] [libc++] Avoid discarding const-ness when copy-constructing
 vectors

Copy-constructing a std::vector<T> currently discards the const-ness of
the elements of the source vector. So for example, if the elements in
the vector have both `T(T&)` and `T(T const&)` constructors, the non-const
constructor is going to be preferred. This patch fixes that and makes
sure that constness is respected by the copy constructor and copy assignment
operators.

rdar://107652763
---
 libcxx/include/vector                         |  6 +++---
 .../vector/vector.cons/assign_copy.pass.cpp   | 20 +++++++++++++++++++
 .../vector/vector.cons/copy.pass.cpp          | 12 +++++++++++
 .../vector/vector.cons/copy_alloc.pass.cpp    | 13 ++++++++++++
 4 files changed, 48 insertions(+), 3 deletions(-)

diff --git a/libcxx/include/vector b/libcxx/include/vector
index 4ec6b602371eaee..d515dbbcf450888 100644
--- a/libcxx/include/vector
+++ b/libcxx/include/vector
@@ -1276,7 +1276,7 @@ _LIBCPP_CONSTEXPR_SINCE_CXX20
 vector<_Tp, _Allocator>::vector(const vector& __x)
     : __end_cap_(nullptr, __alloc_traits::select_on_container_copy_construction(__x.__alloc()))
 {
-  __init_with_size(__x.__begin_, __x.__end_, __x.size());
+  __init_with_size(__x.begin(), __x.end(), __x.size());
 }
 
 template <class _Tp, class _Allocator>
@@ -1284,7 +1284,7 @@ _LIBCPP_CONSTEXPR_SINCE_CXX20
 vector<_Tp, _Allocator>::vector(const vector& __x, const __type_identity_t<allocator_type>& __a)
     : __end_cap_(nullptr, __a)
 {
-  __init_with_size(__x.__begin_, __x.__end_, __x.size());
+  __init_with_size(__x.begin(), __x.end(), __x.size());
 }
 
 template <class _Tp, class _Allocator>
@@ -1409,7 +1409,7 @@ vector<_Tp, _Allocator>::operator=(const vector& __x)
     if (this != std::addressof(__x))
     {
         __copy_assign_alloc(__x);
-        assign(__x.__begin_, __x.__end_);
+        assign(__x.begin(), __x.end());
     }
     return *this;
 }
diff --git a/libcxx/test/std/containers/sequences/vector/vector.cons/assign_copy.pass.cpp b/libcxx/test/std/containers/sequences/vector/vector.cons/assign_copy.pass.cpp
index e0e227e8dc0c64f..98b166d51ab06f0 100644
--- a/libcxx/test/std/containers/sequences/vector/vector.cons/assign_copy.pass.cpp
+++ b/libcxx/test/std/containers/sequences/vector/vector.cons/assign_copy.pass.cpp
@@ -17,6 +17,19 @@
 #include "min_allocator.h"
 #include "allocators.h"
 
+struct HasNonConstCopyAssignment {
+    HasNonConstCopyAssignment() = default;
+    HasNonConstCopyAssignment(HasNonConstCopyAssignment const&) = default;
+    HasNonConstCopyAssignment(HasNonConstCopyAssignment&) { assert(false); }
+    TEST_CONSTEXPR HasNonConstCopyAssignment& operator=(HasNonConstCopyAssignment const&) {
+        return *this;
+    }
+    HasNonConstCopyAssignment& operator=(HasNonConstCopyAssignment&) {
+        assert(false);
+        return *this;
+    }
+};
+
 TEST_CONSTEXPR_CXX20 bool tests() {
     {
         std::vector<int, test_allocator<int> > l(3, 2, test_allocator<int>(5));
@@ -32,6 +45,13 @@ TEST_CONSTEXPR_CXX20 bool tests() {
         assert(l2 == l);
         assert(l2.get_allocator() == other_allocator<int>(5));
     }
+    {
+        // Make sure we copy elements of the vector using the right copy assignment operator.
+        std::vector<HasNonConstCopyAssignment> v(5);
+        std::vector<HasNonConstCopyAssignment> v2;
+        v2 = v;
+        assert(v2.size() == 5);
+    }
 #if TEST_STD_VER >= 11
     {
         // Test with Allocator::propagate_on_container_copy_assignment == false_type
diff --git a/libcxx/test/std/containers/sequences/vector/vector.cons/copy.pass.cpp b/libcxx/test/std/containers/sequences/vector/vector.cons/copy.pass.cpp
index b0e626381454e02..370e7abc6746aa3 100644
--- a/libcxx/test/std/containers/sequences/vector/vector.cons/copy.pass.cpp
+++ b/libcxx/test/std/containers/sequences/vector/vector.cons/copy.pass.cpp
@@ -18,6 +18,12 @@
 #include "min_allocator.h"
 #include "asan_testing.h"
 
+struct HasNonConstCopyConstructor {
+    HasNonConstCopyConstructor() = default;
+    TEST_CONSTEXPR HasNonConstCopyConstructor(HasNonConstCopyConstructor const&) { }
+    HasNonConstCopyConstructor(HasNonConstCopyConstructor&) { assert(false); }
+};
+
 template <class C>
 TEST_CONSTEXPR_CXX20 void
 test(const C& x)
@@ -58,6 +64,12 @@ TEST_CONSTEXPR_CXX20 bool tests() {
         assert(is_contiguous_container_asan_correct(v2));
         assert(v2.empty());
     }
+    {
+        // Make sure we copy elements of the vector using the right copy constructor.
+        std::vector<HasNonConstCopyConstructor> v(5);
+        std::vector<HasNonConstCopyConstructor> v2 = v;
+        assert(v2.size() == 5);
+    }
 #if TEST_STD_VER >= 11
     {
         std::vector<int, other_allocator<int> > v(3, 2, other_allocator<int>(5));
diff --git a/libcxx/test/std/containers/sequences/vector/vector.cons/copy_alloc.pass.cpp b/libcxx/test/std/containers/sequences/vector/vector.cons/copy_alloc.pass.cpp
index 0c03c50aed2e8b3..8a1b599a1a656d5 100644
--- a/libcxx/test/std/containers/sequences/vector/vector.cons/copy_alloc.pass.cpp
+++ b/libcxx/test/std/containers/sequences/vector/vector.cons/copy_alloc.pass.cpp
@@ -18,6 +18,12 @@
 #include "min_allocator.h"
 #include "asan_testing.h"
 
+struct HasNonConstCopyConstructor {
+    HasNonConstCopyConstructor() = default;
+    TEST_CONSTEXPR HasNonConstCopyConstructor(HasNonConstCopyConstructor const&) { }
+    HasNonConstCopyConstructor(HasNonConstCopyConstructor&) { assert(false); }
+};
+
 template <class C>
 TEST_CONSTEXPR_CXX20 void
 test(const C& x, const typename C::allocator_type& a)
@@ -56,6 +62,13 @@ TEST_CONSTEXPR_CXX20 bool tests() {
         assert(l2.get_allocator() == other_allocator<int>(3));
         assert(l2.empty());
     }
+    {
+        // Make sure we copy elements of the vector using the right copy constructor.
+        using Alloc = other_allocator<HasNonConstCopyConstructor>;
+        std::vector<HasNonConstCopyConstructor, Alloc> v(5);
+        std::vector<HasNonConstCopyConstructor, Alloc> v2(v, Alloc(42));
+        assert(v2.size() == 5);
+    }
 #if TEST_STD_VER >= 11
     {
         int a[] = {0, 1, 2, 3, 4, 5, 6, 7, 8, 9, 8, 7, 6, 5, 4, 3, 1, 0};



More information about the libcxx-commits mailing list