Are these vector iteration forms the same?

Question

Form 1:

int main()
{
    std::vector<int> array{1, 2, 3, 4, 5};   

    for(auto i = array.begin(); i != array.end();) {      
        if(*i == 2 || *i == 5) {
            i = array.erase(i);
        } else {          
            i++;
        }
    }   
}

Form 2:

int main()
{
    std::vector<int> array{1, 2, 3, 4, 5};   

    for(auto i = array.begin(); i != array.end(); i++) {
        if(*i == 2 || *i == 5) {
            i-- = array.erase(i);
        }
    }   
}

Are these two form identical? Could I go into problems using one or another? (i.e. with every kind of object, such as linked list?).

Answer 1

this one, form 3:

#include <vector>
#include <algorithm>

int main()
{
    std::vector<int> array{1, 2, 3, 4, 5};

    auto is_2_or_5 = [](int i) {
        return i == 2 || i == 5;
    };

    array.erase(std::remove_if(array.begin(),
                               array.end(),
                               is_2_or_5),
                array.end());

}

Answer 2

i-- = array.erase(i);

Is undefined behavior both in pre C++11 and C++11.

For more gory details check out:

• Undefined behavior and sequence points
• Unsequenced value computations (a.k.a sequence points)
• especially this: Order of evaluation and undefined behaviour

Even if it was well defined I wouldn't use the i-- construct for anything else than a standalone expression. It is difficult to understand what's happening.

Answer 3

Form 2 is broken.

i-- = array.erase(i);

Let's break that down;

i--  

This returns a copy of i, and then decrements i.

array.erase(i) 

This erases the element at i, and then returns the iterator to the element after.

(i--) = (array.erase(i));

This assigns the result of array.erase(i) to the copy returned by i--.

There is an additional complication in that certainly prior to C++11, you don't know whether the decrement was done before or after the call to array.erase(i), and that would have been undefined behaviour. Experimentally, GCC does the decrement before the erase, so you end up erasing the wrong element.

It may help to write the (nearly) equivalent code out:

    auto j = i;
    --i;          // Here??
    auto k = array.erase(i);
    --i;          // Or here?
    j = k;

If you want to do this, write it as:

    const auto j = i;
    --i;
    array.erase(j);

Which makes it much clearer what you are doing.