Quuxplusone created this revision.
Quuxplusone added reviewers: EricWF, mclow.lists, erik.pilkington, vsapsai.
Herald added subscribers: cfe-commits, ldionne, christof.
Inspired by Volodymyr's work on https://reviews.llvm.org/D48753, I've taken it
upon myself to refactor the static member functions
`std::allocator_traits<A>::__construct_forward`, `__construct_backward`, and
`__construct_range_forward` into non-member functions. I think this is
reasonable just in terms of code-cleanliness — they don't *have* to be member
functions for any reason I can think of — and then it also permits a suitably
sadomasochistic programmer to define his own specialization of
`std::allocator_traits` without causing compiler errors in `<vector>`.
I have added a test case in `test/libcxx/` for the sadomasochistic case, which
I describe as "arguably ill-formed." I would be very very happy to see WG21
agree that specializing traits classes (pointer_traits, allocator_traits,
iterator_traits) *is* ill-formed; I believe there's some disagreement on the
subject at the moment. In the meantime, I think this would be a nice patch
just on code-cleanliness grounds.
This patch is also groundwork for the "trivially relocatable" fork that I'm
building on my GitHub
<https://github.com/Quuxplusone/clang/tree/trivially-relocatable>; we'd need an
architecture something like this in order to easily drop in support for trivial
relocatability.
Repository:
rCXX libc++
https://reviews.llvm.org/D49317
Files:
include/memory
include/vector
test/libcxx/containers/sequences/vector/specialized_allocator_traits.pass.cpp
Index: test/libcxx/containers/sequences/vector/specialized_allocator_traits.pass.cpp
===================================================================
--- /dev/null
+++ test/libcxx/containers/sequences/vector/specialized_allocator_traits.pass.cpp
@@ -0,0 +1,100 @@
+//===----------------------------------------------------------------------===//
+//
+// The LLVM Compiler Infrastructure
+//
+// This file is dual licensed under the MIT and the University of Illinois Open
+// Source Licenses. See LICENSE.TXT for details.
+//
+//===----------------------------------------------------------------------===//
+
+// UNSUPPORTED: c++98, c++03
+
+// <vector>
+
+// Test that vector does not use non-standard members of std::allocator_traits.
+// Specializing std::allocator_traits is arguably non-conforming, but libc++'s
+// support for specialized std::allocator_traits is a feature, not a bug.
+// Breaking (and subsequently deleting) this unit test should be done as a
+// conscious decision.
+
+#include <vector>
+
+template <class T>
+class A1
+{
+public:
+ using value_type = T;
+
+ A1() = default;
+
+ template <class U>
+ A1(const A1<U>&) {}
+
+ T *allocate(std::size_t n)
+ {
+ return (T *)std::malloc(n * sizeof (T));
+ }
+
+ void deallocate(T* p, std::size_t)
+ {
+ std::free(p);
+ }
+};
+
+template<class T>
+struct std::allocator_traits<A1<T>> {
+ using allocator_type = A1<T>;
+ using value_type = T;
+ using pointer = T*;
+ using const_pointer = const T*;
+ using void_pointer = void*;
+ using const_void_pointer = const void*;
+ using difference_type = std::ptrdiff_t;
+ using size_type = std::size_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;
+ using is_always_equal = std::true_type;
+
+ template<class U> using rebind_alloc = A1<U>;
+ template<class U> using rebind_traits = std::allocator_traits<A1<U>>;
+
+ static T *allocate(A1<T>& a, size_t n) {
+ return a.allocate(n);
+ }
+
+ static void deallocate(A1<T>& a, T *p, size_t n) {
+ return a.deallocate(p, n);
+ }
+
+ template<class U, class... Args>
+ static void construct(A1<T>&, U *p, Args&&... args) {
+ ::new ((void*)p) U(std::forward<Args>(args)...);
+ }
+
+ template<class U>
+ static void destroy(A1<T>&, U *p) {
+ p->~U();
+ }
+
+ static A1<T> select_on_container_copy_construction(const A1<T>& a) {
+ return a.select_on_container_copy_construction();
+ }
+
+ static size_type max_size(const A1<T>&) {
+ return size_t(-1);
+ }
+};
+
+int main()
+{
+ std::vector<int, A1<int>> v = {1, 2, 3};
+ v.resize(10);
+ v.insert(v.begin() + 4, 4);
+ assert(v[0] == 1);
+ assert(v[1] == 2);
+ assert(v[2] == 3);
+ assert(v[3] == 0);
+ assert(v[4] == 4);
+ assert(v[5] == 0);
+}
Index: include/vector
===================================================================
--- include/vector
+++ include/vector
@@ -460,13 +460,20 @@
}
}
+template<class _Tp, class _Allocator>
+struct __vector_copy_via_memcpy : integral_constant<bool,
+ (is_same<_Allocator, allocator<_Tp> >::value || !__has_construct<_Allocator, _Tp*, _Tp>::value) &&
+ is_trivially_move_constructible<_Tp>::value
+> {};
+
template <class _Tp, class _Allocator /* = allocator<_Tp> */>
class _LIBCPP_TEMPLATE_VIS vector
: private __vector_base<_Tp, _Allocator>
{
private:
typedef __vector_base<_Tp, _Allocator> __base;
typedef allocator<_Tp> __default_allocator_type;
+
public:
typedef vector __self;
typedef _Tp value_type;
@@ -927,8 +934,9 @@
void
vector<_Tp, _Allocator>::__swap_out_circular_buffer(__split_buffer<value_type, allocator_type&>& __v)
{
+ typedef typename __vector_copy_via_memcpy<_Tp, _Allocator>::type __copy_via_memcpy;
__annotate_delete();
- __alloc_traits::__construct_backward(this->__alloc(), this->__begin_, this->__end_, __v.__begin_);
+ _VSTD::__construct_backward(this->__alloc(), this->__begin_, this->__end_, __v.__begin_, __copy_via_memcpy());
_VSTD::swap(this->__begin_, __v.__begin_);
_VSTD::swap(this->__end_, __v.__end_);
_VSTD::swap(this->__end_cap(), __v.__end_cap());
@@ -941,10 +949,11 @@
typename vector<_Tp, _Allocator>::pointer
vector<_Tp, _Allocator>::__swap_out_circular_buffer(__split_buffer<value_type, allocator_type&>& __v, pointer __p)
{
+ typedef typename __vector_copy_via_memcpy<_Tp, _Allocator>::type __copy_via_memcpy;
__annotate_delete();
pointer __r = __v.__begin_;
- __alloc_traits::__construct_backward(this->__alloc(), this->__begin_, __p, __v.__begin_);
- __alloc_traits::__construct_forward(this->__alloc(), __p, this->__end_, __v.__end_);
+ _VSTD::__construct_backward(this->__alloc(), this->__begin_, __p, __v.__begin_, __copy_via_memcpy());
+ _VSTD::__construct_forward(this->__alloc(), __p, this->__end_, __v.__end_, __copy_via_memcpy());
_VSTD::swap(this->__begin_, __v.__begin_);
_VSTD::swap(this->__end_, __v.__end_);
_VSTD::swap(this->__end_cap(), __v.__end_cap());
@@ -1058,9 +1067,10 @@
>::type
vector<_Tp, _Allocator>::__construct_at_end(_ForwardIterator __first, _ForwardIterator __last, size_type __n)
{
+ typedef typename __vector_copy_via_memcpy<_Tp, _Allocator>::type __copy_via_memcpy;
allocator_type& __a = this->__alloc();
__RAII_IncreaseAnnotator __annotator(*this, __n);
- __alloc_traits::__construct_range_forward(__a, __first, __last, this->__end_);
+ _VSTD::__construct_range_forward(__a, __first, __last, this->__end_, __copy_via_memcpy());
__annotator.__done();
}
Index: include/memory
===================================================================
--- include/memory
+++ include/memory
@@ -1606,98 +1606,6 @@
__has_select_on_container_copy_construction<const allocator_type>(),
__a);}
- template <class _Ptr>
- _LIBCPP_INLINE_VISIBILITY
- static
- void
- __construct_forward(allocator_type& __a, _Ptr __begin1, _Ptr __end1, _Ptr& __begin2)
- {
- for (; __begin1 != __end1; ++__begin1, ++__begin2)
- construct(__a, _VSTD::__to_raw_pointer(__begin2), _VSTD::move_if_noexcept(*__begin1));
- }
-
- template <class _Tp>
- _LIBCPP_INLINE_VISIBILITY
- static
- typename enable_if
- <
- (is_same<allocator_type, allocator<_Tp> >::value
- || !__has_construct<allocator_type, _Tp*, _Tp>::value) &&
- is_trivially_move_constructible<_Tp>::value,
- void
- >::type
- __construct_forward(allocator_type&, _Tp* __begin1, _Tp* __end1, _Tp*& __begin2)
- {
- ptrdiff_t _Np = __end1 - __begin1;
- if (_Np > 0)
- {
- _VSTD::memcpy(__begin2, __begin1, _Np * sizeof(_Tp));
- __begin2 += _Np;
- }
- }
-
- template <class _Iter, class _Ptr>
- _LIBCPP_INLINE_VISIBILITY
- static
- void
- __construct_range_forward(allocator_type& __a, _Iter __begin1, _Iter __end1, _Ptr& __begin2)
- {
- for (; __begin1 != __end1; ++__begin1, (void) ++__begin2)
- construct(__a, _VSTD::__to_raw_pointer(__begin2), *__begin1);
- }
-
- template <class _Tp>
- _LIBCPP_INLINE_VISIBILITY
- static
- typename enable_if
- <
- (is_same<allocator_type, allocator<_Tp> >::value
- || !__has_construct<allocator_type, _Tp*, _Tp>::value) &&
- is_trivially_move_constructible<_Tp>::value,
- void
- >::type
- __construct_range_forward(allocator_type&, _Tp* __begin1, _Tp* __end1, _Tp*& __begin2)
- {
- typedef typename remove_const<_Tp>::type _Vp;
- ptrdiff_t _Np = __end1 - __begin1;
- if (_Np > 0)
- {
- _VSTD::memcpy(const_cast<_Vp*>(__begin2), __begin1, _Np * sizeof(_Tp));
- __begin2 += _Np;
- }
- }
-
- template <class _Ptr>
- _LIBCPP_INLINE_VISIBILITY
- static
- void
- __construct_backward(allocator_type& __a, _Ptr __begin1, _Ptr __end1, _Ptr& __end2)
- {
- while (__end1 != __begin1)
- {
- construct(__a, _VSTD::__to_raw_pointer(__end2-1), _VSTD::move_if_noexcept(*--__end1));
- --__end2;
- }
- }
-
- template <class _Tp>
- _LIBCPP_INLINE_VISIBILITY
- static
- typename enable_if
- <
- (is_same<allocator_type, allocator<_Tp> >::value
- || !__has_construct<allocator_type, _Tp*, _Tp>::value) &&
- is_trivially_move_constructible<_Tp>::value,
- void
- >::type
- __construct_backward(allocator_type&, _Tp* __begin1, _Tp* __end1, _Tp*& __end2)
- {
- ptrdiff_t _Np = __end1 - __begin1;
- __end2 -= _Np;
- if (_Np > 0)
- _VSTD::memcpy(__end2, __begin1, _Np * sizeof(_Tp));
- }
-
private:
_LIBCPP_INLINE_VISIBILITY
@@ -1760,6 +1668,81 @@
#endif
};
+template <class _Alloc, class _Ptr>
+_LIBCPP_INLINE_VISIBILITY
+inline void
+__construct_forward(_Alloc& __a, _Ptr __begin1, _Ptr __end1, _Ptr& __begin2, false_type)
+{
+ using _Alloc_traits = allocator_traits<_Alloc>;
+ for (; __begin1 != __end1; ++__begin1, ++__begin2)
+ _Alloc_traits::construct(__a, _VSTD::__to_raw_pointer(__begin2), _VSTD::move_if_noexcept(*__begin1));
+}
+
+template <class _Alloc, class _Ptr>
+_LIBCPP_INLINE_VISIBILITY
+inline void
+__construct_forward(_Alloc&, _Ptr __begin1, _Ptr __end1, _Ptr& __begin2, true_type)
+{
+ typedef typename iterator_traits<_Ptr>::value_type _Tp;
+ ptrdiff_t _Np = __end1 - __begin1;
+ if (_Np > 0)
+ {
+ _VSTD::memcpy(_VSTD::__to_raw_pointer(__begin2), _VSTD::__to_raw_pointer(__begin1), _Np * sizeof(_Tp));
+ __begin2 += _Np;
+ }
+}
+
+template <class _Alloc, class _Iter, class _Ptr, class _CopyViaMemcpy>
+_LIBCPP_INLINE_VISIBILITY
+inline void
+__construct_range_forward(_Alloc& __a, _Iter __begin1, _Iter __end1, _Ptr& __begin2, _CopyViaMemcpy)
+{
+ using _Alloc_traits = allocator_traits<_Alloc>;
+ for (; __begin1 != __end1; ++__begin1, (void)++__begin2)
+ _Alloc_traits::construct(__a, _VSTD::__to_raw_pointer(__begin2), *__begin1);
+}
+
+template <class _Alloc, class _Tp>
+_LIBCPP_INLINE_VISIBILITY
+inline void
+__construct_range_forward(_Alloc&, _Tp* __begin1, _Tp* __end1, _Tp*& __begin2, true_type)
+{
+ typedef typename remove_const<_Tp>::type _Vp;
+ ptrdiff_t _Np = __end1 - __begin1;
+ if (_Np > 0)
+ {
+ _VSTD::memcpy(const_cast<_Vp*>(__begin2), __begin1, _Np * sizeof(_Tp));
+ __begin2 += _Np;
+ }
+}
+
+template <class _Alloc, class _Ptr>
+_LIBCPP_INLINE_VISIBILITY
+inline void
+__construct_backward(_Alloc& __a, _Ptr __begin1, _Ptr __end1, _Ptr& __end2, false_type)
+{
+ using _Alloc_traits = allocator_traits<_Alloc>;
+ while (__end1 != __begin1)
+ {
+ _Alloc_traits::construct(__a, _VSTD::__to_raw_pointer(__end2-1), _VSTD::move_if_noexcept(*--__end1));
+ --__end2;
+ }
+}
+
+template <class _Alloc, class _Ptr>
+_LIBCPP_INLINE_VISIBILITY
+inline void
+__construct_backward(_Alloc&, _Ptr __begin1, _Ptr __end1, _Ptr& __end2, true_type)
+{
+ typedef typename iterator_traits<_Ptr>::value_type _Tp;
+ ptrdiff_t _Np = __end1 - __begin1;
+ if (_Np > 0)
+ {
+ __end2 -= _Np;
+ _VSTD::memcpy(_VSTD::__to_raw_pointer(__end2), _VSTD::__to_raw_pointer(__begin1), _Np * sizeof(_Tp));
+ }
+}
+
// allocator
template <class _Tp>
_______________________________________________
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
