2

While using set in c++ i wrote the following code.It just pushes the elements from 1 to 50 into two sets s1 and s2, and after for both the sets i iterate from the same value to print values till a fixed limit. But i am unable to understand why i am getting different outputs for s1 and s2 since the only difference is s1.erase(it++) is written as {s2.erase(it);it++;}.

#include<bits/stdc++.h>
using namespace std;
set<int> s1,s2;
int main()
{
    int l,r,i;
    for(i=1;i<=50;i++)
    {       s1.insert(i);
            s2.insert(i);
    }
    //scanf("%d%d",&l,&r);
    l=3;r=9;

    printf("Output 1:\n");
    set<int>::iterator it=s1.lower_bound(l);

    while(it!=s1.end() && (*it<=r))
    {
            printf("Before deletion: %d\n",*it);
            s1.erase(it++);
            printf("After deletion: %d\n",*it);
    }

    it=s2.lower_bound(l);
    printf("Output 2\n");
    while(it!=s2.end() && (*it<=r))
    {
            printf("Before deletion: %d\n",*it);
            s2.erase(it);
            it++;
            printf("After deletion: %d\n",*it);
    }
    return 0;
  }

The output is:

 Output 1:
Before deletion: 3
After deletion: 4  
Before deletion: 4
After deletion: 5
Before deletion: 5
After deletion: 6
Before deletion: 6
After deletion: 7
Before deletion: 7
After deletion: 8
Before deletion: 8 
After deletion: 9
Before deletion: 9
After deletion: 10
Output 2
Before deletion: 3
After deletion: 2
Before deletion: 2
After deletion: 4
Before deletion: 4
After deletion: 6
Before deletion: 6
After deletion: 7
Before deletion: 7
After deletion: 5
Before deletion: 5
After deletion: 8
Before deletion: 8
After deletion: 10
nirvan
  • 23
  • 2
  • 2
    `#include` -- Instead of this, specify the standard include headers. – PaulMcKenzie Mar 22 '17 at 13:42
  • Have a look at http://stackoverflow.com/questions/6438086/iterator-invalidation-rules – cup Mar 22 '17 at 13:46
  • FYI, running your code using Visual Studio 2015 (debug), the code crashes at the line pointed out by the answer given by @BobTFish. So the "output" doesn't even get as far as what you're showing (which is what undefined behavior is all about). – PaulMcKenzie Mar 22 '17 at 13:48

1 Answers1

5
s2.erase(it);
it++;

Simple. This is illegal. Once you have erased that node, you aren't allowed to use the iterator, even to increment it. You have Undefined Behaviour.

Undefined Behaviour is nasty business. You code could crash, do something odd, or even appear to work normally. This last case is the worst, because it could break at some unknown point in the future.

Although the language itself doesn't define the details, the practical reason is that this is implemented as a binary tree (Red-Black tree), so each node contains pointers to its left and right children, and possibly its parent. These pointers are used to move through the tree. Once the node has been deleted, you can't possibly expect it to work when you inspect those pointers.

In the other case: s1.erase(it++); you increment the iterator before eraseing it, so you can then happily continue looking at the rest of the container.

Another possibility would be it = s2.erase(it);, since erase returns the next iterator (note this could be end()). This is a more common/idiomatic pattern, and should be preferred for that reason alone, but it also works well for other container types (your versions do not).

The iterator invalidation rules are different for different types of containers, so make sure you carefully read the documentation. (Note this isn't really official documentation, there is only The Standard for that, but this site is very popular).


I'd also recommend you include the standard headers you actually use (e.g. <set>), and avoid using namespace std;.

Community
  • 1
  • 1
BoBTFish
  • 17,936
  • 3
  • 49
  • 73
  • 1
    Side note: Be aware, though, that this applies for sets/maps/lists, but not for vectors, where all iterators pointing to elements after the erased one get invalidated! – Aconcagua Mar 22 '17 at 13:45
  • 2
    it should be `it = s2.erase(it);` please add also a link to docs: http://en.cppreference.com/w/cpp/container/set/erase – Raxvan Mar 22 '17 at 13:46
  • @Raxvan Ha, you've been faster than me - just wanted to add this to my previous comment... – Aconcagua Mar 22 '17 at 13:47
  • `it = s2.erase(it);` is idiomatic. It should be strongly promoted in the answer. – Richard Hodges Mar 22 '17 at 14:34