Relocation (= move+destroy)
Jonathan Wakely
jwakely@redhat.com
Tue Oct 23 10:33:00 GMT 2018
CCing gcc-patches
On 19/10/18 07:33 +0200, Marc Glisse wrote:
>On Thu, 18 Oct 2018, Marc Glisse wrote:
>
>>Uh, why didn't I notice that the function __relocate is unused? I
>>guess I'll resend the same patch without __relocate once retesting
>>has finished :-( Sorry for all the corrections, I guess I didn't
>>check my patch carefully enough before sending it the first time.
>
>2018-10-19 Marc Glisse <marc.glisse@inria.fr>
>
> PR libstdc++/87106
> * include/bits/alloc_traits.h (_S_construct, _S_destroy, construct,
> destroy): Add noexcept specification.
> * include/bits/allocator.h (construct, destroy): Likewise.
> * include/ext/alloc_traits.h (construct, destroy): Likewise.
> * include/ext/malloc_allocator.h (construct, destroy): Likewise.
> * include/ext/new_allocator.h (construct, destroy): Likewise.
> * include/bits/stl_uninitialized.h (__relocate_a, __relocate_a_1):
> New functions.
> (__is_trivially_relocatable): New class.
> * include/bits/stl_vector.h (__use_relocate): New static member.
> * include/bits/vector.tcc (reserve, _M_realloc_insert,
> _M_default_append): Use __relocate_a.
> (reserve, _M_assign_aux, _M_realloc_insert, _M_fill_insert,
> _M_default_append, _M_range_insert): Move _GLIBCXX_ASAN_ANNOTATE_REINIT
> after _Destroy.
> * testsuite/23_containers/vector/modifiers/push_back/49836.cc:
> Replace CopyConsOnlyType with DelAnyAssign.
>
>--
>Marc Glisse
The tricky stuff in <bits/stl_vector.h> all looks right, I only have
some comments on the __relocate_a functions ...
>Index: libstdc++-v3/include/bits/stl_uninitialized.h
>===================================================================
>--- libstdc++-v3/include/bits/stl_uninitialized.h (revision 265289)
>+++ libstdc++-v3/include/bits/stl_uninitialized.h (working copy)
>@@ -872,14 +872,75 @@ _GLIBCXX_BEGIN_NAMESPACE_VERSION
> uninitialized_move_n(_InputIterator __first, _Size __count,
> _ForwardIterator __result)
> {
> auto __res = std::__uninitialized_copy_n_pair
> (_GLIBCXX_MAKE_MOVE_ITERATOR(__first),
> __count, __result);
> return {__res.first.base(), __res.second};
> }
> #endif
>
>+#if __cplusplus >= 201402L
What depends on C++14 here? Just enable_if_t? Because we have
__enable_if_t for use in C++11.
Both GCC and Clang will allow constexpr-if and static_assert with no
message in C++11.
>+ template<typename _Tp, typename _Up, typename _Allocator>
>+ inline void
>+ __relocate_a(_Tp* __dest, _Up* __orig, _Allocator& __alloc)
I find it a little surprising that this overload for single objects
using the memmove argument ordering (dest, source) but the range overload
below uses the STL ordering (source_begin, source_end, dest).
But I wouldn't be surprised if we're already doing that somewhere that
I've forgotten about.
WOuld it make sense to either rename this overload, or to use
consistent argument ordering for the two __relocate_a overloads?
>+ noexcept(noexcept(__gnu_cxx::__alloc_traits<_Allocator>::construct(__alloc,
Since this is C++14 (or maybe C++11) you could just use
std::allocator_traits directly. __gnu_cxx::__alloc_traits is to
provide equivalent functionality in C++98 code.
>+ __dest, std::move(*__orig)))
>+ && noexcept(__gnu_cxx::__alloc_traits<_Allocator>::destroy(
>+ __alloc, std::__addressof(*__orig))))
>+ {
>+ typedef __gnu_cxx::__alloc_traits<_Allocator> __traits;
>+ __traits::construct(__alloc, __dest, std::move(*__orig));
>+ __traits::destroy(__alloc, std::__addressof(*__orig));
>+ }
>+
>+ template<typename _Tp>
>+ struct __is_trivially_relocatable
>+ : is_trivial<_Tp> { };
It might be worth adding a comment that this type might be specialized
in future, so that I don't forget and simplify it to an alias template
later :-)
>+ template <typename _Tp, typename _Up>
>+ inline std::enable_if_t<std::__is_trivially_relocatable<_Tp>::value, _Tp*>
>+ __relocate_a_1(_Tp* __first, _Tp* __last,
>+ _Tp* __result, allocator<_Up>& __alloc)
>+ {
>+ ptrdiff_t __count = __last - __first;
>+ __builtin_memmove(__result, __first, __count * sizeof(_Tp));
>+ return __result + __count;
>+ }
>+
>+ template <typename _InputIterator, typename _ForwardIterator,
>+ typename _Allocator>
>+ inline _ForwardIterator
>+ __relocate_a_1(_InputIterator __first, _InputIterator __last,
>+ _ForwardIterator __result, _Allocator& __alloc)
>+ {
>+ typedef typename iterator_traits<_InputIterator>::value_type
>+ _ValueType;
>+ typedef typename iterator_traits<_ForwardIterator>::value_type
>+ _ValueType2;
>+ static_assert(std::is_same<_ValueType, _ValueType2>::value);
>+ static_assert(noexcept(std::__relocate_a(std::addressof(*__result),
>+ std::addressof(*__first),
>+ __alloc)));
>+ _ForwardIterator __cur = __result;
>+ for (; __first != __last; ++__first, (void)++__cur)
>+ std::__relocate_a(std::__addressof(*__cur),
>+ std::__addressof(*__first), __alloc);
>+ return __cur;
>+ }
>+
>+ template <typename _InputIterator, typename _ForwardIterator,
>+ typename _Allocator>
>+ inline _ForwardIterator
>+ __relocate_a(_InputIterator __first, _InputIterator __last,
>+ _ForwardIterator __result, _Allocator& __alloc)
>+ {
>+ return __relocate_a_1(std::__niter_base(__first),
>+ std::__niter_base(__last),
>+ std::__niter_base(__result), __alloc);
>+ }
>+#endif
>+
> _GLIBCXX_END_NAMESPACE_VERSION
> } // namespace
More information about the Libstdc++
mailing list