| Message ID | 20190205144506.GL4162@redhat.com |
|---|---|
| State | New |
| Headers | show |
| Series | libstdc++/89130 and libstdc++/89090 fixes for vector relocation | expand |
On 05/02/19 14:45 +0000, Jonathan Wakely wrote: >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. Oh, and the test change can be reverted now that types with deleted moves work again. Tested x86_64-linux, committed to trunk. commit 3cd6139661801ab6bd88558e336a8ba12d1ed640 Author: Jonathan Wakely <jwakely@redhat.com> Date: Tue Feb 5 14:50:58 2019 +0000 Restore previous behaviour of test Go back to using CopyConsOnlyType as before r265485, because it works again now. Add test using DelAnyAssign for completeness and additional coverage. * testsuite/23_containers/vector/modifiers/push_back/49836.cc: Restore use of CopyConsOnlyType, but also test DelAnyAssign for completeness. diff --git a/libstdc++-v3/testsuite/23_containers/vector/modifiers/push_back/49836.cc b/libstdc++-v3/testsuite/23_containers/vector/modifiers/push_back/49836.cc index 1a7d8729718..3e59781e7cc 100644 --- a/libstdc++-v3/testsuite/23_containers/vector/modifiers/push_back/49836.cc +++ b/libstdc++-v3/testsuite/23_containers/vector/modifiers/push_back/49836.cc @@ -24,11 +24,12 @@ // libstdc++/49836 void test01() { - using __gnu_test::assign::DelAnyAssign; + using __gnu_test::CopyConsOnlyType; using __gnu_test::MoveConsOnlyType; + using __gnu_test::assign::DelAnyAssign; - std::vector<DelAnyAssign> v1; - DelAnyAssign t1; + std::vector<CopyConsOnlyType> v1; + CopyConsOnlyType t1(1); v1.push_back(t1); v1.push_back(t1); v1.push_back(t1); @@ -40,6 +41,14 @@ void test01() v2.push_back(std::move(t2)); v2.push_back(std::move(t2)); VERIFY( v2.size() == 3 ); + + std::vector<DelAnyAssign> v3; + DelAnyAssign t3; + v3.push_back(t3); + v3.push_back(t3); + v3.push_back(t3); + VERIFY( v3.size() == 3 ); + } int main()
On 05/02/19 14:45 +0000, Jonathan Wakely wrote: >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)); This caused another regression, fixed by the attached patch. Tested powerpc64le-linux, committed to trunk. commit 802fc355d929152fb2c70e5fc1422d16143eeb10 Author: Jonathan Wakely <jwakely@redhat.com> Date: Thu Feb 21 02:28:16 2019 +0000 PR libstdc++/89416 fix __is_move_insertable trait The common base class for __is_move_insertable and __is_copy_insertable instantiates both the copy and move tests, when only one is needed. The unneeded one might cause errors outside the immediate context. The solution used in this patch is to replace them with alias templates, which will only be instantiated as needed. PR libstdc++/89416 * include/bits/alloc_traits.h (__is_alloc_insertable_impl): Replace class template with class. Replace move and copy member types with member alias templates, so they are only instantiated when needed. (__is_copy_insertable, __is_move_insertable): Adjust base class. * testsuite/23_containers/vector/modifiers/push_back/89130.cc: Enable test for C++11/14/17 as well. * testsuite/23_containers/vector/modifiers/push_back/89416.cc: New test. diff --git a/libstdc++-v3/include/bits/alloc_traits.h b/libstdc++-v3/include/bits/alloc_traits.h index 3b0c16fbf64..71892cbfaba 100644 --- a/libstdc++-v3/include/bits/alloc_traits.h +++ b/libstdc++-v3/include/bits/alloc_traits.h @@ -576,33 +576,32 @@ _GLIBCXX_BEGIN_NAMESPACE_VERSION __do_alloc_on_swap(__one, __two, __pocs()); } - template<typename _Alloc> - class __is_alloc_insertable_impl - { - using _Traits = allocator_traits<_Alloc>; - using value_type = typename _Traits::value_type; + class __is_alloc_insertable_impl + { + template<typename _Alloc, typename _Up, + typename _Tp = __remove_cvref_t<_Up>, + typename = decltype(allocator_traits<_Alloc>::construct( + std::declval<_Alloc&>(), std::declval<_Tp*>(), + std::declval<_Up>()))> + static true_type + _M_select(int); - template<typename _Up, typename _Tp = __remove_cvref_t<_Up>, - typename - = decltype(_Traits::construct(std::declval<_Alloc&>(), - std::declval<_Tp*>(), - std::declval<_Up>()))> - static true_type - _M_select(int); + template<typename, typename> + static false_type + _M_select(...); - template<typename _Up> - static false_type - _M_select(...); + protected: + template<typename _Alloc, typename _Tp = typename _Alloc::value_type> + using copy = decltype(_M_select<_Alloc, const _Tp&>(0)); - public: - using copy = decltype(_M_select<const value_type&>(0)); - using move = decltype(_M_select<value_type>(0)); - }; + template<typename _Alloc, typename _Tp = typename _Alloc::value_type> + using move = decltype(_M_select<_Alloc, _Tp>(0)); + }; // true if _Alloc::value_type is CopyInsertable into containers using _Alloc template<typename _Alloc> struct __is_copy_insertable - : __is_alloc_insertable_impl<_Alloc>::copy + : __is_alloc_insertable_impl::template copy<_Alloc> { }; // std::allocator<_Tp> just requires CopyConstructible @@ -614,7 +613,7 @@ _GLIBCXX_BEGIN_NAMESPACE_VERSION // true if _Alloc::value_type is MoveInsertable into containers using _Alloc template<typename _Alloc> struct __is_move_insertable - : __is_alloc_insertable_impl<_Alloc>::move + : __is_alloc_insertable_impl::template move<_Alloc> { }; // std::allocator<_Tp> just requires MoveConstructible 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 index 54b3f53069b..1a34f3e25d3 100644 --- a/libstdc++-v3/testsuite/23_containers/vector/modifiers/push_back/89130.cc +++ b/libstdc++-v3/testsuite/23_containers/vector/modifiers/push_back/89130.cc @@ -15,8 +15,7 @@ // 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 } } +// { dg-do compile { target c++11 } } #include <vector> diff --git a/libstdc++-v3/testsuite/23_containers/vector/modifiers/push_back/89416.cc b/libstdc++-v3/testsuite/23_containers/vector/modifiers/push_back/89416.cc new file mode 100644 index 00000000000..b1077611887 --- /dev/null +++ b/libstdc++-v3/testsuite/23_containers/vector/modifiers/push_back/89416.cc @@ -0,0 +1,44 @@ +// 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-do compile { target c++11 } } + +// PR libstdc++/89416 + +#include <vector> + +template<typename T> + struct Alloc : std::allocator<T> + { + using std::allocator<T>::allocator; + + template<typename U> + struct rebind { using other = Alloc<U>; }; + }; + +struct X +{ + X(int); + X(X&&); +}; + +void test01() +{ + std::vector<X, Alloc<X>> V; + V.push_back(X(1)); + V.emplace_back(1); +}
On 21/02/19 20:47 +0000, Jonathan Wakely wrote: >On 05/02/19 14:45 +0000, Jonathan Wakely wrote: >>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)); > >This caused another regression, fixed by the attached patch. That patch doesn't work with Clang because I made the members protected and forgot to make them public again (and GCC doesn't cre, only Clang notices). Tested powerpc64le-linux, committed to trunk. commit 6f46cf624c814ea231490bf69afe970acf493543 Author: Jonathan Wakely <jwakely@redhat.com> Date: Sun Feb 24 15:32:03 2019 +0000 PR libstdc++/89416 fix accessibility of members PR libstdc++/89416 * include/bits/alloc_traits.h (__is_alloc_insertable_impl): Make copy and move members public. diff --git a/libstdc++-v3/include/bits/alloc_traits.h b/libstdc++-v3/include/bits/alloc_traits.h index 71892cbfaba..9a3d816c42c 100644 --- a/libstdc++-v3/include/bits/alloc_traits.h +++ b/libstdc++-v3/include/bits/alloc_traits.h @@ -590,7 +590,7 @@ _GLIBCXX_BEGIN_NAMESPACE_VERSION static false_type _M_select(...); - protected: + public: template<typename _Alloc, typename _Tp = typename _Alloc::value_type> using copy = decltype(_M_select<_Alloc, const _Tp&>(0));
On 24/02/19 15:46 +0000, Jonathan Wakely wrote: >On 21/02/19 20:47 +0000, Jonathan Wakely wrote: >>On 05/02/19 14:45 +0000, Jonathan Wakely wrote: >>>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)); >> >>This caused another regression, fixed by the attached patch. > >That patch doesn't work with Clang because I made the members >protected and forgot to make them public again (and GCC doesn't cre, >only Clang notices). Aaaand another patch for Clang, because my test using Clang was flawed and only tested std::allocator, which is handled by the partial specialization. These problems are not found using G++ because of the numerous bugs with access control in templates. Tested powerpc64le-linux, committed to trunk. commit f4f6e96aca78d6bfe90dab9b0fd7b94a9e31f241 Author: Jonathan Wakely <jwakely@redhat.com> Date: Tue Feb 26 16:56:36 2019 +0000 PR libstdc++/89416 fix alloc insertable trait for clang (again) PR libstdc++/89416 * include/bits/alloc_traits.h (__is_alloc_insertable_impl): Change to class template and partial specialization using void_t. (__is_copy_insertable, __is_move_insertable): Adjust base class. diff --git a/libstdc++-v3/include/bits/alloc_traits.h b/libstdc++-v3/include/bits/alloc_traits.h index 9a3d816c42c..b8689daf74b 100644 --- a/libstdc++-v3/include/bits/alloc_traits.h +++ b/libstdc++-v3/include/bits/alloc_traits.h @@ -576,32 +576,28 @@ _GLIBCXX_BEGIN_NAMESPACE_VERSION __do_alloc_on_swap(__one, __two, __pocs()); } - class __is_alloc_insertable_impl - { - template<typename _Alloc, typename _Up, - typename _Tp = __remove_cvref_t<_Up>, - typename = decltype(allocator_traits<_Alloc>::construct( - std::declval<_Alloc&>(), std::declval<_Tp*>(), - std::declval<_Up>()))> - static true_type - _M_select(int); + template<typename _Alloc, typename _Tp, + typename _ValueT = __remove_cvref_t<typename _Alloc::value_type>, + typename = void> + struct __is_alloc_insertable_impl + : false_type + { }; - template<typename, typename> - static false_type - _M_select(...); - - public: - template<typename _Alloc, typename _Tp = typename _Alloc::value_type> - using copy = decltype(_M_select<_Alloc, const _Tp&>(0)); - - template<typename _Alloc, typename _Tp = typename _Alloc::value_type> - using move = decltype(_M_select<_Alloc, _Tp>(0)); - }; + template<typename _Alloc, typename _Tp, typename _ValueT> + struct __is_alloc_insertable_impl<_Alloc, _Tp, _ValueT, + __void_t<decltype(allocator_traits<_Alloc>::construct( + std::declval<_Alloc&>(), std::declval<_ValueT*>(), + std::declval<_Tp>()))>> + : true_type + { }; // true if _Alloc::value_type is CopyInsertable into containers using _Alloc + // (might be wrong if _Alloc::construct exists but is not constrained, + // i.e. actually trying to use it would still be invalid. Use with caution.) template<typename _Alloc> struct __is_copy_insertable - : __is_alloc_insertable_impl::template copy<_Alloc> + : __is_alloc_insertable_impl<_Alloc, + typename _Alloc::value_type const&>::type { }; // std::allocator<_Tp> just requires CopyConstructible @@ -611,9 +607,11 @@ _GLIBCXX_BEGIN_NAMESPACE_VERSION { }; // true if _Alloc::value_type is MoveInsertable into containers using _Alloc + // (might be wrong if _Alloc::construct exists but is not constrained, + // i.e. actually trying to use it would still be invalid. Use with caution.) template<typename _Alloc> struct __is_move_insertable - : __is_alloc_insertable_impl::template move<_Alloc> + : __is_alloc_insertable_impl<_Alloc, typename _Alloc::value_type>::type { }; // std::allocator<_Tp> just requires MoveConstructible
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