Search code examples
c++for-looppush-back

Why is it unsafe to call push_back in a for loop even when there is no memory reallocation?


When I read this post: https://stackoverflow.com/a/42448319/3336423

I understand, that calling push_back in a for loop is unsafe because:

If the new size() is greater than capacity() then all iterators and references (including the past-the-end iterator) are invalidated. Otherwise only the past-the-end iterator is invalidated.

Then, I assume that if I guarantee the capacity won't be exceeded, it would be safe....

But apparently not, all implementations below will crash after first push_back is called (using Visual Studio), even if I can see that capacity remains unchanged within the loop (so I assume the vector does not reallocate its memory):

Version1:

std::vector<int> v1{ 3, 4, 5 };
v1.reserve( v1.size()*2 );
size_t c1 = v1.capacity();
for ( auto val : v1 )
{
    v1.push_back( val );
    c1 = v1.capacity();
}

Version2:

std::vector<int> v2{ 3, 4, 5 };
v2.reserve( v2.size()*2 );
size_t c2 = v2.capacity();
auto curEnd = v2.end();
for ( auto iter = v2.begin(); iter != curEnd; ++iter )
{
    v2.push_back( *iter );
    c2 = v2.capacity();
}

Version3:

std::vector<int> v3{ 3, 4, 5 };
v3.reserve( v3.size()*2 );
size_t c3 = v3.capacity();
for ( auto iter = v3.begin(); iter != v3.end(); ++iter )
{
    v3.push_back( *iter );
    c3 = v3.capacity();
}

What makes those code crash?


Solution

  • Both your first two versions have the same problem. The cached past-the-end iterator is invalidated after the first insertion, making any subsequent use of it UB. Yes, even just comparing against it.

    Your third sample crashes because it ends up trying to re-insert the freshly inserted elements. So it ends up needing a reallocation, which ends up causing more UB.

    The correct iterator-based approach is to not go until one past the end. But until the end, assuming the vector is not empty.

    std::vector<int> v{ 3, 4, 5 };
    v.reserve( v.size()*2 );
    
    auto it = v.begin(), end = v.end();
    --end;
    do {
      v.push_back(*it);
    } while (it++ != end); // Check against current position, but increment still