0

I will preface that I'm fairly new to C++ in general. Recently, I ran into some perplexing behavior from the std::set.erase() method, which I have isolated into the following code. This code sample crashes with a segmentation fault when it hits the second for loop:

#include <set>
#include <iostream>

int main() {
    std::set<int> foo = {0, 1, 2};
    std::set<int> bar = {0, 1, 2, 3, 4};
    for(int i : foo) {
        printf("%d\n", i);
    }
    bar.erase(foo.begin(), foo.end());
    for(int i : foo) { //Crash happens right here, before entering the body of the loop.
        printf("%d\n", i);
    }
    return 0;
}

Whereas if you remove the call to erase() it does not crash.

This is somewhat surprising behavior. Calling bar.erase() is obviously supposed to modify "bar" but I would not generally expect it to have any impact on how "foo" functions. What does std::set.erase() do that causes a segmentation fault to occur?

This is simple enough to bypass just by creating a copy of "foo" to supply start and end iterators to erase(), but I am curious why this behavior happens in the first place.

trevorKirkby
  • 1,886
  • 20
  • 47

2 Answers2

1

The arguments to the erase function are iterators. Iterators are not portable across objects. The iterators that you pass to bar.erase are supposed to be iterators on bar. If they're not, you get undefined behavior.

The crash probably happened because the call to bar.erase somehow got rid of the contents of foo, because it used foo-based iterators, but foo thought they were still there.

You should not create a copy of foo to provide iterators to bar.erase. You should use iterators based on bar.

Dennis Sparrow
  • 938
  • 6
  • 13
  • Thank you, that explains it. Aren't iterators *sometimes* portable across objects, though? The std::set.insert() method also lets you supply iterators as arguments, which I presume are *supposed* to be iterators from a different container, because re-adding elements from a set back into that set is not useful. – trevorKirkby Jun 03 '20 at 04:45
  • @trevorKirkby, Those iterators are used for the _values_ in that range, not the range itself. For example, you can tell `insert` to insert every value in a specific subrange of a vector or you can tell `erase` to erase, say, the 2nd through 5th elements. But you can't do vice-versa for either. – chris Jun 03 '20 at 05:46
  • Good question. I think I overgeneralized. In the case of the set::erase operation specifically, the iterators must refer to the same container, but in other cases, an iterator may be anything that supports some specified set of operations, where the required set of operations depends on the specific flavor of iterator. – Dennis Sparrow Jun 03 '20 at 05:50
1

You're passing iterators from foo into the erase call on bar. The iterator-pair overload of erase erases the range denoted by the pair, not every value that appears within that range. Therefore, it works only if you pass a valid range of the set you're calling erase on. If you'd like to remove all elements from the other set, you can use std::set_difference:

bar = [&] {
    std::set<int> result;
    std::set_difference(bar.begin(), bar.end(), foo.begin(), foo.end(), std::inserter(result));
    return result;
}();
chris
  • 60,560
  • 13
  • 143
  • 205