views:

660

answers:

3

Suppose I have a hash_map and a code like

// i is an iterator
i = hash_map.erase(i)

But GCC's STL doesn't return iterator in erase, but a void. Now is a code like

hash_map.erase(i++)

safe (i.e. does not invalidate the iterator or does any other unexpected or unpleasant things)? Please note this is a hash_map.

+4  A: 

Yes, this is safe, because the value of i will have been set to the next value, before the current value is erased.

According to the SGI documentation about hashed containers invalidation does not occur for non-erased elements, nor even for resizing (there is no word on whether insertions cause resizing, so to be careful I admit that as a possibility)---but in the latter case, the iteration order will be changed. But this doesn't apply here, unless you go out of your way to resize the container during traversal or something. :-)

Chris Jester-Young
Can you point out where you read that? This is true for associative container as defined in the ISO C++, but I never read anything like that in the SGI documentation.
PierreBdR
It's on that page linked to in my post. I do assume, for that site, that unless specifically mentioned otherwise, operations do not cause invalidation. See http://www.sgi.com/tech/stl/Vector.html (for example) where all the invalidation cases are mentioned.
Chris Jester-Young
(I picked vector specifically because that's a data type that has (seemingly) more cases of invalidation than any other.)
Chris Jester-Young
A: 

Hate to rain on the parade, but I don't think what you propose is safe.

i++ is the post-increment operator, which means i is incremented after the call to erase. But erase invalidates all iterators pointing to the element being erased. So by the time i is incremented it's not valid any more.

If you're lucky it may work correctly by accident until one day it doesn't any more.

As far as I'm aware there is no way around this but something like:

// tmp and i are both iterators
tmp = i;
++i;
hash_map.erase(tmp);
Rodyland
According to the standard, it++ and it-- (where it is an iterator) are equivalent to creating a temp equal to it, decrementing the iterator, and returning the temp.
hazzen
Yes, but the compiler is free to call (i++) before or after the call to erase, so if the compiler decides to do (i++) after erase then i is invalid.
Rodyland
Rodyland, do you have any citation for that claim? I have never heard that.
rlbond
I've done further reading and it seems I was mistaken - the compiler is forced to evaluate (i++) before calling erase, so it's safe. That said, I still don't like it.
Rodyland
I agree that this is an item not very safe, but the use of the increment operator is defined in this case. More on this same subject:http://stackoverflow.com/questions/436013/is-there-any-way-to-check-if-an-iterator-is-valid
lsalamon
+1  A: 

You can encapsulate erasing to provide the same interface for all containers you use:

namespace detail {
template<typename Container, typename R>
struct SelectErase {
  // by default, assume the next iterator is returned
  template<typename Iterator>
  Iterator erase(Container& c, Iterator where) {
    return c.erase(where);
  }
};
// specialize on return type void
template<typename Container>
struct SelectErase<Container, void> {
  template<typename Iterator>
  Iterator erase(Container& c, Iterator where) {
    Iterator next (where);
    ++next;
    c.erase(where);
    return next;
  }
};

template<typename I, typename Container, typename R>
SelectErase<Container,R> select_erase(R (Container::*)(I)) {
  return SelectErase<Container,R>();
}
} // namespace detail

template<typename Container, typename Iterator>
Iterator erase(Container& container, Iterator where) {
  return detail::select_erase<Iterator>(&Container::erase).erase(container, where);
}

This requires either:

  1. c.erase returns the iterator for the next item. This is how vector, deque, and list work.
  2. c.erase returns void and does not invalidate the next iterator. This is how map, set, and (non-stdlib) hash_map work.
Roger Pate