This is the mail archive of the libstdc++@gcc.gnu.org mailing list for the libstdc++ project.


Index Nav: [Date Index] [Subject Index] [Author Index] [Thread Index]
Message Nav: [Date Prev] [Date Next] [Thread Prev] [Thread Next]
Other format: [Raw text]

Re: Relocation (= move+destroy)


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


Index Nav: [Date Index] [Subject Index] [Author Index] [Thread Index]
Message Nav: [Date Prev] [Date Next] [Thread Prev] [Thread Next]