Prev: const as default
Next: CRTP and typedef
From: Andrea Venturoli on 1 Feb 2010 04:04
On 02/01/10 07:04, Miles Bader wrote:

> But it somehow seems wrong that it's using operator= at all; shouldn't a
> copy-constructor be enough?
>...
>
> Opinions? Is the error here reasonable?

Well, according to the standard's draft, 23.1: Container requirements:

"The type of objects stored in these components must meet the
requirements of CopyConstructible types, and the additional
requirements of Assignable types".

Stangely, the STL docs, just require value_type to be Assignable, not
Copyable.

So, IMVHO, your idea of operator= being enough might possibly stand, but
this is not the way the library was designed.


(Just my two cents)


bye
av.

--
[ See http://www.gotw.ca/resources/clcm.htm for info about ]
[ comp.lang.c++.moderated. First time posters: Do this! ]

From: Johannes Schaub (litb) on 1 Feb 2010 04:04
Miles Bader 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?
>

It's completely reasonable. You are causing undefined behavior, because the
type T you use with the container is required to be Assignable. GCC will
have a good reason for using "insert" i think, and it's allowed to assume
your type is Assignable.

If you insert at the end, there is no need to move any elements subsequent i
think, so there is no performance penalty either.


--
[ See http://www.gotw.ca/resources/clcm.htm for info about ]
[ comp.lang.c++.moderated. First time posters: Do this! ]

First  |  Prev  | 
Pages: 1 2
Prev: const as default
Next: CRTP and typedef