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