What happens to an STL iterator after erasing it in VS, UNIX/Linux?

Question

Please consider the following scenario:


map(T,S*) & GetMap(); //Forward decleration

map(T, S*) T2pS = GetMap();

for(map(T, S*)::iterator it = T2pS.begin(); it != T2pS.end(); ++it)
{
    if(it->second != NULL)
    {
        delete it->second;
        it->second = NULL;
    }
    T2pS.erase(it);
    //In VS2005, after the erase, we will crash on the ++it of the for loop.
    //In UNIX, Linux, this doesn't crash.
}//for

It seems to me that in VS2005, after the "erase", the iterator will be equal to end(), hence the crash while trying to increment it. Are there really differences between compilers in the behavior presented here? If so, what will the iterator after the "erase" equal to in UNIX/Linux?

Thanks...

Asked and answered: http://stackoverflow.com/questions/263945/what-happens-if-you-call-erase-on-a-map-element-while-iterating-from-begin-to-e — Martin York, Jan 11 '09 at 17:24

Johannes Schaub - litb · Accepted Answer · 2009-01-12T20:42:50.670

23

Yes, if you erase an iterator, that iterator gets a so-called singular value, which means it doesn't belong to any container anymore. You can't increment, decrement or read it out/write to it anymore. The correct way to do that loop is:

for(map<T, S*>::iterator it = T2pS.begin(); it != T2pS.end(); T2pS.erase(it++)) {
    // wilhelmtell in the comments is right: no need to check for NULL. 
    // delete of a NULL pointer is a no-op.
    if(it->second != NULL) {
        delete it->second;
        it->second = NULL;
    }
}

For containers that could invalidate other iterators when you erase one iterator, erase returns the next valid iterator. Then you do it with

it = T2pS.erase(it)

That's how it works for std::vector and std::deque, but not for std::map or std::set.

edited Jan 12 '09 at 20:42

answered Jan 11 '09 at 17:00

Johannes Schaub - litb

496,577
130
894
1,212

Correct me if I'm wrong, but there's no difference between: it = T2pS.erase(it++); to the way it's written in the for loop in the question, because in both cases "it" will be invalid by the time you ++. – Gal Goldman Jan 12 '09 at 07:39
Ok, I'll correct you 'cause you're wrong ;-) in erase(it++) the argument is evaluated before the erase is called, it will be (correctly) incremented, it's previous (unincremented) value is returned to erase and that one becomes invalid, but it itself already points to the next element. – Pieter Jan 12 '09 at 08:56
Deleting a null pointer is a noop. There is no need to check for null. – wilhelmtell Jan 12 '09 at 20:20
you might mention that std::list doesn't invalidate any iterator other than the iterator(s) pointing to the erased element(s), when calling the member erase() function. – wilhelmtell Jan 12 '09 at 20:23
wilhelmtell, i thought it would put only more confusion in the game if i mention more containers. your point about deleting a nullpointer is good, imho. ill add it – Johannes Schaub - litb Jan 12 '09 at 20:41
In delete it->second; aren't you deleting the post incremented iterator pointing to second and I think you want to delete the 'it' before the post incremented value. – vkaul11 Apr 04 '13 at 02:54
for deque, how about deque.erase(itor++)? – Wallace Jul 07 '14 at 07:19

score 5 · Answer 2 · answered Jan 11 '09 at 16:58

After you call erase on an iterator into a std::map, it is invalidated. This means that you cannot use it. Attempting to use it (e.g. by incrementing it) is invalid and can cause anything to happen (including a crash). For a std::map, calling erase on an iterator does not invalidate any other iterator so (for example) after this call, (so long as it was not T2pS.end()), it will be valid:

T2pS.erase( it++ );

Of course, if you use this approach, you won't want to unconditionally increment it in the for loop.

For this example, though, why bother to erase in the for loop? Why not just call T2pS.clear() at the end of the loop.

On the other hand, it looks like you have a raw pointer 'on the right' of the map, but the map appears to own the pointed to object. In this case, why not make the thing on the right of the map some sort of smart pointer, such as std::tr1::shared_ptr?

[Incidentally, I don't see any template parameters to map. Have you typedef'ed a specific instantiation of std::map as map in the local namespace?]

score 0 · Answer 3 · answered Jan 11 '09 at 16:44

0

See this:

for (i = v.begin(); i != v.end(); ) {
  //...
  if (erase_required) {
      i = v.erase(i);
  } else {
      ++i;
  }
}

answered Jan 11 '09 at 16:44

orip

73,323
21
116
148

Isn't the type signature void erase(iterator pos) ? – A. Rex Jan 11 '09 at 16:46
Aha! vector::erase(iterator) returns another iterator, while map::erase(iterator) does not. The OP discussed maps, but this is useful information also. – A. Rex Jan 11 '09 at 16:50
In Visual C++, map::erase(iterator) returns an iterator, but this is nonstandard. – ChrisN Jan 11 '09 at 16:55

score 0 · Answer 4 · answered Jan 11 '09 at 16:46

0

I think if you modify the collection you invalidate your iterator. You can't rely on the behavior, as you found out.

answered Jan 11 '09 at 16:46

Jonathan Adelson

3,275
5
29
39

1

Containers never invalidate iterators when an element's value is changed. When an element is inserted or deleted, some container types (e.g. vector) will invalidate iterators pointing at other elements, while most (e.g. set, map, list) will not. – j_random_hacker Jan 11 '09 at 18:44

What happens to an STL iterator after erasing it in VS, UNIX/Linux?

4 Answers4

Linked

Related