http://gcc.gnu.org/bugzilla/show_bug.cgi?id=49836

           Summary: vector<T>::push_back() should not require T to be
                    (move-)assignable
           Product: gcc
           Version: 4.7.0
            Status: UNCONFIRMED
          Severity: normal
          Priority: P3
         Component: libstdc++
        AssignedTo: unassig...@gcc.gnu.org
        ReportedBy: zeratul...@hotmail.com


In the current std::vector implementation, push_back() requires the vector's
element type to be assignable.

For example, in the following code T is copy-constructible but not
copy-assignable, and I attempt to push_back() an T object to a vector<T>:

#include <vector>

struct T
{
    T() = default;
    T(const T&) = default;
    T& operator=(const T&) = delete;
};

int main()
{
    std::vector<T> v;
    T t;
    v.push_back(t);
}

And I get the following errors (with --std=c++0x):

In file included from include/c++/4.7.0/vector:70:0,
                 from test3.cpp:1:
include/c++/4.7.0/bits/vector.tcc: In member function 'void std::vector<_Tp,
_Alloc>::_M_insert_aux(std::vector<_Tp, _Alloc>::iterator, _Args&& ...) [with
_Args = {const T&}, _Tp = T, _Alloc = std::allocator<T>, std::vector<_Tp,
_Alloc>::iterator = __gnu_cxx::__normal_iterator<T*, std::vector<T> >, typename
std::_Vector_base<_Tp, _Alloc>::pointer = T*]':
include/c++/4.7.0/bits/stl_vector.h:905:4:   required from 'void
std::vector<_Tp, _Alloc>::push_back(const value_type&) [with _Tp = T, _Alloc =
std::allocator<T>, std::vector<_Tp, _Alloc>::value_type = T]'
test3.cpp:14:18:   required from here
include/c++/4.7.0/bits/vector.tcc:332:4: error: use of deleted function 'T&
T::operator=(const T&)'
test3.cpp:7:8: error: declared here
In file included from include/c++/4.7.0/vector:61:0,
                 from test3.cpp:1:
include/c++/4.7.0/bits/stl_algobase.h: In static member function 'static _BI2
std::__copy_move_backward<true, false,
std::random_access_iterator_tag>::__copy_move_b(_BI1, _BI1, _BI2) [with _BI1 =
T*, _BI2 = T*]':
include/c++/4.7.0/bits/stl_algobase.h:581:18:   required from '_BI2
std::__copy_move_backward_a(_BI1, _BI1, _BI2) [with bool _IsMove = true, _BI1 =
T*, _BI2 = T*]'
include/c++/4.7.0/bits/stl_algobase.h:590:34:   required from '_BI2
std::__copy_move_backward_a2(_BI1, _BI1, _BI2) [with bool _IsMove = true, _BI1
= T*, _BI2 = T*]'
include/c++/4.7.0/bits/stl_algobase.h:661:15:   required from '_BI2
std::move_backward(_BI1, _BI1, _BI2) [with _BI1 = T*, _BI2 = T*]'
include/c++/4.7.0/bits/vector.tcc:326:4:   required from 'void std::vector<_Tp,
_Alloc>::_M_insert_aux(std::vector<_Tp, _Alloc>::iterator, _Args&& ...) [with
_Args = {const T&}, _Tp = T, _Alloc = std::allocator<T>, std::vector<_Tp,
_Alloc>::iterator = __gnu_cxx::__normal_iterator<T*, std::vector<T> >, typename
std::_Vector_base<_Tp, _Alloc>::pointer = T*]'
include/c++/4.7.0/bits/stl_vector.h:905:4:   required from 'void
std::vector<_Tp, _Alloc>::push_back(const value_type&) [with _Tp = T, _Alloc =
std::allocator<T>, std::vector<_Tp, _Alloc>::value_type = T]'
test3.cpp:14:18:   required from here
include/c++/4.7.0/bits/stl_algobase.h:546:6: error: use of deleted function 'T&
T::operator=(const T&)'
test3.cpp:7:8: error: declared here

A similar problem occurs if I make T move-constructible but not
move-assignable, and try to push_back() an rvalue of type T into the vector.

However, FDIS section 23.2.3/16 specifies that push_back() requires only that T
is CopyInsertable into the vector (or MoveInsertable in the case of the rvalue
version of push_back()).

CopyInsertable is defined in section 23.2.1/13 as the expression 

allocator_traits<A>::construct(m, p, v);

being valid (where m is the allocator, p is of type T*, and v is of type T).
For std::allocator, this is just:

::new((void*)p) T(v)

so basically we are requiring T to be copy-constructible (or move-constructible
if v is an rvalue). There is no requirement for T to be copy- or
move-assignable.


The offending statements in the vector implementation (i.e. those which try to
perform assignments) are in the _M_insert_aux helper function (lines 323-333 in
the current trunk):

#ifndef __GXX_EXPERIMENTAL_CXX0X__
      _Tp __x_copy = __x;
#endif
      _GLIBCXX_MOVE_BACKWARD3(__position.base(),
                  this->_M_impl._M_finish - 2,
                  this->_M_impl._M_finish - 1);
#ifndef __GXX_EXPERIMENTAL_CXX0X__
      *__position = __x_copy;
#else
      *__position = _Tp(std::forward<_Args>(__args)...);
#endif

However, this is in a code path that is not reached from push_back()
(push_back() only calls _M_insert_aux() if _M_impl._M_finish ==
_M_impl._M_end_of_storage, but these statements are in an "if
(this->_M_impl._M_finish != this->_M_impl._M_end_of_storage)" block in
_M_insert_aux()).

(This code path is exercised by other vector functions that call
_M_insert_aux(), such as insert() - and *these* functions *do* require T to be
Copy- or MoveAssignable).

The fix should therefore be simple: duplicate _M_insert_aux() into a separate
version used by push_back() and a separate version used by insert() etc, and
remove the offending (and unused) block from push_back()'s version.

Reply via email to