[PATCH] libstdc++/89130 and libstdc++/89090 fixes for vector relocation

[Jonathan Wakely <jwakely at redhat dot com>]

This fixes two PRs, one trivial (don't use C++17 features in C++11
mode) and one more serious (don't require MoveInsertable when we
should only need CopyInsertable).

It would be nice to rely on if-constexpr in C++11 mode, but it causes
clang warnings, complicates testcase bisection/reduction, and causes
users to file bogus bug reports. So let's just avoid it.

Tested powerpc64le-linux, committed to trunk.


commit 51908e56bd32b5f89bc909193c3da957de01c3e0
Author: Jonathan Wakely <jwakely@redhat.com>
Date:   Tue Feb 5 11:50:18 2019 +0000

    PR libstdc++/89130 restore support for non-MoveConstructible types
    
    The changes to "relocate" std::vector elements can lead to new errors
    outside the immediate context, because moving the elements to new
    storage no longer makes use of the move-if-noexcept utilities. This
    means that types with deleted moves no longer degenerate to copies, but
    are just ill-formed. The errors happen while instantiating the
    noexcept-specifier for __relocate_object_a, when deciding whether to try
    to relocate.
    
    This patch introduces indirections to avoid the ill-formed
    instantiations of std::__relocate_object_a. In order to avoid using
    if-constexpr prior to C++17 this is done by tag dispatching. After this
    patch all uses of std::__relocate_a are guarded by checks that will
    support sensible code (i.e. code not using custom allocators that fool
    the new checks).
    
            PR libstdc++/89130
            * include/bits/alloc_traits.h (__is_copy_insertable_impl): Rename to
            __is_alloc_insertable_impl. Replace single type member with two
            members, one for each of copy and move insertable.
            (__is_move_insertable): New trait for internal use.
            * include/bits/stl_vector.h (vector::_S_nothrow_relocate(true_type))
            (vector::_S_nothrow_relocate(true_type)): New functions to
            conditionally check if __relocate_a can throw.
            (vector::_S_use_relocate()): Dispatch to _S_nothrow_relocate based
            on __is_move_insertable.
            (vector::_S_do_relocate): New overloaded functions to conditionally
            call __relocate_a.
            (vector::_S_relocate): New function that dispatches to _S_do_relocate
            based on _S_use_relocate.
            * include/bits/vector.tcc (vector::reserve, vector::_M_realloc_insert)
            (vector::_M_default_append): Call _S_relocate instead of __relocate_a.
            * testsuite/23_containers/vector/modifiers/push_back/89130.cc: New.

diff --git a/libstdc++-v3/include/bits/alloc_traits.h b/libstdc++-v3/include/bits/alloc_traits.h
index ed61ce845f8..3b0c16fbf64 100644
--- a/libstdc++-v3/include/bits/alloc_traits.h
+++ b/libstdc++-v3/include/bits/alloc_traits.h
@@ -577,14 +577,16 @@ _GLIBCXX_BEGIN_NAMESPACE_VERSION
     }
 
   template<typename _Alloc>
-    class __is_copy_insertable_impl
+    class __is_alloc_insertable_impl
     {
-      typedef allocator_traits<_Alloc> _Traits;
+      using _Traits = allocator_traits<_Alloc>;
+      using value_type = typename _Traits::value_type;
 
-      template<typename _Up, typename
+      template<typename _Up, typename _Tp = __remove_cvref_t<_Up>,
+	       typename
 	       = decltype(_Traits::construct(std::declval<_Alloc&>(),
-					     std::declval<_Up*>(),
-					     std::declval<const _Up&>()))>
+					     std::declval<_Tp*>(),
+					     std::declval<_Up>()))>
 	static true_type
 	_M_select(int);
 
@@ -593,13 +595,14 @@ _GLIBCXX_BEGIN_NAMESPACE_VERSION
 	_M_select(...);
 
     public:
-      typedef decltype(_M_select<typename _Alloc::value_type>(0)) type;
+      using copy = decltype(_M_select<const value_type&>(0));
+      using move = decltype(_M_select<value_type>(0));
     };
 
   // true if _Alloc::value_type is CopyInsertable into containers using _Alloc
   template<typename _Alloc>
     struct __is_copy_insertable
-    : __is_copy_insertable_impl<_Alloc>::type
+    : __is_alloc_insertable_impl<_Alloc>::copy
     { };
 
   // std::allocator<_Tp> just requires CopyConstructible
@@ -608,6 +611,18 @@ _GLIBCXX_BEGIN_NAMESPACE_VERSION
     : is_copy_constructible<_Tp>
     { };
 
+  // true if _Alloc::value_type is MoveInsertable into containers using _Alloc
+  template<typename _Alloc>
+    struct __is_move_insertable
+    : __is_alloc_insertable_impl<_Alloc>::move
+    { };
+
+  // std::allocator<_Tp> just requires MoveConstructible
+  template<typename _Tp>
+    struct __is_move_insertable<allocator<_Tp>>
+    : is_move_constructible<_Tp>
+    { };
+
   // Trait to detect Allocator-like types.
   template<typename _Alloc, typename = void>
     struct __is_allocator : false_type { };
diff --git a/libstdc++-v3/include/bits/stl_vector.h b/libstdc++-v3/include/bits/stl_vector.h
index 43debda54f1..10bf4fac62e 100644
--- a/libstdc++-v3/include/bits/stl_vector.h
+++ b/libstdc++-v3/include/bits/stl_vector.h
@@ -425,14 +425,47 @@ _GLIBCXX_BEGIN_NAMESPACE_CONTAINER
     private:
 #if __cplusplus >= 201103L
       static constexpr bool
-      _S_use_relocate()
+      _S_nothrow_relocate(true_type)
       {
 	return noexcept(std::__relocate_a(std::declval<pointer>(),
 					  std::declval<pointer>(),
 					  std::declval<pointer>(),
 					  std::declval<_Tp_alloc_type&>()));
       }
-#endif
+
+      static constexpr bool
+      _S_nothrow_relocate(false_type)
+      { return false; }
+
+      static constexpr bool
+      _S_use_relocate()
+      {
+	// Instantiating std::__relocate_a might cause an error outside the
+	// immediate context (in __relocate_object_a's noexcept-specifier),
+	// so only do it if we know the type can be move-inserted into *this.
+	return _S_nothrow_relocate(__is_move_insertable<_Tp_alloc_type>{});
+      }
+
+      static pointer
+      _S_do_relocate(pointer __first, pointer __last, pointer __result,
+		     _Tp_alloc_type& __alloc, true_type) noexcept
+      {
+	return std::__relocate_a(__first, __last, __result, __alloc);
+      }
+
+      static pointer
+      _S_do_relocate(pointer, pointer, pointer __result,
+		     _Tp_alloc_type&, false_type) noexcept
+      { return __result; }
+
+      static pointer
+      _S_relocate(pointer __first, pointer __last, pointer __result,
+		  _Tp_alloc_type& __alloc) noexcept
+      {
+	using __do_it = __bool_constant<_S_use_relocate()>;
+	return _S_do_relocate(__first, __last, __result, __alloc, __do_it{});
+      }
+#endif // C++11
 
     protected:
       using _Base::_M_allocate;
diff --git a/libstdc++-v3/include/bits/vector.tcc b/libstdc++-v3/include/bits/vector.tcc
index 54c09774b15..497d9f72247 100644
--- a/libstdc++-v3/include/bits/vector.tcc
+++ b/libstdc++-v3/include/bits/vector.tcc
@@ -76,9 +76,8 @@ _GLIBCXX_BEGIN_NAMESPACE_CONTAINER
 	  if _GLIBCXX17_CONSTEXPR (_S_use_relocate())
 	    {
 	      __tmp = this->_M_allocate(__n);
-	      std::__relocate_a(this->_M_impl._M_start,
-				this->_M_impl._M_finish,
-				__tmp, _M_get_Tp_allocator());
+	      _S_relocate(this->_M_impl._M_start, this->_M_impl._M_finish,
+			  __tmp, _M_get_Tp_allocator());
 	    }
 	  else
 #endif
@@ -459,17 +458,13 @@ _GLIBCXX_BEGIN_NAMESPACE_CONTAINER
 #if __cplusplus >= 201103L
 	  if _GLIBCXX17_CONSTEXPR (_S_use_relocate())
 	    {
-	      __new_finish
-		= std::__relocate_a
-		(__old_start, __position.base(),
-		 __new_start, _M_get_Tp_allocator());
+	      __new_finish = _S_relocate(__old_start, __position.base(),
+					 __new_start, _M_get_Tp_allocator());
 
 	      ++__new_finish;
 
-	      __new_finish
-		= std::__relocate_a
-		(__position.base(), __old_finish,
-		 __new_finish, _M_get_Tp_allocator());
+	      __new_finish = _S_relocate(__position.base(), __old_finish,
+					 __new_finish, _M_get_Tp_allocator());
 	    }
 	  else
 #endif
@@ -650,9 +645,8 @@ _GLIBCXX_BEGIN_NAMESPACE_CONTAINER
 		      _M_deallocate(__new_start, __len);
 		      __throw_exception_again;
 		    }
-		  std::__relocate_a(this->_M_impl._M_start,
-				    this->_M_impl._M_finish,
-				    __new_start, _M_get_Tp_allocator());
+		  _S_relocate(this->_M_impl._M_start, this->_M_impl._M_finish,
+			      __new_start, _M_get_Tp_allocator());
 		}
 	      else
 		{
diff --git a/libstdc++-v3/testsuite/23_containers/vector/modifiers/push_back/89130.cc b/libstdc++-v3/testsuite/23_containers/vector/modifiers/push_back/89130.cc
new file mode 100644
index 00000000000..54b3f53069b
--- /dev/null
+++ b/libstdc++-v3/testsuite/23_containers/vector/modifiers/push_back/89130.cc
@@ -0,0 +1,63 @@
+// Copyright (C) 2019 Free Software Foundation, Inc.
+//
+// This file is part of the GNU ISO C++ Library.  This library is free
+// software; you can redistribute it and/or modify it under the
+// terms of the GNU General Public License as published by the
+// Free Software Foundation; either version 3, or (at your option)
+// any later version.
+
+// This library is distributed in the hope that it will be useful,
+// but WITHOUT ANY WARRANTY; without even the implied warranty of
+// MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See the
+// GNU General Public License for more details.
+
+// You should have received a copy of the GNU General Public License along
+// with this library; see the file COPYING3.  If not see
+// <http://www.gnu.org/licenses/>.
+
+// { dg-options "-std=gnu++2a" }
+// { dg-do compile { target c++2a } }
+
+#include <vector>
+
+struct T
+{
+  T() { }
+  T(const T&) { }
+  T(T&&) = delete;  // this means T is not MoveInsertable into std::vector<T>
+};
+
+void f()
+{
+  const T val;
+  std::vector<T> x;
+  // push_back(const T&) only requires T is CopyInsertable into std::vector<T>:
+  x.push_back(val);
+}
+
+template<typename U>
+struct Alloc
+{
+  using value_type = U;
+  Alloc() = default;
+  Alloc(const Alloc&) = default;
+  template<typename U2>
+    Alloc(const Alloc<U2>&) { }
+
+  U* allocate(unsigned n) { return std::allocator<U>().allocate(n); }
+  void deallocate(U* p, unsigned n) { std::allocator<U>().deallocate(p, n); }
+
+  void construct(Alloc*, U* p, U&& u)
+  {
+    // construct from const lvalue instead of rvalue:
+    ::new(p) U(const_cast<const U&>(u));
+  }
+};
+
+void g()
+{
+  const T val;
+  std::vector<T, Alloc<T>> x;
+  // push_back(const T&) only requires T is CopyInsertable into std::vector<T>:
+  x.push_back(val);
+}

commit 45094752353b4c49fb1bc8dc2f2b5254235a7948
Author: Jonathan Wakely <jwakely@redhat.com>
Date:   Mon Jan 28 21:35:33 2019 +0000

    PR libstdc++/89090 avoid C++17 features in C++11/C++14 code
    
    Although GCC and Clang both allow these features pre-C++17 in system
    headers, Clang does issue warnings with -Wsystem-headers. It can also
    complicate bisection and/or testcase reduction if # line markers are
    stripped, because the code won't be known to come from system headers.
    
            PR libstdc++/89090
            * include/bits/stl_uninitialized.h (__relocate_a_1): Make unused
            parameter unnamed. Add message to static assertion.
            * include/bits/vector.tcc (vector::reserve, vector::_M_realloc_insert)
            (vector::_M_default_append): Use _GLIBCXX17_CONSTEXPR for if constexpr
            in C++11 code.

diff --git a/libstdc++-v3/include/bits/stl_uninitialized.h b/libstdc++-v3/include/bits/stl_uninitialized.h
index 03ed16b8c1a..0d42b253df1 100644
--- a/libstdc++-v3/include/bits/stl_uninitialized.h
+++ b/libstdc++-v3/include/bits/stl_uninitialized.h
@@ -904,7 +904,7 @@ _GLIBCXX_BEGIN_NAMESPACE_VERSION
   template <typename _Tp, typename _Up>
     inline __enable_if_t<std::__is_bitwise_relocatable<_Tp>::value, _Tp*>
     __relocate_a_1(_Tp* __first, _Tp* __last,
-		   _Tp* __result, allocator<_Up>& __alloc) noexcept
+		   _Tp* __result, allocator<_Up>&) noexcept
     {
       ptrdiff_t __count = __last - __first;
       if (__count > 0)
@@ -925,7 +925,8 @@ _GLIBCXX_BEGIN_NAMESPACE_VERSION
 	_ValueType;
       typedef typename iterator_traits<_ForwardIterator>::value_type
 	_ValueType2;
-      static_assert(std::is_same<_ValueType, _ValueType2>::value);
+      static_assert(std::is_same<_ValueType, _ValueType2>::value,
+	  "relocation is only possible for values of the same type");
       _ForwardIterator __cur = __result;
       for (; __first != __last; ++__first, (void)++__cur)
 	std::__relocate_object_a(std::__addressof(*__cur),
diff --git a/libstdc++-v3/include/bits/vector.tcc b/libstdc++-v3/include/bits/vector.tcc
index 4cf0e809fe9..54c09774b15 100644
--- a/libstdc++-v3/include/bits/vector.tcc
+++ b/libstdc++-v3/include/bits/vector.tcc
@@ -73,7 +73,7 @@ _GLIBCXX_BEGIN_NAMESPACE_CONTAINER
 	  const size_type __old_size = size();
 	  pointer __tmp;
 #if __cplusplus >= 201103L
-	  if constexpr (_S_use_relocate())
+	  if _GLIBCXX17_CONSTEXPR (_S_use_relocate())
 	    {
 	      __tmp = this->_M_allocate(__n);
 	      std::__relocate_a(this->_M_impl._M_start,
@@ -457,7 +457,7 @@ _GLIBCXX_BEGIN_NAMESPACE_CONTAINER
 	  __new_finish = pointer();
 
 #if __cplusplus >= 201103L
-	  if constexpr (_S_use_relocate())
+	  if _GLIBCXX17_CONSTEXPR (_S_use_relocate())
 	    {
 	      __new_finish
 		= std::__relocate_a
@@ -498,7 +498,7 @@ _GLIBCXX_BEGIN_NAMESPACE_CONTAINER
 	  __throw_exception_again;
 	}
 #if __cplusplus >= 201103L
-      if constexpr (!_S_use_relocate())
+      if _GLIBCXX17_CONSTEXPR (!_S_use_relocate())
 #endif
 	std::_Destroy(__old_start, __old_finish, _M_get_Tp_allocator());
       _GLIBCXX_ASAN_ANNOTATE_REINIT;
@@ -638,8 +638,7 @@ _GLIBCXX_BEGIN_NAMESPACE_CONTAINER
 	      const size_type __len =
 		_M_check_len(__n, "vector::_M_default_append");
 	      pointer __new_start(this->_M_allocate(__len));
-#if __cplusplus >= 201103L
-	      if constexpr (_S_use_relocate())
+	      if _GLIBCXX17_CONSTEXPR (_S_use_relocate())
 		{
 		  __try
 		    {
@@ -656,7 +655,6 @@ _GLIBCXX_BEGIN_NAMESPACE_CONTAINER
 				    __new_start, _M_get_Tp_allocator());
 		}
 	      else
-#endif
 		{
 		  pointer __destroy_from = pointer();
 		  __try