operator= required for std::vector<>::push_back ??
in [C++]
|
Prev: const as default
Next: CRTP and typedef
From: Miles Bader on 31 Jan 2010 13:04 If I have the following test file, x.cc: #include <vector> class X { public: X (const int &_i) : i (_i) { } X (const X &x) : i (x.i) { } #if 0 X &operator= (const X &x) { if (&x != this) new (this) X (x.i); return *this; } #endif const int &i; }; int test () { std::vector<X> xv; int i; xv.push_back (X (i)); } I get an error during compilation, like: g++ -c -o x.o x.cc x.cc: In member function 'X& X::operator=(const X&)': x.cc:4: instantiated from 'void std::vector<_Tp, _Alloc>::_M_insert_aux(__gnu_cxx::__normal_iterator<typename std::_Vector_base<_Tp, _Alloc>::_Tp_alloc_type::pointer, std::vector<_Tp, _Alloc> >, const _Tp&) [with _Tp = X, _Alloc = std::allocator<X>]' /usr/include/c++/4.4/bits/stl_vector.h:741: instantiated from 'void std::vector<_Tp, _Alloc>::push_back(const _Tp&) [with _Tp = X, _Alloc = std::allocator<X>]' x.cc:23: instantiated from here x.cc:4: error: non-static reference member 'const int& X::i', can't use default assignment operator Changing the "#if 0" to "#if 1" to define operator= makes things work. I understand the immediate reason for the error: the g++ vector implementation does indeed seem to call operator= on vector elements for push_back, and in this case, the compiler-synthesized method isn't good enough. But it somehow seems wrong that it's using operator= at all; shouldn't a copy-constructor be enough? Looking at the g++ STL headers, it seems like the problem is that it's defining push_back in terms of insertion, and for the latter, you need operator= to move elements subsequent elements to make room for a new element. However in the case of push_back, that's really unnecessary, and no copying of objects other than copy-constructing into new memory when the vector's underlying storage is reallocated should be need. Opinions? Is the error here reasonable? Thanks, -Miles -- Happiness, n. An agreeable sensation arising from contemplating the misery of another. [ See http://www.gotw.ca/resources/clcm.htm for info about ] [ comp.lang.c++.moderated. First time posters: Do this! ]
From: Goran on 31 Jan 2010 20:58 On Feb 1, 7:04 am, Miles Bader <mi...(a)gnu.org> wrote: > If I have the following test file, x.cc: > > #include <vector> > > class X > { > public: > X (const int &_i) : i (_i) { } > X (const X &x) : i (x.i) { } > #if 0 > X &operator= (const X &x) > { > if (&x != this) > new (this) X (x.i); > return *this; > } > #endif > const int &i; > }; > > int test () > { > std::vector<X> xv; > > int i; > xv.push_back (X (i)); > } > > I get an error during compilation, like: > > g++ -c -o x.o x.cc > x.cc: In member function �X& X::operator=(const X&)�: > x.cc:4: instantiated from �void std::vector<_Tp, _Alloc>::_M_insert_aux(__gnu_cxx::__normal_iterator<typename std::_Vector_base<_Tp, _Alloc>::_Tp_alloc_type::pointer, std::vector<_Tp, _Alloc> >, const _Tp&) [with _Tp = X, _Alloc = std::allocator<X>]� > /usr/include/c++/4.4/bits/stl_vector.h:741: instantiated from �void std::vector<_Tp, _Alloc>::push_back(const _Tp&) [with _Tp = X, _Alloc = std::allocator<X>]� > x.cc:23: instantiated from here > x.cc:4: error: non-static reference member �const int& X::i�, can't use default assignment operator > > Changing the "#if 0" to "#if 1" to define operator= makes things work. > > I understand the immediate reason for the error: the g++ vector > implementation does indeed seem to call operator= on vector elements for > push_back, and in this case, the compiler-synthesized method isn't good > enough. I would guess that in this case compiler can't emit operator= at all, because you have a reference member. IOW, I think that your class is not copy-able because it has a reference member, and that is the reason why you are using ugly "construction in operator=" trick. That is almost always unwarranted and whoever taught you this is wrong. If you want copying, you could just as well be using a pointer there (but expose a reference-like semantics to users if that's what you need). There is very little, if anything, to to gain in using a reference there. > > But it somehow seems wrong that it's using operator= at all; shouldn't a > copy-constructor be enough? > > Looking at the g++ STL headers, it seems like the problem is that it's > defining push_back in terms of insertion, and for the latter, you need > operator= to move elements subsequent elements to make room for a new > element. However in the case of push_back, that's really unnecessary, > and no copying of objects other than copy-constructing into new memory > when the vector's underlying storage is reallocated should be need. > > Opinions? Is the error here reasonable? Yes, it's reasonable, because push_back might need to re-allocate storage, at which point it needs to copy existing elements to new place. So IMO, the need to copy can be avoided only by dropping part of push_back functionality. Goran. -- [ See http://www.gotw.ca/resources/clcm.htm for info about ] [ comp.lang.c++.moderated. First time posters: Do this! ]
From: Tony Delroy on 31 Jan 2010 22:04 On Feb 1, 3:04 pm, Miles Bader <mi...(a)gnu.org> wrote: > Looking at the g++ STL headers, it seems like the problem is that it's > defining push_back in terms of insertion, and for the latter, you need > operator= to move elements subsequent elements to make room for a new > element. However in the case of push_back, that's really unnecessary, > and no copying of objects other than copy-constructing into new memory > when the vector's underlying storage is reallocated should be need. > > Opinions? Is the error here reasonable? I think it's better for the compilation to fail. Bigger picture: library developers need to be able to say "input: vector of T", and know they can do all the things to the vector that a vector is meant to support. Having half-baked vectors around that will break when someone re-implements some algorithm, using other parts of the vector interface, is unacceptable at the enterprise scale. The interface is not intended to be "fat", with code checking which vector operations a particular instantiation will support.... Cheers, Tony -- [ See http://www.gotw.ca/resources/clcm.htm for info about ] [ comp.lang.c++.moderated. First time posters: Do this! ]
From: Daniel Krügler on 1 Feb 2010 04:04 On 1 Feb., 07:04, Miles Bader <mi...(a)gnu.org> wrote: > If I have the following test file, x.cc: > > #include <vector> > > class X > { > public: > X (const int &_i) : i (_i) { } > X (const X &x) : i (x.i) { } > #if 0 > X &operator= (const X &x) > { > if (&x != this) > new (this) X (x.i); > return *this; > } > #endif > const int &i; > }; > > int test () > { > std::vector<X> xv; > > int i; > xv.push_back (X (i)); > } > > I get an error during compilation, like: > > g++ -c -o x.o x.cc > x.cc: In member function �X& X::operator=(const X&)�: > x.cc:4: instantiated from �void std::vector<_Tp, _Alloc>::_M_insert_aux(__gnu_cxx::__normal_iterator<typename std::_Vector_base<_Tp, _Alloc>::_Tp_alloc_type::pointer, std::vector<_Tp, _Alloc> >, const _Tp&) [with _Tp = X, _Alloc = std::allocator<X>]� > /usr/include/c++/4.4/bits/stl_vector.h:741: instantiated from �void std::vector<_Tp, _Alloc>::push_back(const _Tp&) [with _Tp = X, _Alloc = std::allocator<X>]� > x.cc:23: instantiated from here > x.cc:4: error: non-static reference member �const int& X::i�, can't use default assignment operator > > Changing the "#if 0" to "#if 1" to define operator= makes things work. > > I understand the immediate reason for the error: the g++ vector > implementation does indeed seem to call operator= on vector elements for > push_back, and in this case, the compiler-synthesized method isn't good > enough. > > But it somehow seems wrong that it's using operator= at all; shouldn't a > copy-constructor be enough? > > Looking at the g++ STL headers, it seems like the problem is that it's > defining push_back in terms of insertion, and for the latter, you need > operator= to move elements subsequent elements to make room for a new > element. However in the case of push_back, that's really unnecessary, > and no copying of objects other than copy-constructing into new memory > when the vector's underlying storage is reallocated should be need. > > Opinions? Is the error here reasonable? Yes and no ;-) According to the requirements of the C++03 standard the error message is reasonable, because the fundamental requirement exists, that any value type of a container has to satisfy the CopyConstructible and Assignable requirements ([lib.container.requirements]/3). In C++0x there is some redrafting in progress that will define the requirements more specifically. In case of push_back provided with an rvalue (as in your example), the necessary requirement should be that the value type satisfies the MoveConstructible requirements (which is a subset of CopyConstructible). HTH & Greetings from Bremen, Daniel Kr�gler -- [ See http://www.gotw.ca/resources/clcm.htm for info about ] [ comp.lang.c++.moderated. First time posters: Do this! ]
From: Bart van Ingen Schenau on 1 Feb 2010 04:04
On Feb 1, 7:04 am, Miles Bader <mi...(a)gnu.org> wrote: > If I have the following test file, x.cc: > > #include <vector> > > class X > { > public: > X (const int &_i) : i (_i) { } > X (const X &x) : i (x.i) { } > #if 0 > X &operator= (const X &x) > { > if (&x != this) > new (this) X (x.i); > return *this; > } > #endif > const int &i; > }; > > int test () > { > std::vector<X> xv; > > int i; > xv.push_back (X (i)); > } > > I get an error during compilation, like: > > g++ -c -o x.o x.cc > x.cc: In member function �X& X::operator=(const X&)�: > x.cc:4: instantiated from �void std::vector<_Tp, _Alloc>::_M_insert_aux(__gnu_cxx::__normal_iterator<typename std::_Vector_base<_Tp, _Alloc>::_Tp_alloc_type::pointer, std::vector<_Tp, _Alloc> >, const _Tp&) [with _Tp = X, _Alloc = std::allocator<X>]� > /usr/include/c++/4.4/bits/stl_vector.h:741: instantiated from �void std::vector<_Tp, _Alloc>::push_back(const _Tp&) [with _Tp = X, _Alloc = std::allocator<X>]� > x.cc:23: instantiated from here > x.cc:4: error: non-static reference member �const int& X::i�, can't use default assignment operator > > Changing the "#if 0" to "#if 1" to define operator= makes things work. > > I understand the immediate reason for the error: the g++ vector > implementation does indeed seem to call operator= on vector elements for > push_back, and in this case, the compiler-synthesized method isn't good > enough. > > But it somehow seems wrong that it's using operator= at all; shouldn't a > copy-constructor be enough? > > Looking at the g++ STL headers, it seems like the problem is that it's > defining push_back in terms of insertion, and for the latter, you need > operator= to move elements subsequent elements to make room for a new > element. However in the case of push_back, that's really unnecessary, > and no copying of objects other than copy-constructing into new memory > when the vector's underlying storage is reallocated should be need. > > Opinions? Is the error here reasonable? The error is very reasonable, because 1- The standard states that the element-types used for std::vector must be copy-constructable *and* assignable. 2- The standard describes the operation of push_back in terms of insert So, without the user defined operator=, your type is not assignable and thus does not meet the minimum requirements for being used as element-type in a std::vector. Doing so anyway results in a diagnostic at best and UB at worst. And, as the operation of v.push_back(x) is equivalent to v.insert(v.end (), x), it is only logical to implement one in terms of the other. > > Thanks, > > -Miles > Bart v Ingen Schenau -- [ See http://www.gotw.ca/resources/clcm.htm for info about ] [ comp.lang.c++.moderated. First time posters: Do this! ] |