vector::push_back has wrong type_traits op

[Malcolm Davey]

The vector::push_back has the wrong type_traits optimization code. It tries to optimize the case of reallocation based on whether a class has a trivial assignment operator or not. the reallocation only uses the copy constructor hence should optimise on whether the copy constructor is trivial or not. Currently if only the copy constructor is implemented and the assignment operator is not implemented and not used, then the behavior will be incorrect.

(in stlport _vector.h)

  void push_back(const _Tp& __x) {
    if (this->_M_finish != this->_M_end_of_storage._M_data) {
      _Copy_Construct(this->_M_finish, __x);
      ++this->_M_finish;
    }
    else {
    typedef typename __type_traits<_Tp>::has_trivial_assignment_operator _TrivialCopy;
      _M_insert_overflow(this->_M_finish, __x, _TrivialCopy(), 1UL, true);
    }
  }

The last two lines should be replaced by:
     typedef typename __type_traits<_Tp>::has_trivial_copy _constructor_operator _TrivialUCopy;
      _M_insert_overflow(this->_M_finish, __x, _TrivialUCopy(), 1UL, true);

Part of the original confusion could be the naming of the types "_TrivialCopy" for "assignment" operator. Compare with the names for deque which make sense.

Another confusion is that vector::insert() can do both assignment and copy construction, but in the case of reallocation there is only copy construction. push_back will never need assignment operator. If one wants to test they can put an assert in an assignment operator and see if it gets called.

void _M_fill_insert (iterator __pos, size_type __n, const _Tp& __x); looks like is has the same error. There may be others as well.

[Petr Ovtchenkov]

Looks like you speak about 5.2. This was re-written in mainline. I hope, that after unit tests conversion will finish and (then) will be more tests, this will be reviewed again.

[Malcolm Davey]

Yes the issue is in 5.2 and at least as far back as 5.1.5.
(We had previously modified our 5.1.5 to be the same in effect as 5.2 to get around the issue of forward declarations of class used in the vector)

So it has been fixed?
I'm not sure if we can upgrade to the new version at this stage. We might just need to apply patches.

So far it seems that push_back and insert are wrong - when there is a reallocation.

[Alex]

Is it fixed in 5.2.1 ?

[mickstl]

malcolmd,

Your suggested fix for two lines are incomplete. The fix mentioned causes more compile problems.