[Bug libstdc++/89130] [9 Regression] std::vector relocation fails for types with deleted move constructor
https://gcc.gnu.org/bugzilla/show_bug.cgi?id=89130 --- Comment #9 from Jonathan Wakely --- With the removal of if constexpr we would unconditionally instantiate __relocate_a, which could fail ... but only in cases like a deleted move constructor. This avoids that instantiation, so it doesn't fail for weird types, and doesn't spend time instantiating a function template that isn't needed. Not strictly necessary, but like you say, it avoids gratuitously rejecting some types that are weird, but can still be used in limited ways. OK, let's leave it like this.
[Bug libstdc++/89130] [9 Regression] std::vector relocation fails for types with deleted move constructor
https://gcc.gnu.org/bugzilla/show_bug.cgi?id=89130 --- Comment #8 from Marc Glisse --- (In reply to Jonathan Wakely from comment #7) > So I guess I could revert r268537 then. I don't think it was worth spending time on in the first place, but now that you have written it, it isn't as bad as I feared. I am usually in favor of **not** gratuitously rejecting things because the concept says so, unless we expect some use with SFINAE (although I see in PR 89164 that you prefer to be strict). Besides, isn't part of this patch also necessary for the if constexpr change?
[Bug libstdc++/89130] [9 Regression] std::vector relocation fails for types with deleted move constructor
https://gcc.gnu.org/bugzilla/show_bug.cgi?id=89130 --- Comment #7 from Jonathan Wakely --- Oh, so it does. So I guess I could revert r268537 then. The downstream package where this caused a build failure was already changed to stop (foolishly) deleting move ctors, so it's not causing any more problems that I know of.
[Bug libstdc++/89130] [9 Regression] std::vector relocation fails for types with deleted move constructor
https://gcc.gnu.org/bugzilla/show_bug.cgi?id=89130 --- Comment #6 from Marc Glisse --- (In reply to Jonathan Wakely from comment #5) > Looking at the standard, the requirements for the push_back call in comment > 0 are that X is Cpp17CopyInsertable into vector, which is true. The check > whether to use relocation fails if it isn't also Cpp17MoveInsertable into > vector, which is not a requirement. So we do need to fix it, which I've > now done. "T is Cpp17CopyInsertable into X means that, in addition to T being Cpp17MoveInsertable into X, [...]"
[Bug libstdc++/89130] [9 Regression] std::vector relocation fails for types with deleted move constructor
https://gcc.gnu.org/bugzilla/show_bug.cgi?id=89130 Jonathan Wakely changed: What|Removed |Added Status|ASSIGNED|RESOLVED Resolution|--- |FIXED --- Comment #5 from Jonathan Wakely --- (In reply to Marc Glisse from comment #3) > We already discussed this > https://gcc.gnu.org/ml/libstdc++/2018-09/msg7.html Ah yes, I thought I remembered a slightly different discussion, but it's exactly this issue. Looking at the standard, the requirements for the push_back call in comment 0 are that X is Cpp17CopyInsertable into vector, which is true. The check whether to use relocation fails if it isn't also Cpp17MoveInsertable into vector, which is not a requirement. So we do need to fix it, which I've now done.
[Bug libstdc++/89130] [9 Regression] std::vector relocation fails for types with deleted move constructor
https://gcc.gnu.org/bugzilla/show_bug.cgi?id=89130 --- Comment #4 from Jonathan Wakely --- Author: redi Date: Tue Feb 5 14:45:00 2019 New Revision: 268537 URL: https://gcc.gnu.org/viewcvs?rev=268537=gcc=rev Log: 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. Added: trunk/libstdc++-v3/testsuite/23_containers/vector/modifiers/push_back/89130.cc Modified: trunk/libstdc++-v3/ChangeLog trunk/libstdc++-v3/include/bits/alloc_traits.h trunk/libstdc++-v3/include/bits/stl_vector.h trunk/libstdc++-v3/include/bits/vector.tcc
[Bug libstdc++/89130] [9 Regression] std::vector relocation fails for types with deleted move constructor
https://gcc.gnu.org/bugzilla/show_bug.cgi?id=89130 Richard Biener changed: What|Removed |Added Priority|P3 |P1 Target Milestone|--- |9.0
[Bug libstdc++/89130] [9 Regression] std::vector relocation fails for types with deleted move constructor
https://gcc.gnu.org/bugzilla/show_bug.cgi?id=89130 --- Comment #3 from Marc Glisse --- We already discussed this https://gcc.gnu.org/ml/libstdc++/2018-09/msg7.html
[Bug libstdc++/89130] [9 Regression] std::vector relocation fails for types with deleted move constructor
https://gcc.gnu.org/bugzilla/show_bug.cgi?id=89130 --- Comment #2 from Jonathan Wakely --- However, the code that this relocation replaces uses move_if_noexcept which is also incorrect for weird allocators. So maybe those cases are already incorrect.
[Bug libstdc++/89130] [9 Regression] std::vector relocation fails for types with deleted move constructor
https://gcc.gnu.org/bugzilla/show_bug.cgi?id=89130
Jonathan Wakely changed:
What|Removed |Added
CC||glisse at gcc dot gnu.org
--- Comment #1 from Jonathan Wakely ---
This fixes the example above:
--- a/libstdc++-v3/include/bits/stl_uninitialized.h
+++ b/libstdc++-v3/include/bits/stl_uninitialized.h
@@ -883,7 +883,7 @@ _GLIBCXX_BEGIN_NAMESPACE_VERSION
#if __cplusplus >= 201103L
template
-inline void
+inline __enable_if_t::value>
__relocate_object_a(_Tp* __dest, _Up* __orig, _Allocator& __alloc)
noexcept(noexcept(std::allocator_traits<_Allocator>::construct(__alloc,
__dest, std::move(*__orig)))
@@ -895,6 +895,11 @@ _GLIBCXX_BEGIN_NAMESPACE_VERSION
__traits::destroy(__alloc, std::__addressof(*__orig));
}
+ template
+inline __enable_if_t::value>
+__relocate_object_a(_Tp*, _Up*, _Allocator&) noexcept(false)
+{ }
+
// This class may be specialized for specific types.
template
struct __is_trivially_relocatable
The trick here is to select an overload of __relocate_object_a that has a
potentially throwing noexcept-specifier, which means that std::vector will
never try to use it.
This only works if allocator_traits::construct forwards the arguments
unchanged, because otherwise the is_constructible check is wrong.
Maybe as discussed we need to detect when allocator_traits::construct has been
specialized, and not try to relocate in that case.
Or maybe we should just revert this relocation stuff for now.
[Bug libstdc++/89130] [9 Regression] std::vector relocation fails for types with deleted move constructor
https://gcc.gnu.org/bugzilla/show_bug.cgi?id=89130 Jonathan Wakely changed: What|Removed |Added Status|UNCONFIRMED |ASSIGNED Last reconfirmed||2019-01-31 Ever confirmed|0 |1
