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.

You're erasing by key, so you could just as well invoke `s.erase(-8)` and be done with it. The first for-loop is pointless. — WhozCraig, Aug 08 '18 at 10:19
@WhozCraig this is just a POC example ... the if (it ==-8) part is just an example to pick some arbitrary element for deletion. — OrenIshShalom, Aug 08 '18 at 10:27
Ok, so is your *real* question whether using container modification during ranged-for enumeration on a `std::set` valid in C++17 ? That problem was solved in C++11 using iterators once `std::set::erase` actually started returning one, but you want to know if it is also supported with ranged-for ? I don't think it is. — WhozCraig, Aug 08 '18 at 10:31
The big caveat with C++ is that "everything goes perfect" does not imply that there is no undefined behaviour. (As Dijkstra said, testing can only show the presence of bugs, not their absence.) — molbdnilo, Aug 08 '18 at 10:54

Aconcagua · Accepted Answer · 2018-08-08T10:49:39.387

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);

score 0 · Answer 2 · answered Aug 08 '18 at 11:33

If your C++ implementation supports the Library Fundamentals TS (second edition) you can do:

#include <experimental/set>
#include <iostream>
#include <algorithm>
#include <experimental/iterator>
int main()
{
    std::set<int> s;

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

    std::experimental::erase_if(s,
                                [](auto& key){ return key == -8; });
    std::cout << "s = {";
    std::copy(s.begin(), s.end(),
              std::experimental::make_ostream_joiner(std::cout, " "));
    std::cout << "}\n";
}

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

2 Answers2