Fix std::vector's use of temporary objects
* include/bits/stl_vector.h (emplace(const_iterator, _Args&&...)): Define inline. Forward to _M_emplace_aux. (insert(const_iterator, value_type&&)): Forward to _M_insert_rval. (_M_insert_rval, _M_emplace_aux): Declare new functions. (_Temporary_value): New RAII type using allocator to construct/destroy. (_S_insert_aux_assign): Remove. (_M_insert_aux): Make non-variadic. * include/bits/vector.tcc (insert(const_iterator, const value_type&)): Use _Temporary_value. (emplace(const_iterator, _Args&&...)): Remove definition. (_M_insert_rval, _M_emplace_aux): Define. (_M_insert_aux): Make non-variadic, stop using _S_insert_aux_assign. (_M_fill_insert): Use _Temporary_value. * testsuite/23_containers/vector/allocator/construction.cc: New test. * testsuite/23_containers/vector/modifiers/insert_vs_emplace.cc: Adjust expected results for emplacing an lvalue with reallocation. * testsuite/23_containers/vector/check_construct_destroy.cc: Adjust expected results to account for construction/destruction of temporary using allocator. From-SVN: r237985
This commit is contained in:
Jonathan Wakely
Jonathan Wakely
parent
bf7499197f
commit
9958c7eb58
7 changed files with 243 additions and 48 deletions
|
|
@ -1,3 +1,26 @@
|
|||
2016-07-04 Jonathan Wakely <jwakely@redhat.com>
|
||||
|
||||
* include/bits/stl_vector.h (emplace(const_iterator, _Args&&...)):
|
||||
Define inline. Forward to _M_emplace_aux.
|
||||
(insert(const_iterator, value_type&&)): Forward to _M_insert_rval.
|
||||
(_M_insert_rval, _M_emplace_aux): Declare new functions.
|
||||
(_Temporary_value): New RAII type using allocator to construct/destroy.
|
||||
(_S_insert_aux_assign): Remove.
|
||||
(_M_insert_aux): Make non-variadic.
|
||||
* include/bits/vector.tcc (insert(const_iterator, const value_type&)):
|
||||
Use _Temporary_value.
|
||||
(emplace(const_iterator, _Args&&...)): Remove definition.
|
||||
(_M_insert_rval, _M_emplace_aux): Define.
|
||||
(_M_insert_aux): Make non-variadic, stop using _S_insert_aux_assign.
|
||||
(_M_fill_insert): Use _Temporary_value.
|
||||
* testsuite/23_containers/vector/allocator/construction.cc: New test.
|
||||
* testsuite/23_containers/vector/modifiers/insert_vs_emplace.cc:
|
||||
Adjust expected results for emplacing an lvalue with reallocation.
|
||||
* testsuite/23_containers/vector/check_construct_destroy.cc: Adjust
|
||||
expected results to account for construction/destruction of temporary
|
||||
using allocator.
|
||||
* testsuite/backward/hash_set/check_construct_destroy.cc: Likewise.
|
||||
|
||||
2016-07-04 Ville Voutilainen <ville.voutilainen@gmail.com>
|
||||
|
||||
PR libstdc++/71313
|
||||
|
|
|
|||
|
|
@ -995,7 +995,8 @@ _GLIBCXX_BEGIN_NAMESPACE_CONTAINER
|
|||
*/
|
||||
template<typename... _Args>
|
||||
iterator
|
||||
emplace(const_iterator __position, _Args&&... __args);
|
||||
emplace(const_iterator __position, _Args&&... __args)
|
||||
{ return _M_emplace_aux(__position, std::forward<_Args>(__args)...); }
|
||||
|
||||
/**
|
||||
* @brief Inserts given value into %vector before specified iterator.
|
||||
|
|
@ -1040,7 +1041,7 @@ _GLIBCXX_BEGIN_NAMESPACE_CONTAINER
|
|||
*/
|
||||
iterator
|
||||
insert(const_iterator __position, value_type&& __x)
|
||||
{ return emplace(__position, std::move(__x)); }
|
||||
{ return _M_insert_rval(__position, std::move(__x)); }
|
||||
|
||||
/**
|
||||
* @brief Inserts an initializer_list into the %vector.
|
||||
|
|
@ -1431,30 +1432,65 @@ _GLIBCXX_BEGIN_NAMESPACE_CONTAINER
|
|||
_M_shrink_to_fit();
|
||||
#endif
|
||||
|
||||
// Called by insert(p,x)
|
||||
#if __cplusplus < 201103L
|
||||
// Called by insert(p,x)
|
||||
void
|
||||
_M_insert_aux(iterator __position, const value_type& __x);
|
||||
#else
|
||||
template<typename... _Args>
|
||||
static void
|
||||
_S_insert_aux_assign(iterator __pos, _Args&&... __args)
|
||||
{ *__pos = _Tp(std::forward<_Args>(__args)...); }
|
||||
// A value_type object constructed with _Alloc_traits::construct()
|
||||
// and destroyed with _Alloc_traits::destroy().
|
||||
struct _Temporary_value
|
||||
{
|
||||
template<typename... _Args>
|
||||
explicit
|
||||
_Temporary_value(vector* __vec, _Args&&... __args) : _M_this(__vec)
|
||||
{
|
||||
_Alloc_traits::construct(_M_this->_M_impl, _M_ptr(),
|
||||
std::forward<_Args>(__args)...);
|
||||
}
|
||||
|
||||
static void
|
||||
_S_insert_aux_assign(iterator __pos, _Tp&& __arg)
|
||||
{ *__pos = std::move(__arg); }
|
||||
~_Temporary_value()
|
||||
{ _Alloc_traits::destroy(_M_this->_M_impl, _M_ptr()); }
|
||||
|
||||
template<typename... _Args>
|
||||
value_type&
|
||||
_M_val() { return *reinterpret_cast<_Tp*>(&__buf); }
|
||||
|
||||
private:
|
||||
pointer
|
||||
_M_ptr() { return pointer_traits<pointer>::pointer_to(_M_val()); }
|
||||
|
||||
vector* _M_this;
|
||||
typename aligned_storage<sizeof(_Tp), alignof(_Tp)>::type __buf;
|
||||
};
|
||||
|
||||
// Called by insert(p,x) and other functions when insertion needs to
|
||||
// reallocate or move existing elements. _Arg is either _Tp& or _Tp.
|
||||
template<typename _Arg>
|
||||
void
|
||||
_M_insert_aux(iterator __position, _Args&&... __args);
|
||||
_M_insert_aux(iterator __position, _Arg&& __arg);
|
||||
|
||||
// Either move-construct at the end, or forward to _M_insert_aux.
|
||||
iterator
|
||||
_M_insert_rval(const_iterator __position, value_type&& __v);
|
||||
|
||||
// Called by push_back(x) and emplace_back(args) when they need to
|
||||
// reallocate.
|
||||
template<typename... _Args>
|
||||
void
|
||||
_M_emplace_back_aux(_Args&&... __args);
|
||||
|
||||
// Try to emplace at the end, otherwise forward to _M_insert_aux.
|
||||
template<typename... _Args>
|
||||
iterator
|
||||
_M_emplace_aux(const_iterator __position, _Args&&... __args);
|
||||
|
||||
// Emplacing an rvalue of the correct type can use _M_insert_rval.
|
||||
iterator
|
||||
_M_emplace_aux(const_iterator __position, value_type&& __v)
|
||||
{ return _M_insert_rval(__position, std::move(__v)); }
|
||||
#endif
|
||||
|
||||
// Called by the latter.
|
||||
// Called by _M_fill_insert, _M_insert_aux etc.
|
||||
size_type
|
||||
_M_check_len(size_type __n, const char* __s) const
|
||||
{
|
||||
|
|
|
|||
|
|
@ -124,8 +124,10 @@ _GLIBCXX_BEGIN_NAMESPACE_CONTAINER
|
|||
const auto __pos = begin() + (__position - cbegin());
|
||||
if (this->_M_impl._M_finish != this->_M_impl._M_end_of_storage)
|
||||
{
|
||||
_Tp __x_copy = __x;
|
||||
_M_insert_aux(__pos, std::move(__x_copy));
|
||||
// __x could be an existing element of this vector, so make a
|
||||
// copy of it before _M_insert_aux moves elements around.
|
||||
_Temporary_value __x_copy(this, __x);
|
||||
_M_insert_aux(__pos, std::move(__x_copy._M_val()));
|
||||
}
|
||||
else
|
||||
_M_insert_aux(__pos, __x);
|
||||
|
|
@ -296,31 +298,50 @@ _GLIBCXX_BEGIN_NAMESPACE_CONTAINER
|
|||
}
|
||||
|
||||
#if __cplusplus >= 201103L
|
||||
template<typename _Tp, typename _Alloc>
|
||||
auto
|
||||
vector<_Tp, _Alloc>::
|
||||
_M_insert_rval(const_iterator __position, value_type&& __v) -> iterator
|
||||
{
|
||||
const auto __n = __position - cbegin();
|
||||
if (this->_M_impl._M_finish != this->_M_impl._M_end_of_storage
|
||||
&& __position == cend())
|
||||
{
|
||||
_Alloc_traits::construct(this->_M_impl, this->_M_impl._M_finish,
|
||||
std::move(__v));
|
||||
++this->_M_impl._M_finish;
|
||||
}
|
||||
else
|
||||
_M_insert_aux(begin() + __n, std::move(__v));
|
||||
return iterator(this->_M_impl._M_start + __n);
|
||||
}
|
||||
|
||||
template<typename _Tp, typename _Alloc>
|
||||
template<typename... _Args>
|
||||
typename vector<_Tp, _Alloc>::iterator
|
||||
auto
|
||||
vector<_Tp, _Alloc>::
|
||||
emplace(const_iterator __position, _Args&&... __args)
|
||||
_M_emplace_aux(const_iterator __position, _Args&&... __args)
|
||||
-> iterator
|
||||
{
|
||||
const size_type __n = __position - begin();
|
||||
if (this->_M_impl._M_finish != this->_M_impl._M_end_of_storage
|
||||
&& __position == end())
|
||||
{
|
||||
_Alloc_traits::construct(this->_M_impl, this->_M_impl._M_finish,
|
||||
std::forward<_Args>(__args)...);
|
||||
++this->_M_impl._M_finish;
|
||||
}
|
||||
const auto __n = __position - cbegin();
|
||||
if (__position == cend())
|
||||
emplace_back(std::forward<_Args>(__args)...);
|
||||
else
|
||||
_M_insert_aux(begin() + (__position - cbegin()),
|
||||
std::forward<_Args>(__args)...);
|
||||
{
|
||||
// We need to construct a temporary because something in __args...
|
||||
// could alias one of the elements of the container and so we
|
||||
// need to use it before _M_insert_aux moves elements around.
|
||||
_Temporary_value __tmp(this, std::forward<_Args>(__args)...);
|
||||
_M_insert_aux(begin() + __n, std::move(__tmp._M_val()));
|
||||
}
|
||||
return iterator(this->_M_impl._M_start + __n);
|
||||
}
|
||||
|
||||
template<typename _Tp, typename _Alloc>
|
||||
template<typename... _Args>
|
||||
template<typename _Arg>
|
||||
void
|
||||
vector<_Tp, _Alloc>::
|
||||
_M_insert_aux(iterator __position, _Args&&... __args)
|
||||
_M_insert_aux(iterator __position, _Arg&& __arg)
|
||||
#else
|
||||
template<typename _Tp, typename _Alloc>
|
||||
void
|
||||
|
|
@ -343,7 +364,7 @@ _GLIBCXX_BEGIN_NAMESPACE_CONTAINER
|
|||
#if __cplusplus < 201103L
|
||||
*__position = __x_copy;
|
||||
#else
|
||||
_S_insert_aux_assign(__position, std::forward<_Args>(__args)...);
|
||||
*__position = std::forward<_Arg>(__arg);
|
||||
#endif
|
||||
}
|
||||
else
|
||||
|
|
@ -355,14 +376,15 @@ _GLIBCXX_BEGIN_NAMESPACE_CONTAINER
|
|||
pointer __new_finish(__new_start);
|
||||
__try
|
||||
{
|
||||
// The order of the three operations is dictated by the C++0x
|
||||
// The order of the three operations is dictated by the C++11
|
||||
// case, where the moves could alter a new element belonging
|
||||
// to the existing vector. This is an issue only for callers
|
||||
// taking the element by const lvalue ref (see 23.1/13).
|
||||
// taking the element by lvalue ref (see last bullet of C++11
|
||||
// [res.on.arguments]).
|
||||
_Alloc_traits::construct(this->_M_impl,
|
||||
__new_start + __elems_before,
|
||||
#if __cplusplus >= 201103L
|
||||
std::forward<_Args>(__args)...);
|
||||
std::forward<_Arg>(__arg));
|
||||
#else
|
||||
__x);
|
||||
#endif
|
||||
|
|
@ -455,7 +477,12 @@ _GLIBCXX_BEGIN_NAMESPACE_CONTAINER
|
|||
if (size_type(this->_M_impl._M_end_of_storage
|
||||
- this->_M_impl._M_finish) >= __n)
|
||||
{
|
||||
#if __cplusplus < 201103L
|
||||
value_type __x_copy = __x;
|
||||
#else
|
||||
_Temporary_value __tmp(this, __x);
|
||||
value_type& __x_copy = __tmp._M_val();
|
||||
#endif
|
||||
const size_type __elems_after = end() - __position;
|
||||
pointer __old_finish(this->_M_impl._M_finish);
|
||||
if (__elems_after > __n)
|
||||
|
|
|
|||
|
|
@ -0,0 +1,105 @@
|
|||
// Copyright (C) 2016 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++11" }
|
||||
// { dg-do compile }
|
||||
|
||||
#include <vector>
|
||||
|
||||
struct Tag { };
|
||||
|
||||
template<typename T>
|
||||
struct TaggingAllocator
|
||||
{
|
||||
using value_type = T;
|
||||
|
||||
TaggingAllocator() = default;
|
||||
|
||||
template<typename U>
|
||||
TaggingAllocator(const TaggingAllocator<U>&) { }
|
||||
|
||||
T*
|
||||
allocate(std::size_t n) { return std::allocator<T>{}.allocate(n); }
|
||||
|
||||
void
|
||||
deallocate(T* p, std::size_t n) { std::allocator<T>{}.deallocate(p, n); }
|
||||
|
||||
template<typename U, typename... Args>
|
||||
void
|
||||
construct(U* p, Args&&... args)
|
||||
{ ::new((void*)p) U(Tag{}, std::forward<Args>(args)...); }
|
||||
|
||||
template<typename U, typename... Args>
|
||||
void
|
||||
destroy(U* p)
|
||||
{ p->~U(); }
|
||||
};
|
||||
|
||||
template<typename T, typename U>
|
||||
bool
|
||||
operator==(const TaggingAllocator<T>&, const TaggingAllocator<U>&)
|
||||
{ return true; }
|
||||
|
||||
template<typename T, typename U>
|
||||
bool
|
||||
operator!=(const TaggingAllocator<T>&, const TaggingAllocator<U>&)
|
||||
{ return false; }
|
||||
|
||||
struct X
|
||||
{
|
||||
// All constructors must be passed the Tag type.
|
||||
|
||||
// DefaultInsertable into vector<X, TaggingAllocator<X>>,
|
||||
X(Tag) { }
|
||||
// CopyInsertable into vector<X, TaggingAllocator<X>>,
|
||||
X(Tag, const X&) { }
|
||||
// MoveInsertable into vector<X, TaggingAllocator<X>>, and
|
||||
X(Tag, X&&) { }
|
||||
|
||||
// EmplaceConstructible into vector<X, TaggingAllocator<X>> from args.
|
||||
template<typename... Args>
|
||||
X(Tag, Args&&...) { }
|
||||
|
||||
// not DefaultConstructible, CopyConstructible or MoveConstructible.
|
||||
X() = delete;
|
||||
X(const X&) = delete;
|
||||
X(X&&) = delete;
|
||||
|
||||
// CopyAssignable.
|
||||
X& operator=(const X&) { return *this; }
|
||||
|
||||
// MoveAssignable.
|
||||
X& operator=(X&&) { return *this; }
|
||||
|
||||
private:
|
||||
// Not Destructible.
|
||||
~X() { }
|
||||
|
||||
// Erasable from vector<X, TaggingAllocator<X>>.
|
||||
friend class TaggingAllocator<X>;
|
||||
};
|
||||
|
||||
template class std::vector<X, TaggingAllocator<X>>;
|
||||
|
||||
void test01()
|
||||
{
|
||||
std::vector<X, TaggingAllocator<X>> v;
|
||||
v.reserve(3);
|
||||
v.emplace_back();
|
||||
v.emplace(v.begin());
|
||||
v.emplace(v.begin(), 1, 2, 3);
|
||||
}
|
||||
|
|
@ -49,9 +49,9 @@ int main()
|
|||
c.reserve(100);
|
||||
tracker_allocator_counter::reset();
|
||||
c.insert(c.begin(), arr10[0]);
|
||||
ok = check_construct_destroy("Insert element", 1, 0) && ok;
|
||||
ok = check_construct_destroy("Insert element", 2, 1) && ok;
|
||||
}
|
||||
ok = check_construct_destroy("Insert element", 1, 11) && ok;
|
||||
ok = check_construct_destroy("Insert element", 2, 12) && ok;
|
||||
|
||||
{
|
||||
Container c(arr10, arr10 + 10);
|
||||
|
|
|
|||
|
|
@ -223,7 +223,8 @@ test03()
|
|||
void
|
||||
test04()
|
||||
{
|
||||
const X::special expected{ 0, 3, 1, 0, 3, 0 };
|
||||
const X::special expected_ins{ 0, 3, 1, 0, 3, 0 };
|
||||
const X::special expected_emp{ 0, 4, 1, 0, 4, 0 };
|
||||
X::special ins, emp;
|
||||
{
|
||||
std::vector<X> v;
|
||||
|
|
@ -253,8 +254,8 @@ test04()
|
|||
// std::cout << "----\n";
|
||||
emp = X::sp;
|
||||
}
|
||||
VERIFY( ins == emp );
|
||||
VERIFY( ins == expected );
|
||||
VERIFY( ins == expected_ins );
|
||||
VERIFY( emp == expected_emp );
|
||||
}
|
||||
|
||||
// insert vs emplace xvalue reallocation
|
||||
|
|
|
|||
|
|
@ -39,45 +39,48 @@ int main()
|
|||
|
||||
int buckets;
|
||||
|
||||
// Add 1 to all counts, because the std::vector used internally by the
|
||||
// hashtable creates and destroys a temporary object using the allocator.
|
||||
|
||||
tracker_allocator_counter::reset();
|
||||
{
|
||||
Container c;
|
||||
buckets = c.bucket_count();
|
||||
ok = check_construct_destroy("empty container", buckets, 0) && ok;
|
||||
ok = check_construct_destroy("empty container", buckets+1, 1) && ok;
|
||||
}
|
||||
ok = check_construct_destroy("empty container", buckets, buckets) && ok;
|
||||
ok = check_construct_destroy("empty container", buckets+1, buckets+1) && ok;
|
||||
|
||||
|
||||
tracker_allocator_counter::reset();
|
||||
{
|
||||
Container c(arr10, arr10 + 10);
|
||||
ok = check_construct_destroy("Construct from range", buckets+10, 0) && ok;
|
||||
ok = check_construct_destroy("Construct from range", buckets+10+1, 1) && ok;
|
||||
}
|
||||
ok = check_construct_destroy("Construct from range", buckets+10, buckets+10) && ok;
|
||||
ok = check_construct_destroy("Construct from range", buckets+10+1, buckets+10+1) && ok;
|
||||
|
||||
tracker_allocator_counter::reset();
|
||||
{
|
||||
Container c(arr10, arr10 + 10);
|
||||
c.insert(arr10a[0]);
|
||||
ok = check_construct_destroy("Insert element", buckets+11, 0) && ok;
|
||||
ok = check_construct_destroy("Insert element", buckets+11+1, 1) && ok;
|
||||
}
|
||||
ok = check_construct_destroy("Insert element", buckets+11, buckets+11) && ok;
|
||||
ok = check_construct_destroy("Insert element", buckets+11+1, buckets+11+1) && ok;
|
||||
|
||||
tracker_allocator_counter::reset();
|
||||
{
|
||||
Container c(arr10, arr10 + 10);
|
||||
c.insert(arr10a, arr10a+3);
|
||||
ok = check_construct_destroy("Insert short range", buckets+13, 0) && ok;
|
||||
ok = check_construct_destroy("Insert short range", buckets+13+1, 1) && ok;
|
||||
}
|
||||
ok = check_construct_destroy("Insert short range", buckets+13, buckets+13) && ok;
|
||||
ok = check_construct_destroy("Insert short range", buckets+13+1, buckets+13+1) && ok;
|
||||
|
||||
tracker_allocator_counter::reset();
|
||||
{
|
||||
Container c(arr10, arr10 + 10);
|
||||
c.insert(arr10a, arr10a+10);
|
||||
ok = check_construct_destroy("Insert long range", buckets+20, 0) && ok;
|
||||
ok = check_construct_destroy("Insert long range", buckets+20+1, 1) && ok;
|
||||
}
|
||||
ok = check_construct_destroy("Insert long range", buckets+20, buckets+20) && ok;
|
||||
ok = check_construct_destroy("Insert long range", buckets+20+1, buckets+20+1) && ok;
|
||||
|
||||
return ok ? 0 : 1;
|
||||
}
|
||||
|
|
|
|||
Loading…
Add table
Reference in a new issue