47

Does it works correct(does nothing) when I use

 vector<T> v;
 v.erase(v.end());

I want to use something like

 v.erase(std::find(...));

Should I if is it v.end() or not?
There is no info about it on C++.com and CPPreference

jww
  • 97,681
  • 90
  • 411
  • 885
RiaD
  • 46,822
  • 11
  • 79
  • 123

3 Answers3

40

The standard doesn't quite spell it out, but v.erase(q) is defined, "Erases the element pointed to by q" in [sequence.reqmts]. This means that q must actually point to an element, which the end iterator doesn't. Passing in the end iterator is undefined behavior.

Unfortunately, you need to write:

auto it = std::find(...);
if (it != <the part of ... that specifies the end of the range searched>) {
    v.erase(it);
}

Of course, you could define:

template typename<Sequence, Iterator>
Iterator my_erase(Sequence &s, Iterator it) {
    if (it == s.end()) return it;
    return s.erase(it);
}

my_erase(v, std::find(v.begin(), v.end(), whatever));

c.erase() on an associative container returns void, so to generalize this template to all containers you need some -> decltype action.

ceztko
  • 14,736
  • 5
  • 58
  • 73
Steve Jessop
  • 273,490
  • 39
  • 460
  • 699
  • [docs](http://www.cplusplus.com/reference/vector/vector/erase/) says that " erasing elements in positions other than the vector end causes the container to relocate...". It looks like alowing of end() as parameter. And nowhere said the opposite explicitly. I dont like this... – Pavel Jan 08 '14 at 18:13
  • 1
    @Pavel: then you'll have to take it up with the authors of "cplusplus.com". It is *not* the C++ documentation, the standard is the C++ documentation. But it defines `position` as "Iterator pointing to a single element". An end iterator does not point to a single element. – Steve Jessop Jan 09 '14 at 10:47
  • 1
    @Pavel They've got it wrong. It should say `end() - 1` instead of "vector end". – Emil Laine Jul 26 '15 at 03:22
29

Erasing end() (or for that matter, even looking at the target of end()) is undefined behavior. Undefined behavior is allowed to have any behavior, including "just work" on your platform. That doesn't mean that you should be doing it; it's still undefined behavior, and I'll come bite you in the worst ways when you're least expecting it later on.

Depending on what you're doing, you might want to consider set or unordered_set instead of vector here.

Billy ONeal
  • 104,103
  • 58
  • 317
  • 552
  • thanks, i do know what is UB, I just was wanted to know, is it really UB. – RiaD Mar 06 '12 at 19:12
  • @RiaD: Yes, it is UB. The solution is very simple, though, just check before you erase: `{ auto it = v.find(x); if (it != x.end()) { v.erase(it); } }` – Kerrek SB Mar 06 '12 at 19:13
  • Question for you @Billy. Out of curiosity, does end()-1 work? How is this different from pop_back()? – Gaffi Mar 06 '12 at 19:13
  • 3
    @Gaffi: `end-1` will work if and only if the container is not empty. (Same as `pop_back`) – Billy ONeal Mar 06 '12 at 19:14
  • @KerrekSB, yes I know I can check, it's quite easy:D. It just some ugly and I was thinking about replacing it:) – RiaD Mar 06 '12 at 19:15
  • @Billy, set and even multiset is not OK for me. deletion will very rare operation. – RiaD Mar 06 '12 at 19:16
  • @RiaD: Ok; just wanted to throw it out there. (Most often when I see people asking for this they're treating vectors as sets) – Billy ONeal Mar 06 '12 at 19:18
7

Have you tried this?

v.erase(remove_if(v.begin(), v.end(), (<your criteria>)), v.end());
Billy ONeal
  • 104,103
  • 58
  • 317
  • 552
Gaffi
  • 4,307
  • 8
  • 43
  • 73