Removing an element from a std::set while iterating over it in C++17

Question

I've read this SO post, and this one too regarding the erasure of elements from a std::set during iteration. However, it seems that a simpler solution exists in C++17:

#include <set>
#include <iostream>
int main(int argc,char **argv)
{
    std::set<int> s;

    s.insert(4);
    s.insert(300);
    s.insert(25);
    s.insert(-8);

    for (auto it:s)
    {
        if (it == -8)
        {
            s.erase(it);
        }
    }
    std::cout << "s = {";
    for (auto it:s)
    {
        std::cout << it << " ";
    }
    std::cout << "}\n";
    return 0;
}

When I compile and run it everything goes perfect:

$ g++ -o main main.cpp
$ ./main
s = {4 25 300 }

Are there any caveats in erasing elements like that? thanks.

Answer 1

According to C++17 standard:

9.5.4 The range-based for statement [stmt.ranged]

1 The range-based for statement

for ( for-range-declaration : for-range-initializer ) statement

is equivalent to

{
    auto &&__range = for-range-initializer ;
    auto __begin = begin-expr ;
    auto __end = end-expr ;
    for ( ; __begin != __end; ++__begin )
    {
        for-range-declaration = *__begin;
        statement
    }
}

So no, your code is not valid, as you erase the element the iterator is currently pointing to (std::set can have only one value for the same key!), thus the iterator gets invalidated and is incremented afterwards, which is undefined behaviour.

Be aware that you could erase another element from set, as in std::set (as well as in std::map or std::list) only the iterator erased is invalidated whereas all others remain valid.

If you intend to remove the current element of a container (including std::vector, as erase returns a new, valid iterator), you need to fall back to a classic loop, as shown in the answer to referenced question; I personally like a one-liner variant of:

    iter = /*some condition*/ ? container.erase(iter) : std::next(iter);